From c2292b3d18a8f3e72733c9be3ce7cee239a49b1c Mon Sep 17 00:00:00 2001 From: Hubert Bugaj Date: Mon, 10 Aug 2026 14:53:15 +0200 Subject: [PATCH] fix(rpc): raise `eth_estimateGas` until the message succeeds + tests --- .config/forest.dic | 3 +- .github/workflows/forest.yml | 7 +- CHANGELOG.md | 2 + mise.toml | 9 +- scripts/devnet/README.md | 19 +- scripts/devnet/lotus.env | 2 + .../{wallet_harness.sh => test_harness.sh} | 16 +- src/dev/subcommands/devnet_cmd.rs | 61 +++ .../devnet_cmd/contracts/compile.sh | 19 + .../contracts/nested_gas/nested_gas.hex | 1 + .../contracts/nested_gas/nested_gas.sol | 29 ++ src/dev/subcommands/devnet_cmd/eth_gas.rs | 346 ++++++++++++++++++ src/dev/subcommands/mod.rs | 5 + src/dev/subcommands/tests_cmd.rs | 2 +- src/dev/subcommands/tests_cmd/helpers.rs | 82 +++-- src/rpc/methods/eth.rs | 166 ++++++--- src/rpc/methods/eth/errors.rs | 41 +++ src/rpc/methods/gas.rs | 49 ++- src/shim/executor.rs | 84 +++++ src/state_manager/message_simulation.rs | 14 +- 20 files changed, 822 insertions(+), 135 deletions(-) rename scripts/devnet/{wallet_harness.sh => test_harness.sh} (77%) create mode 100644 src/dev/subcommands/devnet_cmd.rs create mode 100755 src/dev/subcommands/devnet_cmd/contracts/compile.sh create mode 100644 src/dev/subcommands/devnet_cmd/contracts/nested_gas/nested_gas.hex create mode 100644 src/dev/subcommands/devnet_cmd/contracts/nested_gas/nested_gas.sol create mode 100644 src/dev/subcommands/devnet_cmd/eth_gas.rs diff --git a/.config/forest.dic b/.config/forest.dic index 60add798833d..c9d66dcaef7e 100644 --- a/.config/forest.dic +++ b/.config/forest.dic @@ -1,4 +1,5 @@ -294 +295 +ABI Algorand/M API's API/SM diff --git a/.github/workflows/forest.yml b/.github/workflows/forest.yml index 56ea11bd944f..3e1c01bcf92f 100644 --- a/.github/workflows/forest.yml +++ b/.github/workflows/forest.yml @@ -491,13 +491,14 @@ jobs: run: | chmod +x "$GITHUB_WORKSPACE"/forest* echo "$GITHUB_WORKSPACE" >> "$GITHUB_PATH" - - name: Devnet wallet tests + - name: Devnet tests run: | set -euo pipefail - source ./scripts/devnet/wallet_harness.sh - devnet_wallet_env_init + source ./scripts/devnet/test_harness.sh + devnet_test_env_init forest-dev tests mpool forest-dev tests wallet + forest-dev devnet eth-gas timeout-minutes: ${{ fromJSON(env.SCRIPT_TIMEOUT_MINUTES) }} - name: Dump docker logs if: always() diff --git a/CHANGELOG.md b/CHANGELOG.md index 45ac5b99b0bc..2c74df38da4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ ### Fixed +- [#7473](https://github.com/ChainSafe/forest/pull/7473): Fixed `eth_estimateGas` under-estimating gas for nested contract calls (EIP-150's 63/64 rule), which could make transactions fail on chain with `SYS_OUT_OF_GAS`; the estimate is now raised until the message succeeds. Genuine reverts return an `execution reverted` error (JSON-RPC code `3`) with the decoded reason and data, matching Lotus. + - [#7412](https://github.com/ChainSafe/forest/issues/7412): Fixes quicknet "unchained" logic to fetch the `max_beacon_round` for all covered epochs - [#7446](https://github.com/ChainSafe/forest/pull/7446): Fixed a panic condition on `ChainNotify` when a client closes a connection just after subscription. diff --git a/mise.toml b/mise.toml index fcbcf4e56984..69e0b79ede1f 100644 --- a/mise.toml +++ b/mise.toml @@ -225,8 +225,8 @@ forest-dev tests mpool forest-dev tests wallet ''' -[tasks."test:wallet-devnet"] -description = "Run wallet integration tests against a local docker devnet." +[tasks."test:devnet"] +description = "Run the integration test suites against a local docker devnet." shell = "bash -c" run = ''' set -euo pipefail @@ -234,10 +234,11 @@ pushd scripts/devnet ./setup.sh ./check.sh popd -source ./scripts/devnet/wallet_harness.sh -devnet_wallet_env_init +source ./scripts/devnet/test_harness.sh +devnet_test_env_init forest-dev tests mpool forest-dev tests wallet +forest-dev devnet eth-gas ''' [tasks."codecov:nextest"] diff --git a/scripts/devnet/README.md b/scripts/devnet/README.md index fd2a14510e7e..f3d2ecd863f8 100644 --- a/scripts/devnet/README.md +++ b/scripts/devnet/README.md @@ -56,20 +56,25 @@ export FULLNODE_API_INFO=$TOKEN:/dns/forest/tcp/3456/http forest-cli net peers ``` -## Running the wallet integration tests +## Running the integration tests -The same wallet/mpool integration suite that runs against calibnet can be run -against the local devnet. This brings up the devnet, waits for it to sync, wires -up the host environment, and runs the tests: +The wallet/mpool suites that run against calibnet, plus the `eth_estimateGas` +parity suite, can be run against the local devnet. This brings up the devnet, +waits for it to sync, wires up the host environment, and runs the tests: ```shell -mise run test:wallet-devnet +mise run test:devnet ``` -Under the hood this sources `wallet_harness.sh`, which reads the admin token and +Under the hood this sources `test_harness.sh`, which reads the admin token and the funded genesis key from the running `forest` container, exports `FULLNODE_API_INFO` (Forest RPC on port 3456) and `FOREST_TEST_PRELOADED_ADDRESS`, -then runs `forest-dev tests mpool` and `forest-dev tests wallet`. +then runs `forest-dev tests mpool`, `forest-dev tests wallet` and +`forest-dev devnet eth-gas`. + +The `devnet` suites (unlike `tests`) need both nodes reachable: `eth-gas` +compares Forest against Lotus, so it runs pre-flight checks and fails early if +the devnet or harness environment is missing. ## Local devnet development diff --git a/scripts/devnet/lotus.env b/scripts/devnet/lotus.env index 41092fd0515d..d9f84df583e9 100644 --- a/scripts/devnet/lotus.env +++ b/scripts/devnet/lotus.env @@ -3,3 +3,5 @@ LOTUS_MINER_PATH=/lotus_data/lotus-miner-local-net LOTUS_SKIP_GENESIS_CHECK=_yes_ LOTUS_API_LISTENADDRESS=/dns/lotus/tcp/1234/http LOTUS_LIBP2P_LISTENADDRESSES=/ip4/0.0.0.0/tcp/1235 +LOTUS_FEVM_ENABLEETHRPC=1 +LOTUS_CHAININDEXER_ENABLEINDEXER=1 diff --git a/scripts/devnet/wallet_harness.sh b/scripts/devnet/test_harness.sh similarity index 77% rename from scripts/devnet/wallet_harness.sh rename to scripts/devnet/test_harness.sh index 73705c630ecd..3ffa5c41ce17 100644 --- a/scripts/devnet/wallet_harness.sh +++ b/scripts/devnet/test_harness.sh @@ -1,18 +1,22 @@ #!/bin/bash -# Sourced (not executed) helpers for the wallet/mpool suite on the docker -# devnet. Run after the devnet is up (`setup.sh`) and synced (`check.sh`). +# Sourced (not executed) helpers for the devnet test suites (wallet, mpool, +# eth-gas) on the docker devnet. Run after the devnet is up (`setup.sh`) and synced (`check.sh`). # # The genesis key is the Lotus miner's default wallet, so using it as the test # sender causes nonce contention. We fund a dedicated wallet instead. -WALLET_HARNESS_PARENT_PATH=$( cd "$(dirname "${BASH_SOURCE[0]}")" ; pwd -P ) -source "${WALLET_HARNESS_PARENT_PATH}/.env" +TEST_HARNESS_PARENT_PATH=$( cd "$(dirname "${BASH_SOURCE[0]}")" ; pwd -P ) +# Export everything in `.env` so the test binaries inherit the devnet config (ports, data dirs) +# as environment variables; `source` on its own would only set shell variables. +set -a +source "${TEST_HARNESS_PARENT_PATH}/.env" +set +a export FOREST_CLI_PATH="${FOREST_CLI_PATH:-forest-cli}" export FOREST_WALLET_PATH="${FOREST_WALLET_PATH:-forest-wallet}" export DEVNET_TEST_FUND_AMT="${DEVNET_TEST_FUND_AMT:-100 FIL}" -function devnet_wallet_env_init { +function devnet_test_env_init { local token token=$(docker exec forest cat "${FOREST_DATA_DIR}/token.jwt") export FULLNODE_API_INFO="${token}:/ip4/127.0.0.1/tcp/${FOREST_RPC_PORT}/http" @@ -32,7 +36,7 @@ function devnet_wallet_env_init { ${FOREST_WALLET_PATH} --remote-wallet import "${test_key_path}" export FOREST_TEST_PRELOADED_ADDRESS="${test_addr}" - echo "Devnet wallet env initialised:" + echo "Devnet test env initialised:" echo " FULLNODE_API_INFO=:/ip4/127.0.0.1/tcp/${FOREST_RPC_PORT}/http" echo " FOREST_TEST_PRELOADED_ADDRESS=${FOREST_TEST_PRELOADED_ADDRESS}" echo " Funding ${test_addr} with ${DEVNET_TEST_FUND_AMT} from ${genesis_addr}..." diff --git a/src/dev/subcommands/devnet_cmd.rs b/src/dev/subcommands/devnet_cmd.rs new file mode 100644 index 000000000000..ded31996e978 --- /dev/null +++ b/src/dev/subcommands/devnet_cmd.rs @@ -0,0 +1,61 @@ +// Copyright 2019-2026 ChainSafe Systems +// SPDX-License-Identifier: Apache-2.0, MIT + +//! Integration suites that run against the local docker devnet. Unlike the unit +//! suite, these need a running devnet with both a Forest and a Lotus node reachable and the +//! test harness environment wired up. [`preflight`] fails early with actionable errors when +//! that environment is missing, rather than letting a suite surface it as an opaque mid-run error. + +mod eth_gas; + +use crate::dev::subcommands::tests_cmd::helpers::{docker, forest_client, lotus_client}; +use crate::rpc::prelude::*; +use anyhow::{Context as _, ensure}; + +/// Integration tests that require the docker devnet +#[derive(Debug, clap::Subcommand)] +pub enum DevnetCommand { + EthGas(eth_gas::EthGasTestCommand), +} + +impl DevnetCommand { + pub async fn run(self) -> anyhow::Result<()> { + preflight().await.context("devnet pre-flight failed")?; + match self { + Self::EthGas(cmd) => cmd.run().await, + } + } +} + +async fn preflight() -> anyhow::Result<()> { + for container in ["forest", "lotus"] { + let running = docker(&["inspect", "-f", "{{.State.Running}}", container]).with_context( + || format!("could not query container `{container}`; is docker running and the local docker devnet up?"), + )?; + ensure!( + running.trim() == "true", + "devnet container `{container}` is not running; bring the local docker devnet up first" + ); + } + + for var in [ + "FULLNODE_API_INFO", + "FOREST_TEST_PRELOADED_ADDRESS", + "LOTUS_RPC_PORT", + ] { + ensure!( + std::env::var_os(var).is_some(), + "{var} is not set; source the devnet test harness and run `devnet_test_env_init` first" + ); + } + + // Probe eth RPC specifically, not just `ChainHead`: the suites need it, and a devnet with eth + // RPC disabled would otherwise pass here and fail opaquely mid-suite. + for (node, client) in [("forest", forest_client()?), ("lotus", lotus_client()?)] { + EthBlockNumber::call(&client, ()).await.with_context(|| { + format!("{node} eth RPC is not reachable; is the local docker devnet up (with eth RPC enabled) and synced?") + })?; + } + + Ok(()) +} diff --git a/src/dev/subcommands/devnet_cmd/contracts/compile.sh b/src/dev/subcommands/devnet_cmd/contracts/compile.sh new file mode 100755 index 000000000000..2c133cd1c290 --- /dev/null +++ b/src/dev/subcommands/devnet_cmd/contracts/compile.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +# Regenerates the `.hex` bytecode beside every `.sol` fixture here. The tests consume only the +# `.hex`, which is checked in so neither CI nor a developer needs a Solidity toolchain. +# +# The compiler is pinned and each source pins the same version in its `pragma`: bytecode has to +# be reproducible, and a different compiler shifts the gas profile these tests measure. +set -euo pipefail + +SOLC_VERSION=0.8.30 +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)" + +for sol in "${DIR}"/*/*.sol; do + contract_dir="$(dirname "${sol}")" + name="$(basename "${sol}" .sol)" + echo "compiling ${name} with solc ${SOLC_VERSION}" + docker run --rm --volume "${contract_dir}:/src:ro" "ethereum/solc:${SOLC_VERSION}" \ + --bin "/src/${name}.sol" | + awk '/^Binary:/ { getline; print }' > "${contract_dir}/${name}.hex" +done diff --git a/src/dev/subcommands/devnet_cmd/contracts/nested_gas/nested_gas.hex b/src/dev/subcommands/devnet_cmd/contracts/nested_gas/nested_gas.hex new file mode 100644 index 000000000000..4271c57d4a16 --- /dev/null +++ b/src/dev/subcommands/devnet_cmd/contracts/nested_gas/nested_gas.hex @@ -0,0 +1 @@ +6080604052348015600e575f5ffd5b506104d98061001c5f395ff3fe608060405234801561000f575f5ffd5b506004361061003f575f3560e01c80637da1365e14610043578063a7f8d9fc14610061578063b2041d211461006b575b5f5ffd5b61004b610087565b6040516100589190610256565b60405180910390f35b61006961008c565b005b6100856004803603810190610080919061029d565b6100ec565b005b5f5481565b6302faf0805a116100d2576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016100c990610322565b60405180910390fd5b60015f5f8282546100e3919061036d565b92505081905550565b5f81036101105760015f5f828254610104919061036d565b9250508190555061023b565b5f3073ffffffffffffffffffffffffffffffffffffffff1663b2041d2160e01b60018461013d91906103a0565b60405160240161014d9190610256565b604051602081830303815290604052907bffffffffffffffffffffffffffffffffffffffffffffffffffffffff19166020820180517bffffffffffffffffffffffffffffffffffffffffffffffffffffffff83818316178352505050506040516101b79190610425565b5f604051808303815f865af19150503d805f81146101f0576040519150601f19603f3d011682016040523d82523d5f602084013e6101f5565b606091505b5050905080610239576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161023090610485565b60405180910390fd5b505b50565b5f819050919050565b6102508161023e565b82525050565b5f6020820190506102695f830184610247565b92915050565b5f5ffd5b61027c8161023e565b8114610286575f5ffd5b50565b5f8135905061029781610273565b92915050565b5f602082840312156102b2576102b161026f565b5b5f6102bf84828501610289565b91505092915050565b5f82825260208201905092915050565b7f676173206c696d697420746f6f206c6f770000000000000000000000000000005f82015250565b5f61030c6011836102c8565b9150610317826102d8565b602082019050919050565b5f6020820190508181035f83015261033981610300565b9050919050565b7f4e487b71000000000000000000000000000000000000000000000000000000005f52601160045260245ffd5b5f6103778261023e565b91506103828361023e565b925082820190508082111561039a57610399610340565b5b92915050565b5f6103aa8261023e565b91506103b58361023e565b92508282039050818111156103cd576103cc610340565b5b92915050565b5f81519050919050565b5f81905092915050565b8281835e5f83830152505050565b5f6103ff826103d3565b61040981856103dd565b93506104198185602086016103e7565b80840191505092915050565b5f61043082846103f5565b915081905092915050565b7f73756263616c6c206f7574206f662067617300000000000000000000000000005f82015250565b5f61046f6012836102c8565b915061047a8261043b565b602082019050919050565b5f6020820190508181035f83015261049c81610463565b905091905056fea2646970667358221220ee3eeb4b133ad268135c21b8298e2c4c79dbda3759a7da207271b8b0839ce53364736f6c634300081e0033 diff --git a/src/dev/subcommands/devnet_cmd/contracts/nested_gas/nested_gas.sol b/src/dev/subcommands/devnet_cmd/contracts/nested_gas/nested_gas.sol new file mode 100644 index 000000000000..2dab7795cc16 --- /dev/null +++ b/src/dev/subcommands/devnet_cmd/contracts/nested_gas/nested_gas.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT +pragma solidity =0.8.30; + +/// Each `recurse` level forwards at most 63/64 of the remaining gas (EIP-150: +/// https://github.com/ethereum/EIPs/blob/15f61ed0fda82ec86d8d6a872f6b874816f03d96/EIPS/eip-150.md#L32-L33), +/// so the gas *limit* the top-level call needs grows as (64/63)^depth above the +/// gas it actually *uses*. That gap is what `eth_estimateGas`'s search exists to close. +contract NestedGas { + uint256 public acc; + + /// Succeeds only when handed a large gas limit, and otherwise reverts explicitly rather than + /// running out of gas. Raising the limit would in fact fix it, but the estimator has no way to + /// know that, so this is the failure it must report instead of searching around. + function requiresHighGasLimit() external { + require(gasleft() > 50_000_000, "gas limit too low"); + acc += 1; + } + + function recurse(uint256 depth) external { + if (depth == 0) { + acc += 1; + return; + } + (bool ok, ) = address(this).call( + abi.encodeWithSelector(this.recurse.selector, depth - 1) + ); + require(ok, "subcall out of gas"); + } +} diff --git a/src/dev/subcommands/devnet_cmd/eth_gas.rs b/src/dev/subcommands/devnet_cmd/eth_gas.rs new file mode 100644 index 000000000000..eed63a1f47ee --- /dev/null +++ b/src/dev/subcommands/devnet_cmd/eth_gas.rs @@ -0,0 +1,346 @@ +// Copyright 2019-2026 ChainSafe Systems +// SPDX-License-Identifier: Apache-2.0, MIT + +//! `eth_estimateGas` parity tests against the Lotus node on the docker devnet. +//! +//! [EIP-150] caps a `CALL` at 63/64 of remaining gas, so a nested call chain needs a far higher +//! gas *limit* than the gas it *uses*. Estimating from gas used alone therefore under-shoots, +//! and the estimate has to be probed and raised until it succeeds. +//! +//! [EIP-150]: https://github.com/ethereum/EIPs/blob/15f61ed0fda82ec86d8d6a872f6b874816f03d96/EIPS/eip-150.md#L32-L33 + +use crate::dev::subcommands::tests_cmd::helpers::*; +use crate::rpc::Client; +use crate::rpc::eth::errors::EXECUTION_REVERTED_CODE; +use crate::rpc::eth::{ + BlockNumberOrHash, Predefined, + types::{EthAddress, EthBytes, EthCallMessage}, +}; +use crate::rpc::prelude::*; +use crate::shim::address::Address; +use crate::utils::encoding::{hex, keccak_256}; +use anyhow::{Context as _, ensure}; +use cid::Cid; +use jsonrpsee::core::ClientError; +use libtest_mimic::{Arguments, Failed, Trial}; +use std::io::Write as _; +use std::str::FromStr as _; +use tempfile::NamedTempFile; +use tokio::sync::OnceCell; + +/// `NestedGas`, whose `recurse(uint256)` calls itself that many times. +/// Regenerate with `contracts/compile.sh` after editing the source. +const NESTED_GAS_HEX: &str = include_str!("contracts/nested_gas/nested_gas.hex"); +const RECURSE_SIGNATURE: &str = "recurse(uint256)"; +/// Reverts explicitly unless given a large gas limit, so estimating it fails for a reason no +/// amount of extra gas can be shown to fix. +const REQUIRES_HIGH_GAS_SIGNATURE: &str = "requiresHighGasLimit()"; +/// The `require` string in [`REQUIRES_HIGH_GAS_SIGNATURE`]. +const REVERT_REASON: &str = "gas limit too low"; +/// Both implementations prefix this branch's error with it. Asserting on it pins *which* rejection +/// happened: a message that failed earlier, during plain gas estimation, would never carry it. +const GAS_SEARCH_FAILURE: &str = "gas search failed"; +const HEX_IN_CONTAINER: &str = "/tmp/nested_gas.hex"; + +/// Shallow enough that the 63/64 penalty stays inside any estimator's safety margin, so both +/// nodes must agree. Guards against a failure that is really "the two disagree about gas". +const CONTROL_DEPTH: u64 = 0; +/// Deep enough that the penalty is ~1.9x, well clear of the crossover measured around 40-60. +const NESTED_DEPTH: u64 = 100; +/// The nested call needs a gas limit in the hundreds of millions, and a sender that cannot +/// afford it makes the estimate saturate at the block gas limit instead of converging. +const SENDER_FUND_AMT: &str = "10 FIL"; + +/// `eth_estimateGas` parity tests +#[derive(Debug, clap::Args)] +pub struct EthGasTestCommand {} + +impl EthGasTestCommand { + pub async fn run(self) -> anyhow::Result<()> { + let args = Arguments { + test_threads: Some(1), + ..Default::default() + }; + libtest_mimic::run(&args, tests()).exit(); + } +} + +fn tests() -> Vec { + fn trial(name: &'static str, body: fn() -> anyhow::Result<()>) -> Trial { + Trial::test(name, move || { + body().map_err(|e| Failed::from(format!("{e:?}"))) + }) + } + + vec![ + trial("eth_estimate_gas_agrees_without_nesting", || { + block_on(estimate_agrees(CONTROL_DEPTH)) + }), + trial("eth_estimate_gas_agrees_with_nesting", || { + block_on(estimate_agrees(NESTED_DEPTH)) + }), + trial("eth_estimate_gas_is_sufficient_on_chain", || { + block_on(estimate_is_sufficient_on_chain()) + }), + trial("eth_estimate_gas_reports_a_non_gas_failure", || { + block_on(estimate_reports_a_non_gas_failure()) + }), + ] +} + +/// The 4-byte Ethereum function selector: first 4 bytes of `keccak256(signature)`. +fn selector(signature: &str) -> Vec { + keccak_256(signature.as_bytes()) + .get(..4) + .expect("keccak256 is 32 bytes") + .to_vec() +} + +/// ABI calldata for `recurse(uint256)`: the selector followed by `depth` as a 32-byte word. +fn recurse_calldata(depth: u64) -> Vec { + let mut out = selector(RECURSE_SIGNATURE); + out.extend_from_slice(ðereum_types::U256::from(depth).to_big_endian()); + out +} + +/// Deployed `NestedGas` addresses: `eth` for the JSON-RPC calls, `f4` as the `lotus send` target. +struct Deployed { + eth: EthAddress, + f4: Address, +} + +/// Deploys `NestedGas` once per process. +async fn contract() -> anyhow::Result<&'static Deployed> { + static CONTRACT: OnceCell = OnceCell::const_new(); + CONTRACT + .get_or_try_init(|| async { + let mut hex_file = NamedTempFile::new_in(std::env::temp_dir()) + .context("staging the contract bytecode")?; + hex_file.write_all(NESTED_GAS_HEX.trim().as_bytes())?; + hex_file.flush()?; + docker(&[ + "cp", + &hex_file.path().to_string_lossy(), + &format!("lotus:{HEX_IN_CONTAINER}"), + ])?; + + let deploy = lotus_exec(&["evm", "deploy", "--hex", HEX_IN_CONTAINER])?; + let f4 = deploy + .lines() + .find_map(|l| l.trim().strip_prefix("f4 Address: ")) + .with_context(|| format!("no `f4 Address:` in deploy output:\n{deploy}"))?; + let f4 = Address::from_str(f4.trim()).context("parsing the deployed f4 address")?; + eprintln!("deployed NestedGas at {f4}"); + anyhow::Ok(Deployed { + eth: EthAddress::from_filecoin_address(&f4)?, + f4, + }) + }) + .await +} + +/// An `f4` sender funded well enough to afford the gas limits under test. Lotus rejects +/// estimation from an unfunded or non-`f4` sender, so both properties are required. +/// +/// Created in Lotus's keystore rather than Forest's: estimation only needs the address to +/// exist on chain, while submitting needs whoever signs to hold the key. +async fn sender_addr() -> anyhow::Result<&'static Address> { + static SENDER: OnceCell
= OnceCell::const_new(); + SENDER + .get_or_try_init(|| async { + let addr = lotus_exec(&["wallet", "new", "delegated"])?; + let msg = send_from( + &FOREST_TEST_PRELOADED_ADDRESS, + &addr, + SENDER_FUND_AMT, + Backend::Local, + )?; + eprintln!("funding sender {addr} with {SENDER_FUND_AMT}, msg: {msg}"); + let balance = poll_until_funded(&addr, Backend::Local).await?; + eprintln!("sender {addr} funded balance: {balance}"); + Address::from_str(&addr).context("parsing the sender address") + }) + .await +} + +async fn estimate( + client: &Client, + calldata: Vec, + block: BlockNumberOrHash, +) -> anyhow::Result { + let (sender, deployed) = tokio::try_join!(sender_addr(), contract())?; + let msg = EthCallMessage { + from: Some(EthAddress::from_filecoin_address(sender)?), + to: Some(deployed.eth), + data: Some(EthBytes(calldata)), + ..Default::default() + }; + let gas = client + .call(EthEstimateGas::request((msg, Some(block)))?) + .await?; + Ok(gas.0) +} + +/// A height both nodes have already executed. `Latest` is resolved per node, so at an epoch +/// boundary or under slight sync skew the two could pick different tipsets; pinning both to the +/// lower of their heads makes the cross-node comparison deterministic. +async fn common_block_number(a: &Client, b: &Client) -> anyhow::Result { + let (head_a, head_b) = tokio::try_join!( + async { anyhow::Ok(a.call(EthBlockNumber::request(())?).await?) }, + async { anyhow::Ok(b.call(EthBlockNumber::request(())?).await?) }, + )?; + Ok(head_a.0.min(head_b.0) as i64) +} + +/// Deploy + fund, build both node clients, and pin a block height both have executed. Sampling the +/// height only after the deploy/fund guarantees the pinned tipset already contains the contract and +/// sender on both nodes (the funding poll also lets both catch up to the deploy). +async fn pinned_common_block() -> anyhow::Result<(Client, Client, i64)> { + tokio::try_join!(contract(), sender_addr())?; + let (forest_c, lotus_c) = (forest_client()?, lotus_client()?); + let block = common_block_number(&forest_c, &lotus_c).await?; + Ok((forest_c, lotus_c, block)) +} + +/// Forest and Lotus must return the same estimate. +async fn estimate_agrees(depth: u64) -> anyhow::Result<()> { + let (forest_c, lotus_c, block) = pinned_common_block().await?; + let (forest, lotus) = tokio::try_join!( + async { + estimate( + &forest_c, + recurse_calldata(depth), + BlockNumberOrHash::from_block_number(block), + ) + .await + .context("EthEstimateGas on forest") + }, + async { + estimate( + &lotus_c, + recurse_calldata(depth), + BlockNumberOrHash::from_block_number(block), + ) + .await + .context("EthEstimateGas on lotus") + }, + )?; + eprintln!("depth={depth} block={block} forest={forest} lotus={lotus}"); + ensure!( + forest == lotus, + "eth_estimateGas disagrees at recursion depth {depth} (block {block}): forest={forest} lotus={lotus}" + ); + Ok(()) +} + +/// The estimate Forest returns must actually be enough to land the transaction. +async fn estimate_is_sufficient_on_chain() -> anyhow::Result<()> { + let forest = forest_client()?; + // No cross-node comparison here, so `Latest` is fine: the estimate must reflect the same + // fresh state the following `lotus send` executes against. + let estimate = estimate( + &forest, + recurse_calldata(NESTED_DEPTH), + BlockNumberOrHash::PredefinedBlock(Predefined::Latest), + ) + .await?; + let sender = sender_addr().await?.to_string(); + let target = contract().await?.f4.to_string(); + let params = hex::encode(recurse_calldata(NESTED_DEPTH)); + let gas_limit = estimate.to_string(); + // `lotus send` infers `InvokeContract` and CBOR-wraps the params when the sender is an + // eth account, and rejects an explicit `--method`, so pass the bare calldata. Retry the + // submit while Lotus's mpool briefly lags the freshly funded sender. + let out = lotus_exec_retrying_mpool(&[ + "send", + "--from", + &sender, + "--params-hex", + ¶ms, + "--gas-limit", + &gas_limit, + &target, + "0", + ]) + .await?; + let cid = out + .lines() + .last() + .context("no cid from `lotus send`")? + .trim(); + eprintln!("submitted at forest's estimate {estimate}: {cid}"); + + let lookup = forest + .call( + StateWaitMsg::request((Cid::from_str(cid)?, 0, 800, true))?.with_timeout(POLL_TIMEOUT), + ) + .await?; + let exit = lookup.receipt.exit_code(); + ensure!( + exit.is_success(), + "a transaction submitted at forest's own eth_estimateGas value ({estimate}) failed \ + on chain with exit code {exit}; the estimate is not a usable gas limit" + ); + Ok(()) +} + +/// A failure that raising the gas limit cannot be shown to fix must be reported, not searched +/// around. This is the companion of [`estimate_agrees`]: it pins the branch that decides whether +/// a failed probe means "needs more gas" or "is simply broken". +async fn estimate_reports_a_non_gas_failure() -> anyhow::Result<()> { + let (forest_c, lotus_c, block) = pinned_common_block().await?; + for (node, client) in [("forest", &forest_c), ("lotus", &lotus_c)] { + let err = match estimate( + client, + selector(REQUIRES_HIGH_GAS_SIGNATURE), + BlockNumberOrHash::from_block_number(block), + ) + .await + { + Ok(gas) => anyhow::bail!( + "{node} returned an estimate ({gas}) for a message that reverts at that limit; \ + a non-gas failure must be reported, not answered with a gas value" + ), + Err(e) => e, + }; + let Some(ClientError::Call(obj)) = err.downcast_ref::() else { + anyhow::bail!("{node} returned a non-JSON-RPC error, cannot check parity: {err:?}"); + }; + eprintln!( + "{node} rejected the call: code={} has_data={} msg={}", + obj.code(), + obj.data().is_some(), + obj.message() + ); + // Cross-node parity: both name the branch ("gas search failed") and the decoded revert reason. + ensure!( + obj.message().contains(GAS_SEARCH_FAILURE), + "{node} rejected the call before the gas search, so this no longer exercises the \ + branch it is meant to pin (expected `{GAS_SEARCH_FAILURE}`): {}", + obj.message() + ); + ensure!( + obj.message().contains(REVERT_REASON), + "{node} rejected the call without naming the revert reason `{REVERT_REASON}`: {}", + obj.message() + ); + + // Forest returns eth-standard `execution reverted` (code 3) + data, matching current Lotus. + // The devnet's Lotus image predates that refactor (generic code, no data), so code/data + // parity is pinned on Forest alone. + if node == "forest" { + ensure!( + obj.code() == EXECUTION_REVERTED_CODE, + "forest rejected with code {}, expected execution-reverted {EXECUTION_REVERTED_CODE}: {}", + obj.code(), + obj.message() + ); + ensure!( + obj.data().is_some(), + "forest rejected without revert data; eth clients cannot ABI-decode the reason: {}", + obj.message() + ); + } + } + Ok(()) +} diff --git a/src/dev/subcommands/mod.rs b/src/dev/subcommands/mod.rs index 3efcfc0b26aa..a9acf9416a41 100644 --- a/src/dev/subcommands/mod.rs +++ b/src/dev/subcommands/mod.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0, MIT mod archive_missing_cmd; +mod devnet_cmd; mod export_state_tree_cmd; mod export_tipset_lookup_cmd; mod state_cmd; @@ -56,6 +57,9 @@ pub enum Subcommand { ExportStateTree(export_state_tree_cmd::ExportStateTreeCommand), #[command(subcommand)] Tests(tests_cmd::TestsCommand), + /// Integration tests that require the local docker devnet + #[command(subcommand)] + Devnet(devnet_cmd::DevnetCommand), } impl Subcommand { @@ -68,6 +72,7 @@ impl Subcommand { Self::ExportTipsetLookup(cmd) => cmd.run().await, Self::ExportStateTree(cmd) => cmd.run().await, Self::Tests(cmd) => cmd.run().await, + Self::Devnet(cmd) => cmd.run().await, } } } diff --git a/src/dev/subcommands/tests_cmd.rs b/src/dev/subcommands/tests_cmd.rs index 58a36ee3fc5f..9d63f42a419d 100644 --- a/src/dev/subcommands/tests_cmd.rs +++ b/src/dev/subcommands/tests_cmd.rs @@ -1,7 +1,7 @@ // Copyright 2019-2026 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -mod helpers; +pub(crate) mod helpers; mod mpool; mod wallet; diff --git a/src/dev/subcommands/tests_cmd/helpers.rs b/src/dev/subcommands/tests_cmd/helpers.rs index 8c76e1a752a7..dfcd97ea6e98 100644 --- a/src/dev/subcommands/tests_cmd/helpers.rs +++ b/src/dev/subcommands/tests_cmd/helpers.rs @@ -63,20 +63,24 @@ pub fn wallet(backend: Backend, args: &[&str]) -> anyhow::Result { .to_string()) } -/// Same as [`wallet`] but yields raw stdout bytes (used by `export`). +/// Same as [`wallet`] but yields raw stdout bytes. pub fn run_wallet_raw(backend: Backend, args: &[&str]) -> anyhow::Result> { let mut full = Vec::with_capacity(backend.extra_args().len() + args.len()); full.extend_from_slice(backend.extra_args()); full.extend_from_slice(args); + run("forest-wallet", &full) +} - let output = Command::new("forest-wallet") - .args(&full) +/// Runs `program args...` and returns raw stdout, with stderr surfaced on failure. +fn run(program: &str, args: &[&str]) -> anyhow::Result> { + let output = Command::new(program) + .args(args) .output() - .context("failed to spawn `forest-wallet`")?; + .with_context(|| format!("failed to spawn `{program}`"))?; if !output.status.success() { bail!( - "`forest-wallet {}` failed (status={}): {}", - full.join(" "), + "`{program} {}` failed (status={}): {}", + args.join(" "), output.status, String::from_utf8_lossy(&output.stderr) ); @@ -84,6 +88,11 @@ pub fn run_wallet_raw(backend: Backend, args: &[&str]) -> anyhow::Result Ok(output.stdout) } +/// [`run`] with stdout as trimmed UTF-8. +fn run_str(program: &str, args: &[&str]) -> anyhow::Result { + Ok(String::from_utf8(run(program, args)?)?.trim().to_owned()) +} + /// Export `address` from the chosen backend into a temp file ready to feed /// back to `forest-wallet import`. pub fn export_to_temp_file(address: &str, backend: Backend) -> anyhow::Result { @@ -177,6 +186,20 @@ pub async fn poll_until_funded(address: &str, backend: Backend) -> anyhow::Resul poll_until_changed(address, FIL_ZERO, backend).await } +/// Run a `lotus` command, retrying while it fails with the transient mpool `check has failed` +/// (the mpool briefly lags the chain head just after the sender is funded, so a submit can be +/// rejected until it catches up). Any other failure propagates immediately. +pub async fn lotus_exec_retrying_mpool(args: &[&str]) -> anyhow::Result { + poll(&format!("lotus {}", args.join(" ")), || async { + match lotus_exec(args) { + Ok(out) => Ok(Some(out)), + Err(e) if format!("{e:#}").contains("check has failed") => Ok(None), + Err(e) => Err(e), + } + }) + .await +} + /// Delegated signer: create once on local, fund locally, mirror to remote /// for tests that query or sign. pub async fn funded_delegated_addr() -> &'static str { @@ -245,6 +268,31 @@ fn api() -> anyhow::Result<&'static (String, String)> { .map_err(|e| anyhow::anyhow!("FULLNODE_API_INFO unavailable: {e}")) } +/// Typed client for the Lotus node on the docker devnet, whose read methods need no token. +pub fn lotus_client() -> anyhow::Result { + let port = std::env::var("LOTUS_RPC_PORT") + .context("LOTUS_RPC_PORT not set; source the devnet test harness")?; + Ok(crate::rpc::Client::from_url( + format!("http://127.0.0.1:{port}/").parse()?, + )) +} + +/// Typed client for the Forest node under test, from `FULLNODE_API_INFO`. +pub fn forest_client() -> anyhow::Result { + crate::rpc::Client::default_or_from_env(None) +} + +pub fn docker(args: &[&str]) -> anyhow::Result { + run_str("docker", args).context("is the devnet up?") +} + +/// Runs `lotus ` inside the `lotus` container on the docker devnet. +pub fn lotus_exec(args: &[&str]) -> anyhow::Result { + let mut full = vec!["exec", "lotus", "lotus"]; + full.extend_from_slice(args); + docker(&full) +} + /// POST a JSON-RPC v1 request and return the `result` field, or `None` if /// the server responded without one. pub async fn rpc_call_opt(method: &str, params: Value) -> anyhow::Result> { @@ -324,21 +372,8 @@ pub async fn poll_until_state_search_msg(msg_cid: &str) -> anyhow::Result<()> { .await } -/// Run `forest-cli ` and return trimmed stdout. pub fn forest_cli(args: &[&str]) -> anyhow::Result { - let output = Command::new("forest-cli") - .args(args) - .output() - .context("failed to spawn `forest-cli`")?; - if !output.status.success() { - bail!( - "`forest-cli {}` failed (status={}): {}", - args.join(" "), - output.status, - String::from_utf8_lossy(&output.stderr) - ); - } - Ok(String::from_utf8(output.stdout)?.trim().to_string()) + run_str("forest-cli", args) } /// Next nonce for an address @@ -388,9 +423,9 @@ pub async fn filecoin_to_eth(address: &str) -> anyhow::Result { .with_context(|| format!("expected string ETH address, got {result}")) } -pub fn block_on(future: F) -> F::Output +pub fn block_on(future: F) -> F::Output where - F::Output: Send + Sync + 'static, + F::Output: Send + 'static, { std::thread::spawn(|| { let rt = tokio::runtime::Builder::new_multi_thread() @@ -400,5 +435,6 @@ where rt.block_on(future) }) .join() - .unwrap() + // Preserve the panic message instead of `unwrap`'s `Any { .. }`. + .unwrap_or_else(|payload| std::panic::resume_unwind(payload)) } diff --git a/src/rpc/methods/eth.rs b/src/rpc/methods/eth.rs index f7a302e054c0..e47631ef711c 100644 --- a/src/rpc/methods/eth.rs +++ b/src/rpc/methods/eth.rs @@ -33,6 +33,7 @@ use crate::eth::{ EAMMethod, EVMMethod, EthChainId as EthChainIdType, EthEip1559TxArgs, EthLegacyEip155TxArgs, EthLegacyHomesteadTxArgs, parse_eth_transaction, }; +use crate::interpreter::VMTrace; use crate::lotus_json::{HasLotusJson, NotNullVec, lotus_json_with_self}; use crate::message::{ChainMessage, MessageRead as _, MessageReadWrite as _, SignedMessage}; use crate::networks::Height; @@ -62,7 +63,6 @@ use crate::shim::fvm_shared_latest::MethodNum; use crate::shim::fvm_shared_latest::address::{Address as VmAddress, DelegatedAddress}; use crate::shim::gas::GasOutputs; use crate::shim::message::Message; -use crate::shim::trace::{CallReturn, ExecutionEvent}; use crate::shim::{clock::ChainEpoch, state_tree::StateTree}; use crate::state_manager::{ExecutedMessage, ExecutedTipset, StateManager, TipsetState, VMFlush}; use crate::utils::cache::SizeTrackingCache; @@ -1905,6 +1905,17 @@ async fn eth_estimate_gas( } } +/// Builds an eth `ExecutionReverted` (code 3) from a failed message's exit code and return +/// payload, decoding the revert reason and data. +fn execution_reverted_error( + exit_code: impl Into, + return_data: RawBytes, + vm_error: &str, +) -> EthErrors { + let (data, reason) = decode_revert_reason(return_data); + EthErrors::execution_reverted(exit_code.into(), &reason, vm_error, &data) +} + async fn apply_message( ctx: &Ctx, tipset: Option, @@ -1921,7 +1932,7 @@ async fn apply_message( let (invoc_res, _) = ctx .state_manager - .apply_on_state_with_gas(tipset, msg, VMFlush::Skip) + .apply_on_state_with_gas(tipset, msg, VMFlush::Skip, VMTrace::NotTraced) .await .context("failed to apply on state with gas")?; @@ -1930,13 +1941,10 @@ async fn apply_message( None => return Err(anyhow::anyhow!("no message receipt in execution result")), Some(receipt) => { if !receipt.exit_code().is_success() { - let (data, reason) = decode_revert_reason(receipt.return_data()); - - return Err(EthErrors::execution_reverted( - ExitCode::from(receipt.exit_code()), - reason.as_str(), - invoc_res.error.as_str(), - data.as_slice(), + return Err(execution_reverted_error( + receipt.exit_code(), + receipt.return_data(), + &invoc_res.error, ) .into()); } @@ -1947,32 +1955,45 @@ async fn apply_message( } pub async fn eth_gas_search(data: &Ctx, msg: Message, tsk: &ApiTipsetKey) -> anyhow::Result { - let (apply_ret, prior_messages, ts) = - gas::GasEstimateGasLimit::estimate_call_with_gas(data, msg.clone(), tsk).await?; + // Probe the message as the caller specified it: the question is whether *its* limit + // suffices, which the block maximum would always answer yes to. + let (apply_ret, prior_messages, ts, from) = + gas::GasEstimateGasLimit::probe_as_specified(data, msg.clone(), tsk, VMTrace::NotTraced) + .await?; if apply_ret.exit_code().is_success() { return Ok(msg.gas_limit()); } - let exec_trace = apply_ret.exec_trace(); - let _expected_exit_code: ExitCode = fvm_shared4::error::ExitCode::SYS_OUT_OF_GAS.into(); - if exec_trace.iter().any(|t| { - matches!( - t, - &ExecutionEvent::CallReturn(CallReturn { - exit_code: Some(_expected_exit_code), - .. - }) + // Only the trace tells "needs a higher limit" from "fails at any limit", and it is worth one + // re-execution here against the ~30 the search below would spend. The statement keeps the + // trace, one event per gas charge, from outliving the check. + let out_of_gas = data + .state_manager + .call_with_gas( + ChainMessage::for_gas_estimation(msg.clone(), from.protocol()), + prior_messages.shallow_clone(), + Some(ts.shallow_clone()), + VMFlush::Skip, + VMTrace::Traced, ) - }) { - let ret = gas_search(data, &msg, prior_messages, ts).await?; - Ok(((ret as f64) * data.mpool.gas_limit_overestimation()) as u64) - } else { - anyhow::bail!( - "message execution failed: exit {}, reason: {}", + .await? + .0 + .trace_has_call_return_exit_code(fvm_shared4::error::ExitCode::SYS_OUT_OF_GAS); + if !out_of_gas { + // Match Lotus: a code-3 `ExecutionReverted` with the decoded revert data, so eth tooling + // gets the code and can ABI-decode the reason. + let vm_error = apply_ret.failure_info().unwrap_or_default(); + return Err(execution_reverted_error( apply_ret.exit_code(), - apply_ret.failure_info().unwrap_or_default(), - ); + apply_ret.return_data(), + &vm_error, + ) + .with_message_prefix("gas search failed") + .into()); } + + let ret = gas_search(data, &msg, from.protocol(), prior_messages, ts).await?; + Ok(((ret as f64) * data.mpool.gas_limit_overestimation()) as u64) } /// `gas_search` does an exponential search to find a gas value to execute the @@ -1982,37 +2003,32 @@ pub async fn eth_gas_search(data: &Ctx, msg: Message, tsk: &ApiTipsetKey) -> any async fn gas_search( data: &Ctx, msg: &Message, + from_protocol: Protocol, prior_messages: Arc>, ts: Tipset, ) -> anyhow::Result { - let mut high = msg.gas_limit; - let mut low = msg.gas_limit; - - async fn can_succeed( - data: &Ctx, - mut msg: Message, - prior_messages: Arc>, - ts: Tipset, - limit: u64, - ) -> anyhow::Result { + // `max(1)` keeps the doubling below able to make progress. + let mut high = msg.gas_limit.max(1); + let mut low = high; + + let can_succeed = async |limit: u64| { + let mut msg = msg.clone(); msg.gas_limit = limit; let (apply_ret, ..) = data .state_manager - .call_with_gas(msg.into(), prior_messages, Some(ts), VMFlush::Skip) + .call_with_gas( + ChainMessage::for_gas_estimation(msg, from_protocol), + prior_messages.shallow_clone(), + Some(ts.shallow_clone()), + VMFlush::Skip, + VMTrace::NotTraced, + ) .await?; - Ok(apply_ret.exit_code().is_success()) - } + anyhow::Ok(apply_ret.exit_code().is_success()) + }; while high < BLOCK_GAS_LIMIT { - if can_succeed( - data, - msg.clone(), - prior_messages.shallow_clone(), - ts.shallow_clone(), - high, - ) - .await? - { + if can_succeed(high).await? { break; } low = high; @@ -2022,15 +2038,7 @@ async fn gas_search( let mut check_threshold = high / 100; while (high - low) > check_threshold { let median = (high + low) / 2; - if can_succeed( - data, - msg.clone(), - prior_messages.shallow_clone(), - ts.shallow_clone(), - median, - ) - .await? - { + if can_succeed(median).await? { high = median; } else { low = median; @@ -3874,7 +3882,12 @@ impl RpcMethod<3> for EthTraceCall { let (invoke_result, post_state_root) = ctx .state_manager - .apply_on_state_with_gas(Some(ts.shallow_clone()), msg.clone(), VMFlush::Flush) + .apply_on_state_with_gas( + Some(ts.shallow_clone()), + msg.clone(), + VMFlush::Flush, + VMTrace::Traced, + ) .await .context("failed to apply message")?; let post_state_root = @@ -4212,6 +4225,39 @@ mod test { } } + #[test] + fn execution_reverted_from_receipt_decodes_reason_and_data() { + // A receipt carries its return payload CBOR-wrapped. The `Error(string)` ABI decode is + // covered by `eth/utils.rs`; here a non-ABI payload (which decodes to hex) is enough to + // check that the helper threads reason, data, exit code and vm error through. + let payload = b"Ph'nglui mglw'nafh Cthulhu R'lyeh wgah'nagl fhtagn".to_vec(); + let return_data = + RawBytes::new(fvm_ipld_encoding::to_vec(&RawBytes::new(payload.clone())).unwrap()); + let receipt = Receipt::V4(fvm_shared4::receipt::Receipt { + exit_code: fvm_shared4::error::ExitCode::new(33), + return_data, + gas_used: 0, + events_root: None, + }); + + let err = execution_reverted_error(receipt.exit_code(), receipt.return_data(), "backtrace"); + let EthErrors::ExecutionReverted { message, data } = err.clone() else { + panic!("expected ExecutionReverted, got {err:?}"); + }; + let payload_hex = hex::encode_prefixed(&payload); + assert_eq!( + message, + format!( + "message execution failed (exit=[33], revert reason=[{payload_hex}], vm error=[backtrace])" + ) + ); + assert_eq!(data, payload_hex); + + // Surfaces to eth clients as the standard code-3 execution-reverted error. + let server: ServerError = err.into(); + assert_eq!(server.code(), errors::EXECUTION_REVERTED_CODE); + } + #[rstest] // Non-empty access list → JSON array. #[case::populated_array(ApiEthTx { access_list: Some(NotNullVec(vec![EthHash::default()])), ..Default::default() }, Some(1))] diff --git a/src/rpc/methods/eth/errors.rs b/src/rpc/methods/eth/errors.rs index 1cf9f53bfe6b..300a5231b5b0 100644 --- a/src/rpc/methods/eth/errors.rs +++ b/src/rpc/methods/eth/errors.rs @@ -51,6 +51,19 @@ impl EthErrors { } } + /// Prepends `prefix` to the message, keeping the code and data. Needed because the RPC layer + /// rebuilds the wire message from the typed error alone, dropping any `anyhow` context. + #[must_use] + pub fn with_message_prefix(mut self, prefix: &str) -> Self { + match &mut self { + Self::ExecutionReverted { message, .. } | Self::BlockRangeExceeded { message, .. } => { + *message = format!("{prefix}: {message}"); + } + Self::EventsNotYetAvailable | Self::NullRound { .. } => {} + } + self + } + pub fn limit_exceeded(max_block_range: i64, given: i64) -> Self { Self::BlockRangeExceeded { max: max_block_range, @@ -167,4 +180,32 @@ mod tests { "events for the requested block are not yet available" ); } + + #[test] + fn test_with_message_prefix_prepends_and_preserves_code_and_data() { + let err = EthErrors::execution_reverted( + ExitCode::from(33u32), + "boom", + "backtrace", + &[0xde, 0xad], + ) + .with_message_prefix("gas search failed"); + let server_err: ServerError = err.into(); + + assert_eq!(server_err.code(), EXECUTION_REVERTED_CODE); + assert_eq!( + server_err.message(), + "gas search failed: message execution failed (exit=[33], revert reason=[boom], vm error=[backtrace])" + ); + assert_eq!( + server_err.data().map(|d| d.to_string()), + Some("\"0xdead\"".to_string()) + ); + } + + #[test] + fn test_with_message_prefix_is_a_noop_for_messageless_variants() { + let err = EthErrors::null_round(7).with_message_prefix("ignored"); + assert_eq!(err.to_string(), "requested epoch was a null round (7)"); + } } diff --git a/src/rpc/methods/gas.rs b/src/rpc/methods/gas.rs index cd44aaa1eb37..031c6a30e538 100644 --- a/src/rpc/methods/gas.rs +++ b/src/rpc/methods/gas.rs @@ -3,13 +3,13 @@ use crate::blocks::Tipset; use crate::chain::{BASE_FEE_MAX_CHANGE_DENOM, BLOCK_GAS_TARGET}; -use crate::message::{ChainMessage, MessageRead as _, MessageReadWrite as _, SignedMessage}; +use crate::interpreter::VMTrace; +use crate::message::{ChainMessage, MessageRead as _, MessageReadWrite as _}; use crate::prelude::*; use crate::rpc::{ApiPaths, Ctx, Permission, RpcMethod, error::ServerError, types::*}; use crate::shim::executor::ApplyRet; use crate::shim::{ - address::{Address, Protocol}, - crypto::{SECP_SIG_LEN, Signature, SignatureType}, + address::Address, econ::{BLOCK_GAS_LIMIT, TokenAmount}, message::Message, }; @@ -205,15 +205,28 @@ impl RpcMethod<2> for GasEstimateGasLimit { } impl GasEstimateGasLimit { - pub async fn estimate_call_with_gas( + /// Runs `msg` at the block gas limit with the fees zeroed, to measure what it uses. + pub async fn measure_gas_used( data: &Ctx, mut msg: Message, - ApiTipsetKey(tsk): &ApiTipsetKey, - ) -> anyhow::Result<(ApplyRet, Arc>, Tipset)> { + tsk: &ApiTipsetKey, + ) -> anyhow::Result<(ApplyRet, Arc>, Tipset, Address)> { msg.set_gas_limit(BLOCK_GAS_LIMIT); msg.set_gas_fee_cap(TokenAmount::from_atto(0)); msg.set_gas_premium(TokenAmount::from_atto(0)); + Self::probe_as_specified(data, msg, tsk, VMTrace::NotTraced).await + } + /// Runs `msg` exactly as given. The limit and fees are left alone: a gas search compares this + /// against runs of the same message, so rewriting either here would measure something the + /// caller never asked about. Also returns the resolved sender so a caller can avoid + /// resolving it a second time. + pub async fn probe_as_specified( + data: &Ctx, + msg: Message, + ApiTipsetKey(tsk): &ApiTipsetKey, + vm_trace: VMTrace, + ) -> anyhow::Result<(ApplyRet, Arc>, Tipset, Address)> { let curr_ts = data.chain_store().load_required_tipset_or_heaviest(tsk)?; let from_a = data .state_manager @@ -227,23 +240,8 @@ impl GasEstimateGasLimit { .into(); let ts = data.mpool.current_tipset(); - // Pretend that the message is signed. This has an influence on the gas - // cost. We obviously can't generate a valid signature. Instead, we just - // fill the signature with zeros. The validity is not checked. - let chain_msg = match from_a.protocol() { - Protocol::Secp256k1 => { - SignedMessage::new_unchecked(msg, Signature::new_secp256k1(vec![0; SECP_SIG_LEN])) - .into() - } - Protocol::Delegated => SignedMessage::new_unchecked( - msg, - // In Lotus, delegated signatures have the same length as SECP256k1. - // This may or may not change in the future. - Signature::new(SignatureType::Delegated, vec![0; SECP_SIG_LEN]), - ) - .into(), - _ => msg.into(), - }; + // A zeroed signature, because its length changes the inclusion cost. + let chain_msg = ChainMessage::for_gas_estimation(msg, from_a.protocol()); let (apply_ret, ..) = data .state_manager @@ -252,13 +250,14 @@ impl GasEstimateGasLimit { prior_messages.shallow_clone(), Some(ts.shallow_clone()), VMFlush::Skip, + vm_trace, ) .await?; - Ok((apply_ret, prior_messages, ts)) + Ok((apply_ret, prior_messages, ts, from_a)) } pub async fn estimate_gas_limit(data: &Ctx, msg: Message, tsk: &ApiTipsetKey) -> Result { - let (apply_ret, ..) = Self::estimate_call_with_gas(data, msg, tsk) + let (apply_ret, ..) = Self::measure_gas_used(data, msg, tsk) .await .context("gas estimation failed")?; anyhow::ensure!( diff --git a/src/shim/executor.rs b/src/shim/executor.rs index b4d793430be6..ee3c10973b71 100644 --- a/src/shim/executor.rs +++ b/src/shim/executor.rs @@ -53,6 +53,11 @@ impl ApplyRet { delegate_apply_ret!(self.msg_receipt.clone().into()) } + /// The message's return payload, cloned without cloning the rest of the receipt. + pub fn return_data(&self) -> RawBytes { + delegate_apply_ret!(self => |r| r.msg_receipt.return_data.clone()) + } + pub fn exit_code(&self) -> ExitCode { ExitCode::new(delegate_apply_ret!(self => |r| r.msg_receipt.exit_code.value())) } @@ -77,6 +82,27 @@ impl ApplyRet { delegate_apply_ret!(self.over_estimation_burn.borrow().into()) } + /// Whether any traced `CallReturn` carries `code`, without materializing the trace. + /// Mirrors Lotus's `traceContainsExitCode`. FVM2 `CallReturn`s carry no exit code, so they + /// never match. + pub fn trace_has_call_return_exit_code(&self, code: ExitCode) -> bool { + let want = code.value(); + match self { + ApplyRet::V2(_) => false, + ApplyRet::V3(r) => r.exec_trace.iter().any( + |e| matches!(e, fvm3::trace::ExecutionEvent::CallReturn(c, _) if c.value() == want), + ), + ApplyRet::V4(r) => r.exec_trace.iter().any( + |e| matches!(e, fvm4::trace::ExecutionEvent::CallReturn(c, _) if c.value() == want), + ), + } + } + + /// Consuming variant of [`Self::exec_trace`]. + pub fn into_exec_trace(self) -> Vec { + delegate_apply_ret!(self => |r| r.exec_trace.into_iter().map(Into::into).collect()) + } + pub fn exec_trace(&self) -> Vec { delegate_apply_ret!(self => |r| r.exec_trace.iter().cloned().map(Into::into).collect()) } @@ -368,6 +394,7 @@ impl quickcheck::Arbitrary for Receipt { #[cfg(test)] mod tests { use super::*; + use fvm4::trace::ExecutionEvent as E4; use quickcheck_macros::quickcheck; #[quickcheck] @@ -381,4 +408,61 @@ mod tests { .unwrap(); assert_eq!(encoded, encoded2); } + + /// Build an `ApplyRet` of the given variant carrying `exec_trace`; every other field is a + /// zero/empty placeholder (the `fvm` structs have no `Default` to lean on). Trailing + /// `field: value` pairs supply the fields that differ across versions. + macro_rules! apply_ret { + ($variant:ident, $inner:ident, $receipt:expr, $exec_trace:expr $(, $ef:ident: $ev:expr)*) => { + ApplyRet::$variant($inner { + msg_receipt: $receipt, + penalty: Default::default(), + miner_tip: Default::default(), + base_fee_burn: Default::default(), + over_estimation_burn: Default::default(), + refund: Default::default(), + gas_refund: 0, + gas_burned: 0, + failure_info: None, + exec_trace: $exec_trace, + $($ef: $ev,)* + }) + }; + } + + #[test] + fn trace_has_call_return_exit_code_across_versions() { + let want = ExitCode::SYS_OUT_OF_GAS; + let miss = ExitCode::new(33); + + // V4 matches only the wanted code (guards the old footgun that matched any `Some(_)`). + let v4 = |t| apply_ret!(V4, ApplyRet_v4, Receipt_v4 { exit_code: ExitCode::OK, return_data: RawBytes::default(), gas_used: 0, events_root: None }, t, events: vec![], return_codec: None); + assert!(v4(vec![E4::CallReturn(want, None)]).trace_has_call_return_exit_code(want)); + assert!(!v4(vec![E4::CallReturn(want, None)]).trace_has_call_return_exit_code(miss)); + assert!( + !v4(vec![E4::CallReturn(ExitCode::OK, None)]).trace_has_call_return_exit_code(want) + ); + assert!(!v4(vec![]).trace_has_call_return_exit_code(want)); + + // V3 applies the same check to the fvm3 trace. + use fvm3::trace::ExecutionEvent as E3; + let v3 = |t| apply_ret!(V3, ApplyRet_v3, Receipt_v3 { exit_code: fvm_shared3::error::ExitCode::OK, return_data: RawBytes::default(), gas_used: 0, events_root: None }, t, events: vec![]); + let oog3 = fvm_shared3::error::ExitCode::SYS_OUT_OF_GAS; + assert!(v3(vec![E3::CallReturn(oog3, None)]).trace_has_call_return_exit_code(want)); + assert!(!v3(vec![E3::CallReturn(oog3, None)]).trace_has_call_return_exit_code(miss)); + + // FVM2 `CallReturn` carries no exit code, so it never matches. + use fvm2::trace::ExecutionEvent as E2; + let v2 = apply_ret!( + V2, + ApplyRet_v2, + Receipt_v2 { + exit_code: fvm_shared2::error::ExitCode::OK, + return_data: RawBytes::default(), + gas_used: 0 + }, + vec![E2::CallReturn(RawBytes::default())] + ); + assert!(!v2.trace_has_call_return_exit_code(want)); + } } diff --git a/src/state_manager/message_simulation.rs b/src/state_manager/message_simulation.rs index c2c73ec829f9..93782b01b581 100644 --- a/src/state_manager/message_simulation.rs +++ b/src/state_manager/message_simulation.rs @@ -185,6 +185,7 @@ impl StateManager { tipset: Option, msg: Message, vm_flush: VMFlush, + vm_trace: VMTrace, ) -> anyhow::Result<(ApiInvocResult, Option)> { let ts = tipset.unwrap_or_else(|| self.heaviest_tipset()); @@ -192,18 +193,20 @@ impl StateManager { let chain_msg = ChainMessage::for_gas_estimation(msg.clone(), from_a.protocol()); let (apply_ret, duration, state_root) = self - .call_with_gas(chain_msg, Default::default(), Some(ts), vm_flush) + .call_with_gas(chain_msg, Default::default(), Some(ts), vm_flush, vm_trace) .await?; + let msg_rct = Some(apply_ret.msg_receipt()); + let error = apply_ret.failure_info().unwrap_or_default(); Ok(( ApiInvocResult { msg_cid: msg.cid(), msg, - msg_rct: Some(apply_ret.msg_receipt()), - error: apply_ret.failure_info().unwrap_or_default(), + msg_rct, + error, duration: duration.as_nanos().clamp(0, u128::from(u64::MAX)) as u64, gas_cost: MessageGasCost::default(), - execution_trace: structured::parse_events(apply_ret.exec_trace()) + execution_trace: structured::parse_events(apply_ret.into_exec_trace()) .unwrap_or_default(), }, state_root, @@ -218,6 +221,7 @@ impl StateManager { prior_messages: Arc>, tipset: Option, vm_flush: VMFlush, + vm_trace: VMTrace, ) -> Result<(ApplyRet, Duration, Option), Error> { let ts = tipset.unwrap_or_else(|| self.heaviest_tipset()); let TipsetState { state_root, .. } = self @@ -251,7 +255,7 @@ impl StateManager { timestamp: ts.min_timestamp(), }, &this.engine, - VMTrace::NotTraced, + vm_trace, )?; for msg in prior_messages.iter() {