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: 9c4bedda3d843efc8340336b67fb290cffc06f8f
           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/src/benchmarking.rs

@@ -2741,7 +2741,7 @@ 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 gas_used = U256::from(1_000_000);
 
 			for _ in 0..n {
 				// Create real signed transaction with fixed-size input data
@@ -2814,7 +2814,7 @@ 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 gas_used = U256::from(1_000_000);
 
 		for _ in 0..fixed_tx_count {
 			// Create real signed transaction with variable-size input data
@@ -2888,7 +2888,7 @@ mod benchmarks {
 			input_data.clone(),
 		);
 
-		let gas_used = Weight::from_parts(1_000_000, 1000);
+		let gas_used = U256::from(1_000_000);
 
 		// Store transaction
 		let _ = block_storage::bench_with_ethereum_context(|| {
@@ -2950,7 +2950,7 @@ mod benchmarks {
 			input_data.clone(),
 		);
 
-		let gas_used = Weight::from_parts(1_000_000, 1000);
+		let gas_used = U256::from(1_000_000);
 
 		// Store transaction
 		let _ = block_storage::bench_with_ethereum_context(|| {

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,
+		gas_used: U256,
 		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(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(ReceiptGasInfo { gas_used });
 
 		// 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.

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

@@ -15,19 +15,23 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 use crate::{
-	evm::block_hash::{AccumulateReceipt, EthereumBlockBuilder, LogsBloom},
+	dispatch_result,
+	evm::{
+		block_hash::{AccumulateReceipt, EthereumBlockBuilder, LogsBloom},
+		fees::InfoT,
+	},
 	limits,
 	sp_runtime::traits::One,
 	weights::WeightInfo,
-	BlockHash, Config, EthBlockBuilderIR, EthereumBlock, Event, Pallet, ReceiptInfoData,
-	UniqueSaturatedInto, H160, H256,
+	BlockHash, Config, Error, EthBlockBuilderIR, EthereumBlock, Event, Pallet, ReceiptInfoData,
+	UniqueSaturatedInto, Weight, H160, H256,
 };
 use alloc::vec::Vec;
 use environmental::environmental;
 use frame_support::{
+	dispatch::DispatchInfo,
 	pallet_prelude::{DispatchError, DispatchResultWithPostInfo},
 	storage::with_transaction,
-	weights::Weight,
 };
 use sp_core::U256;
 use sp_runtime::TransactionOutcome;
@@ -41,6 +45,57 @@ pub const BLOCK_HASH_COUNT: u32 = 256;
 // that are needed to construct the final transaction receipt.
 environmental!(receipt: AccumulateReceipt);
 
+/// Result of an Ethereum context call execution.
+pub(crate) struct EthereumCallResult {
+	/// The amount of gas used by the call.
+	pub gas_used: U256,
+	/// The dispatch result with post-dispatch information.
+	pub result: DispatchResultWithPostInfo,
+}
+
+impl EthereumCallResult {
+	/// Create a new `EthereumCallResult` from contract execution details.
+	///
+	/// # Parameters
+	///
+	/// - `result`: The execution result
+	/// - `gas_consumed`: The weight consumed during execution
+	/// - `base_call_weight`: The base call weight
+	/// - `encoded_len`: The length of the encoded transaction in bytes
+	/// - `info`: Dispatch information used for fee computation
+	/// - `effective_gas_price`: The EVM gas price
+	pub(crate) fn new<T: Config>(
+		mut result: Result<crate::ExecReturnValue, DispatchError>,
+		mut gas_consumed: Weight,
+		base_call_weight: Weight,
+		encoded_len: u32,
+		info: &DispatchInfo,
+		effective_gas_price: U256,
+	) -> Self {
+		if let Ok(retval) = &result {
+			if retval.did_revert() {
+				result = Err(<Error<T>>::ContractReverted.into());
+			}
+		}
+
+		// Refund pre-charged revert event weight if the call succeeds.
+		if result.is_ok() {
+			gas_consumed.saturating_reduce(T::WeightInfo::deposit_eth_extrinsic_revert_event())
+		}
+
+		let result = dispatch_result(result, gas_consumed, base_call_weight);
+		let native_fee = T::FeeInfo::compute_actual_fee(encoded_len, &info, &result);
+		let result = T::FeeInfo::ensure_not_overdrawn(native_fee, result);
+		let eth_fee = Pallet::<T>::convert_native_to_evm(native_fee);
+		let gas_used = if effective_gas_price.is_zero() {
+			Default::default()
+		} else {
+			eth_fee / effective_gas_price
+		};
+		Self { gas_used, result }
+	}
+}
+
 /// Capture the Ethereum log for the current transaction.
 ///
 /// This method does nothing if called from outside of the ethereum context.
@@ -72,23 +127,19 @@ pub fn bench_with_ethereum_context<R>(f: impl FnOnce() -> R) -> R {
 ///
 /// # Parameters
 /// - transaction_encoded: The RLP encoded transaction bytes.
-/// - call: A closure that executes the transaction logic and returns the gas consumed and result.
+/// - call: A closure that executes the transaction logic and returns an `EthereumCallResult`.
 pub fn with_ethereum_context<T: Config>(
 	transaction_encoded: Vec<u8>,
-	call: impl FnOnce() -> (Weight, DispatchResultWithPostInfo),
+	call: impl FnOnce() -> EthereumCallResult,
 ) -> DispatchResultWithPostInfo {
 	receipt::using(&mut AccumulateReceipt::new(), || {
-		let (err, gas_consumed, mut post_info) =
+		let (err, gas_consumed, post_info) =
 			with_transaction(|| -> TransactionOutcome<Result<_, DispatchError>> {
-				let (gas_consumed, result) = call();
+				let EthereumCallResult { gas_used, result } = call();
 				match result {
-					Ok(post_info) =>
-						TransactionOutcome::Commit(Ok((None, gas_consumed, post_info))),
-					Err(err) => TransactionOutcome::Rollback(Ok((
-						Some(err.error),
-						gas_consumed,
-						err.post_info,
-					))),
+					Ok(post_info) => TransactionOutcome::Commit(Ok((None, gas_used, post_info))),
+					Err(err) =>
+						TransactionOutcome::Rollback(Ok((Some(err.error), gas_used, err.post_info))),
 				}
 			})?;
 
@@ -106,10 +157,6 @@ pub fn with_ethereum_context<T: Config>(
 			deposit_eth_extrinsic_revert_event::<T>(crate::Error::<T>::BenchmarkingError.into());
 
 			crate::block_storage::process_transaction::<T>(transaction_encoded, true, gas_consumed);
-			post_info
-				.actual_weight
-				.as_mut()
-				.map(|w| w.saturating_reduce(T::WeightInfo::deposit_eth_extrinsic_revert_event()));
 			Ok(post_info)
 		}
 	})
@@ -174,11 +221,7 @@ pub fn on_finalize_build_eth_block<T: Config>(
 /// This stores the RLP encoded transaction and receipt details into storage.
 ///
 /// The data is used during the `on_finalize` hook to reconstruct the ETH block.
-pub fn process_transaction<T: Config>(
-	transaction_encoded: Vec<u8>,
-	success: bool,
-	gas_used: Weight,
-) {
+pub fn process_transaction<T: Config>(transaction_encoded: Vec<u8>, success: bool, gas_used: U256) {
 	// Method returns `None` only when called from outside of the ethereum context.
 	// This is not the case here, since this is called from within the
 	// ethereum context.

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

@@ -105,10 +105,18 @@ pub trait InfoT<T: Config>: seal::Sealed {
 		Zero::zero()
 	}
 
-	/// Makes sure that not too much storage deposit was withdrawn.
-	fn ensure_not_overdrawn(
+	/// Compute the actual post_dispatch fee
+	fn compute_actual_fee(
 		_encoded_len: u32,
 		_info: &DispatchInfo,
+		_result: &DispatchResultWithPostInfo,
+	) -> BalanceOf<T> {
+		Default::default()
+	}
+
+	/// Makes sure that not too much storage deposit was withdrawn.
+	fn ensure_not_overdrawn(
+		_fee: BalanceOf<T>,
 		result: DispatchResultWithPostInfo,
 	) -> DispatchResultWithPostInfo {
 		result
@@ -259,9 +267,27 @@ where
 		.saturating_add(Self::length_to_fee(encoded_len))
 	}
 
-	fn ensure_not_overdrawn(
+	fn compute_actual_fee(
 		encoded_len: u32,
 		info: &DispatchInfo,
+		result: &DispatchResultWithPostInfo,
+	) -> BalanceOf<E::Config> {
+		let mut post_info = *match result {
+			Ok(post_info) => post_info,
+			Err(err) => &err.post_info,
+		};
+
+		post_info
+			.actual_weight
+			.as_mut()
+			.map(|w| w.saturating_accrue(info.extension_weight));
+
+		<TxPallet<E::Config>>::compute_actual_fee(encoded_len, info, &post_info, Zero::zero())
+			.into()
+	}
+
+	fn ensure_not_overdrawn(
+		fee: BalanceOf<E::Config>,
 		result: DispatchResultWithPostInfo,
 	) -> DispatchResultWithPostInfo {
 		// if tx is already failing we can ignore
@@ -270,9 +296,6 @@ where
 			return result;
 		};
 
-		let fee: BalanceOf<E::Config> =
-			<TxPallet<E::Config>>::compute_actual_fee(encoded_len, info, &post_info, Zero::zero())
-				.into();
 		let available = Self::remaining_txfee();
 		if fee > available {
 			log::debug!(target: LOG_TARGET, "Drew too much from the txhold. \

substrate/frame/revive/src/lib.rs

@@ -1241,7 +1241,7 @@ pub mod pallet {
 			drop(call);
 
 			block_storage::with_ethereum_context::<T>(transaction_encoded, || {
-				let mut output = Self::bare_instantiate(
+				let output = Self::bare_instantiate(
 					origin,
 					value,
 					gas_limit,
@@ -1256,19 +1256,14 @@ pub mod pallet {
 					),
 				);
 
-				if let Ok(retval) = &output.result {
-					if retval.result.did_revert() {
-						output.result = Err(<Error<T>>::ContractReverted.into());
-					}
-				}
-
-				let result = dispatch_result(
-					output.result.map(|result| result.result),
+				block_storage::EthereumCallResult::new::<T>(
+					output.result.map(|r| r.result),
 					output.gas_consumed,
 					base_info.call_weight,
-				);
-				let result = T::FeeInfo::ensure_not_overdrawn(encoded_len, &info, result);
-				(output.gas_consumed, result)
+					encoded_len,
+					&info,
+					effective_gas_price,
+				)
 			})
 		}
 
@@ -1307,7 +1302,7 @@ pub mod pallet {
 			drop(call);
 
 			block_storage::with_ethereum_context::<T>(transaction_encoded, || {
-				let mut output = Self::bare_call(
+				let output = Self::bare_call(
 					origin,
 					dest,
 					value,
@@ -1321,16 +1316,14 @@ pub mod pallet {
 					),
 				);
 
-				if let Ok(return_value) = &output.result {
-					if return_value.did_revert() {
-						output.result = Err(<Error<T>>::ContractReverted.into());
-					}
-				}
-
-				let result =
-					dispatch_result(output.result, output.gas_consumed, base_info.call_weight);
-				let result = T::FeeInfo::ensure_not_overdrawn(encoded_len, &info, result);
-				(output.gas_consumed, result)
+				block_storage::EthereumCallResult::new::<T>(
+					output.result,
+					output.gas_consumed,
+					base_info.call_weight,
+					encoded_len,
+					&info,
+					effective_gas_price,
+				)
 			})
 		}