use set storage instead of kill storage

Snowfork · Jan 27, 2024 · 963d84f · 963d84f
1 parent 76c2de8
commit 963d84f
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 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
@@ -161,25 +161,17 @@ impl Contains<RuntimeCall> 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::<Runtime>::final_prefix()) ||
+						k.starts_with(&snowbridge_pallet_outbound_queue::Nonce::<Runtime>::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::<Runtime>::final_prefix()) ||
-					k.starts_with(
-						&snowbridge_pallet_outbound_queue::Nonce::<Runtime>::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
@@ -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::<Runtime>(
+		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()),
-			snowbridge_pallet_inbound_queue::Nonce::<Runtime>::hashed_key_for::<ChannelId>(
-				channel_id_one,
-			)
-			.to_vec(),
+			vec![
+				(snowbridge_pallet_outbound_queue::Nonce::<Runtime>::hashed_key_for::<ChannelId>(
+					channel_id_one,
+				)
+				.to_vec(), 0u64.encode()),
+				(snowbridge_pallet_inbound_queue::Nonce::<Runtime>::hashed_key_for::<ChannelId>(
+					channel_id_one,
+				)
+				.to_vec(), 0u64.encode()),
+			],
 			|| {
-				snowbridge_pallet_inbound_queue::Nonce::<Runtime>::insert::<ChannelId, u64>(
+				// Outbound
+				snowbridge_pallet_outbound_queue::Nonce::<Runtime>::insert::<ChannelId, u64>(
 					channel_id_one,
 					nonce,
 				);
-				snowbridge_pallet_inbound_queue::Nonce::<Runtime>::insert::<ChannelId, u64>(
+				snowbridge_pallet_outbound_queue::Nonce::<Runtime>::insert::<ChannelId, u64>(
 					channel_id_two,
 					nonce,
 				);
-			},
-			|| {
-				assert_eq!(
-					snowbridge_pallet_inbound_queue::Nonce::<Runtime>::get(channel_id_one),
-					0
-				);
-				assert_eq!(
-					snowbridge_pallet_inbound_queue::Nonce::<Runtime>::get(channel_id_two),
-					nonce
-				);
-			},
-		);
 
-		// Reset a single outbound channel
-		bridge_hub_test_utils::test_cases::kill_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()),
-			snowbridge_pallet_outbound_queue::Nonce::<Runtime>::hashed_key_for::<ChannelId>(
-				channel_id_one,
-			)
-			.to_vec(),
-			|| {
-				snowbridge_pallet_outbound_queue::Nonce::<Runtime>::insert::<ChannelId, u64>(
+				// Inbound
+				snowbridge_pallet_inbound_queue::Nonce::<Runtime>::insert::<ChannelId, u64>(
 					channel_id_one,
 					nonce,
 				);
-				snowbridge_pallet_outbound_queue::Nonce::<Runtime>::insert::<ChannelId, u64>(
+				snowbridge_pallet_inbound_queue::Nonce::<Runtime>::insert::<ChannelId, u64>(
 					channel_id_two,
 					nonce,
 				);
 			},
 			|| {
+				// Outbound
 				assert_eq!(
 					snowbridge_pallet_outbound_queue::Nonce::<Runtime>::get(channel_id_one),
 					0
@@ -290,6 +275,16 @@ mod bridge_hub_westend_tests {
 					snowbridge_pallet_outbound_queue::Nonce::<Runtime>::get(channel_id_two),
 					nonce
 				);
+
+				// Inbound
+				assert_eq!(
+					snowbridge_pallet_inbound_queue::Nonce::<Runtime>::get(channel_id_one),
+					0
+				);
+				assert_eq!(
+					snowbridge_pallet_inbound_queue::Nonce::<Runtime>::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
@@ -73,7 +73,7 @@ pub type RuntimeHelper<Runtime, AllPalletsWithoutSystem = ()> =
 
 // 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
@@ -92,12 +92,12 @@ pub fn change_storage_constant_by_governance_works<Runtime, StorageConstant, Sto
 		})
 }
 
-/// Test-case makes sure that `Runtime` can kill storage constant via governance-like call
-pub fn kill_storage_keys_by_governance_works<Runtime>(
+/// Test-case makes sure that `Runtime` can change storage constant via governance-like call
+pub fn set_storage_keys_by_governance_works<Runtime>(
 	collator_session_key: CollatorSessionKeys<Runtime>,
 	runtime_para_id: u32,
 	runtime_call_encode: Box<dyn Fn(frame_system::Call<Runtime>) -> Vec<u8>>,
-	storage_constant_key: Vec<u8>,
+	storage_items: Vec<(Vec<u8>, Vec<u8>)>,
 	initialize_storage: impl FnOnce() -> (),
 	assert_storage: impl FnOnce() -> (),
 ) where
@@ -121,14 +121,16 @@ pub fn kill_storage_keys_by_governance_works<Runtime>(
 	});
 	runtime.execute_with(|| {
 		// encode `kill_storage` call
-		let kill_storage_call = runtime_call_encode(frame_system::Call::<Runtime>::kill_storage {
-			keys: vec![storage_constant_key.clone()],
+		let kill_storage_call = runtime_call_encode(frame_system::Call::<Runtime>::set_storage {
+			items: storage_items.clone(),
 		});
 
 		// estimate - storing just 1 value
 		use frame_system::WeightInfo;
 		let require_weight_at_most =
-			<Runtime as frame_system::Config>::SystemWeightInfo::kill_storage(1);
+			<Runtime as frame_system::Config>::SystemWeightInfo::set_storage(
+				storage_items.len().try_into().unwrap(),
+			);
 
 		// execute XCM with Transact to `set_storage` as governance does
 		assert_ok!(RuntimeHelper::<Runtime>::execute_as_governance(