diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 707d1c52f743..5424a11dfd3d 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -977,6 +977,7 @@ impl pallet_revive::Config for Runtime { type Xcm = pallet_xcm::Pallet; 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 for pallet_revive::Call { diff --git a/prdoc/pr_6689.prdoc b/prdoc/pr_6689.prdoc new file mode 100644 index 000000000000..596713e1ca2d --- /dev/null +++ b/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 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 +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 diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index faffcd23fbcf..26b40bba5a5e 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/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 { diff --git a/substrate/frame/revive/rpc/examples/js/bun.lockb b/substrate/frame/revive/rpc/examples/js/bun.lockb index 46994bb14754..67df5841e43f 100755 Binary files a/substrate/frame/revive/rpc/examples/js/bun.lockb and b/substrate/frame/revive/rpc/examples/js/bun.lockb differ diff --git a/substrate/frame/revive/rpc/examples/js/package.json b/substrate/frame/revive/rpc/examples/js/package.json index 6d8d00fd4214..0119f4f34a17 100644 --- a/substrate/frame/revive/rpc/examples/js/package.json +++ b/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", diff --git a/substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts b/substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts index 37ebbc9ea3b3..d5a396aa2aca 100644 --- a/substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts +++ b/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)}`, - }, - ], - }) - }) }) } diff --git a/substrate/frame/revive/rpc/examples/js/src/piggy-bank.ts b/substrate/frame/revive/rpc/examples/js/src/piggy-bank.ts index 0040b0c78dc4..8289ac8b76e3 100644 --- a/substrate/frame/revive/rpc/examples/js/src/piggy-bank.ts +++ b/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,9 +36,6 @@ 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 @@ -46,7 +43,7 @@ assert(contractAddress, 'Contract address should be set') 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', }) diff --git a/substrate/frame/revive/rpc/revive_chain.metadata b/substrate/frame/revive/rpc/revive_chain.metadata index 64b1f2014dd0..903731a245d2 100644 Binary files a/substrate/frame/revive/rpc/revive_chain.metadata and b/substrate/frame/revive/rpc/revive_chain.metadata differ diff --git a/substrate/frame/revive/rpc/src/client.rs b/substrate/frame/revive/rpc/src/client.rs index 901c15e9756b..de97844eccbb 100644 --- a/substrate/frame/revive/rpc/src/client.rs +++ b/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) -> Result { 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(); diff --git a/substrate/frame/revive/rpc/src/lib.rs b/substrate/frame/revive/rpc/src/lib.rs index ccd8bb043e90..230f2f8b7ef9 100644 --- a/substrate/frame/revive/rpc/src/lib.rs +++ b/substrate/frame/revive/rpc/src/lib.rs @@ -148,31 +148,12 @@ impl EthRpcServer for EthRpcServerImpl { async fn send_raw_transaction(&self, transaction: Bytes) -> RpcResult { 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) } diff --git a/substrate/frame/revive/src/evm.rs b/substrate/frame/revive/src/evm.rs index c3495fc0559d..c8c967fbe091 100644 --- a/substrate/frame/revive/src/evm.rs +++ b/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; diff --git a/substrate/frame/revive/src/evm/api/byte.rs b/substrate/frame/revive/src/evm/api/byte.rs index df4ed1740ecd..c2d64f8e5e42 100644 --- a/substrate/frame/revive/src/evm/api/byte.rs +++ b/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) } } diff --git a/substrate/frame/revive/src/evm/gas_encoder.rs b/substrate/frame/revive/src/evm/gas_encoder.rs new file mode 100644 index 000000000000..1e4724a53a25 --- /dev/null +++ b/substrate/frame/revive/src/evm/gas_encoder.rs @@ -0,0 +1,160 @@ +// 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(value: T, mask: T) -> T +where + T: One + Zero + Copy + Rem + Div, + ::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(val: T) -> u128 +where + T: Into, +{ + let val = val.into(); + val.checked_ilog2() + .map(|v| if 1u128 << v == val { v } else { v + 1 }) + .unwrap_or(0) as u128 +} + +/// Encodes/Decodes EVM gas values. +pub trait GasEncoder { + /// 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 GasEncoder for () +where + Balance: Zero + One + CheckedShl + Into, +{ + /// 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>::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>::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>::encode( + Default::default(), + Default::default(), + Default::default(), + ); + + assert_eq!(encoded_gas, U256::from(1_00_00_00)); + + let (decoded_weight, decoded_deposit) = + <() as GasEncoder>::decode(encoded_gas).unwrap(); + assert_eq!(Weight::default(), decoded_weight); + assert_eq!(0u64, decoded_deposit); + } + + #[test] + fn test_overflow() { + assert_eq!(None, <() as GasEncoder>::decode(65_00u128.into()), "Invalid proof size"); + assert_eq!(None, <() as GasEncoder>::decode(65_00_00u128.into()), "Invalid ref_time"); + } +} diff --git a/substrate/frame/revive/src/evm/runtime.rs b/substrate/frame/revive/src/evm/runtime.rs index 24b75de83569..8c87c2c63fe5 100644 --- a/substrate/frame/revive/src/evm/runtime.rs +++ b/substrate/frame/revive/src/evm/runtime.rs @@ -16,9 +16,13 @@ // limitations under the License. //! Runtime types for integrating `pallet-revive` with the EVM. use crate::{ - evm::api::{GenericTransaction, TransactionSigned}, - AccountIdOf, AddressMapper, BalanceOf, MomentOf, Weight, LOG_TARGET, + evm::{ + api::{GenericTransaction, TransactionSigned}, + GasEncoder, + }, + AccountIdOf, AddressMapper, BalanceOf, Config, MomentOf, LOG_TARGET, }; +use alloc::vec::Vec; use codec::{Decode, Encode}; use frame_support::{ dispatch::{DispatchInfo, GetDispatchInfo}, @@ -26,20 +30,17 @@ use frame_support::{ }; use pallet_transaction_payment::OnChargeTransaction; use scale_info::{StaticTypeInfo, TypeInfo}; -use sp_arithmetic::Percent; use sp_core::{Get, H256, U256}; use sp_runtime::{ generic::{self, CheckedExtrinsic, ExtrinsicFormat}, traits::{ - self, Checkable, Dispatchable, ExtrinsicLike, ExtrinsicMetadata, IdentifyAccount, Member, - TransactionExtension, + self, AtLeast32BitUnsigned, Checkable, Dispatchable, ExtrinsicLike, ExtrinsicMetadata, + IdentifyAccount, Member, TransactionExtension, }, transaction_validity::{InvalidTransaction, TransactionValidityError}, OpaqueExtrinsic, RuntimeDebug, Saturating, }; -use alloc::vec::Vec; - type CallOf = ::RuntimeCall; /// The EVM gas price. @@ -48,7 +49,28 @@ type CallOf = ::RuntimeCall; /// We use a fixed value for the gas price. /// This let us calculate the gas estimate for a transaction with the formula: /// `estimate_gas = substrate_fee / gas_price`. -pub const GAS_PRICE: u32 = 1u32; +/// +/// The chosen constant value is balanced to ensure: +/// - It is not too high, allowing components (ref_time, proof_size, and deposit) to be encoded in +/// the lower digits of the gas value. +/// - It is not too low, enabling users to adjust the gas price to define a tip. +pub const GAS_PRICE: u32 = 1_000u32; + +/// Convert a `Balance` into a gas value, using the fixed `GAS_PRICE`. +/// The gas is calculated as `balance / GAS_PRICE`, rounded up to the nearest integer. +pub fn gas_from_fee(fee: Balance) -> U256 +where + u32: Into, + Balance: Into + AtLeast32BitUnsigned + Copy, +{ + let gas_price = GAS_PRICE.into(); + let remainder = fee % gas_price; + if remainder.is_zero() { + (fee / gas_price).into() + } else { + (fee.saturating_add(gas_price) / gas_price).into() + } +} /// Wraps [`generic::UncheckedExtrinsic`] to support checking unsigned /// [`crate::Call::eth_transact`] extrinsic. @@ -140,15 +162,8 @@ where fn check(self, lookup: &Lookup) -> Result { if !self.0.is_signed() { if let Ok(call) = self.0.function.clone().try_into() { - if let crate::Call::eth_transact { payload, gas_limit, storage_deposit_limit } = - call - { - let checked = E::try_into_checked_extrinsic( - payload, - gas_limit, - storage_deposit_limit, - self.encoded_size(), - )?; + if let crate::Call::eth_transact { payload } = call { + let checked = E::try_into_checked_extrinsic(payload, self.encoded_size())?; return Ok(checked) }; } @@ -251,7 +266,7 @@ where /// EthExtra convert an unsigned [`crate::Call::eth_transact`] into a [`CheckedExtrinsic`]. pub trait EthExtra { /// The Runtime configuration. - type Config: crate::Config + pallet_transaction_payment::Config; + type Config: Config + pallet_transaction_payment::Config; /// The Runtime's transaction extension. /// It should include at least: @@ -281,8 +296,6 @@ pub trait EthExtra { /// - `encoded_len`: The encoded length of the extrinsic. fn try_into_checked_extrinsic( payload: Vec, - gas_limit: Weight, - storage_deposit_limit: BalanceOf, encoded_len: usize, ) -> Result< CheckedExtrinsic, CallOf, Self::Extension>, @@ -307,12 +320,11 @@ pub trait EthExtra { InvalidTransaction::BadProof })?; - let signer = - ::AddressMapper::to_fallback_account_id(&signer); + let signer = ::AddressMapper::to_fallback_account_id(&signer); let GenericTransaction { nonce, chain_id, to, value, input, gas, gas_price, .. } = GenericTransaction::from_signed(tx, None); - if chain_id.unwrap_or_default() != ::ChainId::get().into() { + if chain_id.unwrap_or_default() != ::ChainId::get().into() { log::debug!(target: LOG_TARGET, "Invalid chain_id {chain_id:?}"); return Err(InvalidTransaction::Call); } @@ -324,6 +336,15 @@ pub trait EthExtra { })?; let data = input.unwrap_or_default().0; + + let (gas_limit, storage_deposit_limit) = ::EthGasEncoder::decode( + gas.unwrap_or_default(), + ) + .ok_or_else(|| { + log::debug!(target: LOG_TARGET, "Failed to decode gas: {gas:?}"); + InvalidTransaction::Call + })?; + let call = if let Some(dest) = to { crate::Call::call:: { dest, @@ -380,23 +401,18 @@ pub trait EthExtra { Default::default(), ) .into(); - log::trace!(target: LOG_TARGET, "try_into_checked_extrinsic: encoded_len: {encoded_len:?} actual_fee: {actual_fee:?} eth_fee: {eth_fee:?}"); + log::debug!(target: LOG_TARGET, "try_into_checked_extrinsic: encoded_len: {encoded_len:?} actual_fee: {actual_fee:?} eth_fee: {eth_fee:?}"); // The fees from the Ethereum transaction should be greater or equal to the actual fees paid // by the account. if eth_fee < actual_fee { - log::debug!(target: LOG_TARGET, "fees {eth_fee:?} too low for the extrinsic {actual_fee:?}"); + log::debug!(target: LOG_TARGET, "eth fees {eth_fee:?} too low, actual fees: {actual_fee:?}"); return Err(InvalidTransaction::Payment.into()) } - let min = actual_fee.min(eth_fee_no_tip); - let max = actual_fee.max(eth_fee_no_tip); - let diff = Percent::from_rational(max - min, min); - if diff > Percent::from_percent(10) { - log::trace!(target: LOG_TARGET, "Difference between the extrinsic fees {actual_fee:?} and the Ethereum gas fees {eth_fee_no_tip:?} should be no more than 10% got {diff:?}"); + if eth_fee_no_tip > actual_fee.saturating_mul(2u32.into()) { + log::debug!(target: LOG_TARGET, "actual fees {actual_fee:?} too high, base eth fees: {eth_fee_no_tip:?}"); return Err(InvalidTransaction::Call.into()) - } else { - log::trace!(target: LOG_TARGET, "Difference between the extrinsic fees {actual_fee:?} and the Ethereum gas fees {eth_fee_no_tip:?}: {diff:?}"); } let tip = eth_fee.saturating_sub(eth_fee_no_tip); @@ -415,6 +431,7 @@ mod test { evm::*, test_utils::*, tests::{ExtBuilder, RuntimeCall, RuntimeOrigin, Test}, + Weight, }; use frame_support::{error::LookupError, traits::fungible::Mutate}; use pallet_revive_fixtures::compile_module; @@ -467,7 +484,7 @@ mod test { Self { tx: GenericTransaction { from: Some(Account::default().address()), - chain_id: Some(::ChainId::get().into()), + chain_id: Some(::ChainId::get().into()), gas_price: Some(U256::from(GAS_PRICE)), ..Default::default() }, @@ -489,6 +506,11 @@ mod test { Ok(dry_run) => { log::debug!(target: LOG_TARGET, "Estimated gas: {:?}", dry_run.eth_gas); self.tx.gas = Some(dry_run.eth_gas); + let (gas_limit, deposit) = + <::EthGasEncoder as GasEncoder<_>>::decode(dry_run.eth_gas) + .unwrap(); + self.gas_limit = gas_limit; + self.storage_deposit_limit = deposit; }, Err(err) => { log::debug!(target: LOG_TARGET, "Failed to estimate gas: {:?}", err); @@ -526,27 +548,18 @@ mod test { /// Call `check` on the unchecked extrinsic, and `pre_dispatch` on the signed extension. fn check(&self) -> Result<(RuntimeCall, SignedExtra), TransactionValidityError> { ExtBuilder::default().build().execute_with(|| { - let UncheckedExtrinsicBuilder { - tx, - gas_limit, - storage_deposit_limit, - before_validate, - } = self.clone(); + let UncheckedExtrinsicBuilder { tx, before_validate, .. } = self.clone(); // Fund the account. let account = Account::default(); - let _ = ::Currency::set_balance( + let _ = ::Currency::set_balance( &account.substrate_account(), 100_000_000_000_000, ); let payload = account.sign_transaction(tx.try_into_unsigned().unwrap()).signed_payload(); - let call = RuntimeCall::Contracts(crate::Call::eth_transact { - payload, - gas_limit, - storage_deposit_limit, - }); + let call = RuntimeCall::Contracts(crate::Call::eth_transact { payload }); let encoded_len = call.encoded_size(); let uxt: Ex = generic::UncheckedExtrinsic::new_bare(call).into(); @@ -653,7 +666,7 @@ mod test { #[test] fn check_transaction_fees() { - let scenarios: [(_, Box, _); 5] = [ + let scenarios: Vec<(_, Box, _)> = vec![ ( "Eth fees too low", Box::new(|tx| { @@ -671,24 +684,9 @@ mod test { ( "Gas fees too low", Box::new(|tx| { - tx.gas = Some(tx.gas.unwrap() * 2); - }), - InvalidTransaction::Call, - ), - ( - "Diff > 10%", - Box::new(|tx| { - tx.gas = Some(tx.gas.unwrap() * 111 / 100); + tx.gas = Some(tx.gas.unwrap() / 2); }), - InvalidTransaction::Call, - ), - ( - "Diff < 10%", - Box::new(|tx| { - tx.gas_price = Some(tx.gas_price.unwrap() * 2); - tx.gas = Some(tx.gas.unwrap() * 89 / 100); - }), - InvalidTransaction::Call, + InvalidTransaction::Payment, ), ]; diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs index b9a39e7ce4d3..b879fa5b360e 100644 --- a/substrate/frame/revive/src/lib.rs +++ b/substrate/frame/revive/src/lib.rs @@ -41,7 +41,10 @@ pub mod test_utils; pub mod weights; use crate::{ - evm::{runtime::GAS_PRICE, GenericTransaction}, + evm::{ + runtime::{gas_from_fee, GAS_PRICE}, + GasEncoder, GenericTransaction, + }, exec::{AccountIdOf, ExecError, Executable, Ext, Key, Origin, Stack as ExecStack}, gas::GasMeter, storage::{meter::Meter as StorageMeter, ContractInfo, DeletionQueueManager}, @@ -295,6 +298,10 @@ pub mod pallet { /// The ratio between the decimal representation of the native token and the ETH token. #[pallet::constant] type NativeToEthRatio: Get; + + /// Encode and decode Ethereum gas values. See [`GasEncoder`]. + #[pallet::no_default_bounds] + type EthGasEncoder: GasEncoder>; } /// Container for different types that implement [`DefaultConfig`]` of this pallet. @@ -368,6 +375,7 @@ pub mod pallet { type PVFMemory = ConstU32<{ 512 * 1024 * 1024 }>; type ChainId = ConstU64<0>; type NativeToEthRatio = ConstU32<1>; + type EthGasEncoder = (); } } @@ -560,6 +568,8 @@ pub mod pallet { AccountUnmapped, /// Tried to map an account that is already mapped. AccountAlreadyMapped, + /// The transaction used to dry-run a contract is invalid. + InvalidGenericTransaction, } /// A reason for the pallet contracts placing a hold on funds. @@ -761,12 +771,7 @@ pub mod pallet { #[allow(unused_variables)] #[pallet::call_index(0)] #[pallet::weight(Weight::MAX)] - pub fn eth_transact( - origin: OriginFor, - payload: Vec, - gas_limit: Weight, - #[pallet::compact] storage_deposit_limit: BalanceOf, - ) -> DispatchResultWithPostInfo { + pub fn eth_transact(origin: OriginFor, payload: Vec) -> DispatchResultWithPostInfo { Err(frame_system::Error::CallFiltered::.into()) } @@ -1406,11 +1411,8 @@ where return Err(EthTransactError::Message("Invalid transaction".into())); }; - let eth_dispatch_call = crate::Call::::eth_transact { - payload: unsigned_tx.dummy_signed_payload(), - gas_limit: result.gas_required, - storage_deposit_limit: result.storage_deposit, - }; + let eth_dispatch_call = + crate::Call::::eth_transact { payload: unsigned_tx.dummy_signed_payload() }; let encoded_len = utx_encoded_size(eth_dispatch_call); let fee = pallet_transaction_payment::Pallet::::compute_fee( encoded_len, @@ -1418,7 +1420,9 @@ where 0u32.into(), ) .into(); - let eth_gas: U256 = (fee / GAS_PRICE.into()).into(); + let eth_gas = gas_from_fee(fee); + let eth_gas = + T::EthGasEncoder::encode(eth_gas, result.gas_required, result.storage_deposit); if eth_gas == result.eth_gas { log::trace!(target: LOG_TARGET, "bare_eth_call: encoded_len: {encoded_len:?} eth_gas: {eth_gas:?}");