Skip to content

Commit

Permalink
Bridge relayer backwards compatibility for reading storage InboundLan…
Browse files Browse the repository at this point in the history 
…eData/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`.
  • Loading branch information
@bkontur
bkontur authored Oct 4, 2024
1 parent 1344a21 commit 0e68c6f
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 8 deletions.
52 changes: 49 additions & 3 deletions bridges/relays/lib-substrate-relay/src/messages/source.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -63,6 +63,18 @@ use std::ops::RangeInclusive;
pub type SubstrateMessagesProof<C, L> = (Weight, FromBridgedChainMessagesProof<HashOf<C>, 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<P: SubstrateMessageLane, SourceClnt, TargetClnt> {
source_client: SourceClnt,
Expand Down Expand Up @@ -98,7 +110,7 @@ impl<P: SubstrateMessageLane, SourceClnt: Client<P::SourceChain>, TargetClnt>
async fn outbound_lane_data(
&self,
id: SourceHeaderIdOf<MessageLaneAdapter<P>>,
) -> Result<Option<OutboundLaneData>, SubstrateError> {
) -> Result<Option<LegacyOutboundLaneData>, SubstrateError> {
self.source_client
.storage_value(
id.hash(),
Expand Down Expand Up @@ -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);
}
}
97 changes: 92 additions & 5 deletions bridges/relays/lib-substrate-relay/src/messages/target.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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<C, L> =
(UnrewardedRelayersState, FromBridgedChainMessagesDeliveryProof<HashOf<C>, 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<RelayerId> {
relayers: VecDeque<UnrewardedRelayer<RelayerId>>,
last_confirmed_nonce: MessageNonce,
}
impl<RelayerId> Default for LegacyInboundLaneData<RelayerId> {
fn default() -> Self {
let full = bp_messages::InboundLaneData::default();
Self { relayers: full.relayers, last_confirmed_nonce: full.last_confirmed_nonce }
}
}

impl<RelayerId> LegacyInboundLaneData<RelayerId> {
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<RelayerId> From<LegacyInboundLaneData<RelayerId>> for UnrewardedRelayersState {
fn from(value: LegacyInboundLaneData<RelayerId>) -> 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<P: SubstrateMessageLane, SourceClnt, TargetClnt> {
target_client: TargetClnt,
Expand Down Expand Up @@ -94,7 +135,7 @@ where
async fn inbound_lane_data(
&self,
id: TargetHeaderIdOf<MessageLaneAdapter<P>>,
) -> Result<Option<InboundLaneData<AccountIdOf<P::SourceChain>>>, SubstrateError> {
) -> Result<Option<LegacyInboundLaneData<AccountIdOf<P::SourceChain>>>, SubstrateError> {
self.target_client
.storage_value(
id.hash(),
Expand Down Expand Up @@ -217,8 +258,8 @@ where
) -> Result<(TargetHeaderIdOf<MessageLaneAdapter<P>>, 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(
Expand Down Expand Up @@ -321,3 +362,49 @@ fn make_messages_delivery_call<P: SubstrateMessageLane>(
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::<u8> {
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<u8> =
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<u8> =
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());
}
}

0 comments on commit 0e68c6f

Please sign in to comment.