feat(fill-stateful): add --extract-opcode-count opcode tracing#3124
feat(fill-stateful): add --extract-opcode-count opcode tracing#3124skylenet wants to merge 4 commits into
Conversation
Trace each execution-phase block via debug_traceBlockByHash with a JS opcode-counting tracer and record per-opcode execution counts in the fixture's _info.metadata.opcode_count, matching the standard fill benchmark path. - DebugRPC.trace_block_by_hash wraps debug_traceBlockByHash. - ClientBackend gains debug_rpc + extract_opcode_count; a JS tracer constant; extract_block_opcode_count() aggregating per-tx opcode counts; reset_opcode_count() now zeroes the per-test tally. - make_stateful_fixture traces execution-phase blocks only (setup blocks skipped) and accumulates onto the backend, which the filler already serializes into _info.metadata.opcode_count. - fill_stateful plugin: --extract-opcode-count flag, debug_rpc wired into the backend, per-test reset_opcode_count in the t8n override.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #3124 +/- ##
===================================================
- Coverage 93.30% 93.30% -0.01%
===================================================
Files 624 624
Lines 36986 36986
Branches 3383 3383
===================================================
- Hits 34511 34508 -3
- Misses 1693 1695 +2
- Partials 782 783 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address review of the --extract-opcode-count feature: - Decouple metadata from benchmark verification: only accumulate onto the metadata opcode_count, never set benchmark_opcode_count. The live-client trace is recorded for analysis; it no longer activates target-opcode verification (which stays a t8n-path concern and could otherwise fail benchmark tests on >5% trace divergence). - Make extract_block_opcode_count non-fatal: a debug_traceBlockByHash RPC error is logged and skipped instead of aborting the fill. - Normalize opcode names: geth emits 'opcode 0xNN not defined' for undefined opcodes, which OpcodeCount could not parse and would raise on. Reduce such names to the bare 0xNN (UndefinedOpcode); drop any still-unrecognized name with a warning.
With --extract-opcode-count on, feed the live-client traced count to benchmark opcode verification: a test declaring fixed/expected_opcode_ count now fails the fill when its live-client count diverges >5% from the target. Stays None (verification skipped) when the flag is off.
## What Adds an opt-in `extract_opcode_count` option to eest-payloads targets. When enabled, benchmarkoor passes `--extract-opcode-count` to `fill-stateful`, which traces each execution-phase block (`debug_traceBlockByHash` + a JS opcode-counting tracer) and records per-opcode execution counts in each fixture's `_info.metadata.opcode_count`. ## Depends on **ethereum/execution-specs#3124 — the `fill-stateful --extract-opcode-count` flag lives there. This benchmarkoor change is just the passthrough; it's **inert unless enabled**, and only functions when `eest_repo`/`eest_ref` point at a `fill-stateful` that carries the flag (i.e. once #3124 lands, or a fork/branch with it). ## Changes - `pkg/config`: `extract_opcode_count` (`*bool`) on `EESTPayloadTarget` + `EESTPayloadDefaults`, hoistable to `config:` and inherited via `ResolveTarget`. - `pkg/builder`: `buildFillArgs` appends `--extract-opcode-count` when enabled. - Example config: a commented note documenting the option + its requirements. - Tests: `buildFillArgs` cases (enabled/disabled/absent-by-default) and `ResolveTarget` inheritance/override coverage. ## Requirements / caveats - Filler must support the `debug` namespace + JS tracers — **geth** works (besu/nethermind/reth/ethrex don't expose geth-style JS tracers). - **Slow**: a full re-execution trace per block. Opt-in for that reason. ## Validation Ran a real geth build against a branch carrying #3124 (`skylenet/execution-specs @ opcode-filing`), filling the bn128 compute subset. The fill argv included `--extract-opcode-count`, the build succeeded, and **all 36 filled tests** got a non-empty `opcode_count`. Confirmed against the written fixture on disk, e.g.: ``` tests/benchmark/compute/precompile/test_alt_bn128.py::test_bn128_pairings_amortized[fork_Osaka-...-value_10M] _info.metadata.opcode_count = { "CALLDATASIZE":11,"PUSH1":42,"CALLDATACOPY":1,"JUMPDEST":1,"GAS":10,"STATICCALL":10,"POP":9 } ``` (Values are sensible: the BN128 pairing work runs inside the precompile, invoked via `STATICCALL` to `0x08` — counted 10× — so the traced EVM opcodes are the calling harness.) `go build`/`go test` green (except the pre-existing macOS-only `/proc/mounts` `TestValidateDataDirMethods_SchelkBinary`); golangci-lint `--new-from-rev=origin/master` 0 issues.
|
EDIT/NOTE: written with Claude, reflects my thoughts. Preliminary comment — this feature is definitely something we want. One concern: debug_traceBlockByHash is in execution-apis, but custom JS tracers are not — they're a client-specific extension with unspecified output. That's fine today since the trace only runs at fill time against geth, but it locks opcode extraction to JS-tracer-capable clients, and _normalize_opcode_name() already shows the next-fork fragility (geth emitting "opcode 0xNN not defined" for unknown opcodes). Longer term: since we already have testing_buildBlock / testing_commitBlock, a testing_ method returning opcode counts directly would remove the JS-tracer dependency entirely — and it's much cheaper than standardizing full trace output. Question: is the motivation parity with the standard fill benchmark path (diff_opcode_counts.py), or also feeding Nethermind's gas-benchmarks flow? Not blocking — for Glamsterdam we can't wait for a standardized endpoint, so this is the right pragmatic step. Will re-visit this week 👍 😄 |
|
@jochem-brouwer the motivation from my side is mainly getting the opcode counts to be able to show them on benchmarkoor. But the good side effect is that we can also detect opcode divergence early. The feature is behind a flag and not enabled by default so it shouldn't interfere with existing tooling. I agree that maybe some custom testing API endpoint would be the best/fastest, but for now I think it's fine if we do the following: 1 ) Extract via JS tracer. Small response body. Works with nethermind, geth. 2 ) Fallback to a counting them from the structLogs. Big response body, but should work with other clients that don't support JS tracers, e.g. Besu. I'm working now on testing the fallback approach. |
…+ struct-log fallback) - Normalize tracer opcode names case-insensitively so clients emitting non-canonical names (nethermind returns calldatasize / pusH1) are counted instead of dropped as unrecognized. - Fall back to the universally-supported struct-log tracer (disableStack/Memory/Storage, counting structLogs[].op) when a client's JS tracer is unavailable — erroring or empty — so besu (no JS tracer) is covered. Cached per client to avoid re-probing the JS tracer every block. - Update the --extract-opcode-count help text accordingly. Verified against geth (JS), nethermind (JS + normalize) and besu (struct-log fallback): all three produce identical opcode counts.
|
So I've got this working with geth, nethermind (using JS tracer) and besu (using the fallback) |
3 participants
🗒️ Description
Adds an opt-in
--extract-opcode-countflag tofill-stateful.When set, each execution-phase block built against the live client is traced via
debug_traceBlockByHashusing a small JS opcode-counting tracer, and the aggregated per-opcode execution counts are recorded in the fixture's_info.metadata.opcode_count— matching what the standardfillbenchmark path already emits (and whatdiff_opcode_counts.pyreads).Details:
DebugRPC.trace_block_by_hashwrapsdebug_traceBlockByHash.ClientBackendgains optionaldebug_rpc+extract_opcode_countctor params, a JS tracer constant,extract_block_opcode_count()(aggregates per-tx opcode counts across a block), and a realreset_opcode_count()that zeroes the per-test tally (previously a no-op).make_stateful_fixturetraces execution-phase blocks only (setup blocks are skipped — no wasted trace) and accumulates onto the backend, which the existing filler plumbing serializes into_info.metadata.opcode_count. No filler/fixture-model changes needed.fill_statefulplugin: the--extract-opcode-countflag,debug_rpcwired into the backend, and a per-testreset_opcode_count()in thet8noverride.docs/filling_tests/fill_stateful.md.The flag is fully opt-in: with it off, no trace RPCs are issued and
opcode_countstaysNone, so the filler omits the metadata key exactly as before. It requires the client to expose thedebugnamespace with JS tracer support; the tracer re-executes each block, so it is slow and intended for analysis runs.Opcode-count verification: when a benchmark test declares a target opcode count (
fixed_opcode_count/expected_opcode_count) and--extract-opcode-countis on, the live-client trace is verified against that target and the fill fails on >5% divergence — surfacing benchmarks that don't execute what the author expected. With the flag off,benchmark_opcode_countstaysNoneand verification is skipped, exactly as before.Robustness:
debug_traceBlockByHashRPC error is logged and skipped rather than aborting the fill.opcode 0xNN not definedfor undefined opcodes, whichOpcodeCountcould not parse; such names are reduced to the bare0xNN(UndefinedOpcode), and anything still unrecognized is dropped with a warning.just static(mypy, ruff, vulture, ethereum-spec-lint, actionlint, codespell) passes.🔗 Related Issues or PRs
N/A.
✅ Checklist
just static<type>(<area>):, where<type>and<area>come from an approrpriateC-<type>, respectivelyA-<area>, label. The title should match the a target squash commit message.Cute Animal Picture