Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change of gas price calculation for chain doesn't support EIP-1559 #270

Open
ManojJiSharma opened this issue Nov 25, 2024 · 1 comment
Open
Assignees
Labels
bug Something isn't working

Comments

@ManojJiSharma
Copy link
Contributor

Description

Currently, the gas fee estimation logic assumes that all EVM-compatible chains support EIP-1559, which leads to errors on chains that do not, such as Polygon zkEVM, Binance Smart Chain (BNB), and Ethereum Classic. This issue aims to enhance the logic to detect EIP-1559 support dynamically and handle gas price calculations accordingly.

Problem Details:

  • Chains without EIP-1559 support do not provide base_fee_per_gas, making the current estimation method fail.
  • These chains require gas price to be fetched using the eth_gasPrice RPC method instead.

Acceptance Criteria:

Gas fee estimation correctly falls back to eth_gasPrice on chains without EIP-1559.

@ManojJiSharma ManojJiSharma added the bug Something isn't working label Nov 25, 2024
@ManojJiSharma ManojJiSharma self-assigned this Nov 25, 2024
@Lohann
Copy link
Collaborator

Lohann commented Nov 25, 2024

Changes needed:

  • Include in the chain/ethereum/server/src/utils.rs method for detect wether the chain support EIP-1559 or not, this is done in the method calculate_gas_fee I sent in the above snippet.
    • if the base_fee_per_gas is None , then the chain doesn’t support EIP-1559, and the gas price must be retrieved by calling the eth_gasPrice method.
    • If the base_fee_per_gas is Some, then the chain supports EIP-1559, and the gas must be calculated using the existing eip1559_default_estimator which is already implemented.
  • The EthereumMetadata must be updated for support legacy transaction gas estimation, I recommend looking at the FeeType struct I sent in the above snippet.
    • Basically replace the max_priority_fee_per_gas and max_fee_per_gas by the FeeType I sent above.
  • In the chain/ethereum/server/src/client.rs must update the metadata and faucet methods for use the new fee estimator method.
  • The signing method at chains/ethereum/tx/src/lib.rs must be updated for accept transactions LegacyTransaction type if the chain doesn’t support EIP-1559.

Code for compute gas fee for EIP-1559 and Legacy:

/// The number of blocks from the past for which the fee rewards are fetched for fee estimation.
const EIP1559_FEE_ESTIMATION_PAST_BLOCKS: u64 = 10;
/// Multiplier for the current base fee to estimate max base fee for the next block.
const EIP1559_BASE_FEE_MULTIPLIER: u128 = 2;
/// The default percentile of gas premiums that are fetched for fee estimation.
const EIP1559_FEE_ESTIMATION_REWARD_PERCENTILE: f64 = 20.0;
/// The minimum priority fee to provide.
const EIP1559_MIN_PRIORITY_FEE: u128 = 1;

#[derive(Debug, Clone, PartialEq, Eq)]
enum FeeType {
    Legacy(U256),
    Eip1559 {
        max_fee_per_gas: U256,
        max_priority_fee_per_gas: U256,
    },
}

fn estimate_eip1559_max_priority_fee(fee_history: &FeeHistory) -> U256 {
    // The default EIP-1559 fee estimator.
    //
    // Based on the work by [MetaMask](https://github.com/MetaMask/core/blob/main/packages/gas-fee-controller/src/fetchGasEstimatesViaEthFeeHistory/calculateGasFeeEstimatesForPriorityLevels.ts#L56);
    // constants for "medium" priority level are used.
    let mut rewards = fee_history.reward.iter().filter_map(|r| r.first().copied()).filter(|r| !r.is_zero()).collect::<Vec<_>>();
    rewards.sort_unstable();
    
    if rewards.is_empty() {
        U256::from(EIP1559_MIN_PRIORITY_FEE)
    } else {
        let n = rewards.len();
        let median = if n % 2 == 0 {
            (rewards[n / 2 - 1] + rewards[n / 2]) / 2
        } else {
            rewards[n / 2]
        };
        std::cmp::max(median, U256::from(EIP1559_MIN_PRIORITY_FEE))
    }
}


async fn calculate_gas_fee<E, T>(client: T) -> anyhow::Result<FeeType> where E: std::error::Error + Send + Sync + 'static, T: EthereumRpc<Error = E> + Send + Sync + 'static {
    let fee_history = client.fee_history(
        EIP1559_FEE_ESTIMATION_PAST_BLOCKS,
        AtBlock::Latest,
        &[EIP1559_FEE_ESTIMATION_REWARD_PERCENTILE]
    ).await?;

    // if the base fee of the Latest block is 0 then we need check if the latest block even has
    // a base fee/supports EIP1559
    let last_block_base_fee = fee_history.base_fee_per_gas.iter().rev().nth(1).copied();
    let base_fee_per_gas = match last_block_base_fee {
        Some(base_fee) if !base_fee.is_zero() => Some(base_fee),
        _ => {
            let Some(block) = client.block(AtBlock::Latest).await? else {
                anyhow::bail!("failed to get latest block");
            };
            let Some(block_hash) = block.hash else {
                anyhow::bail!("block hash is missing: {block:?}");
            };
            tracing::info!("           latest block: {block_hash:?}");
            // let Some(base_fee_per_gas) = block.header.base_fee_per_gas else {
            //     anyhow::bail!("latest block does not have base fee");
            // };
            match block.header.base_fee_per_gas {
                Some(base_fee_per_gas) if base_fee_per_gas > 0 => Some(U256::from(base_fee_per_gas)),
                _ => None,
            }
        }
    };

    let fee_type = if let Some(base_fee_per_gas) = base_fee_per_gas {
        tracing::info!("        base fee per gas: {base_fee_per_gas:?}");

        let max_priority_fee_per_gas = estimate_eip1559_max_priority_fee(&fee_history);
        tracing::info!("max priority fee per gas: {max_priority_fee_per_gas:?}");

        let potential_max_fee = base_fee_per_gas.saturating_mul(U256::from(EIP1559_BASE_FEE_MULTIPLIER));
        tracing::info!("       potential max fee: {potential_max_fee:?}");

        let max_fee_per_gas = potential_max_fee.saturating_add(max_priority_fee_per_gas);
        tracing::info!("         max fee per gas: {max_fee_per_gas:?}");
        FeeType::Eip1559 { max_fee_per_gas, max_priority_fee_per_gas }
    } else {
        let gas_price = client.gas_price().await?;
        tracing::info!("               gas price: {gas_price:?}");
        FeeType::Legacy(gas_price)
    };
    Ok(fee_type)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants