From 29e2dbf1e785945aeb011b1292e8d571d0e05707 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Fri, 4 Oct 2024 18:57:53 +0200 Subject: [PATCH] Bridge relayer backwards compatibility for reading storage InboundLaneData/OutboundLaneData (#5921) For permissionless lanes, we add `lane_state` to the `InboundLaneData` and `OutboundLaneData` structs. However, for a period of time (until both BHK and BHP are upgraded to the same version), we need the relayer to function with runtimes where one has been migrated with `lane_state` and the other has not. This PR addresses the incompatibility by introducing wrapper structs for decoding without `lane_state`. --- .../src/messages/source.rs | 52 +++++++++- .../src/messages/target.rs | 97 ++++++++++++++++++- 2 files changed, 141 insertions(+), 8 deletions(-) diff --git a/bridges/relays/lib-substrate-relay/src/messages/source.rs b/bridges/relays/lib-substrate-relay/src/messages/source.rs index b560867a235b..3e60ed7abd09 100644 --- a/bridges/relays/lib-substrate-relay/src/messages/source.rs +++ b/bridges/relays/lib-substrate-relay/src/messages/source.rs @@ -35,10 +35,10 @@ use bp_messages::{ storage_keys::{operating_mode_key, outbound_lane_data_key}, target_chain::FromBridgedChainMessagesProof, ChainWithMessages as _, InboundMessageDetails, MessageNonce, MessagePayload, - MessagesOperatingMode, OutboundLaneData, OutboundMessageDetails, + MessagesOperatingMode, OutboundMessageDetails, }; use bp_runtime::{BasicOperatingMode, HeaderIdProvider, RangeInclusiveExt}; -use codec::Encode; +use codec::{Decode, Encode}; use frame_support::weights::Weight; use messages_relay::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, @@ -63,6 +63,18 @@ use std::ops::RangeInclusive; pub type SubstrateMessagesProof = (Weight, FromBridgedChainMessagesProof, L>); type MessagesToRefine<'a> = Vec<(MessagePayload, &'a mut OutboundMessageDetails)>; +/// Outbound lane data - for backwards compatibility with `bp_messages::OutboundLaneData` which has +/// additional `lane_state` attribute. +/// +/// TODO: remove - https://github.com/paritytech/polkadot-sdk/issues/5923 +#[derive(Decode)] +struct LegacyOutboundLaneData { + #[allow(unused)] + oldest_unpruned_nonce: MessageNonce, + latest_received_nonce: MessageNonce, + latest_generated_nonce: MessageNonce, +} + /// Substrate client as Substrate messages source. pub struct SubstrateMessagesSource { source_client: SourceClnt, @@ -98,7 +110,7 @@ impl, TargetClnt> async fn outbound_lane_data( &self, id: SourceHeaderIdOf>, - ) -> Result, SubstrateError> { + ) -> Result, SubstrateError> { self.source_client .storage_value( id.hash(), @@ -743,4 +755,38 @@ mod tests { Ok(vec![2, 4, 3]), ); } + + #[test] + fn outbound_lane_data_wrapper_is_compatible() { + let bytes_without_state = + vec![1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0]; + let bytes_with_state = { + // add state byte `bp_messages::LaneState::Opened` + let mut b = bytes_without_state.clone(); + b.push(0); + b + }; + + let full = bp_messages::OutboundLaneData { + oldest_unpruned_nonce: 1, + latest_received_nonce: 2, + latest_generated_nonce: 3, + state: bp_messages::LaneState::Opened, + }; + assert_eq!(full.encode(), bytes_with_state); + assert_ne!(full.encode(), bytes_without_state); + + // decode from `bytes_with_state` + let decoded: LegacyOutboundLaneData = Decode::decode(&mut &bytes_with_state[..]).unwrap(); + assert_eq!(full.oldest_unpruned_nonce, decoded.oldest_unpruned_nonce); + assert_eq!(full.latest_received_nonce, decoded.latest_received_nonce); + assert_eq!(full.latest_generated_nonce, decoded.latest_generated_nonce); + + // decode from `bytes_without_state` + let decoded: LegacyOutboundLaneData = + Decode::decode(&mut &bytes_without_state[..]).unwrap(); + assert_eq!(full.oldest_unpruned_nonce, decoded.oldest_unpruned_nonce); + assert_eq!(full.latest_received_nonce, decoded.latest_received_nonce); + assert_eq!(full.latest_generated_nonce, decoded.latest_generated_nonce); + } } diff --git a/bridges/relays/lib-substrate-relay/src/messages/target.rs b/bridges/relays/lib-substrate-relay/src/messages/target.rs index 0d1aac88a32a..214819a1c426 100644 --- a/bridges/relays/lib-substrate-relay/src/messages/target.rs +++ b/bridges/relays/lib-substrate-relay/src/messages/target.rs @@ -36,8 +36,9 @@ use async_std::sync::Arc; use async_trait::async_trait; use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, storage_keys::inbound_lane_data_key, - ChainWithMessages as _, InboundLaneData, MessageNonce, UnrewardedRelayersState, + ChainWithMessages as _, LaneState, MessageNonce, UnrewardedRelayer, UnrewardedRelayersState, }; +use codec::Decode; use messages_relay::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{NoncesSubmitArtifacts, TargetClient, TargetClientState}, @@ -48,12 +49,52 @@ use relay_substrate_client::{ }; use relay_utils::relay_loop::Client as RelayClient; use sp_core::Pair; -use std::{convert::TryFrom, ops::RangeInclusive}; +use std::{collections::VecDeque, convert::TryFrom, ops::RangeInclusive}; /// Message receiving proof returned by the target Substrate node. pub type SubstrateMessagesDeliveryProof = (UnrewardedRelayersState, FromBridgedChainMessagesDeliveryProof, L>); +/// Inbound lane data - for backwards compatibility with `bp_messages::InboundLaneData` which has +/// additional `lane_state` attribute. +/// +/// TODO: remove - https://github.com/paritytech/polkadot-sdk/issues/5923 +#[derive(Decode)] +struct LegacyInboundLaneData { + relayers: VecDeque>, + last_confirmed_nonce: MessageNonce, +} +impl Default for LegacyInboundLaneData { + fn default() -> Self { + let full = bp_messages::InboundLaneData::default(); + Self { relayers: full.relayers, last_confirmed_nonce: full.last_confirmed_nonce } + } +} + +impl LegacyInboundLaneData { + pub fn last_delivered_nonce(self) -> MessageNonce { + bp_messages::InboundLaneData { + relayers: self.relayers, + last_confirmed_nonce: self.last_confirmed_nonce, + // we don't care about the state here + state: LaneState::Opened, + } + .last_delivered_nonce() + } +} + +impl From> for UnrewardedRelayersState { + fn from(value: LegacyInboundLaneData) -> Self { + (&bp_messages::InboundLaneData { + relayers: value.relayers, + last_confirmed_nonce: value.last_confirmed_nonce, + // we don't care about the state here + state: LaneState::Opened, + }) + .into() + } +} + /// Substrate client as Substrate messages target. pub struct SubstrateMessagesTarget { target_client: TargetClnt, @@ -94,7 +135,7 @@ where async fn inbound_lane_data( &self, id: TargetHeaderIdOf>, - ) -> Result>>, SubstrateError> { + ) -> Result>>, SubstrateError> { self.target_client .storage_value( id.hash(), @@ -217,8 +258,8 @@ where ) -> Result<(TargetHeaderIdOf>, UnrewardedRelayersState), SubstrateError> { let inbound_lane_data = - self.inbound_lane_data(id).await?.unwrap_or(InboundLaneData::default()); - Ok((id, (&inbound_lane_data).into())) + self.inbound_lane_data(id).await?.unwrap_or(LegacyInboundLaneData::default()); + Ok((id, inbound_lane_data.into())) } async fn prove_messages_receiving( @@ -321,3 +362,49 @@ fn make_messages_delivery_call( trace_call, ) } + +#[cfg(test)] +mod tests { + use super::*; + use bp_messages::{DeliveredMessages, UnrewardedRelayer}; + use codec::Encode; + + #[test] + fn inbound_lane_data_wrapper_is_compatible() { + let bytes_without_state = + vec![4, 0, 2, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0]; + let bytes_with_state = { + // add state byte `bp_messages::LaneState::Opened` + let mut b = bytes_without_state.clone(); + b.push(0); + b + }; + + let full = bp_messages::InboundLaneData:: { + relayers: vec![UnrewardedRelayer { + relayer: Default::default(), + messages: DeliveredMessages { begin: 2, end: 5 }, + }] + .into_iter() + .collect(), + last_confirmed_nonce: 6, + state: bp_messages::LaneState::Opened, + }; + assert_eq!(full.encode(), bytes_with_state); + assert_ne!(full.encode(), bytes_without_state); + + // decode from `bytes_with_state` + let decoded: LegacyInboundLaneData = + Decode::decode(&mut &bytes_with_state[..]).unwrap(); + assert_eq!(full.relayers, decoded.relayers); + assert_eq!(full.last_confirmed_nonce, decoded.last_confirmed_nonce); + assert_eq!(full.last_delivered_nonce(), decoded.last_delivered_nonce()); + + // decode from `bytes_without_state` + let decoded: LegacyInboundLaneData = + Decode::decode(&mut &bytes_without_state[..]).unwrap(); + assert_eq!(full.relayers, decoded.relayers); + assert_eq!(full.last_confirmed_nonce, decoded.last_confirmed_nonce); + assert_eq!(full.last_delivered_nonce(), decoded.last_delivered_nonce()); + } +}