From 963d84fa84fbe539e2060e92628ca001417deab0 Mon Sep 17 00:00:00 2001 From: Alistair Singh Date: Sat, 27 Jan 2024 21:26:01 +0200 Subject: [PATCH] use set storage instead of kill storage --- .../bridge-hub-rococo/src/xcm_config.rs | 20 ++---- .../bridge-hub-rococo/tests/tests.rs | 65 +++++++++---------- .../test-utils/src/test_cases/mod.rs | 2 +- .../runtimes/test-utils/src/test_cases.rs | 14 ++-- 4 files changed, 45 insertions(+), 56 deletions(-) diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs index 7944d451e437..237cfef258a5 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs @@ -161,25 +161,17 @@ impl Contains for SafeCallFilter { match call { RuntimeCall::System(frame_system::Call::set_storage { items }) if items.iter().all(|(k, _)| { - k.eq(&DeliveryRewardInBalance::key()) | - k.eq(&RequiredStakeForStakeAndSlash::key()) | - k.eq(&EthereumGatewayAddress::key()) + k.eq(&DeliveryRewardInBalance::key()) || + k.eq(&RequiredStakeForStakeAndSlash::key()) || + k.eq(&EthereumGatewayAddress::key()) || + // Allow resetting of Ethereum nonces in Rococo only. + k.starts_with(&snowbridge_pallet_inbound_queue::Nonce::::final_prefix()) || + k.starts_with(&snowbridge_pallet_outbound_queue::Nonce::::final_prefix()) }) => return true, _ => (), }; - // Allow to removed dedicated storage items (called by governance-like) - if let RuntimeCall::System(frame_system::Call::kill_storage { keys }) = call { - return keys.iter().all(|k| { - // Allow resetting of Ethereum nonces in Rococo only. - k.starts_with(&snowbridge_pallet_inbound_queue::Nonce::::final_prefix()) || - k.starts_with( - &snowbridge_pallet_outbound_queue::Nonce::::final_prefix(), - ) - }); - } - matches!( call, RuntimeCall::PolkadotXcm( 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 9861d88afe56..4131dfbc3deb 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 @@ -26,9 +26,7 @@ use bridge_hub_rococo_runtime::{ }; use bridge_hub_test_utils::SlotDurations; use codec::{Decode, Encode}; -use frame_support::{ - dispatch::GetDispatchInfo, parameter_types, traits::ConstU8, StoragePrefixedMap, -}; +use frame_support::{dispatch::GetDispatchInfo, parameter_types, traits::ConstU8}; use parachains_common::{ rococo::{consensus::RELAY_CHAIN_SLOT_DURATION_MILLIS, fee::WeightToFee}, AccountId, AuraId, Balance, SLOT_DURATION, @@ -226,62 +224,49 @@ mod bridge_hub_westend_tests { } #[test] - fn kill_ethereum_nonces_by_governance_works() { + fn change_ethereum_nonces_by_governance_works() { let channel_id_one: ChannelId = [1; 32].into(); let channel_id_two: ChannelId = [2; 32].into(); let nonce = 42; // Reset a single inbound channel - bridge_hub_test_utils::test_cases::kill_storage_keys_by_governance_works::( + 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()), - snowbridge_pallet_inbound_queue::Nonce::::hashed_key_for::( - channel_id_one, - ) - .to_vec(), + vec![ + (snowbridge_pallet_outbound_queue::Nonce::::hashed_key_for::( + channel_id_one, + ) + .to_vec(), 0u64.encode()), + (snowbridge_pallet_inbound_queue::Nonce::::hashed_key_for::( + channel_id_one, + ) + .to_vec(), 0u64.encode()), + ], || { - snowbridge_pallet_inbound_queue::Nonce::::insert::( + // Outbound + snowbridge_pallet_outbound_queue::Nonce::::insert::( channel_id_one, nonce, ); - snowbridge_pallet_inbound_queue::Nonce::::insert::( + snowbridge_pallet_outbound_queue::Nonce::::insert::( channel_id_two, nonce, ); - }, - || { - assert_eq!( - snowbridge_pallet_inbound_queue::Nonce::::get(channel_id_one), - 0 - ); - assert_eq!( - snowbridge_pallet_inbound_queue::Nonce::::get(channel_id_two), - nonce - ); - }, - ); - // Reset a single outbound channel - bridge_hub_test_utils::test_cases::kill_storage_keys_by_governance_works::( - collator_session_keys(), - bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID, - Box::new(|call| RuntimeCall::System(call).encode()), - snowbridge_pallet_outbound_queue::Nonce::::hashed_key_for::( - channel_id_one, - ) - .to_vec(), - || { - snowbridge_pallet_outbound_queue::Nonce::::insert::( + // Inbound + snowbridge_pallet_inbound_queue::Nonce::::insert::( channel_id_one, nonce, ); - snowbridge_pallet_outbound_queue::Nonce::::insert::( + snowbridge_pallet_inbound_queue::Nonce::::insert::( channel_id_two, nonce, ); }, || { + // Outbound assert_eq!( snowbridge_pallet_outbound_queue::Nonce::::get(channel_id_one), 0 @@ -290,6 +275,16 @@ mod bridge_hub_westend_tests { snowbridge_pallet_outbound_queue::Nonce::::get(channel_id_two), nonce ); + + // Inbound + assert_eq!( + snowbridge_pallet_inbound_queue::Nonce::::get(channel_id_one), + 0 + ); + assert_eq!( + snowbridge_pallet_inbound_queue::Nonce::::get(channel_id_two), + nonce + ); }, ); } 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 edee39ef2116..9b7d28932686 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 @@ -73,7 +73,7 @@ pub type RuntimeHelper = // Re-export test_case from `parachains-runtimes-test-utils` pub use parachains_runtimes_test_utils::test_cases::{ - change_storage_constant_by_governance_works, kill_storage_keys_by_governance_works, + change_storage_constant_by_governance_works, set_storage_keys_by_governance_works, }; /// Prepare default runtime storage and run test within this context. diff --git a/cumulus/parachains/runtimes/test-utils/src/test_cases.rs b/cumulus/parachains/runtimes/test-utils/src/test_cases.rs index 87c8d70468b1..f78bf9877ec2 100644 --- a/cumulus/parachains/runtimes/test-utils/src/test_cases.rs +++ b/cumulus/parachains/runtimes/test-utils/src/test_cases.rs @@ -92,12 +92,12 @@ pub fn change_storage_constant_by_governance_works( +/// Test-case makes sure that `Runtime` can change storage constant via governance-like call +pub fn set_storage_keys_by_governance_works( collator_session_key: CollatorSessionKeys, runtime_para_id: u32, runtime_call_encode: Box) -> Vec>, - storage_constant_key: Vec, + storage_items: Vec<(Vec, Vec)>, initialize_storage: impl FnOnce() -> (), assert_storage: impl FnOnce() -> (), ) where @@ -121,14 +121,16 @@ pub fn kill_storage_keys_by_governance_works( }); runtime.execute_with(|| { // encode `kill_storage` call - let kill_storage_call = runtime_call_encode(frame_system::Call::::kill_storage { - keys: vec![storage_constant_key.clone()], + let kill_storage_call = runtime_call_encode(frame_system::Call::::set_storage { + items: storage_items.clone(), }); // estimate - storing just 1 value use frame_system::WeightInfo; let require_weight_at_most = - ::SystemWeightInfo::kill_storage(1); + ::SystemWeightInfo::set_storage( + storage_items.len().try_into().unwrap(), + ); // execute XCM with Transact to `set_storage` as governance does assert_ok!(RuntimeHelper::::execute_as_governance(