Skip to content

Commit

Permalink
contracts: Improve contract address derivation (paritytech#12883)
Browse files Browse the repository at this point in the history
* Add prefix to address derivation

* Extend benchmark

* Fix node test

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* Adapt to new benchmark

* Update dispatchable benchmarks

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* Use benchmark results

* Apply suggestions from code review

Co-authored-by: Sasha Gryaznov <[email protected]>

* Don't use T::AdressGenerator directly

* Rename constructor_args to input_data

Co-authored-by: command-bot <>
Co-authored-by: Sasha Gryaznov <[email protected]>
  • Loading branch information
2 people authored and ltfschoen committed Feb 22, 2023
1 parent 27f4cf8 commit 69e5b4b
Show file tree
Hide file tree
Showing 18 changed files with 1,559 additions and 1,479 deletions.
3 changes: 2 additions & 1 deletion bin/node/executor/tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,8 @@ fn deploying_wasm_contract_should_work() {
let transfer_code = wat::parse_str(CODE_TRANSFER).unwrap();
let transfer_ch = <Runtime as frame_system::Config>::Hashing::hash(&transfer_code);

let addr = pallet_contracts::Pallet::<Runtime>::contract_address(&charlie(), &transfer_ch, &[]);
let addr =
pallet_contracts::Pallet::<Runtime>::contract_address(&charlie(), &transfer_ch, &[], &[]);

let time = 42 * 1000;
let b = construct_block(
Expand Down
6 changes: 1 addition & 5 deletions frame/contracts/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,11 +404,7 @@ fn expand_impls(def: &mut EnvDef) -> TokenStream2 {
let dummy_impls = expand_functions(def, false, quote! { () });

quote! {
impl<'a, E> crate::wasm::Environment<crate::wasm::runtime::Runtime<'a, E>> for Env
where
E: Ext,
<E::T as ::frame_system::Config>::AccountId:
::sp_core::crypto::UncheckedFrom<<E::T as ::frame_system::Config>::Hash> + ::core::convert::AsRef<[::core::primitive::u8]>,
impl<'a, E: Ext> crate::wasm::Environment<crate::wasm::runtime::Runtime<'a, E>> for Env
{
fn define(store: &mut ::wasmi::Store<crate::wasm::Runtime<E>>, linker: &mut ::wasmi::Linker<crate::wasm::Runtime<E>>, allow_unstable: bool) -> Result<(), ::wasmi::errors::LinkerError> {
#impls
Expand Down
25 changes: 4 additions & 21 deletions frame/contracts/src/benchmarking/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

use crate::{Config, Determinism};
use frame_support::traits::Get;
use sp_core::crypto::UncheckedFrom;
use sp_runtime::traits::Hash;
use sp_std::{borrow::ToOwned, prelude::*};
use wasm_instrument::{
Expand Down Expand Up @@ -105,11 +104,7 @@ pub struct ImportedMemory {
}

impl ImportedMemory {
pub fn max<T: Config>() -> Self
where
T: Config,
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
{
pub fn max<T: Config>() -> Self {
let pages = max_pages::<T>();
Self { min_pages: pages, max_pages: pages }
}
Expand All @@ -130,11 +125,7 @@ pub struct WasmModule<T: Config> {
pub memory: Option<ImportedMemory>,
}

impl<T: Config> From<ModuleDefinition> for WasmModule<T>
where
T: Config,
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
{
impl<T: Config> From<ModuleDefinition> for WasmModule<T> {
fn from(def: ModuleDefinition) -> Self {
// internal functions start at that offset.
let func_offset = u32::try_from(def.imported_functions.len()).unwrap();
Expand Down Expand Up @@ -259,11 +250,7 @@ where
}
}

impl<T: Config> WasmModule<T>
where
T: Config,
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
{
impl<T: Config> WasmModule<T> {
/// Uses the supplied wasm module and instruments it when requested.
pub fn instrumented(code: &[u8], inject_gas: bool, inject_stack: bool) -> Self {
let module = {
Expand Down Expand Up @@ -533,11 +520,7 @@ pub mod body {
}

/// The maximum amount of pages any contract is allowed to have according to the current `Schedule`.
pub fn max_pages<T: Config>() -> u32
where
T: Config,
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
{
pub fn max_pages<T: Config>() -> u32 {
T::Schedule::get().limits.memory_pages
}

Expand Down
31 changes: 17 additions & 14 deletions frame/contracts/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ struct Contract<T: Config> {

impl<T: Config> Contract<T>
where
T: Config,
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
<BalanceOf<T> as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode,
{
/// Create new contract and use a default account id as instantiator.
Expand All @@ -90,7 +88,7 @@ where
let value = Pallet::<T>::min_balance();
T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
let salt = vec![0xff];
let addr = Contracts::<T>::contract_address(&caller, &module.hash, &salt);
let addr = Contracts::<T>::contract_address(&caller, &module.hash, &data, &salt);

Contracts::<T>::store_code_raw(module.code, caller.clone())?;
Contracts::<T>::instantiate(
Expand Down Expand Up @@ -203,8 +201,6 @@ macro_rules! load_benchmark {

benchmarks! {
where_clause { where
T::AccountId: UncheckedFrom<T::Hash>,
T::AccountId: AsRef<[u8]>,
<BalanceOf<T> as codec::HasCompact>::Type: Clone + Eq + PartialEq + sp_std::fmt::Debug + scale_info::TypeInfo + codec::Encode,
}

Expand Down Expand Up @@ -270,6 +266,7 @@ benchmarks! {
// a code of that size into the sandbox.
//
// `c`: Size of the code in kilobytes.
// `i`: Size of the input in kilobytes.
// `s`: Size of the salt in kilobytes.
//
// # Note
Expand All @@ -278,15 +275,17 @@ benchmarks! {
// to be larger than the maximum size **after instrumentation**.
instantiate_with_code {
let c in 0 .. Perbill::from_percent(49).mul_ceil(T::MaxCodeLen::get());
let i in 0 .. code::max_pages::<T>() * 64 * 1024;
let s in 0 .. code::max_pages::<T>() * 64 * 1024;
let input = vec![42u8; i as usize];
let salt = vec![42u8; s as usize];
let value = Pallet::<T>::min_balance();
let caller = whitelisted_caller();
T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
let WasmModule { code, hash, .. } = WasmModule::<T>::sized(c, Location::Call);
let origin = RawOrigin::Signed(caller.clone());
let addr = Contracts::<T>::contract_address(&caller, &hash, &salt);
}: _(origin, value, Weight::MAX, None, code, vec![], salt)
let addr = Contracts::<T>::contract_address(&caller, &hash, &input, &salt);
}: _(origin, value, Weight::MAX, None, code, input, salt)
verify {
// the contract itself does not trigger any reserves
let deposit = T::Currency::reserved_balance(&addr);
Expand All @@ -303,18 +302,21 @@ benchmarks! {
}

// Instantiate uses a dummy contract constructor to measure the overhead of the instantiate.
// `i`: Size of the input in kilobytes.
// `s`: Size of the salt in kilobytes.
instantiate {
let i in 0 .. code::max_pages::<T>() * 64 * 1024;
let s in 0 .. code::max_pages::<T>() * 64 * 1024;
let input = vec![42u8; i as usize];
let salt = vec![42u8; s as usize];
let value = Pallet::<T>::min_balance();
let caller = whitelisted_caller();
T::Currency::make_free_balance_be(&caller, caller_funding::<T>());
let WasmModule { code, hash, .. } = WasmModule::<T>::dummy();
let origin = RawOrigin::Signed(caller.clone());
let addr = Contracts::<T>::contract_address(&caller, &hash, &salt);
let addr = Contracts::<T>::contract_address(&caller, &hash, &input, &salt);
Contracts::<T>::store_code_raw(code, caller.clone())?;
}: _(origin, value, Weight::MAX, None, hash, vec![], salt)
}: _(origin, value, Weight::MAX, None, hash, input, salt)
verify {
// the contract itself does not trigger any reserves
let deposit = T::Currency::reserved_balance(&addr);
Expand Down Expand Up @@ -1779,7 +1781,7 @@ benchmarks! {
let addresses = hashes
.iter()
.map(|hash| Contracts::<T>::contract_address(
&instance.account_id, hash, &[],
&instance.account_id, hash, &[], &[],
))
.collect::<Vec<_>>();

Expand All @@ -1796,8 +1798,9 @@ benchmarks! {
}
}

seal_instantiate_per_transfer_salt_kb {
seal_instantiate_per_transfer_input_salt_kb {
let t in 0 .. 1;
let i in 0 .. (code::max_pages::<T>() - 1) * 64;
let s in 0 .. (code::max_pages::<T>() - 1) * 64;
let callee_code = WasmModule::<T>::dummy();
let hash = callee_code.hash;
Expand Down Expand Up @@ -1865,14 +1868,14 @@ benchmarks! {
Regular(Instruction::I64Const(0)), // gas
Regular(Instruction::I32Const(value_offset as i32)), // value_ptr
Regular(Instruction::I32Const(value_len as i32)), // value_len
Regular(Instruction::I32Const(0)), // input_data_ptr
Regular(Instruction::I32Const(0)), // input_data_len
Counter(salt_offset as u32, salt_len as u32), // input_data_ptr
Regular(Instruction::I32Const((i * 1024) as i32)), // input_data_len
Regular(Instruction::I32Const((addr_len_offset + addr_len) as i32)), // address_ptr
Regular(Instruction::I32Const(addr_len_offset as i32)), // address_len_ptr
Regular(Instruction::I32Const(SENTINEL as i32)), // output_ptr
Regular(Instruction::I32Const(0)), // output_len_ptr
Counter(salt_offset as u32, salt_len as u32), // salt_ptr
Regular(Instruction::I32Const((s * 1024).max(salt_len as u32) as i32)), // salt_len
Regular(Instruction::I32Const((s * 1024) as i32)), // salt_len
Regular(Instruction::Call(0)),
Regular(Instruction::I32Eqz),
Regular(Instruction::If(BlockType::NoResult)),
Expand Down
7 changes: 1 addition & 6 deletions frame/contracts/src/benchmarking/sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
/// ! environment that provides the seal interface as imported functions.
use super::{code::WasmModule, Config};
use crate::wasm::{Environment, PrefabWasmModule};
use sp_core::crypto::UncheckedFrom;
use wasmi::{errors::LinkerError, Func, Linker, StackLimits, Store};

/// Minimal execution environment without any imported functions.
Expand All @@ -36,11 +35,7 @@ impl Sandbox {
}
}

impl<T: Config> From<&WasmModule<T>> for Sandbox
where
T: Config,
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
{
impl<T: Config> From<&WasmModule<T>> for Sandbox {
/// Creates an instance from the supplied module and supplies as much memory
/// to the instance as the module declares as imported.
fn from(module: &WasmModule<T>) -> Self {
Expand Down
27 changes: 5 additions & 22 deletions frame/contracts/src/chain_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ use sp_std::{marker::PhantomData, vec::Vec};
pub use crate::{exec::Ext, Config};
pub use frame_system::Config as SysConfig;
pub use pallet_contracts_primitives::ReturnFlags;
pub use sp_core::crypto::UncheckedFrom;

/// Result that returns a [`DispatchError`] on error.
pub type Result<T> = sp_std::result::Result<T, DispatchError>;
Expand Down Expand Up @@ -114,10 +113,7 @@ pub trait ChainExtension<C: Config> {
/// In case of `Err` the contract execution is immediately suspended and the passed error
/// is returned to the caller. Otherwise the value of [`RetVal`] determines the exit
/// behaviour.
fn call<E>(&mut self, env: Environment<E, InitState>) -> Result<RetVal>
where
E: Ext<T = C>,
<E::T as SysConfig>::AccountId: UncheckedFrom<<E::T as SysConfig>::Hash> + AsRef<[u8]>;
fn call<E: Ext<T = C>>(&mut self, env: Environment<E, InitState>) -> Result<RetVal>;

/// Determines whether chain extensions are enabled for this chain.
///
Expand Down Expand Up @@ -153,11 +149,7 @@ pub trait RegisteredChainExtension<C: Config>: ChainExtension<C> {
#[impl_trait_for_tuples::impl_for_tuples(10)]
#[tuple_types_custom_trait_bound(RegisteredChainExtension<C>)]
impl<C: Config> ChainExtension<C> for Tuple {
fn call<E>(&mut self, mut env: Environment<E, InitState>) -> Result<RetVal>
where
E: Ext<T = C>,
<E::T as SysConfig>::AccountId: UncheckedFrom<<E::T as SysConfig>::Hash> + AsRef<[u8]>,
{
fn call<E: Ext<T = C>>(&mut self, mut env: Environment<E, InitState>) -> Result<RetVal> {
for_tuples!(
#(
if (Tuple::ID == env.ext_id()) && Tuple::enabled() {
Expand Down Expand Up @@ -205,10 +197,7 @@ pub struct Environment<'a, 'b, E: Ext, S: State> {
}

/// Functions that are available in every state of this type.
impl<'a, 'b, E: Ext, S: State> Environment<'a, 'b, E, S>
where
<E::T as SysConfig>::AccountId: UncheckedFrom<<E::T as SysConfig>::Hash> + AsRef<[u8]>,
{
impl<'a, 'b, E: Ext, S: State> Environment<'a, 'b, E, S> {
/// The function id within the `id` passed by a contract.
///
/// It returns the two least significant bytes of the `id` passed by a contract as the other
Expand Down Expand Up @@ -326,10 +315,7 @@ impl<'a, 'b, E: Ext, S: PrimOut> Environment<'a, 'b, E, S> {
}

/// Functions to use the input arguments as pointer to a buffer.
impl<'a, 'b, E: Ext, S: BufIn> Environment<'a, 'b, E, S>
where
<E::T as SysConfig>::AccountId: UncheckedFrom<<E::T as SysConfig>::Hash> + AsRef<[u8]>,
{
impl<'a, 'b, E: Ext, S: BufIn> Environment<'a, 'b, E, S> {
/// Reads `min(max_len, in_len)` from contract memory.
///
/// This does **not** charge any weight. The caller must make sure that the an
Expand Down Expand Up @@ -401,10 +387,7 @@ where
}

/// Functions to use the output arguments as pointer to a buffer.
impl<'a, 'b, E: Ext, S: BufOut> Environment<'a, 'b, E, S>
where
<E::T as SysConfig>::AccountId: UncheckedFrom<<E::T as SysConfig>::Hash> + AsRef<[u8]>,
{
impl<'a, 'b, E: Ext, S: BufOut> Environment<'a, 'b, E, S> {
/// Write the supplied buffer to contract memory.
///
/// If the contract supplied buffer is smaller than the passed `buffer` an `Err` is returned.
Expand Down
18 changes: 12 additions & 6 deletions frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use frame_support::{
use frame_system::RawOrigin;
use pallet_contracts_primitives::ExecReturnValue;
use smallvec::{Array, SmallVec};
use sp_core::{crypto::UncheckedFrom, ecdsa::Public as ECDSAPublic};
use sp_core::ecdsa::Public as ECDSAPublic;
use sp_io::{crypto::secp256k1_ecdsa_recover_compressed, hashing::blake2_256};
use sp_runtime::traits::{Convert, Hash};
use sp_std::{marker::PhantomData, mem, prelude::*};
Expand Down Expand Up @@ -475,6 +475,8 @@ enum FrameArgs<'a, T: Config, E> {
executable: E,
/// A salt used in the contract address deriviation of the new contract.
salt: &'a [u8],
/// The input data is used in the contract address deriviation of the new contract.
input_data: &'a [u8],
},
}

Expand Down Expand Up @@ -596,7 +598,6 @@ impl<T: Config> CachedContract<T> {
impl<'a, T, E> Stack<'a, T, E>
where
T: Config,
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
E: Executable<T>,
{
/// Create and run a new call stack by calling into `dest`.
Expand Down Expand Up @@ -660,6 +661,7 @@ where
nonce: <Nonce<T>>::get().wrapping_add(1),
executable,
salt,
input_data: input_data.as_ref(),
},
origin,
gas_meter,
Expand Down Expand Up @@ -742,9 +744,13 @@ where

(dest, contract, executable, delegate_caller, ExportedFunction::Call, None)
},
FrameArgs::Instantiate { sender, nonce, executable, salt } => {
let account_id =
<Contracts<T>>::contract_address(&sender, executable.code_hash(), salt);
FrameArgs::Instantiate { sender, nonce, executable, salt, input_data } => {
let account_id = Contracts::<T>::contract_address(
&sender,
executable.code_hash(),
input_data,
salt,
);
let trie_id = Storage::<T>::generate_trie_id(&account_id, nonce);
let contract =
Storage::<T>::new_contract(&account_id, trie_id, *executable.code_hash())?;
Expand Down Expand Up @@ -1080,7 +1086,6 @@ where
impl<'a, T, E> Ext for Stack<'a, T, E>
where
T: Config,
T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
E: Executable<T>,
{
type T = T;
Expand Down Expand Up @@ -1167,6 +1172,7 @@ where
nonce,
executable,
salt,
input_data: input_data.as_ref(),
},
value,
gas_limit,
Expand Down
6 changes: 1 addition & 5 deletions frame/contracts/src/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use frame_support::{
weights::Weight,
DefaultNoBound,
};
use sp_core::crypto::UncheckedFrom;
use sp_runtime::traits::Zero;
use sp_std::marker::PhantomData;

Expand Down Expand Up @@ -86,10 +85,7 @@ pub struct GasMeter<T: Config> {
tokens: Vec<ErasedToken>,
}

impl<T: Config> GasMeter<T>
where
T::AccountId: UncheckedFrom<<T as frame_system::Config>::Hash> + AsRef<[u8]>,
{
impl<T: Config> GasMeter<T> {
pub fn new(gas_limit: Weight) -> Self {
GasMeter {
gas_limit,
Expand Down
Loading

0 comments on commit 69e5b4b

Please sign in to comment.