Skip to content

Commit 3bd2cd5

Browse files
GrapeBaBatcoratger
andauthored
fix(forkchoice): derive finalized from canonical head, not an independent max (leanEthereum#1001)
* fix(forkchoice): derive finalized from canonical head, not an independent max The store advanced latest_finalized by an independent monotonic max over every imported block's post-state, decoupling it from the head. A fork that finalized a higher slot but then lost head selection left its finalized checkpoint latched in the store. get_attestation_target derives the attestation target against store.latest_finalized, while the state transition validates targets against the canonical state's latest_finalized; once the two finalized slots diverged, is_justifiable_after disagreed and every advancing target was rejected, so finalization froze. Derive the finalized checkpoint from the canonical head's chain instead, in update_head, so it is recomputed on every head change -- block import, acceptance ticks, and proposals: take the finalized slot named by the head state and recover its root as the head's own ancestor at that slot. The checkpoint then always lies on the head chain, store.latest_finalized.slot matches the slot the state transition validates against, and a checkpoint-sync anchor survives because the head descends from it (its state cannot name its own block, so the root is recovered as the head's ancestor). This restores the 3sf-mini reference, where a staker reads latest_finalized from post_states[head], not a max over states. Block production carried a second copy of the same independent max: it advanced store.latest_finalized from the produced block's own state and pruned against it, stranding -- and over-pruning -- when the proposal lost head selection. Remove that write; update_head, which get_proposal_head runs just before each proposal, owns the finalized checkpoint. The fork choice is not clamped to finalized descendants: leanSpec finalization is order dependent, so a fork that finalizes a higher slot can still lose head selection, and clamping would latch the head onto that losing fork. The attestation source keeps using store.latest_justified (the LMD anchor), matching 3sf-mini's get_latest_justified_hash and the deliberate choice in leanEthereum#595. * Update src/lean_spec/spec/forks/lstar/fork_choice.py Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> * Update src/lean_spec/spec/forks/lstar/fork_choice.py Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> * refactor(forkchoice): address review -- inline finalized helper, tidy comments Inline the single-use _finalized_on_head_chain into update_head, folding the head and finalized updates into one model_copy; trim the verbose comments in update_head and produce_block. No behavior change. * fix(forkchoice): guard the finalized walk against a checkpoint-sync boot The ancestor walk in head recomputation assumed every parent was already in the store. On a fresh checkpoint-synced node the store holds only the anchor block, whose state's finalized slot sits below the anchor block slot, so the first head recompute stepped to a parent root absent from the store and raised KeyError. The vectors missed it because the test anchor rebases its finalized up to the anchor slot, so the loop never iterated. Guard the walk as the other ancestor walks in this file do, stop when the parent is absent, and keep the trusted anchor when no block sits exactly at the finalized slot (which also covers a skipped finalized slot). Add a regression vector: a mid-chain anchor whose finalized stays at genesis, below the anchor slot, no longer crashes head recomputation. * test(forkchoice): cover the proposer path not pinning a higher finalized Build genesis -> block_1 -> block_2 justifying slot 1, then produce block_3 whose own state finalizes slot 1. Block production does not advance its own head, so block_3 stays off the head while the head remains block_2, finalized at the genesis boundary. Assert the store's finalized tracks the head state, not block_3's higher finalized, and the counted pool is left untouched. Fails on the pre-fix advance-and-prune in block production, passes on the fix. --------- Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
1 parent 003968f commit 3bd2cd5

5 files changed

Lines changed: 362 additions & 20 deletions

File tree

src/lean_spec/spec/forks/lstar/fork_choice.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -596,15 +596,14 @@ def on_block(
596596
# Run the state transition from the parent state to this block's post-state.
597597
post_state = self.state_transition(parent_state, block)
598598

599-
# Advance the justified and finalized checkpoints from the post-state.
599+
# Advance the justified checkpoint from the post-state.
600600
#
601601
# A candidate wins only when its slot is strictly higher than the store's.
602602
# On a slot tie the store keeps its checkpoint, avoiding a silent root swap:
603603
#
604604
# store slot 5, candidate slot 7 -> take candidate
605605
# store slot 5, candidate slot 5 -> keep store
606606
latest_justified = store.latest_justified.advance_to(post_state.latest_justified)
607-
latest_finalized = store.latest_finalized.advance_to(post_state.latest_finalized)
608607

609608
# Seed each block-carried vote into the known pool with an empty proof set.
610609
#
@@ -625,7 +624,6 @@ def on_block(
625624
"blocks": store.blocks | {block_root: block},
626625
"states": store.states | {block_root: post_state},
627626
"latest_justified": latest_justified,
628-
"latest_finalized": latest_finalized,
629627
"latest_known_aggregated_payloads": new_known_aggregated_payloads,
630628
}
631629
)
@@ -830,7 +828,24 @@ def update_head(self, store: LstarStore) -> LstarStore:
830828
start_root=store.latest_justified.root,
831829
attestations=latest_votes,
832830
)
833-
return store.model_copy(update={"head": new_head})
831+
# Invariant: the finalized checkpoint stays on the head's chain.
832+
# Climb from the head to its ancestor at the finalized slot.
833+
finalized_slot = store.states[new_head].latest_finalized.slot
834+
finalized_root = new_head
835+
while finalized_root in store.blocks and store.blocks[finalized_root].slot > finalized_slot:
836+
parent_root = store.blocks[finalized_root].parent_root
837+
if parent_root not in store.blocks:
838+
break
839+
finalized_root = parent_root
840+
841+
# A fresh checkpoint-sync anchor stores no block at that slot.
842+
# Keep the trusted anchor there rather than emit an unresolved root.
843+
if store.blocks[finalized_root].slot == finalized_slot:
844+
latest_finalized = Checkpoint(root=finalized_root, slot=finalized_slot)
845+
else:
846+
latest_finalized = store.latest_finalized
847+
848+
return store.model_copy(update={"head": new_head, "latest_finalized": latest_finalized})
834849

835850
def accept_new_attestations(self, store: LstarStore) -> LstarStore:
836851
"""

src/lean_spec/spec/forks/lstar/validator_duties.py

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -199,29 +199,17 @@ def produce_block_with_signatures(
199199
# Compute block hash for storage.
200200
block_hash = hash_tree_root(final_block)
201201

202-
# Update checkpoints from post-state.
203-
#
204-
# Locally produced blocks bypass normal block processing.
205-
# Checkpoint advances must be propagated manually here.
206-
#
207-
# Tie semantics mirror the block-import path.
208-
# A candidate needs a strictly higher slot to replace the store's view.
202+
# A locally produced block skips the import path.
203+
# Advance the justified checkpoint manually here.
204+
# Leave the finalized checkpoint to head recomputation.
205+
# Pinning it from this block's own state would strand it on a later reorg.
209206
latest_justified = store.latest_justified.advance_to(final_post_state.latest_justified)
210-
latest_finalized = store.latest_finalized.advance_to(final_post_state.latest_finalized)
211-
212-
# Persist block and state.
213-
previous_finalized_slot = store.latest_finalized.slot
214207
store = store.model_copy(
215208
update={
216209
"blocks": store.blocks | {block_hash: final_block},
217210
"states": store.states | {block_hash: final_post_state},
218211
"latest_justified": latest_justified,
219-
"latest_finalized": latest_finalized,
220212
}
221213
)
222214

223-
# Prune stale attestation data when finalization advances
224-
if store.latest_finalized.slot > previous_finalized_slot:
225-
store = self.prune_stale_attestation_data(store)
226-
227215
return store, final_block, signatures

tests/consensus/lstar/fork_choice/test_checkpoint_sync.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,55 @@ def test_store_init_from_non_genesis_anchor(
8282
)
8383

8484

85+
def test_head_recompute_on_anchor_below_its_finalized_slot(
86+
fork_choice_test: ForkChoiceTestFiller,
87+
) -> None:
88+
"""
89+
Appending a block above a mid-chain anchor whose finalized sits below it does not crash.
90+
91+
Given
92+
-----
93+
- 4 validators.
94+
- a mid-chain anchor at slot 10 built without rebasing its checkpoints.
95+
- the anchor state's finalized stays at the genesis boundary, slot 0.
96+
- slot 0 sits below the anchor block slot.
97+
- the store holds only the anchor block.
98+
99+
When
100+
----
101+
- one block is appended above the anchor, recomputing the head over the anchor chain.
102+
103+
Then
104+
----
105+
- head advances to the appended block at slot 11.
106+
- finalized stays at the trusted anchor at slot 10.
107+
- the parent walk stops at the anchor, so head recomputation does not raise.
108+
"""
109+
anchor_state, anchor_block = build_anchor(
110+
synced=False, num_validators=NUM_VALIDATORS, anchor_slot=ANCHOR_SLOT
111+
)
112+
113+
fork_choice_test(
114+
anchor_state=anchor_state,
115+
anchor_block=anchor_block,
116+
steps=[
117+
BlockStep(
118+
block=BlockSpec(
119+
slot=Slot(11),
120+
parent_label="genesis",
121+
label="block_11",
122+
),
123+
checks=StoreChecks(
124+
head_slot=Slot(11),
125+
head_root_label="block_11",
126+
latest_finalized_slot=ANCHOR_SLOT,
127+
latest_finalized_root_label="genesis",
128+
),
129+
),
130+
],
131+
)
132+
133+
85134
def test_extend_chain_from_non_genesis_anchor(
86135
fork_choice_test: ForkChoiceTestFiller,
87136
) -> None:

tests/consensus/lstar/fork_choice/test_finalized_safety.py

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,3 +387,192 @@ def test_fork_above_finalized_wins_at_or_below_loses(
387387
),
388388
],
389389
)
390+
391+
392+
def test_losing_fork_higher_finalized_does_not_latch(
393+
fork_choice_test: ForkChoiceTestFiller,
394+
) -> None:
395+
"""
396+
A fork that finalizes a higher slot but loses head selection must not leave its
397+
finalized checkpoint latched in the store.
398+
399+
Given
400+
-----
401+
- 8 validators; a slot needs 6 votes (2/3) to be justified.
402+
- the chain:
403+
genesis
404+
- block_1(1) -> block_2(2) -> block_3(3)
405+
- dead_4(4) -> dead_5(5) -> dead_6(6)
406+
- heavy_7(7) -> heavy_8(8)
407+
- block_2 includes V0..V5's votes for block_1.
408+
- block_3 includes V0..V5's votes for block_2.
409+
- the chain through block_3 justifies slot 2 and finalizes slot 1.
410+
- the dead fork branches off block_3:
411+
- dead_4 includes V0..V5's votes for block_3, finalizing slot 2.
412+
- dead_5 includes V0..V5's votes for dead_4, finalizing slot 3.
413+
- dead_6 includes V0..V5's votes for dead_5, finalizing slot 4.
414+
so the dead fork reaches justified slot 5 and finalized slot 4 on dead_4.
415+
- the heavy fork branches off block_3:
416+
- heavy_8 includes V0..V5's votes for heavy_7, source block_2.
417+
- those 6 votes justify heavy_7 at slot 7.
418+
- slots 3, 4, 5, 6 between source block_2 and target heavy_7 are justifiable,
419+
so the heavy fork finalizes nothing beyond slot 1.
420+
421+
When
422+
----
423+
- the dead fork is built to justified slot 5 and finalized slot 4, then the
424+
heavy fork justifies slot 7, the highest justified checkpoint in the store.
425+
426+
Then
427+
----
428+
- the justified checkpoint moves to heavy_7 at slot 7, on the heavy fork.
429+
- head moves onto the heavy fork at heavy_8.
430+
- the finalized checkpoint tracks the canonical head heavy_8's state: slot 1 on
431+
block_1. It must not stay at the dead fork's higher finalized slot 4 on dead_4,
432+
which is not an ancestor of the head.
433+
434+
Reachability
435+
------------
436+
- head selection starts from the justified root at heavy_7.
437+
- the dead fork branches off block_3, below the justified root, so the forward
438+
walk never reaches it and its finalized checkpoint cannot stay canonical.
439+
"""
440+
fork_choice_test(
441+
anchor_state=generate_pre_state(num_validators=8),
442+
steps=[
443+
BlockStep(
444+
block=BlockSpec(slot=Slot(1), label="block_1"),
445+
checks=StoreChecks(head_slot=Slot(1)),
446+
),
447+
BlockStep(
448+
block=BlockSpec(
449+
slot=Slot(2),
450+
parent_label="block_1",
451+
label="block_2",
452+
attestations=[
453+
AggregatedAttestationSpec(
454+
validator_indices=[ValidatorIndex(i) for i in range(6)],
455+
slot=Slot(2),
456+
target_slot=Slot(1),
457+
target_root_label="block_1",
458+
),
459+
],
460+
),
461+
checks=StoreChecks(latest_justified_slot=Slot(1)),
462+
),
463+
BlockStep(
464+
block=BlockSpec(
465+
slot=Slot(3),
466+
parent_label="block_2",
467+
label="block_3",
468+
attestations=[
469+
AggregatedAttestationSpec(
470+
validator_indices=[ValidatorIndex(i) for i in range(6)],
471+
slot=Slot(3),
472+
target_slot=Slot(2),
473+
target_root_label="block_2",
474+
source_slot=Slot(1),
475+
source_root_label="block_1",
476+
),
477+
],
478+
),
479+
checks=StoreChecks(
480+
latest_justified_slot=Slot(2),
481+
latest_finalized_slot=Slot(1),
482+
),
483+
),
484+
BlockStep(
485+
block=BlockSpec(
486+
slot=Slot(4),
487+
parent_label="block_3",
488+
label="dead_4",
489+
attestations=[
490+
AggregatedAttestationSpec(
491+
validator_indices=[ValidatorIndex(i) for i in range(6)],
492+
slot=Slot(4),
493+
target_slot=Slot(3),
494+
target_root_label="block_3",
495+
source_slot=Slot(2),
496+
source_root_label="block_2",
497+
),
498+
],
499+
),
500+
checks=StoreChecks(
501+
latest_justified_slot=Slot(3),
502+
latest_finalized_slot=Slot(2),
503+
),
504+
),
505+
BlockStep(
506+
block=BlockSpec(
507+
slot=Slot(5),
508+
parent_label="dead_4",
509+
label="dead_5",
510+
attestations=[
511+
AggregatedAttestationSpec(
512+
validator_indices=[ValidatorIndex(i) for i in range(6)],
513+
slot=Slot(5),
514+
target_slot=Slot(4),
515+
target_root_label="dead_4",
516+
source_slot=Slot(3),
517+
source_root_label="block_3",
518+
),
519+
],
520+
),
521+
checks=StoreChecks(
522+
latest_justified_slot=Slot(4),
523+
latest_finalized_slot=Slot(3),
524+
),
525+
),
526+
BlockStep(
527+
block=BlockSpec(
528+
slot=Slot(6),
529+
parent_label="dead_5",
530+
label="dead_6",
531+
attestations=[
532+
AggregatedAttestationSpec(
533+
validator_indices=[ValidatorIndex(i) for i in range(6)],
534+
slot=Slot(6),
535+
target_slot=Slot(5),
536+
target_root_label="dead_5",
537+
source_slot=Slot(4),
538+
source_root_label="dead_4",
539+
),
540+
],
541+
),
542+
checks=StoreChecks(
543+
head_root_label="dead_6",
544+
latest_justified_slot=Slot(5),
545+
latest_finalized_slot=Slot(4),
546+
latest_finalized_root_label="dead_4",
547+
),
548+
),
549+
BlockStep(
550+
block=BlockSpec(slot=Slot(7), parent_label="block_3", label="heavy_7"),
551+
checks=StoreChecks(head_root_label="dead_6"),
552+
),
553+
BlockStep(
554+
block=BlockSpec(
555+
slot=Slot(8),
556+
parent_label="heavy_7",
557+
label="heavy_8",
558+
attestations=[
559+
AggregatedAttestationSpec(
560+
validator_indices=[ValidatorIndex(i) for i in range(6)],
561+
slot=Slot(8),
562+
target_slot=Slot(7),
563+
target_root_label="heavy_7",
564+
source_slot=Slot(2),
565+
source_root_label="block_2",
566+
),
567+
],
568+
),
569+
checks=StoreChecks(
570+
head_root_label="heavy_8",
571+
latest_justified_slot=Slot(7),
572+
latest_justified_root_label="heavy_7",
573+
latest_finalized_slot=Slot(1),
574+
latest_finalized_root_label="block_1",
575+
),
576+
),
577+
],
578+
)

0 commit comments

Comments
 (0)