[pallet-revive] update dry-run logic

prdoc/pr_8662.prdoc

@@ -0,0 +1,12 @@
+title: '[pallet-revive] update dry-run logic'
+doc:
+- audience: Runtime Dev
+  description: |-
+    - Revert #8504
+    - Add a `prepare_dry_run` that run before dry_run
+    - Add `dry_run_*` wrapper fn for bare_call / bare_instantiate
+
+    depends on #8652
+crates:
+- name: pallet-revive
+  bump: major

substrate/frame/revive/src/call_builder.rs

@@ -33,8 +33,7 @@ use crate::{
 	transient_storage::MeterEntry,
 	wasm::{PreparedCall, Runtime},
 	BalanceOf, Code, CodeInfoOf, Config, ContractInfo, ContractInfoOf, DepositLimit, Error,
-	GasMeter, MomentOf, NonceAlreadyIncremented, Origin, Pallet as Contracts, PristineCode,
-	WasmBlob, Weight,
+	GasMeter, MomentOf, Origin, Pallet as Contracts, PristineCode, WasmBlob, Weight,
 };
 use alloc::{vec, vec::Vec};
 use frame_support::{storage::child, traits::fungible::Mutate};
@@ -271,7 +270,6 @@ where
 			Code::Upload(module.code),
 			data,
 			salt,
-			NonceAlreadyIncremented::No,
 		);
 
 		let address = outcome.result?.addr;

substrate/frame/revive/src/evm/runtime.rs

@@ -468,7 +468,7 @@ mod test {
 		}
 
 		fn estimate_gas(&mut self) {
-			let dry_run = crate::Pallet::<Test>::bare_eth_transact(
+			let dry_run = crate::Pallet::<Test>::dry_run_eth_transact(
 				self.tx.clone(),
 				Weight::MAX,
 				|call, mut info| {

substrate/frame/revive/src/exec.rs

@@ -26,7 +26,7 @@ use crate::{
 	tracing::if_tracing,
 	transient_storage::TransientStorage,
 	BalanceOf, CodeInfo, CodeInfoOf, Config, ContractInfo, ContractInfoOf, ConversionPrecision,
-	Error, Event, ImmutableData, ImmutableDataOf, NonceAlreadyIncremented, Pallet as Contracts,
+	Error, Event, ImmutableData, ImmutableDataOf, Pallet as Contracts,
 };
 use alloc::vec::Vec;
 use core::{fmt::Debug, marker::PhantomData, mem};
@@ -614,7 +614,6 @@ enum FrameArgs<'a, T: Config, E> {
 		salt: Option<&'a [u8; 32]>,
 		/// The input data is used in the contract address derivation of the new contract.
 		input_data: &'a [u8],
-		nonce_already_incremented: NonceAlreadyIncremented,
 	},
 }
 
@@ -808,15 +807,13 @@ where
 		input_data: Vec<u8>,
 		salt: Option<&[u8; 32]>,
 		skip_transfer: bool,
-		nonce_already_incremented: NonceAlreadyIncremented,
 	) -> Result<(H160, ExecReturnValue), ExecError> {
 		let (mut stack, executable) = Self::new(
 			FrameArgs::Instantiate {
 				sender: origin.clone(),
 				executable,
 				salt,
 				input_data: input_data.as_ref(),
-				nonce_already_incremented,
 			},
 			Origin::from_account_id(origin),
 			gas_meter,
@@ -877,6 +874,7 @@ where
 			storage_meter,
 			BalanceOf::<T>::max_value(),
 			false,
+			true,
 		)?
 		else {
 			return Ok(None);
@@ -910,6 +908,7 @@ where
 		storage_meter: &mut storage::meter::GenericMeter<T, S>,
 		deposit_limit: BalanceOf<T>,
 		read_only: bool,
+		origin_is_caller: bool,
 	) -> Result<Option<(Frame<T>, ExecutableOrPrecompile<T, E, Self>)>, ExecError> {
 		let (account_id, contract_info, executable, delegate, entry_point) = match frame_args {
 			FrameArgs::Call { dest, cached_info, delegated_call } => {
@@ -973,13 +972,7 @@ where
 
 				(dest, contract, executable, delegated_call, ExportedFunction::Call)
 			},
-			FrameArgs::Instantiate {
-				sender,
-				executable,
-				salt,
-				input_data,
-				nonce_already_incremented,
-			} => {
+			FrameArgs::Instantiate { sender, executable, salt, input_data } => {
 				let deployer = T::AddressMapper::to_address(&sender);
 				let account_nonce = <System<T>>::account_nonce(&sender);
 				let address = if let Some(salt) = salt {
@@ -990,7 +983,7 @@ where
 						&deployer,
 						// the Nonce from the origin has been incremented pre-dispatch, so we
 						// need to subtract 1 to get the nonce at the time of the call.
-						if matches!(nonce_already_incremented, NonceAlreadyIncremented::Yes) {
+						if origin_is_caller {
 							account_nonce.saturating_sub(1u32.into()).saturated_into()
 						} else {
 							account_nonce.saturated_into()
@@ -1066,6 +1059,7 @@ where
 			nested_storage,
 			deposit_limit,
 			read_only,
+			false,
 		)? {
 			self.frames.try_push(frame).map_err(|_| Error::<T>::MaxCallDepthReached)?;
 			Ok(Some(executable))
@@ -1671,7 +1665,6 @@ where
 				executable,
 				salt,
 				input_data: input_data.as_ref(),
-				nonce_already_incremented: NonceAlreadyIncremented::No,
 			},
 			value.try_into().map_err(|_| Error::<T>::BalanceConversionFailed)?,
 			gas_limit,

substrate/frame/revive/src/exec/tests.rs

@@ -566,7 +566,6 @@ fn input_data_to_instantiate() {
 				vec![1, 2, 3, 4],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			);
 			assert_matches!(result, Ok(_));
 		});
@@ -1070,7 +1069,6 @@ fn refuse_instantiate_with_value_below_existential_deposit() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			),
 			Err(_)
 		);
@@ -1106,7 +1104,6 @@ fn instantiation_work_with_success_output() {
 					vec![],
 					Some(&[0 ;32]),
 					false,
-					NonceAlreadyIncremented::Yes,
 				),
 				Ok((address, ref output)) if output.data == vec![80, 65, 83, 83] => address
 			);
@@ -1153,7 +1150,6 @@ fn instantiation_fails_with_failing_output() {
 					vec![],
 					Some(&[0; 32]),
 					false,
-					NonceAlreadyIncremented::Yes,
 				),
 				Ok((address, ref output)) if output.data == vec![70, 65, 73, 76] => address
 			);
@@ -1317,7 +1313,6 @@ fn termination_from_instantiate_fails() {
 					vec![],
 					Some(&[0; 32]),
 					false,
-					NonceAlreadyIncremented::Yes,
 				),
 				Err(ExecError {
 					error: Error::<Test>::TerminatedInConstructor.into(),
@@ -1446,7 +1441,6 @@ fn recursive_call_during_constructor_is_balance_transfer() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			);
 			assert_matches!(result, Ok(_));
 		});
@@ -1808,7 +1802,6 @@ fn nonce() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			)
 			.ok();
 			assert_eq!(System::account_nonce(&ALICE), 0);
@@ -1822,7 +1815,6 @@ fn nonce() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			));
 			assert_eq!(System::account_nonce(&ALICE), 1);
 
@@ -1835,7 +1827,6 @@ fn nonce() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			));
 			assert_eq!(System::account_nonce(&ALICE), 2);
 
@@ -1848,7 +1839,6 @@ fn nonce() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			));
 			assert_eq!(System::account_nonce(&ALICE), 3);
 		});
@@ -2835,7 +2825,6 @@ fn immutable_data_set_overrides() {
 				vec![],
 				None,
 				false,
-				NonceAlreadyIncremented::Yes,
 			)
 			.unwrap()
 			.0;

substrate/frame/revive/src/lib.rs

@@ -518,18 +518,6 @@ pub mod pallet {
 		AddressMapping,
 	}
 
-	#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-	pub enum NonceAlreadyIncremented {
-		/// Indicates that the nonce has not been incremented yet.
-		///
-		/// This happens when the instantiation is triggered by a dry-run or another contract.
-		No,
-		/// Indicates that the nonce has already been incremented.
-		///
-		/// This happens when the instantiation is triggered by a transaction.
-		Yes,
-	}
-
 	/// A mapping from a contract's code hash to its code.
 	#[pallet::storage]
 	pub(crate) type PristineCode<T: Config> = StorageMap<_, Identity, H256, CodeVec>;
@@ -814,7 +802,6 @@ pub mod pallet {
 				Code::Existing(code_hash),
 				data,
 				salt,
-				NonceAlreadyIncremented::Yes,
 			);
 			if let Ok(retval) = &output.result {
 				if retval.result.did_revert() {
@@ -879,7 +866,6 @@ pub mod pallet {
 				Code::Upload(code),
 				data,
 				salt,
-				NonceAlreadyIncremented::Yes,
 			);
 			if let Ok(retval) = &output.result {
 				if retval.result.did_revert() {
@@ -1081,6 +1067,11 @@ where
 		}
 	}
 
+	/// Prepare a dry run for the given account.
+	fn prepare_dry_run(account: &T::AccountId) {
+		frame_system::Pallet::<T>::inc_account_nonce(account);
+	}
+
 	/// A generalized version of [`Self::instantiate`] or [`Self::instantiate_with_code`].
 	///
 	/// Identical to [`Self::instantiate`] or [`Self::instantiate_with_code`] but tailored towards
@@ -1094,7 +1085,6 @@ where
 		code: Code,
 		data: Vec<u8>,
 		salt: Option<[u8; 32]>,
-		nonce_already_incremented: NonceAlreadyIncremented,
 	) -> ContractResult<InstantiateReturnValue, BalanceOf<T>> {
 		let mut gas_meter = GasMeter::new(gas_limit);
 		let mut storage_deposit = Default::default();
@@ -1137,7 +1127,6 @@ where
 				data,
 				salt.as_ref(),
 				unchecked_deposit_limit,
-				nonce_already_incremented,
 			);
 			storage_deposit = storage_meter
 				.try_into_deposit(&instantiate_origin, unchecked_deposit_limit)?
@@ -1155,14 +1144,45 @@ where
 		}
 	}
 
-	/// A version of [`Self::eth_transact`] used to dry-run Ethereum calls.
+	/// dry-run a contract call.
+	pub fn dry_run_call(
+		origin: OriginFor<T>,
+		dest: H160,
+		value: BalanceOf<T>,
+		gas_limit: Weight,
+		storage_deposit_limit: DepositLimit<BalanceOf<T>>,
+		data: Vec<u8>,
+	) -> ContractResult<ExecReturnValue, BalanceOf<T>> {
+		let account_id = ensure_signed(origin.clone())
+			.unwrap_or_else(|_| T::AddressMapper::to_account_id(&H160::default()));
+		Self::prepare_dry_run(&account_id);
+		Self::bare_call(origin, dest, value, gas_limit, storage_deposit_limit, data)
+	}
+
+	/// dry-run instantiate a contract deployment.
+	pub fn dry_run_instantiate(
+		origin: OriginFor<T>,
+		value: BalanceOf<T>,
+		gas_limit: Weight,
+		storage_deposit_limit: DepositLimit<BalanceOf<T>>,
+		code: Code,
+		data: Vec<u8>,
+		salt: Option<[u8; 32]>,
+	) -> ContractResult<InstantiateReturnValue, BalanceOf<T>> {
+		let account_id = ensure_signed(origin.clone())
+			.unwrap_or_else(|_| T::AddressMapper::to_account_id(&H160::default()));
+		Self::prepare_dry_run(&account_id);
+		Self::bare_instantiate(origin, value, gas_limit, storage_deposit_limit, code, data, salt)
+	}
+
+	/// Dry-run Ethereum calls.
 	///
 	/// # Parameters
 	///
 	/// - `tx`: The Ethereum transaction to simulate.
 	/// - `gas_limit`: The gas limit enforced during contract execution.
 	/// - `tx_fee`: A function that returns the fee for the given call and dispatch info.
-	pub fn bare_eth_transact(
+	pub fn dry_run_eth_transact(
 		mut tx: GenericTransaction,
 		gas_limit: Weight,
 		tx_fee: impl Fn(Call<T>, DispatchInfo) -> BalanceOf<T>,
@@ -1175,10 +1195,11 @@ where
 		T::Nonce: Into<U256>,
 		T::Hash: frame_support::traits::IsType<H256>,
 	{
-		log::trace!(target: LOG_TARGET, "bare_eth_transact: tx: {tx:?} gas_limit: {gas_limit:?}");
+		log::trace!(target: LOG_TARGET, "dry_run_eth_transact: {tx:?} gas_limit: {gas_limit:?}");
 
 		let from = tx.from.unwrap_or_default();
 		let origin = T::AddressMapper::to_account_id(&from);
+		Self::prepare_dry_run(&origin);
 
 		let storage_deposit_limit = if tx.gas.is_some() {
 			DepositLimit::Balance(BalanceOf::<T>::max_value())
@@ -1307,7 +1328,6 @@ where
 					Code::Upload(code.to_vec()),
 					data.to_vec(),
 					None,
-					NonceAlreadyIncremented::No,
 				);
 
 				let returned_data = match result.result {
@@ -1579,7 +1599,7 @@ sp_api::decl_runtime_apis! {
 
 		/// Perform an Ethereum call.
 		///
-		/// See [`crate::Pallet::bare_eth_transact`]
+		/// See [`crate::Pallet::dry_run_eth_transact`]
 		fn eth_transact(tx: GenericTransaction) -> Result<EthTransactInfo<Balance>, EthTransactError>;
 
 		/// Upload new code without instantiating a contract from it.
@@ -1702,7 +1722,7 @@ macro_rules! impl_runtime_apis_plus_revive {
 
 					let blockweights: $crate::BlockWeights =
 						<Self as $crate::frame_system::Config>::BlockWeights::get();
-					$crate::Pallet::<Self>::bare_eth_transact(tx, blockweights.max_block, tx_fee)
+					$crate::Pallet::<Self>::dry_run_eth_transact(tx, blockweights.max_block, tx_fee)
 				}
 
 				fn call(
@@ -1719,7 +1739,7 @@ macro_rules! impl_runtime_apis_plus_revive {
 
 					let origin =
 						<Self as $crate::frame_system::Config>::RuntimeOrigin::signed(origin);
-					$crate::Pallet::<Self>::bare_call(
+					$crate::Pallet::<Self>::dry_run_call(
 						origin,
 						dest,
 						value,
@@ -1744,15 +1764,14 @@ macro_rules! impl_runtime_apis_plus_revive {
 
 					let origin =
 						<Self as $crate::frame_system::Config>::RuntimeOrigin::signed(origin);
-					$crate::Pallet::<Self>::bare_instantiate(
+					$crate::Pallet::<Self>::dry_run_instantiate(
 						origin,
 						value,
 						gas_limit.unwrap_or(blockweights.max_block),
 						$crate::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)),
 						code,
 						data,
 						salt,
-						$crate::NonceAlreadyIncremented::No,
 					)
 				}