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 5fb495e4e8cf7d928ef6b8a8cc1537b88508fc2e..71cfdc58cceb3071e9c4e251579c9d6e4ff2168d 100644
--- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs
+++ b/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> {
diff --git a/prdoc/pr_6689.prdoc b/prdoc/pr_6689.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..2cbb49cd7dd2457ab256a1325ca88c4f0830e57e
--- /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, 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
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 93b134e8165ffc93561034325fbd88dd752b39b4..7de04b27ff8307e432ba819a112b42ec3d8c2c23 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 46994bb147547bfb9b960b7630d1a6d274ee75dd..67df5841e43fba141c7a146a1e4a8958b4c7a84c 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 6d8d00fd4214721e3f42418c78e440aaa3309f4e..0119f4f34a177d6647158b45e830e58af9a85424 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 b9ee877927bb8d65ed1fef94926ec989a42ebbe4..871adeccbc9a450d273987f7f8b53576d79a1d42 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 0040b0c78dc47f66521aeef559d8d2797201ea70..8289ac8b76e333e9d1361d09f52d0e9b5a7325b1 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 64b1f2014dd06815fcea6a87bc96306eb00eda8b..402e8c2d22b21471929e9c61acd2cc968af614cf 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 901c15e9756b65b6059602c59b2bc190d927d7b0..de97844eccbbf85efabfec11ddedc2df62af5209 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<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();
diff --git a/substrate/frame/revive/rpc/src/lib.rs b/substrate/frame/revive/rpc/src/lib.rs
index ccd8bb043e90ec77008eed81a87c83bd5baa35df..230f2f8b7ef963c742d7cdf2ce9b5c1f47506a1d 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<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)
}
diff --git a/substrate/frame/revive/src/evm.rs b/substrate/frame/revive/src/evm.rs
index c3495fc0559d220a7ccaaeaac488a12963b98a79..c8c967fbe091bb2af5bfaacaefbe854d4f9d0043 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 df4ed1740ecdb75a8a027fc17fdc6b6f222b0ae9..c2d64f8e5e424b2086e0247e5ccea0e08d4350b0 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 0000000000000000000000000000000000000000..ffdf8b13c0439140a9e2b41d9830bf8c581f3421
--- /dev/null
+++ b/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");
+ }
+}
diff --git a/substrate/frame/revive/src/evm/runtime.rs b/substrate/frame/revive/src/evm/runtime.rs
index 24b75de835698697311d191a812df36f6fd81390..d4b344e20eb850e6c42d84bdf870204c1382cb67 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<T> = <T as frame_system::Config>::RuntimeCall;
/// The EVM gas price.
@@ -48,7 +49,28 @@ type CallOf<T> = <T as frame_system::Config>::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:
+/// - Not too high, ensuring the gas value is large enough (at least 7 digits) to encode the
+/// ref_time, proof_size, and deposit into the less significant (6 lower) digits of the gas value.
+/// - 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<Balance>(fee: Balance) -> U256
+where
+ u32: Into<Balance>,
+ Balance: Into<U256> + 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<Self::Checked, TransactionValidityError> {
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<u8>,
- gas_limit: Weight,
- storage_deposit_limit: BalanceOf<Self::Config>,
encoded_len: usize,
) -> Result<
CheckedExtrinsic<AccountIdOf<Self::Config>, CallOf<Self::Config>, Self::Extension>,
@@ -307,12 +320,16 @@ pub trait EthExtra {
InvalidTransaction::BadProof
})?;
- let signer =
- <Self::Config as crate::Config>::AddressMapper::to_fallback_account_id(&signer);
+ let signer = <Self::Config as Config>::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() != <Self::Config as crate::Config>::ChainId::get().into() {
+ let Some(gas) = gas else {
+ log::debug!(target: LOG_TARGET, "No gas provided");
+ return Err(InvalidTransaction::Call);
+ };
+
+ if chain_id.unwrap_or_default() != <Self::Config as Config>::ChainId::get().into() {
log::debug!(target: LOG_TARGET, "Invalid chain_id {chain_id:?}");
return Err(InvalidTransaction::Call);
}
@@ -324,6 +341,13 @@ pub trait EthExtra {
})?;
let data = input.unwrap_or_default().0;
+
+ let (gas_limit, storage_deposit_limit) =
+ <Self::Config as Config>::EthGasEncoder::decode(gas).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::<Self::Config> {
dest,
@@ -359,13 +383,13 @@ pub trait EthExtra {
// Fees calculated with the fixed `GAS_PRICE`
// When we dry-run the transaction, we set the gas to `Fee / GAS_PRICE`
let eth_fee_no_tip = U256::from(GAS_PRICE)
- .saturating_mul(gas.unwrap_or_default())
+ .saturating_mul(gas)
.try_into()
.map_err(|_| InvalidTransaction::Call)?;
// Fees with the actual gas_price from the transaction.
let eth_fee: BalanceOf<Self::Config> = U256::from(gas_price.unwrap_or_default())
- .saturating_mul(gas.unwrap_or_default())
+ .saturating_mul(gas)
.try_into()
.map_err(|_| InvalidTransaction::Call)?;
@@ -380,27 +404,17 @@ 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: gas_price: {gas_price:?}, 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:?}");
- 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);
- log::debug!(target: LOG_TARGET, "Created checked Ethereum transaction with nonce {nonce:?} and tip: {tip:?}");
+ log::debug!(target: LOG_TARGET, "Created checked Ethereum transaction with nonce: {nonce:?} and tip: {tip:?}");
Ok(CheckedExtrinsic {
format: ExtrinsicFormat::Signed(signer.into(), Self::get_eth_extension(nonce, tip)),
function,
@@ -415,6 +429,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;
@@ -456,8 +471,6 @@ mod test {
#[derive(Clone)]
struct UncheckedExtrinsicBuilder {
tx: GenericTransaction,
- gas_limit: Weight,
- storage_deposit_limit: BalanceOf<Test>,
before_validate: Option<std::sync::Arc<dyn Fn() + Send + Sync>>,
}
@@ -467,12 +480,10 @@ mod test {
Self {
tx: GenericTransaction {
from: Some(Account::default().address()),
- chain_id: Some(<Test as crate::Config>::ChainId::get().into()),
+ chain_id: Some(<Test as Config>::ChainId::get().into()),
gas_price: Some(U256::from(GAS_PRICE)),
..Default::default()
},
- gas_limit: Weight::zero(),
- storage_deposit_limit: 0,
before_validate: None,
}
}
@@ -500,7 +511,6 @@ mod test {
fn call_with(dest: H160) -> Self {
let mut builder = Self::new();
builder.tx.to = Some(dest);
- ExtBuilder::default().build().execute_with(|| builder.estimate_gas());
builder
}
@@ -508,45 +518,42 @@ mod test {
fn instantiate_with(code: Vec<u8>, data: Vec<u8>) -> Self {
let mut builder = Self::new();
builder.tx.input = Some(Bytes(code.into_iter().chain(data.into_iter()).collect()));
- ExtBuilder::default().build().execute_with(|| builder.estimate_gas());
builder
}
- /// Update the transaction with the given function.
- fn update(mut self, f: impl FnOnce(&mut GenericTransaction) -> ()) -> Self {
- f(&mut self.tx);
- self
- }
/// Set before_validate function.
fn before_validate(mut self, f: impl Fn() + Send + Sync + 'static) -> Self {
self.before_validate = Some(std::sync::Arc::new(f));
self
}
+ fn check(
+ self,
+ ) -> Result<(RuntimeCall, SignedExtra, GenericTransaction), TransactionValidityError> {
+ self.mutate_estimate_and_check(Box::new(|_| ()))
+ }
+
/// Call `check` on the unchecked extrinsic, and `pre_dispatch` on the signed extension.
- fn check(&self) -> Result<(RuntimeCall, SignedExtra), TransactionValidityError> {
+ fn mutate_estimate_and_check(
+ mut self,
+ f: Box<dyn FnOnce(&mut GenericTransaction) -> ()>,
+ ) -> Result<(RuntimeCall, SignedExtra, GenericTransaction), TransactionValidityError> {
+ ExtBuilder::default().build().execute_with(|| self.estimate_gas());
+ f(&mut self.tx);
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 _ = <Test as crate::Config>::Currency::set_balance(
+ let _ = <Test as Config>::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 payload = account
+ .sign_transaction(tx.clone().try_into_unsigned().unwrap())
+ .signed_payload();
+ let call = RuntimeCall::Contracts(crate::Call::eth_transact { payload });
let encoded_len = call.encoded_size();
let uxt: Ex = generic::UncheckedExtrinsic::new_bare(call).into();
@@ -565,7 +572,7 @@ mod test {
0,
)?;
- Ok((result.function, extra))
+ Ok((result.function, extra, tx))
})
}
}
@@ -573,14 +580,18 @@ mod test {
#[test]
fn check_eth_transact_call_works() {
let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]));
+ let (call, _, tx) = builder.check().unwrap();
+ let (gas_limit, storage_deposit_limit) =
+ <<Test as Config>::EthGasEncoder as GasEncoder<_>>::decode(tx.gas.unwrap()).unwrap();
+
assert_eq!(
- builder.check().unwrap().0,
+ call,
crate::Call::call::<Test> {
- dest: builder.tx.to.unwrap(),
- value: builder.tx.value.unwrap_or_default().as_u64(),
- gas_limit: builder.gas_limit,
- storage_deposit_limit: builder.storage_deposit_limit,
- data: builder.tx.input.unwrap_or_default().0
+ dest: tx.to.unwrap(),
+ value: tx.value.unwrap_or_default().as_u64(),
+ data: tx.input.unwrap_or_default().0,
+ gas_limit,
+ storage_deposit_limit
}
.into()
);
@@ -591,16 +602,19 @@ mod test {
let (code, _) = compile_module("dummy").unwrap();
let data = vec![];
let builder = UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone());
+ let (call, _, tx) = builder.check().unwrap();
+ let (gas_limit, storage_deposit_limit) =
+ <<Test as Config>::EthGasEncoder as GasEncoder<_>>::decode(tx.gas.unwrap()).unwrap();
assert_eq!(
- builder.check().unwrap().0,
+ call,
crate::Call::instantiate_with_code::<Test> {
- value: builder.tx.value.unwrap_or_default().as_u64(),
- gas_limit: builder.gas_limit,
- storage_deposit_limit: builder.storage_deposit_limit,
+ value: tx.value.unwrap_or_default().as_u64(),
code,
data,
- salt: None
+ salt: None,
+ gas_limit,
+ storage_deposit_limit
}
.into()
);
@@ -608,11 +622,10 @@ mod test {
#[test]
fn check_eth_transact_nonce_works() {
- let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]))
- .update(|tx| tx.nonce = Some(1u32.into()));
+ let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]));
assert_eq!(
- builder.check(),
+ builder.mutate_estimate_and_check(Box::new(|tx| tx.nonce = Some(1u32.into()))),
Err(TransactionValidityError::Invalid(InvalidTransaction::Future))
);
@@ -629,11 +642,10 @@ mod test {
#[test]
fn check_eth_transact_chain_id_works() {
- let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]))
- .update(|tx| tx.chain_id = Some(42.into()));
+ let builder = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]));
assert_eq!(
- builder.check(),
+ builder.mutate_estimate_and_check(Box::new(|tx| tx.chain_id = Some(42.into()))),
Err(TransactionValidityError::Invalid(InvalidTransaction::Call))
);
}
@@ -646,14 +658,14 @@ mod test {
// Fail because the tx input fail to get the blob length
assert_eq!(
- builder.clone().update(|tx| tx.input = Some(Bytes(vec![1, 2, 3]))).check(),
+ builder.mutate_estimate_and_check(Box::new(|tx| tx.input = Some(Bytes(vec![1, 2, 3])))),
Err(TransactionValidityError::Invalid(InvalidTransaction::Call))
);
}
#[test]
fn check_transaction_fees() {
- let scenarios: [(_, Box<dyn FnOnce(&mut GenericTransaction)>, _); 5] = [
+ let scenarios: Vec<(_, Box<dyn FnOnce(&mut GenericTransaction)>, _)> = vec![
(
"Eth fees too low",
Box::new(|tx| {
@@ -661,42 +673,20 @@ mod test {
}),
InvalidTransaction::Payment,
),
- (
- "Gas fees too high",
- Box::new(|tx| {
- tx.gas = Some(tx.gas.unwrap() * 2);
- }),
- InvalidTransaction::Call,
- ),
(
"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,
),
];
for (msg, update_tx, err) in scenarios {
- let builder =
- UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20])).update(update_tx);
+ let res = UncheckedExtrinsicBuilder::call_with(H160::from([1u8; 20]))
+ .mutate_estimate_and_check(update_tx);
- assert_eq!(builder.check(), Err(TransactionValidityError::Invalid(err)), "{}", msg);
+ assert_eq!(res, Err(TransactionValidityError::Invalid(err)), "{}", msg);
}
}
@@ -704,16 +694,16 @@ mod test {
fn check_transaction_tip() {
let (code, _) = compile_module("dummy").unwrap();
let data = vec![];
- let builder = UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone())
- .update(|tx| {
- tx.gas_price = Some(tx.gas_price.unwrap() * 103 / 100);
- log::debug!(target: LOG_TARGET, "Gas price: {:?}", tx.gas_price);
- });
+ let (_, extra, tx) =
+ UncheckedExtrinsicBuilder::instantiate_with(code.clone(), data.clone())
+ .mutate_estimate_and_check(Box::new(|tx| {
+ tx.gas_price = Some(tx.gas_price.unwrap() * 103 / 100);
+ log::debug!(target: LOG_TARGET, "Gas price: {:?}", tx.gas_price);
+ }))
+ .unwrap();
- let tx = &builder.tx;
let expected_tip =
tx.gas_price.unwrap() * tx.gas.unwrap() - U256::from(GAS_PRICE) * tx.gas.unwrap();
- let (_, extra) = builder.check().unwrap();
assert_eq!(U256::from(extra.1.tip()), expected_tip);
}
}
diff --git a/substrate/frame/revive/src/lib.rs b/substrate/frame/revive/src/lib.rs
index b9a39e7ce4d370dfe33d88a5cef52b58d9dcdb80..04bce264a188c1b55f1a9c53398903a9a9e55545 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,11 @@ pub mod pallet {
/// The ratio between the decimal representation of the native token and the ETH token.
#[pallet::constant]
type NativeToEthRatio: Get<u32>;
+
+ /// Encode and decode Ethereum gas values.
+ /// Only valid value is `()`. See [`GasEncoder`].
+ #[pallet::no_default_bounds]
+ type EthGasEncoder: GasEncoder<BalanceOf<Self>>;
}
/// Container for different types that implement [`DefaultConfig`]` of this pallet.
@@ -368,6 +376,7 @@ pub mod pallet {
type PVFMemory = ConstU32<{ 512 * 1024 * 1024 }>;
type ChainId = ConstU64<0>;
type NativeToEthRatio = ConstU32<1>;
+ type EthGasEncoder = ();
}
}
@@ -560,6 +569,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 +772,7 @@ pub mod pallet {
#[allow(unused_variables)]
#[pallet::call_index(0)]
#[pallet::weight(Weight::MAX)]
- pub fn eth_transact(
- origin: OriginFor<T>,
- payload: Vec<u8>,
- gas_limit: Weight,
- #[pallet::compact] storage_deposit_limit: BalanceOf<T>,
- ) -> DispatchResultWithPostInfo {
+ pub fn eth_transact(origin: OriginFor<T>, payload: Vec<u8>) -> DispatchResultWithPostInfo {
Err(frame_system::Error::CallFiltered::<T>.into())
}
@@ -1406,11 +1412,8 @@ where
return Err(EthTransactError::Message("Invalid transaction".into()));
};
- let eth_dispatch_call = crate::Call::<T>::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::<T>::eth_transact { payload: unsigned_tx.dummy_signed_payload() };
let encoded_len = utx_encoded_size(eth_dispatch_call);
let fee = pallet_transaction_payment::Pallet::<T>::compute_fee(
encoded_len,
@@ -1418,7 +1421,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:?}");