From fbc24378ed694ce09b92cef06e46a3b85d360da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erce=20Can=20Bekt=C3=BCre?= <47954181+ercecan@users.noreply.github.com> Date: Wed, 2 Oct 2024 17:45:31 +0300 Subject: [PATCH] Fix estiamte gas l1 fee issue when metamask max amount is selected (#1261) * Fix estiamte gas l1 fee issue when metamask max amount is selected * Fix tests * Remove unnecessary comment --- bin/citrea/tests/evm/tracing.rs | 12 +++---- crates/evm/src/evm/handler.rs | 8 ++--- crates/evm/src/query.rs | 33 +++++++++++++++---- .../adapters/mock-da/src/service.rs | 4 +-- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/bin/citrea/tests/evm/tracing.rs b/bin/citrea/tests/evm/tracing.rs index 752a09e16..dd09fb458 100644 --- a/bin/citrea/tests/evm/tracing.rs +++ b/bin/citrea/tests/evm/tracing.rs @@ -116,14 +116,14 @@ async fn tracing_tests() -> Result<(), Box> { // It was replaced with the gas limit in our trace. let reth_json = serde_json::from_value::(json![{ "from": "0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266", - "gas": "0x679c", + "gas": "0x6a7d", "gasUsed": "0xba65", "to": "0xe7f1725e7734ce288f8367e1bb143e90bb3f0512", "input": "0xb7d5b6580000000000000000000000005fbdb2315678afecb367f032d93f642f64180aa30000000000000000000000000000000000000000000000000000000000000003", "calls": [ { "from": "0xe7f1725e7734ce288f8367e1bb143e90bb3f0512", - "gas": "0x5833", + "gas": "0x5b09", "gasUsed": "0x57f2", "to": "0x5fbdb2315678afecb367f032d93f642f64180aa3", "input": "0x60fe47b10000000000000000000000000000000000000000000000000000000000000003", @@ -178,7 +178,7 @@ async fn tracing_tests() -> Result<(), Box> { .await; let expected_send_eth_trace = serde_json::from_value::( - json![{"from":"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266","gas":"0x1","gasUsed":"0x5208", + json![{"from":"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266","gas":"0x1d1","gasUsed":"0x5208", "to":"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92255","input":"0x","value":"0x4563918244f40000","type":"CALL"}], ).unwrap(); assert_eq!(send_eth_trace, CallTracer(expected_send_eth_trace.clone())); @@ -207,11 +207,11 @@ async fn tracing_tests() -> Result<(), Box> { ); let expected_call_get_trace = serde_json::from_value::( - json![{"from":"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266","gas":"0x1886","gasUsed":"0x6b64","to":"0xe7f1725e7734ce288f8367e1bb143e90bb3f0512", + json![{"from":"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266","gas":"0x19c7","gasUsed":"0x6b64","to":"0xe7f1725e7734ce288f8367e1bb143e90bb3f0512", "input":"0x35c152bd0000000000000000000000005fbdb2315678afecb367f032d93f642f64180aa3", "output":"0x0000000000000000000000000000000000000000000000000000000000000000", "calls":[{"from":"0xe7f1725e7734ce288f8367e1bb143e90bb3f0512", - "gas":"0xc3e","gasUsed":"0x996","to":"0x5fbdb2315678afecb367f032d93f642f64180aa3", + "gas":"0xd7a","gasUsed":"0x996","to":"0x5fbdb2315678afecb367f032d93f642f64180aa3", "input":"0x6d4ce63c","output":"0x0000000000000000000000000000000000000000000000000000000000000003","type":"STATICCALL"}], "value":"0x0","type":"CALL"}], ).unwrap(); @@ -291,7 +291,7 @@ async fn tracing_tests() -> Result<(), Box> { .await; let expected_top_call_only_call_get_trace = serde_json::from_value::( - json![{"from":"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266","gas":"0x1886","gasUsed":"0x6b64","to":"0xe7f1725e7734ce288f8367e1bb143e90bb3f0512", + json![{"from":"0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266","gas":"0x19c7","gasUsed":"0x6b64","to":"0xe7f1725e7734ce288f8367e1bb143e90bb3f0512", "input":"0x35c152bd0000000000000000000000005fbdb2315678afecb367f032d93f642f64180aa3", "output":"0x0000000000000000000000000000000000000000000000000000000000000000", "calls":[], diff --git a/crates/evm/src/evm/handler.rs b/crates/evm/src/evm/handler.rs index 1d8906c0b..b3112066e 100644 --- a/crates/evm/src/evm/handler.rs +++ b/crates/evm/src/evm/handler.rs @@ -25,11 +25,11 @@ use crate::system_events::SYSTEM_SIGNER; use crate::{BASE_FEE_VAULT, L1_FEE_VAULT}; /// Eoa size is reduced because code_hash for eoas are None on state diff, converted to empty Keccak internally for evm operations -const DB_ACCOUNT_SIZE_EOA: usize = 42; +pub(crate) const DB_ACCOUNT_SIZE_EOA: usize = 42; const DB_ACCOUNT_SIZE_CONTRACT: usize = 75; /// Normally db account key is: 6 bytes of prefix ("Evm/a/") + 1 byte for size of remaining data + 20 bytes of address = 27 bytes -const DB_ACCOUNT_KEY_SIZE: usize = 27; +pub(crate) const DB_ACCOUNT_KEY_SIZE: usize = 27; /// Storage key is 59 bytes because of sov sdk prefix ("Evm/s/") const STORAGE_KEY_SIZE: usize = 59; @@ -70,9 +70,9 @@ Let's consider a batch of 1 block with the following transactions: If every user pays 0.75 of the balance state diff they created, the total balance state diff will be covered */ /// Nonce and balance are stored together so we use single constant -const NONCE_DISCOUNTED_PERCENTAGE: usize = 55; +pub(crate) const NONCE_DISCOUNTED_PERCENTAGE: usize = 55; const STORAGE_DISCOUNTED_PERCENTAGE: usize = 66; -const ACCOUNT_DISCOUNTED_PERCENTAGE: usize = 29; +pub(crate) const ACCOUNT_DISCOUNTED_PERCENTAGE: usize = 29; #[derive(Copy, Clone, Default, Debug)] pub struct TxInfo { diff --git a/crates/evm/src/query.rs b/crates/evm/src/query.rs index 9adacd625..1adda33dc 100644 --- a/crates/evm/src/query.rs +++ b/crates/evm/src/query.rs @@ -6,7 +6,6 @@ use alloy_eips::eip2930::AccessListWithGasUsed; use alloy_primitives::Uint; use alloy_rlp::Encodable; use jsonrpsee::core::RpcResult; -use reth_primitives::constants::GWEI_TO_WEI; use reth_primitives::TxKind::{Call, Create}; use reth_primitives::{ Block, BlockId, BlockNumberOrTag, SealedHeader, TransactionSignedEcRecovered, U256, U64, @@ -38,7 +37,10 @@ use crate::evm::call::prepare_call_env; use crate::evm::db::EvmDb; use crate::evm::primitive_types::{BlockEnv, Receipt, SealedBlock, TransactionSignedAndRecovered}; use crate::evm::DbAccount; -use crate::handler::TxInfo; +use crate::handler::{ + TxInfo, ACCOUNT_DISCOUNTED_PERCENTAGE, DB_ACCOUNT_KEY_SIZE, DB_ACCOUNT_SIZE_EOA, + NONCE_DISCOUNTED_PERCENTAGE, +}; use crate::rpc_helpers::*; use crate::{BloomFilter, Evm, EvmChainConfig, FilterBlockOption, FilterError}; @@ -71,8 +73,7 @@ impl EstimatedTxExpenses { /// Return total estimated gas used including evm gas and L1 fee. pub(crate) fn gas_with_l1_overhead(&self) -> U256 { // Actually not an L1 fee but l1_fee / base_fee. - let l1_fee_overhead = - U256::from(1).max(self.l1_fee / (self.base_fee + U256::from(GWEI_TO_WEI))); // assume 1 gwei priority fee + let l1_fee_overhead = U256::from(1).max(self.l1_fee.div_ceil(self.base_fee)); l1_fee_overhead + U256::from(self.gas_used) } } @@ -879,11 +880,31 @@ impl Evm { if let Ok((res, tx_info)) = res { if res.result.is_success() { + // If value is zero we should add extra balance transfer diff size assuming the first estimate gas was done by metamask + // we do this because on metamask when trying to send max amount to an address it will send 2 estimate_gas requests + // One with 0 value and the other with the remaining balance that extract from the current balance after the gas fee is deducted + // This causes the diff size to be lower than the actual diff size, and the tx to fail due to not enough l1 fee + let mut diff_size = tx_info.l1_diff_size; + let mut l1_fee = tx_info.l1_fee; + if tx_env.value.is_zero() { + // Calculation taken from diff size calculation in handler.rs + let balance_diff_size = + (DB_ACCOUNT_KEY_SIZE * ACCOUNT_DISCOUNTED_PERCENTAGE / 100) + as u64 + + ((DB_ACCOUNT_SIZE_EOA + DB_ACCOUNT_KEY_SIZE) + * NONCE_DISCOUNTED_PERCENTAGE + / 100) as u64; + + diff_size += balance_diff_size; + l1_fee = l1_fee.saturating_add( + U256::from(l1_fee_rate) * (U256::from(balance_diff_size)), + ); + } return Ok(EstimatedTxExpenses { gas_used: U64::from(MIN_TRANSACTION_GAS), base_fee: env_base_fee, - l1_fee: tx_info.l1_fee, - l1_diff_size: tx_info.l1_diff_size, + l1_fee, + l1_diff_size: diff_size, }); } } diff --git a/crates/sovereign-sdk/adapters/mock-da/src/service.rs b/crates/sovereign-sdk/adapters/mock-da/src/service.rs index e32d24c92..d14e74a0e 100644 --- a/crates/sovereign-sdk/adapters/mock-da/src/service.rs +++ b/crates/sovereign-sdk/adapters/mock-da/src/service.rs @@ -502,8 +502,8 @@ impl DaService for MockDaService { } async fn get_fee_rate(&self) -> Result { - // Mock constant - Ok(10_u128) + // Mock constant, use min possible in bitcoin + Ok(2500000000_u128) } async fn get_block_by_hash(