diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index 206e84121d5..82d7c8366d5 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -55,6 +55,7 @@ use sp_std::prelude::*; #[cfg(feature = "std")] use sp_version::NativeVersion; use sp_version::RuntimeVersion; +use xcm_builder::NetworkExportTable; // to be able to use Millau runtime in `bridge-runtime-common` tests pub use bridge_runtime_common; @@ -548,18 +549,21 @@ impl pallet_utility::Config for Runtime { // this config is totally incorrect - the pallet is not actually used at this runtime. We need // it only to be able to run benchmarks and make required traits (and default weights for tests). +parameter_types! { + pub BridgeTable: Vec<(xcm::prelude::NetworkId, xcm::prelude::MultiLocation, Option)> + = vec![(xcm_config::RialtoNetwork::get(), xcm_config::TokenLocation::get(), Some((xcm_config::TokenAssetId::get(), 1_000_000_000_u128).into()))]; +} impl pallet_xcm_bridge_hub_router::Config for Runtime { type WeightInfo = (); type UniversalLocation = xcm_config::UniversalLocation; - type SiblingBridgeHubLocation = xcm_config::TokenLocation; type BridgedNetworkId = xcm_config::RialtoNetwork; + type Bridges = NetworkExportTable; type BridgeHubOrigin = frame_system::EnsureRoot; type ToBridgeHubSender = xcm_config::XcmRouter; type WithBridgeHubChannel = xcm_config::EmulatedSiblingXcmpChannel; - type BaseFee = ConstU128<1_000_000_000>; type ByteFee = ConstU128<1_000>; type FeeAsset = xcm_config::TokenAssetId; } diff --git a/bin/millau/runtime/src/rialto_messages.rs b/bin/millau/runtime/src/rialto_messages.rs index e5808024479..47a765c59f8 100644 --- a/bin/millau/runtime/src/rialto_messages.rs +++ b/bin/millau/runtime/src/rialto_messages.rs @@ -133,7 +133,7 @@ impl XcmBlobHauler for ToRialtoXcmBlobHauler { type MessagesInstance = WithRialtoMessagesInstance; type SenderAndLane = RialtoSenderAndLane; - type ToSendingChainSender = crate::xcm_config::XcmRouter; + type ToSourceChainSender = crate::xcm_config::XcmRouter; type CongestedMessage = DummyXcmMessage; type UncongestedMessage = DummyXcmMessage; diff --git a/bin/millau/runtime/src/rialto_parachain_messages.rs b/bin/millau/runtime/src/rialto_parachain_messages.rs index 03518f1f252..fb1d79d006b 100644 --- a/bin/millau/runtime/src/rialto_parachain_messages.rs +++ b/bin/millau/runtime/src/rialto_parachain_messages.rs @@ -133,7 +133,7 @@ impl XcmBlobHauler for ToRialtoParachainXcmBlobHauler { type MessagesInstance = WithRialtoParachainMessagesInstance; type SenderAndLane = RialtoParachainSenderAndLane; - type ToSendingChainSender = crate::xcm_config::XcmRouter; + type ToSourceChainSender = crate::xcm_config::XcmRouter; type CongestedMessage = DummyXcmMessage; type UncongestedMessage = DummyXcmMessage; diff --git a/bin/millau/runtime/src/xcm_config.rs b/bin/millau/runtime/src/xcm_config.rs index cf1f8f6c9a5..45236360ddc 100644 --- a/bin/millau/runtime/src/xcm_config.rs +++ b/bin/millau/runtime/src/xcm_config.rs @@ -263,7 +263,7 @@ impl EmulatedSiblingXcmpChannel { } } -impl bp_xcm_bridge_hub_router::LocalXcmChannel for EmulatedSiblingXcmpChannel { +impl bp_xcm_bridge_hub_router::XcmChannelStatusProvider for EmulatedSiblingXcmpChannel { fn is_congested() -> bool { frame_support::storage::unhashed::get_or_default(b"EmulatedSiblingXcmpChannel.Congested") } diff --git a/bin/rialto-parachain/runtime/src/millau_messages.rs b/bin/rialto-parachain/runtime/src/millau_messages.rs index cc54188ff3c..fb6659b7f0d 100644 --- a/bin/rialto-parachain/runtime/src/millau_messages.rs +++ b/bin/rialto-parachain/runtime/src/millau_messages.rs @@ -133,7 +133,7 @@ impl XcmBlobHauler for ToMillauXcmBlobHauler { type MessagesInstance = WithMillauMessagesInstance; type SenderAndLane = MullauSenderAndLane; - type ToSendingChainSender = crate::XcmRouter; + type ToSourceChainSender = crate::XcmRouter; type CongestedMessage = DummyXcmMessage; type UncongestedMessage = DummyXcmMessage; diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index b5b82c4f58b..0927d9e405d 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -132,7 +132,7 @@ impl XcmBlobHauler for ToMillauXcmBlobHauler { type MessagesInstance = WithMillauMessagesInstance; type SenderAndLane = MullauSenderAndLane; - type ToSendingChainSender = crate::xcm_config::XcmRouter; + type ToSourceChainSender = crate::xcm_config::XcmRouter; type CongestedMessage = DummyXcmMessage; type UncongestedMessage = DummyXcmMessage; diff --git a/bin/runtime-common/src/messages_xcm_extension.rs b/bin/runtime-common/src/messages_xcm_extension.rs index ed9d23290ca..a2e4d82e269 100644 --- a/bin/runtime-common/src/messages_xcm_extension.rs +++ b/bin/runtime-common/src/messages_xcm_extension.rs @@ -27,7 +27,7 @@ use bp_messages::{ LaneId, MessageNonce, }; use bp_runtime::messages::MessageDispatchResult; -use bp_xcm_bridge_hub_router::LocalXcmChannel; +use bp_xcm_bridge_hub_router::XcmChannelStatusProvider; use codec::{Decode, Encode}; use frame_support::{dispatch::Weight, traits::Get, CloneNoBound, EqNoBound, PartialEqNoBound}; use frame_system::Config as SystemConfig; @@ -59,8 +59,11 @@ pub struct XcmBlobMessageDispatch { _marker: sp_std::marker::PhantomData<(DispatchBlob, Weights, Channel)>, } -impl - MessageDispatch for XcmBlobMessageDispatch +impl< + BlobDispatcher: DispatchBlob, + Weights: MessagesPalletWeights, + Channel: XcmChannelStatusProvider, + > MessageDispatch for XcmBlobMessageDispatch { type DispatchPayload = XcmAsPlainPayload; type DispatchLevelResult = XcmBlobMessageDispatchResult; @@ -145,22 +148,23 @@ pub trait XcmBlobHauler { /// Returns lane used by this hauler. type SenderAndLane: Get; - /// Actual XCM message sender (`HRMP` or `UMP`) to the sending chain + /// Actual XCM message sender (`HRMP` or `UMP`) to the source chain /// location (`Self::SenderAndLane::get().location`). - type ToSendingChainSender: SendXcm; + type ToSourceChainSender: SendXcm; /// An XCM message that is sent to the sending chain when the bridge queue becomes congested. type CongestedMessage: Get>; - /// An XCM message that is sent to the sending chain when the bridge queue becomes uncongested. + /// An XCM message that is sent to the sending chain when the bridge queue becomes not + /// congested. type UncongestedMessage: Get>; - /// Runtime message sender origin, which is used by [`Self::MessageSender`]. + /// Runtime message sender origin, which is used by the associated messages pallet. type MessageSenderOrigin; /// Runtime origin for our (i.e. this bridge hub) location within the Consensus Universe. fn message_sender_origin() -> Self::MessageSenderOrigin; } -/// XCM bridge adapter which connects [`XcmBlobHauler`] with [`XcmBlobHauler::MessageSender`] and -/// makes sure that XCM blob is sent to the [`pallet_bridge_messages`] queue to be relayed. +/// XCM bridge adapter which connects [`XcmBlobHauler`] with [`pallet_bridge_messages`] and +/// makes sure that XCM blob is sent to the outbound lane to be relayed. /// /// It needs to be used at the source bridge hub. pub struct XcmBlobHaulerAdapter(sp_std::marker::PhantomData); @@ -233,7 +237,7 @@ pub struct LocalXcmQueueManager(PhantomData); const OUTBOUND_LANE_CONGESTED_THRESHOLD: MessageNonce = 8_192; /// After we have sent "congestion" XCM message to the sending chain, we wait until number -/// of messages in the outbound bridge queue drops to this count, before sending "uncongestion" +/// of messages in the outbound bridge queue drops to this count, before sending `uncongestion` /// XCM message. const OUTBOUND_LANE_UNCONGESTED_THRESHOLD: MessageNonce = 1_024; @@ -243,8 +247,7 @@ impl LocalXcmQueueManager { sender_and_lane: &SenderAndLane, enqueued_messages: MessageNonce, ) { - // if we have alsender_and_laneready sent the congestion signal, we don't want to do - // anything + // if we have already sent the congestion signal, we don't want to do anything if Self::is_congested_signal_sent(sender_and_lane.lane) { return } @@ -311,12 +314,12 @@ impl LocalXcmQueueManager { /// Returns true if we have sent "congested" signal to the `sending_chain_location`. fn is_congested_signal_sent(lane: LaneId) -> bool { - OutboundLanesCongestedSignals::::get(&lane) + OutboundLanesCongestedSignals::::get(lane) } /// Send congested signal to the `sending_chain_location`. fn send_congested_signal(sender_and_lane: &SenderAndLane) -> Result<(), SendError> { - send_xcm::(sender_and_lane.location, H::CongestedMessage::get())?; + send_xcm::(sender_and_lane.location, H::CongestedMessage::get())?; OutboundLanesCongestedSignals::::insert( sender_and_lane.lane, true, @@ -324,15 +327,169 @@ impl LocalXcmQueueManager { Ok(()) } - /// Send uncongested signal to the `sending_chain_location`. + /// Send `uncongested` signal to the `sending_chain_location`. fn send_uncongested_signal(sender_and_lane: &SenderAndLane) -> Result<(), SendError> { - send_xcm::( - sender_and_lane.location, - H::UncongestedMessage::get(), - )?; + send_xcm::(sender_and_lane.location, H::UncongestedMessage::get())?; OutboundLanesCongestedSignals::::remove( - &sender_and_lane.lane, + sender_and_lane.lane, ); Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::*; + + use bp_messages::OutboundLaneData; + use frame_support::parameter_types; + use pallet_bridge_messages::OutboundLanes; + + parameter_types! { + pub TestSenderAndLane: SenderAndLane = SenderAndLane { + location: MultiLocation::new(1, X1(Parachain(1000))), + lane: TEST_LANE_ID, + }; + pub DummyXcmMessage: Xcm<()> = Xcm::new(); + } + + struct DummySendXcm; + + impl DummySendXcm { + fn messages_sent() -> u32 { + frame_support::storage::unhashed::get(b"DummySendXcm").unwrap_or(0) + } + } + + impl SendXcm for DummySendXcm { + type Ticket = (); + + fn validate( + _destination: &mut Option, + _message: &mut Option>, + ) -> SendResult { + Ok(((), Default::default())) + } + + fn deliver(_ticket: Self::Ticket) -> Result { + let messages_sent: u32 = Self::messages_sent(); + frame_support::storage::unhashed::put(b"DummySendXcm", &(messages_sent + 1)); + Ok(XcmHash::default()) + } + } + + struct TestBlobHauler; + + impl XcmBlobHauler for TestBlobHauler { + type Runtime = TestRuntime; + type MessagesInstance = (); + type SenderAndLane = TestSenderAndLane; + + type ToSourceChainSender = DummySendXcm; + type CongestedMessage = DummyXcmMessage; + type UncongestedMessage = DummyXcmMessage; + + type MessageSenderOrigin = RuntimeOrigin; + + fn message_sender_origin() -> Self::MessageSenderOrigin { + RuntimeOrigin::root() + } + } + + type TestBlobHaulerAdapter = XcmBlobHaulerAdapter; + + fn fill_up_lane_to_congestion() { + OutboundLanes::::insert( + TEST_LANE_ID, + OutboundLaneData { + oldest_unpruned_nonce: 0, + latest_received_nonce: 0, + latest_generated_nonce: OUTBOUND_LANE_CONGESTED_THRESHOLD, + }, + ); + } + + #[test] + fn congested_signal_is_not_sent_twice() { + run_test(|| { + fill_up_lane_to_congestion(); + + // next sent message leads to congested signal + TestBlobHaulerAdapter::haul_blob(vec![42]).unwrap(); + assert_eq!(DummySendXcm::messages_sent(), 1); + + // next sent message => we don't sent another congested signal + TestBlobHaulerAdapter::haul_blob(vec![42]).unwrap(); + assert_eq!(DummySendXcm::messages_sent(), 1); + }); + } + + #[test] + fn congested_signal_is_not_sent_when_outbound_lane_is_not_congested() { + run_test(|| { + TestBlobHaulerAdapter::haul_blob(vec![42]).unwrap(); + assert_eq!(DummySendXcm::messages_sent(), 0); + }); + } + + #[test] + fn congested_signal_is_sent_when_outbound_lane_is_congested() { + run_test(|| { + fill_up_lane_to_congestion(); + + // next sent message leads to congested signal + TestBlobHaulerAdapter::haul_blob(vec![42]).unwrap(); + assert_eq!(DummySendXcm::messages_sent(), 1); + assert!(LocalXcmQueueManager::::is_congested_signal_sent(TEST_LANE_ID)); + }); + } + + #[test] + fn uncongested_signal_is_not_sent_when_messages_are_delivered_at_other_lane() { + run_test(|| { + LocalXcmQueueManager::::send_congested_signal(&TestSenderAndLane::get()).unwrap(); + 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); + assert_eq!(DummySendXcm::messages_sent(), 1); + }); + } + + #[test] + fn uncongested_signal_is_not_sent_when_we_havent_send_congested_signal_before() { + run_test(|| { + TestBlobHaulerAdapter::on_messages_delivered(TEST_LANE_ID, 0); + assert_eq!(DummySendXcm::messages_sent(), 0); + }); + } + + #[test] + fn uncongested_signal_is_not_sent_if_outbound_lane_is_still_congested() { + run_test(|| { + LocalXcmQueueManager::::send_congested_signal(&TestSenderAndLane::get()).unwrap(); + assert_eq!(DummySendXcm::messages_sent(), 1); + + TestBlobHaulerAdapter::on_messages_delivered( + TEST_LANE_ID, + OUTBOUND_LANE_UNCONGESTED_THRESHOLD + 1, + ); + assert_eq!(DummySendXcm::messages_sent(), 1); + }); + } + + #[test] + fn uncongested_signal_is_sent_if_outbound_lane_is_uncongested() { + run_test(|| { + LocalXcmQueueManager::::send_congested_signal(&TestSenderAndLane::get()).unwrap(); + assert_eq!(DummySendXcm::messages_sent(), 1); + + TestBlobHaulerAdapter::on_messages_delivered( + TEST_LANE_ID, + OUTBOUND_LANE_UNCONGESTED_THRESHOLD, + ); + assert_eq!(DummySendXcm::messages_sent(), 2); + }); + } +} diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 27f98bc84d4..78f97745a5b 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -498,7 +498,7 @@ pub mod pallet { // notify others about messages delivery T::OnMessagesDelivered::on_messages_delivered( lane_id, - lane.data().queued_messages().checked_len().unwrap_or(0), + lane.data().queued_messages().saturating_len(), ); // because of lags, the inbound lane state (`lane_data`) may have entries for @@ -599,10 +599,12 @@ pub mod pallet { /// Map of lane id => is congested signal sent. It is managed by the /// `bridge_runtime_common::LocalXcmQueueManager`. /// - /// **bridges-v1**: this map is temporary and will be dropped in the v2. We can emulate - /// a storage map using unhashed storage functions, but then benchmarks are not accounting - /// its `proof_size`, so it is missing from the final weights. So we need to make it a map - /// inside some pallet. + /// **bridges-v1**: this map is a temporary hack and will be dropped in the `v2`. We can emulate + /// a storage map using `sp_io::unhashed` storage functions, but then benchmarks are not + /// accounting its `proof_size`, so it is missing from the final weights. So we need to make it + /// a map inside some pallet. We could use a simply value instead of map here, because + /// in `v1` we'll only have a single lane. But in the case of adding another lane before `v2`, + /// it'll be easier to deal with the isolated storage map instead. #[pallet::storage] pub type OutboundLanesCongestedSignals, I: 'static = ()> = StorageMap< Hasher = Blake2_128Concat, @@ -928,9 +930,10 @@ mod tests { inbound_unrewarded_relayers_state, message, message_payload, run_test, unrewarded_relayer, AccountId, DbWeight, RuntimeEvent as TestEvent, RuntimeOrigin, TestDeliveryConfirmationPayments, TestDeliveryPayments, TestMessageDispatch, - TestMessagesDeliveryProof, TestMessagesProof, TestRelayer, TestRuntime, TestWeightInfo, - MAX_OUTBOUND_PAYLOAD_SIZE, PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, - TEST_LANE_ID, TEST_LANE_ID_2, TEST_LANE_ID_3, TEST_RELAYER_A, TEST_RELAYER_B, + TestMessagesDeliveryProof, TestMessagesProof, TestOnMessagesDelivered, TestRelayer, + TestRuntime, TestWeightInfo, MAX_OUTBOUND_PAYLOAD_SIZE, + PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_LANE_ID_2, + TEST_LANE_ID_3, TEST_RELAYER_A, TEST_RELAYER_B, }, outbound_lane::ReceivalConfirmationError, }; @@ -1401,6 +1404,7 @@ mod tests { ); 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, 1))); // this reports delivery of both message 1 and message 2 => reward is paid only to // TEST_RELAYER_B @@ -1443,6 +1447,7 @@ mod tests { ); 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))); }); } diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index ffbbf120eeb..aac83998bbe 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -21,7 +21,9 @@ use crate::Config; use bp_messages::{ calc_relayers_rewards, - source_chain::{DeliveryConfirmationPayments, LaneMessageVerifier, TargetHeaderChain}, + source_chain::{ + DeliveryConfirmationPayments, LaneMessageVerifier, OnMessagesDelivered, TargetHeaderChain, + }, target_chain::{ DeliveryPayments, DispatchMessage, DispatchMessageData, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain, @@ -161,7 +163,7 @@ impl Config for TestRuntime { type TargetHeaderChain = TestTargetHeaderChain; type LaneMessageVerifier = TestLaneMessageVerifier; type DeliveryConfirmationPayments = TestDeliveryConfirmationPayments; - type OnMessagesDelivered = (); + type OnMessagesDelivered = TestOnMessagesDelivered; type SourceHeaderChain = TestSourceHeaderChain; type MessageDispatch = TestMessageDispatch; @@ -441,6 +443,24 @@ impl MessageDispatch for TestMessageDispatch { } } +/// Test callback, called during message delivery confirmation transaction. +pub struct TestOnMessagesDelivered; + +impl TestOnMessagesDelivered { + pub fn call_arguments() -> Option<(LaneId, MessageNonce)> { + frame_support::storage::unhashed::get(b"TestOnMessagesDelivered.OnMessagesDelivered") + } +} + +impl OnMessagesDelivered for TestOnMessagesDelivered { + fn on_messages_delivered(lane: LaneId, enqueued_messages: MessageNonce) { + frame_support::storage::unhashed::put( + b"TestOnMessagesDelivered.OnMessagesDelivered", + &(lane, enqueued_messages), + ); + } +} + /// 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() } diff --git a/modules/xcm-bridge-hub-router/src/benchmarking.rs b/modules/xcm-bridge-hub-router/src/benchmarking.rs index 007b131aeee..f8c666eea75 100644 --- a/modules/xcm-bridge-hub-router/src/benchmarking.rs +++ b/modules/xcm-bridge-hub-router/src/benchmarking.rs @@ -20,16 +20,13 @@ use crate::{Bridge, Call}; -use bp_xcm_bridge_hub_router::BridgeState; +use bp_xcm_bridge_hub_router::{BridgeState, MINIMAL_DELIVERY_FEE_FACTOR}; use frame_benchmarking::benchmarks_instance_pallet; use frame_support::{ dispatch::UnfilteredDispatchable, traits::{EnsureOrigin, Get, Hooks}, }; -use sp_runtime::{ - traits::{One, Zero}, - FixedU128, -}; +use sp_runtime::traits::Zero; use xcm::prelude::*; /// Pallet we're benchmarking here. @@ -45,7 +42,7 @@ benchmarks_instance_pallet! { on_initialize_when_non_congested { Bridge::::put(BridgeState { is_congested: false, - delivery_fee_factor: FixedU128::one() + FixedU128::one(), + delivery_fee_factor: MINIMAL_DELIVERY_FEE_FACTOR + MINIMAL_DELIVERY_FEE_FACTOR, }); }: { crate::Pallet::::on_initialize(Zero::zero()) @@ -54,7 +51,7 @@ benchmarks_instance_pallet! { on_initialize_when_congested { Bridge::::put(BridgeState { is_congested: false, - delivery_fee_factor: FixedU128::one() + FixedU128::one(), + delivery_fee_factor: MINIMAL_DELIVERY_FEE_FACTOR + MINIMAL_DELIVERY_FEE_FACTOR, }); T::make_congested(); }: { @@ -80,13 +77,13 @@ benchmarks_instance_pallet! { let dest = MultiLocation::new( T::UniversalLocation::get().len() as u8, - X1(GlobalConsensus(T::BridgedNetworkId::get())), + X1(GlobalConsensus(T::BridgedNetworkId::get().unwrap())), ); let xcm = sp_std::vec![].into(); }: { send_xcm::>(dest, xcm).expect("message is sent") } verify { - assert!(Bridge::::get().delivery_fee_factor > FixedU128::one()); + assert!(Bridge::::get().delivery_fee_factor > MINIMAL_DELIVERY_FEE_FACTOR); } } diff --git a/modules/xcm-bridge-hub-router/src/lib.rs b/modules/xcm-bridge-hub-router/src/lib.rs index 35f851e0de3..87e050b45c7 100644 --- a/modules/xcm-bridge-hub-router/src/lib.rs +++ b/modules/xcm-bridge-hub-router/src/lib.rs @@ -22,14 +22,21 @@ //! All other bridge hub queues offer some backpressure mechanisms. So if at least one //! of all queues is congested, it will eventually lead to the growth of the queue at //! this chain. +//! +//! **A note on terminology**: when we mention the bridge hub here, we mean the chain that +//! has the messages pallet deployed (`pallet-bridge-grandpa`, `pallet-bridge-messages`, +//! `pallet-xcm-bridge-hub`, ...). It may be the system bridge hub parachain or any other +//! chain. #![cfg_attr(not(feature = "std"), no_std)] -use bp_xcm_bridge_hub_router::{BridgeState, LocalXcmChannel}; +use bp_xcm_bridge_hub_router::{ + BridgeState, XcmChannelStatusProvider, MINIMAL_DELIVERY_FEE_FACTOR, +}; use codec::Encode; use frame_support::traits::Get; use sp_core::H256; -use sp_runtime::{traits::One, FixedPointNumber, FixedU128, Saturating}; +use sp_runtime::{FixedPointNumber, FixedU128, Saturating}; use xcm::prelude::*; use xcm_builder::{ExporterFor, SovereignPaidRemoteExporter}; @@ -74,10 +81,14 @@ pub mod pallet { /// Universal location of this runtime. type UniversalLocation: Get; - /// Relative location of the sibling bridge hub. - type SiblingBridgeHubLocation: Get; - /// The bridged network that this config is for. - type BridgedNetworkId: Get; + /// The bridged network that this config is for if specified. + /// Also used for filtering `Bridges` by `BridgedNetworkId`. + /// If not specified, allows all networks pass through. + type BridgedNetworkId: Get>; + /// Configuration for supported **bridged networks/locations** with **bridge location** and + /// **possible fee**. Allows to externalize better control over allowed **bridged + /// networks/locations**. + type Bridges: ExporterFor; /// Origin of the sibling bridge hub that is allowed to report bridge status. type BridgeHubOrigin: EnsureOrigin; @@ -85,10 +96,8 @@ pub mod pallet { type ToBridgeHubSender: SendXcm; /// Underlying channel with the sibling bridge hub. It must match the channel, used /// by the `Self::ToBridgeHubSender`. - type WithBridgeHubChannel: LocalXcmChannel; + type WithBridgeHubChannel: XcmChannelStatusProvider; - /// Base bridge fee that is paid for every outbound message. - type BaseFee: Get; /// Additional fee that is paid for every byte of the outbound message. type ByteFee: Get; /// Asset that is used to paid bridge fee. @@ -101,40 +110,40 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { fn on_initialize(_n: BlockNumberFor) -> Weight { - Bridge::::mutate(|bridge| { - // TODO: make sure that `WithBridgeHubChannel::is_congested` returns true if either - // of XCM channels (outbound/inbound) is suspended. Because if outbound is suspended - // that is definitely congestion. If inbound is suspended, then we are not able to - // receive the "report_bridge_status" signal (that maybe sent by the bridge hub). - - // if the channel with sibling/child bridge hub is suspended, we don't change - // anything - if T::WithBridgeHubChannel::is_congested() { - return T::WeightInfo::on_initialize_when_congested() - } + // TODO: make sure that `WithBridgeHubChannel::is_congested` returns true if either + // of XCM channels (outbound/inbound) is suspended. Because if outbound is suspended + // that is definitely congestion. If inbound is suspended, then we are not able to + // receive the "report_bridge_status" signal (that maybe sent by the bridge hub). + + // if the channel with sibling/child bridge hub is suspended, we don't change + // anything + if T::WithBridgeHubChannel::is_congested() { + return T::WeightInfo::on_initialize_when_congested() + } - // if bridge has reported congestion, we don't change anything - if bridge.is_congested { - return T::WeightInfo::on_initialize_when_congested() - } + // if bridge has reported congestion, we don't change anything + let mut bridge = Self::bridge(); + if bridge.is_congested { + return T::WeightInfo::on_initialize_when_congested() + } - // if fee factor is already minimal, we don't change anything - if bridge.delivery_fee_factor == FixedU128::one() { - return T::WeightInfo::on_initialize_when_congested() - } + // if fee factor is already minimal, we don't change anything + if bridge.delivery_fee_factor == MINIMAL_DELIVERY_FEE_FACTOR { + return T::WeightInfo::on_initialize_when_congested() + } - let previous_factor = bridge.delivery_fee_factor; - bridge.delivery_fee_factor = - FixedU128::one().max(bridge.delivery_fee_factor / EXPONENTIAL_FEE_BASE); - log::info!( - target: LOG_TARGET, - "Bridge queue is uncongested. Decreased fee factor from {} to {}", - previous_factor, - bridge.delivery_fee_factor, - ); + let previous_factor = bridge.delivery_fee_factor; + bridge.delivery_fee_factor = + MINIMAL_DELIVERY_FEE_FACTOR.max(bridge.delivery_fee_factor / EXPONENTIAL_FEE_BASE); + log::info!( + target: LOG_TARGET, + "Bridge queue is uncongested. Decreased fee factor from {} to {}", + previous_factor, + bridge.delivery_fee_factor, + ); - T::WeightInfo::on_initialize_when_non_congested() - }) + Bridge::::put(bridge); + T::WeightInfo::on_initialize_when_non_congested() } } @@ -142,15 +151,23 @@ pub mod pallet { impl, I: 'static> Pallet { /// Notification about congested bridge queue. #[pallet::call_index(0)] - #[pallet::weight(Weight::zero())] // TODO + #[pallet::weight(T::WeightInfo::report_bridge_status())] pub fn report_bridge_status( origin: OriginFor, // this argument is not currently used, but to ease future migration, we'll keep it // here - _bridge_id: H256, + bridge_id: H256, is_congested: bool, ) -> DispatchResult { let _ = T::BridgeHubOrigin::ensure_origin(origin)?; + + log::info!( + target: LOG_TARGET, + "Received bridge status from {:?}: congested = {}", + bridge_id, + is_congested, + ); + Bridge::::mutate(|bridge| { bridge.is_congested = is_congested; }); @@ -162,7 +179,7 @@ pub mod pallet { /// /// **bridges-v1** assumptions: all outbound messages through this router are using single lane /// and to single remote consensus. If there is some other remote consensus that uses the same - /// bridge hub, the separate pallet instance shall be used, In v2 we'll have all required + /// bridge hub, the separate pallet instance shall be used, In `v2` we'll have all required /// primitives (lane-id aka bridge-id, derived from XCM locations) to support multiple bridges /// by the same pallet instance. #[pallet::storage] @@ -192,7 +209,7 @@ pub mod pallet { log::info!( target: LOG_TARGET, - "Bridge queue is congested. Increased fee factor from {} to {}", + "Bridge channel is congested. Increased fee factor from {} to {}", previous_factor, bridge.delivery_fee_factor, ); @@ -219,30 +236,76 @@ impl, I: 'static> ExporterFor for Pallet { remote_location: &InteriorMultiLocation, message: &Xcm<()>, ) -> Option<(MultiLocation, Option)> { - // ensure that the message is sent to the expected bridged network - if *network != T::BridgedNetworkId::get() { - return None + // ensure that the message is sent to the expected bridged network (if specified). + if let Some(bridged_network) = T::BridgedNetworkId::get() { + if *network != bridged_network { + log::trace!( + target: LOG_TARGET, + "Router with bridged_network_id {:?} does not support bridging to network {:?}!", + bridged_network, + network, + ); + return None + } } + // ensure that the message is sent to the expected bridged network and location. + let Some((bridge_hub_location, maybe_payment)) = + T::Bridges::exporter_for(network, remote_location, message) + else { + log::trace!( + target: LOG_TARGET, + "Router with bridged_network_id {:?} does not support bridging to network {:?} and remote_location {:?}!", + T::BridgedNetworkId::get(), + network, + remote_location, + ); + return None + }; + + // take `base_fee` from `T::Brides`, but it has to be the same `T::FeeAsset` + let base_fee = match maybe_payment { + Some(payment) => match payment { + MultiAsset { fun: Fungible(amount), id } if id.eq(&T::FeeAsset::get()) => amount, + invalid_asset => { + log::error!( + target: LOG_TARGET, + "Router with bridged_network_id {:?} is configured for `T::FeeAsset` {:?} which is not \ + compatible with {:?} for bridge_hub_location: {:?} for bridging to {:?}/{:?}!", + T::BridgedNetworkId::get(), + T::FeeAsset::get(), + invalid_asset, + bridge_hub_location, + network, + remote_location, + ); + return None + }, + }, + None => 0, + }; + // compute fee amount. Keep in mind that this is only the bridge fee. The fee for sending // message from this chain to child/sibling bridge hub is determined by the // `Config::ToBridgeHubSender` let message_size = message.encoded_size(); let message_fee = (message_size as u128).saturating_mul(T::ByteFee::get()); - let fee_sum = T::BaseFee::get().saturating_add(message_fee); + let fee_sum = base_fee.saturating_add(message_fee); let fee_factor = Self::bridge().delivery_fee_factor; let fee = fee_factor.saturating_mul_int(fee_sum); + let fee = if fee > 0 { Some((T::FeeAsset::get(), fee).into()) } else { None }; + log::info!( target: LOG_TARGET, - "Going to send message to {:?} ({} bytes) over bridge. Computed bridge fee {} using fee factor {}", + "Going to send message to {:?} ({} bytes) over bridge. Computed bridge fee {:?} using fee factor {}", (network, remote_location), - fee, - fee_factor, message_size, + fee, + fee_factor ); - Some((T::SiblingBridgeHubLocation::get(), Some((T::FeeAsset::get(), fee).into()))) + Some((bridge_hub_location, fee)) } } @@ -298,6 +361,7 @@ mod tests { use mock::*; use frame_support::traits::Hooks; + use sp_runtime::traits::One; fn congested_bridge(delivery_fee_factor: FixedU128) -> BridgeState { BridgeState { is_congested: true, delivery_fee_factor } @@ -310,7 +374,10 @@ mod tests { #[test] fn initial_fee_factor_is_one() { run_test(|| { - assert_eq!(Bridge::::get(), uncongested_bridge(FixedU128::one()),); + assert_eq!( + Bridge::::get(), + uncongested_bridge(MINIMAL_DELIVERY_FEE_FACTOR), + ); }) } @@ -345,13 +412,16 @@ mod tests { Bridge::::put(uncongested_bridge(FixedU128::from_rational(125, 100))); // it shold eventually decreased to one - while XcmBridgeHubRouter::bridge().delivery_fee_factor > FixedU128::one() { + while XcmBridgeHubRouter::bridge().delivery_fee_factor > MINIMAL_DELIVERY_FEE_FACTOR { XcmBridgeHubRouter::on_initialize(One::one()); } // verify that it doesn't decreases anymore XcmBridgeHubRouter::on_initialize(One::one()); - assert_eq!(XcmBridgeHubRouter::bridge(), uncongested_bridge(FixedU128::one())); + assert_eq!( + XcmBridgeHubRouter::bridge(), + uncongested_bridge(MINIMAL_DELIVERY_FEE_FACTOR) + ); }) } @@ -463,7 +533,7 @@ mod tests { #[test] fn sent_message_increases_factor_if_bridge_has_reported_congestion() { run_test(|| { - Bridge::::put(congested_bridge(FixedU128::one())); + Bridge::::put(congested_bridge(MINIMAL_DELIVERY_FEE_FACTOR)); let old_bridge = XcmBridgeHubRouter::bridge(); assert_eq!( diff --git a/modules/xcm-bridge-hub-router/src/mock.rs b/modules/xcm-bridge-hub-router/src/mock.rs index 7c22e302ad6..5ad7be4890a 100644 --- a/modules/xcm-bridge-hub-router/src/mock.rs +++ b/modules/xcm-bridge-hub-router/src/mock.rs @@ -18,7 +18,7 @@ use crate as pallet_xcm_bridge_hub_router; -use bp_xcm_bridge_hub_router::LocalXcmChannel; +use bp_xcm_bridge_hub_router::XcmChannelStatusProvider; use frame_support::{construct_runtime, parameter_types}; use frame_system::EnsureRoot; use sp_core::H256; @@ -27,6 +27,7 @@ use sp_runtime::{ BuildStorage, }; use xcm::prelude::*; +use xcm_builder::NetworkExportTable; pub type AccountId = u64; type Block = frame_system::mocking::MockBlock; @@ -52,6 +53,8 @@ parameter_types! { pub UniversalLocation: InteriorMultiLocation = X2(GlobalConsensus(ThisNetworkId::get()), Parachain(1000)); pub SiblingBridgeHubLocation: MultiLocation = ParentThen(X1(Parachain(1002))).into(); pub BridgeFeeAsset: AssetId = MultiLocation::parent().into(); + pub BridgeTable: Vec<(NetworkId, MultiLocation, Option)> + = vec![(BridgedNetworkId::get(), SiblingBridgeHubLocation::get(), Some((BridgeFeeAsset::get(), BASE_FEE).into()))]; } impl frame_system::Config for TestRuntime { @@ -84,14 +87,13 @@ impl pallet_xcm_bridge_hub_router::Config<()> for TestRuntime { type WeightInfo = (); type UniversalLocation = UniversalLocation; - type SiblingBridgeHubLocation = SiblingBridgeHubLocation; type BridgedNetworkId = BridgedNetworkId; + type Bridges = NetworkExportTable; type BridgeHubOrigin = EnsureRoot; type ToBridgeHubSender = TestToBridgeHubSender; type WithBridgeHubChannel = TestWithBridgeHubChannel; - type BaseFee = ConstU128; type ByteFee = ConstU128; type FeeAsset = BridgeFeeAsset; } @@ -128,7 +130,7 @@ impl TestWithBridgeHubChannel { } } -impl LocalXcmChannel for TestWithBridgeHubChannel { +impl XcmChannelStatusProvider for TestWithBridgeHubChannel { fn is_congested() -> bool { frame_support::storage::unhashed::get_or_default(b"TestWithBridgeHubChannel.Congested") } diff --git a/primitives/messages/src/source_chain.rs b/primitives/messages/src/source_chain.rs index fa067b611d0..ce612e83e73 100644 --- a/primitives/messages/src/source_chain.rs +++ b/primitives/messages/src/source_chain.rs @@ -116,10 +116,10 @@ impl DeliveryConfirmationPayments for () { } } -/// Callback that is called at the source chain (bridge hub) when we get delivery confrimation +/// Callback that is called at the source chain (bridge hub) when we get delivery confirmation /// for new messages. pub trait OnMessagesDelivered { - /// New messages delivery has been confimed. + /// New messages delivery has been confirmed. /// /// The only argument of the function is the number of yet undelivered messages fn on_messages_delivered(lane: LaneId, enqueued_messages: MessageNonce); diff --git a/primitives/xcm-bridge-hub-router/src/lib.rs b/primitives/xcm-bridge-hub-router/src/lib.rs index 4fe5b789c08..1f1a92cc75f 100644 --- a/primitives/xcm-bridge-hub-router/src/lib.rs +++ b/primitives/xcm-bridge-hub-router/src/lib.rs @@ -20,15 +20,21 @@ use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; -use sp_runtime::{traits::One, FixedU128, RuntimeDebug}; +use sp_runtime::{FixedU128, RuntimeDebug}; -/// Local XCM channel that may report whether it is congested or not. -pub trait LocalXcmChannel { - /// Returns true if the queue is currently congested. +/// Minimal delivery fee factor. +pub const MINIMAL_DELIVERY_FEE_FACTOR: FixedU128 = FixedU128::from_u32(1); + +/// XCM channel status provider that may report whether it is congested or not. +/// +/// By channel we mean the physical channel that is used to deliver messages of one +/// of the bridge queues. +pub trait XcmChannelStatusProvider { + /// Returns true if the channel is currently congested. fn is_congested() -> bool; } -impl LocalXcmChannel for () { +impl XcmChannelStatusProvider for () { fn is_congested() -> bool { false } @@ -45,6 +51,6 @@ pub struct BridgeState { impl Default for BridgeState { fn default() -> BridgeState { - BridgeState { delivery_fee_factor: FixedU128::one(), is_congested: false } + BridgeState { delivery_fee_factor: MINIMAL_DELIVERY_FEE_FACTOR, is_congested: false } } }