contracts: Improve contract address derivation (paritytech#12883)

* 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 <hi@agryaznov.com>

* Don't use T::AdressGenerator directly

* Rename constructor_args to input_data

Co-authored-by: command-bot <>
Co-authored-by: Sasha Gryaznov <hi@agryaznov.com>

bin/node/executor/tests/basic.rs

@@ -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(

frame/contracts/proc-macro/src/lib.rs

@@ -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

frame/contracts/src/benchmarking/code.rs

@@ -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::{
@@ -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 }
 	}
@@ -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();
@@ -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 = {
@@ -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
 }
 

frame/contracts/src/benchmarking/mod.rs

@@ -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.
@@ -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(
@@ -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,
 	}
 
@@ -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
@@ -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);
@@ -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);
@@ -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<_>>();
 
@@ -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;
@@ -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)),

frame/contracts/src/benchmarking/sandbox.rs

@@ -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.
@@ -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 {

frame/contracts/src/chain_extension.rs

@@ -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>;
@@ -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.
 	///
@@ -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() {
@@ -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
@@ -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
@@ -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.

frame/contracts/src/exec.rs

@@ -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::*};
@@ -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],
 	},
 }
 
@@ -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`.
@@ -660,6 +661,7 @@ where
 				nonce: <Nonce<T>>::get().wrapping_add(1),
 				executable,
 				salt,
+				input_data: input_data.as_ref(),
 			},
 			origin,
 			gas_meter,
@@ -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())?;
@@ -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;
@@ -1167,6 +1172,7 @@ where
 				nonce,
 				executable,
 				salt,
+				input_data: input_data.as_ref(),
 			},
 			value,
 			gas_limit,

frame/contracts/src/gas.rs

@@ -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;
 
@@ -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,