[pallet-revive] Update gas encoding (paritytech#6689)

Update the current approach to attach the `ref_time`, `pov` and
`deposit` parameters to an Ethereum transaction.
Previously we will pass these 3 parameters along with the signed
payload, and check that the fees resulting from `gas x gas_price` match
the actual fees paid by the user for the extrinsic.

This approach unfortunately can be attacked. A malicious actor could
force such a transaction to fail by injecting low values for some of
these extra parameters as they are not part of the signed payload.

The new approach encodes these 3 extra parameters in the lower digits of
the transaction gas, approximating the the log2 of the actual values to
encode each components on 2 digits

---------

Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: command-bot <>

cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs

@@ -981,6 +981,7 @@ impl pallet_revive::Config for Runtime {
 	type Xcm = pallet_xcm::Pallet<Self>;
 	type ChainId = ConstU64<420_420_421>;
 	type NativeToEthRatio = ConstU32<1_000_000>; // 10^(18 - 12) Eth is 10^18, Native is 10^12.
+	type EthGasEncoder = ();
 }
 
 impl TryFrom<RuntimeCall> for pallet_revive::Call<Runtime> {

prdoc/pr_6689.prdoc

@@ -0,0 +1,19 @@
+title: '[pallet-revive] Update gas encoding'
+doc:
+- audience: Runtime Dev
+  description: |-
+    Update the current approach to attach the `ref_time`, `pov` and `deposit` parameters to an Ethereum transaction.
+Previously, these three parameters were passed along with the signed payload, and the fees resulting from gas × gas_price were checked to ensure they matched the actual fees paid by the user for the extrinsic
+
+    This approach unfortunately can be attacked. A malicious actor could force such a transaction to fail by injecting low values for some of these extra parameters as they are not part of the signed payload.
+
+    The new approach encodes these 3 extra parameters in the lower digits of the transaction gas, using the log2 of the actual values to  encode each components on 2 digits
+crates:
+- name: pallet-revive-eth-rpc
+  bump: minor
+- name: pallet-revive
+  bump: minor
+- name: asset-hub-westend-runtime
+  bump: minor
+- name: pallet-revive-mock-network
+  bump: minor

substrate/bin/node/runtime/src/lib.rs

@@ -1468,6 +1468,7 @@ impl pallet_revive::Config for Runtime {
 	type Xcm = ();
 	type ChainId = ConstU64<420_420_420>;
 	type NativeToEthRatio = ConstU32<1_000_000>; // 10^(18 - 12) Eth is 10^18, Native is 10^12.
+	type EthGasEncoder = ();
 }
 
 impl pallet_sudo::Config for Runtime {

substrate/frame/revive/rpc/examples/js/package.json

@@ -9,10 +9,10 @@
 		"preview": "vite preview"
 	},
 	"dependencies": {
+		"@parity/revive": "^0.0.5",
 		"ethers": "^6.13.4",
 		"solc": "^0.8.28",
-		"viem": "^2.21.47",
-		"@parity/revive": "^0.0.5"
+		"viem": "^2.21.47"
 	},
 	"devDependencies": {
 		"prettier": "^3.3.3",

substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts

@@ -289,27 +289,5 @@ for (const env of envs) {
 				],
 			})
 		})
-
-		test.only('eth_estimate (no gas specified) child_call', async () => {
-			let balance = await env.serverWallet.getBalance(env.accountWallet.account)
-			expect(balance).toBe(0n)
-
-			const data = encodeFunctionData({
-				abi: FlipperCallerAbi,
-				functionName: 'callFlip',
-			})
-
-			await env.accountWallet.request({
-				method: 'eth_estimateGas',
-				params: [
-					{
-						data,
-						from: env.accountWallet.account.address,
-						to: flipperCallerAddr,
-						gas: `0x${Number(1000000).toString(16)}`,
-					},
-				],
-			})
-		})
 	})
 }

substrate/frame/revive/rpc/examples/js/src/piggy-bank.ts

@@ -1,9 +1,9 @@
 import { assert, getByteCode, walletClient } from './lib.ts'
-import { abi } from '../abi/piggyBank.ts'
+import { PiggyBankAbi } from '../abi/piggyBank.ts'
 import { parseEther } from 'viem'
 
 const hash = await walletClient.deployContract({
-	abi,
+	abi: PiggyBankAbi,
 	bytecode: getByteCode('piggyBank'),
 })
 const deployReceipt = await walletClient.waitForTransactionReceipt({ hash })
@@ -16,7 +16,7 @@ assert(contractAddress, 'Contract address should be set')
 	const result = await walletClient.estimateContractGas({
 		account: walletClient.account,
 		address: contractAddress,
-		abi,
+		abi: PiggyBankAbi,
 		functionName: 'deposit',
 		value: parseEther('10'),
 	})
@@ -26,7 +26,7 @@ assert(contractAddress, 'Contract address should be set')
 	const { request } = await walletClient.simulateContract({
 		account: walletClient.account,
 		address: contractAddress,
-		abi,
+		abi: PiggyBankAbi,
 		functionName: 'deposit',
 		value: parseEther('10'),
 	})
@@ -36,17 +36,14 @@ assert(contractAddress, 'Contract address should be set')
 
 	const receipt = await walletClient.waitForTransactionReceipt({ hash })
 	console.log(`Deposit receipt: ${receipt.status}`)
-	if (process.env.STOP) {
-		process.exit(0)
-	}
 }
 
 // Withdraw 5 WST
 {
 	const { request } = await walletClient.simulateContract({
 		account: walletClient.account,
 		address: contractAddress,
-		abi,
+		abi: PiggyBankAbi,
 		functionName: 'withdraw',
 		args: [parseEther('5')],
 	})
@@ -58,7 +55,7 @@ assert(contractAddress, 'Contract address should be set')
 	// Check remaining balance
 	const balance = await walletClient.readContract({
 		address: contractAddress,
-		abi,
+		abi: PiggyBankAbi,
 		functionName: 'getDeposit',
 	})
 

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

@@ -17,7 +17,7 @@
 //! The client connects to the source substrate chain
 //! and is used by the rpc server to query and send transactions to the substrate chain.
 use crate::{
-	runtime::GAS_PRICE,
+	runtime::gas_from_fee,
 	subxt_client::{
 		revive::{calls::types::EthTransact, events::ContractEmitted},
 		runtime_types::pallet_revive::storage::ContractInfo,
@@ -771,7 +771,7 @@ impl Client {
 	pub async fn evm_block(&self, block: Arc<SubstrateBlock>) -> Result<Block, ClientError> {
 		let runtime_api = self.inner.api.runtime_api().at(block.hash());
 		let max_fee = Self::weight_to_fee(&runtime_api, self.max_block_weight()).await?;
-		let gas_limit = U256::from(max_fee / GAS_PRICE as u128);
+		let gas_limit = gas_from_fee(max_fee);
 
 		let header = block.header();
 		let timestamp = extract_block_timestamp(&block).await.unwrap_or_default();

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

@@ -148,31 +148,12 @@ impl EthRpcServer for EthRpcServerImpl {
 
 	async fn send_raw_transaction(&self, transaction: Bytes) -> RpcResult<H256> {
 		let hash = H256(keccak_256(&transaction.0));
-
-		let tx = TransactionSigned::decode(&transaction.0).map_err(|err| {
-			log::debug!(target: LOG_TARGET, "Failed to decode transaction: {err:?}");
-			EthRpcError::from(err)
-		})?;
-
-		let eth_addr = tx.recover_eth_address().map_err(|err| {
-			log::debug!(target: LOG_TARGET, "Failed to recover eth address: {err:?}");
-			EthRpcError::InvalidSignature
-		})?;
-
-		let tx = GenericTransaction::from_signed(tx, Some(eth_addr));
-
-		// Dry run the transaction to get the weight limit and storage deposit limit
-		let dry_run = self.client.dry_run(tx, BlockTag::Latest.into()).await?;
-
-		let call = subxt_client::tx().revive().eth_transact(
-			transaction.0,
-			dry_run.gas_required.into(),
-			dry_run.storage_deposit,
-		);
+		let call = subxt_client::tx().revive().eth_transact(transaction.0);
 		self.client.submit(call).await.map_err(|err| {
 			log::debug!(target: LOG_TARGET, "submit call failed: {err:?}");
 			err
 		})?;
+
 		log::debug!(target: LOG_TARGET, "send_raw_transaction hash: {hash:?}");
 		Ok(hash)
 	}

substrate/frame/revive/src/evm.rs

@@ -19,4 +19,6 @@
 
 mod api;
 pub use api::*;
+mod gas_encoder;
+pub use gas_encoder::*;
 pub mod runtime;

substrate/frame/revive/src/evm/api/byte.rs

@@ -116,7 +116,10 @@ macro_rules! impl_hex {
 
         impl Debug for $type {
             fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult {
-                write!(f, concat!(stringify!($type), "({})"), self.0.to_hex())
+				let hex_str = self.0.to_hex();
+				let truncated = &hex_str[..hex_str.len().min(100)];
+				let ellipsis = if hex_str.len() > 100 { "..." } else { "" };
+                write!(f, concat!(stringify!($type), "({}{})"), truncated,ellipsis)
             }
         }
 

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

@@ -0,0 +1,174 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//! Encodes/Decodes EVM gas values.
+
+use crate::Weight;
+use core::ops::{Div, Rem};
+use frame_support::pallet_prelude::CheckedShl;
+use sp_arithmetic::traits::{One, Zero};
+use sp_core::U256;
+
+// We use 3 digits to store each component.
+const SCALE: u128 = 100;
+
+/// Rounds up the given value to the nearest multiple of the mask.
+///
+/// # Panics
+/// Panics if the `mask` is zero.
+fn round_up<T>(value: T, mask: T) -> T
+where
+	T: One + Zero + Copy + Rem<Output = T> + Div<Output = T>,
+	<T as Rem>::Output: PartialEq,
+{
+	let rest = if value % mask == T::zero() { T::zero() } else { T::one() };
+	value / mask + rest
+}
+
+/// Rounds up the log2 of the given value to the nearest integer.
+fn log2_round_up<T>(val: T) -> u128
+where
+	T: Into<u128>,
+{
+	let val = val.into();
+	val.checked_ilog2()
+		.map(|v| if 1u128 << v == val { v } else { v + 1 })
+		.unwrap_or(0) as u128
+}
+
+mod private {
+	pub trait Sealed {}
+	impl Sealed for () {}
+}
+
+/// Encodes/Decodes EVM gas values.
+///
+/// # Note
+///
+/// This is defined as a trait rather than standalone functions to allow
+/// it to be added as an associated type to [`crate::Config`]. This way,
+/// it can be invoked without requiring the implementation bounds to be
+/// explicitly specified.
+///
+/// This trait is sealed and cannot be implemented by downstream crates.
+pub trait GasEncoder<Balance>: private::Sealed {
+	/// Encodes all components (deposit limit, weight reference time, and proof size) into a single
+	/// gas value.
+	fn encode(gas_limit: U256, weight: Weight, deposit: Balance) -> U256;
+
+	/// Decodes the weight and deposit from the encoded gas value.
+	/// Returns `None` if the gas value is invalid
+	fn decode(gas: U256) -> Option<(Weight, Balance)>;
+}
+
+impl<Balance> GasEncoder<Balance> for ()
+where
+	Balance: Zero + One + CheckedShl + Into<u128>,
+{
+	/// The encoding follows the pattern `g...grrppdd`, where:
+	/// - `dd`: log2 Deposit value, encoded in the lowest 2 digits.
+	/// - `pp`: log2 Proof size, encoded in the next 2 digits.
+	/// - `rr`: log2 Reference time, encoded in the next 2 digits.
+	/// - `g...g`: Gas limit, encoded in the highest digits.
+	///
+	/// # Note
+	/// - The deposit value is maxed by 2^99
+	fn encode(gas_limit: U256, weight: Weight, deposit: Balance) -> U256 {
+		let deposit: u128 = deposit.into();
+		let deposit_component = log2_round_up(deposit);
+
+		let proof_size = weight.proof_size();
+		let proof_size_component = SCALE * log2_round_up(proof_size);
+
+		let ref_time = weight.ref_time();
+		let ref_time_component = SCALE.pow(2) * log2_round_up(ref_time);
+
+		let components = U256::from(deposit_component + proof_size_component + ref_time_component);
+
+		let raw_gas_mask = U256::from(SCALE).pow(3.into());
+		let raw_gas_component = if gas_limit < raw_gas_mask.saturating_add(components) {
+			raw_gas_mask
+		} else {
+			round_up(gas_limit, raw_gas_mask).saturating_mul(raw_gas_mask)
+		};
+
+		components.saturating_add(raw_gas_component)
+	}
+
+	fn decode(gas: U256) -> Option<(Weight, Balance)> {
+		let deposit = gas % SCALE;
+
+		// Casting with as_u32 is safe since all values are maxed by `SCALE`.
+		let deposit = deposit.as_u32();
+		let proof_time = ((gas / SCALE) % SCALE).as_u32();
+		let ref_time = ((gas / SCALE.pow(2)) % SCALE).as_u32();
+
+		let weight = Weight::from_parts(
+			if ref_time == 0 { 0 } else { 1u64.checked_shl(ref_time)? },
+			if proof_time == 0 { 0 } else { 1u64.checked_shl(proof_time)? },
+		);
+		let deposit =
+			if deposit == 0 { Balance::zero() } else { Balance::one().checked_shl(deposit)? };
+
+		Some((weight, deposit))
+	}
+}
+
+#[cfg(test)]
+mod test {
+	use super::*;
+
+	#[test]
+	fn test_gas_encoding_decoding_works() {
+		let raw_gas_limit = 111_111_999_999_999u128;
+		let weight = Weight::from_parts(222_999_999, 333_999_999);
+		let deposit = 444_999_999u64;
+
+		let encoded_gas = <() as GasEncoder<u64>>::encode(raw_gas_limit.into(), weight, deposit);
+		assert_eq!(encoded_gas, U256::from(111_112_000_282_929u128));
+		assert!(encoded_gas > raw_gas_limit.into());
+
+		let (decoded_weight, decoded_deposit) =
+			<() as GasEncoder<u64>>::decode(encoded_gas).unwrap();
+		assert!(decoded_weight.all_gte(weight));
+		assert!(weight.mul(2).all_gte(weight));
+
+		assert!(decoded_deposit >= deposit);
+		assert!(deposit * 2 >= decoded_deposit);
+	}
+
+	#[test]
+	fn test_encoding_zero_values_work() {
+		let encoded_gas = <() as GasEncoder<u64>>::encode(
+			Default::default(),
+			Default::default(),
+			Default::default(),
+		);
+
+		assert_eq!(encoded_gas, U256::from(1_00_00_00));
+
+		let (decoded_weight, decoded_deposit) =
+			<() as GasEncoder<u64>>::decode(encoded_gas).unwrap();
+		assert_eq!(Weight::default(), decoded_weight);
+		assert_eq!(0u64, decoded_deposit);
+	}
+
+	#[test]
+	fn test_overflow() {
+		assert_eq!(None, <() as GasEncoder<u64>>::decode(65_00u128.into()), "Invalid proof size");
+		assert_eq!(None, <() as GasEncoder<u64>>::decode(65_00_00u128.into()), "Invalid ref_time");
+	}
+}