Skip to content

Commit 51e7779

Browse files
bomanapstcoratger
andauthored
fix use updated finalized_slot for justifiability (leanEthereum#443)
* fix use updated finalized_slot for justifiability * test verify mid-loop finalized_slot visibility * Apply suggestions from code review * Update tests/consensus/devnet/fc/test_finalization_mid_processing.py * Update tests/consensus/devnet/fc/test_finalization_mid_processing.py --------- Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
1 parent 9715f5a commit 51e7779

2 files changed

Lines changed: 158 additions & 2 deletions

File tree

src/lean_spec/subspecs/containers/state/state.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ def process_attestations(
486486
#
487487
# Any target outside this pattern is not eligible for justification,
488488
# so votes for it are simply ignored.
489-
if not target.slot.is_justifiable_after(self.latest_finalized.slot):
489+
if not target.slot.is_justifiable_after(finalized_slot):
490490
continue
491491

492492
# Record the vote.
@@ -540,7 +540,7 @@ def process_attestations(
540540
#
541541
# If there is no break in the chain, advance finalization.
542542
if not any(
543-
Slot(slot).is_justifiable_after(self.latest_finalized.slot)
543+
Slot(slot).is_justifiable_after(finalized_slot)
544544
for slot in range(source.slot + Slot(1), target.slot)
545545
):
546546
old_finalized_slot = finalized_slot
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
"""Fork Choice: Finalization advances mid-attestation processing.
2+
3+
This test verifies that attestations see updated finalized_slot during processing,
4+
as required by the 3sf-mini specification.
5+
6+
Reference: https://github.com/leanEthereum/leanSpec/pull/443
7+
"""
8+
9+
import pytest
10+
from consensus_testing import (
11+
AggregatedAttestationSpec,
12+
BlockSpec,
13+
BlockStep,
14+
ForkChoiceTestFiller,
15+
StoreChecks,
16+
)
17+
18+
from lean_spec.subspecs.containers.slot import Slot
19+
from lean_spec.subspecs.containers.validator import ValidatorIndex
20+
21+
pytestmark = pytest.mark.valid_until("Devnet")
22+
23+
24+
def test_finalization_advances_mid_attestation_processing(
25+
fork_choice_test: ForkChoiceTestFiller,
26+
) -> None:
27+
"""
28+
Verify attestations see updated finalized_slot during processing.
29+
30+
Scenario
31+
--------
32+
Process two attestations (both with supermajority) in the same block:
33+
34+
- Attestation A: source=1, target=2 -> justifies slot 2, finalizes slot 1
35+
- Attestation B: source=1, target=7 -> only justifiable after finalization
36+
37+
Justifiability
38+
--------------
39+
Slot 7 justifiability depends on finalized_slot:
40+
41+
- finalized=0: delta=7, NOT justifiable (7 > 5, not square, not pronic)
42+
- finalized=1: delta=6, IS justifiable (pronic = 2*3)
43+
44+
Expected Behavior
45+
----------------------------
46+
47+
1. Attestation A justifies slot 2 and finalizes slot 1
48+
2. Attestation B sees updated finalized_slot=1, is justifiable, gets processed
49+
3. Attestation B justifies slot 7 (supermajority)
50+
4. latest_justified_slot = 7 (B processed after A)
51+
52+
Reference
53+
---------
54+
https://github.com/leanEthereum/leanSpec/pull/443
55+
"""
56+
fork_choice_test(
57+
steps=[
58+
# Build chain through slot 7
59+
BlockStep(
60+
block=BlockSpec(slot=Slot(1), label="block_1"),
61+
checks=StoreChecks(head_slot=Slot(1)),
62+
),
63+
BlockStep(
64+
block=BlockSpec(slot=Slot(2), label="block_2"),
65+
checks=StoreChecks(head_slot=Slot(2)),
66+
),
67+
# Slot 3: Justify slot 1 (source=0 -> target=1)
68+
# Need 3/4 validators for supermajority (3*3=9 >= 2*4=8)
69+
BlockStep(
70+
block=BlockSpec(
71+
slot=Slot(3),
72+
label="block_3",
73+
attestations=[
74+
AggregatedAttestationSpec(
75+
validator_ids=[
76+
ValidatorIndex(0),
77+
ValidatorIndex(1),
78+
ValidatorIndex(2),
79+
],
80+
slot=Slot(3),
81+
target_slot=Slot(1),
82+
target_root_label="block_1",
83+
),
84+
],
85+
),
86+
checks=StoreChecks(
87+
head_slot=Slot(3),
88+
latest_justified_slot=Slot(1),
89+
latest_finalized_slot=Slot(0),
90+
),
91+
),
92+
# Extend chain to slot 7
93+
BlockStep(
94+
block=BlockSpec(slot=Slot(4), label="block_4"),
95+
checks=StoreChecks(head_slot=Slot(4)),
96+
),
97+
BlockStep(
98+
block=BlockSpec(slot=Slot(5), label="block_5"),
99+
checks=StoreChecks(head_slot=Slot(5)),
100+
),
101+
BlockStep(
102+
block=BlockSpec(slot=Slot(6), label="block_6"),
103+
checks=StoreChecks(head_slot=Slot(6)),
104+
),
105+
BlockStep(
106+
block=BlockSpec(slot=Slot(7), label="block_7"),
107+
checks=StoreChecks(head_slot=Slot(7)),
108+
),
109+
# Slot 8: The critical block with both attestations
110+
BlockStep(
111+
block=BlockSpec(
112+
slot=Slot(8),
113+
attestations=[
114+
# Attestation A: Justify slot 2 and finalize slot 1
115+
# Source will be slot 1 (latest_justified from parent state)
116+
# Target is slot 2
117+
# Finalization: range(1+1, 2) = empty -> finalizes slot 1
118+
# Need 3/4 validators for supermajority
119+
AggregatedAttestationSpec(
120+
validator_ids=[
121+
ValidatorIndex(0),
122+
ValidatorIndex(1),
123+
ValidatorIndex(2),
124+
],
125+
slot=Slot(8),
126+
target_slot=Slot(2),
127+
target_root_label="block_2",
128+
),
129+
# Attestation B: Target slot 7 - ALSO needs supermajority
130+
# With finalized=0: delta=7, NOT justifiable -> SKIPPED
131+
# With finalized=1: delta=6, IS justifiable (pronic) -> PROCESSED
132+
#
133+
# If processed, slot 7 becomes justified and latest_justified=7
134+
# If skipped, latest_justified stays at 2
135+
# This is how we detect the bug!
136+
AggregatedAttestationSpec(
137+
validator_ids=[
138+
ValidatorIndex(0),
139+
ValidatorIndex(1),
140+
ValidatorIndex(2),
141+
],
142+
slot=Slot(8),
143+
target_slot=Slot(7),
144+
target_root_label="block_7",
145+
),
146+
],
147+
),
148+
checks=StoreChecks(
149+
head_slot=Slot(8),
150+
# B is processed -> slot 7 justified -> latest_justified=7
151+
latest_justified_slot=Slot(7),
152+
latest_finalized_slot=Slot(1),
153+
),
154+
),
155+
],
156+
)

0 commit comments

Comments
 (0)