refactor(forks/lstar): split spec.py into per-concern mixins#817
Merged
Conversation
Break the single large LstarSpec class into one file per concern (state transition, fork choice, block production, signatures, aggregation, timeline, validator duties), assembled back into LstarSpec via mixins over a shared typing contract (LstarSpecContract). Pure structural move: every method body is byte-for-byte identical, with no change to any external call site or type annotation. The only edits are relocating the chain-match static helper to a module-level function and moving the LstarStore alias into the contract module (still re-exported from spec.py).
c93eed2 to
e3072b0
Compare
tcoratger
approved these changes
Jun 1, 2026
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
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.
refactor(forks/lstar): split spec.py into per-concern mixins
Summary
src/lean_spec/spec/forks/lstar/spec.pyhad grown to a single ~1875-line class,LstarSpec, owning every concern of the fork: state transition, fork choice,block production, signature verification, aggregation, interval ticking, and
validator duties. This PR splits that class into one file per concern, assembled
back into the same
LstarSpecvia mixins.The split is a pure structural move: every method body is byte-for-byte
identical to before, and there are no changes to any external call site or type
annotation.
LstarSpecis still a single concrete class exposing the samemethods with the same signatures.
Motivation
and the state-transition function are large, independent subsystems that were
interleaved in one class body.
while preserving the exact public surface the node services and tests depend on.
changing the agnostic
ForkProtocolor the way callers use the spec.What changed
LstarSpecis now assembled from seven mixins plus a small typing contract:_contract.pyLstarSpecContract(ForkProtocol)— the only new construct. Declares the concrete container-factory types and the cross-mixin method surface. Also hosts theLstarStorealias.state_transition.pyStateTransitionMixin+ the module functionattestation_data_matches_chain.signatures.pySignatureMixin.block_production.pyBlockProductionMixin.fork_choice.pyForkChoiceMixin.aggregation.pyAggregationMixin.timeline.pyTimelineMixin.validator_duties.pyValidatorDutiesMixin.spec.pyLstarSpec(...the seven mixins, LstarSpecContract)— identity fields, the*_classvalues, and theLstarStorere-export.Two deliberate edits, everything else verbatim:
_attestation_data_matches_chainstatic method became a module-levelfunction
attestation_data_matches_chain(it never usedself). Its two callsites now call it as a free function;
block_production.pyimports it fromstate_transition.py.LstarStore = Store[State, Block]alias moved into_contract.pyand isre-exported from
spec.py, sofrom .spec import LstarStore(used by both__init__.pyfiles) keeps working.The typing contract
The mixins call each other through
self(e.g. fork choice's block import callsself.state_transition(...)). For the type checker to accept those cross-filecalls, each mixin needs to know the signatures of the siblings it calls and the
concrete types of the container factories it uses.
LstarSpecContractprovidesexactly that:
*_classattributes, annotated with the concrete lstar types(
type[State], nottype[SpecStateType]), soself.state_class(...)callstype-check. No
ClassVar(it would block subclass narrowing).@abstractmethodsignatures. (Methods only called within their own file are notin the contract.)
LstarSpecContractlives inside thelstarpackage, never inprotocol.py,because it references concrete lstar containers and
ForkProtocolmust stayfork-agnostic (enforced by an existing AST import-guard test).
How callers are unaffected
ForkProtocoldeclares only three abstract methods (generate_genesis,create_store,upgrade_state); every other method is called through theconcrete
LstarSpectype — by the node services (spec: LstarSpec) and thetests. Because the split keeps
LstarSpecas one class exposing all those methodsunchanged, no caller, annotation, or import outside this package changes.
Extending a fork: descendants override methods directly, not mixins
This is the part most relevant to the multi-fork roadmap. The mixins are an
internal organization detail of lstar. A descendant fork does not need to
know they exist, and does not add or re-compose its own mixins.
A new fork subclasses the assembled class and overrides what it needs, directly on
the fork class:
Why overriding on the fork class is enough — late binding. Every sibling call
goes through
self, so it is resolved against the runtime type on every call.Lstar's inherited
tick_interval(in the timeline mixin) callsself.update_safe_target(...); when invoked on aDevnet5Specinstance,selfis
Devnet5Spec, so it dispatches to Devnet5's override — even thoughtick_intervalwas written long before Devnet5 existed and lives in a differentfile. Change one method on the fork class, and every inherited caller picks it up.
This is identical to overriding a method on any ordinary subclass; the mixin
decomposition changes nothing about it.
The only condition: a method must call its sibling as
self.B(...)(which thesplit preserves everywhere). To extend rather than replace, use
super():And if a fork changes only a container (not logic), it overrides no method at all:
the inherited bodies already build via
self.block_class(...), which resolves tothe fork's container at runtime.
So: subclass
LstarSpec, override the attributes/methods that differ, done. Nonew mixins, no new contract base.
When a mixin is not the right tool (and you want components instead)
Mixins are excellent for method-level reuse and override, which is what the
roadmap's "swap a container, inherit the logic, occasionally tweak a method" needs.
They are a poor fit for subsystem-level swapping, and that is the signal to
reach for a different structure (composed components — "Option 3" in the design
notes).
Reach for a component, not a mixin override, when any of these hold:
different fork-choice rule. Overriding a dozen interrelated methods scattered
across the MRO is fragile; a replacement still has to silently honor the implicit
contract the other mixins rely on, with no enforced interface.
Mixins bind one implementation per class via the MRO; you cannot hold two. A
component is an object you can choose, inject, or swap.
selfnamespace and cannot cleanly own its own fields or collaborators; acomponent can.
replacement is complete). The mixin contract is an internal convenience, not a
public API; a component exposes an explicit protocol.
assemble a fork. That re-composition is the smell: the concern wants to be a
swappable object, not a class fragment glued by inheritance order.
When that day comes, the migration is incremental: each mixin already encapsulates
one concern and can be promoted to a held collaborator (e.g.
self.fork_choice)exposing a typed interface, without disturbing the other concerns.
Rule of thumb: tweak a method → override on the fork subclass (mixins are
invisible). Replace a subsystem or need two of something → make it a component.
Verification
relocated chain-match helper) against their new homes confirms byte-identical
bodies, modulo the two intended edits above.
exactly; an independent recomputation of the cross-file call graph confirms the
contract covers exactly those 10 methods (none missing, none unused).
LstarSpec()instantiates; the MRO is the expected linear chain;__abstractmethods__is empty.just checkpasses (ruff lint + format,tytype check, codespell, mdformat,lock).
just testpasses (3084 tests, ~93% coverage); the consensus fixture path runsthrough
fillagainst the refactored spec.contract/typing, API compatibility, docs/style, plan adherence) found no
blocker or major issues. After rebasing onto current
main, the split wasre-extracted from the updated
spec.py(which had gained the sharedancestor-weight-walk helper and dropped a vestigial parameter) and re-verified
with the same verbatim, contract-parity, type-check, and full-test gates.
Compatibility notes
LstarStoreisrelocated, not aliased twice; the only re-export is the pre-existing public path.
ForkProtocol/protocol.pyis untouched and stays fork-agnostic.*_classnarrowing as an invariantoverride; this is pre-existing — the original
LstarSpecnarrowed the sameattributes — and accepted by
ty, which is the project's type checker.Follow-ups (out of scope)
aggregation logic. It violates the comment-style rule but predates this change;
leaving it keeps this PR a pure move. Worth a separate doc-cleanup pass.