diff --git a/client/rpc/src/eth.rs b/client/rpc/src/eth.rs index 97af5d4842..ffbec0924a 100644 --- a/client/rpc/src/eth.rs +++ b/client/rpc/src/eth.rs @@ -468,6 +468,12 @@ fn fee_details( match (request_gas_price, request_max_fee, request_priority) { (gas_price, None, None) => { // Legacy request, all default to gas price. + // A zero-set gas price is None. + let gas_price = if gas_price.unwrap_or_default().is_zero() { + None + } else { + gas_price + }; Ok(FeeDetails { gas_price, max_fee_per_gas: gas_price, @@ -476,6 +482,12 @@ fn fee_details( } (_, max_fee, max_priority) => { // eip-1559 + // A zero-set max fee is None. + let max_fee = if max_fee.unwrap_or_default().is_zero() { + None + } else { + max_fee + }; // Ensure `max_priority_fee_per_gas` is less or equal to `max_fee_per_gas`. if let Some(max_priority) = max_priority { let max_fee = max_fee.unwrap_or_default(); diff --git a/frame/ethereum/src/lib.rs b/frame/ethereum/src/lib.rs index b412bbdf9c..f8e55e83da 100644 --- a/frame/ethereum/src/lib.rs +++ b/frame/ethereum/src/lib.rs @@ -796,6 +796,7 @@ impl Pallet { } }; + let is_transactional = true; match action { ethereum::TransactionAction::Call(target) => { let res = T::Runner::call( @@ -808,6 +809,7 @@ impl Pallet { max_priority_fee_per_gas, nonce, access_list, + is_transactional, config.as_ref().unwrap_or(T::config()), ) .map_err(Into::into)?; @@ -824,6 +826,7 @@ impl Pallet { max_priority_fee_per_gas, nonce, access_list, + is_transactional, config.as_ref().unwrap_or(T::config()), ) .map_err(Into::into)?; diff --git a/frame/evm/src/benchmarking.rs b/frame/evm/src/benchmarking.rs index 60ada9c2a0..896b8b2944 100644 --- a/frame/evm/src/benchmarking.rs +++ b/frame/evm/src/benchmarking.rs @@ -76,6 +76,7 @@ benchmarks! { let value = U256::default(); let gas_limit_create: u64 = 1_250_000 * 1_000_000_000; + let is_transactional = true; let create_runner_results = T::Runner::create( caller, contract_bytecode, @@ -85,6 +86,7 @@ benchmarks! { None, Some(nonce_as_u256), Vec::new(), + is_transactional, T::config(), ); assert_eq!(create_runner_results.is_ok(), true, "create() failed"); @@ -106,6 +108,7 @@ benchmarks! { nonce = nonce + 1; let nonce_as_u256: U256 = nonce.into(); + let is_transactional = true; let call_runner_results = T::Runner::call( caller, contract_address, @@ -116,6 +119,7 @@ benchmarks! { None, Some(nonce_as_u256), Vec::new(), + is_transactional, T::config(), ); assert_eq!(call_runner_results.is_ok(), true, "call() failed"); diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index 66b94ee7d5..cf6104f14a 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -192,6 +192,7 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { T::CallOrigin::ensure_address_origin(&source, origin)?; + let is_transactional = true; let info = T::Runner::call( source, target, @@ -202,6 +203,7 @@ pub mod pallet { max_priority_fee_per_gas, nonce, access_list, + is_transactional, T::config(), )?; @@ -238,6 +240,7 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { T::CallOrigin::ensure_address_origin(&source, origin)?; + let is_transactional = true; let info = T::Runner::create( source, init, @@ -247,6 +250,7 @@ pub mod pallet { max_priority_fee_per_gas, nonce, access_list, + is_transactional, T::config(), )?; @@ -291,6 +295,7 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { T::CallOrigin::ensure_address_origin(&source, origin)?; + let is_transactional = true; let info = T::Runner::create2( source, init, @@ -301,6 +306,7 @@ pub mod pallet { max_priority_fee_per_gas, nonce, access_list, + is_transactional, T::config(), )?; @@ -734,6 +740,9 @@ where type LiquidityInfo = Option>; fn withdraw_fee(who: &H160, fee: U256) -> Result> { + if fee.is_zero() { + return Ok(None); + } let account_id = T::AddressMapping::into_account_id(*who); let imbalance = C::withdraw( &account_id, diff --git a/frame/evm/src/runner/mod.rs b/frame/evm/src/runner/mod.rs index bc0de56fc5..221ae73c4d 100644 --- a/frame/evm/src/runner/mod.rs +++ b/frame/evm/src/runner/mod.rs @@ -35,6 +35,7 @@ pub trait Runner { max_priority_fee_per_gas: Option, nonce: Option, access_list: Vec<(H160, Vec)>, + is_transactional: bool, config: &evm::Config, ) -> Result; @@ -47,6 +48,7 @@ pub trait Runner { max_priority_fee_per_gas: Option, nonce: Option, access_list: Vec<(H160, Vec)>, + is_transactional: bool, config: &evm::Config, ) -> Result; @@ -60,6 +62,7 @@ pub trait Runner { max_priority_fee_per_gas: Option, nonce: Option, access_list: Vec<(H160, Vec)>, + is_transactional: bool, config: &evm::Config, ) -> Result; } diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index 54a769e21e..a71ea4ad79 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -52,6 +52,7 @@ impl Runner { nonce: Option, config: &'config evm::Config, precompiles: &'precompiles T::PrecompilesType, + is_transactional: bool, f: F, ) -> Result, Error> where @@ -65,21 +66,24 @@ impl Runner { ) -> (ExitReason, R), { let base_fee = T::FeeCalculator::min_gas_price(); - - let max_fee_per_gas = match (max_fee_per_gas, max_priority_fee_per_gas) { - (Some(max_fee_per_gas), Some(max_priority_fee_per_gas)) => { - ensure!(max_fee_per_gas >= base_fee, Error::::GasPriceTooLow); - ensure!(max_fee_per_gas >= max_priority_fee_per_gas, Error::::GasPriceTooLow); - max_fee_per_gas - }, - (Some(max_fee_per_gas), None) => { + let max_fee_per_gas = match (max_fee_per_gas, is_transactional) { + (Some(max_fee_per_gas), _) => { ensure!(max_fee_per_gas >= base_fee, Error::::GasPriceTooLow); max_fee_per_gas - }, - // Gas price check is skipped when performing a gas estimation. - _ => Default::default() + } + // Gas price check is skipped for non-transactional calls that don't + // define a `max_fee_per_gas` input. + (None, false) => Default::default(), + _ => return Err(Error::::GasPriceTooLow), }; + if let Some(max_priority_fee) = max_priority_fee_per_gas { + ensure!( + max_fee_per_gas >= max_priority_fee, + Error::::GasPriceTooLow + ); + } + // After eip-1559 we make sure the account can pay both the evm execution and priority fees. let total_fee = max_fee_per_gas .checked_mul(U256::from(gas_limit)) @@ -89,15 +93,22 @@ impl Runner { .checked_add(total_fee) .ok_or(Error::::PaymentOverflow)?; let source_account = Pallet::::account_basic(&source); - ensure!( - source_account.balance >= total_payment, - Error::::BalanceLow - ); + // Account balance check is skipped if fee is Zero. + // This case is previously verified to only happen on either: + // - Non-transactional calls. + // - BaseFee is configured to be Zero. + if total_fee > U256::zero() { + ensure!( + source_account.balance >= total_payment, + Error::::BalanceLow + ); + } if let Some(nonce) = nonce { ensure!(source_account.nonce == nonce, Error::::InvalidNonce); } - // Deduct fee from the `source` account. + + // Deduct fee from the `source` account. Returns `None` if `total_fee` is Zero. let fee = T::OnChargeTransaction::withdraw_fee(&source, total_fee)?; // Execute the EVM call. @@ -131,12 +142,13 @@ impl Runner { }; log::debug!( target: "evm", - "Execution {:?} [source: {:?}, value: {}, gas_limit: {}, actual_fee: {}]", + "Execution {:?} [source: {:?}, value: {}, gas_limit: {}, actual_fee: {}, is_transactional: {}]", reason, source, value, gas_limit, - actual_fee + actual_fee, + is_transactional ); // The difference between initially withdrawn and the actual cost is refunded. // @@ -214,6 +226,7 @@ impl RunnerT for Runner { max_priority_fee_per_gas: Option, nonce: Option, access_list: Vec<(H160, Vec)>, + is_transactional: bool, config: &evm::Config, ) -> Result { let precompiles = T::PrecompilesValue::get(); @@ -226,6 +239,7 @@ impl RunnerT for Runner { nonce, config, &precompiles, + is_transactional, |executor| executor.transact_call(source, target, value, input, gas_limit, access_list), ) } @@ -239,6 +253,7 @@ impl RunnerT for Runner { max_priority_fee_per_gas: Option, nonce: Option, access_list: Vec<(H160, Vec)>, + is_transactional: bool, config: &evm::Config, ) -> Result { let precompiles = T::PrecompilesValue::get(); @@ -251,6 +266,7 @@ impl RunnerT for Runner { nonce, config, &precompiles, + is_transactional, |executor| { let address = executor.create_address(evm::CreateScheme::Legacy { caller: source }); ( @@ -271,6 +287,7 @@ impl RunnerT for Runner { max_priority_fee_per_gas: Option, nonce: Option, access_list: Vec<(H160, Vec)>, + is_transactional: bool, config: &evm::Config, ) -> Result { let precompiles = T::PrecompilesValue::get(); @@ -284,6 +301,7 @@ impl RunnerT for Runner { nonce, config, &precompiles, + is_transactional, |executor| { let address = executor.create_address(evm::CreateScheme::Create2 { caller: source, diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs index 8bef7ecbff..0219097a8f 100644 --- a/frame/evm/src/tests.rs +++ b/frame/evm/src/tests.rs @@ -496,3 +496,121 @@ fn test_hotfix_inc_account_sufficients_increments_with_saturation_if_nonce_nonze assert_eq!(account.nonce, 1); }); } + +#[test] +fn runner_non_transactional_calls_with_non_balance_accounts_is_ok_without_gas_price() { + // Expect to skip checks for gas price and account balance when both: + // - The call is non transactional (`is_transactional == false`). + // - The `max_fee_per_gas` is None. + new_test_ext().execute_with(|| { + let non_balance_account = + H160::from_str("7700000000000000000000000000000000000001").unwrap(); + assert_eq!( + EVM::account_basic(&non_balance_account).balance, + U256::zero() + ); + let _ = ::Runner::call( + non_balance_account, + H160::from_str("1000000000000000000000000000000000000001").unwrap(), + Vec::new(), + U256::from(1u32), + 1000000, + None, + None, + None, + Vec::new(), + false, + &::config().clone(), + ) + .expect("Non transactional call succeeds"); + assert_eq!( + EVM::account_basic(&non_balance_account).balance, + U256::zero() + ); + }); +} + +#[test] +fn runner_non_transactional_calls_with_non_balance_accounts_is_err_with_gas_price() { + // In non transactional calls where `Some(gas_price)` is defined, expect it to be + // checked against the `BaseFee`, and expect the account to have enough balance + // to pay for the call. + new_test_ext().execute_with(|| { + let non_balance_account = + H160::from_str("7700000000000000000000000000000000000001").unwrap(); + assert_eq!( + EVM::account_basic(&non_balance_account).balance, + U256::zero() + ); + let res = ::Runner::call( + non_balance_account, + H160::from_str("1000000000000000000000000000000000000001").unwrap(), + Vec::new(), + U256::from(1u32), + 1000000, + Some(U256::from(1_000_000_000)), + None, + None, + Vec::new(), + false, + &::config().clone(), + ); + assert!(res.is_err()); + }); +} + +#[test] +fn runner_transactional_call_with_zero_gas_price_fails() { + // Transactional calls are rejected when `max_fee_per_gas == None`. + new_test_ext().execute_with(|| { + let res = ::Runner::call( + H160::default(), + H160::from_str("1000000000000000000000000000000000000001").unwrap(), + Vec::new(), + U256::from(1u32), + 1000000, + None, + None, + None, + Vec::new(), + true, + &::config().clone(), + ); + assert!(res.is_err()); + }); +} + +#[test] +fn runner_max_fee_per_gas_gte_max_priority_fee_per_gas() { + // Transactional and non transactional calls enforce `max_fee_per_gas >= max_priority_fee_per_gas`. + new_test_ext().execute_with(|| { + let res = ::Runner::call( + H160::default(), + H160::from_str("1000000000000000000000000000000000000001").unwrap(), + Vec::new(), + U256::from(1u32), + 1000000, + Some(U256::from(1_000_000_000)), + Some(U256::from(2_000_000_000)), + None, + Vec::new(), + true, + &::config().clone(), + ); + assert!(res.is_err()); + let res = ::Runner::call( + H160::default(), + H160::from_str("1000000000000000000000000000000000000001").unwrap(), + Vec::new(), + U256::from(1u32), + 1000000, + Some(U256::from(1_000_000_000)), + Some(U256::from(2_000_000_000)), + None, + Vec::new(), + false, + &::config().clone(), + ); + assert!(res.is_err()); + }); +} diff --git a/template/runtime/src/lib.rs b/template/runtime/src/lib.rs index 01130762b2..61c384f45f 100644 --- a/template/runtime/src/lib.rs +++ b/template/runtime/src/lib.rs @@ -621,6 +621,7 @@ impl_runtime_apis! { None }; + let is_transactional = false; ::Runner::call( from, to, @@ -631,6 +632,7 @@ impl_runtime_apis! { max_priority_fee_per_gas, nonce, access_list.unwrap_or_default(), + is_transactional, config.as_ref().unwrap_or(::config()), ).map_err(|err| err.into()) } @@ -654,6 +656,7 @@ impl_runtime_apis! { None }; + let is_transactional = false; ::Runner::create( from, data, @@ -663,6 +666,7 @@ impl_runtime_apis! { max_priority_fee_per_gas, nonce, access_list.unwrap_or_default(), + is_transactional, config.as_ref().unwrap_or(::config()), ).map_err(|err| err.into()) } diff --git a/ts-tests/tests/test-gas.ts b/ts-tests/tests/test-gas.ts index 210353d264..d5b2e90b67 100644 --- a/ts-tests/tests/test-gas.ts +++ b/ts-tests/tests/test-gas.ts @@ -128,4 +128,17 @@ describeWithFrontier("Frontier RPC (Gas)", (context) => { expect(result).to.equal(context.web3.utils.numberToHex(binary_search_estimation)); }); + it("eth_estimateGas 0x0 gasPrice is equivalent to not setting one", async function () { + let result = (await context.web3.eth.estimateGas({ + from: GENESIS_ACCOUNT, + data: Test.bytecode, + gasPrice: "0x0", + })); + expect(result).to.equal(197690); + result = (await context.web3.eth.estimateGas({ + from: GENESIS_ACCOUNT, + data: Test.bytecode, + })); + expect(result).to.equal(197690); + }); });