From d821c08ef67a6d1972e825a669f2d218e44ff09b Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Sun, 3 Nov 2024 00:34:54 +0100 Subject: [PATCH] Nits --- .../modules/xcm-bridge-hub-router/src/lib.rs | 33 ++++++++- .../modules/xcm-bridge-hub-router/src/mock.rs | 4 +- .../modules/xcm-bridge-hub/src/exporter.rs | 68 ++++++++++++++++++- bridges/modules/xcm-bridge-hub/src/mock.rs | 53 ++++++++------- 4 files changed, 125 insertions(+), 33 deletions(-) diff --git a/bridges/modules/xcm-bridge-hub-router/src/lib.rs b/bridges/modules/xcm-bridge-hub-router/src/lib.rs index e032c1228c820..1d4a4c4fdcdbe 100644 --- a/bridges/modules/xcm-bridge-hub-router/src/lib.rs +++ b/bridges/modules/xcm-bridge-hub-router/src/lib.rs @@ -29,7 +29,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -pub use bp_xcm_bridge_hub_router::XcmChannelStatusProvider; use bp_xcm_bridge_hub_router::{BridgeState, ResolveBridgeId}; use codec::Encode; use frame_support::traits::Get; @@ -76,9 +75,35 @@ pub mod pallet { use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; - #[pallet::config] + /// Default implementations of [`DefaultConfig`], which can be used to implement [`Config`]. + pub mod config_preludes { + use super::*; + use frame_support::{derive_impl, traits::ConstU128}; + + /// A type providing default configurations for this pallet in testing environment. + pub struct TestDefaultConfig; + + #[derive_impl(frame_system::config_preludes::TestDefaultConfig, no_aggregated_types)] + impl frame_system::DefaultConfig for TestDefaultConfig {} + + #[frame_support::register_default_impl(TestDefaultConfig)] + impl DefaultConfig for TestDefaultConfig { + #[inject_runtime_type] + type RuntimeEvent = (); + type WeightInfo = (); + type DestinationVersion = AlwaysLatest; + + // We don't need (optional) message_size fees. + type ByteFee = ConstU128<0>; + // We don't need (optional) message_size fees. + type FeeAsset = (); + } + } + + #[pallet::config(with_default)] pub trait Config: frame_system::Config { /// The overarching event type. + #[pallet::no_default_bounds] type RuntimeEvent: From> + IsType<::RuntimeEvent>; /// Benchmarks results from runtime we're plugged into. @@ -94,10 +119,12 @@ pub mod pallet { /// - The local chain, in which case we need an implementation for `T::ToBridgeHubSender` /// that does not use `ExportMessage` but instead directly calls the `ExportXcm` /// implementation. + #[pallet::no_default] type ToBridgeHubSender: SendXcm; /// Resolves a specific `BridgeId` for `dest`, used for identifying the bridge in cases of congestion and dynamic fees. /// If it resolves to `None`, it means no congestion or dynamic fees are handled for `dest`. + #[pallet::no_default] type BridgeIdResolver: ResolveBridgeId; /// Additional fee that is paid for every byte of the outbound message. @@ -179,7 +206,7 @@ pub mod pallet { /// Stores `BridgeState` for congestion control and dynamic fees for each resolved bridge ID associated with a destination. #[pallet::storage] - pub type Bridges, I: 'static = ()> = StorageMap<_, Blake2_128Concat, BridgeIdOf, BridgeState, OptionQuery>; + pub type Bridges, I: 'static = ()> = StorageMap<_, Identity, BridgeIdOf, BridgeState, OptionQuery>; impl, I: 'static> Pallet { /// Called when new message is sent to the `dest` (queued to local outbound XCM queue). diff --git a/bridges/modules/xcm-bridge-hub-router/src/mock.rs b/bridges/modules/xcm-bridge-hub-router/src/mock.rs index 2488f35c660a7..a3b078c2c471f 100644 --- a/bridges/modules/xcm-bridge-hub-router/src/mock.rs +++ b/bridges/modules/xcm-bridge-hub-router/src/mock.rs @@ -86,10 +86,8 @@ impl ResolveBridgeId for EveryDestinationToSameBridgeIdResolver { } /// An instance of `pallet_xcm_bridge_hub_router` configured to use a remote exporter with the `ExportMessage` instruction, which will be delivered to a sibling parachain using `SiblingBridgeHubLocation`. +#[derive_impl(pallet_xcm_bridge_hub_router::config_preludes::TestDefaultConfig)] impl pallet_xcm_bridge_hub_router::Config<()> for TestRuntime { - type RuntimeEvent = RuntimeEvent; - type WeightInfo = (); - type DestinationVersion = LatestOrNoneForLocationVersionChecker>; diff --git a/bridges/modules/xcm-bridge-hub/src/exporter.rs b/bridges/modules/xcm-bridge-hub/src/exporter.rs index 295ea2301ab4f..bbdca5dd6dd9e 100644 --- a/bridges/modules/xcm-bridge-hub/src/exporter.rs +++ b/bridges/modules/xcm-bridge-hub/src/exporter.rs @@ -365,6 +365,7 @@ mod tests { use bp_runtime::RangeInclusiveExt; use bp_xcm_bridge_hub::{Bridge, BridgeLocations, BridgeState}; + use bp_xcm_bridge_hub_router::ResolveBridgeId; use frame_support::{ assert_ok, traits::{Contains, EnsureOrigin}, @@ -632,7 +633,16 @@ mod tests { let origin = OpenBridgeOrigin::sibling_parachain_origin(); let origin_as_location = OpenBridgeOriginOf::::try_origin(origin.clone()).unwrap(); - let (_, expected_lane_id) = open_lane(origin); + let (bridge, expected_lane_id) = open_lane(origin); + + // we need to set `UniversalLocation` for `sibling_parachain_origin` for `XcmOverBridgeWrappedWithExportMessageRouterInstance`. + ExportMessageOriginUniversalLocation::set(Some(SiblingUniversalLocation::get())); + + // check compatible bridge_id + assert_eq!( + bridge.bridge_id(), + &>::BridgeIdResolver::resolve_for_dest(&dest).unwrap() + ); // check before - no messages assert_eq!( @@ -683,7 +693,13 @@ mod tests { // open bridge as a root on the local chain, which should be converted as // `Location::here()` - let (_, expected_lane_id) = open_lane(RuntimeOrigin::root()); + let (bridge, expected_lane_id) = open_lane(RuntimeOrigin::root()); + + // check compatible bridge_id + assert_eq!( + bridge.bridge_id(), + &>::BridgeIdResolver::resolve_for_dest(&dest).unwrap() + ); // check before - no messages assert_eq!( @@ -804,4 +820,52 @@ mod tests { assert_eq!(None, dest_wrapper); }); } + + #[test] + fn congestion_with_pallet_xcm_bridge_hub_router_works() { + run_test(|| { + // valid routable destination + let dest = Location::new(2, BridgedUniversalDestination::get()); + + fn router_bridge_state, I: 'static>(dest: &Location) -> Option { + let bridge_id = ::resolve_for_dest(dest).unwrap(); + pallet_xcm_bridge_hub_router::Bridges::::get(&bridge_id) + } + + // open two bridges + let origin = OpenBridgeOrigin::sibling_parachain_origin(); + let origin_as_location = OpenBridgeOriginOf::::try_origin(origin.clone()).unwrap(); + let (bridge_1, expected_lane_id_1) = open_lane(origin); + let (bridge_2, expected_lane_id_2) = open_lane(RuntimeOrigin::root()); + assert_ne!(expected_lane_id_1, expected_lane_id_2); + assert_ne!(bridge_1.bridge_id(), bridge_2.bridge_id()); + + // we need to set `UniversalLocation` for `sibling_parachain_origin` for `XcmOverBridgeWrappedWithExportMessageRouterInstance`. + ExportMessageOriginUniversalLocation::set(Some(SiblingUniversalLocation::get())); + + // check before + assert_eq!(XcmOverBridge::bridge(bridge_1.bridge_id()).unwrap().state, BridgeState::Opened); + assert_eq!(XcmOverBridge::bridge(bridge_2.bridge_id()).unwrap().state, BridgeState::Opened); + // both routers are uncongested + assert!(!router_bridge_state::(&dest).map(|bs| bs.is_congested).unwrap_or(false)); + assert!(!router_bridge_state::(&dest).map(|bs| bs.is_congested).unwrap_or(false)); + + // make bridges congested with sending too much messages + for _ in 1..(OUTBOUND_LANE_CONGESTED_THRESHOLD + 2) { + // send `ExportMessage(message)` by `pallet_xcm_bridge_hub_router`. + ExecuteXcmOverSendXcm::set_origin_for_execute(origin_as_location.clone()); + assert_ok!(send_xcm::(dest.clone(), Xcm::<()>::default())); + + // call direct `ExportXcm` by `pallet_xcm_bridge_hub_router`. + assert_ok!(send_xcm::(dest.clone(), Xcm::<()>::default())); + } + + // checks after + assert_eq!(XcmOverBridge::bridge(bridge_1.bridge_id()).unwrap().state, BridgeState::Suspended); + assert_eq!(XcmOverBridge::bridge(bridge_2.bridge_id()).unwrap().state, BridgeState::Suspended); + // both routers are congested + assert!(router_bridge_state::(&dest).unwrap().is_congested); + assert!(router_bridge_state::(&dest).unwrap().is_congested); + }) + } } diff --git a/bridges/modules/xcm-bridge-hub/src/mock.rs b/bridges/modules/xcm-bridge-hub/src/mock.rs index d6dc00f1eaa77..63f767045cf54 100644 --- a/bridges/modules/xcm-bridge-hub/src/mock.rs +++ b/bridges/modules/xcm-bridge-hub/src/mock.rs @@ -38,7 +38,7 @@ use polkadot_parachain_primitives::primitives::Sibling; use sp_core::H256; use sp_runtime::{ testing::Header as SubstrateHeader, - traits::{BlakeTwo256, ConstU128, ConstU32, IdentityLookup}, + traits::{BlakeTwo256, ConstU32, IdentityLookup}, AccountId32, BuildStorage, StateVersion, }; use sp_std::cell::RefCell; @@ -172,7 +172,6 @@ parameter_types! { // configuration for pallet_xcm_bridge_hub_router pub BridgeHubLocation: Location = Here.into(); - pub BridgeFeeAsset: AssetId = Location::here().into(); pub BridgeTable: Vec = vec![ NetworkExportTableItem::new( @@ -220,12 +219,9 @@ impl pallet_xcm_bridge_hub::Config for TestRuntime { /// A router instance simulates a scenario where the router is deployed on a different chain than /// the `MessageExporter`. This means that the router sends an `ExportMessage`. +pub type XcmOverBridgeWrappedWithExportMessageRouterInstance = (); +#[derive_impl(pallet_xcm_bridge_hub_router::config_preludes::TestDefaultConfig)] impl pallet_xcm_bridge_hub_router::Config<()> for TestRuntime { - type RuntimeEvent = RuntimeEvent; - type WeightInfo = (); - - type DestinationVersion = AlwaysLatest; - // We use `SovereignPaidRemoteExporter` here to test and ensure that the `ExportMessage` // produced by `pallet_xcm_bridge_hub_router` is compatible with the `ExportXcm` implementation // of `pallet_xcm_bridge_hub`. @@ -241,36 +237,27 @@ impl pallet_xcm_bridge_hub_router::Config<()> for TestRuntime { // calls the `ExportXcm` implementation of `pallet_xcm_bridge_hub` as the // `MessageExporter`. ExecuteXcmOverSendXcm, - UniversalLocation, + ExportMessageOriginUniversalLocation, >; - type BridgeIdResolver = BridgeIdResolver; - - type ByteFee = ConstU128<0>; - type FeeAsset = BridgeFeeAsset; + type BridgeIdResolver = EnsureIsRemoteBridgeIdResolver; } /// A router instance simulates a scenario where the router is deployed on the same chain as the /// `MessageExporter`. This means that the router triggers `ExportXcm` trait directly. -impl pallet_xcm_bridge_hub_router::Config for TestRuntime { - type RuntimeEvent = RuntimeEvent; - type WeightInfo = (); - - type DestinationVersion = AlwaysLatest; - +pub type XcmOverBridgeByExportXcmRouterInstance = pallet_xcm_bridge_hub_router::Instance2; +#[derive_impl(pallet_xcm_bridge_hub_router::config_preludes::TestDefaultConfig)] +impl pallet_xcm_bridge_hub_router::Config for TestRuntime { // We use `UnpaidLocalExporter` here to test and ensure that `pallet_xcm_bridge_hub_router` can // trigger directly `pallet_xcm_bridge_hub` as exporter. type ToBridgeHubSender = UnpaidLocalExporter; - type BridgeIdResolver = BridgeIdResolver; - - type ByteFee = ConstU128<0>; - type FeeAsset = BridgeFeeAsset; + type BridgeIdResolver = EnsureIsRemoteBridgeIdResolver; } -/// Implementation of `ResolveBridgeId` returning `BridgeId`. -pub struct BridgeIdResolver(PhantomData); -impl> ResolveBridgeId for BridgeIdResolver { +/// Implementation of `ResolveBridgeId` returning `BridgeId` based on the configured `UniversalLocation`. +pub struct EnsureIsRemoteBridgeIdResolver(PhantomData); +impl> ResolveBridgeId for EnsureIsRemoteBridgeIdResolver { type BridgeId = BridgeId; fn resolve_for_dest(dest: &Location) -> Option { @@ -284,6 +271,22 @@ impl> ResolveBridgeId for BridgeIdResol } } +/// A dynamic way to set different universal location for the origin which sends `ExportMessage`. +pub struct ExportMessageOriginUniversalLocation; +impl ExportMessageOriginUniversalLocation { + pub(crate) fn set(universal_location: Option) { + EXPORT_MESSAGE_ORIGIN_UNIVERSAL_LOCATION.with(|o| *o.borrow_mut() = universal_location); + } +} +impl Get for ExportMessageOriginUniversalLocation { + fn get() -> InteriorLocation { + EXPORT_MESSAGE_ORIGIN_UNIVERSAL_LOCATION.with(|o| o.borrow().clone().expect("`EXPORT_MESSAGE_ORIGIN_UNIVERSAL_LOCATION` is not set!")) + } +} +thread_local! { + pub static EXPORT_MESSAGE_ORIGIN_UNIVERSAL_LOCATION: RefCell> = RefCell::new(None); +} + pub struct XcmConfig; impl xcm_executor::Config for XcmConfig { type RuntimeCall = RuntimeCall;