From 081088a1273543286be750eb54ad19b65f2bdc6f Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 20 Feb 2025 23:02:21 +0100 Subject: [PATCH 1/8] Generic test-case for `authorize_upgrade` --- .../parachains/runtimes/test-utils/src/lib.rs | 83 +++++++++++++++++-- .../runtimes/test-utils/src/test_cases.rs | 36 +++++++- 2 files changed, 109 insertions(+), 10 deletions(-) diff --git a/cumulus/parachains/runtimes/test-utils/src/lib.rs b/cumulus/parachains/runtimes/test-utils/src/lib.rs index b46a68312aa17..ab24875ee221a 100644 --- a/cumulus/parachains/runtimes/test-utils/src/lib.rs +++ b/cumulus/parachains/runtimes/test-utils/src/lib.rs @@ -34,7 +34,10 @@ use polkadot_parachain_primitives::primitives::{ }; use sp_consensus_aura::{SlotDuration, AURA_ENGINE_ID}; use sp_core::{Encode, U256}; -use sp_runtime::{traits::Header, BuildStorage, Digest, DigestItem, SaturatedConversion}; +use sp_runtime::{ + traits::{Dispatchable, Header}, + BuildStorage, Digest, DigestItem, DispatchError, Either, SaturatedConversion, +}; use xcm::{ latest::{Asset, Location, XcmContext, XcmHash}, prelude::*, @@ -464,6 +467,55 @@ impl< ) } + pub fn execute_as_governance_call( + call: Call, + governance_origin: GovernanceOrigin, + ) -> Result<(), Either> + { + // execute xcm as governance would send + let execute_xcm = |call: Call, governance_location, descend_origin| { + // prepare xcm + let xcm = if let Some(descend_origin) = descend_origin { + Xcm::builder_unsafe().descend_origin(descend_origin) + } else { + Xcm::builder_unsafe() + } + .unpaid_execution(Unlimited, None) + .transact(OriginKind::Superuser, None, call.encode()) + .expect_transact_status(MaybeErrorCode::Success) + .build(); + + let xcm_max_weight = Self::xcm_max_weight_for_location(&governance_location); + let mut hash = xcm.using_encoded(sp_io::hashing::blake2_256); + + <::XcmExecutor>::prepare_and_execute( + governance_location, + xcm, + &mut hash, + xcm_max_weight, + Weight::zero(), + ) + }; + + match governance_origin { + GovernanceOrigin::Location(location) => { + execute_xcm(call, location, None) + .ensure_complete() + .map_err(Either::Right) + }, + GovernanceOrigin::LocationAndDescendOrigin(location, descend_origin) => { + execute_xcm(call, location, Some(descend_origin)) + .ensure_complete() + .map_err(Either::Right) + }, + GovernanceOrigin::Origin(origin) => { + call.dispatch(origin) + .map(|_| ()) + .map_err(|e| Either::Left(e.error)) + }, + } + } + pub fn execute_as_origin( (origin, origin_kind): (Location, OriginKind), call: Call, @@ -484,23 +536,27 @@ impl< ExpectTransactStatus(MaybeErrorCode::Success), ]); let xcm = Xcm(instructions); + let xcm_max_weight = Self::xcm_max_weight_for_location(&origin); // execute xcm as parent origin let mut hash = xcm.using_encoded(sp_io::hashing::blake2_256); <::XcmExecutor>::prepare_and_execute( - origin.clone(), + origin, xcm, &mut hash, - Self::xcm_max_weight(if origin == Location::parent() { - XcmReceivedFrom::Parent - } else { - XcmReceivedFrom::Sibling - }), + xcm_max_weight, Weight::zero(), ) } } +/// Enum representing governance origin/location. +pub enum GovernanceOrigin { + Location(Location), + LocationAndDescendOrigin(Location, InteriorLocation), + Origin(RuntimeOrigin), +} + pub enum XcmReceivedFrom { Parent, Sibling, @@ -515,6 +571,14 @@ impl ParachainSystem::ReservedXcmpWeight::get(), } } + + pub fn xcm_max_weight_for_location(location: &Location) -> Weight { + Self::xcm_max_weight(if location == &Location::parent() { + XcmReceivedFrom::Parent + } else { + XcmReceivedFrom::Sibling + }) + } } impl @@ -555,8 +619,9 @@ impl< .into_iter() .filter_map(|e| unwrap_xcmp_queue_event(e.event.encode())) .find_map(|e| match e { - cumulus_pallet_xcmp_queue::Event::XcmpMessageSent { message_hash } => - Some(message_hash), + cumulus_pallet_xcmp_queue::Event::XcmpMessageSent { message_hash } => { + Some(message_hash) + }, _ => None, }) } diff --git a/cumulus/parachains/runtimes/test-utils/src/test_cases.rs b/cumulus/parachains/runtimes/test-utils/src/test_cases.rs index 8a7f8d6303c86..528f12bf42a12 100644 --- a/cumulus/parachains/runtimes/test-utils/src/test_cases.rs +++ b/cumulus/parachains/runtimes/test-utils/src/test_cases.rs @@ -16,7 +16,10 @@ //! Module contains predefined test-case scenarios for `Runtime` with common functionality. -use crate::{AccountIdOf, CollatorSessionKeys, ExtBuilder, ValidatorIdOf}; +use crate::{ + AccountIdOf, BasicParachainRuntime, CollatorSessionKeys, ExtBuilder, GovernanceOrigin, + ValidatorIdOf, +}; use codec::Encode; use frame_support::{ assert_ok, @@ -24,6 +27,8 @@ use frame_support::{ }; use parachains_common::AccountId; use sp_runtime::traits::{Block as BlockT, StaticLookup}; +use sp_runtime::{DispatchError, Either}; +use xcm::prelude::XcmError; use xcm_runtime_apis::fees::{ runtime_decl_for_xcm_payment_api::XcmPaymentApiV1, Error as XcmPaymentApiError, }; @@ -193,3 +198,32 @@ where assert_eq!(execution_fees, Err(XcmPaymentApiError::AssetNotFound)); }); } + +/// Generic test case for Cumulus-based parachain that verifies if runtime can process `frame_system::Call::authorize_upgrade` from governance system. +pub fn can_governance_authorize_upgrade( + governance_origin: GovernanceOrigin, +) -> Result<(), Either> +where + Runtime: BasicParachainRuntime + + frame_system::Config, +{ + ExtBuilder::::default().build().execute_with(|| { + // check before + assert!(frame_system::Pallet::::authorized_upgrade().is_none()); + + // execute call as governance does + let code_hash = Runtime::Hash::default(); + let authorize_upgrade_call: ::RuntimeCall = + frame_system::Call::::authorize_upgrade { code_hash }.into(); + RuntimeHelper::::execute_as_governance_call( + authorize_upgrade_call, + governance_origin, + )?; + + // check after + match frame_system::Pallet::::authorized_upgrade() { + None => Err(Either::Left(frame_system::Error::::NothingAuthorized.into())), + Some(_) => Ok(()), + } + }) +} From e006cfc0d815dd2af2414733fb23ae716da56b18 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 20 Feb 2025 23:02:34 +0100 Subject: [PATCH 2/8] Random typo --- .../tests/assets/asset-hub-rococo/src/tests/treasury.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/treasury.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/treasury.rs index 8648c8ce9311d..1452ba93a76f2 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/treasury.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/treasury.rs @@ -83,7 +83,7 @@ fn spend_roc_on_asset_hub() { })), }); - // Dispatched from Root to `despatch_as` `Signed(treasury_account)`. + // Dispatched from Root to `dispatch_as` `Signed(treasury_account)`. assert_ok!(teleport_call.dispatch(root)); assert_expected_events!( From 5002a12635c689f3c8dfb5b63d014462f54ccd52 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Fri, 21 Feb 2025 08:56:35 +0100 Subject: [PATCH 3/8] Add `can_governance_authorize_upgrade` test for all system chains --- .../asset-hub-westend/src/xcm_config.rs | 9 +-- .../assets/asset-hub-westend/tests/tests.rs | 56 ++++++++++++++++++- .../bridge-hub-westend/src/xcm_config.rs | 1 + .../bridge-hub-westend/tests/tests.rs | 55 +++++++++++++++++- .../collectives-westend/src/xcm_config.rs | 4 +- .../collectives-westend/tests/tests.rs | 55 ++++++++++++++++++ .../coretime-westend/src/xcm_config.rs | 7 ++- .../coretime/coretime-westend/tests/tests.rs | 55 ++++++++++++++++++ .../people/people-westend/src/xcm_config.rs | 7 ++- .../people/people-westend/tests/tests.rs | 55 ++++++++++++++++++ 10 files changed, 290 insertions(+), 14 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs index e144f36dd6b88..46c4e4d0b28e4 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs @@ -44,6 +44,7 @@ use polkadot_parachain_primitives::primitives::Sibling; use polkadot_runtime_common::xcm_sender::ExponentialPrice; use snowbridge_router_primitives::inbound::EthereumLocationsConverterFor; use sp_runtime::traits::{AccountIdConversion, ConvertInto, TryConvertInto}; +use westend_runtime_constants::system_parachain::COLLECTIVES_ID; use xcm::latest::{prelude::*, ROCOCO_GENESIS_HASH, WESTEND_GENESIS_HASH}; use xcm_builder::{ AccountId32Aliases, AliasChildLocation, AllowExplicitUnpaidExecutionFrom, @@ -265,9 +266,9 @@ impl Contains for FellowshipEntities { fn contains(location: &Location) -> bool { matches!( location.unpack(), - (1, [Parachain(1001), Plurality { id: BodyId::Technical, .. }]) | - (1, [Parachain(1001), PalletInstance(64)]) | - (1, [Parachain(1001), PalletInstance(65)]) + (1, [Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }]) | + (1, [Parachain(COLLECTIVES_ID), PalletInstance(64)]) | + (1, [Parachain(COLLECTIVES_ID), PalletInstance(65)]) ) } } @@ -275,7 +276,7 @@ impl Contains for FellowshipEntities { pub struct AmbassadorEntities; impl Contains for AmbassadorEntities { fn contains(location: &Location) -> bool { - matches!(location.unpack(), (1, [Parachain(1001), PalletInstance(74)])) + matches!(location.unpack(), (1, [Parachain(COLLECTIVES_ID), PalletInstance(74)])) } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs index 24b6d83ffae41..3c3218e12c396 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs @@ -17,6 +17,7 @@ //! Tests for the Westmint (Westend Assets Hub) chain. +use asset_hub_westend_runtime::xcm_config::GovernanceLocation; use asset_hub_westend_runtime::{ xcm_config, xcm_config::{ @@ -37,7 +38,7 @@ use asset_test_utils::{ use codec::{Decode, Encode}; use cumulus_primitives_utility::ChargeWeightInFungibles; use frame_support::{ - assert_noop, assert_ok, + assert_err, assert_noop, assert_ok, traits::{ fungible::{Inspect, Mutate}, fungibles::{ @@ -47,9 +48,11 @@ use frame_support::{ weights::{Weight, WeightToFee as WeightToFeeT}, }; use parachains_common::{AccountId, AssetIdForTrustBackedAssets, AuraId, Balance}; +use parachains_runtimes_test_utils::GovernanceOrigin; use sp_consensus_aura::SlotDuration; use sp_core::crypto::Ss58Codec; use sp_runtime::traits::MaybeEquivalence; +use sp_runtime::Either; use std::{convert::Into, ops::Mul}; use testnet_parachains_constants::westend::{consensus::*, currency::UNITS, fee::WeightToFee}; use xcm::latest::{ @@ -1512,3 +1515,54 @@ fn xcm_payment_api_works() { Block, >(); } + +#[test] +fn governance_authorize_upgrade_works() { + use westend_runtime_constants::system_parachain::{ASSET_HUB_ID, COLLECTIVES_ID}; + + // no - random para + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(12334)))), + Either::Right(XcmError::Barrier) + ); + // no - AssetHub + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(ASSET_HUB_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(COLLECTIVES_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives Voice of Fellows plurality + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::LocationAndDescendOrigin( + Location::new(1, Parachain(COLLECTIVES_ID)), + Plurality { id: BodyId::Technical, part: BodyPart::Voice }.into() + )), + Either::Right(XcmError::BadOrigin) + ); + + // ok - relaychain + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::parent()))); + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(GovernanceLocation::get()))); +} diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs index 3e844f9841654..4b6132c5be6c6 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs @@ -66,6 +66,7 @@ parameter_types! { pub const MaxAssetsIntoHolding: u32 = 64; pub TreasuryAccount: AccountId = TREASURY_PALLET_ID.into_account_truncating(); pub RelayTreasuryLocation: Location = (Parent, PalletInstance(westend_runtime_constants::TREASURY_PALLET_ID)).into(); + pub const GovernanceLocation: Location = Location::parent(); } /// Type for specifying how a `Location` can be converted into an `AccountId`. This is used diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs index b414338aab2d1..acc9fa49e4eed 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs @@ -24,6 +24,7 @@ use bridge_hub_test_utils::{ test_cases::{from_parachain, run_test}, SlotDurations, }; +use bridge_hub_westend_runtime::xcm_config::GovernanceLocation; use bridge_hub_westend_runtime::{ bridge_common_config, bridge_to_rococo_config, bridge_to_rococo_config::RococoGlobalConsensusNetwork, @@ -48,12 +49,13 @@ use frame_support::{ }, }; use parachains_common::{AccountId, AuraId, Balance}; +use parachains_runtimes_test_utils::GovernanceOrigin; use sp_consensus_aura::SlotDuration; use sp_core::crypto::Ss58Codec; use sp_keyring::Sr25519Keyring::{Alice, Bob}; use sp_runtime::{ generic::{Era, SignedPayload}, - AccountId32, Perbill, + AccountId32, Either, Perbill, }; use testnet_parachains_constants::westend::{consensus::*, fee::WeightToFee}; use xcm::latest::{prelude::*, WESTEND_GENESIS_HASH}; @@ -624,3 +626,54 @@ pub fn bridge_rewards_works() { }, ); } + +#[test] +fn governance_authorize_upgrade_works() { + use westend_runtime_constants::system_parachain::{ASSET_HUB_ID, COLLECTIVES_ID}; + + // no - random para + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(12334)))), + Either::Right(XcmError::Barrier) + ); + // no - AssetHub + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(ASSET_HUB_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(COLLECTIVES_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives Voice of Fellows plurality + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::LocationAndDescendOrigin( + Location::new(1, Parachain(COLLECTIVES_ID)), + Plurality { id: BodyId::Technical, part: BodyPart::Voice }.into() + )), + Either::Right(XcmError::Barrier) + ); + + // ok - relaychain + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::parent()))); + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(GovernanceLocation::get()))); +} diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/xcm_config.rs index 71208b7ce875f..5d33d006c293c 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/xcm_config.rs @@ -31,7 +31,7 @@ use parachains_common::xcm_config::{ }; use polkadot_parachain_primitives::primitives::Sibling; use polkadot_runtime_common::xcm_sender::ExponentialPrice; -use westend_runtime_constants::xcm as xcm_constants; +use westend_runtime_constants::{system_parachain::ASSET_HUB_ID, xcm as xcm_constants}; use xcm::latest::{prelude::*, WESTEND_GENESIS_HASH}; use xcm_builder::{ AccountId32Aliases, AliasChildLocation, AliasOriginRootUsingFilter, @@ -58,7 +58,7 @@ parameter_types! { pub CheckingAccount: AccountId = PolkadotXcm::check_account(); pub const GovernanceLocation: Location = Location::parent(); pub const FellowshipAdminBodyId: BodyId = BodyId::Index(xcm_constants::body::FELLOWSHIP_ADMIN_INDEX); - pub AssetHub: Location = (Parent, Parachain(1000)).into(); + pub AssetHub: Location = (Parent, Parachain(ASSET_HUB_ID)).into(); pub const TreasurerBodyId: BodyId = BodyId::Treasury; pub AssetHubUsdtId: AssetId = (PalletInstance(50), GeneralIndex(1984)).into(); pub UsdtAssetHub: LocatableAssetId = LocatableAssetId { diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs index f99bc39fc8344..6528ceeb9368c 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs @@ -19,8 +19,12 @@ use collectives_westend_runtime::{ xcm_config::LocationToAccountId, Block, Runtime, RuntimeCall, RuntimeOrigin, }; +use collectives_westend_runtime::xcm_config::GovernanceLocation; +use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; +use parachains_runtimes_test_utils::GovernanceOrigin; use sp_core::crypto::Ss58Codec; +use sp_runtime::Either; use xcm::latest::prelude::*; use xcm_runtime_apis::conversions::LocationToAccountHelper; @@ -144,3 +148,54 @@ fn xcm_payment_api_works() { Block, >(); } + +#[test] +fn governance_authorize_upgrade_works() { + use westend_runtime_constants::system_parachain::{ASSET_HUB_ID, COLLECTIVES_ID}; + + // no - random para + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(12334)))), + Either::Right(XcmError::Barrier) + ); + // no - AssetHub + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(ASSET_HUB_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(COLLECTIVES_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives Voice of Fellows plurality + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::LocationAndDescendOrigin( + Location::new(1, Parachain(COLLECTIVES_ID)), + Plurality { id: BodyId::Technical, part: BodyPart::Voice }.into() + )), + Either::Right(XcmError::Barrier) + ); + + // ok - relaychain + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::parent()))); + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(GovernanceLocation::get()))); +} diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs index bff798259dfeb..ce9ce094328be 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs @@ -37,6 +37,7 @@ use parachains_common::{ use polkadot_parachain_primitives::primitives::Sibling; use polkadot_runtime_common::xcm_sender::ExponentialPrice; use sp_runtime::traits::AccountIdConversion; +use westend_runtime_constants::system_parachain::{ASSET_HUB_ID, COLLECTIVES_ID}; use xcm::latest::{prelude::*, WESTEND_GENESIS_HASH}; use xcm_builder::{ AccountId32Aliases, AliasChildLocation, AliasOriginRootUsingFilter, @@ -55,7 +56,7 @@ use xcm_executor::XcmExecutor; parameter_types! { pub const RootLocation: Location = Location::here(); pub const TokenRelayLocation: Location = Location::parent(); - pub AssetHubLocation: Location = Location::new(1, [Parachain(1000)]); + pub AssetHubLocation: Location = Location::new(1, [Parachain(ASSET_HUB_ID)]); pub const RelayNetwork: Option = Some(NetworkId::ByGenesis(WESTEND_GENESIS_HASH)); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); pub UniversalLocation: InteriorLocation = @@ -64,7 +65,7 @@ parameter_types! { PalletInstance(::index() as u8).into(); pub const MaxInstructions: u32 = 100; pub const MaxAssetsIntoHolding: u32 = 64; - pub FellowshipLocation: Location = Location::new(1, Parachain(1001)); + pub FellowshipLocation: Location = Location::new(1, Parachain(COLLECTIVES_ID)); pub const GovernanceLocation: Location = Location::parent(); } @@ -148,7 +149,7 @@ impl Contains for ParentOrParentsPlurality { pub struct FellowsPlurality; impl Contains for FellowsPlurality { fn contains(location: &Location) -> bool { - matches!(location.unpack(), (1, [Parachain(1001), Plurality { id: BodyId::Technical, .. }])) + matches!(location.unpack(), (1, [Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }])) } } diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs index f92486f369614..0e25c1d3153f0 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs @@ -16,11 +16,15 @@ #![cfg(test)] +use coretime_westend_runtime::xcm_config::GovernanceLocation; use coretime_westend_runtime::{ xcm_config::LocationToAccountId, Block, Runtime, RuntimeCall, RuntimeOrigin, }; +use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; +use parachains_runtimes_test_utils::GovernanceOrigin; use sp_core::crypto::Ss58Codec; +use sp_runtime::Either; use xcm::latest::prelude::*; use xcm_runtime_apis::conversions::LocationToAccountHelper; @@ -144,3 +148,54 @@ fn xcm_payment_api_works() { Block, >(); } + +#[test] +fn governance_authorize_upgrade_works() { + use westend_runtime_constants::system_parachain::{ASSET_HUB_ID, COLLECTIVES_ID}; + + // no - random para + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(12334)))), + Either::Right(XcmError::Barrier) + ); + // no - AssetHub + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(ASSET_HUB_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(COLLECTIVES_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives Voice of Fellows plurality + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::LocationAndDescendOrigin( + Location::new(1, Parachain(COLLECTIVES_ID)), + Plurality { id: BodyId::Technical, part: BodyPart::Voice }.into() + )), + Either::Right(XcmError::BadOrigin) + ); + + // ok - relaychain + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::parent()))); + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(GovernanceLocation::get()))); +} diff --git a/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs index bd319e3ac3b48..516c18c8786c3 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs @@ -34,6 +34,7 @@ use parachains_common::{ }; use polkadot_parachain_primitives::primitives::Sibling; use sp_runtime::traits::AccountIdConversion; +use westend_runtime_constants::system_parachain::{ASSET_HUB_ID, COLLECTIVES_ID}; use xcm::latest::{prelude::*, WESTEND_GENESIS_HASH}; use xcm_builder::{ AccountId32Aliases, AliasChildLocation, AliasOriginRootUsingFilter, @@ -52,14 +53,14 @@ use xcm_executor::XcmExecutor; parameter_types! { pub const RootLocation: Location = Location::here(); pub const RelayLocation: Location = Location::parent(); - pub AssetHubLocation: Location = Location::new(1, [Parachain(1000)]); + pub AssetHubLocation: Location = Location::new(1, [Parachain(ASSET_HUB_ID)]); pub const RelayNetwork: Option = Some(NetworkId::ByGenesis(WESTEND_GENESIS_HASH)); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); pub UniversalLocation: InteriorLocation = [GlobalConsensus(RelayNetwork::get().unwrap()), Parachain(ParachainInfo::parachain_id().into())].into(); pub const MaxInstructions: u32 = 100; pub const MaxAssetsIntoHolding: u32 = 64; - pub FellowshipLocation: Location = Location::new(1, Parachain(1001)); + pub FellowshipLocation: Location = Location::new(1, Parachain(COLLECTIVES_ID)); pub const GovernanceLocation: Location = Location::parent(); /// The asset ID for the asset that we use to pay for message delivery fees. Just WND. pub FeeAssetId: AssetId = AssetId(RelayLocation::get()); @@ -156,7 +157,7 @@ impl Contains for ParentOrParentsPlurality { pub struct FellowsPlurality; impl Contains for FellowsPlurality { fn contains(location: &Location) -> bool { - matches!(location.unpack(), (1, [Parachain(1001), Plurality { id: BodyId::Technical, .. }])) + matches!(location.unpack(), (1, [Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }])) } } diff --git a/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs b/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs index 3df1558b6f9d8..b61fb713088f9 100644 --- a/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs @@ -16,11 +16,15 @@ #![cfg(test)] +use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; +use parachains_runtimes_test_utils::GovernanceOrigin; +use people_westend_runtime::xcm_config::GovernanceLocation; use people_westend_runtime::{ xcm_config::LocationToAccountId, Block, Runtime, RuntimeCall, RuntimeOrigin, }; use sp_core::crypto::Ss58Codec; +use sp_runtime::Either; use xcm::latest::prelude::*; use xcm_runtime_apis::conversions::LocationToAccountHelper; @@ -144,3 +148,54 @@ fn xcm_payment_api_works() { Block, >(); } + +#[test] +fn governance_authorize_upgrade_works() { + use westend_runtime_constants::system_parachain::{ASSET_HUB_ID, COLLECTIVES_ID}; + + // no - random para + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(12334)))), + Either::Right(XcmError::Barrier) + ); + // no - AssetHub + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(ASSET_HUB_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::new(1, Parachain(COLLECTIVES_ID)))), + Either::Right(XcmError::Barrier) + ); + // no - Collectives Voice of Fellows plurality + assert_err!( + parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::LocationAndDescendOrigin( + Location::new(1, Parachain(COLLECTIVES_ID)), + Plurality { id: BodyId::Technical, part: BodyPart::Voice }.into() + )), + Either::Right(XcmError::BadOrigin) + ); + + // ok - relaychain + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(Location::parent()))); + assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< + Runtime, + RuntimeOrigin, + >(GovernanceOrigin::Location(GovernanceLocation::get()))); +} From 7a739e869bd8bb0f3f0c882b9ae403d599d52eec Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Wed, 26 Feb 2025 09:41:29 +0100 Subject: [PATCH 4/8] Change tests using `execute_as_governance` to `execute_as_governance_call` --- .../assets/asset-hub-rococo/tests/tests.rs | 16 ++++++----- .../assets/asset-hub-westend/tests/tests.rs | 16 ++++++----- .../bridge-hub-rococo/tests/tests.rs | 25 ++++++++--------- .../bridge-hub-westend/tests/tests.rs | 26 ++++++++---------- .../test-utils/src/test_cases/mod.rs | 27 +++++++------------ .../collectives-westend/tests/tests.rs | 3 +-- .../coretime/coretime-westend/tests/tests.rs | 3 +-- .../people/people-westend/tests/tests.rs | 3 +-- .../parachains/runtimes/test-utils/src/lib.rs | 4 +++ .../runtimes/test-utils/src/test_cases.rs | 17 +++++------- 10 files changed, 66 insertions(+), 74 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs index 144934ecd4abd..1bb8d254f9fd2 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs @@ -20,7 +20,7 @@ use asset_hub_rococo_runtime::{ xcm_config, xcm_config::{ - bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, CheckingAccount, + bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, CheckingAccount, GovernanceLocation, ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, LocationToAccountId, StakingPot, TokenLocation, TrustBackedAssetsPalletLocation, XcmConfig, }, @@ -32,13 +32,13 @@ use asset_hub_rococo_runtime::{ }; use asset_test_utils::{ test_cases_over_bridge::TestBridgingConfig, CollatorSessionKey, CollatorSessionKeys, - ExtBuilder, SlotDurations, + ExtBuilder, GovernanceOrigin, SlotDurations, }; use codec::{Decode, Encode}; use core::ops::Mul; use cumulus_primitives_utility::ChargeWeightInFungibles; use frame_support::{ - assert_noop, assert_ok, + assert_noop, assert_ok, parameter_types, traits::{ fungible::{Inspect, Mutate}, fungibles::{ @@ -61,6 +61,10 @@ use xcm_runtime_apis::conversions::LocationToAccountHelper; const ALICE: [u8; 32] = [1u8; 32]; const SOME_ASSET_ADMIN: [u8; 32] = [5u8; 32]; +parameter_types! { + pub Governance: GovernanceOrigin = GovernanceOrigin::Location(GovernanceLocation::get()); +} + type AssetIdForTrustBackedAssetsConvert = assets_common::AssetIdForTrustBackedAssetsConvert; @@ -1335,7 +1339,7 @@ fn change_xcm_bridge_hub_router_byte_fee_by_governance_works() { >( collator_session_keys(), 1000, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || { ( bridging::XcmBridgeHubRouterByteFee::key().to_vec(), @@ -1361,7 +1365,7 @@ fn change_xcm_bridge_hub_router_base_fee_by_governance_works() { >( collator_session_keys(), 1000, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || { log::error!( target: "bridges::estimate", @@ -1393,7 +1397,7 @@ fn change_xcm_bridge_hub_ethereum_base_fee_by_governance_works() { >( collator_session_keys(), 1000, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || { log::error!( target: "bridges::estimate", diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs index 3c3218e12c396..dcf41ef58ec06 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs @@ -17,11 +17,10 @@ //! Tests for the Westmint (Westend Assets Hub) chain. -use asset_hub_westend_runtime::xcm_config::GovernanceLocation; use asset_hub_westend_runtime::{ xcm_config, xcm_config::{ - bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, CheckingAccount, + bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, GovernanceLocation, CheckingAccount, ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, LocationToAccountId, StakingPot, TrustBackedAssetsPalletLocation, WestendLocation, XcmConfig, }, @@ -33,12 +32,12 @@ use asset_hub_westend_runtime::{ pub use asset_hub_westend_runtime::{AssetConversion, AssetDeposit, CollatorSelection, System}; use asset_test_utils::{ test_cases_over_bridge::TestBridgingConfig, CollatorSessionKey, CollatorSessionKeys, - ExtBuilder, SlotDurations, + ExtBuilder, GovernanceOrigin, SlotDurations, }; use codec::{Decode, Encode}; use cumulus_primitives_utility::ChargeWeightInFungibles; use frame_support::{ - assert_err, assert_noop, assert_ok, + assert_err, assert_noop, assert_ok, parameter_types, traits::{ fungible::{Inspect, Mutate}, fungibles::{ @@ -48,7 +47,6 @@ use frame_support::{ weights::{Weight, WeightToFee as WeightToFeeT}, }; use parachains_common::{AccountId, AssetIdForTrustBackedAssets, AuraId, Balance}; -use parachains_runtimes_test_utils::GovernanceOrigin; use sp_consensus_aura::SlotDuration; use sp_core::crypto::Ss58Codec; use sp_runtime::traits::MaybeEquivalence; @@ -66,6 +64,10 @@ use xcm_runtime_apis::conversions::LocationToAccountHelper; const ALICE: [u8; 32] = [1u8; 32]; const SOME_ASSET_ADMIN: [u8; 32] = [5u8; 32]; +parameter_types! { + pub Governance: GovernanceOrigin = GovernanceOrigin::Location(GovernanceLocation::get()); +} + type AssetIdForTrustBackedAssetsConvert = assets_common::AssetIdForTrustBackedAssetsConvert; @@ -1312,7 +1314,7 @@ fn change_xcm_bridge_hub_router_byte_fee_by_governance_works() { >( collator_session_keys(), 1000, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || { ( bridging::XcmBridgeHubRouterByteFee::key().to_vec(), @@ -1338,7 +1340,7 @@ fn change_xcm_bridge_hub_router_base_fee_by_governance_works() { >( collator_session_keys(), 1000, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || { log::error!( target: "bridges::estimate", diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs index 23b85388b0cba..c39cf2208910a 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs @@ -24,7 +24,7 @@ use bridge_hub_rococo_runtime::{ ExistentialDeposit, ParachainSystem, PolkadotXcm, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, SessionKeys, TransactionPayment, TxExtension, UncheckedExtrinsic, }; -use bridge_hub_test_utils::SlotDurations; +use bridge_hub_test_utils::{GovernanceOrigin, SlotDurations}; use codec::{Decode, Encode}; use frame_support::{dispatch::GetDispatchInfo, parameter_types, traits::ConstU8}; use parachains_common::{AccountId, AuraId, Balance}; @@ -42,6 +42,7 @@ use xcm_runtime_apis::conversions::LocationToAccountHelper; parameter_types! { pub CheckingAccount: AccountId = PolkadotXcm::check_account(); + pub Governance: GovernanceOrigin = GovernanceOrigin::Location(Location::parent()); } fn construct_extrinsic( @@ -164,7 +165,7 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::initialize_bridge_by_governance_works::< Runtime, BridgeGrandpaWestendInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) } #[test] @@ -173,7 +174,7 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::change_bridge_grandpa_pallet_mode_by_governance_works::< Runtime, BridgeGrandpaWestendInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) } #[test] @@ -182,7 +183,7 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::change_bridge_parachains_pallet_mode_by_governance_works::< Runtime, BridgeParachainWestendInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) } #[test] @@ -191,7 +192,7 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::change_bridge_messages_pallet_mode_by_governance_works::< Runtime, WithBridgeHubWestendMessagesInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) } #[test] @@ -203,7 +204,7 @@ mod bridge_hub_westend_tests { >( collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || (EthereumGatewayAddress::key().to_vec(), EthereumGatewayAddress::get()), |_| [1; 20].into(), ) @@ -219,7 +220,7 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::set_storage_keys_by_governance_works::( collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), vec![ (snowbridge_pallet_outbound_queue::Nonce::::hashed_key_for::( channel_id_one, @@ -284,7 +285,7 @@ mod bridge_hub_westend_tests { >( collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || (DeliveryRewardInBalance::key().to_vec(), DeliveryRewardInBalance::get()), |old_value| old_value.checked_mul(2).unwrap(), ) @@ -536,7 +537,7 @@ mod bridge_hub_bulletin_tests { bridge_hub_test_utils::test_cases::initialize_bridge_by_governance_works::< Runtime, BridgeGrandpaRococoBulletinInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) } #[test] @@ -545,7 +546,7 @@ mod bridge_hub_bulletin_tests { bridge_hub_test_utils::test_cases::change_bridge_grandpa_pallet_mode_by_governance_works::< Runtime, BridgeGrandpaRococoBulletinInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) } #[test] @@ -554,7 +555,7 @@ mod bridge_hub_bulletin_tests { bridge_hub_test_utils::test_cases::change_bridge_messages_pallet_mode_by_governance_works::< Runtime, WithRococoBulletinMessagesInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) } #[test] @@ -716,7 +717,7 @@ fn change_required_stake_by_governance_works() { >( collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || { ( bridge_common_config::RequiredStakeForStakeAndSlash::key().to_vec(), diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs index acc9fa49e4eed..15bfb558d64ba 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs @@ -22,13 +22,12 @@ use bp_relayers::{PayRewardFromAccount, RewardsAccountOwner, RewardsAccountParam use bridge_common_config::{BridgeRelayersInstance, BridgeReward, RequiredStakeForStakeAndSlash}; use bridge_hub_test_utils::{ test_cases::{from_parachain, run_test}, - SlotDurations, + GovernanceOrigin, SlotDurations, }; -use bridge_hub_westend_runtime::xcm_config::GovernanceLocation; use bridge_hub_westend_runtime::{ bridge_common_config, bridge_to_rococo_config, bridge_to_rococo_config::RococoGlobalConsensusNetwork, - xcm_config::{LocationToAccountId, RelayNetwork, WestendLocation, XcmConfig}, + xcm_config::{GovernanceLocation, LocationToAccountId, RelayNetwork, WestendLocation, XcmConfig}, AllPalletsWithoutSystem, Balances, Block, BridgeRejectObsoleteHeadersAndMessages, BridgeRelayers, Executive, ExistentialDeposit, ParachainSystem, PolkadotXcm, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, SessionKeys, TransactionPayment, TxExtension, @@ -49,7 +48,6 @@ use frame_support::{ }, }; use parachains_common::{AccountId, AuraId, Balance}; -use parachains_runtimes_test_utils::GovernanceOrigin; use sp_consensus_aura::SlotDuration; use sp_core::crypto::Ss58Codec; use sp_keyring::Sr25519Keyring::{Alice, Bob}; @@ -72,6 +70,8 @@ parameter_types! { pub SiblingParachainLocation: Location = Location::new(1, [Parachain(SIBLING_PARACHAIN_ID)]); pub SiblingSystemParachainLocation: Location = Location::new(1, [Parachain(SIBLING_SYSTEM_PARACHAIN_ID)]); pub BridgedUniversalLocation: InteriorLocation = [GlobalConsensus(RococoGlobalConsensusNetwork::get()), Parachain(BRIDGED_LOCATION_PARACHAIN_ID)].into(); + pub CheckingAccount: AccountId = PolkadotXcm::check_account(); + pub Governance: GovernanceOrigin = GovernanceOrigin::Location(GovernanceLocation::get()); } // Runtime from tests PoV @@ -84,10 +84,6 @@ type RuntimeTestsAdapter = from_parachain::WithRemoteParachainHelperAdapter< BridgeRelayersInstance, >; -parameter_types! { - pub CheckingAccount: AccountId = PolkadotXcm::check_account(); -} - fn construct_extrinsic( sender: sp_keyring::Sr25519Keyring, call: RuntimeCall, @@ -168,7 +164,7 @@ fn initialize_bridge_by_governance_works() { bridge_hub_test_utils::test_cases::initialize_bridge_by_governance_works::< Runtime, BridgeGrandpaRococoInstance, - >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, Governance::get()) } #[test] @@ -176,7 +172,7 @@ fn change_bridge_grandpa_pallet_mode_by_governance_works() { bridge_hub_test_utils::test_cases::change_bridge_grandpa_pallet_mode_by_governance_works::< Runtime, BridgeGrandpaRococoInstance, - >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, Governance::get()) } #[test] @@ -184,7 +180,7 @@ fn change_bridge_parachains_pallet_mode_by_governance_works() { bridge_hub_test_utils::test_cases::change_bridge_parachains_pallet_mode_by_governance_works::< Runtime, BridgeParachainRococoInstance, - >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, Governance::get()) } #[test] @@ -192,7 +188,7 @@ fn change_bridge_messages_pallet_mode_by_governance_works() { bridge_hub_test_utils::test_cases::change_bridge_messages_pallet_mode_by_governance_works::< Runtime, WithBridgeHubRococoMessagesInstance, - >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID) + >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, Governance::get()) } #[test] @@ -204,7 +200,7 @@ fn change_delivery_reward_by_governance_works() { >( collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || (DeliveryRewardInBalance::key().to_vec(), DeliveryRewardInBalance::get()), |old_value| old_value.checked_mul(2).unwrap(), ) @@ -219,7 +215,7 @@ fn change_required_stake_by_governance_works() { >( collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, - Box::new(|call| RuntimeCall::System(call).encode()), + Governance::get(), || (RequiredStakeForStakeAndSlash::key().to_vec(), RequiredStakeForStakeAndSlash::get()), |old_value| old_value.checked_mul(2).unwrap(), ) @@ -675,5 +671,5 @@ fn governance_authorize_upgrade_works() { assert_ok!(parachains_runtimes_test_utils::test_cases::can_governance_authorize_upgrade::< Runtime, RuntimeOrigin, - >(GovernanceOrigin::Location(GovernanceLocation::get()))); + >(Governance::get())); } diff --git a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs index a964cb26da0f2..ab2cae283d0cd 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs @@ -40,10 +40,7 @@ use frame_support::{ }; use frame_system::pallet_prelude::BlockNumberFor; use parachains_common::AccountId; -use parachains_runtimes_test_utils::{ - mock_open_hrmp_channel, AccountIdOf, BalanceOf, CollatorSessionKeys, ExtBuilder, RuntimeCallOf, - SlotDurations, XcmReceivedFrom, -}; +use parachains_runtimes_test_utils::{mock_open_hrmp_channel, AccountIdOf, BalanceOf, CollatorSessionKeys, ExtBuilder, GovernanceOrigin, RuntimeCallOf, RuntimeOriginOf, SlotDurations, XcmReceivedFrom}; use sp_runtime::{traits::Zero, AccountId32}; use xcm::{latest::prelude::*, AlwaysLatest}; use xcm_builder::DispatchBlobError; @@ -104,6 +101,7 @@ where pub fn initialize_bridge_by_governance_works( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, + governance_origin: GovernanceOrigin>, ) where Runtime: BasicParachainRuntime + BridgeGrandpaConfig, GrandpaPalletInstance: 'static, @@ -126,8 +124,7 @@ pub fn initialize_bridge_by_governance_works( }); // execute XCM with Transacts to `initialize bridge` as governance does - assert_ok!(RuntimeHelper::::execute_as_governance(initialize_call.encode(),) - .ensure_complete()); + assert_ok!(RuntimeHelper::::execute_as_governance_call(initialize_call, governance_origin)); // check mode after assert_eq!( @@ -142,6 +139,7 @@ pub fn initialize_bridge_by_governance_works( pub fn change_bridge_grandpa_pallet_mode_by_governance_works( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, + governance_origin: GovernanceOrigin>, ) where Runtime: BasicParachainRuntime + BridgeGrandpaConfig, GrandpaPalletInstance: 'static, @@ -164,10 +162,7 @@ pub fn change_bridge_grandpa_pallet_mode_by_governance_works::execute_as_governance( - set_operating_mode_call.encode(), - ) - .ensure_complete()); + assert_ok!(RuntimeHelper::::execute_as_governance_call(set_operating_mode_call, governance_origin.clone())); // check mode after assert_eq!( @@ -192,6 +187,7 @@ pub fn change_bridge_grandpa_pallet_mode_by_governance_works( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, + governance_origin: GovernanceOrigin>, ) where Runtime: BasicParachainRuntime + BridgeParachainsConfig, ParachainsPalletInstance: 'static, @@ -216,10 +212,7 @@ pub fn change_bridge_parachains_pallet_mode_by_governance_works::execute_as_governance( - set_operating_mode_call.encode(), - ) - .ensure_complete()); + assert_ok!(RuntimeHelper::::execute_as_governance_call(set_operating_mode_call, governance_origin.clone())); // check mode after assert_eq!( @@ -244,6 +237,7 @@ pub fn change_bridge_parachains_pallet_mode_by_governance_works( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, + governance_origin: GovernanceOrigin>, ) where Runtime: BasicParachainRuntime + BridgeMessagesConfig, MessagesPalletInstance: 'static, @@ -268,10 +262,7 @@ pub fn change_bridge_messages_pallet_mode_by_governance_works::execute_as_governance( - set_operating_mode_call.encode(), - ) - .ensure_complete()); + assert_ok!(RuntimeHelper::::execute_as_governance_call(set_operating_mode_call, governance_origin.clone())); // check mode after assert_eq!( diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs index 6528ceeb9368c..13d1899d50129 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs @@ -17,9 +17,8 @@ #![cfg(test)] use collectives_westend_runtime::{ - xcm_config::LocationToAccountId, Block, Runtime, RuntimeCall, RuntimeOrigin, + xcm_config::{GovernanceLocation, LocationToAccountId}, Block, Runtime, RuntimeCall, RuntimeOrigin, }; -use collectives_westend_runtime::xcm_config::GovernanceLocation; use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; use parachains_runtimes_test_utils::GovernanceOrigin; diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs index 0e25c1d3153f0..873ed8ebf4d03 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs @@ -16,9 +16,8 @@ #![cfg(test)] -use coretime_westend_runtime::xcm_config::GovernanceLocation; use coretime_westend_runtime::{ - xcm_config::LocationToAccountId, Block, Runtime, RuntimeCall, RuntimeOrigin, + xcm_config::{GovernanceLocation, LocationToAccountId}, Block, Runtime, RuntimeCall, RuntimeOrigin, }; use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; diff --git a/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs b/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs index b61fb713088f9..898e53e32b5f1 100644 --- a/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs @@ -19,9 +19,8 @@ use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; use parachains_runtimes_test_utils::GovernanceOrigin; -use people_westend_runtime::xcm_config::GovernanceLocation; use people_westend_runtime::{ - xcm_config::LocationToAccountId, Block, Runtime, RuntimeCall, RuntimeOrigin, + xcm_config::{GovernanceLocation, LocationToAccountId}, Block, Runtime, RuntimeCall, RuntimeOrigin, }; use sp_core::crypto::Ss58Codec; use sp_runtime::Either; diff --git a/cumulus/parachains/runtimes/test-utils/src/lib.rs b/cumulus/parachains/runtimes/test-utils/src/lib.rs index ab24875ee221a..812181da03b64 100644 --- a/cumulus/parachains/runtimes/test-utils/src/lib.rs +++ b/cumulus/parachains/runtimes/test-utils/src/lib.rs @@ -50,6 +50,7 @@ pub mod test_cases; pub type BalanceOf = ::Balance; pub type AccountIdOf = ::AccountId; pub type RuntimeCallOf = ::RuntimeCall; +pub type RuntimeOriginOf = ::RuntimeOrigin; pub type ValidatorIdOf = ::ValidatorId; pub type SessionKeysOf = ::Keys; @@ -444,6 +445,8 @@ impl< AllPalletsWithoutSystem, > RuntimeHelper { + #[deprecated(note = "Will be removed after Aug 2025; It uses hard-coded `Location::parent()`, \ + use `execute_as_governance_call` instead.")] pub fn execute_as_governance(call: Vec) -> Outcome { // prepare xcm as governance will do let xcm = Xcm(vec![ @@ -551,6 +554,7 @@ impl< } /// Enum representing governance origin/location. +#[derive(Clone)] pub enum GovernanceOrigin { Location(Location), LocationAndDescendOrigin(Location, InteriorLocation), diff --git a/cumulus/parachains/runtimes/test-utils/src/test_cases.rs b/cumulus/parachains/runtimes/test-utils/src/test_cases.rs index 528f12bf42a12..113e4f591238d 100644 --- a/cumulus/parachains/runtimes/test-utils/src/test_cases.rs +++ b/cumulus/parachains/runtimes/test-utils/src/test_cases.rs @@ -18,7 +18,7 @@ use crate::{ AccountIdOf, BasicParachainRuntime, CollatorSessionKeys, ExtBuilder, GovernanceOrigin, - ValidatorIdOf, + RuntimeCallOf, RuntimeOriginOf, ValidatorIdOf, }; use codec::Encode; use frame_support::{ @@ -40,7 +40,7 @@ type RuntimeHelper = pub fn change_storage_constant_by_governance_works( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, - runtime_call_encode: Box) -> Vec>, + governance_origin: GovernanceOrigin>, storage_constant_key_value: fn() -> (Vec, StorageConstantType), new_storage_constant_value: fn(&StorageConstantType) -> StorageConstantType, ) where @@ -78,7 +78,7 @@ pub fn change_storage_constant_by_governance_works::set_storage { + RuntimeCallOf::::from(frame_system::Call::::set_storage { items: vec![( storage_constant_key.clone(), new_storage_constant_value.encode(), @@ -86,8 +86,7 @@ pub fn change_storage_constant_by_governance_works::execute_as_governance(set_storage_call,) - .ensure_complete()); + assert_ok!(RuntimeHelper::::execute_as_governance_call(set_storage_call, governance_origin)); // check delivery reward constant after (stored) assert_eq!(StorageConstant::get(), new_storage_constant_value); @@ -102,7 +101,7 @@ pub fn change_storage_constant_by_governance_works( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, - runtime_call_encode: Box) -> Vec>, + governance_origin: GovernanceOrigin>, storage_items: Vec<(Vec, Vec)>, initialize_storage: impl FnOnce() -> (), assert_storage: impl FnOnce() -> (), @@ -128,14 +127,12 @@ pub fn set_storage_keys_by_governance_works( }); runtime.execute_with(|| { // encode `kill_storage` call - let kill_storage_call = runtime_call_encode(frame_system::Call::::set_storage { + let kill_storage_call = RuntimeCallOf::::from(frame_system::Call::::set_storage { items: storage_items.clone(), }); // execute XCM with Transact to `set_storage` as governance does - assert_ok!( - RuntimeHelper::::execute_as_governance(kill_storage_call,).ensure_complete() - ); + assert_ok!(RuntimeHelper::::execute_as_governance_call(kill_storage_call, governance_origin)); }); runtime.execute_with(|| { assert_storage(); From 2c4688b3b18818f0b9dcb7010dd2634b26d9adf7 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 26 Feb 2025 08:50:07 +0000 Subject: [PATCH 5/8] Update from bkontur running command 'prdoc --audience runtime_dev --bump patch' --- prdoc/pr_7656.prdoc | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 prdoc/pr_7656.prdoc diff --git a/prdoc/pr_7656.prdoc b/prdoc/pr_7656.prdoc new file mode 100644 index 0000000000000..a142674c474c8 --- /dev/null +++ b/prdoc/pr_7656.prdoc @@ -0,0 +1,34 @@ +title: Authorize upgrade tests for testnet runtimes +doc: +- audience: Runtime Dev + description: |- + Relates to: https://github.com/paritytech/polkadot-sdk/issues/7541 + Relates to: https://github.com/paritytech/polkadot-sdk/issues/7566 + + This PR contains improved test cases that rely on the governance location as a preparation for AHM to capture the state as it is. + + ## TODO + - [x] rewrite all tests using `RuntimeHelper::::execute_as_governance` (because it is using hard-coded `Location::parent()`) to use `RuntimeHelper::::execute_as_governance_call` + + ## Follow-up + - [ ] add similar test for westend-runtime + - [ ] add test that ensure xcm-executor adds `ClearOrigin` before all side-effect sent to different chain +crates: +- name: parachains-runtimes-test-utils + bump: patch +- name: asset-hub-westend-runtime + bump: patch +- name: bridge-hub-westend-runtime + bump: patch +- name: collectives-westend-runtime + bump: patch +- name: coretime-westend-runtime + bump: patch +- name: people-westend-runtime + bump: patch +- name: asset-hub-rococo-runtime + bump: patch +- name: bridge-hub-rococo-runtime + bump: patch +- name: bridge-hub-test-utils + bump: patch From 89724c393fa1182a756dd340e0cdb8ad0b7cdbdf Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 26 Feb 2025 08:58:17 +0000 Subject: [PATCH 6/8] Update from bkontur running command 'fmt' --- .../assets/asset-hub-rococo/tests/tests.rs | 6 +-- .../assets/asset-hub-westend/tests/tests.rs | 10 ++--- .../bridge-hub-rococo/tests/tests.rs | 42 +++++++++++++++---- .../bridge-hub-westend/tests/tests.rs | 28 ++++++++++--- .../test-utils/src/test_cases/mod.rs | 25 ++++++++--- .../collectives-westend/tests/tests.rs | 3 +- .../coretime-westend/src/xcm_config.rs | 5 ++- .../coretime/coretime-westend/tests/tests.rs | 3 +- .../people/people-westend/src/xcm_config.rs | 5 ++- .../people/people-westend/tests/tests.rs | 3 +- .../parachains/runtimes/test-utils/src/lib.rs | 33 ++++++--------- .../runtimes/test-utils/src/test_cases.rs | 26 ++++++++---- 12 files changed, 131 insertions(+), 58 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs index 1bb8d254f9fd2..0057459fc93c6 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs @@ -20,9 +20,9 @@ use asset_hub_rococo_runtime::{ xcm_config, xcm_config::{ - bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, CheckingAccount, GovernanceLocation, - ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, LocationToAccountId, StakingPot, - TokenLocation, TrustBackedAssetsPalletLocation, XcmConfig, + bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, CheckingAccount, + ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, GovernanceLocation, + LocationToAccountId, StakingPot, TokenLocation, TrustBackedAssetsPalletLocation, XcmConfig, }, AllPalletsWithoutSystem, AssetConversion, AssetDeposit, Assets, Balances, Block, CollatorSelection, ExistentialDeposit, ForeignAssets, ForeignAssetsInstance, diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs index dcf41ef58ec06..38673606541b0 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs @@ -20,9 +20,10 @@ use asset_hub_westend_runtime::{ xcm_config, xcm_config::{ - bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, GovernanceLocation, CheckingAccount, - ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, LocationToAccountId, StakingPot, - TrustBackedAssetsPalletLocation, WestendLocation, XcmConfig, + bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, CheckingAccount, + ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, GovernanceLocation, + LocationToAccountId, StakingPot, TrustBackedAssetsPalletLocation, WestendLocation, + XcmConfig, }, AllPalletsWithoutSystem, Assets, Balances, Block, ExistentialDeposit, ForeignAssets, ForeignAssetsInstance, MetadataDepositBase, MetadataDepositPerByte, ParachainSystem, @@ -49,8 +50,7 @@ use frame_support::{ use parachains_common::{AccountId, AssetIdForTrustBackedAssets, AuraId, Balance}; use sp_consensus_aura::SlotDuration; use sp_core::crypto::Ss58Codec; -use sp_runtime::traits::MaybeEquivalence; -use sp_runtime::Either; +use sp_runtime::{traits::MaybeEquivalence, Either}; use std::{convert::Into, ops::Mul}; use testnet_parachains_constants::westend::{consensus::*, currency::UNITS, fee::WeightToFee}; use xcm::latest::{ diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs index c39cf2208910a..111da98b65c7c 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs @@ -165,7 +165,11 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::initialize_bridge_by_governance_works::< Runtime, BridgeGrandpaWestendInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -174,7 +178,11 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::change_bridge_grandpa_pallet_mode_by_governance_works::< Runtime, BridgeGrandpaWestendInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -183,7 +191,11 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::change_bridge_parachains_pallet_mode_by_governance_works::< Runtime, BridgeParachainWestendInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -192,7 +204,11 @@ mod bridge_hub_westend_tests { bridge_hub_test_utils::test_cases::change_bridge_messages_pallet_mode_by_governance_works::< Runtime, WithBridgeHubWestendMessagesInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -537,7 +553,11 @@ mod bridge_hub_bulletin_tests { bridge_hub_test_utils::test_cases::initialize_bridge_by_governance_works::< Runtime, BridgeGrandpaRococoBulletinInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -546,7 +566,11 @@ mod bridge_hub_bulletin_tests { bridge_hub_test_utils::test_cases::change_bridge_grandpa_pallet_mode_by_governance_works::< Runtime, BridgeGrandpaRococoBulletinInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -555,7 +579,11 @@ mod bridge_hub_bulletin_tests { bridge_hub_test_utils::test_cases::change_bridge_messages_pallet_mode_by_governance_works::< Runtime, WithRococoBulletinMessagesInstance, - >(collator_session_keys(), bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, + Governance::get(), + ) } #[test] diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs index 15bfb558d64ba..d9b69c5480481 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/tests/tests.rs @@ -27,7 +27,9 @@ use bridge_hub_test_utils::{ use bridge_hub_westend_runtime::{ bridge_common_config, bridge_to_rococo_config, bridge_to_rococo_config::RococoGlobalConsensusNetwork, - xcm_config::{GovernanceLocation, LocationToAccountId, RelayNetwork, WestendLocation, XcmConfig}, + xcm_config::{ + GovernanceLocation, LocationToAccountId, RelayNetwork, WestendLocation, XcmConfig, + }, AllPalletsWithoutSystem, Balances, Block, BridgeRejectObsoleteHeadersAndMessages, BridgeRelayers, Executive, ExistentialDeposit, ParachainSystem, PolkadotXcm, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, SessionKeys, TransactionPayment, TxExtension, @@ -164,7 +166,11 @@ fn initialize_bridge_by_governance_works() { bridge_hub_test_utils::test_cases::initialize_bridge_by_governance_works::< Runtime, BridgeGrandpaRococoInstance, - >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -172,7 +178,11 @@ fn change_bridge_grandpa_pallet_mode_by_governance_works() { bridge_hub_test_utils::test_cases::change_bridge_grandpa_pallet_mode_by_governance_works::< Runtime, BridgeGrandpaRococoInstance, - >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -180,7 +190,11 @@ fn change_bridge_parachains_pallet_mode_by_governance_works() { bridge_hub_test_utils::test_cases::change_bridge_parachains_pallet_mode_by_governance_works::< Runtime, BridgeParachainRococoInstance, - >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, + Governance::get(), + ) } #[test] @@ -188,7 +202,11 @@ fn change_bridge_messages_pallet_mode_by_governance_works() { bridge_hub_test_utils::test_cases::change_bridge_messages_pallet_mode_by_governance_works::< Runtime, WithBridgeHubRococoMessagesInstance, - >(collator_session_keys(), bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, Governance::get()) + >( + collator_session_keys(), + bp_bridge_hub_westend::BRIDGE_HUB_WESTEND_PARACHAIN_ID, + Governance::get(), + ) } #[test] diff --git a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs index ab2cae283d0cd..b966d445c225a 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/mod.rs @@ -40,7 +40,10 @@ use frame_support::{ }; use frame_system::pallet_prelude::BlockNumberFor; use parachains_common::AccountId; -use parachains_runtimes_test_utils::{mock_open_hrmp_channel, AccountIdOf, BalanceOf, CollatorSessionKeys, ExtBuilder, GovernanceOrigin, RuntimeCallOf, RuntimeOriginOf, SlotDurations, XcmReceivedFrom}; +use parachains_runtimes_test_utils::{ + mock_open_hrmp_channel, AccountIdOf, BalanceOf, CollatorSessionKeys, ExtBuilder, + GovernanceOrigin, RuntimeCallOf, RuntimeOriginOf, SlotDurations, XcmReceivedFrom, +}; use sp_runtime::{traits::Zero, AccountId32}; use xcm::{latest::prelude::*, AlwaysLatest}; use xcm_builder::DispatchBlobError; @@ -124,7 +127,10 @@ pub fn initialize_bridge_by_governance_works( }); // execute XCM with Transacts to `initialize bridge` as governance does - assert_ok!(RuntimeHelper::::execute_as_governance_call(initialize_call, governance_origin)); + assert_ok!(RuntimeHelper::::execute_as_governance_call( + initialize_call, + governance_origin + )); // check mode after assert_eq!( @@ -162,7 +168,10 @@ pub fn change_bridge_grandpa_pallet_mode_by_governance_works::execute_as_governance_call(set_operating_mode_call, governance_origin.clone())); + assert_ok!(RuntimeHelper::::execute_as_governance_call( + set_operating_mode_call, + governance_origin.clone() + )); // check mode after assert_eq!( @@ -212,7 +221,10 @@ pub fn change_bridge_parachains_pallet_mode_by_governance_works::execute_as_governance_call(set_operating_mode_call, governance_origin.clone())); + assert_ok!(RuntimeHelper::::execute_as_governance_call( + set_operating_mode_call, + governance_origin.clone() + )); // check mode after assert_eq!( @@ -262,7 +274,10 @@ pub fn change_bridge_messages_pallet_mode_by_governance_works::execute_as_governance_call(set_operating_mode_call, governance_origin.clone())); + assert_ok!(RuntimeHelper::::execute_as_governance_call( + set_operating_mode_call, + governance_origin.clone() + )); // check mode after assert_eq!( diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs index 13d1899d50129..016538c35715f 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/tests/tests.rs @@ -17,7 +17,8 @@ #![cfg(test)] use collectives_westend_runtime::{ - xcm_config::{GovernanceLocation, LocationToAccountId}, Block, Runtime, RuntimeCall, RuntimeOrigin, + xcm_config::{GovernanceLocation, LocationToAccountId}, + Block, Runtime, RuntimeCall, RuntimeOrigin, }; use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs index ce9ce094328be..707b9a42c8558 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs @@ -149,7 +149,10 @@ impl Contains for ParentOrParentsPlurality { pub struct FellowsPlurality; impl Contains for FellowsPlurality { fn contains(location: &Location) -> bool { - matches!(location.unpack(), (1, [Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }])) + matches!( + location.unpack(), + (1, [Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }]) + ) } } diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs index 873ed8ebf4d03..b8e51f2d8e781 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/tests/tests.rs @@ -17,7 +17,8 @@ #![cfg(test)] use coretime_westend_runtime::{ - xcm_config::{GovernanceLocation, LocationToAccountId}, Block, Runtime, RuntimeCall, RuntimeOrigin, + xcm_config::{GovernanceLocation, LocationToAccountId}, + Block, Runtime, RuntimeCall, RuntimeOrigin, }; use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; diff --git a/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs index 516c18c8786c3..98be08379a525 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/xcm_config.rs @@ -157,7 +157,10 @@ impl Contains for ParentOrParentsPlurality { pub struct FellowsPlurality; impl Contains for FellowsPlurality { fn contains(location: &Location) -> bool { - matches!(location.unpack(), (1, [Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }])) + matches!( + location.unpack(), + (1, [Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }]) + ) } } diff --git a/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs b/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs index 898e53e32b5f1..3d06ab8368a65 100644 --- a/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs +++ b/cumulus/parachains/runtimes/people/people-westend/tests/tests.rs @@ -20,7 +20,8 @@ use frame_support::{assert_err, assert_ok}; use parachains_common::AccountId; use parachains_runtimes_test_utils::GovernanceOrigin; use people_westend_runtime::{ - xcm_config::{GovernanceLocation, LocationToAccountId}, Block, Runtime, RuntimeCall, RuntimeOrigin, + xcm_config::{GovernanceLocation, LocationToAccountId}, + Block, Runtime, RuntimeCall, RuntimeOrigin, }; use sp_core::crypto::Ss58Codec; use sp_runtime::Either; diff --git a/cumulus/parachains/runtimes/test-utils/src/lib.rs b/cumulus/parachains/runtimes/test-utils/src/lib.rs index 812181da03b64..7bbcb7beb0e79 100644 --- a/cumulus/parachains/runtimes/test-utils/src/lib.rs +++ b/cumulus/parachains/runtimes/test-utils/src/lib.rs @@ -445,8 +445,10 @@ impl< AllPalletsWithoutSystem, > RuntimeHelper { - #[deprecated(note = "Will be removed after Aug 2025; It uses hard-coded `Location::parent()`, \ - use `execute_as_governance_call` instead.")] + #[deprecated( + note = "Will be removed after Aug 2025; It uses hard-coded `Location::parent()`, \ + use `execute_as_governance_call` instead." + )] pub fn execute_as_governance(call: Vec) -> Outcome { // prepare xcm as governance will do let xcm = Xcm(vec![ @@ -473,8 +475,7 @@ impl< pub fn execute_as_governance_call( call: Call, governance_origin: GovernanceOrigin, - ) -> Result<(), Either> - { + ) -> Result<(), Either> { // execute xcm as governance would send let execute_xcm = |call: Call, governance_location, descend_origin| { // prepare xcm @@ -501,21 +502,14 @@ impl< }; match governance_origin { - GovernanceOrigin::Location(location) => { - execute_xcm(call, location, None) - .ensure_complete() - .map_err(Either::Right) - }, - GovernanceOrigin::LocationAndDescendOrigin(location, descend_origin) => { + GovernanceOrigin::Location(location) => + execute_xcm(call, location, None).ensure_complete().map_err(Either::Right), + GovernanceOrigin::LocationAndDescendOrigin(location, descend_origin) => execute_xcm(call, location, Some(descend_origin)) .ensure_complete() - .map_err(Either::Right) - }, - GovernanceOrigin::Origin(origin) => { - call.dispatch(origin) - .map(|_| ()) - .map_err(|e| Either::Left(e.error)) - }, + .map_err(Either::Right), + GovernanceOrigin::Origin(origin) => + call.dispatch(origin).map(|_| ()).map_err(|e| Either::Left(e.error)), } } @@ -623,9 +617,8 @@ impl< .into_iter() .filter_map(|e| unwrap_xcmp_queue_event(e.event.encode())) .find_map(|e| match e { - cumulus_pallet_xcmp_queue::Event::XcmpMessageSent { message_hash } => { - Some(message_hash) - }, + cumulus_pallet_xcmp_queue::Event::XcmpMessageSent { message_hash } => + Some(message_hash), _ => None, }) } diff --git a/cumulus/parachains/runtimes/test-utils/src/test_cases.rs b/cumulus/parachains/runtimes/test-utils/src/test_cases.rs index 113e4f591238d..c6b1ccdac3c12 100644 --- a/cumulus/parachains/runtimes/test-utils/src/test_cases.rs +++ b/cumulus/parachains/runtimes/test-utils/src/test_cases.rs @@ -26,8 +26,10 @@ use frame_support::{ traits::{Get, OriginTrait}, }; use parachains_common::AccountId; -use sp_runtime::traits::{Block as BlockT, StaticLookup}; -use sp_runtime::{DispatchError, Either}; +use sp_runtime::{ + traits::{Block as BlockT, StaticLookup}, + DispatchError, Either, +}; use xcm::prelude::XcmError; use xcm_runtime_apis::fees::{ runtime_decl_for_xcm_payment_api::XcmPaymentApiV1, Error as XcmPaymentApiError, @@ -86,7 +88,10 @@ pub fn change_storage_constant_by_governance_works::execute_as_governance_call(set_storage_call, governance_origin)); + assert_ok!(RuntimeHelper::::execute_as_governance_call( + set_storage_call, + governance_origin + )); // check delivery reward constant after (stored) assert_eq!(StorageConstant::get(), new_storage_constant_value); @@ -127,12 +132,16 @@ pub fn set_storage_keys_by_governance_works( }); runtime.execute_with(|| { // encode `kill_storage` call - let kill_storage_call = RuntimeCallOf::::from(frame_system::Call::::set_storage { - items: storage_items.clone(), - }); + let kill_storage_call = + RuntimeCallOf::::from(frame_system::Call::::set_storage { + items: storage_items.clone(), + }); // execute XCM with Transact to `set_storage` as governance does - assert_ok!(RuntimeHelper::::execute_as_governance_call(kill_storage_call, governance_origin)); + assert_ok!(RuntimeHelper::::execute_as_governance_call( + kill_storage_call, + governance_origin + )); }); runtime.execute_with(|| { assert_storage(); @@ -196,7 +205,8 @@ where }); } -/// Generic test case for Cumulus-based parachain that verifies if runtime can process `frame_system::Call::authorize_upgrade` from governance system. +/// Generic test case for Cumulus-based parachain that verifies if runtime can process +/// `frame_system::Call::authorize_upgrade` from governance system. pub fn can_governance_authorize_upgrade( governance_origin: GovernanceOrigin, ) -> Result<(), Either> From 7defb1f39f086ba15b120cd88ac5306fc60d3ea8 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 26 Feb 2025 09:05:15 +0000 Subject: [PATCH 7/8] Update from bkontur running command 'prdoc --audience runtime_dev --bump patch --force' --- prdoc/pr_7656.prdoc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/prdoc/pr_7656.prdoc b/prdoc/pr_7656.prdoc index a142674c474c8..43550d2339182 100644 --- a/prdoc/pr_7656.prdoc +++ b/prdoc/pr_7656.prdoc @@ -1,11 +1,15 @@ -title: Authorize upgrade tests for testnet runtimes +title: Authorize upgrade tests for testnet runtimes + `execute_as_governance` refactor doc: - audience: Runtime Dev description: |- Relates to: https://github.com/paritytech/polkadot-sdk/issues/7541 Relates to: https://github.com/paritytech/polkadot-sdk/issues/7566 - This PR contains improved test cases that rely on the governance location as a preparation for AHM to capture the state as it is. + This PR contains improved test cases that rely on the governance location as preparation for AHM to capture the state as it is. + It introduces `execute_as_governance_call`, which can be configured with various governance location setups instead of the hard-coded `Location::parent()`. + + Additionally, it adds a test for `authorize_upgrade` to all SP testnets. + ## TODO - [x] rewrite all tests using `RuntimeHelper::::execute_as_governance` (because it is using hard-coded `Location::parent()`) to use `RuntimeHelper::::execute_as_governance_call` From 431229833ea00f7cee240a9aba79290fc1fb262a Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Wed, 26 Feb 2025 10:59:39 +0100 Subject: [PATCH 8/8] Apply suggestions from code review --- prdoc/pr_7656.prdoc | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/prdoc/pr_7656.prdoc b/prdoc/pr_7656.prdoc index 43550d2339182..80a62c284daa1 100644 --- a/prdoc/pr_7656.prdoc +++ b/prdoc/pr_7656.prdoc @@ -2,28 +2,15 @@ title: Authorize upgrade tests for testnet runtimes + `execute_as_governance` re doc: - audience: Runtime Dev description: |- - Relates to: https://github.com/paritytech/polkadot-sdk/issues/7541 - Relates to: https://github.com/paritytech/polkadot-sdk/issues/7566 - This PR contains improved test cases that rely on the governance location as preparation for AHM to capture the state as it is. It introduces `execute_as_governance_call`, which can be configured with various governance location setups instead of the hard-coded `Location::parent()`. - - Additionally, it adds a test for `authorize_upgrade` to all SP testnets. - - - ## TODO - - [x] rewrite all tests using `RuntimeHelper::::execute_as_governance` (because it is using hard-coded `Location::parent()`) to use `RuntimeHelper::::execute_as_governance_call` - - ## Follow-up - - [ ] add similar test for westend-runtime - - [ ] add test that ensure xcm-executor adds `ClearOrigin` before all side-effect sent to different chain crates: - name: parachains-runtimes-test-utils - bump: patch + bump: major - name: asset-hub-westend-runtime bump: patch - name: bridge-hub-westend-runtime - bump: patch + bump: minor - name: collectives-westend-runtime bump: patch - name: coretime-westend-runtime @@ -35,4 +22,4 @@ crates: - name: bridge-hub-rococo-runtime bump: patch - name: bridge-hub-test-utils - bump: patch + bump: major