From 3b4a761f8571bec59e426521472af1d402d2c50c Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 22 Sep 2023 12:33:45 +0300 Subject: [PATCH] move messages_call_ext.rs from bridge-runtime-common to pallet-bridge-messages (#2580) --- .../extensions/check_obsolete_extension.rs | 6 +- .../extensions/refund_relayer_extension.rs | 21 ++- bridges/bin/runtime-common/src/lib.rs | 1 - .../messages/src/call_ext.rs} | 137 +++++++----------- bridges/modules/messages/src/lib.rs | 2 + bridges/modules/messages/src/tests/mock.rs | 8 + 6 files changed, 75 insertions(+), 100 deletions(-) rename bridges/{bin/runtime-common/src/messages_call_ext.rs => modules/messages/src/call_ext.rs} (85%) diff --git a/bridges/bin/runtime-common/src/extensions/check_obsolete_extension.rs b/bridges/bin/runtime-common/src/extensions/check_obsolete_extension.rs index 2c152aef68226..3fdb705081263 100644 --- a/bridges/bin/runtime-common/src/extensions/check_obsolete_extension.rs +++ b/bridges/bin/runtime-common/src/extensions/check_obsolete_extension.rs @@ -18,15 +18,13 @@ //! obsolete (duplicated) data or do not pass some additional pallet-specific //! checks. -use crate::{ - extensions::refund_relayer_extension::RefundableParachainId, - messages_call_ext::MessagesCallSubType, -}; +use crate::extensions::refund_relayer_extension::RefundableParachainId; use bp_relayers::ExplicitOrAccountParams; use bp_runtime::Parachain; use pallet_bridge_grandpa::{ BridgedBlockNumber, CallSubType as GrandpaCallSubType, SubmitFinalityProofHelper, }; +use pallet_bridge_messages::CallSubType as MessagesCallSubType; use pallet_bridge_parachains::{ CallSubType as ParachainsCallSubtype, SubmitParachainHeadsHelper, SubmitParachainHeadsInfo, }; 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 622007c6bd46f..c0ee8a55fe3e2 100644 --- a/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs +++ b/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs @@ -19,9 +19,6 @@ //! 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 bp_messages::{ChainWithMessages, LaneId, MessageNonce}; use bp_relayers::{ExplicitOrAccountParams, RewardsAccountOwner, RewardsAccountParams}; use bp_runtime::{Chain, Parachain, RangeInclusiveExt, StaticStrProvider}; @@ -35,7 +32,10 @@ use frame_support::{ use pallet_bridge_grandpa::{ CallSubType as GrandpaCallSubType, SubmitFinalityProofHelper, SubmitFinalityProofInfo, }; -use pallet_bridge_messages::Config as MessagesConfig; +use pallet_bridge_messages::{ + CallHelper as MessagesCallHelper, CallInfo as MessagesCallInfo, + CallSubType as MessagesCallSubType, Config as MessagesConfig, +}; use pallet_bridge_parachains::{ BoundedBridgeGrandpaConfig, CallSubType as ParachainsCallSubType, Config as ParachainsConfig, RelayBlockNumber, SubmitParachainHeadsHelper, SubmitParachainHeadsInfo, @@ -935,13 +935,7 @@ where #[cfg(test)] pub(crate) mod tests { use super::*; - use crate::{ - messages_call_ext::{ - BaseMessagesProofInfo, ReceiveMessagesDeliveryProofInfo, ReceiveMessagesProofInfo, - UnrewardedRelayerOccupation, - }, - mock::*, - }; + use crate::mock::*; use bp_header_chain::StoredHeaderDataBuilder; use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, @@ -959,7 +953,10 @@ pub(crate) mod tests { weights::Weight, }; use pallet_bridge_grandpa::{Call as GrandpaCall, Pallet as GrandpaPallet, StoredAuthoritySet}; - use pallet_bridge_messages::{Call as MessagesCall, Pallet as MessagesPallet}; + use pallet_bridge_messages::{ + BaseMessagesProofInfo, Call as MessagesCall, Pallet as MessagesPallet, + ReceiveMessagesDeliveryProofInfo, ReceiveMessagesProofInfo, UnrewardedRelayerOccupation, + }; use pallet_bridge_parachains::{ Call as ParachainsCall, Pallet as ParachainsPallet, RelayBlockHash, }; diff --git a/bridges/bin/runtime-common/src/lib.rs b/bridges/bin/runtime-common/src/lib.rs index 3e3394688f18d..de9ee1b0bbbca 100644 --- a/bridges/bin/runtime-common/src/lib.rs +++ b/bridges/bin/runtime-common/src/lib.rs @@ -23,7 +23,6 @@ pub mod extensions; pub mod messages_api; pub mod messages_benchmarking; -pub mod messages_call_ext; pub mod parachains_benchmarking; mod mock; diff --git a/bridges/bin/runtime-common/src/messages_call_ext.rs b/bridges/modules/messages/src/call_ext.rs similarity index 85% rename from bridges/bin/runtime-common/src/messages_call_ext.rs rename to bridges/modules/messages/src/call_ext.rs index 3e2c679b431e4..10b91021b6a48 100644 --- a/bridges/bin/runtime-common/src/messages_call_ext.rs +++ b/bridges/modules/messages/src/call_ext.rs @@ -16,12 +16,13 @@ //! Helpers for easier manipulation of call processing with signed extensions. +use crate::{BridgedChainOf, Config, InboundLanes, OutboundLanes, Pallet, LOG_TARGET}; + use bp_messages::{ target_chain::MessageDispatch, ChainWithMessages, InboundLaneData, LaneId, MessageNonce, }; use bp_runtime::{AccountIdOf, OwnedBridgeModule}; use frame_support::{dispatch::CallableCallFor, traits::IsSubType}; -use pallet_bridge_messages::{BridgedChainOf, Config, Pallet}; use sp_runtime::{transaction_validity::TransactionValidity, RuntimeDebug}; use sp_std::ops::RangeInclusive; @@ -145,11 +146,10 @@ impl, I: 'static> CallHelper { pub fn was_successful(info: &CallInfo) -> bool { match info { CallInfo::ReceiveMessagesProof(info) => { - let inbound_lane_data = - match pallet_bridge_messages::InboundLanes::::get(info.base.lane_id) { - Some(inbound_lane_data) => inbound_lane_data, - None => return false, - }; + let inbound_lane_data = match 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); @@ -164,11 +164,10 @@ impl, I: 'static> CallHelper { inbound_lane_data.last_delivered_nonce() == *info.base.bundled_range.end() }, CallInfo::ReceiveMessagesDeliveryProof(info) => { - let outbound_lane_data = - match pallet_bridge_messages::OutboundLanes::::get(info.0.lane_id) { - Some(outbound_lane_data) => outbound_lane_data, - None => return false, - }; + let outbound_lane_data = match 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() }, } @@ -176,7 +175,7 @@ impl, I: 'static> CallHelper { } /// Trait representing a call that is a sub type of `pallet_bridge_messages::Call`. -pub trait MessagesCallSubType, I: 'static>: +pub trait CallSubType, I: 'static>: IsSubType, T>> { /// Create a new instance of `ReceiveMessagesProofInfo` from a `ReceiveMessagesProof` call. @@ -217,15 +216,13 @@ impl< Call: IsSubType, T>>, T: frame_system::Config + Config, I: 'static, - > MessagesCallSubType for T::RuntimeCall + > CallSubType for T::RuntimeCall { fn receive_messages_proof_info(&self) -> Option { - if let Some(pallet_bridge_messages::Call::::receive_messages_proof { - ref proof, - .. - }) = self.is_sub_type() + if let Some(crate::Call::::receive_messages_proof { ref proof, .. }) = + self.is_sub_type() { - let inbound_lane_data = pallet_bridge_messages::InboundLanes::::get(proof.lane)?; + let inbound_lane_data = InboundLanes::::get(proof.lane)?; return Some(ReceiveMessagesProofInfo { base: BaseMessagesProofInfo { @@ -243,14 +240,13 @@ impl< } fn receive_messages_delivery_proof_info(&self) -> Option { - if let Some(pallet_bridge_messages::Call::::receive_messages_delivery_proof { + if let Some(crate::Call::::receive_messages_delivery_proof { ref proof, ref relayers_state, .. }) = self.is_sub_type() { - let outbound_lane_data = - pallet_bridge_messages::OutboundLanes::::get(proof.lane)?; + let outbound_lane_data = OutboundLanes::::get(proof.lane)?; return Some(ReceiveMessagesDeliveryProofInfo(BaseMessagesProofInfo { lane_id: proof.lane, @@ -294,7 +290,7 @@ impl< match self.call_info() { Some(proof_info) if is_pallet_halted => { log::trace!( - target: pallet_bridge_messages::LOG_TARGET, + target: LOG_TARGET, "Rejecting messages transaction on halted pallet: {:?}", proof_info ); @@ -306,7 +302,7 @@ impl< .is_obsolete(T::MessageDispatch::is_active(proof_info.base.lane_id)) => { log::trace!( - target: pallet_bridge_messages::LOG_TARGET, + target: LOG_TARGET, "Rejecting obsolete messages delivery transaction: {:?}", proof_info ); @@ -317,7 +313,7 @@ impl< if proof_info.is_obsolete() => { log::trace!( - target: pallet_bridge_messages::LOG_TARGET, + target: LOG_TARGET, "Rejecting obsolete messages confirmation transaction: {:?}", proof_info, ); @@ -351,10 +347,7 @@ fn unrewarded_relayers_occupation, I: 'static>( #[cfg(test)] mod tests { use super::*; - use crate::{ - messages_call_ext::MessagesCallSubType, - mock::{BridgedUnderlyingChain, DummyMessageDispatch, TestRuntime, ThisChainRuntimeCall}, - }; + use crate::tests::mock::*; use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, target_chain::FromBridgedChainMessagesProof, DeliveredMessages, InboundLaneData, LaneState, @@ -367,38 +360,30 @@ mod tests { } fn fill_unrewarded_relayers() { - let mut inbound_lane_state = - pallet_bridge_messages::InboundLanes::::get(test_lane_id()).unwrap(); - for n in 0..BridgedUnderlyingChain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX { + let mut inbound_lane_state = InboundLanes::::get(test_lane_id()).unwrap(); + for n in 0..BridgedChain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX { inbound_lane_state.relayers.push_back(UnrewardedRelayer { relayer: Default::default(), messages: DeliveredMessages { begin: n + 1, end: n + 1 }, }); } - pallet_bridge_messages::InboundLanes::::insert( - test_lane_id(), - inbound_lane_state, - ); + InboundLanes::::insert(test_lane_id(), inbound_lane_state); } fn fill_unrewarded_messages() { - let mut inbound_lane_state = - pallet_bridge_messages::InboundLanes::::get(test_lane_id()).unwrap(); + let mut inbound_lane_state = InboundLanes::::get(test_lane_id()).unwrap(); inbound_lane_state.relayers.push_back(UnrewardedRelayer { relayer: Default::default(), messages: DeliveredMessages { begin: 1, - end: BridgedUnderlyingChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX, + end: BridgedChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX, }, }); - pallet_bridge_messages::InboundLanes::::insert( - test_lane_id(), - inbound_lane_state, - ); + InboundLanes::::insert(test_lane_id(), inbound_lane_state); } fn deliver_message_10() { - pallet_bridge_messages::InboundLanes::::insert( + InboundLanes::::insert( test_lane_id(), bp_messages::InboundLaneData { state: LaneState::Opened, @@ -412,35 +397,26 @@ mod tests { nonces_start: bp_messages::MessageNonce, nonces_end: bp_messages::MessageNonce, ) -> bool { - ThisChainRuntimeCall::BridgeMessages( - pallet_bridge_messages::Call::::receive_messages_proof { - relayer_id_at_bridged_chain: 42, - messages_count: nonces_end.checked_sub(nonces_start).map(|x| x + 1).unwrap_or(0) - as u32, - dispatch_weight: frame_support::weights::Weight::zero(), - proof: Box::new(FromBridgedChainMessagesProof { - bridged_header_hash: Default::default(), - storage: Default::default(), - lane: test_lane_id(), - nonces_start, - nonces_end, - }), - }, - ) + RuntimeCall::Messages(crate::Call::::receive_messages_proof { + relayer_id_at_bridged_chain: 42, + messages_count: nonces_end.checked_sub(nonces_start).map(|x| x + 1).unwrap_or(0) as u32, + dispatch_weight: frame_support::weights::Weight::zero(), + proof: Box::new(FromBridgedChainMessagesProof { + bridged_header_hash: Default::default(), + storage: Default::default(), + lane: test_lane_id(), + nonces_start, + nonces_end, + }), + }) .check_obsolete_call() .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(), - ); + InboundLanes::::insert(test_lane_id(), InboundLaneData::opened()); + OutboundLanes::::insert(test_lane_id(), OutboundLaneData::opened()); test() }) } @@ -492,7 +468,7 @@ mod tests { // => tx is accepted, but we have inactive dispatcher, so... deliver_message_10(); - DummyMessageDispatch::deactivate(test_lane_id()); + TestMessageDispatch::deactivate(test_lane_id()); assert!(!validate_message_delivery(11, 15)); }); } @@ -522,8 +498,8 @@ mod tests { run_test(|| { fill_unrewarded_messages(); assert!(validate_message_delivery( - BridgedUnderlyingChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX, - BridgedUnderlyingChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX - 1 + BridgedChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX, + BridgedChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX - 1 )); }); } @@ -539,7 +515,7 @@ mod tests { } fn confirm_message_10() { - pallet_bridge_messages::OutboundLanes::::insert( + OutboundLanes::::insert( test_lane_id(), bp_messages::OutboundLaneData { state: LaneState::Opened, @@ -551,19 +527,14 @@ mod tests { } fn validate_message_confirmation(last_delivered_nonce: bp_messages::MessageNonce) -> bool { - ThisChainRuntimeCall::BridgeMessages( - pallet_bridge_messages::Call::::receive_messages_delivery_proof { - proof: FromBridgedChainMessagesDeliveryProof { - bridged_header_hash: Default::default(), - storage_proof: Default::default(), - lane: test_lane_id(), - }, - relayers_state: UnrewardedRelayersState { - last_delivered_nonce, - ..Default::default() - }, + RuntimeCall::Messages(crate::Call::::receive_messages_delivery_proof { + proof: FromBridgedChainMessagesDeliveryProof { + bridged_header_hash: Default::default(), + storage_proof: Default::default(), + lane: test_lane_id(), }, - ) + relayers_state: UnrewardedRelayersState { last_delivered_nonce, ..Default::default() }, + }) .check_obsolete_call() .is_ok() } @@ -623,7 +594,7 @@ mod tests { free_message_slots: if is_empty { 0 } else { - BridgedUnderlyingChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX + BridgedChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX }, }, }, diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index e25a34e13fb78..715541b7ffd96 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -72,6 +72,7 @@ use codec::{Decode, Encode}; use frame_support::{dispatch::PostDispatchInfo, ensure, fail, traits::Get, DefaultNoBound}; use sp_std::{marker::PhantomData, prelude::*}; +mod call_ext; mod inbound_lane; mod lanes_manager; mod outbound_lane; @@ -84,6 +85,7 @@ pub mod weights; #[cfg(feature = "runtime-benchmarks")] pub mod benchmarking; +pub use call_ext::*; pub use pallet::*; #[cfg(feature = "test-helpers")] pub use tests::*; diff --git a/bridges/modules/messages/src/tests/mock.rs b/bridges/modules/messages/src/tests/mock.rs index 8ed47859ee00b..cc8f095f0a8e8 100644 --- a/bridges/modules/messages/src/tests/mock.rs +++ b/bridges/modules/messages/src/tests/mock.rs @@ -342,6 +342,14 @@ impl DeliveryConfirmationPayments for TestDeliveryConfirmationPayment pub struct TestMessageDispatch; impl TestMessageDispatch { + pub fn deactivate(lane: LaneId) { + // "enqueue" enough (to deactivate dispatcher) messages at dispatcher + let latest_received_nonce = BridgedChain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX + 1; + for _ in 1..=latest_received_nonce { + Self::emulate_enqueued_message(lane); + } + } + pub fn emulate_enqueued_message(lane: LaneId) { let key = (b"dispatched", lane).encode(); let dispatched = frame_support::storage::unhashed::get_or_default::(&key[..]);