[pallet-revive] update dry-run logic

cumulus/parachains/runtimes/assets/asset-hub-westend/tests/tests.rs

@@ -50,7 +50,7 @@ use frame_support::{
 	weights::{Weight, WeightToFee as WeightToFeeT},
 };
 use hex_literal::hex;
-use pallet_revive::{Code, DepositLimit, InstantiateReturnValue, NonceAlreadyIncremented};
+use pallet_revive::{Code, DepositLimit, InstantiateReturnValue};
 use pallet_revive_fixtures::compile_module;
 use parachains_common::{AccountId, AssetIdForTrustBackedAssets, AuraId, Balance};
 use sp_consensus_aura::SlotDuration;
@@ -1514,7 +1514,6 @@ fn withdraw_and_deposit_erc20s() {
 			Code::Upload(code),
 			constructor_data,
 			None,
-			NonceAlreadyIncremented::Yes,
 		);
 		let Ok(InstantiateReturnValue { addr: erc20_address, .. }) = result.result else {
 			unreachable!("contract should initialize")
@@ -1628,7 +1627,6 @@ fn smart_contract_not_erc20_will_error() {
 			Code::Upload(code),
 			Vec::new(),
 			None,
-			NonceAlreadyIncremented::Yes,
 		);
 		let Ok(InstantiateReturnValue { addr: non_erc20_address, .. }) = result.result else {
 			unreachable!("contract should initialize")
@@ -1693,7 +1691,6 @@ fn smart_contract_does_not_return_bool_fails() {
 			Code::Upload(code),
 			constructor_data,
 			None,
-			NonceAlreadyIncremented::Yes,
 		);
 		let Ok(InstantiateReturnValue { addr: non_erc20_address, .. }) = result.result else {
 			unreachable!("contract should initialize")
@@ -1756,7 +1753,6 @@ fn expensive_erc20_runs_out_of_gas() {
 			Code::Upload(code),
 			constructor_data,
 			None,
-			NonceAlreadyIncremented::Yes,
 		);
 		let Ok(InstantiateReturnValue { addr: non_erc20_address, .. }) = result.result else {
 			unreachable!("contract should initialize")

prdoc/pr_8662.prdoc

@@ -0,0 +1,10 @@
+title: '[pallet-revive] update dry-run logic'
+doc:
+- audience: Runtime Dev
+  description: |-
+    - Revert #8504
+    - Add a `prepare_dry_run` that run before dry_run
+
+crates:
+- name: pallet-revive
+  bump: major

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

@@ -21,6 +21,8 @@
 use crate::{
 	cli::{self, CliCommand},
 	example::TransactionBuilder,
+	subxt_client,
+	subxt_client::{src_chain::runtime_types::pallet_revive::primitives::Code, SrcChainConfig},
 	EthRpcClient,
 };
 use clap::Parser;
@@ -32,6 +34,7 @@ use pallet_revive::{
 use static_init::dynamic;
 use std::{sync::Arc, thread};
 use substrate_cli_test_utils::*;
+use subxt::OnlineClient;
 
 /// Create a websocket client with a 120s timeout.
 async fn ws_client_with_retry(url: &str) -> WsClient {
@@ -141,7 +144,6 @@ async fn deploy_and_call() -> anyhow::Result<()> {
 	// Balance transfer
 	let ethan = Account::from(subxt_signer::eth::dev::ethan());
 	let initial_balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?;
-
 	let value = 1_000_000_000_000_000_000_000u128.into();
 	let tx = TransactionBuilder::new(&client).value(value).to(ethan.address()).send().await?;
 
@@ -218,6 +220,37 @@ async fn deploy_and_call() -> anyhow::Result<()> {
 	Ok(())
 }
 
+#[tokio::test]
+async fn runtime_api_dry_run_addr_works() -> anyhow::Result<()> {
+	let _lock = SHARED_RESOURCES.write();
+	let client = std::sync::Arc::new(SharedResources::client().await);
+
+	let account = Account::default();
+	let origin: [u8; 32] = account.substrate_account().into();
+	let data = b"hello world".to_vec();
+	let value = 5_000_000_000_000u128;
+	let (bytes, _) = pallet_revive_fixtures::compile_module("dummy")?;
+
+	let payload = subxt_client::apis().revive_api().instantiate(
+		subxt::utils::AccountId32(origin),
+		value,
+		None,
+		None,
+		Code::Upload(bytes),
+		data,
+		None,
+	);
+
+	let nonce = client.get_transaction_count(account.address(), BlockTag::Latest.into()).await?;
+	let contract_address = create1(&account.address(), nonce.try_into().unwrap());
+
+	let c = OnlineClient::<SrcChainConfig>::from_url("ws://localhost:45789").await?;
+	let res = c.runtime_api().at_latest().await?.call(payload).await?.result.unwrap();
+
+	assert_eq!(res.addr, contract_address);
+	Ok(())
+}
+
 #[tokio::test]
 async fn invalid_transaction() -> anyhow::Result<()> {
 	let _lock = SHARED_RESOURCES.write();

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};
@@ -619,7 +619,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,
 	},
 }
 
@@ -813,15 +812,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,
@@ -882,6 +879,7 @@ where
 			storage_meter,
 			BalanceOf::<T>::max_value(),
 			false,
+			true,
 		)?
 		else {
 			return Ok(None);
@@ -915,6 +913,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 } => {
@@ -978,13 +977,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 {
@@ -995,7 +988,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()
@@ -1071,6 +1064,7 @@ where
 			nested_storage,
 			deposit_limit,
 			read_only,
+			false,
 		)? {
 			self.frames.try_push(frame).map_err(|_| Error::<T>::MaxCallDepthReached)?;
 			Ok(Some(executable))
@@ -1680,7 +1674,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(_));
 		});
@@ -1069,7 +1068,6 @@ fn refuse_instantiate_with_value_below_existential_deposit() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			),
 			Err(_)
 		);
@@ -1103,7 +1101,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
 			);
@@ -1148,7 +1145,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
 			);
@@ -1309,7 +1305,6 @@ fn termination_from_instantiate_fails() {
 					vec![],
 					Some(&[0; 32]),
 					false,
-					NonceAlreadyIncremented::Yes,
 				),
 				Err(ExecError {
 					error: Error::<Test>::TerminatedInConstructor.into(),
@@ -1436,7 +1431,6 @@ fn recursive_call_during_constructor_is_balance_transfer() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			);
 			assert_matches!(result, Ok(_));
 		});
@@ -1795,7 +1789,6 @@ fn nonce() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			)
 			.ok();
 			assert_eq!(System::account_nonce(&ALICE), 0);
@@ -1809,7 +1802,6 @@ fn nonce() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			));
 			assert_eq!(System::account_nonce(&ALICE), 1);
 
@@ -1822,7 +1814,6 @@ fn nonce() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			));
 			assert_eq!(System::account_nonce(&ALICE), 2);
 
@@ -1835,7 +1826,6 @@ fn nonce() {
 				vec![],
 				Some(&[0; 32]),
 				false,
-				NonceAlreadyIncremented::Yes,
 			));
 			assert_eq!(System::account_nonce(&ALICE), 3);
 		});
@@ -2815,7 +2805,6 @@ fn immutable_data_set_overrides() {
 				vec![],
 				None,
 				false,
-				NonceAlreadyIncremented::Yes,
 			)
 			.unwrap()
 			.0;