fix: ethgasprice faster & better (#5358)

CHANGELOG.md

@@ -39,6 +39,8 @@
 
 - [#5377](https://github.com/ChainSafe/forest/pull/5377) Fix incorrect handling of `max_height` for `latest` predefined block in `Filecoin.EthGetLogs`.
 
+- [#5356](https://github.com/ChainSafe/forest/issues/5356) Fixed slow (and incorrect!) `Filecoin.EthGasPrice` RPC method. The method now returns the correct gas price of the latest tipset.
+
 ## Forest v0.24.0 "Treebeard"
 
 Non-mandatory release without network upgrades. It includes a number of potentially breaking changes (see below), new RPC methods, fixes and other improvements.

src/rpc/methods/eth.rs

@@ -58,7 +58,7 @@ use num::{BigInt, Zero as _};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::str::FromStr;
-use std::{ops::Add, sync::Arc};
+use std::sync::Arc;
 use utils::{decode_payload, lookup_eth_address};
 
 const MASKED_ID_PREFIX: [u8; 12] = [0xff, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];
@@ -744,6 +744,7 @@ impl RpcMethod<0> for EthGasPrice {
     const PARAM_NAMES: [&'static str; 0] = [];
     const API_PATHS: ApiPaths = ApiPaths::V1;
     const PERMISSION: Permission = Permission::Read;
+    const DESCRIPTION: Option<&'static str> = Some("Returns the current gas price in attoFIL");
 
     type Params = ();
     type Ok = GasPriceResult;
@@ -752,15 +753,16 @@ impl RpcMethod<0> for EthGasPrice {
         ctx: Ctx<impl Blockstore + Send + Sync + 'static>,
         (): Self::Params,
     ) -> Result<Self::Ok, ServerError> {
+        // According to Geth's implementation, eth_gasPrice should return base + tip
+        // Ref: https://github.com/ethereum/pm/issues/328#issuecomment-853234014
         let ts = ctx.chain_store().heaviest_tipset();
         let block0 = ts.block_headers().first();
-        let base_fee = &block0.parent_base_fee;
-        if let Ok(premium) = gas::estimate_gas_premium(&ctx, 10000).await {
-            let gas_price = base_fee.add(premium);
-            Ok(EthBigInt(gas_price.atto().clone()))
-        } else {
-            Ok(EthBigInt(BigInt::zero()))
-        }
+        let base_fee = block0.parent_base_fee.atto();
+        let tip = crate::rpc::gas::estimate_gas_premium(&ctx, 0)
+            .await
+            .map(|gas_premium| gas_premium.atto().to_owned())
+            .unwrap_or_default();
+        Ok(EthBigInt(base_fee + tip))
     }
 }
 

src/tool/subcommands/api_cmd.rs

@@ -129,6 +129,10 @@ pub enum ApiCommands {
         #[arg(long, required = true)]
         /// Folder into which test snapshots are dumped
         out_dir: PathBuf,
+        /// Allow generating snapshot even if Lotus generated a different response. This is useful
+        /// when the response is not deterministic.
+        #[arg(long)]
+        allow_response_mismatch: bool,
     },
     DumpTests {
         #[command(flatten)]
@@ -216,6 +220,7 @@ impl ApiCommands {
                 db,
                 chain,
                 out_dir,
+                allow_response_mismatch,
             } => {
                 std::env::set_var("FOREST_TIPSET_CACHE_DISABLED", "1");
                 if !out_dir.is_dir() {
@@ -238,6 +243,7 @@ impl ApiCommands {
                         &test_dump,
                         tracking_db.clone(),
                         &chain,
+                        allow_response_mismatch,
                     )
                     .await
                     {

src/tool/subcommands/api_cmd/api_compare_tests.rs

@@ -41,6 +41,7 @@ use ipld_core::ipld::Ipld;
 use itertools::Itertools as _;
 use jsonrpsee::types::ErrorCode;
 use libp2p::PeerId;
+use num_traits::Signed as _;
 use once_cell::sync::Lazy;
 use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
@@ -1161,9 +1162,10 @@ fn eth_tests() -> Vec<RpcTest> {
         tests.push(RpcTest::identity(
             EthChainId::request_with_alias((), use_alias).unwrap(),
         ));
-        // There is randomness in the result of this API
-        tests.push(RpcTest::basic(
+        // There is randomness in the result of this API, but at least check that the results are non-zero.
+        tests.push(RpcTest::validate(
             EthGasPrice::request_with_alias((), use_alias).unwrap(),
+            |forest, lotus| forest.0.is_positive() && lotus.0.is_positive(),
         ));
         tests.push(RpcTest::basic(
             EthSyncing::request_with_alias((), use_alias).unwrap(),

src/tool/subcommands/api_cmd/generate_test_snapshot.rs

@@ -31,6 +31,7 @@ pub async fn run_test_with_dump(
     test_dump: &TestDump,
     db: Arc<ReadOpsTrackingStore<ManyCar<ParityDb>>>,
     chain: &NetworkChain,
+    allow_response_mismatch: bool,
 ) -> anyhow::Result<()> {
     if chain.is_testnet() {
         CurrentNetwork::set_global(Network::Testnet);
@@ -45,7 +46,8 @@ pub async fn run_test_with_dump(
                 let params = <$ty>::parse_params(params_raw.clone(), ParamStructure::Either)?;
                 let result = <$ty>::handle(ctx.clone(), params).await?;
                 anyhow::ensure!(
-                    test_dump.forest_response == Ok(result.into_lotus_json_value()?),
+                    allow_response_mismatch
+                        || test_dump.forest_response == Ok(result.into_lotus_json_value()?),
                     "Response mismatch between Forest and Lotus"
                 );
                 run = true;

src/tool/subcommands/api_cmd/test_snapshot.rs

@@ -214,6 +214,11 @@ mod tests {
             }
         }));
 
+        // We need to set RNG seed so that tests are run with deterministic
+        // output. The snapshots should be generated with a node running with the same seed, if
+        // they are testing methods that are not deterministic, e.g.,
+        // `[`crate::rpc::methods::gas::estimate_gas_premium`]`.
+        std::env::set_var(crate::utils::rand::FIXED_RNG_SEED_ENV, "4213666");
         while let Some((filename, file_path)) = tasks.next().await {
             print!("Testing {filename} ...");
             run_test_from_snapshot(&file_path).await.unwrap();

src/tool/subcommands/api_cmd/test_snapshots.txt

@@ -22,7 +22,7 @@ filecoin_ethblocknumber_1741272348346171.rpcsnap.json.zst
 filecoin_ethcall_1737446676693468.rpcsnap.json.zst
 filecoin_ethchainid_1736937942819147.rpcsnap.json.zst
 filecoin_ethfeehistory_1737446676883828.rpcsnap.json.zst
-filecoin_ethgasprice_1740132537695619.rpcsnap.json.zst
+filecoin_ethgasprice_1741280031788015.rpcsnap.json.zst
 filecoin_ethgetbalance_1737446676695335.rpcsnap.json.zst
 filecoin_ethgetbalance_1740048634848277.rpcsnap.json.zst
 filecoin_ethgetblockbyhash_1740132537807408.rpcsnap.json.zst

src/utils/rand/mod.rs

@@ -17,7 +17,7 @@ pub fn forest_os_rng() -> impl Rng + CryptoRng {
     forest_rng_internal(ForestRngMode::OsRng)
 }
 
-const FIXED_RNG_SEED_ENV: &str = "FOREST_TEST_RNG_FIXED_SEED";
+pub const FIXED_RNG_SEED_ENV: &str = "FOREST_TEST_RNG_FIXED_SEED";
 
 enum ForestRngMode {
     ThreadRng,