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

Before we only supported CREATE2 semantics for contract address
derivations. In order to be compatible we also want to allow CREATE1
semantics. We accomplish this to make the salt an `Option` in all places
where it is used. Supplying `None` will use CREATE1 semantics by just
using the deployers account nonce.

## Todo
- [x] Add new tests specific for CREATE1

prdoc/pr_5556.prdoc

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

substrate/bin/node/runtime/src/lib.rs

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

substrate/frame/revive/src/address.rs

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

substrate/frame/revive/src/benchmarking/mod.rs

@@ -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(),
@@ -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() }>,
@@ -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);
@@ -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];
@@ -403,7 +402,7 @@ mod benchmarks {
 			storage_deposit,
 			hash,
 			input,
-			salt,
+			Some(salt),
 		);
 
 		let deposit =
@@ -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();

substrate/frame/revive/src/exec.rs

@@ -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;
@@ -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.
@@ -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],
 	},
@@ -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(
@@ -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),
@@ -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;
@@ -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(_));
@@ -2497,7 +2502,7 @@ mod tests {
 					&mut storage_meter,
 					0, // <- zero value
 					vec![],
-					&[0; 32],
+					Some(&[0; 32]),
 					None,
 				),
 				Err(_)
@@ -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
@@ -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
@@ -2620,7 +2625,7 @@ mod tests {
 						dummy_ch,
 						<Test as Config>::Currency::minimum_balance(),
 						vec![],
-						&[48; 32],
+						Some(&[48; 32]),
 					)
 					.unwrap();
 
@@ -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!"),
@@ -2771,7 +2776,7 @@ mod tests {
 						&mut storage_meter,
 						100,
 						vec![],
-						&[0; 32],
+						Some(&[0; 32]),
 						None,
 					),
 					Err(Error::<Test>::TerminatedInConstructor.into())
@@ -2882,7 +2887,7 @@ mod tests {
 					&mut storage_meter,
 					min_balance,
 					vec![],
-					&[0; 32],
+					Some(&[0; 32]),
 					None,
 				);
 				assert_matches!(result, Ok(_));
@@ -3250,7 +3255,7 @@ mod tests {
 					fail_code,
 					ctx.ext.minimum_balance() * 100,
 					vec![],
-					&[0; 32],
+					Some(&[0; 32]),
 				)
 				.ok();
 			exec_success()
@@ -3267,7 +3272,7 @@ mod tests {
 					success_code,
 					ctx.ext.minimum_balance() * 100,
 					vec![],
-					&[0; 32],
+					Some(&[0; 32]),
 				)
 				.unwrap();
 
@@ -3318,7 +3323,7 @@ mod tests {
 					&mut storage_meter,
 					min_balance * 100,
 					vec![],
-					&[0; 32],
+					Some(&[0; 32]),
 					None,
 				)
 				.ok();
@@ -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);
@@ -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);
@@ -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);

substrate/frame/revive/src/lib.rs

@@ -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>,
@@ -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,
@@ -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),
 			)
 		}
 
@@ -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:
 		///
@@ -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(
@@ -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,
@@ -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),
 			)
 		}
 
@@ -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>> {
@@ -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
@@ -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.