Skip to content

fix: report function selector on estimation failure#114

Draft
rubydusa wants to merge 1 commit into
mainfrom
fix/issue-22-error-report-function-selector
Draft

fix: report function selector on estimation failure#114
rubydusa wants to merge 1 commit into
mainfrom
fix/issue-22-error-report-function-selector

Conversation

@rubydusa
Copy link
Copy Markdown
Collaborator

Closes #22.

Summary

GasKillerReport::report_error previously hard-coded function_selector to FixedBytes::default() (0x00000000). On the success path the selector was correctly extracted from the transaction's input, but every error report carried zero — so a failed estimation looked indistinguishable from a selector-less call, and there was no way to tell which method triggered the failure.

The fix lifts the selector lookup into get_report so it's available on both branches:

  • get_report calls a new helper fetch_function_selector(provider, tx_hash) once at the top.
  • The selector is threaded into gaskiller_reporter (replacing the in-place lookup there) and into report_error on the failure path.
  • The outer fallback in gas_estimate_block is only reachable now if the transaction fetch itself fails, in which case there's no selector to report and we fall back to FixedBytes::default().

The in-gaskiller_reporter get_transaction_by_hash call is removed since the caller now supplies the selector — net no extra RPC roundtrips.

Behavioural notes

  • Success reports: unchanged.
  • Error reports (estimation failure): now carry the real selector instead of zero.
  • Plain ETH transfers / sub-4-byte inputs: previously erroring inside gaskiller_reporter with \"could not get function selector\", now degrade gracefully — function_selector() returns None, we use unwrap_or_default() (zero), and the rest of the pipeline runs. Upstream filters in gas_estimate_tx (smart-contract check) and gas_estimate_block (to.is_some()) already exclude these in the common case, so this should mostly be defence-in-depth.
  • Single-tx CLI path (gas_estimate_tx): if get_transaction_by_hash itself fails (network error, missing tx), get_report now propagates Err instead of silently producing a zero-selector error report. The CLI already handled top-level Err via ?.

Verification

  • cargo test -p gas-analyzer-anvil --lib — 11/11 pass locally (against Sepolia via RPC_URL, Foundry v1.5.1).
  • cargo test (default workspace, EvmSketch path) — pass.
  • cargo clippy -p gas-analyzer-anvil --all-targets -- -D warnings — clean.
  • cargo clippy --workspace --exclude gas-killer-wasm --exclude gas-analyzer-anvil --all-targets -- -D warnings — clean (matches CI lint).
  • cargo fmt --all -- --check — clean.

🤖 Generated with Claude Code

`GasKillerReport::report_error` previously hard-coded `function_selector` to
zero, so any failed estimation produced a report indistinguishable from a
selector-less call. The selector is needed to identify which method was
attempted when estimation fails.

Fetch the selector up front in `get_report` and thread it through both the
success path (already passed via `ReportDetails`) and the error path. The
existing in-`gaskiller_reporter` selector lookup is removed since the
caller now supplies it, avoiding a redundant `get_transaction_by_hash`.

Closes #22

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rubydusa
Copy link
Copy Markdown
Collaborator Author

ci is failing because of new foundry release: see
#96 (comment)

@rubydusa rubydusa marked this pull request as draft April 30, 2026 15:11
@rubydusa
Copy link
Copy Markdown
Collaborator Author

put on draft as long as #96 isn't merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix: report function selector even on estimation failure

1 participant