Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Commit

Permalink
Upgrade initializer should be mandatory
Browse files Browse the repository at this point in the history
  • Loading branch information
vgeddes committed Jun 15, 2024
1 parent d79ef62 commit d537e98
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 36 deletions.
1 change: 1 addition & 0 deletions bridges/snowbridge/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions bridges/snowbridge/pallets/outbound-queue/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ where
command: Command::Upgrade {
impl_address: H160::zero(),
impl_code_hash: H256::zero(),
initializer: None,
initializer: Default::default(),
},
}
}
Expand All @@ -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::<Vec<u8>>(),
maximum_required_gas: 0,
}),
},
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion bridges/snowbridge/pallets/outbound-queue/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 3 additions & 4 deletions bridges/snowbridge/pallets/system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ pub mod pallet {
Upgrade {
impl_address: H160,
impl_code_hash: H256,
initializer_params_hash: Option<H256>,
initializer_params_hash: H256,
},
/// An CreateAgent message was sent to the Gateway
CreateAgent {
Expand Down Expand Up @@ -278,7 +278,7 @@ pub mod pallet {
origin: OriginFor<T>,
impl_address: H160,
impl_code_hash: H256,
initializer: Option<Initializer>,
initializer: Initializer,
) -> DispatchResult {
ensure_root(origin)?;

Expand All @@ -287,8 +287,7 @@ pub mod pallet {
Error::<T>::InvalidUpgradeParameters
);

let initializer_params_hash: Option<H256> =
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::<T>::No)?;

Expand Down
22 changes: 4 additions & 18 deletions bridges/snowbridge/pallets/system/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}));
});
}
Expand All @@ -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<Initializer> =
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);
});
}

Expand Down Expand Up @@ -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<Initializer> =
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::<Test>(para_id.into());
Expand Down
15 changes: 5 additions & 10 deletions bridges/snowbridge/primitives/core/src/outbound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Initializer>,
/// Invoke initializer delegated to the implementation contract
initializer: Initializer,
},
/// Create an agent representing a consensus system on Polkadot
CreateAgent {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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<u8>,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit d537e98

Please sign in to comment.