revive fix reported gas used (#10148)

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

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: pgherveou

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