refactor(lstar): freeze the Block family#843
Conversation
Set frozen=True on the lstar containers that are built once and never mutated after construction: GenesisConfig, Validator, AggregatedAttestation, and SignedAggregatedAttestation. This completes the freeze begun on Checkpoint, AttestationData, Attestation, SingleMessageAggregate, and MultiMessageAggregate, so accidental post-construction mutation now raises instead of silently succeeding. Freezing Validator required one call-site change: the fork-choice test fixture injected public keys by mutating each validator in place. It now rebuilds each validator with model_copy(update=...), which is byte-identical in hash_tree_root and leaves the originals untouched. Block, BlockHeader, BlockBody, and SignedBlock stay mutable for now. Their state_root and body are patched in place during block production and the state transition (block_production.py and state_transition.py), a two-pass pattern entangled with the larger state-transition refactor. State and Store remain mutable by design. Frozen is an enforcement aid, not a hard immutability guarantee: model_config is itself a mutable dict and an unfreeze-then-assign sequence can still leave a field mutable (pydantic#12361). It catches accidental mutation; it is not a soundness boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Set frozen=True on BlockBody, BlockHeader, Block, and SignedBlock, the last
mutable containers in the lstar object graph apart from State and Store. With
this, every consensus container except the State accumulator and the
fork-choice Store is immutable: accidental post-construction mutation now
raises instead of silently corrupting a value that may already be referenced
elsewhere or committed to a root.
These four were deferred from the initial freeze because each was patched in
place after construction. Every such site is migrated to model_copy(update=...),
which is byte-identical in hash_tree_root:
- Block production builds the block with a zero state root, processes it,
then rebuilds the block with the real root.
- The slot transition rebuilds the latest header with its cached state root
and reassigns it on the still-mutable State, instead of mutating the header
in place.
- The test-framework builders and the signature-tampering helpers rebuild
blocks and bodies through layered model_copy, matching the pattern the
attestation-swap tamper already used.
State and Store stay mutable. State is the deepcopy-and-mutate accumulator at
the heart of the state transition; converting it to return a new value is a
larger, separate change. Store is fork-choice scratch state.
As with the earlier container freeze, frozen is an enforcement aid, not a hard
immutability guarantee (model_config is a mutable dict; pydantic#12361).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tcoratger
left a comment
There was a problem hiding this comment.
After a discussion with @JustinDrake, I removed most of them here #789 to improve readability because this is pretty verbose and because we thought that formal verification may be far away. But now that you have a look at it, very happy to put them back if this can help you in the future.
I even think that to avoid
model_config = Container.model_config | {"frozen": True}everywhere, we can enforce it directly here
leanSpec/src/lean_spec/base.py
Lines 48 to 60 in 7ed65b1
so that all of our classes will inherit but let's do that in a followup!
|
Thanks, this is useful context since I started looking at this after 789, My sense was that it would be good to have this where possible and that it would be more disruptive to do it later.
Yep this would be worth looking into. I think if we agree that we should aim for things to be immutable (and as the caveat in the PR body says, the solution in this PR is really a bandaid around a mutable structure) then we can focus on retaining legibility. |
Restore the frozen constraint that leanEthereum#789 removed, this time enforced once in the base model instead of per class. Every spec type is now immutable by default, with no opt-outs: the State accumulator and the fork-choice Store are frozen too, and every remaining in-place mutation site is converted back to the model_copy(update=...) functional style. Follow-up to leanEthereum#842/leanEthereum#843, as discussed in the leanEthereum#843 review thread. Changes: - base.py: add frozen to StrictBaseModel; restore the pre-leanEthereum#789 docstring. - Delete the 17 per-class model_config | {"frozen": True} overrides (block, checkpoint, attestation, aggregation, validator, xmss, eth2) now that the base enforces them. - state_transition.py: process_slots rebinds through model_copy (the deepcopy barrier is no longer needed), process_block_header applies its updates atomically in one final copy, process_attestations returns a new state. - fork_choice.py, timeline.py, validator_duties.py, aggregation.py: every store update flows through model_copy; dicts and inner sets are shallow-copied before growing so the caller's store is left untouched. - node/chain/service.py, node/sync/service.py: rebind the store instead of patching it in place. - xmss/interface.py: advance_preparation returns a rebuilt secret key. - enr.py: from_rlp rebuilds the record with the computed node id. - packages/testing + tests: all fixture-setup mutations converted to model_copy rebinding; helpers that mutated arguments now return the new instance. - Restore test_frozen_rejects_assignment and the immutability wording in the SSZ patterns rule; add mirrored immutability tests for State and Store. Validation: - just check passes (ruff lint + format, ty, codespell, mdformat, lock) - Unit suites: lstar spec 257 passed, node 1682 passed, crypto/ssz/enr/containers/base 1660 passed Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restore the frozen constraint that #789 removed, this time enforced once in the base model instead of per class. Every spec type is now immutable by default, with no opt-outs: the State accumulator and the fork-choice Store are frozen too, and every remaining in-place mutation site is converted back to the model_copy(update=...) functional style. Follow-up to #842/#843, as discussed in the #843 review thread. Changes: - base.py: add frozen to StrictBaseModel; restore the pre-#789 docstring. - Delete the 17 per-class model_config | {"frozen": True} overrides (block, checkpoint, attestation, aggregation, validator, xmss, eth2) now that the base enforces them. - state_transition.py: process_slots rebinds through model_copy (the deepcopy barrier is no longer needed), process_block_header applies its updates atomically in one final copy, process_attestations returns a new state. - fork_choice.py, timeline.py, validator_duties.py, aggregation.py: every store update flows through model_copy; dicts and inner sets are shallow-copied before growing so the caller's store is left untouched. - node/chain/service.py, node/sync/service.py: rebind the store instead of patching it in place. - xmss/interface.py: advance_preparation returns a rebuilt secret key. - enr.py: from_rlp rebuilds the record with the computed node id. - packages/testing + tests: all fixture-setup mutations converted to model_copy rebinding; helpers that mutated arguments now return the new instance. - Restore test_frozen_rejects_assignment and the immutability wording in the SSZ patterns rule; add mirrored immutability tests for State and Store. Validation: - just check passes (ruff lint + format, ty, codespell, mdformat, lock) - Unit suites: lstar spec 257 passed, node 1682 passed, crypto/ssz/enr/containers/base 1660 passed Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Set
frozen=Trueon the last mutable containers in the lstar object graph apart fromStateandStore:BlockBody,BlockHeader,Block,SignedBlock(containers/block.py)After this, every consensus container except the
Stateaccumulator and the fork-choiceStoreis immutable.Why
These four were deferred from #842 because each was patched in place after construction. Closer review showed those mutations are not blocked by the future
Staterefactor — every site migrates cleanly tomodel_copy(update=...), which is byte-identical inhash_tree_root. Freezing closes the last accidental-mutation gap on the block object graph, which matters because aBlock/Checkpointis routinely shared by reference across fork choice, the chain, and attestations.Migrated mutation sites
Production (
src/):block_production.py— build with a zero state root, process, then rebuild the block with the real root.state_transition.py— rebuild the latest header with its cached state root and reassign it on the still-mutableState, instead of mutating the header in place.Test framework + tests: the block builders, the signature-tampering helpers (
verify_signatures.py), and the block-production unit tests now rebuild blocks and bodies through layeredmodel_copy, matching the pattern the attestation-swap tamper already used.Deferred
StateandStorestay mutable.Stateis thecopy.deepcopy+ in-place accumulator at the heart of the state transition; converting it to return a new value is a larger, separate change.Storeis fork-choice scratch state.Caveat
frozenis an enforcement aid, not a hard immutability guarantee (model_configis a mutable dict; pydantic#12361).Tests
Adds immutability tests for the four containers in the source-mirrored path (new
test_block.py), following the established idiom.Validation
ruff(lint + format),ty,codespell— cleanuv run fill --fork=lstar --clean: 520 passed, vectors regenerate cleanly (every migration preserveshash_tree_root, so fixtures are byte-identical)🤖 Generated with Claude Code