From 86b308274baafbffd8380f160effd500d162efb2 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 22 Jun 2023 13:13:53 +0300 Subject: [PATCH] Change LaneId underlying type from [u8; 4] to H256 (#2221) * change LaneId underlying type from [u8; 4] to H256 * fixed typo * added some tests * spelling * started fixing testnets * uncommented call size test * changed RewardsAccountParams encoding + added values separator when computing LaneId * review suggestions --- Cargo.lock | 2 + .../extensions/refund_relayer_extension.rs | 53 +++++---- .../runtime-common/src/messages_call_ext.rs | 28 ++--- .../src/messages_xcm_extension.rs | 37 ++++-- bridges/bin/runtime-common/src/mock.rs | 6 +- bridges/modules/messages/src/benchmarking.rs | 4 +- bridges/modules/messages/src/inbound_lane.rs | 36 +++--- bridges/modules/messages/src/outbound_lane.rs | 16 +-- bridges/modules/messages/src/proofs.rs | 10 +- bridges/modules/messages/src/tests/mock.rs | 24 ++-- .../messages/src/tests/pallet_tests.rs | 105 +++++++++-------- bridges/modules/relayers/src/benchmarking.rs | 6 +- bridges/modules/relayers/src/lib.rs | 24 ++-- bridges/modules/relayers/src/mock.rs | 9 +- .../modules/relayers/src/payment_adapter.rs | 14 +-- bridges/modules/relayers/src/stake_adapter.rs | 4 +- .../modules/xcm-bridge-hub/src/exporter.rs | 4 +- bridges/modules/xcm-bridge-hub/src/mock.rs | 14 +-- bridges/primitives/messages/Cargo.toml | 4 +- bridges/primitives/messages/src/lib.rs | 109 ++++++++++++++++-- .../primitives/messages/src/storage_keys.rs | 12 +- bridges/primitives/relayers/src/lib.rs | 23 ++-- .../primitives/runtime/src/storage_proof.rs | 10 +- bridges/relays/lib-substrate-relay/Cargo.toml | 1 + .../relays/lib-substrate-relay/src/cli/mod.rs | 11 +- .../src/cli/relay_headers_and_messages/mod.rs | 7 +- .../src/cli/relay_messages.rs | 8 +- .../lib-substrate-relay/src/messages/mod.rs | 2 +- .../src/messages/source.rs | 2 +- .../relays/messages/src/message_lane_loop.rs | 12 +- 30 files changed, 369 insertions(+), 228 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7c93274c62398..e11b0092d590d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1772,6 +1772,7 @@ dependencies = [ "scale-info", "serde", "sp-core", + "sp-io", "sp-std 14.0.0", ] @@ -20645,6 +20646,7 @@ dependencies = [ "rbtag", "relay-substrate-client", "relay-utils", + "rustc-hex", "sp-consensus-grandpa", "sp-core", "sp-runtime", diff --git a/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs b/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs index f8f0fa80756f5..264bd1d509654 100644 --- a/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs +++ b/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs @@ -19,8 +19,11 @@ //! with calls that are: delivering new message and all necessary underlying headers //! (parachain or relay chain). -use crate::messages_call_ext::{ - CallHelper as MessagesCallHelper, CallInfo as MessagesCallInfo, MessagesCallSubType, +use crate::{ + messages_call_ext::{ + CallHelper as MessagesCallHelper, CallInfo as MessagesCallInfo, MessagesCallSubType, + }, + messages_xcm_extension::LaneIdFromChainId, }; use bp_messages::{ChainWithMessages, LaneId, MessageNonce}; use bp_relayers::{ExplicitOrAccountParams, RewardsAccountOwner, RewardsAccountParams}; @@ -92,15 +95,15 @@ pub trait RefundableMessagesLaneId { } /// Default implementation of `RefundableMessagesLaneId`. -pub struct RefundableMessagesLane(PhantomData<(Instance, Id)>); +pub struct RefundableMessagesLane(PhantomData<(Runtime, Instance)>); -impl RefundableMessagesLaneId for RefundableMessagesLane +impl RefundableMessagesLaneId for RefundableMessagesLane where + Runtime: MessagesConfig, Instance: 'static, - Id: Get, { type Instance = Instance; - type Id = Id; + type Id = LaneIdFromChainId; } /// Refund calculator. @@ -968,14 +971,14 @@ pub(crate) mod tests { }; parameter_types! { - pub TestLaneId: LaneId = TEST_LANE_ID; + TestParachain: u32 = 1000; pub MsgProofsRewardsAccount: RewardsAccountParams = RewardsAccountParams::new( - TEST_LANE_ID, + test_lane_id(), TEST_BRIDGED_CHAIN_ID, RewardsAccountOwner::ThisChain, ); pub MsgDeliveryProofsRewardsAccount: RewardsAccountParams = RewardsAccountParams::new( - TEST_LANE_ID, + test_lane_id(), TEST_BRIDGED_CHAIN_ID, RewardsAccountOwner::BridgedChain, ); @@ -985,7 +988,7 @@ pub(crate) mod tests { type TestMessagesExtensionProvider = RefundBridgedMessages< TestRuntime, - RefundableMessagesLane<(), TestLaneId>, + RefundableMessagesLane, ActualFeeRefund, ConstU64<1>, StrTestExtension, @@ -994,7 +997,7 @@ pub(crate) mod tests { type TestGrandpaExtensionProvider = RefundBridgedGrandpaMessages< TestRuntime, (), - RefundableMessagesLane<(), TestLaneId>, + RefundableMessagesLane, ActualFeeRefund, ConstU64<1>, StrTestExtension, @@ -1003,7 +1006,7 @@ pub(crate) mod tests { type TestExtensionProvider = RefundBridgedParachainMessages< TestRuntime, RefundableParachain<(), BridgedUnderlyingParachain>, - RefundableMessagesLane<(), TestLaneId>, + RefundableMessagesLane, ActualFeeRefund, ConstU64<1>, StrTestExtension, @@ -1060,7 +1063,7 @@ pub(crate) mod tests { }; pallet_bridge_parachains::ParasInfo::::insert(para_id, para_info); - let lane_id = TestLaneId::get(); + let lane_id = test_lane_id(); let in_lane_data = InboundLaneData { last_confirmed_nonce: best_message, ..Default::default() }; pallet_bridge_messages::InboundLanes::::insert(lane_id, in_lane_data); @@ -1145,9 +1148,9 @@ pub(crate) mod tests { proof: Box::new(FromBridgedChainMessagesProof { bridged_header_hash: Default::default(), storage: Default::default(), - lane: TestLaneId::get(), + lane: test_lane_id(), nonces_start: pallet_bridge_messages::InboundLanes::::get( - TEST_LANE_ID, + test_lane_id(), ) .unwrap() .last_delivered_nonce() + @@ -1164,7 +1167,7 @@ pub(crate) mod tests { proof: FromBridgedChainMessagesDeliveryProof { bridged_header_hash: Default::default(), storage_proof: Default::default(), - lane: TestLaneId::get(), + lane: test_lane_id(), }, relayers_state: UnrewardedRelayersState { last_delivered_nonce: best_message, @@ -1321,7 +1324,7 @@ pub(crate) mod tests { }, MessagesCallInfo::ReceiveMessagesProof(ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range: 101..=200, best_stored_nonce: 100, }, @@ -1363,7 +1366,7 @@ pub(crate) mod tests { }, MessagesCallInfo::ReceiveMessagesDeliveryProof(ReceiveMessagesDeliveryProofInfo( BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range: 101..=200, best_stored_nonce: 100, }, @@ -1393,7 +1396,7 @@ pub(crate) mod tests { }, MessagesCallInfo::ReceiveMessagesProof(ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range: 101..=200, best_stored_nonce: 100, }, @@ -1429,7 +1432,7 @@ pub(crate) mod tests { }, MessagesCallInfo::ReceiveMessagesDeliveryProof(ReceiveMessagesDeliveryProofInfo( BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range: 101..=200, best_stored_nonce: 100, }, @@ -1457,7 +1460,7 @@ pub(crate) mod tests { }, MessagesCallInfo::ReceiveMessagesProof(ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range: 101..=200, best_stored_nonce: 100, }, @@ -1484,7 +1487,7 @@ pub(crate) mod tests { }, MessagesCallInfo::ReceiveMessagesDeliveryProof(ReceiveMessagesDeliveryProofInfo( BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range: 101..=200, best_stored_nonce: 100, }, @@ -1499,7 +1502,7 @@ pub(crate) mod tests { call_info: CallInfo::Msgs(MessagesCallInfo::ReceiveMessagesProof( ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range: 101..=200, best_stored_nonce: 100, }, @@ -1519,7 +1522,7 @@ pub(crate) mod tests { relayer: relayer_account_at_this_chain(), call_info: CallInfo::Msgs(MessagesCallInfo::ReceiveMessagesDeliveryProof( ReceiveMessagesDeliveryProofInfo(BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range: 101..=200, best_stored_nonce: 100, }), @@ -2883,7 +2886,7 @@ pub(crate) mod tests { .unwrap(); // allow empty message delivery transactions - let lane_id = TestLaneId::get(); + let lane_id = test_lane_id(); let in_lane_data = InboundLaneData { state: LaneState::Opened, last_confirmed_nonce: 0, diff --git a/bridges/bin/runtime-common/src/messages_call_ext.rs b/bridges/bin/runtime-common/src/messages_call_ext.rs index 58bf354968bc5..7c47b0f09b12e 100644 --- a/bridges/bin/runtime-common/src/messages_call_ext.rs +++ b/bridges/bin/runtime-common/src/messages_call_ext.rs @@ -361,11 +361,13 @@ mod tests { }; use sp_std::ops::RangeInclusive; - const TEST_LANE_ID: LaneId = LaneId([0, 0, 0, 0]); + fn test_lane_id() -> LaneId { + LaneId::new(1, 2) + } fn fill_unrewarded_relayers() { let mut inbound_lane_state = - pallet_bridge_messages::InboundLanes::::get(TEST_LANE_ID).unwrap(); + pallet_bridge_messages::InboundLanes::::get(test_lane_id()).unwrap(); for n in 0..BridgedUnderlyingChain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX { inbound_lane_state.relayers.push_back(UnrewardedRelayer { relayer: Default::default(), @@ -373,14 +375,14 @@ mod tests { }); } pallet_bridge_messages::InboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), inbound_lane_state, ); } fn fill_unrewarded_messages() { let mut inbound_lane_state = - pallet_bridge_messages::InboundLanes::::get(TEST_LANE_ID).unwrap(); + pallet_bridge_messages::InboundLanes::::get(test_lane_id()).unwrap(); inbound_lane_state.relayers.push_back(UnrewardedRelayer { relayer: Default::default(), messages: DeliveredMessages { @@ -389,14 +391,14 @@ mod tests { }, }); pallet_bridge_messages::InboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), inbound_lane_state, ); } fn deliver_message_10() { pallet_bridge_messages::InboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), bp_messages::InboundLaneData { state: LaneState::Opened, relayers: Default::default(), @@ -418,7 +420,7 @@ mod tests { proof: Box::new(FromBridgedChainMessagesProof { bridged_header_hash: Default::default(), storage: Default::default(), - lane: TEST_LANE_ID, + lane: test_lane_id(), nonces_start, nonces_end, }), @@ -431,11 +433,11 @@ mod tests { fn run_test(test: impl Fn() -> T) -> T { sp_io::TestExternalities::new(Default::default()).execute_with(|| { pallet_bridge_messages::InboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), InboundLaneData::opened(), ); pallet_bridge_messages::OutboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), OutboundLaneData::opened(), ); test() @@ -537,7 +539,7 @@ mod tests { fn confirm_message_10() { pallet_bridge_messages::OutboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), bp_messages::OutboundLaneData { state: LaneState::Opened, oldest_unpruned_nonce: 0, @@ -553,7 +555,7 @@ mod tests { proof: FromBridgedChainMessagesDeliveryProof { bridged_header_hash: Default::default(), storage_proof: Default::default(), - lane: TEST_LANE_ID, + lane: test_lane_id(), }, relayers_state: UnrewardedRelayersState { last_delivered_nonce, @@ -611,7 +613,7 @@ mod tests { CallHelper::::was_successful(&CallInfo::ReceiveMessagesProof( ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range, best_stored_nonce: 0, // doesn't matter for `was_successful` }, @@ -671,7 +673,7 @@ mod tests { fn was_message_confirmation_successful(bundled_range: RangeInclusive) -> bool { CallHelper::::was_successful(&CallInfo::ReceiveMessagesDeliveryProof( ReceiveMessagesDeliveryProofInfo(BaseMessagesProofInfo { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), bundled_range, best_stored_nonce: 0, // doesn't matter for `was_successful` }), diff --git a/bridges/bin/runtime-common/src/messages_xcm_extension.rs b/bridges/bin/runtime-common/src/messages_xcm_extension.rs index 0ea2f8b9619af..5ce906bef61d5 100644 --- a/bridges/bin/runtime-common/src/messages_xcm_extension.rs +++ b/bridges/bin/runtime-common/src/messages_xcm_extension.rs @@ -26,7 +26,7 @@ use bp_messages::{ target_chain::{DispatchMessage, MessageDispatch}, LaneId, MessageNonce, }; -use bp_runtime::messages::MessageDispatchResult; +use bp_runtime::{messages::MessageDispatchResult, Chain}; pub use bp_xcm_bridge_hub::XcmAsPlainPayload; use bp_xcm_bridge_hub_router::XcmChannelStatusProvider; use codec::{Decode, Encode}; @@ -38,8 +38,27 @@ use scale_info::TypeInfo; use sp_runtime::SaturatedConversion; use sp_std::{fmt::Debug, marker::PhantomData}; use xcm::prelude::*; + use xcm_builder::{DispatchBlob, DispatchBlobError}; +/// Make LaneId from chain identifiers of two bridge endpoints. +// TODO: https://github.com/paritytech/parity-bridges-common/issues/1666: this function +// is a temporary solution, because `ChainId` and will be removed soon. +pub struct LaneIdFromChainId(PhantomData<(R, I)>); + +impl Get for LaneIdFromChainId +where + R: pallet_bridge_messages::Config, + I: 'static, +{ + fn get() -> LaneId { + LaneId::new( + pallet_bridge_messages::ThisChainOf::::ID, + pallet_bridge_messages::BridgedChainOf::::ID, + ) + } +} + /// Message dispatch result type for single message. #[derive(CloneNoBound, EqNoBound, PartialEqNoBound, Encode, Decode, Debug, TypeInfo)] pub enum XcmBlobMessageDispatchResult { @@ -346,7 +365,7 @@ mod tests { parameter_types! { pub TestSenderAndLane: SenderAndLane = SenderAndLane { location: Location::new(1, [Parachain(1000)]), - lane: TEST_LANE_ID, + lane: test_lane_id(), }; pub TestLanes: sp_std::vec::Vec<(SenderAndLane, (NetworkId, InteriorLocation))> = sp_std::vec![ (TestSenderAndLane::get(), (NetworkId::ByGenesis([0; 32]), InteriorLocation::Here)) @@ -395,7 +414,7 @@ mod tests { fn fill_up_lane_to_congestion() -> MessageNonce { let latest_generated_nonce = OUTBOUND_LANE_CONGESTED_THRESHOLD; OutboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), OutboundLaneData { state: LaneState::Opened, oldest_unpruned_nonce: 0, @@ -449,7 +468,9 @@ mod tests { enqueued + 1, ); assert_eq!(DummySendXcm::messages_sent(), 1); - assert!(LocalXcmQueueManager::::is_congested_signal_sent(TEST_LANE_ID)); + assert!(LocalXcmQueueManager::::is_congested_signal_sent( + test_lane_id() + )); }); } @@ -460,7 +481,7 @@ mod tests { assert_eq!(DummySendXcm::messages_sent(), 1); // when we receive a delivery report for other lane, we don't send an uncongested signal - TestBlobHaulerAdapter::on_messages_delivered(LaneId([42, 42, 42, 42]), 0); + TestBlobHaulerAdapter::on_messages_delivered(LaneId::new(1, 3), 0); assert_eq!(DummySendXcm::messages_sent(), 1); }); } @@ -468,7 +489,7 @@ mod tests { #[test] fn uncongested_signal_is_not_sent_when_we_havent_send_congested_signal_before() { run_test(|| { - TestBlobHaulerAdapter::on_messages_delivered(TEST_LANE_ID, 0); + TestBlobHaulerAdapter::on_messages_delivered(test_lane_id(), 0); assert_eq!(DummySendXcm::messages_sent(), 0); }); } @@ -480,7 +501,7 @@ mod tests { assert_eq!(DummySendXcm::messages_sent(), 1); TestBlobHaulerAdapter::on_messages_delivered( - TEST_LANE_ID, + test_lane_id(), OUTBOUND_LANE_UNCONGESTED_THRESHOLD + 1, ); assert_eq!(DummySendXcm::messages_sent(), 1); @@ -494,7 +515,7 @@ mod tests { assert_eq!(DummySendXcm::messages_sent(), 1); TestBlobHaulerAdapter::on_messages_delivered( - TEST_LANE_ID, + test_lane_id(), OUTBOUND_LANE_UNCONGESTED_THRESHOLD, ); assert_eq!(DummySendXcm::messages_sent(), 2); diff --git a/bridges/bin/runtime-common/src/mock.rs b/bridges/bin/runtime-common/src/mock.rs index e6bd73f21f18d..366c92a60e109 100644 --- a/bridges/bin/runtime-common/src/mock.rs +++ b/bridges/bin/runtime-common/src/mock.rs @@ -33,6 +33,7 @@ use frame_support::{ weights::{ConstantMultiplier, IdentityFee, RuntimeDbWeight, Weight}, }; use pallet_transaction_payment::Multiplier; +use sp_core::Get; use sp_runtime::{ testing::H256, traits::{BlakeTwo256, ConstU32, ConstU64, ConstU8}, @@ -85,7 +86,10 @@ pub type TestStakeAndSlash = pallet_bridge_relayers::StakeAndSlashNamed< >; /// Message lane used in tests. -pub const TEST_LANE_ID: LaneId = LaneId([0, 0, 0, 0]); +pub fn test_lane_id() -> LaneId { + crate::messages_xcm_extension::LaneIdFromChainId::::get() +} + /// Bridged chain id used in tests. pub const TEST_BRIDGED_CHAIN_ID: ChainId = *b"brdg"; /// Maximal extrinsic size at the `BridgedChain`. diff --git a/bridges/modules/messages/src/benchmarking.rs b/bridges/modules/messages/src/benchmarking.rs index 8613911033fc7..14dcbd7be4d67 100644 --- a/bridges/modules/messages/src/benchmarking.rs +++ b/bridges/modules/messages/src/benchmarking.rs @@ -74,10 +74,8 @@ pub struct MessageDeliveryProofParams { /// Trait that must be implemented by runtime. pub trait Config: crate::Config { /// Lane id to use in benchmarks. - /// - /// By default, lane 00000000 is used. fn bench_lane_id() -> LaneId { - LaneId([0, 0, 0, 0]) + LaneId::new(1, 2) } /// Return id of relayer account at the bridged chain. diff --git a/bridges/modules/messages/src/inbound_lane.rs b/bridges/modules/messages/src/inbound_lane.rs index 5061d63bd30e4..3f397d025d271 100644 --- a/bridges/modules/messages/src/inbound_lane.rs +++ b/bridges/modules/messages/src/inbound_lane.rs @@ -216,15 +216,7 @@ impl InboundLane { #[cfg(test)] mod tests { use super::*; - use crate::{ - inbound_lane, - tests::mock::{ - dispatch_result, inbound_message_data, inbound_unrewarded_relayers_state, run_test, - unrewarded_relayer, BridgedChain, TestMessageDispatch, TestRuntime, REGULAR_PAYLOAD, - TEST_LANE_ID, TEST_RELAYER_A, TEST_RELAYER_B, TEST_RELAYER_C, - }, - RuntimeInboundLaneStorage, - }; + use crate::{inbound_lane, tests::mock::*, RuntimeInboundLaneStorage}; use bp_messages::UnrewardedRelayersState; fn receive_regular_message( @@ -244,7 +236,7 @@ mod tests { #[test] fn receive_status_update_ignores_status_from_the_future() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); assert_eq!( lane.receive_state_update(OutboundLaneData { @@ -261,7 +253,7 @@ mod tests { #[test] fn receive_status_update_ignores_obsolete_status() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); receive_regular_message(&mut lane, 2); receive_regular_message(&mut lane, 3); @@ -288,7 +280,7 @@ mod tests { #[test] fn receive_status_update_works() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); receive_regular_message(&mut lane, 2); receive_regular_message(&mut lane, 3); @@ -326,7 +318,7 @@ mod tests { #[test] fn receive_status_update_works_with_batches_from_relayers() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); let mut seed_storage_data = lane.storage.data(); // Prepare data seed_storage_data.last_confirmed_nonce = 0; @@ -357,7 +349,7 @@ mod tests { #[test] fn fails_to_receive_message_with_incorrect_nonce() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -373,7 +365,7 @@ mod tests { #[test] fn fails_to_receive_messages_above_unrewarded_relayer_entries_limit_per_lane() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); let max_nonce = BridgedChain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX; for current_nonce in 1..max_nonce + 1 { assert_eq!( @@ -409,7 +401,7 @@ mod tests { #[test] fn fails_to_receive_messages_above_unconfirmed_messages_limit_per_lane() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); let max_nonce = BridgedChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX; for current_nonce in 1..=max_nonce { assert_eq!( @@ -445,7 +437,7 @@ mod tests { #[test] fn correctly_receives_following_messages_from_two_relayers_alternately() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -484,7 +476,7 @@ mod tests { #[test] fn rejects_same_message_from_two_different_relayers() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -507,7 +499,7 @@ mod tests { #[test] fn correct_message_is_processed_instantly() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); assert_eq!(lane.storage.data().last_delivered_nonce(), 1); }); @@ -516,7 +508,7 @@ mod tests { #[test] fn unspent_weight_is_returned_by_receive_message() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); let mut payload = REGULAR_PAYLOAD; *payload.dispatch_result.unspent_weight.ref_time_mut() = 1; assert_eq!( @@ -533,7 +525,7 @@ mod tests { #[test] fn first_message_is_confirmed_correctly() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = inbound_lane::(test_lane_id()).unwrap(); receive_regular_message(&mut lane, 1); receive_regular_message(&mut lane, 2); assert_eq!( @@ -544,7 +536,7 @@ mod tests { Some(1), ); assert_eq!( - inbound_unrewarded_relayers_state(TEST_LANE_ID), + inbound_unrewarded_relayers_state(test_lane_id()), UnrewardedRelayersState { unrewarded_relayer_entries: 1, messages_in_oldest_entry: 1, diff --git a/bridges/modules/messages/src/outbound_lane.rs b/bridges/modules/messages/src/outbound_lane.rs index f7beeb17fbf10..07144afdab81d 100644 --- a/bridges/modules/messages/src/outbound_lane.rs +++ b/bridges/modules/messages/src/outbound_lane.rs @@ -190,8 +190,8 @@ mod tests { use crate::{ outbound_lane, tests::mock::{ - outbound_message_data, run_test, unrewarded_relayer, TestRelayer, TestRuntime, - REGULAR_PAYLOAD, TEST_LANE_ID, + outbound_message_data, run_test, test_lane_id, unrewarded_relayer, TestRelayer, + TestRuntime, REGULAR_PAYLOAD, }, }; use sp_std::ops::RangeInclusive; @@ -213,7 +213,7 @@ mod tests { relayers: &VecDeque>, ) -> Result, ReceptionConfirmationError> { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = outbound_lane::(test_lane_id()).unwrap(); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); @@ -229,7 +229,7 @@ mod tests { #[test] fn send_message_works() { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = outbound_lane::(test_lane_id()).unwrap(); assert_eq!(lane.storage.data().latest_generated_nonce, 0); assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1); assert!(lane.storage.message(&1).is_some()); @@ -240,7 +240,7 @@ mod tests { #[test] fn confirm_delivery_works() { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = outbound_lane::(test_lane_id()).unwrap(); assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1); assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 2); assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 3); @@ -260,7 +260,7 @@ mod tests { #[test] fn confirm_partial_delivery_works() { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = outbound_lane::(test_lane_id()).unwrap(); assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1); assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 2); assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 3); @@ -289,7 +289,7 @@ mod tests { #[test] fn confirm_delivery_rejects_nonce_lesser_than_latest_received() { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = outbound_lane::(test_lane_id()).unwrap(); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); @@ -369,7 +369,7 @@ mod tests { #[test] fn confirm_delivery_detects_when_more_than_expected_messages_are_confirmed() { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID).unwrap(); + let mut lane = outbound_lane::(test_lane_id()).unwrap(); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); lane.send_message(outbound_message_data(REGULAR_PAYLOAD)); diff --git a/bridges/modules/messages/src/proofs.rs b/bridges/modules/messages/src/proofs.rs index 0e5edac635eb6..db80952b63425 100644 --- a/bridges/modules/messages/src/proofs.rs +++ b/bridges/modules/messages/src/proofs.rs @@ -184,7 +184,7 @@ mod tests { test: impl Fn(FromBridgedChainMessagesProof) -> R, ) -> R { let (state_root, storage) = prepare_messages_storage_proof::( - TEST_LANE_ID, + test_lane_id(), 1..=nonces_end, outbound_lane_data, bp_runtime::StorageProofSize::Minimal(0), @@ -216,7 +216,7 @@ mod tests { test(FromBridgedChainMessagesProof { bridged_header_hash, storage, - lane: TEST_LANE_ID, + lane: test_lane_id(), nonces_start: 1, nonces_end, }) @@ -442,7 +442,7 @@ mod tests { |proof| verify_messages_proof::(proof, 0), ), Ok(vec![( - TEST_LANE_ID, + test_lane_id(), ProvedLaneMessages { lane_state: Some(OutboundLaneData { state: LaneState::Opened, @@ -476,7 +476,7 @@ mod tests { |proof| verify_messages_proof::(proof, 1), ), Ok(vec![( - TEST_LANE_ID, + test_lane_id(), ProvedLaneMessages { lane_state: Some(OutboundLaneData { state: LaneState::Opened, @@ -485,7 +485,7 @@ mod tests { latest_generated_nonce: 1, }), messages: vec![Message { - key: MessageKey { lane_id: TEST_LANE_ID, nonce: 1 }, + key: MessageKey { lane_id: test_lane_id(), nonce: 1 }, payload: vec![42], }], }, diff --git a/bridges/modules/messages/src/tests/mock.rs b/bridges/modules/messages/src/tests/mock.rs index 15bc6a3033a91..14f7943434f5b 100644 --- a/bridges/modules/messages/src/tests/mock.rs +++ b/bridges/modules/messages/src/tests/mock.rs @@ -212,7 +212,7 @@ impl Config for TestRuntime { #[cfg(feature = "runtime-benchmarks")] impl crate::benchmarking::Config<()> for TestRuntime { fn bench_lane_id() -> LaneId { - TEST_LANE_ID + test_lane_id() } fn prepare_message_proof( @@ -263,13 +263,19 @@ pub const TEST_RELAYER_B: AccountId = 101; pub const TEST_RELAYER_C: AccountId = 102; /// Lane that we're using in tests. -pub const TEST_LANE_ID: LaneId = LaneId([0, 0, 0, 1]); +pub fn test_lane_id() -> LaneId { + LaneId::new(1, 2) +} /// Lane that is completely unknown to our runtime. -pub const UNKNOWN_LANE_ID: LaneId = LaneId([0, 0, 0, 2]); +pub fn unknown_lane_id() -> LaneId { + LaneId::new(1, 3) +} /// Lane that is registered, but it is closed. -pub const CLOSED_LANE_ID: LaneId = LaneId([0, 0, 0, 3]); +pub fn closed_lane_id() -> LaneId { + LaneId::new(1, 4) +} /// Regular message payload. pub const REGULAR_PAYLOAD: TestPayload = message_payload(0, 50); @@ -391,7 +397,7 @@ impl OnMessagesDelivered for TestOnMessagesDelivered { /// Return test lane message with given nonce and payload. pub fn message(nonce: MessageNonce, payload: TestPayload) -> Message { - Message { key: MessageKey { lane_id: TEST_LANE_ID, nonce }, payload: payload.encode() } + Message { key: MessageKey { lane_id: test_lane_id(), nonce }, payload: payload.encode() } } /// Return valid outbound message data, constructed from given payload. @@ -451,14 +457,14 @@ pub fn new_test_ext() -> sp_io::TestExternalities { /// Run pallet test. pub fn run_test(test: impl FnOnce() -> T) -> T { new_test_ext().execute_with(|| { - crate::InboundLanes::::insert(TEST_LANE_ID, InboundLaneData::opened()); - crate::OutboundLanes::::insert(TEST_LANE_ID, OutboundLaneData::opened()); + crate::InboundLanes::::insert(test_lane_id(), InboundLaneData::opened()); + crate::OutboundLanes::::insert(test_lane_id(), OutboundLaneData::opened()); crate::InboundLanes::::insert( - CLOSED_LANE_ID, + closed_lane_id(), InboundLaneData { state: LaneState::Closed, ..Default::default() }, ); crate::OutboundLanes::::insert( - CLOSED_LANE_ID, + closed_lane_id(), OutboundLaneData { state: LaneState::Closed, ..Default::default() }, ); test() diff --git a/bridges/modules/messages/src/tests/pallet_tests.rs b/bridges/modules/messages/src/tests/pallet_tests.rs index 2ed7fa16ff513..0b8acd99afc39 100644 --- a/bridges/modules/messages/src/tests/pallet_tests.rs +++ b/bridges/modules/messages/src/tests/pallet_tests.rs @@ -79,7 +79,7 @@ fn receive_messages_delivery_proof() { assert_ok!(Pallet::::receive_messages_delivery_proof( RuntimeOrigin::signed(1), prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { state: LaneState::Opened, last_confirmed_nonce: 1, @@ -103,7 +103,7 @@ fn receive_messages_delivery_proof() { vec![EventRecord { phase: Phase::Initialization, event: TestEvent::Messages(Event::MessagesDelivered { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), messages: DeliveredMessages::new(1), }), topics: vec![], @@ -115,14 +115,14 @@ fn receive_messages_delivery_proof() { fn pallet_rejects_transactions_if_halted() { run_test(|| { // send message first to be able to check that delivery_proof fails later - send_regular_message(TEST_LANE_ID); + send_regular_message(test_lane_id()); PalletOperatingMode::::put(MessagesOperatingMode::Basic( BasicOperatingMode::Halted, )); assert_noop!( - Pallet::::validate_message(TEST_LANE_ID, ®ULAR_PAYLOAD), + Pallet::::validate_message(test_lane_id(), ®ULAR_PAYLOAD), Error::::NotOperatingNormally, ); @@ -139,7 +139,7 @@ fn pallet_rejects_transactions_if_halted() { ); let delivery_proof = prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { state: LaneState::Opened, last_confirmed_nonce: 1, @@ -166,14 +166,14 @@ fn pallet_rejects_transactions_if_halted() { fn pallet_rejects_new_messages_in_rejecting_outbound_messages_operating_mode() { run_test(|| { // send message first to be able to check that delivery_proof fails later - send_regular_message(TEST_LANE_ID); + send_regular_message(test_lane_id()); PalletOperatingMode::::put( MessagesOperatingMode::RejectingOutboundMessages, ); assert_noop!( - Pallet::::validate_message(TEST_LANE_ID, ®ULAR_PAYLOAD), + Pallet::::validate_message(test_lane_id(), ®ULAR_PAYLOAD), Error::::NotOperatingNormally, ); @@ -188,7 +188,7 @@ fn pallet_rejects_new_messages_in_rejecting_outbound_messages_operating_mode() { assert_ok!(Pallet::::receive_messages_delivery_proof( RuntimeOrigin::signed(1), prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { state: LaneState::Opened, last_confirmed_nonce: 1, @@ -208,7 +208,7 @@ fn pallet_rejects_new_messages_in_rejecting_outbound_messages_operating_mode() { #[test] fn send_message_works() { run_test(|| { - send_regular_message(TEST_LANE_ID); + send_regular_message(test_lane_id()); }); } @@ -223,7 +223,7 @@ fn send_message_rejects_too_large_message() { .extra .extend_from_slice(&vec![0u8; max_outbound_payload_size as usize]); assert_noop!( - Pallet::::validate_message(TEST_LANE_ID, &message_payload.clone(),), + Pallet::::validate_message(test_lane_id(), &message_payload.clone(),), Error::::MessageRejectedByPallet(VerificationError::MessageTooLarge), ); @@ -234,7 +234,7 @@ fn send_message_rejects_too_large_message() { assert_eq!(message_payload.encoded_size() as u32, max_outbound_payload_size); let valid_message = - Pallet::::validate_message(TEST_LANE_ID, &message_payload) + Pallet::::validate_message(test_lane_id(), &message_payload) .expect("validate_message has failed"); Pallet::::send_message(valid_message); }) @@ -252,7 +252,10 @@ fn receive_messages_proof_works() { )); assert_eq!( - InboundLanes::::get(TEST_LANE_ID).unwrap().0.last_delivered_nonce(), + InboundLanes::::get(test_lane_id()) + .unwrap() + .0 + .last_delivered_nonce(), 1 ); @@ -265,7 +268,7 @@ fn receive_messages_proof_updates_confirmed_message_nonce() { run_test(|| { // say we have received 10 messages && last confirmed message is 8 InboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { state: LaneState::Opened, last_confirmed_nonce: 8, @@ -277,7 +280,7 @@ fn receive_messages_proof_updates_confirmed_message_nonce() { }, ); assert_eq!( - inbound_unrewarded_relayers_state(TEST_LANE_ID), + inbound_unrewarded_relayers_state(test_lane_id()), UnrewardedRelayersState { unrewarded_relayer_entries: 2, messages_in_oldest_entry: 1, @@ -299,7 +302,7 @@ fn receive_messages_proof_updates_confirmed_message_nonce() { )); assert_eq!( - InboundLanes::::get(TEST_LANE_ID).unwrap().0, + InboundLanes::::get(test_lane_id()).unwrap().0, InboundLaneData { state: LaneState::Opened, last_confirmed_nonce: 9, @@ -311,7 +314,7 @@ fn receive_messages_proof_updates_confirmed_message_nonce() { }, ); assert_eq!( - inbound_unrewarded_relayers_state(TEST_LANE_ID), + inbound_unrewarded_relayers_state(test_lane_id()), UnrewardedRelayersState { unrewarded_relayer_entries: 2, messages_in_oldest_entry: 1, @@ -340,7 +343,7 @@ fn receive_messages_proof_does_not_accept_message_if_dispatch_weight_is_not_enou Error::::InsufficientDispatchWeight ); assert_eq!( - InboundLanes::::get(TEST_LANE_ID).unwrap().last_delivered_nonce(), + InboundLanes::::get(test_lane_id()).unwrap().last_delivered_nonce(), 0 ); }); @@ -385,11 +388,11 @@ fn receive_messages_proof_rejects_proof_with_too_many_messages() { #[test] fn receive_messages_delivery_proof_works() { run_test(|| { - send_regular_message(TEST_LANE_ID); + send_regular_message(test_lane_id()); receive_messages_delivery_proof(); assert_eq!( - OutboundLanes::::get(TEST_LANE_ID) + OutboundLanes::::get(test_lane_id()) .unwrap() .latest_received_nonce, 1, @@ -400,12 +403,12 @@ fn receive_messages_delivery_proof_works() { #[test] fn receive_messages_delivery_proof_rewards_relayers() { run_test(|| { - send_regular_message(TEST_LANE_ID); - send_regular_message(TEST_LANE_ID); + send_regular_message(test_lane_id()); + send_regular_message(test_lane_id()); // this reports delivery of message 1 => reward is paid to TEST_RELAYER_A let single_message_delivery_proof = prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { relayers: vec![unrewarded_relayer(1, 1, TEST_RELAYER_A)].into(), ..Default::default() @@ -440,7 +443,7 @@ fn receive_messages_delivery_proof_rewards_relayers() { // this reports delivery of both message 1 and message 2 => reward is paid only to // TEST_RELAYER_B let two_messages_delivery_proof = prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { relayers: vec![ unrewarded_relayer(1, 1, TEST_RELAYER_A), @@ -477,15 +480,15 @@ fn receive_messages_delivery_proof_rewards_relayers() { ); assert!(!TestDeliveryConfirmationPayments::is_reward_paid(TEST_RELAYER_A, 1)); assert!(TestDeliveryConfirmationPayments::is_reward_paid(TEST_RELAYER_B, 1)); - assert_eq!(TestOnMessagesDelivered::call_arguments(), Some((TEST_LANE_ID, 0))); + assert_eq!(TestOnMessagesDelivered::call_arguments(), Some((test_lane_id(), 0))); }); } #[test] fn receive_messages_delivery_proof_rejects_invalid_proof() { run_test(|| { - let mut proof = prepare_messages_delivery_proof(TEST_LANE_ID, Default::default()); - proof.lane = bp_messages::LaneId([42, 42, 42, 42]); + let mut proof = prepare_messages_delivery_proof(test_lane_id(), Default::default()); + proof.lane = bp_messages::LaneId::new(42, 84); assert_noop!( Pallet::::receive_messages_delivery_proof( @@ -503,7 +506,7 @@ fn receive_messages_delivery_proof_rejects_proof_if_declared_relayers_state_is_i run_test(|| { // when number of relayers entries is invalid let proof = prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { relayers: vec![ unrewarded_relayer(1, 1, TEST_RELAYER_A), @@ -529,7 +532,7 @@ fn receive_messages_delivery_proof_rejects_proof_if_declared_relayers_state_is_i // when number of messages is invalid let proof = prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { relayers: vec![ unrewarded_relayer(1, 1, TEST_RELAYER_A), @@ -555,7 +558,7 @@ fn receive_messages_delivery_proof_rejects_proof_if_declared_relayers_state_is_i // when last delivered nonce is invalid let proof = prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { relayers: vec![ unrewarded_relayer(1, 1, TEST_RELAYER_A), @@ -597,7 +600,7 @@ fn receive_messages_accepts_single_message_with_invalid_payload() { ),); assert_eq!( - InboundLanes::::get(TEST_LANE_ID).unwrap().last_delivered_nonce(), + InboundLanes::::get(test_lane_id()).unwrap().last_delivered_nonce(), 1, ); }); @@ -621,7 +624,7 @@ fn receive_messages_accepts_batch_with_message_with_invalid_payload() { ),); assert_eq!( - InboundLanes::::get(TEST_LANE_ID).unwrap().last_delivered_nonce(), + InboundLanes::::get(test_lane_id()).unwrap().last_delivered_nonce(), 3, ); }); @@ -647,7 +650,7 @@ fn actual_dispatch_weight_does_not_overlow() { Error::::InsufficientDispatchWeight ); assert_eq!( - InboundLanes::::get(TEST_LANE_ID).unwrap().last_delivered_nonce(), + InboundLanes::::get(test_lane_id()).unwrap().last_delivered_nonce(), 0 ); }); @@ -726,7 +729,7 @@ fn proof_size_refund_from_receive_messages_proof_works() { REGULAR_PAYLOAD.declared_weight, ); InboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), StoredInboundLaneData(InboundLaneData { state: LaneState::Opened, relayers: vec![ @@ -755,7 +758,7 @@ fn proof_size_refund_from_receive_messages_proof_works() { // if count of unrewarded relayer entries is less than maximal, then some `proof_size` // must be refunded InboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), StoredInboundLaneData(InboundLaneData { state: LaneState::Opened, relayers: vec![ @@ -793,7 +796,7 @@ fn receive_messages_delivery_proof_rejects_proof_if_trying_to_confirm_more_messa { run_test(|| { // send message first to be able to check that delivery_proof fails later - send_regular_message(TEST_LANE_ID); + send_regular_message(test_lane_id()); // 1) InboundLaneData declares that the `last_confirmed_nonce` is 1; // 2) InboundLaneData has no entries => `InboundLaneData::last_delivered_nonce()` returns @@ -802,7 +805,7 @@ fn receive_messages_delivery_proof_rejects_proof_if_trying_to_confirm_more_messa // 4) so the number of declared messages (see `UnrewardedRelayersState`) is `0` and numer of // actually confirmed messages is `1`. let proof = prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { state: LaneState::Opened, last_confirmed_nonce: 1, @@ -831,20 +834,20 @@ fn storage_keys_computed_properly() { assert_eq!( OutboundMessages::::storage_map_final_key(MessageKey { - lane_id: TEST_LANE_ID, + lane_id: test_lane_id(), nonce: 42 }), - bp_messages::storage_keys::message_key("Messages", &TEST_LANE_ID, 42).0, + bp_messages::storage_keys::message_key("Messages", &test_lane_id(), 42).0, ); assert_eq!( - OutboundLanes::::storage_map_final_key(TEST_LANE_ID), - bp_messages::storage_keys::outbound_lane_data_key("Messages", &TEST_LANE_ID).0, + OutboundLanes::::storage_map_final_key(test_lane_id()), + bp_messages::storage_keys::outbound_lane_data_key("Messages", &test_lane_id()).0, ); assert_eq!( - InboundLanes::::storage_map_final_key(TEST_LANE_ID), - bp_messages::storage_keys::inbound_lane_data_key("Messages", &TEST_LANE_ID).0, + InboundLanes::::storage_map_final_key(test_lane_id()), + bp_messages::storage_keys::inbound_lane_data_key("Messages", &test_lane_id()).0, ); } @@ -853,7 +856,7 @@ fn inbound_message_details_works() { run_test(|| { assert_eq!( Pallet::::inbound_message_data( - TEST_LANE_ID, + test_lane_id(), REGULAR_PAYLOAD.encode(), OutboundMessageDetails { nonce: 0, dispatch_weight: Weight::zero(), size: 0 }, ), @@ -868,7 +871,7 @@ fn test_bridge_messages_call_is_correctly_defined() { let account_id = 1; let message_proof = prepare_messages_proof(vec![message(1, REGULAR_PAYLOAD)], None); let message_delivery_proof = prepare_messages_delivery_proof( - TEST_LANE_ID, + test_lane_id(), InboundLaneData { state: LaneState::Opened, last_confirmed_nonce: 1, @@ -940,7 +943,7 @@ fn inbound_storage_extra_proof_size_bytes_works() { fn storage(relayer_entries: usize) -> RuntimeInboundLaneStorage { RuntimeInboundLaneStorage { - lane_id: Default::default(), + lane_id: LaneId::new(1, 2), cached_data: InboundLaneData { state: LaneState::Opened, relayers: vec![relayer_entry(); relayer_entries].into(), @@ -974,12 +977,12 @@ fn inbound_storage_extra_proof_size_bytes_works() { fn send_messages_fails_if_outbound_lane_is_not_opened() { run_test(|| { assert_noop!( - Pallet::::validate_message(UNKNOWN_LANE_ID, ®ULAR_PAYLOAD), + Pallet::::validate_message(unknown_lane_id(), ®ULAR_PAYLOAD), Error::::UnknownOutboundLane, ); assert_noop!( - Pallet::::validate_message(CLOSED_LANE_ID, ®ULAR_PAYLOAD), + Pallet::::validate_message(closed_lane_id(), ®ULAR_PAYLOAD), Error::::ClosedOutboundLane, ); }); @@ -989,7 +992,7 @@ fn send_messages_fails_if_outbound_lane_is_not_opened() { fn receive_messages_proof_fails_if_inbound_lane_is_not_opened() { run_test(|| { let mut message = message(1, REGULAR_PAYLOAD); - message.key.lane_id = UNKNOWN_LANE_ID; + message.key.lane_id = unknown_lane_id(); let proof = prepare_messages_proof(vec![message.clone()], None); assert_noop!( @@ -1003,7 +1006,7 @@ fn receive_messages_proof_fails_if_inbound_lane_is_not_opened() { Error::::UnknownInboundLane, ); - message.key.lane_id = CLOSED_LANE_ID; + message.key.lane_id = closed_lane_id(); let proof = prepare_messages_proof(vec![message], None); assert_noop!( @@ -1037,7 +1040,7 @@ fn receive_messages_delivery_proof_fails_if_outbound_lane_is_unknown() { ) }; - let proof = make_proof(UNKNOWN_LANE_ID); + let proof = make_proof(unknown_lane_id()); assert_noop!( Pallet::::receive_messages_delivery_proof( RuntimeOrigin::signed(1), @@ -1052,7 +1055,7 @@ fn receive_messages_delivery_proof_fails_if_outbound_lane_is_unknown() { Error::::UnknownOutboundLane, ); - let proof = make_proof(CLOSED_LANE_ID); + let proof = make_proof(closed_lane_id()); assert_noop!( Pallet::::receive_messages_delivery_proof( RuntimeOrigin::signed(1), diff --git a/bridges/modules/relayers/src/benchmarking.rs b/bridges/modules/relayers/src/benchmarking.rs index ca312d44edfdd..8a3f905a8f291 100644 --- a/bridges/modules/relayers/src/benchmarking.rs +++ b/bridges/modules/relayers/src/benchmarking.rs @@ -43,7 +43,7 @@ pub trait Config: crate::Config { benchmarks! { // Benchmark `claim_rewards` call. claim_rewards { - let lane = LaneId([0, 0, 0, 0]); + let lane = LaneId::new(1, 2); let account_params = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); let relayer: T::AccountId = whitelisted_caller(); @@ -102,7 +102,7 @@ benchmarks! { crate::Pallet::::register(RawOrigin::Signed(relayer.clone()).into(), valid_till).unwrap(); // create slash destination account - let lane = LaneId([0, 0, 0, 0]); + let lane = LaneId::new(1, 2); let slash_destination = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); T::prepare_rewards_account(slash_destination, Zero::zero()); }: { @@ -116,7 +116,7 @@ benchmarks! { // the weight of message delivery call if `RefundBridgedParachainMessages` signed extension // is deployed at runtime level. register_relayer_reward { - let lane = LaneId([0, 0, 0, 0]); + let lane = LaneId::new(1, 2); let relayer: T::AccountId = whitelisted_caller(); let account_params = RewardsAccountParams::new(lane, *b"test", RewardsAccountOwner::ThisChain); diff --git a/bridges/modules/relayers/src/lib.rs b/bridges/modules/relayers/src/lib.rs index 2c86ec01f5b91..114f8ca3fb1ce 100644 --- a/bridges/modules/relayers/src/lib.rs +++ b/bridges/modules/relayers/src/lib.rs @@ -492,7 +492,7 @@ mod tests { get_ready_for_events(); Pallet::::register_relayer_reward( - TEST_REWARDS_ACCOUNT_PARAMS, + test_reward_account_param(), ®ULAR_RELAYER, 100, ); @@ -504,7 +504,7 @@ mod tests { phase: Phase::Initialization, event: TestEvent::Relayers(RewardRegistered { relayer: REGULAR_RELAYER, - rewards_account_params: TEST_REWARDS_ACCOUNT_PARAMS, + rewards_account_params: test_reward_account_param(), reward: 100 }), topics: vec![], @@ -519,7 +519,7 @@ mod tests { assert_noop!( Pallet::::claim_rewards( RuntimeOrigin::root(), - TEST_REWARDS_ACCOUNT_PARAMS + test_reward_account_param() ), DispatchError::BadOrigin, ); @@ -532,7 +532,7 @@ mod tests { assert_noop!( Pallet::::claim_rewards( RuntimeOrigin::signed(REGULAR_RELAYER), - TEST_REWARDS_ACCOUNT_PARAMS + test_reward_account_param() ), Error::::NoRewardForRelayer, ); @@ -544,13 +544,13 @@ mod tests { run_test(|| { RelayerRewards::::insert( FAILING_RELAYER, - TEST_REWARDS_ACCOUNT_PARAMS, + test_reward_account_param(), 100, ); assert_noop!( Pallet::::claim_rewards( RuntimeOrigin::signed(FAILING_RELAYER), - TEST_REWARDS_ACCOUNT_PARAMS + test_reward_account_param() ), Error::::FailedToPayReward, ); @@ -564,15 +564,15 @@ mod tests { RelayerRewards::::insert( REGULAR_RELAYER, - TEST_REWARDS_ACCOUNT_PARAMS, + test_reward_account_param(), 100, ); assert_ok!(Pallet::::claim_rewards( RuntimeOrigin::signed(REGULAR_RELAYER), - TEST_REWARDS_ACCOUNT_PARAMS + test_reward_account_param() )); assert_eq!( - RelayerRewards::::get(REGULAR_RELAYER, TEST_REWARDS_ACCOUNT_PARAMS), + RelayerRewards::::get(REGULAR_RELAYER, test_reward_account_param()), None ); @@ -583,7 +583,7 @@ mod tests { phase: Phase::Initialization, event: TestEvent::Relayers(RewardPaid { relayer: REGULAR_RELAYER, - rewards_account_params: TEST_REWARDS_ACCOUNT_PARAMS, + rewards_account_params: test_reward_account_param(), reward: 100 }), topics: vec![], @@ -599,12 +599,12 @@ mod tests { run_test(|| { let in_lane_0 = RewardsAccountParams::new( - LaneId([0, 0, 0, 0]), + LaneId::new(1, 2), *b"test", RewardsAccountOwner::ThisChain, ); let out_lane_1 = RewardsAccountParams::new( - LaneId([0, 0, 0, 1]), + LaneId::new(1, 3), *b"test", RewardsAccountOwner::BridgedChain, ); diff --git a/bridges/modules/relayers/src/mock.rs b/bridges/modules/relayers/src/mock.rs index 3124787896c3e..81993589de619 100644 --- a/bridges/modules/relayers/src/mock.rs +++ b/bridges/modules/relayers/src/mock.rs @@ -95,10 +95,6 @@ impl pallet_bridge_relayers::benchmarking::Config for TestRuntime { } } -/// Message lane that we're using in tests. -pub const TEST_REWARDS_ACCOUNT_PARAMS: RewardsAccountParams = - RewardsAccountParams::new(LaneId([0, 0, 0, 0]), *b"test", RewardsAccountOwner::ThisChain); - /// Regular relayer that may receive rewards. pub const REGULAR_RELAYER: AccountId = 1; @@ -132,6 +128,11 @@ impl PaymentProcedure for TestPaymentProcedure { } } +/// Reward account params that we are using in tests. +pub fn test_reward_account_param() -> RewardsAccountParams { + RewardsAccountParams::new(LaneId::new(1, 2), *b"test", RewardsAccountOwner::ThisChain) +} + /// Return test externalities to use in tests. pub fn new_test_ext() -> sp_io::TestExternalities { let t = frame_system::GenesisConfig::::default().build_storage().unwrap(); diff --git a/bridges/modules/relayers/src/payment_adapter.rs b/bridges/modules/relayers/src/payment_adapter.rs index f75c409aca4f3..beec412c12035 100644 --- a/bridges/modules/relayers/src/payment_adapter.rs +++ b/bridges/modules/relayers/src/payment_adapter.rs @@ -117,16 +117,16 @@ mod tests { register_relayers_rewards::( &RELAYER_2, relayers_rewards(), - TEST_REWARDS_ACCOUNT_PARAMS, + test_reward_account_param(), 50, ); assert_eq!( - RelayerRewards::::get(RELAYER_1, TEST_REWARDS_ACCOUNT_PARAMS), + RelayerRewards::::get(RELAYER_1, test_reward_account_param()), Some(100) ); assert_eq!( - RelayerRewards::::get(RELAYER_2, TEST_REWARDS_ACCOUNT_PARAMS), + RelayerRewards::::get(RELAYER_2, test_reward_account_param()), Some(150) ); }); @@ -138,20 +138,20 @@ mod tests { register_relayers_rewards::( &RELAYER_3, relayers_rewards(), - TEST_REWARDS_ACCOUNT_PARAMS, + test_reward_account_param(), 50, ); assert_eq!( - RelayerRewards::::get(RELAYER_1, TEST_REWARDS_ACCOUNT_PARAMS), + RelayerRewards::::get(RELAYER_1, test_reward_account_param()), Some(100) ); assert_eq!( - RelayerRewards::::get(RELAYER_2, TEST_REWARDS_ACCOUNT_PARAMS), + RelayerRewards::::get(RELAYER_2, test_reward_account_param()), Some(150) ); assert_eq!( - RelayerRewards::::get(RELAYER_3, TEST_REWARDS_ACCOUNT_PARAMS), + RelayerRewards::::get(RELAYER_3, test_reward_account_param()), None ); }); diff --git a/bridges/modules/relayers/src/stake_adapter.rs b/bridges/modules/relayers/src/stake_adapter.rs index 7ba90d91dfd94..ce097344d56be 100644 --- a/bridges/modules/relayers/src/stake_adapter.rs +++ b/bridges/modules/relayers/src/stake_adapter.rs @@ -130,7 +130,7 @@ mod tests { #[test] fn repatriate_reserved_works() { run_test(|| { - let beneficiary = TEST_REWARDS_ACCOUNT_PARAMS; + let beneficiary = test_reward_account_param(); let beneficiary_account = TestPaymentProcedure::rewards_account(beneficiary); let mut expected_balance = ExistentialDeposit::get(); @@ -186,7 +186,7 @@ mod tests { #[test] fn repatriate_reserved_doesnt_work_when_beneficiary_account_is_missing() { run_test(|| { - let beneficiary = TEST_REWARDS_ACCOUNT_PARAMS; + let beneficiary = test_reward_account_param(); let beneficiary_account = TestPaymentProcedure::rewards_account(beneficiary); Balances::mint_into(&3, test_stake() * 2).unwrap(); diff --git a/bridges/modules/xcm-bridge-hub/src/exporter.rs b/bridges/modules/xcm-bridge-hub/src/exporter.rs index 2ed8c46e7890c..af803900e5d7e 100644 --- a/bridges/modules/xcm-bridge-hub/src/exporter.rs +++ b/bridges/modules/xcm-bridge-hub/src/exporter.rs @@ -148,7 +148,7 @@ mod tests { run_test(|| { sp_tracing::try_init_simple(); pallet_bridge_messages::OutboundLanes::::insert( - TEST_LANE_ID, + test_lane_id(), OutboundLaneData { state: LaneState::Opened, ..Default::default() }, ); assert_ok!(export_xcm::( @@ -192,7 +192,7 @@ mod tests { fn exporter_computes_correct_lane_id() { run_test(|| { sp_tracing::try_init_simple(); - let expected_lane_id = TEST_LANE_ID; + let expected_lane_id = test_lane_id(); pallet_bridge_messages::OutboundLanes::::insert( expected_lane_id, diff --git a/bridges/modules/xcm-bridge-hub/src/mock.rs b/bridges/modules/xcm-bridge-hub/src/mock.rs index c988c8167a328..b9af95c27cb6f 100644 --- a/bridges/modules/xcm-bridge-hub/src/mock.rs +++ b/bridges/modules/xcm-bridge-hub/src/mock.rs @@ -32,7 +32,7 @@ use bridge_runtime_common::{ }; use codec::Encode; use frame_support::{derive_impl, parameter_types, weights::RuntimeDbWeight}; -use sp_core::H256; +use sp_core::{Get, H256}; use sp_runtime::{ testing::Header as SubstrateHeader, traits::{BlakeTwo256, IdentityLookup}, @@ -48,7 +48,11 @@ type Block = frame_system::mocking::MockBlock; pub const SIBLING_ASSET_HUB_ID: u32 = 2001; pub const THIS_BRIDGE_HUB_ID: u32 = 2002; pub const BRIDGED_ASSET_HUB_ID: u32 = 1001; -pub const TEST_LANE_ID: LaneId = LaneId([0, 0, 0, 1]); + +/// Message lane used in tests. +pub fn test_lane_id() -> LaneId { + bridge_runtime_common::messages_xcm_extension::LaneIdFromChainId::::get() +} frame_support::construct_runtime! { pub enum TestRuntime { @@ -77,10 +81,6 @@ impl pallet_balances::Config for TestRuntime { type AccountStore = System; } -parameter_types! { - pub const ActiveOutboundLanes: &'static [LaneId] = &[TEST_LANE_ID]; -} - impl pallet_bridge_messages::Config for TestRuntime { type RuntimeEvent = RuntimeEvent; type WeightInfo = TestMessagesWeights; @@ -169,7 +169,7 @@ impl pallet_xcm_bridge_hub::Config for TestRuntime { parameter_types! { pub TestSenderAndLane: SenderAndLane = SenderAndLane { location: Location::new(1, [Parachain(SIBLING_ASSET_HUB_ID)]), - lane: TEST_LANE_ID, + lane: test_lane_id(), }; pub BridgedDestination: InteriorLocation = [ Parachain(BRIDGED_ASSET_HUB_ID) diff --git a/bridges/primitives/messages/Cargo.toml b/bridges/primitives/messages/Cargo.toml index 20337873c2e6a..a9cce3c5ecba6 100644 --- a/bridges/primitives/messages/Cargo.toml +++ b/bridges/primitives/messages/Cargo.toml @@ -25,6 +25,7 @@ bp-header-chain = { path = "../header-chain", default-features = false } frame-support = { path = "../../../substrate/frame/support", default-features = false } sp-core = { path = "../../../substrate/primitives/core", default-features = false } sp-std = { path = "../../../substrate/primitives/std", default-features = false } +sp-io = { path = "../../../substrate/primitives/io", default-features = false } [dev-dependencies] hex = "0.4" @@ -40,5 +41,6 @@ std = [ "scale-info/std", "serde/std", "sp-core/std", - "sp-std/std", + "sp-io/std", + "sp-std/std" ] diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index 81841892c7660..69f254c2db830 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/bridges/primitives/messages/src/lib.rs @@ -31,7 +31,8 @@ pub use frame_support::weights::Weight; use scale_info::TypeInfo; use serde::{Deserialize, Serialize}; use source_chain::RelayersRewards; -use sp_core::{RuntimeDebug, TypeId}; +use sp_core::{RuntimeDebug, TypeId, H256}; +use sp_io::hashing::blake2_256; use sp_std::{collections::vec_deque::VecDeque, ops::RangeInclusive, prelude::*}; pub mod source_chain; @@ -165,7 +166,24 @@ impl OperatingMode for MessagesOperatingMode { } } -/// Lane id which implements `TypeId`. +/// Bridge lane identifier. +/// +/// Lane connects two endpoints at both sides of the bridge. We assume that every endpoint +/// has its own unique identifier. We want lane identifiers to be the same on the both sides +/// of the bridge (and naturally unique across global consensus if endpoints have unique +/// identifiers). So lane id is the hash (`blake2_256`) of **ordered** encoded locations +/// concatenation (separated by some binary data). I.e.: +/// +/// ```nocompile +/// let endpoint1 = X2(GlobalConsensus(NetworkId::Rococo), Parachain(42)); +/// let endpoint2 = X2(GlobalConsensus(NetworkId::Wococo), Parachain(777)); +/// +/// let final_lane_key = if endpoint1 < endpoint2 { +/// (endpoint1, VALUES_SEPARATOR, endpoint2) +/// } else { +/// (endpoint2, VALUES_SEPARATOR, endpoint1) +/// }.using_encoded(blake2_256); +/// ``` #[derive( Clone, Copy, @@ -181,7 +199,39 @@ impl OperatingMode for MessagesOperatingMode { Serialize, Deserialize, )] -pub struct LaneId(pub [u8; 4]); +pub struct LaneId(H256); + +impl LaneId { + /// Create lane identifier from two locations. + pub fn new(endpoint1: T, endpoint2: T) -> Self { + const VALUES_SEPARATOR: [u8; 31] = *b"bridges-lane-id-value-separator"; + + LaneId( + if endpoint1 < endpoint2 { + (endpoint1, VALUES_SEPARATOR, endpoint2) + } else { + (endpoint2, VALUES_SEPARATOR, endpoint1) + } + .using_encoded(blake2_256) + .into(), + ) + } + + /// Create lane identifier from given hash. + /// + /// There's no `From` implementation for the `LaneId`, because using this conversion + /// in a wrong way (i.e. computing hash of endpoints manually) may lead to issues. So we + /// want the call to be explicit. + pub const fn from_inner(hash: H256) -> Self { + LaneId(hash) + } +} + +impl core::fmt::Display for LaneId { + fn fmt(&self, fmt: &mut core::fmt::Formatter) -> core::fmt::Result { + self.0.fmt(fmt) + } +} impl core::fmt::Debug for LaneId { fn fmt(&self, fmt: &mut core::fmt::Formatter) -> core::fmt::Result { @@ -189,8 +239,8 @@ impl core::fmt::Debug for LaneId { } } -impl AsRef<[u8]> for LaneId { - fn as_ref(&self) -> &[u8] { +impl AsRef for LaneId { + fn as_ref(&self) -> &H256 { &self.0 } } @@ -380,7 +430,7 @@ pub struct UnrewardedRelayer { } /// Received messages with their dispatch result. -#[derive(Clone, Default, Encode, Decode, RuntimeDebug, PartialEq, Eq, TypeInfo)] +#[derive(Clone, Encode, Decode, RuntimeDebug, PartialEq, Eq, TypeInfo)] pub struct ReceivedMessages { /// Id of the lane which is receiving messages. pub lane: LaneId, @@ -684,7 +734,50 @@ mod tests { } #[test] - fn lane_id_debug_format_matches_inner_array_format() { - assert_eq!(format!("{:?}", LaneId([0, 0, 0, 0])), format!("{:?}", [0, 0, 0, 0]),); + fn lane_id_debug_format_matches_inner_hash_format() { + assert_eq!( + format!("{:?}", LaneId(H256::from([1u8; 32]))), + format!("{:?}", H256::from([1u8; 32])), + ); + } + + #[test] + fn lane_id_is_generated_using_ordered_endpoints() { + assert_eq!(LaneId::new(1, 2), LaneId::new(2, 1)); + } + + #[test] + fn lane_id_is_different_for_different_endpoints() { + assert_ne!(LaneId::new(1, 2), LaneId::new(1, 3)); + } + + #[test] + fn lane_id_is_different_even_if_arguments_has_partial_matching_encoding() { + /// Some artificial type that generates the same encoding for different values + /// concatenations. I.e. the encoding for `(Either::Two(1, 2), Either::Two(3, 4))` + /// is the same as encoding of `(Either::Three(1, 2, 3), Either::One(4))`. + /// In practice, this type is not useful, because you can't do a proper decoding. + /// But still there may be some collisions even in proper types. + #[derive(Eq, Ord, PartialEq, PartialOrd)] + enum Either { + Three(u64, u64, u64), + Two(u64, u64), + One(u64), + } + + impl codec::Encode for Either { + fn encode(&self) -> Vec { + match *self { + Self::One(a) => a.encode(), + Self::Two(a, b) => (a, b).encode(), + Self::Three(a, b, c) => (a, b, c).encode(), + } + } + } + + assert_ne!( + LaneId::new(Either::Two(1, 2), Either::Two(3, 4)), + LaneId::new(Either::Three(1, 2, 3), Either::One(4)), + ); } } diff --git a/bridges/primitives/messages/src/storage_keys.rs b/bridges/primitives/messages/src/storage_keys.rs index 8eedf8fcc7ac9..3e7b04a50b0c8 100644 --- a/bridges/primitives/messages/src/storage_keys.rs +++ b/bridges/primitives/messages/src/storage_keys.rs @@ -91,10 +91,10 @@ mod tests { fn storage_message_key_computed_properly() { // If this test fails, then something has been changed in module storage that is breaking // all previously crafted messages proofs. - let storage_key = message_key("BridgeMessages", &LaneId(*b"test"), 42).0; + let storage_key = message_key("BridgeMessages", &LaneId::new(1, 2), 42).0; assert_eq!( storage_key, - hex!("dd16c784ebd3390a9bc0357c7511ed018a395e6242c6813b196ca31ed0547ea79446af0e09063bd4a7874aef8a997cec746573742a00000000000000").to_vec(), + hex!("dd16c784ebd3390a9bc0357c7511ed018a395e6242c6813b196ca31ed0547ea70e9bdb8f50c68d12f06eabb57759ee5eb1d3dccd8b3c3a012afe265f3e3c4432129b8aee50c9dcf87f9793be208e5ea02a00000000000000").to_vec(), "Unexpected storage key: {}", hex::encode(&storage_key), ); @@ -104,10 +104,10 @@ mod tests { fn outbound_lane_data_key_computed_properly() { // If this test fails, then something has been changed in module storage that is breaking // all previously crafted outbound lane state proofs. - let storage_key = outbound_lane_data_key("BridgeMessages", &LaneId(*b"test")).0; + let storage_key = outbound_lane_data_key("BridgeMessages", &LaneId::new(1, 2)).0; assert_eq!( storage_key, - hex!("dd16c784ebd3390a9bc0357c7511ed0196c246acb9b55077390e3ca723a0ca1f44a8995dd50b6657a037a7839304535b74657374").to_vec(), + hex!("dd16c784ebd3390a9bc0357c7511ed0196c246acb9b55077390e3ca723a0ca1fd3bef8b00df8ca7b01813b5e2741950db1d3dccd8b3c3a012afe265f3e3c4432129b8aee50c9dcf87f9793be208e5ea0").to_vec(), "Unexpected storage key: {}", hex::encode(&storage_key), ); @@ -117,10 +117,10 @@ mod tests { fn inbound_lane_data_key_computed_properly() { // If this test fails, then something has been changed in module storage that is breaking // all previously crafted inbound lane state proofs. - let storage_key = inbound_lane_data_key("BridgeMessages", &LaneId(*b"test")).0; + let storage_key = inbound_lane_data_key("BridgeMessages", &LaneId::new(1, 2)).0; assert_eq!( storage_key, - hex!("dd16c784ebd3390a9bc0357c7511ed01e5f83cf83f2127eb47afdc35d6e43fab44a8995dd50b6657a037a7839304535b74657374").to_vec(), + hex!("dd16c784ebd3390a9bc0357c7511ed01e5f83cf83f2127eb47afdc35d6e43fabd3bef8b00df8ca7b01813b5e2741950db1d3dccd8b3c3a012afe265f3e3c4432129b8aee50c9dcf87f9793be208e5ea0").to_vec(), "Unexpected storage key: {}", hex::encode(&storage_key), ); diff --git a/bridges/primitives/relayers/src/lib.rs b/bridges/primitives/relayers/src/lib.rs index 436f33db40080..704fef7c8e6ba 100644 --- a/bridges/primitives/relayers/src/lib.rs +++ b/bridges/primitives/relayers/src/lib.rs @@ -57,9 +57,12 @@ pub enum RewardsAccountOwner { /// parameters to identify the account that pays a reward to the relayer. #[derive(Copy, Clone, Debug, Decode, Encode, Eq, PartialEq, TypeInfo, MaxEncodedLen)] pub struct RewardsAccountParams { - lane_id: LaneId, - bridged_chain_id: ChainId, + // **IMPORTANT NOTE**: the order of fields here matters - we are using + // `into_account_truncating` and lane id is already `32` byte, so if other fields are encoded + // after it, they're simply dropped. So lane id shall be the last field. owner: RewardsAccountOwner, + bridged_chain_id: ChainId, + lane_id: LaneId, } impl RewardsAccountParams { @@ -162,21 +165,21 @@ mod tests { fn different_lanes_are_using_different_accounts() { assert_eq!( PayRewardFromAccount::<(), H256>::rewards_account(RewardsAccountParams::new( - LaneId([0, 0, 0, 0]), + LaneId::new(1, 2), *b"test", RewardsAccountOwner::ThisChain )), - hex_literal::hex!("62726170000000007465737400726577617264732d6163636f756e7400000000") + hex_literal::hex!("627261700074657374b1d3dccd8b3c3a012afe265f3e3c4432129b8aee50c9dc") .into(), ); assert_eq!( PayRewardFromAccount::<(), H256>::rewards_account(RewardsAccountParams::new( - LaneId([0, 0, 0, 1]), + LaneId::new(1, 3), *b"test", RewardsAccountOwner::ThisChain )), - hex_literal::hex!("62726170000000017465737400726577617264732d6163636f756e7400000000") + hex_literal::hex!("627261700074657374a43e8951aa302c133beb5f85821a21645f07b487270ef3") .into(), ); } @@ -185,21 +188,21 @@ mod tests { fn different_directions_are_using_different_accounts() { assert_eq!( PayRewardFromAccount::<(), H256>::rewards_account(RewardsAccountParams::new( - LaneId([0, 0, 0, 0]), + LaneId::new(1, 2), *b"test", RewardsAccountOwner::ThisChain )), - hex_literal::hex!("62726170000000007465737400726577617264732d6163636f756e7400000000") + hex_literal::hex!("627261700074657374b1d3dccd8b3c3a012afe265f3e3c4432129b8aee50c9dc") .into(), ); assert_eq!( PayRewardFromAccount::<(), H256>::rewards_account(RewardsAccountParams::new( - LaneId([0, 0, 0, 0]), + LaneId::new(1, 2), *b"test", RewardsAccountOwner::BridgedChain )), - hex_literal::hex!("62726170000000007465737401726577617264732d6163636f756e7400000000") + hex_literal::hex!("627261700174657374b1d3dccd8b3c3a012afe265f3e3c4432129b8aee50c9dc") .into(), ); } diff --git a/bridges/primitives/runtime/src/storage_proof.rs b/bridges/primitives/runtime/src/storage_proof.rs index a780a508bdd35..4ca38abf209e9 100644 --- a/bridges/primitives/runtime/src/storage_proof.rs +++ b/bridges/primitives/runtime/src/storage_proof.rs @@ -21,8 +21,8 @@ use sp_core::{storage::TrackedStorageKey, RuntimeDebug}; use sp_runtime::{SaturatedConversion, StateVersion}; use sp_std::{default::Default, vec, vec::Vec}; use sp_trie::{ - generate_trie_proof, verify_trie_proof, LayoutV0, LayoutV1, PrefixedMemoryDB, StorageProof, - TrieDBBuilder, TrieHash, + generate_trie_proof, verify_trie_proof, LayoutV0, LayoutV1, StorageProof, TrieDBBuilder, + TrieHash, }; use codec::{Decode, Encode}; @@ -108,8 +108,10 @@ impl UnverifiedStorageProof { let keys: Vec<_> = entries.iter().map(|(key, _)| key.clone()).collect(); let entries: Vec<_> = entries.iter().cloned().map(|(key, val)| (None, vec![(key, val)])).collect(); - let backend = - sp_state_machine::TrieBackend::, H>::from((entries, state_version)); + let backend = sp_state_machine::TrieBackend::, H>::from(( + entries, + state_version, + )); let root = *backend.root(); Ok((root, UnverifiedStorageProof::try_from_db(backend.backend_storage(), root, keys)?)) diff --git a/bridges/relays/lib-substrate-relay/Cargo.toml b/bridges/relays/lib-substrate-relay/Cargo.toml index 077d1b1ff356a..08633a2bc6873 100644 --- a/bridges/relays/lib-substrate-relay/Cargo.toml +++ b/bridges/relays/lib-substrate-relay/Cargo.toml @@ -20,6 +20,7 @@ hex = "0.4" log = { workspace = true } num-traits = "0.2" rbtag = "0.3" +rustc-hex = "2.1" structopt = "0.3" strum = { version = "0.26.2", features = ["derive"] } thiserror = { workspace = true } diff --git a/bridges/relays/lib-substrate-relay/src/cli/mod.rs b/bridges/relays/lib-substrate-relay/src/cli/mod.rs index b367b09d45500..74458f7068df9 100644 --- a/bridges/relays/lib-substrate-relay/src/cli/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/cli/mod.rs @@ -18,6 +18,7 @@ use codec::{Decode, Encode}; use rbtag::BuildInfo; +use sp_core::H256; use structopt::StructOpt; use strum::{EnumString, VariantNames}; @@ -42,21 +43,19 @@ pub type DefaultClient = relay_substrate_client::RpcWithCachingClient; /// Lane id. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct HexLaneId(pub [u8; 4]); +pub struct HexLaneId(pub H256); impl From for LaneId { fn from(lane_id: HexLaneId) -> LaneId { - LaneId(lane_id.0) + LaneId::from_inner(lane_id.0) } } impl std::str::FromStr for HexLaneId { - type Err = hex::FromHexError; + type Err = rustc_hex::FromHexError; fn from_str(s: &str) -> Result { - let mut lane_id = [0u8; 4]; - hex::decode_to_slice(s, &mut lane_id)?; - Ok(HexLaneId(lane_id)) + Ok(HexLaneId(H256::from_str(s)?)) } } diff --git a/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs b/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs index 338dda3c63309..8e524fe22dec3 100644 --- a/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs @@ -55,7 +55,7 @@ use sp_core::Pair; #[derive(Debug, PartialEq, StructOpt)] pub struct HeadersAndMessagesSharedParams { /// Hex-encoded lane identifiers that should be served by the complex relay. - #[structopt(long, default_value = "00000000")] + #[structopt(long)] pub lane: Vec, /// If passed, only mandatory headers (headers that are changing the GRANDPA authorities set) /// are relayed. @@ -359,6 +359,7 @@ mod tests { use crate::{cli::chain_schema::RuntimeVersionType, declare_chain_cli_schema}; use relay_substrate_client::{ChainRuntimeVersion, Parachain, SimpleRuntimeVersion}; + use sp_core::H256; #[test] // We need `#[allow(dead_code)]` because some of the methods generated by the macros @@ -422,7 +423,7 @@ mod tests { "--polkadot-port", "9944", "--lane", - "00000000", + "0000000000000000000000000000000000000000000000000000000000000000", "--prometheus-host", "0.0.0.0", ]); @@ -432,7 +433,7 @@ mod tests { res, BridgeHubKusamaBridgeHubPolkadotHeadersAndMessages { shared: HeadersAndMessagesSharedParams { - lane: vec![HexLaneId([0x00, 0x00, 0x00, 0x00])], + lane: vec![HexLaneId(H256::from([0x00u8; 32]))], only_mandatory_headers: false, only_free_headers: false, prometheus_params: PrometheusParams { diff --git a/bridges/relays/lib-substrate-relay/src/cli/relay_messages.rs b/bridges/relays/lib-substrate-relay/src/cli/relay_messages.rs index c711b63c6d046..4377c057f58dc 100644 --- a/bridges/relays/lib-substrate-relay/src/cli/relay_messages.rs +++ b/bridges/relays/lib-substrate-relay/src/cli/relay_messages.rs @@ -37,8 +37,8 @@ use relay_utils::UniqueSaturatedInto; /// Messages relaying params. #[derive(StructOpt)] pub struct RelayMessagesParams { - /// Hex-encoded lane id that should be served by the relay. Defaults to `00000000`. - #[structopt(long, default_value = "00000000")] + /// Hex-encoded lane id that should be served by the relay. + #[structopt(long)] lane: HexLaneId, #[structopt(flatten)] source: SourceConnectionParams, @@ -59,8 +59,8 @@ pub struct RelayMessagesRangeParams { /// This header must be previously proved to the target chain. #[structopt(long)] at_source_block: u128, - /// Hex-encoded lane id that should be served by the relay. Defaults to `00000000`. - #[structopt(long, default_value = "00000000")] + /// Hex-encoded lane id that should be served by the relay. + #[structopt(long)] lane: HexLaneId, /// Nonce (inclusive) of the first message to relay. #[structopt(long)] diff --git a/bridges/relays/lib-substrate-relay/src/messages/mod.rs b/bridges/relays/lib-substrate-relay/src/messages/mod.rs index 7b2916186f966..023bdab0e5296 100644 --- a/bridges/relays/lib-substrate-relay/src/messages/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/messages/mod.rs @@ -606,7 +606,7 @@ where FromBridgedChainMessagesProof { bridged_header_hash: Default::default(), storage: Default::default(), - lane: Default::default(), + lane: LaneId::new(1, 2), nonces_start: 1, nonces_end: messages as u64, }, diff --git a/bridges/relays/lib-substrate-relay/src/messages/source.rs b/bridges/relays/lib-substrate-relay/src/messages/source.rs index 8f0e1b54460cd..ae55957fcee23 100644 --- a/bridges/relays/lib-substrate-relay/src/messages/source.rs +++ b/bridges/relays/lib-substrate-relay/src/messages/source.rs @@ -659,7 +659,7 @@ mod tests { } let maybe_batches = - split_msgs_to_refine::(Default::default(), msgs_to_refine); + split_msgs_to_refine::(LaneId::new(1, 2), msgs_to_refine); match expected_batches { Ok(expected_batches) => { let batches = maybe_batches.unwrap(); diff --git a/bridges/relays/messages/src/message_lane_loop.rs b/bridges/relays/messages/src/message_lane_loop.rs index b681d86d2ae8f..995499092c3ee 100644 --- a/bridges/relays/messages/src/message_lane_loop.rs +++ b/bridges/relays/messages/src/message_lane_loop.rs @@ -276,7 +276,7 @@ pub struct ClientsState { /// Return prefix that will be used by default to expose Prometheus metrics of the finality proofs /// sync loop. pub fn metrics_prefix(lane: &LaneId) -> String { - format!("{}_to_{}_MessageLane_{}", P::SOURCE_NAME, P::TARGET_NAME, hex::encode(lane)) + format!("{}_to_{}_MessageLane_{:?}", P::SOURCE_NAME, P::TARGET_NAME, lane) } /// Run message lane service loop. @@ -957,7 +957,7 @@ pub(crate) mod tests { }; let _ = run( Params { - lane: LaneId([0, 0, 0, 0]), + lane: LaneId::new(1, 2), source_tick: Duration::from_millis(100), target_tick: Duration::from_millis(100), reconnect_delay: Duration::from_millis(0), @@ -1274,4 +1274,12 @@ pub(crate) mod tests { assert!(!result.target_to_source_header_requirements.is_empty()); assert!(!result.source_to_target_header_requirements.is_empty()); } + + #[test] + fn metrics_prefix_is_valid() { + assert!(MessageLaneLoopMetrics::new(Some(&metrics_prefix::( + &LaneId::new(1, 2) + ))) + .is_ok()); + } }