revive fix reported gas used

.github/workflows/tests-evm.yml

@@ -99,7 +99,7 @@ jobs:
         uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
         with:
           repository: paritytech/evm-test-suite
-          ref: c2422cace2fb8a4337fc1c704c49e458c8a79d6b
+          ref: 460b2c9aa3a3019d3508bb5a34a2498ea86035ff
           path: evm-test-suite
 
       - uses: denoland/setup-deno@v1

prdoc/pr_10148.prdoc

@@ -0,0 +1,23 @@
+title: revive fix reported gas used
+doc:
+- audience: Runtime Dev
+  description: |-
+    Fix `gas_used` calculation introduced in #9418 to use the actual gas instead of just `ref_time`.
+
+    With these changes we now guarantee that `tx_cost = effective_gas_price * gas`.
+    Note that since we compute gas as `fee / gas_price`, this can lead to rounding errors when the chain uses `SlowAdjustingFeeUpdate` (i.e. the fee is not a multiple of the gas price).
+    The changes in this PR ensure the fee still matches by burning the rounding remainder.
+
+    This PR also fixes how the actual fee is computed and introduces a new `compute_actual_fee` in `Config::FeeInfo`.
+    The previous fee calculation was skipping the `extension_weight` in the fee calculation.
+
+    The updated tests ensure that the tx cost reported in the receipt matches the fees deducted from the user account:
+
+    https://github.com/paritytech/evm-test-suite/blob/460b2c9aa3a3019d3508bb5a34a2498ea86035ff/src/gas.test.ts?plain=1#L31-L61
+crates:
+- name: pallet-revive
+  bump: patch
+- name: revive-dev-runtime
+  bump: patch
+- name: pallet-revive-eth-rpc
+  bump: patch

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

@@ -17,7 +17,6 @@
 use crate::{
 	client::{runtime_api::RuntimeApi, SubstrateBlock, SubstrateBlockNumber},
 	subxt_client::{
-		self,
 		revive::{
 			calls::types::EthTransact,
 			events::{ContractEmitted, EthExtrinsicRevert},
@@ -36,10 +35,6 @@ use sp_core::keccak_256;
 use std::{future::Future, pin::Pin, sync::Arc};
 use subxt::{blocks::ExtrinsicDetails, OnlineClient};
 
-type FetchGasPriceFn = Arc<
-	dyn Fn(H256) -> Pin<Box<dyn Future<Output = Result<U256, ClientError>> + Send>> + Send + Sync,
->;
-
 type FetchReceiptDataFn = Arc<
 	dyn Fn(H256) -> Pin<Box<dyn Future<Output = Option<Vec<ReceiptGasInfo>>> + Send>> + Send + Sync,
 >;
@@ -58,9 +53,6 @@ pub struct ReceiptExtractor {
 	/// Fetch ethereum block hash.
 	fetch_eth_block_hash: FetchEthBlockHashFn,
 
-	/// Fetch the gas price from the chain.
-	fetch_gas_price: FetchGasPriceFn,
-
 	/// Earliest block number to consider when searching for transaction receipts.
 	earliest_receipt_block: Option<SubstrateBlockNumber>,
 
@@ -109,20 +101,6 @@ impl ReceiptExtractor {
 			Box::pin(fut) as Pin<Box<_>>
 		});
 
-		let api_inner = api.clone();
-		let fetch_gas_price = Arc::new(move |block_hash| {
-			let api_inner = api_inner.clone();
-
-			let fut = async move {
-				let runtime_api = api_inner.runtime_api().at(block_hash);
-				let payload = subxt_client::apis().revive_api().gas_price();
-				let base_gas_price = runtime_api.call(payload).await?;
-				Ok(*base_gas_price)
-			};
-
-			Box::pin(fut) as Pin<Box<_>>
-		});
-
 		let api_inner = api.clone();
 		let fetch_receipt_data = Arc::new(move |block_hash| {
 			let api_inner = api_inner.clone();
@@ -138,7 +116,6 @@ impl ReceiptExtractor {
 		Ok(Self {
 			fetch_receipt_data,
 			fetch_eth_block_hash,
-			fetch_gas_price,
 			earliest_receipt_block,
 			recover_eth_address: recover_eth_address_fn,
 		})
@@ -154,13 +131,10 @@ impl ReceiptExtractor {
 			let eth_block_hash = H256::from(keccak_256(&bytes));
 			Box::pin(std::future::ready(Some(eth_block_hash))) as Pin<Box<_>>
 		});
-		let fetch_gas_price =
-			Arc::new(|_| Box::pin(std::future::ready(Ok(U256::from(1000)))) as Pin<Box<_>>);
 
 		Self {
 			fetch_receipt_data,
 			fetch_eth_block_hash,
-			fetch_gas_price,
 			earliest_receipt_block: None,
 			recover_eth_address: Arc::new(|signed_tx: &TransactionSigned| {
 				signed_tx.recover_eth_address()
@@ -197,11 +171,11 @@ impl ReceiptExtractor {
 			ClientError::RecoverEthAddressFailed
 		})?;
 
-		let base_gas_price = (self.fetch_gas_price)(substrate_block.hash()).await?;
-		let tx_info =
-			GenericTransaction::from_signed(signed_tx.clone(), base_gas_price, Some(from));
-
-		let gas_price = tx_info.gas_price.unwrap_or_default();
+		let tx_info = GenericTransaction::from_signed(
+			signed_tx.clone(),
+			receipt_gas_info.effective_gas_price,
+			Some(from),
+		);
 
 		// get logs from ContractEmitted event
 		let logs = events
@@ -244,7 +218,7 @@ impl ReceiptExtractor {
 			from,
 			logs,
 			tx_info.to,
-			gas_price,
+			receipt_gas_info.effective_gas_price,
 			U256::from(receipt_gas_info.gas_used),
 			success,
 			transaction_hash,

substrate/frame/revive/src/benchmarking.rs

@@ -287,10 +287,13 @@ mod benchmarks {
 		c: Linear<0, { 100 * 1024 }>,
 		i: Linear<0, { limits::CALLDATA_BYTES }>,
 		d: Linear<0, 1>,
-	) {
-		let pallet_account = whitelisted_pallet_account::<T>();
+	) -> Result<(), BenchmarkError> {
 		let input = vec![42u8; i as usize];
 
+		// Use an `effective_gas_price` that is not a multiple of `T::NativeToEthRatio`
+		// to hit the code that charge the rounding error so that tx_cost == effective_gas_price *
+		// gas_used
+		let effective_gas_price = Pallet::<T>::evm_base_fee() + 1;
 		let value = Pallet::<T>::min_balance();
 		let dust = 42u32 * d;
 		let evm_value =
@@ -303,7 +306,6 @@ mod benchmarks {
 		let deployer = T::AddressMapper::to_address(&caller);
 		let nonce = System::<T>::account_nonce(&caller).try_into().unwrap_or_default();
 		let addr = crate::address::create1(&deployer, nonce);
-		let account_id = T::AddressMapper::to_fallback_account_id(&addr);
 
 		assert!(AccountInfoOf::<T>::get(&deployer).is_none());
 
@@ -319,33 +321,13 @@ mod benchmarks {
 			code,
 			input,
 			TransactionSigned::default().signed_payload(),
-			0u32.into(),
+			effective_gas_price,
 			0,
 		);
 
-		let deposit =
-			T::Currency::balance_on_hold(&HoldReason::StorageDepositReserve.into(), &account_id);
-		// uploading the code reserves some balance in the pallet account
-		let code_deposit = T::Currency::balance_on_hold(
-			&HoldReason::CodeUploadDepositReserve.into(),
-			&pallet_account,
-		);
-		let mapping_deposit =
-			T::Currency::balance_on_hold(&HoldReason::AddressMapping.into(), &caller);
-
-		assert_eq!(
-			<T as Config>::FeeInfo::remaining_txfee(),
-			caller_funding::<T>() - deposit - code_deposit - Pallet::<T>::min_balance(),
-		);
-		assert_eq!(
-			Pallet::<T>::evm_balance(&deployer),
-			Pallet::<T>::convert_native_to_evm(
-				caller_funding::<T>() - Pallet::<T>::min_balance() - value - mapping_deposit,
-			) - dust,
-		);
-
 		// contract has the full value
 		assert_eq!(Pallet::<T>::evm_balance(&addr), evm_value);
+		Ok(())
 	}
 
 	#[benchmark(pov_mode = Measured)]
@@ -451,11 +433,14 @@ mod benchmarks {
 	// `d`: with or without dust value to transfer
 	#[benchmark(pov_mode = Measured)]
 	fn eth_call(d: Linear<0, 1>) -> Result<(), BenchmarkError> {
-		let pallet_account = whitelisted_pallet_account::<T>();
 		let data = vec![42u8; 1024];
 		let instance =
 			Contract::<T>::with_caller(whitelisted_caller(), VmBinaryModule::dummy(), vec![])?;
 
+		// Use an `effective_gas_price` that is not a multiple of `T::NativeToEthRatio`
+		// to hit the code that charge the rounding error so that tx_cost == effective_gas_price *
+		// gas_used
+		let effective_gas_price = Pallet::<T>::evm_base_fee() + 1;
 		let value = Pallet::<T>::min_balance();
 		let dust = 42u32 * d;
 		let evm_value =
@@ -466,7 +451,6 @@ mod benchmarks {
 			<T as Config>::Currency::issue(caller_funding::<T>()),
 		);
 
-		let caller_addr = T::AddressMapper::to_address(&instance.caller);
 		let origin = Origin::EthTransaction(instance.caller.clone());
 		let before = Pallet::<T>::evm_balance(&instance.address);
 
@@ -478,30 +462,9 @@ mod benchmarks {
 			Weight::MAX,
 			data,
 			TransactionSigned::default().signed_payload(),
-			0u32.into(),
+			effective_gas_price,
 			0,
 		);
-		let deposit = T::Currency::balance_on_hold(
-			&HoldReason::StorageDepositReserve.into(),
-			&instance.account_id,
-		);
-		let code_deposit = T::Currency::balance_on_hold(
-			&HoldReason::CodeUploadDepositReserve.into(),
-			&pallet_account,
-		);
-		let mapping_deposit =
-			T::Currency::balance_on_hold(&HoldReason::AddressMapping.into(), &instance.caller);
-		// value and value transferred via call should be removed from the caller
-		assert_eq!(
-			Pallet::<T>::evm_balance(&caller_addr),
-			Pallet::<T>::convert_native_to_evm(
-				caller_funding::<T>() -
-					Pallet::<T>::min_balance() -
-					Pallet::<T>::min_balance() -
-					value - deposit - code_deposit -
-					mapping_deposit,
-			) - dust,
-		);
 
 		// contract should have received the value
 		assert_eq!(Pallet::<T>::evm_balance(&instance.address), before + evm_value);
@@ -2741,7 +2704,10 @@ mod benchmarks {
 
 			// Create input data of fixed size for consistent transaction payloads
 			let input_data = vec![0x42u8; fixed_payload_size];
-			let gas_used = Weight::from_parts(1_000_000, 1000);
+			let receipt_gas_info = ReceiptGasInfo {
+				gas_used: U256::from(1_000_000),
+				effective_gas_price: Pallet::<T>::evm_base_fee(),
+			};
 
 			for _ in 0..n {
 				// Create real signed transaction with fixed-size input data
@@ -2763,7 +2729,7 @@ mod benchmarks {
 					block_builder.process_transaction(
 						signed_transaction,
 						true,
-						gas_used,
+						receipt_gas_info.clone(),
 						encoded_logs,
 						bloom,
 					);
@@ -2814,7 +2780,10 @@ mod benchmarks {
 
 		// Create input data of variable size p for realistic transaction payloads
 		let input_data = vec![0x42u8; d as usize];
-		let gas_used = Weight::from_parts(1_000_000, 1000);
+		let receipt_gas_info = ReceiptGasInfo {
+			gas_used: U256::from(1_000_000),
+			effective_gas_price: Pallet::<T>::evm_base_fee(),
+		};
 
 		for _ in 0..fixed_tx_count {
 			// Create real signed transaction with variable-size input data
@@ -2836,7 +2805,7 @@ mod benchmarks {
 				block_builder.process_transaction(
 					signed_transaction,
 					true,
-					gas_used,
+					receipt_gas_info.clone(),
 					encoded_logs,
 					bloom,
 				);
@@ -2888,7 +2857,10 @@ mod benchmarks {
 			input_data.clone(),
 		);
 
-		let gas_used = Weight::from_parts(1_000_000, 1000);
+		let receipt_gas_info = ReceiptGasInfo {
+			gas_used: U256::from(1_000_000),
+			effective_gas_price: Pallet::<T>::evm_base_fee(),
+		};
 
 		// Store transaction
 		let _ = block_storage::bench_with_ethereum_context(|| {
@@ -2900,7 +2872,7 @@ mod benchmarks {
 			block_builder.process_transaction(
 				signed_transaction,
 				true,
-				gas_used,
+				receipt_gas_info.clone(),
 				encoded_logs,
 				bloom,
 			);
@@ -2950,7 +2922,10 @@ mod benchmarks {
 			input_data.clone(),
 		);
 
-		let gas_used = Weight::from_parts(1_000_000, 1000);
+		let receipt_gas_info = ReceiptGasInfo {
+			gas_used: U256::from(1_000_000),
+			effective_gas_price: Pallet::<T>::evm_base_fee(),
+		};
 
 		// Store transaction
 		let _ = block_storage::bench_with_ethereum_context(|| {
@@ -2962,7 +2937,7 @@ mod benchmarks {
 			block_builder.process_transaction(
 				signed_transaction,
 				true,
-				gas_used,
+				receipt_gas_info,
 				encoded_logs,
 				bloom,
 			);

substrate/frame/revive/src/evm.rs

@@ -35,4 +35,8 @@ pub use block_hash::ReceiptGasInfo;
 /// Ethereum block storage module.
 pub(crate) mod block_storage;
 
+/// Transfer with dust functionality.
+mod transfer_with_dust;
+pub(crate) use transfer_with_dust::*;
+
 type OnChargeTransactionBalanceOf<T> = <<T as pallet_transaction_payment::Config>::OnChargeTransaction as pallet_transaction_payment::OnChargeTransaction<T>>::Balance;

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

@@ -42,6 +42,9 @@ use sp_core::{H256, U256};
 pub struct ReceiptGasInfo {
 	/// The amount of gas used for this specific transaction alone.
 	pub gas_used: U256,
+
+	/// The effective gas price for this transaction.
+	pub effective_gas_price: U256,
 }
 
 impl Block {