From cfb29254f74412cea35e8048d8aea94bc789fcb1 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Wed, 11 Oct 2023 10:32:49 +0200 Subject: [PATCH] Xcm emulator nits (#1649) # Desription ## Summary This PR introduces several nits and tweaks to xcm emulator tests for system parachains. ## Explanation **Deduplicate `XcmPallet::send(` with root origin code** - Introduced `send_transact_to_parachain` which could be easily reuse for scenarios like _governance call from relay chain to parachain_. **Refactor `send_transact_sudo_from_relay_to_system_para_works`** - Test covered just one use-case which was moved to the `do_force_create_asset_from_relay_to_system_para`, so now we can extend this test with more _governance-like_ senarios. - Renamed to `send_transact_as_superuser_from_relay_to_system_para_works`. **Remove `send_transact_native_from_relay_to_system_para_fails` test** - This test and/or description is kind of misleading, because system paras support Native from relay chain by `RelayChainAsNative` with correct xcm origin. - It tested only sending on relay chain which should go directly to the relay chain unit-tests (does not even need to be in xcm emulator level). ## Future directions Check restructure parachains integration tests [issue](https://github.com/paritytech/polkadot-sdk/issues/1389) and [PR with more TODOs](https://github.com/paritytech/polkadot-sdk/pull/1693). --------- Co-authored-by: Ignacio Palacios --- .../src/tests/reserve_transfer.rs | 2 + .../asset-hub-westend/src/tests/send.rs | 51 ++-------- .../src/tests/set_xcm_versions.rs | 38 +++---- .../emulated/common/src/impls.rs | 98 ++++++++++++------- .../emulated/common/src/lib.rs | 14 +-- 5 files changed, 89 insertions(+), 114 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/reserve_transfer.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/reserve_transfer.rs index 805c8811b2dd..8f8b7a7dde77 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/reserve_transfer.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/reserve_transfer.rs @@ -349,6 +349,7 @@ fn limited_reserve_transfer_asset_from_system_para_to_para() { ASSET_MIN_BALANCE, true, AssetHubWestendSender::get(), + Some(Weight::from_parts(1_019_445_000, 200_000)), ASSET_MIN_BALANCE * 1000000, ); @@ -384,6 +385,7 @@ fn reserve_transfer_asset_from_system_para_to_para() { ASSET_MIN_BALANCE, true, AssetHubWestendSender::get(), + Some(Weight::from_parts(1_019_445_000, 200_000)), ASSET_MIN_BALANCE * 1000000, ); diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/send.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/send.rs index d0a71e88c674..e603af685bb5 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/send.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/send.rs @@ -16,52 +16,16 @@ use crate::*; /// Relay Chain should be able to execute `Transact` instructions in System Parachain -/// when `OriginKind::Superuser` and signer is `sudo` +/// when `OriginKind::Superuser`. #[test] -fn send_transact_sudo_from_relay_to_system_para_works() { - // Init tests variables - let root_origin = ::RuntimeOrigin::root(); - let system_para_destination = Westend::child_location_of(AssetHubWestend::para_id()).into(); - let asset_owner: AccountId = AssetHubWestendSender::get().into(); - let xcm = AssetHubWestend::force_create_asset_xcm( - OriginKind::Superuser, +fn send_transact_as_superuser_from_relay_to_system_para_works() { + AssetHubWestend::force_create_asset_from_relay_as_root( ASSET_ID, - asset_owner.clone(), + ASSET_MIN_BALANCE, true, - 1000, - ); - // Send XCM message from Relay Chain - Westend::execute_with(|| { - assert_ok!(::XcmPallet::send( - root_origin, - bx!(system_para_destination), - bx!(xcm), - )); - - Westend::assert_xcm_pallet_sent(); - }); - - // Receive XCM message in Assets Parachain - AssetHubWestend::execute_with(|| { - type RuntimeEvent = ::RuntimeEvent; - - AssetHubWestend::assert_dmp_queue_complete(Some(Weight::from_parts( - 1_019_445_000, - 200_000, - ))); - - assert_expected_events!( - AssetHubWestend, - vec![ - RuntimeEvent::Assets(pallet_assets::Event::ForceCreated { asset_id, owner }) => { - asset_id: *asset_id == ASSET_ID, - owner: *owner == asset_owner, - }, - ] - ); - - assert!(::Assets::asset_exists(ASSET_ID)); - }); + AssetHubWestendSender::get().into(), + Some(Weight::from_parts(1_019_445_000, 200_000)), + ) } /// Parachain should be able to send XCM paying its fee with sufficient asset @@ -78,6 +42,7 @@ fn send_xcm_from_para_to_system_para_paying_fee_with_assets_works() { ASSET_MIN_BALANCE, true, para_sovereign_account.clone(), + Some(Weight::from_parts(1_019_445_000, 200_000)), ASSET_MIN_BALANCE * 1000000000, ); diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/set_xcm_versions.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/set_xcm_versions.rs index 2720095aac00..2133d5e5fb7c 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/set_xcm_versions.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/set_xcm_versions.rs @@ -47,32 +47,22 @@ fn relay_sets_system_para_xcm_supported_version() { #[test] fn system_para_sets_relay_xcm_supported_version() { // Init test variables - let sudo_origin = ::RuntimeOrigin::root(); let parent_location = AssetHubWestend::parent_location(); - let system_para_destination: VersionedMultiLocation = - Westend::child_location_of(AssetHubWestend::para_id()).into(); - let call = ::RuntimeCall::PolkadotXcm(pallet_xcm::Call::< - ::Runtime, - >::force_xcm_version { - location: bx!(parent_location), - version: XCM_V3, - }) - .encode() - .into(); - let origin_kind = OriginKind::Superuser; - - let xcm = xcm_transact_unpaid_execution(call, origin_kind); - - // System Parachain sets supported version for Relay Chain throught it - Westend::execute_with(|| { - assert_ok!(::XcmPallet::send( - sudo_origin, - bx!(system_para_destination), - bx!(xcm), - )); + let force_xcm_version_call = + ::RuntimeCall::PolkadotXcm(pallet_xcm::Call::< + ::Runtime, + >::force_xcm_version { + location: bx!(parent_location), + version: XCM_V3, + }) + .encode() + .into(); - Westend::assert_xcm_pallet_sent(); - }); + // System Parachain sets supported version for Relay Chain through it + Westend::send_unpaid_transact_to_parachain_as_root( + AssetHubWestend::para_id(), + force_xcm_version_call, + ); // System Parachain receive the XCM message AssetHubWestend::execute_with(|| { diff --git a/cumulus/parachains/integration-tests/emulated/common/src/impls.rs b/cumulus/parachains/integration-tests/emulated/common/src/impls.rs index 5e11922d859c..bb4c9d102e98 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/impls.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/impls.rs @@ -54,7 +54,7 @@ pub use polkadot_runtime_parachains::{ inclusion::{AggregateMessageOrigin, UmpQueueId}, }; pub use xcm::{ - prelude::{OriginKind, Outcome, VersionedXcm, Weight}, + prelude::{MultiLocation, OriginKind, Outcome, VersionedXcm, Weight}, v3::Error, DoubleEncoded, }; @@ -80,21 +80,11 @@ impl From for LaneIdWrapper { type BridgeHubRococoRuntime = ::Runtime; type BridgeHubWococoRuntime = ::Runtime; -// TODO: uncomment when https://github.com/paritytech/polkadot-sdk/pull/1352 is merged -// type BridgeHubPolkadotRuntime = ::Runtime; -// type BridgeHubKusamaRuntime = ::Runtime; - pub type RococoWococoMessageHandler = BridgeHubMessageHandler; pub type WococoRococoMessageHandler = BridgeHubMessageHandler; -// TODO: uncomment when https://github.com/paritytech/polkadot-sdk/pull/1352 is merged -// pub type PolkadotKusamaMessageHandler -// = BridgeHubMessageHandler; -// pub type KusamaPolkadotMessageHandler -// = BridgeHubMessageHandler; - impl BridgeMessageHandler for BridgeHubMessageHandler where S: Config, @@ -356,6 +346,37 @@ macro_rules! impl_hrmp_channels_helpers_for_relay_chain { }; } +#[macro_export] +macro_rules! impl_send_transact_helpers_for_relay_chain { + ( $chain:ident ) => { + $crate::impls::paste::paste! { + impl $chain { + /// A root origin (as governance) sends `xcm::Transact` with `UnpaidExecution` and encoded `call` to child parachain. + pub fn send_unpaid_transact_to_parachain_as_root( + recipient: $crate::impls::ParaId, + call: $crate::impls::DoubleEncoded<()> + ) { + use $crate::impls::{bx, Chain, RelayChain}; + + ::execute_with(|| { + let root_origin = ::RuntimeOrigin::root(); + let destination: $crate::impls::MultiLocation = ::child_location_of(recipient); + let xcm = $crate::impls::xcm_transact_unpaid_execution(call, $crate::impls::OriginKind::Superuser); + + // Send XCM `Transact` + $crate::impls::assert_ok!(]>::XcmPallet::send( + root_origin, + bx!(destination.into()), + bx!(xcm), + )); + Self::assert_xcm_pallet_sent(); + }); + } + } + } + }; +} + #[macro_export] macro_rules! impl_accounts_helpers_for_parachain { ( $chain:ident ) => { @@ -616,53 +637,58 @@ macro_rules! impl_assets_helpers_for_parachain { min_balance: u128, is_sufficient: bool, asset_owner: $crate::impls::AccountId, + dmp_weight_threshold: Option<$crate::impls::Weight>, amount_to_mint: u128, ) { - use $crate::impls::{bx, Chain, RelayChain, Parachain, Inspect, TestExt}; - // Init values for Relay Chain - let root_origin = <$relay_chain as Chain>::RuntimeOrigin::root(); - let destination = <$relay_chain>::child_location_of(<$chain>::para_id()); - let xcm = Self::force_create_asset_xcm( - $crate::impls::OriginKind::Superuser, + use $crate::impls::Chain; + + // Force create asset + Self::force_create_asset_from_relay_as_root( id, - asset_owner.clone(), - is_sufficient, min_balance, + is_sufficient, + asset_owner.clone(), + dmp_weight_threshold ); - <$relay_chain>::execute_with(|| { - $crate::impls::assert_ok!(<$relay_chain as [<$relay_chain Pallet>]>::XcmPallet::send( - root_origin, - bx!(destination.into()), - bx!(xcm), - )); + // Mint asset for System Parachain's sender + let signed_origin = ::RuntimeOrigin::signed(asset_owner.clone()); + Self::mint_asset(signed_origin, id, asset_owner, amount_to_mint); + } - <$relay_chain>::assert_xcm_pallet_sent(); - }); + /// Relay Chain sends `Transact` instruction with `force_create_asset` to Parachain with `Assets` instance of `pallet_assets` . + pub fn force_create_asset_from_relay_as_root( + id: u32, + min_balance: u128, + is_sufficient: bool, + asset_owner: $crate::impls::AccountId, + dmp_weight_threshold: Option<$crate::impls::Weight>, + ) { + use $crate::impls::{Parachain, Inspect, TestExt}; - Self::execute_with(|| { - Self::assert_dmp_queue_complete(Some($crate::impls::Weight::from_parts(1_019_445_000, 200_000))); + <$relay_chain>::send_unpaid_transact_to_parachain_as_root( + Self::para_id(), + Self::force_create_asset_call(id, asset_owner.clone(), is_sufficient, min_balance), + ); + // Receive XCM message in Assets Parachain + Self::execute_with(|| { type RuntimeEvent = <$chain as $crate::impls::Chain>::RuntimeEvent; + Self::assert_dmp_queue_complete(dmp_weight_threshold); + $crate::impls::assert_expected_events!( Self, vec![ - // Asset has been created RuntimeEvent::Assets($crate::impls::pallet_assets::Event::ForceCreated { asset_id, owner }) => { asset_id: *asset_id == id, - owner: *owner == asset_owner.clone(), + owner: *owner == asset_owner, }, ] ); assert!(]>::Assets::asset_exists(id.into())); }); - - let signed_origin = ::RuntimeOrigin::signed(asset_owner.clone()); - - // Mint asset for System Parachain's sender - Self::mint_asset(signed_origin, id, asset_owner, amount_to_mint); } } } diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index 068f0238f497..c2e065ccadcd 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -250,30 +250,22 @@ decl_test_bridges! { target = BridgeHubRococo, handler = WococoRococoMessageHandler } - // TODO: uncomment when https://github.com/paritytech/polkadot-sdk/pull/1352 is merged - // pub struct PolkadotKusamaMockBridge { - // source = BridgeHubPolkadot, - // target = BridgeHubKusama, - // handler = PolkadotKusamaMessageHandler - // }, - // pub struct KusamaPolkadotMockBridge { - // source = BridgeHubKusama, - // target = BridgeHubPolkadot, - // handler = KusamaPolkadotMessageHandler - // } } // Westend implementation impl_accounts_helpers_for_relay_chain!(Westend); impl_assert_events_helpers_for_relay_chain!(Westend); +impl_send_transact_helpers_for_relay_chain!(Westend); // Rococo implementation impl_accounts_helpers_for_relay_chain!(Rococo); impl_assert_events_helpers_for_relay_chain!(Rococo); +impl_send_transact_helpers_for_relay_chain!(Rococo); // Wococo implementation impl_accounts_helpers_for_relay_chain!(Wococo); impl_assert_events_helpers_for_relay_chain!(Wococo); +impl_send_transact_helpers_for_relay_chain!(Wococo); // AssetHubWestend implementation impl_accounts_helpers_for_parachain!(AssetHubWestend);