revive fix reported gas used

.github/workflows/tests-evm.yml

@@ -43,7 +43,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,15 @@
+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.
+    Fix as well the compute_actual_fee, and introduce a new `compute_actual_fee` in Config::FeeInfo.
+
+    The current calculation was skipping the `extension_weight`
+
+    The updated test here, ensure that the tx cost reported by the receipt match the fees deducted from the user account
+
+    https://github.com/paritytech/evm-test-suite/blob/a1110499ca39610be22645d1f4d7a7484cca0ed7/src/methods.test.ts?plain=1#L272-L264
+crates:
+- name: pallet-revive
+  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

@@ -291,6 +291,10 @@ mod benchmarks {
 		let pallet_account = whitelisted_pallet_account::<T>();
 		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 =
@@ -319,7 +323,7 @@ mod benchmarks {
 			code,
 			input,
 			TransactionSigned::default().signed_payload(),
-			0u32.into(),
+			effective_gas_price,
 			0,
 		);
 
@@ -333,6 +337,9 @@ mod benchmarks {
 		let mapping_deposit =
 			T::Currency::balance_on_hold(&HoldReason::AddressMapping.into(), &caller);
 
+		let pallet_addr = T::AddressMapper::to_address(&pallet_account);
+		let rounding_err = AccountInfoOf::<T>::get(&pallet_addr).unwrap().dust;
+
 		assert_eq!(
 			<T as Config>::FeeInfo::remaining_txfee(),
 			caller_funding::<T>() - deposit - code_deposit - Pallet::<T>::min_balance(),
@@ -341,7 +348,7 @@ mod benchmarks {
 			Pallet::<T>::evm_balance(&deployer),
 			Pallet::<T>::convert_native_to_evm(
 				caller_funding::<T>() - Pallet::<T>::min_balance() - value - mapping_deposit,
-			) - dust,
+			) - dust - rounding_err
 		);
 
 		// contract has the full value
@@ -456,6 +463,10 @@ mod benchmarks {
 		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 =
@@ -478,7 +489,7 @@ mod benchmarks {
 			Weight::MAX,
 			data,
 			TransactionSigned::default().signed_payload(),
-			0u32.into(),
+			effective_gas_price,
 			0,
 		);
 		let deposit = T::Currency::balance_on_hold(
@@ -492,6 +503,10 @@ mod benchmarks {
 		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
+
+		let pallet_addr = T::AddressMapper::to_address(&pallet_account);
+		let rounding_err = AccountInfoOf::<T>::get(&pallet_addr).unwrap().dust;
+
 		assert_eq!(
 			Pallet::<T>::evm_balance(&caller_addr),
 			Pallet::<T>::convert_native_to_evm(
@@ -500,7 +515,7 @@ mod benchmarks {
 					Pallet::<T>::min_balance() -
 					value - deposit - code_deposit -
 					mapping_deposit,
-			) - dust,
+			) - dust - rounding_err
 		);
 
 		// contract should have received the value
@@ -2741,7 +2756,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 +2781,7 @@ mod benchmarks {
 					block_builder.process_transaction(
 						signed_transaction,
 						true,
-						gas_used,
+						receipt_gas_info.clone(),
 						encoded_logs,
 						bloom,
 					);
@@ -2814,7 +2832,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 +2857,7 @@ mod benchmarks {
 				block_builder.process_transaction(
 					signed_transaction,
 					true,
-					gas_used,
+					receipt_gas_info.clone(),
 					encoded_logs,
 					bloom,
 				);
@@ -2888,7 +2909,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 +2924,7 @@ mod benchmarks {
 			block_builder.process_transaction(
 				signed_transaction,
 				true,
-				gas_used,
+				receipt_gas_info.clone(),
 				encoded_logs,
 				bloom,
 			);
@@ -2950,7 +2974,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 +2989,7 @@ mod benchmarks {
 			block_builder.process_transaction(
 				signed_transaction,
 				true,
-				gas_used,
+				receipt_gas_info,
 				encoded_logs,
 				bloom,
 			);

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 {

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

@@ -31,7 +31,7 @@ use crate::{
 use alloc::{vec, vec::Vec};
 
 use codec::{Decode, Encode};
-use frame_support::{weights::Weight, DefaultNoBound};
+use frame_support::DefaultNoBound;
 use scale_info::TypeInfo;
 use sp_core::{keccak_256, H160, H256, U256};
 
@@ -97,7 +97,7 @@ impl<T: crate::Config> EthereumBlockBuilder<T> {
 		&mut self,
 		transaction_encoded: Vec<u8>,
 		success: bool,
-		gas_used: Weight,
+		receipt_gas_info: ReceiptGasInfo,
 		encoded_logs: Vec<u8>,
 		receipt_bloom: LogsBloom,
 	) {
@@ -108,7 +108,7 @@ impl<T: crate::Config> EthereumBlockBuilder<T> {
 		let transaction_type = Self::extract_transaction_type(transaction_encoded.as_slice());
 
 		// Update gas and logs bloom.
-		self.gas_used = self.gas_used.saturating_add(gas_used.ref_time().into());
+		self.gas_used = self.gas_used.saturating_add(receipt_gas_info.gas_used);
 		self.logs_bloom.accrue_bloom(&receipt_bloom);
 
 		// Update the receipt trie.
@@ -120,7 +120,7 @@ impl<T: crate::Config> EthereumBlockBuilder<T> {
 			transaction_type,
 		);
 
-		self.gas_info.push(ReceiptGasInfo { gas_used: gas_used.ref_time().into() });
+		self.gas_info.push(receipt_gas_info);
 
 		// The first transaction and receipt are returned to be stored in the pallet storage.
 		// The index of the incremental hash builders already expects the next items.
@@ -389,15 +389,18 @@ mod test {
 					tx_info.transaction_signed.signed_payload(),
 					logs,
 					receipt_info.status.unwrap_or_default() == 1.into(),
-					receipt_info.gas_used.as_u64(),
+					ReceiptGasInfo {
+						gas_used: receipt_info.gas_used,
+						effective_gas_price: receipt_info.effective_gas_price,
+					},
 				)
 			})
 			.collect();
 
 		ExtBuilder::default().build().execute_with(|| {
 			// Build the ethereum block incrementally.
 			let mut incremental_block = EthereumBlockBuilder::<Test>::default();
-			for (signed, logs, success, gas_used) in transaction_details {
+			for (signed, logs, success, receipt_gas_info) in transaction_details {
 				let mut log_size = 0;
 
 				let mut accumulate_receipt = AccumulateReceipt::new();
@@ -410,7 +413,7 @@ mod test {
 				incremental_block.process_transaction(
 					signed,
 					success,
-					gas_used.into(),
+					receipt_gas_info,
 					accumulate_receipt.encoding,
 					accumulate_receipt.bloom,
 				);