Skip to content

Commit

Permalink
Authorize upgrade tests for testnet runtimes + `execute_as_governance…
Browse files Browse the repository at this point in the history
…` refactor (#7656)

Relates to: #7541 
Relates to: #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::<Runtime>::execute_as_governance` (because it is using
hard-coded `Location::parent()`) to use
`RuntimeHelper::<Runtime>::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

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
bkontur and github-actions[bot] authored Feb 26, 2025
1 parent 5117049 commit e9be92d
Show file tree
Hide file tree
Showing 17 changed files with 556 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
18 changes: 11 additions & 7 deletions cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ use asset_hub_rococo_runtime::{
xcm_config,
xcm_config::{
bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, CheckingAccount,
ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, LocationToAccountId, StakingPot,
TokenLocation, TrustBackedAssetsPalletLocation, XcmConfig,
ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, GovernanceLocation,
LocationToAccountId, StakingPot, TokenLocation, TrustBackedAssetsPalletLocation, XcmConfig,
},
AllPalletsWithoutSystem, AssetConversion, AssetDeposit, Assets, Balances, Block,
CollatorSelection, ExistentialDeposit, ForeignAssets, ForeignAssetsInstance,
Expand All @@ -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::{
Expand All @@ -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<RuntimeOrigin> = GovernanceOrigin::Location(GovernanceLocation::get());
}

type AssetIdForTrustBackedAssetsConvert =
assets_common::AssetIdForTrustBackedAssetsConvert<TrustBackedAssetsPalletLocation>;

Expand Down Expand Up @@ -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(),
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -265,17 +266,17 @@ impl Contains<Location> 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)])
)
}
}

pub struct AmbassadorEntities;
impl Contains<Location> for AmbassadorEntities {
fn contains(location: &Location) -> bool {
matches!(location.unpack(), (1, [Parachain(1001), PalletInstance(74)]))
matches!(location.unpack(), (1, [Parachain(COLLECTIVES_ID), PalletInstance(74)]))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ use asset_hub_westend_runtime::{
xcm_config,
xcm_config::{
bridging, AssetFeeAsExistentialDepositMultiplierFeeCharger, CheckingAccount,
ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, LocationToAccountId, StakingPot,
TrustBackedAssetsPalletLocation, WestendLocation, XcmConfig,
ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger, GovernanceLocation,
LocationToAccountId, StakingPot, TrustBackedAssetsPalletLocation, WestendLocation,
XcmConfig,
},
AllPalletsWithoutSystem, Assets, Balances, Block, ExistentialDeposit, ForeignAssets,
ForeignAssetsInstance, MetadataDepositBase, MetadataDepositPerByte, ParachainSystem,
Expand All @@ -32,12 +33,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_noop, assert_ok,
assert_err, assert_noop, assert_ok, parameter_types,
traits::{
fungible::{Inspect, Mutate},
fungibles::{
Expand All @@ -49,7 +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::{traits::MaybeEquivalence, Either};
use std::{convert::Into, ops::Mul};
use testnet_parachains_constants::westend::{consensus::*, currency::UNITS, fee::WeightToFee};
use xcm::latest::{
Expand All @@ -63,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<RuntimeOrigin> = GovernanceOrigin::Location(GovernanceLocation::get());
}

type AssetIdForTrustBackedAssetsConvert =
assets_common::AssetIdForTrustBackedAssetsConvert<TrustBackedAssetsPalletLocation>;

Expand Down Expand Up @@ -1309,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(),
Expand All @@ -1335,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",
Expand Down Expand Up @@ -1512,3 +1517,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())));
}
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -42,6 +42,7 @@ use xcm_runtime_apis::conversions::LocationToAccountHelper;

parameter_types! {
pub CheckingAccount: AccountId = PolkadotXcm::check_account();
pub Governance: GovernanceOrigin<RuntimeOrigin> = GovernanceOrigin::Location(Location::parent());
}

fn construct_extrinsic(
Expand Down Expand Up @@ -164,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)
>(
collator_session_keys(),
bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
Governance::get(),
)
}

#[test]
Expand All @@ -173,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)
>(
collator_session_keys(),
bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
Governance::get(),
)
}

#[test]
Expand All @@ -182,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)
>(
collator_session_keys(),
bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
Governance::get(),
)
}

#[test]
Expand All @@ -191,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)
>(
collator_session_keys(),
bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
Governance::get(),
)
}

#[test]
Expand All @@ -203,7 +220,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(),
)
Expand All @@ -219,7 +236,7 @@ mod bridge_hub_westend_tests {
bridge_hub_test_utils::test_cases::set_storage_keys_by_governance_works::<Runtime>(
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::<Runtime>::hashed_key_for::<ChannelId>(
channel_id_one,
Expand Down Expand Up @@ -284,7 +301,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(),
)
Expand Down Expand Up @@ -536,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)
>(
collator_session_keys(),
bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
Governance::get(),
)
}

#[test]
Expand All @@ -545,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)
>(
collator_session_keys(),
bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
Governance::get(),
)
}

#[test]
Expand All @@ -554,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)
>(
collator_session_keys(),
bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
Governance::get(),
)
}

#[test]
Expand Down Expand Up @@ -716,7 +745,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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit e9be92d

Please sign in to comment.