Skip to content

op-reth: acceptance tests for forkchoice updates with a stale safe or finalized hash #23399

Open

Description

@einar-oplabs

What: Tests for the forkchoice updates op-node and kona-node send with a safe or finalized hash that is not on the head's chain, which reth now rejects before applying anything.
Why: reth#27247 and reth#27429, carried by the bump's pin, return an invalid forkchoice state instead of partially applying the update. The CL and EL interplay after an L1 reorg that rewinds the safe head, a finalized hash behind a deep unsafe reorg, or EL sync with an unknown safe hash is covered only incidentally by the green acceptance suites.

Description

  1. An L1 reorg rewinds the verifier's safe head (mechanics in op-acceptance-tests/tests/sync/elsync/reorg/ and interop/reorgs/); assert the CL recovers and the safe head advances again.
  2. EL sync from scratch where the first forkchoice update carries safe and finalized hashes reth has not downloaded; assert sync completes.
  3. A raw ForkchoiceUpdate through the DSL (L2ELNode.ForkchoiceUpdate) with finalized set to a reorged-out block, pinning the error reth returns.
  4. A raw update whose head is a persisted block reorged out below the in-memory window (reth#27429's case), pinning that it is not reported as valid.

Matrix for this test

CL {op-node, kona-node} x sync mode {CL, EL}; threshold {7, 50} for case 4.

Verification criterion

  • Passes on rust!: update reth to v2.6.0 #23046; cases 3 and 4 document the exact error codes.
  • A failing cell is a finding against the bump that may need an upstream fix; record it in the PR.

Shared context for whoever picks this up

  • The bump. rust!: update reth to v2.6.0 #23046 (reth v2.6.0), tracked in ethereum-optimism/protocol-team#354. The PR description lists the migration notes: changed defaults, rejected flags and the upstream behaviour changes. The persistence side is explained in the internal write-up Proofs exex, state masking, and persistence.
  • What changes under op-reth with no diff in our tree. reth's persistence threshold default moves from 7 to 50 (now an in-memory block count; backpressure 16 to 100), so an unclean stop loses up to the threshold plus --engine.memory-block-buffer-target blocks, about 55 instead of about 12. State masking exists in reth 2.6 but op-reth forces it to 0 (ethereum-optimism/protocol-team#457). Forkchoice updates whose safe or finalized hash is not on the head's chain are rejected (reth#27247, reth#27429). --txpool.minimum-priority-fee now also applies to local and conditional transactions (reth#26861). eth_getLogs returns error 4444 for a range whose receipts are missing and -32602 for a fromBlock beyond the head (reth#27002, reth#26891). Peers negotiate eth/71 and the transaction fetcher was rewritten (reth#26924). Block access list flags and RPC methods are rejected. The deferred persistence handoff from the fork fix reth#26708 is replaced by upstream's overlay snapshots, and its regression test rust/op-reth/crates/node/tests/it/payload_handoff.rs was deleted.
  • Existing coverage. CI on the bump head is green for memory-all-opn-op-reth, memory-all-kona-op-reth, rust-e2e-restart, op-reth-e2e-sysgo-tests and op-reth-integration-tests. Acceptance tests restart nodes only gracefully (nothing sends SIGKILL), several EL-restart and EL-sync tests are SkipOnOpReth, no test crosses a persistence boundary on purpose, and op-acceptance-tests/tests/flashblocks has no tests.
  • Devstack seams. sysgo.OpRethWithBinary selects a per-node binary (resolved through RUST_BINARY_PATH_<NAME>), OpRethWithExtraArgs adds flags, and a follower gets its options through MixedSingleChainNodeSpec.OpRethOpts (presets.WithOpRethOption reaches only sequencer-capable ELs). Missing today: a hard kill (SubProcess.Stop(false) only waits; add Kill()), an L2 CL sync-mode option in the mixed runtime, and a hook to assert on op-reth log lines. op-reth: acceptance test for proofs history across an unclean shutdown #23380 adds the kill seam and op-reth: add a windowed proof oracle to the devstack DSL #23379 the proof oracle; depend on them rather than duplicating.
  • Conventions. docs/ai/writing-acceptance-tests.md, docs/ai/acceptance-tests.md, docs/ai/rust-dev.md, and docs/ai/reth-update-review.md (precondition 6: a per-account storage root is consensus on OP, so test it with persistence engaged). Scoped commits; run rust-code-reviewer or go-code-reviewer before opening the PR.
  • Target branch. Write against develop where the behaviour exists there. A test that only makes sense on the bump is written against rust!: update reth to v2.6.0 #23046's branch and says so in the PR. A test that fails for a tracked reason is kept and skipped naming the issue.

Parameter matrix (applies to every sub-issue)

Persistence-related tests run over reth threshold {7 (develop), 50 (bump)} via --engine.persistence-threshold, masking {0, 30 when reachable (ethereum-optimism/protocol-team#457)}, and where the CL matters {op-node, kona-node} (CI runs both). Go tests use table-driven subtests with the configuration in the subtest name; Rust tests use rstest tables, one case per line. Every skipped cell says why in the skip message.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions