Conversation
Introduce the fixture format consumed by the new reorg conformance suite: a DAG of Engine API payloads (blocks keyed by label, each naming its parent) plus an ordered, branching script of newPayload/forkchoiceUpdated/JSON-RPC steps. Unlike BlockchainEngineFixture's linear payload list, a step's `expect` is a list of every outcome the Engine API specification allows a conformant client to return; the consumer picks whichever one matches its own observed response and continues down that outcome's `branches`. This lets one fixture describe side chains, explicit forkchoice states (head/safe/finalized), client-built payloads bound to new labels via getPayload, and RPC observations (balances, receipts, logs, tx-pool status), without hard-coding a single "correct" sequence that would fail on any compliant client whose behavior legitimately differs (e.g. VALID vs ACCEPTED for a side-chain payload, or an implementation-specific reorg-depth cap).
Add a small simulator of one client's forkchoice-relevant state (known-block validity, head/safe/finalized) over a reorg test's block DAG. It derives the full set of execution-apis-legal outcomes for any newPayload/forkchoiceUpdated step an author leaves unannotated, and appends an assertHead check after every forkchoice branch, so most reorg tests only need to describe the DAG and step order, not enumerate every client-observable outcome by hand. Rules follow src/engine/paris.md as amended by execution-apis#786: the no-reorg shortcut (step 2) only applies to a VALID ancestor of the latest known *finalized* block (not just of the canonical head, per the older, since-restricted text); a rewind to a canonical ancestor above finalized is a real reorg, applied or refused with -38006 (too deep); safe/finalized off head's chain is -38002. Genuinely disputed cases (pre-ethereum#786 no-op behavior some clients still exhibit, head == finalized boundary conditions) are modeled as multiple legal outcomes tagged `disputed`, never silently narrowed to one reading.
Add the filler-facing spec type: a list of DAG blocks (each naming its parent by label, so side chains and blocks built on an invalid parent are first-class) and the step script, generating BlockchainEngineReorgFixture. ReorgTest reuses BlockchainTest's block-materialization pipeline unmodified (generate_block_data, make_genesis, apply_new_parent, environment_from_parent_header, validate_receipt_status, get_fixture_engine_new_payload) walking the labeled DAG instead of a linear block list, so every existing per-block correctness check (t8n exception verification, BAL consistency, receipt validation) applies unchanged to DAG blocks. validate_dag() and _validate_step_labels() catch duplicate/reserved labels, unresolved parents and dangling label references at fill time, before a test ever reaches a client. model_post_init rejects is_inclusion_test, since that check assumes self.blocks is a linear chain and a labeled DAG has no single "last" block for it to validate against.
Wire a new `consume reorg` subcommand and hive test suite (eels/consume-reorg) that executes blockchain_test_engine_reorg fixtures: start one fresh client (plus any peers the fixture declares, admin_addPeer'd for sync-delivered reorgs), verify genesis, then walk the fixture's step script. Each step is sent verbatim; the first outcome whose constraints match the observed response is selected and logged, its branch steps run, and the step fails only if the observed response matches none of the fixture's listed outcomes. All test-specific policy lives in the fixture; the runner makes no decisions of its own. Extract two small pieces shared with the existing engine simulator instead of duplicating them: - helpers/genesis.py: the initial forkchoiceUpdated-to-genesis and getBlockByNumber(0) hash verification, used by both test_via_engine.py (single client, once-per-client cache) and the new test_via_reorg.py (looped over the main client and any peers). - single_test_client.client_environment(): the HIVE_* environment dict building, factored out of the generic `environment` fixture so the reorg simulator's fixture (which is not a BlockchainFixtureCommon, since it has no single linear post-state) can build the same base environment and layer its own `requires` variables on top, instead of recomputing it inline. Also drop the leftover `hive_simulators_reorg` plugin stub package from earlier iteration; the simulator lives under `plugins/consume/simulators/reorg/` alongside engine/sync/enginex.
Add the ReorgTest fillers exercising chain-reorganization behavior across clients, ported from hive's existing suites/engine/reorg.go tests plus scenarios drawn from geth, reth, besu, nethermind and erigon's own reorg test suites (cited per test): - test_forkchoice_reorg / test_forkchoice_behaviour: sibling and side-chain reorgs of varying depth and direction, invalid side chains, execution-apis#786 forkchoice semantics (no-reorg shortcut, inconsistent-state errors, rewind above finalized). - test_depth_matrix: reorg depth vs. client-default and HIVE_ENGINE_MAX_REORG_DEPTH-tuned caps. - test_fork_boundary_reorg: reorgs across a Shanghai->Cancun boundary, including two competing first-Cancun blocks. - test_invalid_payloads_reorg: latestValidHash propagation for corrupted payloads (bad hash, self-parent, per-field corruption, descendants of an invalid ancestor delivered out of order). - test_payload_building_reorg: engine_getPayload builds on a reorganized or previously-validated side-chain head, and pool transactions reorged in and out of a built payload. - test_tx_receipts_logs_reorg: shared/dropped/postponed/added transactions across a reorg, eth_getLogs following the canonical chain, and tx-pool status after a conflicting reorg. Most tests leave `expect` unannotated and let the Engine API reference model fill in the legal outcome set; a handful pin specific, disputed multi-client behavior explicitly with a `disputed=` rationale on the outcome.
Add the fixture-format reference page (structures for every step type, Outcome, the block DAG, differences from blockchain_test_engine) alongside the other formats, a navigation entry, and a "Reorg" section plus comparison-table row in Methods of Running Tests, matching the existing engine/enginex/sync/rlp documentation.
CPerezz
force-pushed
the
reorg-suite-pr
branch
from
September 10, 2026 10:43
7521868 to
9c3bbfc
Compare
CPerezz
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #3556 +/- ##
================================================
Coverage 94.53% 94.53%
================================================
Files 624 624
Lines 37021 37021
Branches 3349 3349
================================================
Hits 34999 34999
Misses 1429 1429
Partials 593 593
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:
|
…built, not forkchoiceUpdated's
This was referenced Sep 15, 2026
…a rewind without a known finalized block is applied
This was referenced Sep 15, 2026
Closed
…wrap a long docstring line
…ewind to a canonical ancestor
…heck the payload block hash before the parent lookup
CPerezz
marked this pull request as ready for review
September 16, 2026 09:56
Exercises the multi-client fields of the fixture format: the peer receives the chain through the Engine API, the main client is given the resulting head without ever receiving a payload, and has to converge on it by syncing from the peer over devp2p. Verified on go-ethereum, reth, nethermind, besu and erigon.
…the fork coverage rationale
At 500,000 gas, Amsterdam's state gas for the CREATE2 and for the SELFDESTRUCT's new beneficiary leaves the destroying call out of gas, so x1 keeps the contract alive with its endowment. The transition tool agrees, the block stays valid, and every client reported the same balance, but the authored assertState still expected the account destroyed: all five clients failed test_reorg_over_same_tx_selfdestruct[fork_Amsterdam]. With 1,000,000 gas the call self-destructs on every fork from Cancun to Amsterdam, as the test intends.
The Amsterdam payload-attribute defaults look up the requested head's slot and gas limit, but only genesis and fixture blocks had entries: a build request on a getPayload-bound label raised KeyError at fill time. Record each build request's attributes per head and give the bound label its timestamp, slot and target gas limit. A getPayload with no build request on its parent now fails at fill time instead of defaulting the timestamp: the consumer could not run it. A build request's forkchoiceUpdated version now follows the fork of its attributes' timestamp, not the head's, so a request across a fork boundary no longer sends V4 attributes over V3. The Amsterdam attribute test now chains two builds, and drops the unused transaction scaffolding. Review: PR3556-R0001
Moving PayloadAttributes into fixtures/blockchain.py left rpc_types re-exporting it under a noqa for five importers. Point them at the defining module and drop the re-export. Review: PR3556-R0001
A step's later siblings continued from its first outcome's model, known-block map included, and the continuation check compared only head/safe/finalized. When outcomes disagree on a block's state that choice leaks into later generated steps: newPayload(a1) expecting [syncing, valid] made a generated FCU(a1) expect only SYNCING, while [valid, syncing] made it expect only VALID, so a client taking the other listed outcome failed a check for a state it never reached. RECEIVED versus VALID is the one pair that generates the same outcomes; unknown, INVALID and received-but-invalid do not. Mark a block that the branches leave in different effective states as diverged, and reject generating an outcome that depends on it (a newPayload of its child, a forkchoiceUpdated to it) with the step that caused it; a definite later newPayload of the block settles it. The divergence check and the continuation now live in one _continue(). apply_new_payload takes the single outcome it is always given. test_invalid_side_chain's descendants of the invalid block depended on such a block; they now state their INVALID-or-SYNCING expectation explicitly, which is what the model generated, so the fixtures are unchanged. Review: PR3556-R0013, PR3556-R0015, PR3556-R0016, PR3556-R0017
annotate_steps still set headMoved on any expect list holding both an "applied" and a "noop" id, overriding what an author wrote, and the model picked payloadId by id as well. Set headMoved on the model's own head == finalized pair where it creates it, and derive payloadId from the outcome's forkchoice effect. The ids are only branch keys again; generated fixtures are unchanged. Review: PR3556-R0012
With payload attributes, anyError matches both -38003, which applies the update, and errors that do not, so its generated head assertion could only be right for one of them. Reject it and ask for errorCode. The -38003 regression now checks the assertHead the filler emits into the error branch rather than the model's internal state, and the duplicate -38002 unchanged-state test is gone. Trim the effect docstrings to one statement of the rule. Review: PR3556-R0014
Outcome.error_code repeated FixtureEngineNewPayload's inline Annotated[EngineAPIError, PlainSerializer(...)]; both now use one EngineAPIErrorCode alias beside PayloadStatusEnum. Serialized fixtures are unchanged. Move the three Outcome model tests next to the other Outcome tests in fixtures/tests/test_reorg.py and make the errorCode one an actual round-trip. Import EngineAPIError from the package root in tests/reorg like the rest of the tests, and drop Outcome docstrings that restate the enum types. Review: PR3556-R0002
The assertHead, assertCanonical, assertReceipt, assertLogs and assertTxStatus labels and both block parents were still plain str, so on the consumer side, where validate_dag never runs, a reserved name was accepted. Type them with BlockLabel or the new BlockRef (a label or "genesis"). The now-unreachable genesis check on TxRef blocks goes. assertState re-implemented assertCanonical's hash-at-height check, and its message was wrong when the height had no block. Both now use one _check_canonical(). The consumer test stubs RPC with MagicMock like the other consume tests. Review: PR3556-R0009
`post` must now stay empty, so the PostVerifications built from it were always empty; stop emitting them. validate_dag no longer checks reserved names (BlockLabel does), so say what it does check, and shorten the post/model_post_init docstrings to the rule they enforce. The unmatched-branch-key tests use non-word keys, so whitelist.txt no longer needs the misspellings "aplied" and "applide". Review: PR3556-R0005, PR3556-R0008
minReorgDepth docs now say what the consumer does with it: it sets the client's reorg-depth cap to that value. The environment test fills an empty ReorgTest instead of hand-writing a genesis header. add_get_payload_wait_time_option loses a default no caller used, and the format doc points at the option without restating its value. Remove what the multi-client removal left behind: "Start the main client" in the format doc, an f-string that lost its placeholder, and a skipped test's docstring repeating its module's. Review: PR3556-R0006, PR3556-R0007, PR3556-R0019
Cut the new unit-test docstrings to one line, drop comments that restate their assertions, and shorten the format doc's conformance, meta, payload and payload-attribute paragraphs to their rule. test_reorg_to_fork_behind_finalized no longer names the clients that currently fail it. The serialization test now covers the getPayload version too, as R0003 asked. Review: PR3556-R0003, PR3556-R0011, PR3556-R0017, PR3556-R0018
specs/reorg.py re-exported Hash and engine_model re-exported LATEST_VALID_HASH_ANY, which it does not use; nothing imports either from there. ModelDag.number had no caller outside its own test.
R0018 left DISPUTED_ZERO_SAFE and DISPUTED_SHORTCUT_VS_38006 to check against the conformance rule. Both are genuine spec gaps, filed as execution-apis#892 (zero safeBlockHash after finality) and ethereum#891 (order of the no-reorg shortcut against -38002 and -38006), so they stay disputed. They now cite those issues instead of recording which clients answered what; so do DISPUTED_SHORTCUT_VS_38002 and the model's head == finalized case, which ethereum#891 also raises. Review: PR3556-R0018
StepRunner.block_timestamp has no caller. The filler's per-label slot map spelled out an is-None fallback twice where `or 0` says the same.
A fixture edited or produced outside the filler could carry a forkchoiceUpdated or getPayload step without a version, or an Engine API step with an empty expect list. It loaded fine; the consumer then failed at run time or reported "matches none of []". Loading now rejects both, so the consumer's run-time version checks become assertions. Review: PR3556-R0008
The filler knows every fixture block's post-state, but an authored assertState was never compared with it: the Amsterdam selfdestruct fixture expected a destroyed account the transition tool kept alive, and only running all five clients showed it. Annotation now checks each assertState against the state of the block it reads (the model's head for "latest"), treating an absent account or slot as zero like the RPC calls do; client-built payloads are skipped, since their state is unknown at fill time. All 64 such checks in the corpus pass. With the old 500,000 gas, the selfdestruct fixture now fails at fill. Review: PR3556-R0005
A payload whose contents do not hash to its blockHash is rejected before it is stored, so no block exists under the hash an FCU then names. The model treated the label as a known-INVALID block and generated INVALID only; a client that keeps no record of rejected hashes answers SYNCING, which is equally legal. No generated step in the corpus hits this; the one authored case already allows both.
The rule summary listed -38002 before the no-reorg shortcut, the reverse of what the model does, and missed the hash-mismatch case. It also did not say that the model takes the spec's MAY skip for a head below finalized as the only answer: a client that instead rewinds, or reports -38002/-38006 first, fails generated steps. That order is the open question in execution-apis#891; authored steps may list those answers as disputed. Review: PR3556-R0018
CPerezz
added
C-feat
CPerezz
changed the title
CPerezz
added
A-test-consume
Conflict in client_clis/client_backend.py: keep this branch's PayloadAttributes import from fixtures.blockchain next to upstream's Requests import from forks.
Contributor
Author
|
Contributor
Author
|
One point regarding in the model ( Reading the spec's @danceratopz, happy to widen it if you read the spec differently. |
Contributor
Author
|
One deliberate narrowing in the model ( |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
blockchain_test_engine_reorg, a fixture format for testing chain reorganizations through the Engine API, with its filler, a reference model of the Engine API, aconsume reorghive simulator and atests/reorgcorpus.fixtures/reorg.py, format docs): a DAG of payloads keyed by label, plus an ordered, branching list of Engine API / JSON-RPC steps. Each Engine API step lists its spec-legal outcomes (expect) and the steps to run after each (branches). One client per fixture; the consumer resolves labels to hashes, so fixtures are identical across clients.specs/reorg.py):ReorgTestbuilds every block on its labelled parent's post-state, fills payload attributes and method versions, and checks authoredexpected_post_stateandassertStatevalues against the filled state.specs/engine_model.py): fills emptyexpectlists fromparis.mdas amended by execution-apis#786, checks every forkchoice update's head right after it, and rejects expectations it cannot make exact.consume reorg): a fresh client per fixture; sends each request as written, takes the first outcome that matches, runs its branch.--get-payload-wait-timesets the build wait;minReorgDepthsets the client's reorg-depth cap.tests/reorg, 288 fixtures from Cancun to Amsterdam plus three fork transitions): the scenarios of hive'sreorg.goand of the clients' own reorg tests.danceratopz's review findings PR3556-R0001 to R0019 are addressed in commits that carry their IDs; each thread names its fix commit.
Related Issues or PRs
consume-reorgsimulator and five-client results.disputedoutcomes.Checklist
just static<type>(<area>): <title>, where<type>and<area>come from an appropriateC-<type>, respectivelyA-<area>, label. The title should match the target squash commit message.Cute Animal Picture