From d537e984dd08232248e09a02a26de399b813615c Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sat, 15 Jun 2024 14:03:41 +0200 Subject: [PATCH 1/4] Upgrade initializer should be mandatory --- bridges/snowbridge/Cargo.lock | 1 + .../pallets/outbound-queue/src/mock.rs | 6 ++--- .../pallets/outbound-queue/src/test.rs | 2 +- bridges/snowbridge/pallets/system/src/lib.rs | 7 +++--- .../snowbridge/pallets/system/src/tests.rs | 22 ++++--------------- .../primitives/core/src/outbound.rs | 15 +++++-------- 6 files changed, 17 insertions(+), 36 deletions(-) diff --git a/bridges/snowbridge/Cargo.lock b/bridges/snowbridge/Cargo.lock index fa1335eef6fc..e99f222a0a1b 100644 --- a/bridges/snowbridge/Cargo.lock +++ b/bridges/snowbridge/Cargo.lock @@ -3073,6 +3073,7 @@ name = "pallet-migrations" version = "1.0.0" dependencies = [ "docify", + "frame-benchmarking", "frame-support", "frame-system", "impl-trait-for-tuples", diff --git a/bridges/snowbridge/pallets/outbound-queue/src/mock.rs b/bridges/snowbridge/pallets/outbound-queue/src/mock.rs index d65a96e2702d..0838efce14ba 100644 --- a/bridges/snowbridge/pallets/outbound-queue/src/mock.rs +++ b/bridges/snowbridge/pallets/outbound-queue/src/mock.rs @@ -134,7 +134,7 @@ where command: Command::Upgrade { impl_address: H160::zero(), impl_code_hash: H256::zero(), - initializer: None, + initializer: Default::default(), }, } } @@ -152,10 +152,10 @@ where command: Command::Upgrade { impl_address: H160::zero(), impl_code_hash: H256::zero(), - initializer: Some(Initializer { + initializer: Initializer { params: (0..1000).map(|_| 1u8).collect::>(), maximum_required_gas: 0, - }), + }, }, } } diff --git a/bridges/snowbridge/pallets/outbound-queue/src/test.rs b/bridges/snowbridge/pallets/outbound-queue/src/test.rs index 4e9ea36e24bc..782173724edd 100644 --- a/bridges/snowbridge/pallets/outbound-queue/src/test.rs +++ b/bridges/snowbridge/pallets/outbound-queue/src/test.rs @@ -85,7 +85,7 @@ fn process_message_yields_on_max_messages_per_block() { command: Command::Upgrade { impl_address: Default::default(), impl_code_hash: Default::default(), - initializer: None, + initializer: Default::default(), }, } .encode(); diff --git a/bridges/snowbridge/pallets/system/src/lib.rs b/bridges/snowbridge/pallets/system/src/lib.rs index 39c73e3630e7..f5805614cf0f 100644 --- a/bridges/snowbridge/pallets/system/src/lib.rs +++ b/bridges/snowbridge/pallets/system/src/lib.rs @@ -175,7 +175,7 @@ pub mod pallet { Upgrade { impl_address: H160, impl_code_hash: H256, - initializer_params_hash: Option, + initializer_params_hash: H256, }, /// An CreateAgent message was sent to the Gateway CreateAgent { @@ -278,7 +278,7 @@ pub mod pallet { origin: OriginFor, impl_address: H160, impl_code_hash: H256, - initializer: Option, + initializer: Initializer, ) -> DispatchResult { ensure_root(origin)?; @@ -287,8 +287,7 @@ pub mod pallet { Error::::InvalidUpgradeParameters ); - let initializer_params_hash: Option = - initializer.as_ref().map(|i| H256::from(blake2_256(i.params.as_ref()))); + let initializer_params_hash: H256 = blake2_256(initializer.params.as_ref()).into(); let command = Command::Upgrade { impl_address, impl_code_hash, initializer }; Self::send(PRIMARY_GOVERNANCE_CHANNEL, command, PaysFee::::No)?; diff --git a/bridges/snowbridge/pallets/system/src/tests.rs b/bridges/snowbridge/pallets/system/src/tests.rs index 09f24195a30a..2be4867678a4 100644 --- a/bridges/snowbridge/pallets/system/src/tests.rs +++ b/bridges/snowbridge/pallets/system/src/tests.rs @@ -87,12 +87,12 @@ fn upgrade_as_root() { let address: H160 = [1_u8; 20].into(); let code_hash: H256 = [1_u8; 32].into(); - assert_ok!(EthereumSystem::upgrade(origin, address, code_hash, None)); + assert_ok!(EthereumSystem::upgrade(origin, address, code_hash, Default::default())); System::assert_last_event(RuntimeEvent::EthereumSystem(crate::Event::Upgrade { impl_address: address, impl_code_hash: code_hash, - initializer_params_hash: None, + initializer_params_hash: hex!("0e5751c026e543b2e8ab2eb06099daa1d1e5df47778f7787faab45cdf12fe3a8").into() })); }); } @@ -104,19 +104,7 @@ fn upgrade_as_signed_fails() { let address: H160 = Default::default(); let code_hash: H256 = Default::default(); - assert_noop!(EthereumSystem::upgrade(origin, address, code_hash, None), BadOrigin); - }); -} - -#[test] -fn upgrade_with_params() { - new_test_ext(true).execute_with(|| { - let origin = RuntimeOrigin::root(); - let address: H160 = [1_u8; 20].into(); - let code_hash: H256 = [1_u8; 32].into(); - let initializer: Option = - Some(Initializer { params: [0; 256].into(), maximum_required_gas: 10000 }); - assert_ok!(EthereumSystem::upgrade(origin, address, code_hash, initializer)); + assert_noop!(EthereumSystem::upgrade(origin, address, code_hash, Default::default()), BadOrigin); }); } @@ -607,9 +595,7 @@ fn charge_fee_for_upgrade() { let origin = RuntimeOrigin::root(); let address: H160 = [1_u8; 20].into(); let code_hash: H256 = [1_u8; 32].into(); - let initializer: Option = - Some(Initializer { params: [0; 256].into(), maximum_required_gas: 10000 }); - assert_ok!(EthereumSystem::upgrade(origin, address, code_hash, initializer.clone())); + assert_ok!(EthereumSystem::upgrade(origin, address, code_hash, Default::default())); // assert sovereign_balance does not change as we do not charge for sudo operations let sovereign_account = sibling_sovereign_account::(para_id.into()); diff --git a/bridges/snowbridge/primitives/core/src/outbound.rs b/bridges/snowbridge/primitives/core/src/outbound.rs index 0ba0fdb61089..2b66da5893aa 100644 --- a/bridges/snowbridge/primitives/core/src/outbound.rs +++ b/bridges/snowbridge/primitives/core/src/outbound.rs @@ -82,8 +82,8 @@ mod v1 { impl_address: H160, /// Codehash of the implementation contract impl_code_hash: H256, - /// Optionally invoke an initializer in the implementation contract - initializer: Option, + /// Invoke initializer delegated to the implementation contract + initializer: Initializer, }, /// Create an agent representing a consensus system on Polkadot CreateAgent { @@ -169,9 +169,7 @@ mod v1 { ethabi::encode(&[Token::Tuple(vec![ Token::Address(*impl_address), Token::FixedBytes(impl_code_hash.as_bytes().to_owned()), - initializer - .clone() - .map_or(Token::Bytes(vec![]), |i| Token::Bytes(i.params)), + Token::Bytes(initializer.params.clone()), ])]), Command::CreateAgent { agent_id } => ethabi::encode(&[Token::Tuple(vec![Token::FixedBytes( @@ -218,6 +216,7 @@ mod v1 { /// Representation of a call to the initializer of an implementation contract. /// The initializer has the following ABI signature: `initialize(bytes)`. #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, TypeInfo)] + #[cfg_attr(feature = "std", derive(Default))] pub struct Initializer { /// ABI-encoded params of type `bytes` to pass to the initializer pub params: Vec, @@ -393,13 +392,9 @@ impl GasMeter for ConstantGasMeter { AgentExecuteCommand::TransferToken { .. } => 100_000, }, Command::Upgrade { initializer, .. } => { - let initializer_max_gas = match *initializer { - Some(Initializer { maximum_required_gas, .. }) => maximum_required_gas, - None => 0, - }; // total maximum gas must also include the gas used for updating the proxy before // the the initializer is called. - 50_000 + initializer_max_gas + 50_000 + initializer.maximum_required_gas }, Command::SetTokenTransferFees { .. } => 60_000, Command::SetPricingParameters { .. } => 60_000, From 85c5cf7928e39b85fbb4468395f5bbbd49cb0755 Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sat, 15 Jun 2024 14:15:59 +0200 Subject: [PATCH 2/4] Update benchmarks --- bridges/snowbridge/pallets/system/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridges/snowbridge/pallets/system/src/benchmarking.rs b/bridges/snowbridge/pallets/system/src/benchmarking.rs index ef908ad6a3f9..1622aab95ae2 100644 --- a/bridges/snowbridge/pallets/system/src/benchmarking.rs +++ b/bridges/snowbridge/pallets/system/src/benchmarking.rs @@ -36,7 +36,7 @@ mod benchmarks { RawOrigin::Root, impl_address, impl_code_hash, - Some(Initializer { params, maximum_required_gas: 100000 }), + Initializer { params, maximum_required_gas: 100000 }, ); Ok(()) From 1820b838d363d0b34865d7a9269dad2f862b6cfe Mon Sep 17 00:00:00 2001 From: ron Date: Wed, 26 Jun 2024 09:06:41 +0800 Subject: [PATCH 3/4] Fix format --- bridges/snowbridge/pallets/system/src/tests.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/bridges/snowbridge/pallets/system/src/tests.rs b/bridges/snowbridge/pallets/system/src/tests.rs index 2be4867678a4..3206a0d71496 100644 --- a/bridges/snowbridge/pallets/system/src/tests.rs +++ b/bridges/snowbridge/pallets/system/src/tests.rs @@ -92,7 +92,10 @@ fn upgrade_as_root() { System::assert_last_event(RuntimeEvent::EthereumSystem(crate::Event::Upgrade { impl_address: address, impl_code_hash: code_hash, - initializer_params_hash: hex!("0e5751c026e543b2e8ab2eb06099daa1d1e5df47778f7787faab45cdf12fe3a8").into() + initializer_params_hash: hex!( + "0e5751c026e543b2e8ab2eb06099daa1d1e5df47778f7787faab45cdf12fe3a8" + ) + .into(), })); }); } @@ -104,7 +107,10 @@ fn upgrade_as_signed_fails() { let address: H160 = Default::default(); let code_hash: H256 = Default::default(); - assert_noop!(EthereumSystem::upgrade(origin, address, code_hash, Default::default()), BadOrigin); + assert_noop!( + EthereumSystem::upgrade(origin, address, code_hash, Default::default()), + BadOrigin + ); }); } From c3e3abb6570d147b0163fa1baa4dd98ba47cdf7c Mon Sep 17 00:00:00 2001 From: ron Date: Wed, 26 Jun 2024 09:57:47 +0800 Subject: [PATCH 4/4] Fix test --- bridges/snowbridge/pallets/outbound-queue/src/benchmarking.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bridges/snowbridge/pallets/outbound-queue/src/benchmarking.rs b/bridges/snowbridge/pallets/outbound-queue/src/benchmarking.rs index ee5754e86962..99c469ba86da 100644 --- a/bridges/snowbridge/pallets/outbound-queue/src/benchmarking.rs +++ b/bridges/snowbridge/pallets/outbound-queue/src/benchmarking.rs @@ -30,10 +30,10 @@ mod benchmarks { command: Command::Upgrade { impl_address: H160::zero(), impl_code_hash: H256::zero(), - initializer: Some(Initializer { + initializer: Initializer { params: [7u8; 256].into_iter().collect(), maximum_required_gas: 200_000, - }), + }, }, }; let origin = AggregateMessageOrigin::Snowbridge([1; 32].into());