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 6ba3506377d0..0b74fa898bca 100644 --- a/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs +++ b/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs @@ -943,7 +943,7 @@ pub(crate) mod tests { use bp_header_chain::StoredHeaderDataBuilder; use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, - target_chain::FromBridgedChainMessagesProof, DeliveredMessages, InboundLaneData, + target_chain::FromBridgedChainMessagesProof, DeliveredMessages, InboundLaneData, LaneState, MessageNonce, MessagesOperatingMode, OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, }; @@ -1149,6 +1149,7 @@ pub(crate) mod tests { nonces_start: pallet_bridge_messages::InboundLanes::::get( TEST_LANE_ID, ) + .unwrap() .last_delivered_nonce() + 1, nonces_end: best_message, @@ -2884,6 +2885,7 @@ pub(crate) mod tests { // allow empty message delivery transactions let lane_id = TestLaneId::get(); let in_lane_data = InboundLaneData { + state: LaneState::Opened, last_confirmed_nonce: 0, relayers: vec![UnrewardedRelayer { relayer: relayer_account_at_bridged_chain(), diff --git a/bridges/bin/runtime-common/src/integrity.rs b/bridges/bin/runtime-common/src/integrity.rs index f661db8a2205..339da4dd39e4 100644 --- a/bridges/bin/runtime-common/src/integrity.rs +++ b/bridges/bin/runtime-common/src/integrity.rs @@ -165,12 +165,6 @@ where R: pallet_bridge_messages::Config, MI: 'static, { - assert!( - !R::ActiveOutboundLanes::get().is_empty(), - "ActiveOutboundLanes ({:?}) must not be empty", - R::ActiveOutboundLanes::get(), - ); - assert!( pallet_bridge_messages::BridgedChainOf::::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX <= pallet_bridge_messages::BridgedChainOf::::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX, diff --git a/bridges/bin/runtime-common/src/messages_call_ext.rs b/bridges/bin/runtime-common/src/messages_call_ext.rs index a9ee1969ae0c..5eadb0a32480 100644 --- a/bridges/bin/runtime-common/src/messages_call_ext.rs +++ b/bridges/bin/runtime-common/src/messages_call_ext.rs @@ -146,7 +146,10 @@ impl, I: 'static> CallHelper { match info { CallInfo::ReceiveMessagesProof(info) => { let inbound_lane_data = - pallet_bridge_messages::InboundLanes::::get(info.base.lane_id); + match pallet_bridge_messages::InboundLanes::::get(info.base.lane_id) { + Some(inbound_lane_data) => inbound_lane_data, + None => return false, + }; if info.base.bundled_range.is_empty() { let post_occupation = unrewarded_relayers_occupation::(&inbound_lane_data); @@ -162,7 +165,10 @@ impl, I: 'static> CallHelper { }, CallInfo::ReceiveMessagesDeliveryProof(info) => { let outbound_lane_data = - pallet_bridge_messages::OutboundLanes::::get(info.0.lane_id); + match pallet_bridge_messages::OutboundLanes::::get(info.0.lane_id) { + Some(outbound_lane_data) => outbound_lane_data, + None => return false, + }; outbound_lane_data.latest_received_nonce == *info.0.bundled_range.end() }, } @@ -219,7 +225,7 @@ impl< .. }) = self.is_sub_type() { - let inbound_lane_data = pallet_bridge_messages::InboundLanes::::get(proof.lane); + let inbound_lane_data = pallet_bridge_messages::InboundLanes::::get(proof.lane)?; return Some(ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { @@ -243,7 +249,8 @@ impl< .. }) = self.is_sub_type() { - let outbound_lane_data = pallet_bridge_messages::OutboundLanes::::get(proof.lane); + let outbound_lane_data = + pallet_bridge_messages::OutboundLanes::::get(proof.lane)?; return Some(ReceiveMessagesDeliveryProofInfo(BaseMessagesProofInfo { lane_id: proof.lane, @@ -349,14 +356,16 @@ mod tests { }; use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, - target_chain::FromBridgedChainMessagesProof, DeliveredMessages, UnrewardedRelayer, - UnrewardedRelayersState, + target_chain::FromBridgedChainMessagesProof, DeliveredMessages, InboundLaneData, LaneState, + OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, }; use sp_std::ops::RangeInclusive; + const TEST_LANE_ID: LaneId = LaneId([0, 0, 0, 0]); + fn fill_unrewarded_relayers() { let mut inbound_lane_state = - pallet_bridge_messages::InboundLanes::::get(LaneId([0, 0, 0, 0])); + 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(), @@ -364,14 +373,14 @@ mod tests { }); } pallet_bridge_messages::InboundLanes::::insert( - LaneId([0, 0, 0, 0]), + TEST_LANE_ID, inbound_lane_state, ); } fn fill_unrewarded_messages() { let mut inbound_lane_state = - pallet_bridge_messages::InboundLanes::::get(LaneId([0, 0, 0, 0])); + pallet_bridge_messages::InboundLanes::::get(TEST_LANE_ID).unwrap(); inbound_lane_state.relayers.push_back(UnrewardedRelayer { relayer: Default::default(), messages: DeliveredMessages { @@ -380,15 +389,19 @@ mod tests { }, }); pallet_bridge_messages::InboundLanes::::insert( - LaneId([0, 0, 0, 0]), + TEST_LANE_ID, inbound_lane_state, ); } fn deliver_message_10() { pallet_bridge_messages::InboundLanes::::insert( - LaneId([0, 0, 0, 0]), - bp_messages::InboundLaneData { relayers: Default::default(), last_confirmed_nonce: 10 }, + TEST_LANE_ID, + bp_messages::InboundLaneData { + state: LaneState::Opened, + relayers: Default::default(), + last_confirmed_nonce: 10, + }, ); } @@ -405,7 +418,7 @@ mod tests { proof: Box::new(FromBridgedChainMessagesProof { bridged_header_hash: Default::default(), storage_proof: Default::default(), - lane: LaneId([0, 0, 0, 0]), + lane: TEST_LANE_ID, nonces_start, nonces_end, }), @@ -415,9 +428,23 @@ mod tests { .is_ok() } + fn run_test(test: impl Fn() -> T) -> T { + sp_io::TestExternalities::new(Default::default()).execute_with(|| { + pallet_bridge_messages::InboundLanes::::insert( + TEST_LANE_ID, + InboundLaneData::opened(), + ); + pallet_bridge_messages::OutboundLanes::::insert( + TEST_LANE_ID, + OutboundLaneData::opened(), + ); + test() + }) + } + #[test] fn extension_rejects_obsolete_messages() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { // when current best delivered is message#10 and we're trying to deliver messages 8..=9 // => tx is rejected deliver_message_10(); @@ -427,7 +454,7 @@ mod tests { #[test] fn extension_rejects_same_message() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { // when current best delivered is message#10 and we're trying to import messages 10..=10 // => tx is rejected deliver_message_10(); @@ -437,7 +464,7 @@ mod tests { #[test] fn extension_rejects_call_with_some_obsolete_messages() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { // when current best delivered is message#10 and we're trying to deliver messages // 10..=15 => tx is rejected deliver_message_10(); @@ -447,7 +474,7 @@ mod tests { #[test] fn extension_rejects_call_with_future_messages() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { // when current best delivered is message#10 and we're trying to deliver messages // 13..=15 => tx is rejected deliver_message_10(); @@ -470,7 +497,7 @@ mod tests { #[test] fn extension_rejects_empty_delivery_with_rewards_confirmations_if_there_are_free_relayer_and_message_slots( ) { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { deliver_message_10(); assert!(!validate_message_delivery(10, 9)); }); @@ -479,7 +506,7 @@ mod tests { #[test] fn extension_accepts_empty_delivery_with_rewards_confirmations_if_there_are_no_free_relayer_slots( ) { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { deliver_message_10(); fill_unrewarded_relayers(); assert!(validate_message_delivery(10, 9)); @@ -489,7 +516,7 @@ mod tests { #[test] fn extension_accepts_empty_delivery_with_rewards_confirmations_if_there_are_no_free_message_slots( ) { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { fill_unrewarded_messages(); assert!(validate_message_delivery( BridgedUnderlyingChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX, @@ -500,7 +527,7 @@ mod tests { #[test] fn extension_accepts_new_messages() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { // when current best delivered is message#10 and we're trying to deliver message 11..=15 // => tx is accepted deliver_message_10(); @@ -510,8 +537,9 @@ mod tests { fn confirm_message_10() { pallet_bridge_messages::OutboundLanes::::insert( - LaneId([0, 0, 0, 0]), + TEST_LANE_ID, bp_messages::OutboundLaneData { + state: LaneState::Opened, oldest_unpruned_nonce: 0, latest_received_nonce: 10, latest_generated_nonce: 10, @@ -525,7 +553,7 @@ mod tests { proof: FromBridgedChainMessagesDeliveryProof { bridged_header_hash: Default::default(), storage_proof: Default::default(), - lane: LaneId([0, 0, 0, 0]), + lane: TEST_LANE_ID, }, relayers_state: UnrewardedRelayersState { last_delivered_nonce, @@ -539,7 +567,7 @@ mod tests { #[test] fn extension_rejects_obsolete_confirmations() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { // when current best confirmed is message#10 and we're trying to confirm message#5 => tx // is rejected confirm_message_10(); @@ -549,7 +577,7 @@ mod tests { #[test] fn extension_rejects_same_confirmation() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { // when current best confirmed is message#10 and we're trying to confirm message#10 => // tx is rejected confirm_message_10(); @@ -559,7 +587,7 @@ mod tests { #[test] fn extension_rejects_empty_confirmation_even_if_there_are_no_free_unrewarded_entries() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { confirm_message_10(); fill_unrewarded_relayers(); assert!(!validate_message_confirmation(10)); @@ -568,7 +596,7 @@ mod tests { #[test] fn extension_accepts_new_confirmation() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { // when current best confirmed is message#10 and we're trying to confirm message#15 => // tx is accepted confirm_message_10(); @@ -583,7 +611,7 @@ mod tests { CallHelper::::was_successful(&CallInfo::ReceiveMessagesProof( ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { - lane_id: LaneId([0, 0, 0, 0]), + lane_id: TEST_LANE_ID, bundled_range, best_stored_nonce: 0, // doesn't matter for `was_successful` }, @@ -602,7 +630,7 @@ mod tests { #[test] #[allow(clippy::reversed_empty_ranges)] fn was_successful_returns_false_for_failed_reward_confirmation_transaction() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { fill_unrewarded_messages(); assert!(!was_message_delivery_successful(10..=9, true)); }); @@ -611,14 +639,14 @@ mod tests { #[test] #[allow(clippy::reversed_empty_ranges)] fn was_successful_returns_true_for_successful_reward_confirmation_transaction() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { assert!(was_message_delivery_successful(10..=9, true)); }); } #[test] fn was_successful_returns_false_for_failed_delivery() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { deliver_message_10(); assert!(!was_message_delivery_successful(10..=12, false)); }); @@ -626,7 +654,7 @@ mod tests { #[test] fn was_successful_returns_false_for_partially_successful_delivery() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { deliver_message_10(); assert!(!was_message_delivery_successful(9..=12, false)); }); @@ -634,7 +662,7 @@ mod tests { #[test] fn was_successful_returns_true_for_successful_delivery() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { deliver_message_10(); assert!(was_message_delivery_successful(9..=10, false)); }); @@ -643,7 +671,7 @@ mod tests { fn was_message_confirmation_successful(bundled_range: RangeInclusive) -> bool { CallHelper::::was_successful(&CallInfo::ReceiveMessagesDeliveryProof( ReceiveMessagesDeliveryProofInfo(BaseMessagesProofInfo { - lane_id: LaneId([0, 0, 0, 0]), + lane_id: TEST_LANE_ID, bundled_range, best_stored_nonce: 0, // doesn't matter for `was_successful` }), @@ -652,7 +680,7 @@ mod tests { #[test] fn was_successful_returns_false_for_failed_confirmation() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { confirm_message_10(); assert!(!was_message_confirmation_successful(10..=12)); }); @@ -660,7 +688,7 @@ mod tests { #[test] fn was_successful_returns_false_for_partially_successful_confirmation() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { confirm_message_10(); assert!(!was_message_confirmation_successful(9..=12)); }); @@ -668,7 +696,7 @@ mod tests { #[test] fn was_successful_returns_true_for_successful_confirmation() { - sp_io::TestExternalities::new(Default::default()).execute_with(|| { + run_test(|| { confirm_message_10(); assert!(was_message_confirmation_successful(9..=10)); }); diff --git a/bridges/bin/runtime-common/src/messages_xcm_extension.rs b/bridges/bin/runtime-common/src/messages_xcm_extension.rs index 46ed4da0d854..0ea2f8b9619a 100644 --- a/bridges/bin/runtime-common/src/messages_xcm_extension.rs +++ b/bridges/bin/runtime-common/src/messages_xcm_extension.rs @@ -339,7 +339,7 @@ mod tests { use super::*; use crate::mock::*; - use bp_messages::OutboundLaneData; + use bp_messages::{LaneState, OutboundLaneData}; use frame_support::parameter_types; use pallet_bridge_messages::OutboundLanes; @@ -397,6 +397,7 @@ mod tests { OutboundLanes::::insert( TEST_LANE_ID, OutboundLaneData { + state: LaneState::Opened, oldest_unpruned_nonce: 0, latest_received_nonce: 0, latest_generated_nonce, diff --git a/bridges/bin/runtime-common/src/mock.rs b/bridges/bin/runtime-common/src/mock.rs index 2f248a7162a6..decbf5c687d1 100644 --- a/bridges/bin/runtime-common/src/mock.rs +++ b/bridges/bin/runtime-common/src/mock.rs @@ -111,7 +111,6 @@ crate::generate_bridge_reject_obsolete_headers_and_messages! { } parameter_types! { - pub const ActiveOutboundLanes: &'static [LaneId] = &[TEST_LANE_ID]; pub const BridgedParasPalletName: &'static str = "Paras"; pub const ExistentialDeposit: ThisChainBalance = 500; pub const DbWeight: RuntimeDbWeight = RuntimeDbWeight { read: 1, write: 2 }; @@ -185,7 +184,6 @@ impl pallet_bridge_parachains::Config for TestRuntime { impl pallet_bridge_messages::Config for TestRuntime { type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_bridge_messages::weights::BridgeWeight; - type ActiveOutboundLanes = ActiveOutboundLanes; type OutboundPayload = XcmAsPlainPayload; diff --git a/bridges/modules/messages/README.md b/bridges/modules/messages/README.md index 80fd92eb0e5a..bf5cced43854 100644 --- a/bridges/modules/messages/README.md +++ b/bridges/modules/messages/README.md @@ -142,10 +142,9 @@ and will simply reject all transactions, related to inbound messages. ### What about other Constants in the Messages Module Configuration Trait? -Two settings that are used to check messages in the `send_message()` function. The -`pallet_bridge_messages::Config::ActiveOutboundLanes` is an array of all message lanes, that -may be used to send messages. All messages sent using other lanes are rejected. All messages that have -size above `pallet_bridge_messages::Config::MaximalOutboundPayloadSize` will also be rejected. +`pallet_bridge_messages::Config::MaximalOutboundPayloadSize` constant defines the maximal size +of outbound message that may be sent. If the message size is above this limit, the message is +rejected. To be able to reward the relayer for delivering messages, we store a map of message nonces range => identifier of the relayer that has delivered this range at the target chain runtime storage. If a diff --git a/bridges/modules/messages/src/benchmarking.rs b/bridges/modules/messages/src/benchmarking.rs index d38aaf32dc94..f73c5ba1442a 100644 --- a/bridges/modules/messages/src/benchmarking.rs +++ b/bridges/modules/messages/src/benchmarking.rs @@ -19,14 +19,14 @@ #![cfg(feature = "runtime-benchmarks")] use crate::{ - inbound_lane::InboundLaneStorage, outbound_lane, weights_ext::EXPECTED_DEFAULT_MESSAGE_LENGTH, - BridgedChainOf, Call, OutboundLanes, RuntimeInboundLaneStorage, + outbound_lane, weights_ext::EXPECTED_DEFAULT_MESSAGE_LENGTH, BridgedChainOf, Call, + InboundLanes, OutboundLanes, }; use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, target_chain::FromBridgedChainMessagesProof, ChainWithMessages, DeliveredMessages, - InboundLaneData, LaneId, MessageNonce, OutboundLaneData, UnrewardedRelayer, + InboundLaneData, LaneId, LaneState, MessageNonce, OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, }; use bp_runtime::{AccountIdOf, HashOf, UnverifiedStorageProofParams}; @@ -113,22 +113,32 @@ pub trait Config: crate::Config { } fn send_regular_message, I: 'static>() { - let mut outbound_lane = outbound_lane::(T::bench_lane_id()); + OutboundLanes::::insert( + T::bench_lane_id(), + OutboundLaneData { + state: LaneState::Opened, + latest_generated_nonce: 1, + ..Default::default() + }, + ); + + let mut outbound_lane = outbound_lane::(T::bench_lane_id()).unwrap(); outbound_lane.send_message(BoundedVec::try_from(vec![]).expect("We craft valid messages")); } fn receive_messages, I: 'static>(nonce: MessageNonce) { - let mut inbound_lane_storage = - RuntimeInboundLaneStorage::::from_lane_id(T::bench_lane_id()); - inbound_lane_storage.set_data(InboundLaneData { - relayers: vec![UnrewardedRelayer { - relayer: T::bridged_relayer_id(), - messages: DeliveredMessages::new(nonce), - }] - .into_iter() - .collect(), - last_confirmed_nonce: 0, - }); + InboundLanes::::insert( + T::bench_lane_id(), + InboundLaneData { + state: LaneState::Opened, + relayers: vec![UnrewardedRelayer { + relayer: T::bridged_relayer_id(), + messages: DeliveredMessages::new(nonce), + }] + .into(), + last_confirmed_nonce: 0, + }, + ); } struct ReceiveMessagesProofSetup, I: 'static> { @@ -173,8 +183,8 @@ impl, I: 'static> ReceiveMessagesProofSetup { fn check_last_nonce(&self) { assert_eq!( - crate::InboundLanes::::get(&T::bench_lane_id()).last_delivered_nonce(), - self.last_nonce(), + crate::InboundLanes::::get(&T::bench_lane_id()).map(|d| d.last_delivered_nonce()), + Some(self.last_nonce()), ); } } @@ -277,6 +287,7 @@ mod benchmarks { lane: T::bench_lane_id(), message_nonces: setup.nonces(), outbound_lane_data: Some(OutboundLaneData { + state: LaneState::Opened, oldest_unpruned_nonce: setup.last_nonce(), latest_received_nonce: ReceiveMessagesProofSetup::::LATEST_RECEIVED_NONCE, latest_generated_nonce: setup.last_nonce(), @@ -356,6 +367,7 @@ mod benchmarks { let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams { lane: T::bench_lane_id(), inbound_lane_data: InboundLaneData { + state: LaneState::Opened, relayers: vec![UnrewardedRelayer { relayer: relayer_id.clone(), messages: DeliveredMessages::new(1), @@ -374,7 +386,10 @@ mod benchmarks { relayers_state, ); - assert_eq!(OutboundLanes::::get(T::bench_lane_id()).latest_received_nonce, 1); + assert_eq!( + OutboundLanes::::get(T::bench_lane_id()).map(|s| s.latest_received_nonce), + Some(1) + ); assert!(T::is_relayer_rewarded(&relayer_id)); } @@ -404,6 +419,7 @@ mod benchmarks { let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams { lane: T::bench_lane_id(), inbound_lane_data: InboundLaneData { + state: LaneState::Opened, relayers: vec![UnrewardedRelayer { relayer: relayer_id.clone(), messages: delivered_messages, @@ -422,7 +438,10 @@ mod benchmarks { relayers_state, ); - assert_eq!(OutboundLanes::::get(T::bench_lane_id()).latest_received_nonce, 2); + assert_eq!( + OutboundLanes::::get(T::bench_lane_id()).map(|s| s.latest_received_nonce), + Some(2) + ); assert!(T::is_relayer_rewarded(&relayer_id)); } @@ -451,6 +470,7 @@ mod benchmarks { let proof = T::prepare_message_delivery_proof(MessageDeliveryProofParams { lane: T::bench_lane_id(), inbound_lane_data: InboundLaneData { + state: LaneState::Opened, relayers: vec![ UnrewardedRelayer { relayer: relayer1_id.clone(), @@ -475,7 +495,10 @@ mod benchmarks { relayers_state, ); - assert_eq!(OutboundLanes::::get(T::bench_lane_id()).latest_received_nonce, 2); + assert_eq!( + OutboundLanes::::get(T::bench_lane_id()).map(|s| s.latest_received_nonce), + Some(2) + ); assert!(T::is_relayer_rewarded(&relayer1_id)); assert!(T::is_relayer_rewarded(&relayer2_id)); } diff --git a/bridges/modules/messages/src/inbound_lane.rs b/bridges/modules/messages/src/inbound_lane.rs index 7ef4599a93c4..5061d63bd30e 100644 --- a/bridges/modules/messages/src/inbound_lane.rs +++ b/bridges/modules/messages/src/inbound_lane.rs @@ -41,7 +41,7 @@ pub trait InboundLaneStorage { /// Return maximal number of unconfirmed messages in inbound lane. fn max_unconfirmed_messages(&self) -> MessageNonce; /// Get lane data from the storage. - fn get_or_init_data(&mut self) -> InboundLaneData; + fn data(&self) -> InboundLaneData; /// Update lane data in the storage. fn set_data(&mut self, data: InboundLaneData); } @@ -130,7 +130,7 @@ impl InboundLane { &mut self, outbound_lane_data: OutboundLaneData, ) -> Option { - let mut data = self.storage.get_or_init_data(); + let mut data = self.storage.data(); let last_delivered_nonce = data.last_delivered_nonce(); if outbound_lane_data.latest_received_nonce > last_delivered_nonce { @@ -173,7 +173,7 @@ impl InboundLane { nonce: MessageNonce, message_data: DispatchMessageData, ) -> ReceptionResult { - let mut data = self.storage.get_or_init_data(); + let mut data = self.storage.data(); if Some(nonce) != data.last_delivered_nonce().checked_add(1) { return ReceptionResult::InvalidNonce } @@ -244,7 +244,7 @@ mod tests { #[test] fn receive_status_update_ignores_status_from_the_future() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); receive_regular_message(&mut lane, 1); assert_eq!( lane.receive_state_update(OutboundLaneData { @@ -254,14 +254,14 @@ mod tests { None, ); - assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 0); + assert_eq!(lane.storage.data().last_confirmed_nonce, 0); }); } #[test] fn receive_status_update_ignores_obsolete_status() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + 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); @@ -272,7 +272,7 @@ mod tests { }), Some(3), ); - assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 3); + assert_eq!(lane.storage.data().last_confirmed_nonce, 3); assert_eq!( lane.receive_state_update(OutboundLaneData { @@ -281,20 +281,20 @@ mod tests { }), None, ); - assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 3); + assert_eq!(lane.storage.data().last_confirmed_nonce, 3); }); } #[test] fn receive_status_update_works() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + 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); - assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 0); + assert_eq!(lane.storage.data().last_confirmed_nonce, 0); assert_eq!( - lane.storage.get_or_init_data().relayers, + lane.storage.data().relayers, vec![unrewarded_relayer(1, 3, TEST_RELAYER_A)] ); @@ -305,9 +305,9 @@ mod tests { }), Some(2), ); - assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 2); + assert_eq!(lane.storage.data().last_confirmed_nonce, 2); assert_eq!( - lane.storage.get_or_init_data().relayers, + lane.storage.data().relayers, vec![unrewarded_relayer(3, 3, TEST_RELAYER_A)] ); @@ -318,16 +318,16 @@ mod tests { }), Some(3), ); - assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 3); - assert_eq!(lane.storage.get_or_init_data().relayers, vec![]); + assert_eq!(lane.storage.data().last_confirmed_nonce, 3); + assert_eq!(lane.storage.data().relayers, vec![]); }); } #[test] fn receive_status_update_works_with_batches_from_relayers() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); - let mut seed_storage_data = lane.storage.get_or_init_data(); + 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; seed_storage_data.relayers.push_back(unrewarded_relayer(1, 1, TEST_RELAYER_A)); @@ -343,9 +343,9 @@ mod tests { }), Some(3), ); - assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 3); + assert_eq!(lane.storage.data().last_confirmed_nonce, 3); assert_eq!( - lane.storage.get_or_init_data().relayers, + lane.storage.data().relayers, vec![ unrewarded_relayer(4, 4, TEST_RELAYER_B), unrewarded_relayer(5, 5, TEST_RELAYER_C) @@ -357,7 +357,7 @@ mod tests { #[test] fn fails_to_receive_message_with_incorrect_nonce() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -366,14 +366,14 @@ mod tests { ), ReceptionResult::InvalidNonce ); - assert_eq!(lane.storage.get_or_init_data().last_delivered_nonce(), 0); + assert_eq!(lane.storage.data().last_delivered_nonce(), 0); }); } #[test] fn fails_to_receive_messages_above_unrewarded_relayer_entries_limit_per_lane() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + 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 +409,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); + 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 +445,7 @@ mod tests { #[test] fn correctly_receives_following_messages_from_two_relayers_alternately() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -471,7 +471,7 @@ mod tests { ReceptionResult::Dispatched(dispatch_result(0)) ); assert_eq!( - lane.storage.get_or_init_data().relayers, + lane.storage.data().relayers, vec![ unrewarded_relayer(1, 1, TEST_RELAYER_A), unrewarded_relayer(2, 2, TEST_RELAYER_B), @@ -484,7 +484,7 @@ mod tests { #[test] fn rejects_same_message_from_two_different_relayers() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); assert_eq!( lane.receive_message::( &TEST_RELAYER_A, @@ -507,16 +507,16 @@ mod tests { #[test] fn correct_message_is_processed_instantly() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); receive_regular_message(&mut lane, 1); - assert_eq!(lane.storage.get_or_init_data().last_delivered_nonce(), 1); + assert_eq!(lane.storage.data().last_delivered_nonce(), 1); }); } #[test] fn unspent_weight_is_returned_by_receive_message() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + 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 +533,7 @@ mod tests { #[test] fn first_message_is_confirmed_correctly() { run_test(|| { - let mut lane = inbound_lane::(TEST_LANE_ID); + let mut lane = inbound_lane::(TEST_LANE_ID).unwrap(); receive_regular_message(&mut lane, 1); receive_regular_message(&mut lane, 2); assert_eq!( diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index f664f9931308..b8a7fba6c1e8 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -61,7 +61,7 @@ use bp_messages::{ ProvedLaneMessages, ProvedMessages, }, ChainWithMessages, DeliveredMessages, InboundLaneData, InboundMessageDetails, LaneId, - MessageKey, MessageNonce, MessagePayload, MessagesOperatingMode, OutboundLaneData, + LaneState, MessageKey, MessageNonce, MessagePayload, MessagesOperatingMode, OutboundLaneData, OutboundMessageDetails, UnrewardedRelayersState, VerificationError, }; use bp_runtime::{ @@ -115,9 +115,6 @@ pub mod pallet { /// Bridged chain headers provider. type BridgedHeaderChain: HeaderChain; - /// Get all active outbound lanes that the message pallet is serving. - type ActiveOutboundLanes: Get<&'static [LaneId]>; - /// Payload type of outbound messages. This payload is dispatched on the bridged chain. type OutboundPayload: Parameter + Size; /// Payload type of inbound messages. This payload is dispatched on this chain. @@ -249,7 +246,7 @@ pub mod pallet { let mut messages_received_status = Vec::with_capacity(messages.len()); let mut dispatch_weight_left = dispatch_weight; for (lane_id, lane_data) in messages { - let mut lane = inbound_lane::(lane_id); + let mut lane = inbound_lane::(lane_id)?; // subtract extra storage proof bytes from the actual PoV size - there may be // less unrewarded relayers than the maximal configured value @@ -266,7 +263,7 @@ pub mod pallet { "Received lane {:?} state update: latest_confirmed_nonce={}. Unrewarded relayers: {:?}", lane_id, updated_latest_confirmed_nonce, - UnrewardedRelayersState::from(&lane.storage_mut().get_or_init_data()), + UnrewardedRelayersState::from(&lane.storage_mut().data()), ); } } @@ -377,7 +374,7 @@ pub mod pallet { ); // mark messages as delivered - let mut lane = outbound_lane::(lane_id); + let mut lane = outbound_lane::(lane_id)?; let last_delivered_nonce = lane_data.last_delivered_nonce(); let confirmed_messages = lane .confirm_delivery( @@ -468,10 +465,17 @@ pub mod pallet { pub enum Error { /// Pallet is not in Normal operating mode. NotOperatingNormally, - /// The outbound lane is inactive. - InactiveOutboundLane, /// The inbound message dispatcher is inactive. MessageDispatchInactive, + /// The outbound lane is unknown. + UnknownOutboundLane, + /// The outbound lane exists, but it is currently closed. + ClosedOutboundLane, + /// The inbound lane is unknown. + UnknownInboundLane, + /// The inbound lane exists, but it is currently closed. + ClosedInboundLane, + /// Message has been treated as invalid by chain verifier. /// Message has been treated as invalid by the pallet logic. MessageRejectedByPallet(VerificationError), /// Submitter has failed to pay fee for delivering and dispatching messages. @@ -514,10 +518,13 @@ pub mod pallet { pub type PalletOperatingMode, I: 'static = ()> = StorageValue<_, MessagesOperatingMode, ValueQuery>; + // TODO: https://github.com/paritytech/parity-bridges-common/pull/2213: let's limit number of + // possible opened lanes && use it to constraint maps below + /// Map of lane id => inbound lane data. #[pallet::storage] pub type InboundLanes, I: 'static = ()> = - StorageMap<_, Blake2_128Concat, LaneId, StoredInboundLaneData, ValueQuery>; + StorageMap<_, Blake2_128Concat, LaneId, StoredInboundLaneData, OptionQuery>; /// Map of lane id => outbound lane data. #[pallet::storage] @@ -525,11 +532,10 @@ pub mod pallet { Hasher = Blake2_128Concat, Key = LaneId, Value = OutboundLaneData, - QueryKind = ValueQuery, - OnEmpty = GetDefault, - MaxValues = MaybeOutboundLanesCount, + QueryKind = OptionQuery, >; + // TODO:(bridges-v2) - do we still need this? /// Map of lane id => is congested signal sent. It is managed by the /// `bridge_runtime_common::LocalXcmQueueManager`. /// @@ -561,6 +567,8 @@ pub mod pallet { pub operating_mode: MessagesOperatingMode, /// Initial pallet owner. pub owner: Option, + /// Opened lanes. + pub opened_lanes: Vec, /// Dummy marker. pub phantom: sp_std::marker::PhantomData, } @@ -572,6 +580,11 @@ pub mod pallet { if let Some(ref owner) = self.owner { PalletOwner::::put(owner); } + + for lane_id in &self.opened_lanes { + InboundLanes::::insert(lane_id, InboundLaneData::opened()); + OutboundLanes::::insert(lane_id, OutboundLaneData::opened()); + } } } @@ -605,15 +618,15 @@ pub mod pallet { } /// Return outbound lane data. - pub fn outbound_lane_data(lane: LaneId) -> OutboundLaneData { + pub fn outbound_lane_data(lane: LaneId) -> Option { OutboundLanes::::get(lane) } /// Return inbound lane data. pub fn inbound_lane_data( lane: LaneId, - ) -> InboundLaneData>> { - InboundLanes::::get(lane).0 + ) -> Option>>> { + InboundLanes::::get(lane).map(|lane| lane.0) } } @@ -676,7 +689,7 @@ pub mod pallet { impl, I: 'static> Get> for MaybeOutboundLanesCount { fn get() -> Option { - Some(T::ActiveOutboundLanes::get().len() as u32) + Some(OutboundLanes::::iter().count() as u32) } } } @@ -686,6 +699,7 @@ pub mod pallet { #[derive(Debug, PartialEq, Eq)] pub struct SendMessageArgs, I: 'static> { lane_id: LaneId, + lane: OutboundLane>, payload: StoredMessagePayload, } @@ -698,16 +712,17 @@ where type SendMessageArgs = SendMessageArgs; fn validate_message( - lane: LaneId, + lane_id: LaneId, message: &T::OutboundPayload, ) -> Result, Self::Error> { ensure_normal_operating_mode::()?; - // let's check if outbound lane is active - ensure!(T::ActiveOutboundLanes::get().contains(&lane), Error::::InactiveOutboundLane); + // check lane + let lane = outbound_lane::(lane_id)?; Ok(SendMessageArgs { - lane_id: lane, + lane_id, + lane, payload: StoredMessagePayload::::try_from(message.encode()).map_err(|_| { Error::::MessageRejectedByPallet(VerificationError::MessageTooLarge) })?, @@ -716,7 +731,7 @@ where fn send_message(args: SendMessageArgs) -> SendMessageArtifacts { // save message in outbound storage and emit event - let mut lane = outbound_lane::(args.lane_id); + let mut lane = args.lane; let message_len = args.payload.len(); let nonce = lane.send_message(args.payload); @@ -751,28 +766,35 @@ fn ensure_normal_operating_mode, I: 'static>() -> Result<(), Error< /// Creates new inbound lane object, backed by runtime storage. fn inbound_lane, I: 'static>( lane_id: LaneId, -) -> InboundLane> { - InboundLane::new(RuntimeInboundLaneStorage::from_lane_id(lane_id)) +) -> Result>, Error> { + Ok(InboundLane::new(RuntimeInboundLaneStorage::from_lane_id(lane_id)?)) } /// Creates new outbound lane object, backed by runtime storage. fn outbound_lane, I: 'static>( lane_id: LaneId, -) -> OutboundLane> { - OutboundLane::new(RuntimeOutboundLaneStorage { lane_id, _phantom: Default::default() }) +) -> Result>, Error> { + Ok(OutboundLane::new(RuntimeOutboundLaneStorage::from_lane_id(lane_id)?)) } /// Runtime inbound lane storage. struct RuntimeInboundLaneStorage, I: 'static = ()> { lane_id: LaneId, - cached_data: Option>>>, + cached_data: InboundLaneData>>, _phantom: PhantomData, } impl, I: 'static> RuntimeInboundLaneStorage { - /// Creates new runtime inbound lane storage. - fn from_lane_id(lane_id: LaneId) -> RuntimeInboundLaneStorage { - RuntimeInboundLaneStorage { lane_id, cached_data: None, _phantom: Default::default() } + /// Creates new runtime inbound lane storage for given **existing** lane. + fn from_lane_id(lane_id: LaneId) -> Result, Error> { + let cached_data = + InboundLanes::::get(lane_id).ok_or(Error::::UnknownInboundLane)?; + ensure!(cached_data.state == LaneState::Opened, Error::::ClosedInboundLane); + Ok(RuntimeInboundLaneStorage { + lane_id, + cached_data: cached_data.into(), + _phantom: Default::default(), + }) } } @@ -787,7 +809,7 @@ impl, I: 'static> RuntimeInboundLaneStorage { /// we may subtract extra bytes from this component. pub fn extra_proof_size_bytes(&mut self) -> u64 { let max_encoded_len = StoredInboundLaneData::::max_encoded_len(); - let relayers_count = self.get_or_init_data().relayers.len(); + let relayers_count = self.data().relayers.len(); let actual_encoded_len = InboundLaneData::>>::encoded_size_hint(relayers_count) .unwrap_or(usize::MAX); @@ -810,30 +832,34 @@ impl, I: 'static> InboundLaneStorage for RuntimeInboundLaneStorage< BridgedChainOf::::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX } - fn get_or_init_data(&mut self) -> InboundLaneData>> { - match self.cached_data { - Some(ref data) => data.clone(), - None => { - let data: InboundLaneData>> = - InboundLanes::::get(self.lane_id).into(); - self.cached_data = Some(data.clone()); - data - }, - } + fn data(&self) -> InboundLaneData>> { + self.cached_data.clone() } fn set_data(&mut self, data: InboundLaneData>>) { - self.cached_data = Some(data.clone()); + self.cached_data = data.clone(); InboundLanes::::insert(self.lane_id, StoredInboundLaneData::(data)) } } /// Runtime outbound lane storage. +#[derive(Debug, PartialEq, Eq)] struct RuntimeOutboundLaneStorage { lane_id: LaneId, + cached_data: OutboundLaneData, _phantom: PhantomData<(T, I)>, } +impl, I: 'static> RuntimeOutboundLaneStorage { + /// Creates new runtime outbound lane storage for given **existing** lane. + fn from_lane_id(lane_id: LaneId) -> Result> { + let cached_data = + OutboundLanes::::get(lane_id).ok_or(Error::::UnknownOutboundLane)?; + ensure!(cached_data.state == LaneState::Opened, Error::::ClosedOutboundLane); + Ok(Self { lane_id, cached_data, _phantom: PhantomData }) + } +} + impl, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorage { type StoredMessagePayload = StoredMessagePayload; @@ -842,10 +868,11 @@ impl, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorag } fn data(&self) -> OutboundLaneData { - OutboundLanes::::get(self.lane_id) + self.cached_data.clone() } fn set_data(&mut self, data: OutboundLaneData) { + self.cached_data = data.clone(); OutboundLanes::::insert(self.lane_id, data) } diff --git a/bridges/modules/messages/src/outbound_lane.rs b/bridges/modules/messages/src/outbound_lane.rs index 788a13e82b1b..b1cd453286ba 100644 --- a/bridges/modules/messages/src/outbound_lane.rs +++ b/bridges/modules/messages/src/outbound_lane.rs @@ -75,6 +75,7 @@ pub enum ReceptionConfirmationError { } /// Outbound messages lane. +#[derive(Debug, PartialEq, Eq)] pub struct OutboundLane { storage: S, } @@ -212,7 +213,7 @@ mod tests { relayers: &VecDeque>, ) -> Result, ReceptionConfirmationError> { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID); + 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)); @@ -228,7 +229,7 @@ mod tests { #[test] fn send_message_works() { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID); + 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()); @@ -239,7 +240,7 @@ mod tests { #[test] fn confirm_delivery_works() { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID); + 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); @@ -259,7 +260,7 @@ mod tests { #[test] fn confirm_partial_delivery_works() { run_test(|| { - let mut lane = outbound_lane::(TEST_LANE_ID); + 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); @@ -288,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); + 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)); @@ -368,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); + 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 9b5c15fb1929..038d159d79ad 100644 --- a/bridges/modules/messages/src/proofs.rs +++ b/bridges/modules/messages/src/proofs.rs @@ -220,7 +220,8 @@ mod tests { mock::*, }; - use bp_header_chain::StoredHeaderDataBuilder; + use bp_header_chain::{HeaderChainError, StoredHeaderDataBuilder}; + use bp_messages::LaneState; use bp_runtime::{HeaderId, StorageProofError}; use codec::Encode; use sp_runtime::traits::Header; @@ -440,6 +441,7 @@ mod tests { using_messages_proof( 10, Some(OutboundLaneData { + state: LaneState::Opened, oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, @@ -480,6 +482,7 @@ mod tests { using_messages_proof( 0, Some(OutboundLaneData { + state: LaneState::Opened, oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, @@ -494,6 +497,7 @@ mod tests { TEST_LANE_ID, ProvedLaneMessages { lane_state: Some(OutboundLaneData { + state: LaneState::Opened, oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, @@ -512,6 +516,7 @@ mod tests { using_messages_proof( 1, Some(OutboundLaneData { + state: LaneState::Opened, oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, @@ -526,6 +531,7 @@ mod tests { TEST_LANE_ID, ProvedLaneMessages { lane_state: Some(OutboundLaneData { + state: LaneState::Opened, oldest_unpruned_nonce: 1, latest_received_nonce: 1, latest_generated_nonce: 1, diff --git a/bridges/modules/messages/src/tests/mock.rs b/bridges/modules/messages/src/tests/mock.rs index ffdd536830b5..75caf3a35ab8 100644 --- a/bridges/modules/messages/src/tests/mock.rs +++ b/bridges/modules/messages/src/tests/mock.rs @@ -35,7 +35,7 @@ use bp_messages::{ DeliveryPayments, DispatchMessage, DispatchMessageData, FromBridgedChainMessagesProof, MessageDispatch, }, - ChainWithMessages, DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, + ChainWithMessages, DeliveredMessages, InboundLaneData, LaneId, LaneState, Message, MessageKey, MessageNonce, OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, }; use bp_runtime::{ @@ -186,7 +186,6 @@ impl pallet_bridge_grandpa::Config for TestRuntime { parameter_types! { pub const MaxMessagesToPruneAtOnce: u64 = 10; pub const TestBridgedChainId: bp_runtime::ChainId = *b"test"; - pub const ActiveOutboundLanes: &'static [LaneId] = &[TEST_LANE_ID, TEST_LANE_ID_2]; } /// weights of messages pallet calls we use in tests. @@ -200,8 +199,6 @@ impl Config for TestRuntime { type BridgedChain = BridgedChain; type BridgedHeaderChain = BridgedChainGrandpa; - type ActiveOutboundLanes = ActiveOutboundLanes; - type OutboundPayload = TestPayload; type InboundPayload = TestPayload; @@ -269,11 +266,11 @@ 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]); -/// Secondary lane that we're using in tests. -pub const TEST_LANE_ID_2: LaneId = LaneId([0, 0, 0, 2]); +/// Lane that is completely unknown to our runtime. +pub const UNKNOWN_LANE_ID: LaneId = LaneId([0, 0, 0, 2]); -/// Inactive outbound lane. -pub const TEST_LANE_ID_3: LaneId = LaneId([0, 0, 0, 3]); +/// Lane that is registered, but it is closed. +pub const CLOSED_LANE_ID: LaneId = LaneId([0, 0, 0, 3]); /// Regular message payload. pub const REGULAR_PAYLOAD: TestPayload = message_payload(0, 50); @@ -439,7 +436,7 @@ pub fn unrewarded_relayer( /// Returns unrewarded relayers state at given lane. pub fn inbound_unrewarded_relayers_state(lane: bp_messages::LaneId) -> UnrewardedRelayersState { - let inbound_lane_data = crate::InboundLanes::::get(lane).0; + let inbound_lane_data = crate::InboundLanes::::get(lane).unwrap().0; UnrewardedRelayersState::from(&inbound_lane_data) } @@ -454,7 +451,19 @@ 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(test) + new_test_ext().execute_with(|| { + crate::InboundLanes::::insert(TEST_LANE_ID, InboundLaneData::opened()); + crate::OutboundLanes::::insert(TEST_LANE_ID, OutboundLaneData::opened()); + crate::InboundLanes::::insert( + CLOSED_LANE_ID, + InboundLaneData { state: LaneState::Closed, ..Default::default() }, + ); + crate::OutboundLanes::::insert( + CLOSED_LANE_ID, + OutboundLaneData { state: LaneState::Closed, ..Default::default() }, + ); + test() + }) } /// Prepare valid storage proof for given messages and insert appropriate header to the @@ -471,7 +480,7 @@ pub fn prepare_messages_proof( let nonces_start = messages.first().unwrap().key.nonce; let nonces_end = messages.last().unwrap().key.nonce; let (storage_root, storage_proof) = prepare_messages_storage_proof::( - TEST_LANE_ID, + lane, nonces_start..=nonces_end, outbound_lane_data, UnverifiedStorageProofParams::default(), diff --git a/bridges/modules/messages/src/tests/pallet_tests.rs b/bridges/modules/messages/src/tests/pallet_tests.rs index cc43125d0c35..0eb4edbe249b 100644 --- a/bridges/modules/messages/src/tests/pallet_tests.rs +++ b/bridges/modules/messages/src/tests/pallet_tests.rs @@ -19,18 +19,17 @@ use crate::{ outbound_lane, outbound_lane::ReceptionConfirmationError, - tests::mock::{self, RuntimeEvent as TestEvent, *}, + tests::mock::{RuntimeEvent as TestEvent, *}, weights_ext::WeightInfoExt, - Call, Config, Error, Event, InboundLanes, MaybeOutboundLanesCount, OutboundLanes, - OutboundMessages, Pallet, PalletOperatingMode, PalletOwner, RuntimeInboundLaneStorage, - StoredInboundLaneData, + Call, Config, Error, Event, InboundLanes, OutboundLanes, OutboundMessages, Pallet, + PalletOperatingMode, PalletOwner, RuntimeInboundLaneStorage, StoredInboundLaneData, }; use bp_messages::{ source_chain::{FromBridgedChainMessagesDeliveryProof, MessagesBridge}, target_chain::FromBridgedChainMessagesProof, BridgeMessagesCall, ChainWithMessages, DeliveredMessages, InboundLaneData, - InboundMessageDetails, LaneId, MessageKey, MessageNonce, MessagesOperatingMode, + InboundMessageDetails, LaneId, LaneState, MessageKey, MessageNonce, MessagesOperatingMode, OutboundLaneData, OutboundMessageDetails, UnrewardedRelayer, UnrewardedRelayersState, VerificationError, }; @@ -44,7 +43,6 @@ use frame_support::{ weights::Weight, }; use frame_system::{EventRecord, Pallet as System, Phase}; -use sp_core::Get; use sp_runtime::DispatchError; fn get_ready_for_events() { @@ -55,7 +53,7 @@ fn get_ready_for_events() { fn send_regular_message(lane_id: LaneId) { get_ready_for_events(); - let outbound_lane = outbound_lane::(lane_id); + let outbound_lane = outbound_lane::(lane_id).unwrap(); let message_nonce = outbound_lane.data().latest_generated_nonce + 1; let prev_enqueued_messages = outbound_lane.data().queued_messages().saturating_len(); let valid_message = Pallet::::validate_message(lane_id, ®ULAR_PAYLOAD) @@ -83,6 +81,7 @@ fn receive_messages_delivery_proof() { prepare_messages_delivery_proof( TEST_LANE_ID, InboundLaneData { + state: LaneState::Opened, last_confirmed_nonce: 1, relayers: vec![UnrewardedRelayer { relayer: 0, @@ -143,6 +142,7 @@ fn pallet_rejects_transactions_if_halted() { let delivery_proof = prepare_messages_delivery_proof( TEST_LANE_ID, InboundLaneData { + state: LaneState::Opened, last_confirmed_nonce: 1, relayers: vec![unrewarded_relayer(1, 1, TEST_RELAYER_A)].into(), }, @@ -210,6 +210,7 @@ fn pallet_rejects_new_messages_in_rejecting_outbound_messages_operating_mode() { prepare_messages_delivery_proof( TEST_LANE_ID, InboundLaneData { + state: LaneState::Opened, last_confirmed_nonce: 1, relayers: vec![unrewarded_relayer(1, 1, TEST_RELAYER_A)].into(), }, @@ -271,7 +272,10 @@ fn receive_messages_proof_works() { REGULAR_PAYLOAD.declared_weight, )); - assert_eq!(InboundLanes::::get(TEST_LANE_ID).0.last_delivered_nonce(), 1); + assert_eq!( + InboundLanes::::get(TEST_LANE_ID).unwrap().0.last_delivered_nonce(), + 1 + ); assert!(TestDeliveryPayments::is_reward_paid(1)); }); @@ -284,6 +288,7 @@ fn receive_messages_proof_updates_confirmed_message_nonce() { InboundLanes::::insert( TEST_LANE_ID, InboundLaneData { + state: LaneState::Opened, last_confirmed_nonce: 8, relayers: vec![ unrewarded_relayer(9, 9, TEST_RELAYER_A), @@ -315,8 +320,9 @@ fn receive_messages_proof_updates_confirmed_message_nonce() { )); assert_eq!( - InboundLanes::::get(TEST_LANE_ID).0, + InboundLanes::::get(TEST_LANE_ID).unwrap().0, InboundLaneData { + state: LaneState::Opened, last_confirmed_nonce: 9, relayers: vec![ unrewarded_relayer(10, 10, TEST_RELAYER_B), @@ -354,7 +360,10 @@ fn receive_messages_proof_does_not_accept_message_if_dispatch_weight_is_not_enou ), Error::::InsufficientDispatchWeight ); - assert_eq!(InboundLanes::::get(TEST_LANE_ID).last_delivered_nonce(), 0); + assert_eq!( + InboundLanes::::get(TEST_LANE_ID).unwrap().last_delivered_nonce(), + 0 + ); }); } @@ -397,14 +406,34 @@ fn receive_messages_proof_rejects_proof_with_too_many_messages() { #[test] fn receive_messages_delivery_proof_works() { run_test(|| { - assert_eq!(OutboundLanes::::get(TEST_LANE_ID).latest_received_nonce, 0); - assert_eq!(OutboundLanes::::get(TEST_LANE_ID).oldest_unpruned_nonce, 1); + assert_eq!( + OutboundLanes::::get(TEST_LANE_ID) + .unwrap() + .latest_received_nonce, + 0, + ); + assert_eq!( + OutboundLanes::::get(TEST_LANE_ID) + .unwrap() + .oldest_unpruned_nonce, + 1, + ); send_regular_message(TEST_LANE_ID); receive_messages_delivery_proof(); - assert_eq!(OutboundLanes::::get(TEST_LANE_ID).latest_received_nonce, 1); - assert_eq!(OutboundLanes::::get(TEST_LANE_ID).oldest_unpruned_nonce, 2); + assert_eq!( + OutboundLanes::::get(TEST_LANE_ID) + .unwrap() + .latest_received_nonce, + 1, + ); + assert_eq!( + OutboundLanes::::get(TEST_LANE_ID) + .unwrap() + .oldest_unpruned_nonce, + 2, + ); }); } @@ -609,7 +638,10 @@ fn receive_messages_accepts_single_message_with_invalid_payload() { * improperly encoded) */ ),); - assert_eq!(InboundLanes::::get(TEST_LANE_ID).last_delivered_nonce(), 1,); + assert_eq!( + InboundLanes::::get(TEST_LANE_ID).unwrap().last_delivered_nonce(), + 1, + ); }); } @@ -630,7 +662,10 @@ fn receive_messages_accepts_batch_with_message_with_invalid_payload() { REGULAR_PAYLOAD.declared_weight + REGULAR_PAYLOAD.declared_weight, ),); - assert_eq!(InboundLanes::::get(TEST_LANE_ID).last_delivered_nonce(), 3,); + assert_eq!( + InboundLanes::::get(TEST_LANE_ID).unwrap().last_delivered_nonce(), + 3, + ); }); } @@ -653,7 +688,10 @@ fn actual_dispatch_weight_does_not_overflow() { ), Error::::InsufficientDispatchWeight ); - assert_eq!(InboundLanes::::get(TEST_LANE_ID).last_delivered_nonce(), 0); + assert_eq!( + InboundLanes::::get(TEST_LANE_ID).unwrap().last_delivered_nonce(), + 0 + ); }); } @@ -732,6 +770,7 @@ fn proof_size_refund_from_receive_messages_proof_works() { InboundLanes::::insert( TEST_LANE_ID, StoredInboundLaneData(InboundLaneData { + state: LaneState::Opened, relayers: vec![ UnrewardedRelayer { relayer: 42, @@ -760,6 +799,7 @@ fn proof_size_refund_from_receive_messages_proof_works() { InboundLanes::::insert( TEST_LANE_ID, StoredInboundLaneData(InboundLaneData { + state: LaneState::Opened, relayers: vec![ UnrewardedRelayer { relayer: 42, @@ -805,7 +845,11 @@ fn receive_messages_delivery_proof_rejects_proof_if_trying_to_confirm_more_messa // actually confirmed messages is `1`. let proof = prepare_messages_delivery_proof( TEST_LANE_ID, - InboundLaneData { last_confirmed_nonce: 1, relayers: Default::default() }, + InboundLaneData { + state: LaneState::Opened, + last_confirmed_nonce: 1, + relayers: Default::default(), + }, ); assert_noop!( Pallet::::receive_messages_delivery_proof( @@ -860,16 +904,6 @@ fn inbound_message_details_works() { }); } -#[test] -fn outbound_message_from_unconfigured_lane_is_rejected() { - run_test(|| { - assert_noop!( - Pallet::::validate_message(TEST_LANE_ID_3, ®ULAR_PAYLOAD,), - Error::::InactiveOutboundLane, - ); - }); -} - #[test] fn test_bridge_messages_call_is_correctly_defined() { run_test(|| { @@ -878,6 +912,7 @@ fn test_bridge_messages_call_is_correctly_defined() { let message_delivery_proof = prepare_messages_delivery_proof( TEST_LANE_ID, InboundLaneData { + state: LaneState::Opened, last_confirmed_nonce: 1, relayers: vec![UnrewardedRelayer { relayer: 0, @@ -948,10 +983,11 @@ fn inbound_storage_extra_proof_size_bytes_works() { fn storage(relayer_entries: usize) -> RuntimeInboundLaneStorage { RuntimeInboundLaneStorage { lane_id: Default::default(), - cached_data: Some(InboundLaneData { + cached_data: InboundLaneData { + state: LaneState::Opened, relayers: vec![relayer_entry(); relayer_entries].into(), last_confirmed_nonce: 0, - }), + }, _phantom: Default::default(), } } @@ -977,9 +1013,100 @@ fn inbound_storage_extra_proof_size_bytes_works() { } #[test] -fn maybe_outbound_lanes_count_returns_correct_value() { - assert_eq!( - MaybeOutboundLanesCount::::get(), - Some(mock::ActiveOutboundLanes::get().len() as u32) - ); +fn send_messages_fails_if_outbound_lane_is_not_opened() { + run_test(|| { + assert_noop!( + Pallet::::validate_message(UNKNOWN_LANE_ID, ®ULAR_PAYLOAD), + Error::::UnknownOutboundLane, + ); + + assert_noop!( + Pallet::::validate_message(CLOSED_LANE_ID, ®ULAR_PAYLOAD), + Error::::ClosedOutboundLane, + ); + }); +} + +#[test] +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; + let proof = prepare_messages_proof(vec![message.clone()], None); + + assert_noop!( + Pallet::::receive_messages_proof( + RuntimeOrigin::signed(1), + TEST_RELAYER_A, + proof, + 1, + REGULAR_PAYLOAD.declared_weight, + ), + Error::::UnknownInboundLane, + ); + + message.key.lane_id = CLOSED_LANE_ID; + let proof = prepare_messages_proof(vec![message], None); + + assert_noop!( + Pallet::::receive_messages_proof( + RuntimeOrigin::signed(1), + TEST_RELAYER_A, + proof, + 1, + REGULAR_PAYLOAD.declared_weight, + ), + Error::::ClosedInboundLane, + ); + }); +} + +#[test] +fn receive_messages_delivery_proof_fails_if_outbound_lane_is_unknown() { + run_test(|| { + let make_proof = |lane: LaneId| { + prepare_messages_delivery_proof( + lane, + InboundLaneData { + state: LaneState::Opened, + last_confirmed_nonce: 1, + relayers: vec![UnrewardedRelayer { + relayer: 0, + messages: DeliveredMessages::new(1), + }] + .into(), + }, + ) + }; + + let proof = make_proof(UNKNOWN_LANE_ID); + assert_noop!( + Pallet::::receive_messages_delivery_proof( + RuntimeOrigin::signed(1), + proof, + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + messages_in_oldest_entry: 1, + total_messages: 1, + last_delivered_nonce: 1, + }, + ), + Error::::UnknownOutboundLane, + ); + + let proof = make_proof(CLOSED_LANE_ID); + assert_noop!( + Pallet::::receive_messages_delivery_proof( + RuntimeOrigin::signed(1), + proof, + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + messages_in_oldest_entry: 1, + total_messages: 1, + last_delivered_nonce: 1, + }, + ), + Error::::ClosedOutboundLane, + ); + }); } diff --git a/bridges/modules/xcm-bridge-hub/src/exporter.rs b/bridges/modules/xcm-bridge-hub/src/exporter.rs index 94ec8b5f106f..2ed8c46e7890 100644 --- a/bridges/modules/xcm-bridge-hub/src/exporter.rs +++ b/bridges/modules/xcm-bridge-hub/src/exporter.rs @@ -77,7 +77,7 @@ where let bridge_message = MessagesPallet::::validate_message(sender_and_lane.lane, &blob) .map_err(|e| { - log::debug!( + log::error!( target: LOG_TARGET, "XCM message {:?} cannot be exported because of bridge error {:?} on bridge {:?}", id, @@ -131,7 +131,8 @@ impl HaulBlob for DummyHaulBlob { mod tests { use super::*; use crate::mock::*; - use frame_support::assert_ok; + use bp_messages::{LaneState, OutboundLaneData}; + use frame_support::{__private::sp_tracing, assert_ok}; use xcm_executor::traits::export_xcm; fn universal_source() -> InteriorLocation { @@ -145,6 +146,11 @@ mod tests { #[test] fn export_works() { run_test(|| { + sp_tracing::try_init_simple(); + pallet_bridge_messages::OutboundLanes::::insert( + TEST_LANE_ID, + OutboundLaneData { state: LaneState::Opened, ..Default::default() }, + ); assert_ok!(export_xcm::( BridgedRelayNetwork::get(), 0, @@ -185,8 +191,14 @@ mod tests { #[test] fn exporter_computes_correct_lane_id() { run_test(|| { + sp_tracing::try_init_simple(); let expected_lane_id = TEST_LANE_ID; + pallet_bridge_messages::OutboundLanes::::insert( + expected_lane_id, + OutboundLaneData { state: LaneState::Opened, ..Default::default() }, + ); + assert_eq!( XcmOverBridge::validate( BridgedRelayNetwork::get(), diff --git a/bridges/modules/xcm-bridge-hub/src/mock.rs b/bridges/modules/xcm-bridge-hub/src/mock.rs index df72e7a3c4fc..b997b6d5a9db 100644 --- a/bridges/modules/xcm-bridge-hub/src/mock.rs +++ b/bridges/modules/xcm-bridge-hub/src/mock.rs @@ -79,7 +79,6 @@ impl pallet_bridge_messages::Config for TestRuntime { type RuntimeEvent = RuntimeEvent; type WeightInfo = TestMessagesWeights; - type ActiveOutboundLanes = ActiveOutboundLanes; type OutboundPayload = Vec; type InboundPayload = Vec; type DeliveryPayments = (); diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index 9984f8ac3222..6a5a16866fc4 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/bridges/primitives/messages/src/lib.rs @@ -199,6 +199,21 @@ impl TypeId for LaneId { const TYPE_ID: [u8; 4] = *b"blan"; } +/// Lane state. +#[derive(Clone, Copy, Decode, Encode, Eq, PartialEq, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub enum LaneState { + /// Lane is closed and all attempts to send/receive messages to/from this lane + /// will fail. + /// + /// Keep in mind that the lane has two ends and the state of the same lane at + /// its ends may be different. Those who are controlling/serving the lane + /// and/or sending messages over the lane, have to coordinate their actions on + /// both ends to make sure that lane is operating smoothly on both ends. + Closed, + /// Lane is opened and messages may be sent/received over it. + Opened, +} + /// Message nonce. Valid messages will never have 0 nonce. pub type MessageNonce = u64; @@ -229,6 +244,11 @@ pub struct Message { /// Inbound lane data. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, TypeInfo)] pub struct InboundLaneData { + /// Inbound lane state. + /// + /// If state is `Closed`, then all attempts to deliver messages to this end will fail. + pub state: LaneState, + /// Identifiers of relayers and messages that they have delivered to this lane (ordered by /// message nonce). /// @@ -261,11 +281,20 @@ pub struct InboundLaneData { impl Default for InboundLaneData { fn default() -> Self { - InboundLaneData { relayers: VecDeque::new(), last_confirmed_nonce: 0 } + InboundLaneData { + state: LaneState::Closed, + relayers: VecDeque::new(), + last_confirmed_nonce: 0, + } } } impl InboundLaneData { + /// Returns default inbound lane data with opened state. + pub fn opened() -> Self { + InboundLaneData { state: LaneState::Opened, ..Default::default() } + } + /// Returns approximate size of the struct, given a number of entries in the `relayers` set and /// size of each entry. /// @@ -464,6 +493,10 @@ impl From<&InboundLaneData> for UnrewardedRelayersState { /// Outbound lane data. #[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq, Eq, TypeInfo, MaxEncodedLen)] pub struct OutboundLaneData { + /// Lane state. + /// + /// If state is `Closed`, then all attempts to send messages messages at this end will fail. + pub state: LaneState, /// Nonce of the oldest message that we haven't yet pruned. May point to not-yet-generated /// message if all sent messages are already pruned. pub oldest_unpruned_nonce: MessageNonce, @@ -473,9 +506,17 @@ pub struct OutboundLaneData { pub latest_generated_nonce: MessageNonce, } +impl OutboundLaneData { + /// Returns default outbound lane data with opened state. + pub fn opened() -> Self { + OutboundLaneData { state: LaneState::Opened, ..Default::default() } + } +} + impl Default for OutboundLaneData { fn default() -> Self { OutboundLaneData { + state: LaneState::Closed, // it is 1 because we're pruning everything in [oldest_unpruned_nonce; // latest_received_nonce] oldest_unpruned_nonce: 1, @@ -569,9 +610,16 @@ pub enum VerificationError { mod tests { use super::*; + #[test] + fn lane_is_closed_by_default() { + assert_eq!(InboundLaneData::<()>::default().state, LaneState::Closed); + assert_eq!(OutboundLaneData::default().state, LaneState::Closed); + } + #[test] fn total_unrewarded_messages_does_not_overflow() { let lane_data = InboundLaneData { + state: LaneState::Opened, relayers: vec![ UnrewardedRelayer { relayer: 1, messages: DeliveredMessages::new(0) }, UnrewardedRelayer { @@ -599,6 +647,7 @@ mod tests { for (relayer_entries, messages_count) in test_cases { let expected_size = InboundLaneData::::encoded_size_hint(relayer_entries as _); let actual_size = InboundLaneData { + state: LaneState::Opened, relayers: (1u8..=relayer_entries) .map(|i| UnrewardedRelayer { relayer: i, diff --git a/bridges/relays/lib-substrate-relay/src/messages/mod.rs b/bridges/relays/lib-substrate-relay/src/messages/mod.rs index e52b70206669..19036a8804c9 100644 --- a/bridges/relays/lib-substrate-relay/src/messages/mod.rs +++ b/bridges/relays/lib-substrate-relay/src/messages/mod.rs @@ -838,7 +838,6 @@ mod tests { type ThisChain = ThisUnderlyingChain; type BridgedChain = BridgedUnderlyingChain; type BridgedHeaderChain = BridgedHeaderChain; - type ActiveOutboundLanes = (); type OutboundPayload = Vec; type InboundPayload = Vec; type DeliveryPayments = ();