Stage 1: tighten ForkProtocol surface (trivial items)#3
Merged
tcoratger merged 3 commits intotcoratger:multi-fork-architecturefrom Apr 27, 2026
Merged
Conversation
Bundles four small, behavior-preserving improvements to the multi-fork skeleton from PR leanEthereum#638. Tests + all-checks green. * Expose `GOSSIP_DIGEST: ClassVar[str]` on `ForkProtocol`. The gossipsub fork digest is fork metadata, not a top-level constant. Devnet4Spec binds it to "devnet0" (preserving the existing network agreement); Devnet5Spec inherits. Removes the hardcoded `GOSSIP_FORK_DIGEST` constant and the now-unused `Final` import from `__main__.py`, and routes every consumer through `fork.GOSSIP_DIGEST`. * Use `DEFAULT_RUNNER` directly in `__main__.py` instead of building a second `SpecRunner(FORK_SEQUENCE)`. The forks package already exports the singleton; the redundant construction is dropped along with the unused `FORK_SEQUENCE` and `SpecRunner` imports. * Add `previous: ClassVar[type[ForkProtocol] | None]` linking each fork to its predecessor. `Devnet4Spec.previous = None`; `Devnet5Spec.previous = Devnet4Spec`. Sets up the foundation for a future registry that derives ordering from topology and for chained state migrations via `upgrade_state`. * Tidy `devnet5/spec.py`: drop the module-level `_Devnet5State` and `_Devnet5Store` identity aliases; bind the container classes directly on `Devnet5Spec`. When devnet5 grows its own `forks/devnet5/containers/` package, the swap is a file-level diff in the imports rather than an alias rebinding. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The class is a registry — it stores a sequence of forks and looks them up by name — not a dispatcher routing spec calls per fork. The old name implied the latter and set wrong expectations for follow-ups. * Renames the file `runner.py` -> `registry.py`. * Renames the class `SpecRunner` -> `ForkRegistry`. * Renames the singleton `DEFAULT_RUNNER` -> `DEFAULT_REGISTRY`. * Updates the docstring on `FORK_SEQUENCE` and the error message in the empty-list guard. * Updates the test class name and assertions. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the silent identity default with an abstract method, forcing every concrete fork to declare its migration explicitly. This prevents the silent-no-op footgun: a future fork that adds a State field but forgets to override upgrade_state would otherwise migrate by simply not migrating. * `ForkProtocol.upgrade_state` becomes `@abstractmethod` with a docstring-only body. * `Devnet4Spec.upgrade_state` is identity, documented as "root fork, no predecessor, no migration". * `Devnet5Spec.upgrade_state` is identity, documented as "currently mirrors devnet4; replace when devnet5's State diverges". * Adds a test that asserts `ForkProtocol()` raises TypeError, locking in the abstract-class contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
38 tasks
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.
Stage 1 cleanups for the multi-fork architecture, against PR leanEthereum#638's
multi-fork-architecturebranch.Tracking issue: leanEthereum#686 (Stage 1).
Scope
Six trivial, behavior-preserving items from the Stage 1 list. Each landed as its own commit so they're independently revertible:
refactor(forks): stage 1 trivial cleanups for ForkProtocol surface— bundles four small additions:GOSSIP_DIGEST: ClassVar[str]toForkProtocol; route consumers throughfork.GOSSIP_DIGEST. Drops the hardcodedGOSSIP_FORK_DIGESTconstant and the now-unusedFinalimport.DEFAULT_RUNNERdirectly in__main__.pyinstead of constructing a redundantSpecRunner(FORK_SEQUENCE).previous: ClassVar[type[ForkProtocol] | None]linking each fork to its predecessor (root →None, devnet5 →Devnet4Spec).devnet5/spec.py: drop the module-level identity aliases, bind container classes directly.refactor(forks): rename SpecRunner to ForkRegistry— pure rename. The class is a registry, not a dispatcher.runner.py→registry.pySpecRunner→ForkRegistryDEFAULT_RUNNER→DEFAULT_REGISTRYfeat(forks): make upgrade_state abstract on ForkProtocol— replaces the silent identity default with@abstractmethod. Every concrete fork must declare its migration explicitly. Devnet4 (root) and Devnet5 (currently identical) both implement as identity with documented intent. New test assertsForkProtocol()raisesTypeError.Out of scope (Stage 1 follow-ups)
These remain in Stage 1 but are larger / interdependent and will be separate PRs:
AnyinForkProtocolmethod signaturesSpecBlockTypeProtocol; declare structural attributes onSpecStateType/SpecStoreTypeClassVarfromstate_class/block_class/store_class(per Vision B)generate_pre_statefork fallback (requires fixture plumbing)Test plan
uvx tox -e all-checksclean (ruff, format, ty, codespell, mdformat)uv run pytest --no-cov— 3263 passedtest_cannot_instantiate_directly) locks in the abstract-class contract