Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revive: Make salt salt optional to allow for CREATE1 semantics #5556

Merged
merged 7 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions prdoc/pr_5556.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
title: Make salt optional

doc:
- audience: Runtime Dev
description: |
By making salt optional we allow clients to use CREATE1 semantics
when deploying a new contract.

crates:
- name: pallet-revive
bump: major
2 changes: 1 addition & 1 deletion substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3017,7 +3017,7 @@ impl_runtime_apis! {
storage_deposit_limit: Option<Balance>,
code: pallet_revive::Code,
data: Vec<u8>,
salt: [u8; 32],
salt: Option<[u8; 32]>,
) -> pallet_revive::ContractInstantiateResult<Balance, EventRecord>
{
Revive::bare_instantiate(
Expand Down
1 change: 0 additions & 1 deletion substrate/frame/revive/src/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ impl AddressMapper<AccountId32> for DefaultAddressMapper {
}

/// Determine the address of a contract using CREATE semantics.
#[allow(dead_code)]
pub fn create1(deployer: &H160, nonce: u64) -> H160 {
let mut list = rlp::RlpStream::new_list(2);
list.append(&deployer.as_bytes());
Expand Down
10 changes: 4 additions & 6 deletions substrate/frame/revive/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ where
data: Vec<u8>,
) -> Result<Contract<T>, &'static str> {
T::Currency::set_balance(&caller, caller_funding::<T>());
let salt = [0xffu8; 32];
let salt = Some([0xffu8; 32]);

let outcome = Contracts::<T>::bare_instantiate(
RawOrigin::Signed(caller.clone()).into(),
Expand Down Expand Up @@ -344,7 +344,6 @@ mod benchmarks {

// `c`: Size of the code in bytes.
// `i`: Size of the input in bytes.
// `s`: Size of the salt in bytes.
#[benchmark(pov_mode = Measured)]
fn instantiate_with_code(
c: Linear<0, { T::MaxCodeLen::get() }>,
Expand All @@ -362,7 +361,7 @@ mod benchmarks {
let account_id = T::AddressMapper::to_account_id_contract(&addr);
let storage_deposit = default_deposit_limit::<T>();
#[extrinsic_call]
_(origin, value, Weight::MAX, storage_deposit, code, input, salt);
_(origin, value, Weight::MAX, storage_deposit, code, input, Some(salt));

let deposit =
T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &account_id);
Expand All @@ -378,7 +377,7 @@ mod benchmarks {
}

// `i`: Size of the input in bytes.
// `s`: Size of the salt in bytes.
// `s`: Size of e salt in bytes.
#[benchmark(pov_mode = Measured)]
fn instantiate(i: Linear<0, { limits::MEMORY_BYTES }>) -> Result<(), BenchmarkError> {
let input = vec![42u8; i as usize];
Expand All @@ -403,7 +402,7 @@ mod benchmarks {
storage_deposit,
hash,
input,
salt,
Some(salt),
);

let deposit =
Expand Down Expand Up @@ -1529,7 +1528,6 @@ mod benchmarks {

// t: value to transfer
// i: size of input in bytes
// s: size of salt in bytes
#[benchmark(pov_mode = Measured)]
fn seal_instantiate(i: Linear<0, { limits::MEMORY_BYTES }>) -> Result<(), BenchmarkError> {
let code = WasmModule::dummy();
Expand Down
45 changes: 25 additions & 20 deletions substrate/frame/revive/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use sp_core::{
use sp_io::{crypto::secp256k1_ecdsa_recover_compressed, hashing::blake2_256};
use sp_runtime::{
traits::{BadOrigin, Convert, Dispatchable, Zero},
DispatchError,
DispatchError, SaturatedConversion,
};

pub type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
Expand Down Expand Up @@ -213,7 +213,7 @@ pub trait Ext: sealing::Sealed {
code: H256,
value: BalanceOf<Self::T>,
input_data: Vec<u8>,
salt: &[u8; 32],
salt: Option<&[u8; 32]>,
) -> Result<(H160, ExecReturnValue), ExecError>;

/// Transfer all funds to `beneficiary` and delete the contract.
Expand Down Expand Up @@ -573,7 +573,7 @@ enum FrameArgs<'a, T: Config, E> {
/// The executable whose `deploy` function is run.
executable: E,
/// A salt used in the contract address derivation of the new contract.
salt: &'a [u8; 32],
salt: Option<&'a [u8; 32]>,
/// The input data is used in the contract address derivation of the new contract.
input_data: &'a [u8],
},
Expand Down Expand Up @@ -750,7 +750,7 @@ where
storage_meter: &'a mut storage::meter::Meter<T>,
value: BalanceOf<T>,
input_data: Vec<u8>,
salt: &[u8; 32],
salt: Option<&[u8; 32]>,
debug_message: Option<&'a mut DebugBuffer>,
) -> Result<(H160, ExecReturnValue), ExecError> {
let (mut stack, executable) = Self::new(
Expand Down Expand Up @@ -863,7 +863,12 @@ where
},
FrameArgs::Instantiate { sender, executable, salt, input_data } => {
let deployer = T::AddressMapper::to_address(&sender);
let address = address::create2(&deployer, executable.code(), input_data, salt);
let account_nonce = <System<T>>::account_nonce(&sender);
let address = if let Some(salt) = salt {
address::create2(&deployer, executable.code(), input_data, salt)
} else {
address::create1(&deployer, account_nonce.saturated_into())
};
let contract = ContractInfo::new(
&address,
<System<T>>::account_nonce(&sender),
Expand Down Expand Up @@ -1321,7 +1326,7 @@ where
code_hash: H256,
value: BalanceOf<T>,
input_data: Vec<u8>,
salt: &[u8; 32],
salt: Option<&[u8; 32]>,
) -> Result<(H160, ExecReturnValue), ExecError> {
let executable = E::from_storage(code_hash, self.gas_meter_mut())?;
let sender = &self.top_frame().account_id;
Expand Down Expand Up @@ -2088,7 +2093,7 @@ mod tests {
&mut storage_meter,
min_balance,
vec![1, 2, 3, 4],
&[0; 32],
Some(&[0; 32]),
None,
);
assert_matches!(result, Ok(_));
Expand Down Expand Up @@ -2497,7 +2502,7 @@ mod tests {
&mut storage_meter,
0, // <- zero value
vec![],
&[0; 32],
Some(&[0; 32]),
None,
),
Err(_)
Expand Down Expand Up @@ -2533,7 +2538,7 @@ mod tests {

min_balance,
vec![],
&[0;32],
Some(&[0 ;32]),
None,
),
Ok((address, ref output)) if output.data == vec![80, 65, 83, 83] => address
Expand Down Expand Up @@ -2587,7 +2592,7 @@ mod tests {

min_balance,
vec![],
&[0;32],
Some(&[0; 32]),
None,
),
Ok((address, ref output)) if output.data == vec![70, 65, 73, 76] => address
Expand Down Expand Up @@ -2620,7 +2625,7 @@ mod tests {
dummy_ch,
<Test as Config>::Currency::minimum_balance(),
vec![],
&[48; 32],
Some(&[48; 32]),
)
.unwrap();

Expand Down Expand Up @@ -2698,7 +2703,7 @@ mod tests {
dummy_ch,
<Test as Config>::Currency::minimum_balance(),
vec![],
&[0; 32],
Some(&[0; 32]),
),
Err(ExecError {
error: DispatchError::Other("It's a trap!"),
Expand Down Expand Up @@ -2771,7 +2776,7 @@ mod tests {
&mut storage_meter,
100,
vec![],
&[0; 32],
Some(&[0; 32]),
None,
),
Err(Error::<Test>::TerminatedInConstructor.into())
Expand Down Expand Up @@ -2882,7 +2887,7 @@ mod tests {
&mut storage_meter,
min_balance,
vec![],
&[0; 32],
Some(&[0; 32]),
None,
);
assert_matches!(result, Ok(_));
Expand Down Expand Up @@ -3250,7 +3255,7 @@ mod tests {
fail_code,
ctx.ext.minimum_balance() * 100,
vec![],
&[0; 32],
Some(&[0; 32]),
)
.ok();
exec_success()
Expand All @@ -3267,7 +3272,7 @@ mod tests {
success_code,
ctx.ext.minimum_balance() * 100,
vec![],
&[0; 32],
Some(&[0; 32]),
)
.unwrap();

Expand Down Expand Up @@ -3318,7 +3323,7 @@ mod tests {
&mut storage_meter,
min_balance * 100,
vec![],
&[0; 32],
Some(&[0; 32]),
None,
)
.ok();
Expand All @@ -3331,7 +3336,7 @@ mod tests {
&mut storage_meter,
min_balance * 100,
vec![],
&[0; 32],
Some(&[0; 32]),
None,
));
assert_eq!(System::account_nonce(&ALICE), 1);
Expand All @@ -3343,7 +3348,7 @@ mod tests {
&mut storage_meter,
min_balance * 200,
vec![],
&[0; 32],
Some(&[0; 32]),
None,
));
assert_eq!(System::account_nonce(&ALICE), 2);
Expand All @@ -3355,7 +3360,7 @@ mod tests {
&mut storage_meter,
min_balance * 200,
vec![],
&[0; 32],
Some(&[0; 32]),
None,
));
assert_eq!(System::account_nonce(&ALICE), 3);
Expand Down
24 changes: 12 additions & 12 deletions substrate/frame/revive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,7 @@ pub mod pallet {
/// must be supplied.
#[pallet::call_index(1)]
#[pallet::weight(
T::WeightInfo::instantiate(data.len() as u32, salt.len() as u32).saturating_add(*gas_limit)
T::WeightInfo::instantiate(data.len() as u32, 32).saturating_add(*gas_limit)
)]
pub fn instantiate(
origin: OriginFor<T>,
Expand All @@ -833,10 +833,9 @@ pub mod pallet {
#[pallet::compact] storage_deposit_limit: BalanceOf<T>,
code_hash: sp_core::H256,
data: Vec<u8>,
salt: [u8; 32],
salt: Option<[u8; 32]>,
) -> DispatchResultWithPostInfo {
let data_len = data.len() as u32;
let salt_len = salt.len() as u32;
let mut output = Self::bare_instantiate(
origin,
value,
Expand All @@ -856,7 +855,7 @@ pub mod pallet {
dispatch_result(
output.result.map(|result| result.result),
output.gas_consumed,
T::WeightInfo::instantiate(data_len, salt_len),
T::WeightInfo::instantiate(data_len, 32),
)
}

Expand All @@ -875,7 +874,9 @@ pub mod pallet {
/// from the caller to pay for the storage consumed.
/// * `code`: The contract code to deploy in raw bytes.
/// * `data`: The input data to pass to the contract constructor.
/// * `salt`: Used for the address derivation. See [`crate::address::create2`].
/// * `salt`: Used for the address derivation. If `Some` is supplied then `CREATE2`
/// semantics are used. If `None` then `CRATE1` is used.
///
///
/// Instantiation is executed as follows:
///
Expand All @@ -887,7 +888,7 @@ pub mod pallet {
/// - The `deploy` function is executed in the context of the newly-created account.
#[pallet::call_index(2)]
#[pallet::weight(
T::WeightInfo::instantiate_with_code(code.len() as u32, data.len() as u32, salt.len() as u32)
T::WeightInfo::instantiate_with_code(code.len() as u32, data.len() as u32, 32)
.saturating_add(*gas_limit)
)]
pub fn instantiate_with_code(
Expand All @@ -897,11 +898,10 @@ pub mod pallet {
#[pallet::compact] storage_deposit_limit: BalanceOf<T>,
code: Vec<u8>,
data: Vec<u8>,
salt: [u8; 32],
salt: Option<[u8; 32]>,
) -> DispatchResultWithPostInfo {
let code_len = code.len() as u32;
let data_len = data.len() as u32;
let salt_len = salt.len() as u32;
let mut output = Self::bare_instantiate(
origin,
value,
Expand All @@ -921,7 +921,7 @@ pub mod pallet {
dispatch_result(
output.result.map(|result| result.result),
output.gas_consumed,
T::WeightInfo::instantiate_with_code(code_len, data_len, salt_len),
T::WeightInfo::instantiate_with_code(code_len, data_len, 32),
)
}

Expand Down Expand Up @@ -1122,7 +1122,7 @@ impl<T: Config> Pallet<T> {
mut storage_deposit_limit: BalanceOf<T>,
code: Code,
data: Vec<u8>,
salt: [u8; 32],
salt: Option<[u8; 32]>,
debug: DebugInfo,
collect_events: CollectEvents,
) -> ContractInstantiateResult<BalanceOf<T>, EventRecordOf<T>> {
Expand Down Expand Up @@ -1158,7 +1158,7 @@ impl<T: Config> Pallet<T> {
&mut storage_meter,
value,
data,
&salt,
salt.as_ref(),
debug_message.as_mut(),
);
storage_deposit = storage_meter
Expand Down Expand Up @@ -1298,7 +1298,7 @@ sp_api::decl_runtime_apis! {
storage_deposit_limit: Option<Balance>,
code: Code,
data: Vec<u8>,
salt: [u8; 32],
salt: Option<[u8; 32]>,
) -> ContractInstantiateResult<Balance, EventRecord>;

/// Upload new code without instantiating a contract from it.
Expand Down
Loading