core, eth, miner: make block witnesses reproducible between producer and importer

[lucca30]

⛔ Blocked on pos-workflows#53 — merge that first. pipeline-e2e-tests fails here by design until it lands; all other 18 checks pass. See Dependency — merge order below.

Problem

A witness is only useful if every node derives the same bytes for the same block. WIT/2 relies on that: the cross-peer page-count check treats a witness that disagrees with its peers as a misbehaving peer and drops and jails it. There are three ways a node currently produces a witness nobody else can reproduce.

A dropped transaction leaves its reads behind. The producer attempts a transaction and then abandons it — the build deadline fires mid-execution, the nonce turns out to be taken, the transaction is simply invalid. RevertToSnapshot undoes the state, but the witness has no journal of its own, so whatever that transaction read stays in the witness of a block that does not contain it. The producer then signs a witness hash no importing node can reproduce.

Two engines write into one witness. On import, ProcessBlock hands the same *stateless.Witness to the serial processor and to Block STM, runs them concurrently, and keeps whichever finishes first. Both have already written into it by then, so the witness is the union of the two engines' reads and its size depends on how far the loser got before cancellation — that is, on wall clock. ParallelEVM.Enable defaults to true, so this is the default configuration, not a corner case.

Pipelined import SRC drains an unattributed reader. The SRC witness is derived from a FlatDiff, and CommitSnapshot finishes by draining the shared readerWithCache into it. That read set carries no attribution, so the speculative block prefetcher's reads land in it alongside the block's own — and how far the prefetcher got before the block finished is wall-clock dependent. TestPipelinedSRCDiffCarriesBlockPrefetcherReads pins this; it is a tripwire that starts failing the day the shared reader gains read attribution, which is the signal to drop the guard.

Changes

Per-transaction witness scoping. Witness gains BeginTx / CommitTx / DiscardTx. While a scope is open, code blobs and state nodes are staged rather than added, and the header extension AddBlockHash performs is recorded so it can be rolled back — AddBlockHash only ever appends, so rolling back is a truncation. StateDB wraps this and additionally defers the read-driven bookkeeping that ends up in the witness indirectly: read-prefetch scheduling, whose resolved trie paths IntermediateRoot harvests, and non-existent-account reads, which drive proof-of-absence walks.

DiscardWitnessTx also evicts the read caches the abandoned transaction populated. This is not optional. A cache hit returns ahead of the read-prefetch call, so leaving those entries behind would turn a later, genuinely included read into a silent cache hit whose trie path is never resolved — a witness that is too small, which fails stateless execution outright rather than merely carrying too much.

The producer uses the scope. commitTransaction opens a scope per attempt and discards it on every error path, after the state revert so the cache eviction cannot strand journalled changes. The conditional-transaction path does the same around ValidateKnownAccounts.

Witness recording forces the serial, non-pipelined path. A node with witness production enabled now logs a warning and disables Block STM and pipelined import SRC, rather than refusing to boot — witness generation is the feature the operator asked for, and a node that keeps running without the accelerators is strictly better than one that does not start. Independently, any block carrying a witness runs the serial processor regardless of configuration. That second layer covers what configuration cannot see: stateless self-validation and single-block InsertChain witness generation both hand a witness to a node that is otherwise entitled to run Block STM.

This half restores 0e88032c2, which landed on v2.9.2-candidate and was never forward-ported. No commit ever reverted it on develop; it simply never arrived. Combined with ParallelEVM.Enable defaulting to true, every witness-recording node cut from develop has been running the broken configuration.

Performance

Determinism here is bought on the block producer's hot path, so the cost is measured rather than asserted, and the benchmarks ship with the change.

Nodes that do not record witnesses — every node on mainnet today — are unaffected. BeginWitnessTx returns immediately without a witness, and the refactored read paths fall through to the identical prefetch call. benchstat, 6 runs each, against develop:

	develop	this PR
InsertChain_ring1000_memdb	3.879 ms	3.881 ms	~ (p=0.394)
BlockChain_1x1000Executions	8.618 ms	8.633 ms	~ (p=0.699)
allocs	76.64k	76.65k	~ (p=0.738)

The witness bookkeeping itself costs ~8-9% — 782 µs to 856 µs for a block's worth of staged additions — and about five allocations per transaction. That is some 60-75 µs against a block import measured in tens of milliseconds, which is why it does not surface in either chain benchmark above.

The scheduling change is the part that could plausibly hurt block building. The scope moves read-prefetch scheduling from the moment the EVM touches a slot to the end of the transaction. BenchmarkPrefetchSchedule* mirrors CommitWitnessTx call for call — accounts coalesced into one call, storage slots still one call each — and returns only once the prefetcher has drained, since that is what the state root computation waits on. Transaction execution is spun rather than slept, so the sealing thread holds a core the way it does in production. Shapes come from mainnet producers, where worker_txApplyDuration reports p50 ~155-210 µs and p75 ~600-850 µs.

accounts	slots	tx work	vs immediate
4	6	20 µs	~ (p=0.065)
4	6	180 µs	−3.4%
4	26	20 µs	+7.8%
4	26	180 µs	−3.8%
4	26	600 µs	−2.9%
2	78	180 µs	−2.7%

Geomean −1.0%. At realistic shapes the deferral is a small win, from batching the handful of account reads. One shape regresses: many storage reads with almost no execution, where the prefetcher is nearest the critical path and there is nothing to batch. That sits below the p25 execution time these producers report, but it is the shape to watch if the transaction mix moves that way.

Two scoping notes. The EVM never blocks on the trie prefetcher — scheduling appends to a queue and sends a non-blocking wake — so this change cannot stall execution; it only shifts when background trie work starts. And the pool prefetcher is not involved: it runs on a separate throwaway state that never has a trie prefetcher, warming a shared reader cache instead.

Numbers are memdb-backed and understate absolute trie-walk cost on a real disk. The lock and wake savings do not depend on storage speed. An end-to-end producer run on real storage would close the remaining gap.

Drop-path cost. A discarded transaction forces its cached reads to be re-read. Mainnet producers report 1,199-2,098 mid-execution interrupts per day against 4.5-5.6M successful transaction applies. The other discard paths — nonce races, invalidity, conditional-transaction failures — are not currently instrumented, so their frequency is unmeasured.

Dependency — merge order

This PR turns pipelined import SRC off on witness-producing nodes, which contradicts an assertion in 0xPolygon/pos-workflows that the kurtosis-pipeline-e2e leg runs: "on a non-mining full-sync witness producer, every witness must come from the pipelined SRC completion path." That assertion is why pipeline-e2e-tests is red here.

pos-workflows PR: 0xPolygon/pos-workflows#53 moves witness producers into the same self-gating class as stateless-sync nodes. bor's workflow pins pos-workflows@main, so that PR deliberately leaves the two src checks commented rather than inverted — asserting src == 0 before this guard is on develop would fail every open bor PR. It must merge before this one. Once both are in, the two marked lines there tighten to -eq 0.

Worth a reviewer's attention: after this change participant 6, the plain pipelined rpc node, is the only node exercising pipelined SRC end to end. That is thinner than before. Restoring it means adding a second non-witness pipelined rpc node, which changes the topology the stateless suite asserts on, so it is left for a follow-up.

Testing

• TestWitnessBlockRunsSerialProcessor — hash and path schemes, enforceParallelProcessor on and off. Proven load-bearing: dropping the witness == nil condition fails it with "parallel processor ran for a witness-recording block".
• TestWitnessSafeAccelerators — the boot-time rule, both witness flags independently: either alone disables both accelerators; a node recording no witnesses keeps what the operator configured.
• witness_tx_test.go and core/state/witness_tx_scope_test.go — scope semantics from both sides: header truncation on discard, read-cache eviction, the dirty-object exemption, and the IntermediateRoot fail-safe.
• witness_drop_determinism_test.go and witness_drop_fixture_test.go — the interrupt, nonce-race and invalidity drop paths, asserting producer and importer agree.
• TestPipelinedProducerFlatDiffIgnoresDroppedTransaction stays skipped, and the skip message now reports a test that has been run rather than one that reads the code. All four drop modes leak. Two corrections went with it: the producer path it models is not latent but unreachable — miner.isPipelineEligible is hard-wired to false, so nothing is sealed through miner/pipeline.go — while the import path is live and leaks through the speculative block prefetcher instead, because importers drop nothing.
• core + miner 813 passed; core/state + core/stateless + eth 941 passed; go build ./... clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YBppHd4UqVqBBkEf7uDT7h

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 20 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (v2.10.2-candidate@4b07a05). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
core/state/statedb.go	84.94%	9 Missing and 5 partials ⚠️
miner/worker.go	88.57%	3 Missing and 1 partial ⚠️
eth/backend.go	92.30%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@                 Coverage Diff                  @@
##             v2.10.2-candidate    #2405   +/-   ##
====================================================
  Coverage                     ?   55.62%           
====================================================
  Files                        ?      918           
  Lines                        ?   167312           
  Branches                     ?        0           
====================================================
  Hits                         ?    93067           
  Misses                       ?    68757           
  Partials                     ?     5488           

Files with missing lines	Coverage Δ
core/blockchain.go	72.08% <100.00%> (ø)
core/state/state_object.go	88.48% <100.00%> (ø)
core/stateless/witness.go	76.19% <100.00%> (ø)
eth/backend.go	55.09% <92.30%> (ø)
miner/worker.go	84.90% <88.57%> (ø)
core/state/statedb.go	79.12% <84.94%> (ø)

Files with missing lines	Coverage Δ
core/blockchain.go	72.08% <100.00%> (ø)
core/state/state_object.go	88.48% <100.00%> (ø)
core/stateless/witness.go	76.19% <100.00%> (ø)
eth/backend.go	55.09% <92.30%> (ø)
miner/worker.go	84.90% <88.57%> (ø)
core/state/statedb.go	79.12% <84.94%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[lucca30]

@claude review

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟡 eth/backend.go — Operators running a witness-producing node with --vmtrace lose live tracing even though ParallelEVM ends up disabled for them. The check at eth/backend.go:315 reads the raw config.ParallelEVM.Enable and warns/skips tracer setup, but witnessSafeAccelerators (line 366), which forces parallelEVM off when WitnessProtocol or SyncAndProduceWitnesses is set, runs after this check, so its adjusted value never reaches it. Fix: compute witnessSafeAccelerators before the vmCfg/tracer block and gate the VMTrace check on that effective parallelEVM value instead of config.ParallelEVM.Enable, so tracing is enabled whenever BlockSTM is actually off.

  Extended reasoning...

  Operator sets --parallelevm.enable=true (common/default), --witness.enable (or --witness.producewitnesses), and --vmtrace=. eth.New() builds vmCfg at lines 306-334. Line 315 checks config.ParallelEVM.Enable, which is still true (raw config, untouched). It takes the true branch, logs 'Live tracing requested but not supported with ParallelEVM enabled', and never sets vmCfg.Tracer. Line 366 then calls witnessSafeAccelerators(config), which would have returned parallelEVM=false and logged its own warning disabling BlockSTM specifically because witness generation is enabled. The blockchain is built via core.NewBlockChain (serial only, no parallelProcessor) using the adjusted parallelEVM=false, so ParallelEVM is in fact off for this node. But the tracer decision already happened using the stale true value, so the operator's live tracer is silently never installed even though nothing prevents it from running.

  Verification: severity: nit. The candidate accurately reads the code. eth/backend.go:315 gates live-tracer setup on the raw config.ParallelEVM.Enable ("if config.VMTrace != "" && config.ParallelEVM.Enable { log.Warn(...) } else if config.VMTrace != "" { ...; vmCfg.Tracer = t }"), while this diff changed processor selection to use the adjusted value: parallelEVM, pipelinedImportSRC :=… | nit. The…

[claude]

🟣 A PIP-15 conditional transaction (KnownAccounts) that passes validation but is then dropped by commitTransaction leaves its known-accounts state reads permanently in the block's witness. env.state.CommitWitnessTx() at miner/worker.go:1846 closes and commits that scope as soon as ValidateKnownAccounts succeeds, before w.commitTransaction (line 1872) decides whether the tx is actually included; the drop branches at miner/worker.go:1879-1946 (ErrNonceTooLow, vm.ErrInterrupt, invalidity) only discard commitTransaction's own later scope, never the earlier committed one. Fix: keep the known-accounts reads staged (or otherwise tie their fate) until commitTransaction's inclusion outcome is known, discarding both together on every non-inclusion path, not just committing early on validation success.

Extended reasoning...

commitTransactions reaches a tx with options.KnownAccounts set (~worker.go:1821). BeginWitnessTx (1841) opens a scope; ValidateKnownAccounts (1842) reads account/storage tries for the check. It succeeds, so CommitWitnessTx (1846) folds those staged reads into the witness's Codes/State sets permanently -- no later undo path exists for this scope. Code proceeds to w.commitTransaction(env, tx) (1872), which opens its own separate scope (line 1656) around real EVM execution. Execution hits ErrNonceTooLow, vm.ErrInterrupt, or other invalidity; commitTransaction discards only its own scope (line 1663) and returns an error. The switch (1878-1947) pops/shifts the tx, excluding it from the block; nothing revisits the already-committed known-accounts scope. The witness now carries trie nodes for a tx absent from the block -- the exact producer/importer mismatch this PR exists to eliminate, but for conditional txs. The PR's own data (1,199-2,098 mid-execution interrupts/day) shows drops are routine, so any conditional tx unlucky enough to be interrupted after passing its known-accounts check…

Verification: pre-existing. The leak is real and reachable but the base already fails the same way by the same route, so merging is not worse than base. Real gap: For a conditional (PIP-15) tx, miner/worker.go:1841 opens a witness scope, ValidateKnownAccounts (statedb.go:3228-3259, reading s.StorageTrie(k).Hash() at :3237-3239 and s.GetState(k,slot) at :3248) stages reads, and on success CommitWitnessTx() at…

[lucca30]

Confirmed and fixed in ae82af1 — thank you, this was a real hole.

One correction to the classification: this is not pre-existing. The base has no witness scoping at all, so it leaks everything everywhere. What this PR did was introduce a scope around ValidateKnownAccounts that looked like it covered the conditional path and didn't — the PR body even claimed it did. That's a regression in the claim, not in the behaviour, and it's worth fixing before merge rather than after.

I didn't take the suggested fix, though. Keeping the reads staged until commitTransaction's outcome is known would require nesting, which the scope design explicitly forbids (BeginTx replaces any open scope, so the inner BeginTx at worker.go:1656 would silently discard the outer staged set — a witness that is too small, which is the worse failure mode).

The deeper point is that these reads don't belong in the witness even when the transaction is included. PIP-15 conditional options arrive with the submission and never travel in the block, so no importing node re-runs ValidateKnownAccounts. Committing its reads makes the producer's witness a strict superset of every importer's — exactly the divergence WIT/2's page-count check treats as misbehaviour.

So the scope is now discarded unconditionally, on both branches. The discard also evicts the read caches those lookups populated, so a later genuinely-included read of the same slot still resolves its trie path instead of returning a silent cache hit.

TestKnownAccountsValidationIsWitnessNeutral pins it with a non-vacuity control: committing the same scope adds 2 nodes, discarding leaves the witness at the baseline.

[lucca30]

Both review findings are addressed.

eth/backend.go live tracer — already fixed in cb2b5fcf7, which landed one commit after the reviewed SHA (e195e8256), so the review couldn't see it. The diagnosis matches exactly: the tracer block read the raw config.ParallelEVM.Enable while witnessSafeAccelerators ran later, so a witness-recording node lost live tracing and was told parallel EVM was in the way — untrue by the time the blockchain was built. Resolved by hoisting witnessSafeAccelerators above the tracer block and branching on the effective value, which is the suggested fix.

PIP-15 known-accounts leak — fixed in ae82af178; see the inline thread for why the scope is now discarded unconditionally rather than widened, and why I don't think "pre-existing" is the right classification.

Also addressed from CI since the review ran:

• Quality metrics now passes. Function size was over threshold because the gating logic sat inline in eth.New and BlockChain.ProcessBlock; it's extracted into witnessSafeAccelerators and processorsFor, both now unit-testable. The mutation leg then flagged surviving mutants on the scope's guard conditions — three were genuinely untested branches (the Witness nil-receiver guards, CommitWitnessTx's prefetcher nil-guard, and the immediate no-scope read path), each now covered by a test verified to fail under the mutation and pass without it.
• Coverage on core/state/statedb.go was low because the scope's tests lived in core, and codecov attributes per package. Added core/state/witness_tx_scope_test.go.

pipeline-e2e-tests is expected to stay red until a companion change merges. This PR disables pipelined import SRC on witness-producing nodes, which contradicts an assertion in 0xPolygon/pos-workflows that witnesses on such a node must come from the pipelined SRC path. That assertion is deliberate (POS-3697), so it needs changing there rather than being worked around here — see the "Dependency — merge order" section in the description.

[lucca30]

⛔ Blocked on 0xPolygon/pos-workflows#53 — do not merge this first

0xPolygon/pos-workflows#53 must merge before this PR.

pipeline-e2e-tests is the only failing check here, and it fails by design. This PR stops witness-producing nodes from running pipelined import SRC; the pipeline leg asserts the opposite — that on a witness producer every witness comes from the pipelined SRC completion path. That assertion is deliberate (POS-3697), so it is being changed at the source rather than worked around here.

Why the order matters

bor's kurtosis-pipeline-e2e.yml checks out pos-workflows at ref: main. Merging this PR first leaves the old assertion live against the new behaviour, so the leg stays red on develop and on every open bor PR — not just this one. #53 is deliberately written to pass against both old and new bor, so it can land first with no red window:

	witness validators	l2-el-7 (witness rpc)	l2-el-6 (plain rpc)
bor develop today	src>0	src=602, witness=602	src>0
bor with this PR	src=0, witness=620	src=0, witness=601	src=590
#53 verdict	passes both	passes both	strict src>0, passes both

Sequence

1. Merge pos-workflows#53
2. Merge this PR
3. One-line follow-up on pos-workflows tightening the two TIGHTEN AFTER bor #2405 checks to -eq 0

The latest pipeline run failed for exactly the three predicted assertions, nothing else

l2-el-1: src=0 mismatch=0 witness=607
l2-el-2: src=0 mismatch=0 witness=597
l2-el-3: src=0 mismatch=0 witness=8
❌ witness validators: pipeline not active on any node (aggregate src=0)
l2-el-6: src=590 mismatch=0 witness=0      ← pipelined SRC still works where witnesses are off
l2-el-7: src=0 mismatch=0 witness=601
❌ l2-el-7: witness/src divergence (601 vs 0)
❌ l2-el-7: pipeline not active

Everything substantive in that run is healthy: all 8 nodes reached block 127 with the same hash and continued past the veblop HF, mismatch=0 on every node, and witnesses are still produced. The ❌ stopped service / ❌ errors in container logs lines below those come from the on-failure diagnostics step, not from independent failures.

All other 18 checks pass.

[lucca30]

Closing. This PR forces producer/importer witnesses to be byte-reproducible, but we've decided not to continue treating witnesses as deterministic — BlockSTM speculative reads make them legitimately vary node-to-node (observed ±5–9% node-set spread on mainnet).

A non-determinism-tolerant WIT/2 design supersedes this approach:

• the BP-signed commitment becomes a size-range oracle — accept/relay a witness if its size is within 3× the BP-signed size, with the pre-existing absolute ceiling retained underneath as a hard cap + starvation fallback;
• a per-node witness self-diagnosis (bytes actually touched during execution vs bytes received) detects oversized-but-valid witnesses locally, immune to cross-peer variance;
• stateless execution against the block state root becomes the sole correctness arbiter, so honest divergence is never punished and blame for a bad-but-signed witness attaches to the BP signer, not honest relayers. This preserves WIT/2's core property: re-announce/serve a trusted signed witness before self-validating it.

The per-tx witness scope here is also not needed for witness completeness: that is already guaranteed on develop by #2333 — both processors share one witness object, ProcessBlock waits for both, and the BlockSTM-v2 winner collects flat-served reads via CollectStateWitness + the read-set prewalk. Verified 2026-09-16 (a 20k-account probe could not produce an incomplete witness; TestCollectStateWitnessIncludesFlatServedReads + TestV2WitnessRegeneration* pass). So the processorsFor guard in this PR is belt-and-suspenders, not a fix.

One piece worth keeping — the validateConditionalOptions extraction in miner/worker.go (a plain PIP-15 admission-check refactor, no witness dependency) — will be lifted as a standalone cleanup.