Skip to content

Commit ea08e26

Browse files
fix: skip finalization advance for stale finalized sources (#405)
## Summary Ports [leanSpec #802](leanEthereum/leanSpec#802) to ethlambda. A supermajority attestation whose source checkpoint sits **at or behind the finalized boundary** is a valid justification anchor (`is_slot_justified` treats slots `<= finalized.slot` as already justified), so it may justify a newer target. But once it crosses the supermajority threshold, the finalization logic scanned `source.slot + 1 .. target.slot` and could try to advance (rewind/re-finalize) below the finalized boundary. The fix gates finalization advancement on `source.slot > finalized_slot`. Stale or boundary sources may still justify newer targets; they no longer try to rewind or scan below the finalized boundary. ## Relationship to #402 This supersedes [#402](#402) (`fix(stf): reject blocks justifying a target whose source is below finalized`), which was the pre-#802 attempt that fixed the issue by *rejecting* such blocks. The spec instead **accepts** the justification and only skips the finalization advance, so this PR follows the merged spec approach. #402 can be closed in favor of this. ## Changes Two mirrored spots get the `source.slot > finalized_slot` guard: | Location | Role | |----------|------| | `try_finalize` (state transition) | The actual finalization advance | | `score_entry` (block builder) | Projects/scores finalization while packing blocks | ## Tests - `state_transition`: `stale_finalized_source_justifies_without_rewinding_finalization` — supermajority from a stale source justifies the target while finalization stays pinned. - `block_builder`: `score_entry_does_not_finalize_source_at_boundary` — a boundary source is scored `Justify`, not `Finalize`. ``` cargo test -p ethlambda-state-transition --lib # 2 passed cargo test -p ethlambda-blockchain --lib block_builder # 8 passed ``` fmt + clippy (`-D warnings`) clean on both crates. Co-authored-by: Pablo Deymonnaz <pdeymon@fi.uba.ar>
1 parent 8e1fadb commit ea08e26

2 files changed

Lines changed: 133 additions & 6 deletions

File tree

crates/blockchain/src/block_builder.rs

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,10 +364,14 @@ fn score_entry(
364364
let total = prior_count + new_voters.len();
365365
let crosses_2_3 = 3 * total >= 2 * validator_count;
366366

367-
// 3SF-mini finalization requires no slot strictly between source.slot
368-
// and target.slot to still be justifiable (so source and target are
369-
// consecutive justified checkpoints in the projected post-state).
367+
// 3SF-mini finalization requires the source to lie past the finalized
368+
// boundary (a source at or behind it is already final and must not
369+
// re-finalize) and no slot strictly between source.slot and target.slot to
370+
// still be justifiable (so source and target are consecutive justified
371+
// checkpoints in the projected post-state). Mirrors `try_finalize` in the
372+
// state transition.
370373
let finalizes = crosses_2_3
374+
&& att_data.source.slot > projected_finalized_slot
371375
&& (att_data.source.slot + 1..att_data.target.slot)
372376
.all(|s| !slot_is_justifiable_after(s, projected_finalized_slot));
373377

@@ -688,6 +692,52 @@ mod tests {
688692
bits
689693
}
690694

695+
/// Regression (leanSpec #802): a supermajority entry whose source sits at
696+
/// the finalized boundary must be scored `Justify`, not `Finalize`. Such a
697+
/// source is already final, so it advances nothing; the empty scan range
698+
/// `(source.slot + 1..target.slot)` would otherwise make `.all(...)`
699+
/// vacuously true and mis-tier the entry as a finalizer.
700+
#[test]
701+
fn score_entry_does_not_finalize_source_at_boundary() {
702+
const NUM_VALIDATORS: usize = 4;
703+
const FINALIZED_SLOT: u64 = 4;
704+
705+
// Source at the finalized boundary, target one slot ahead (empty scan).
706+
let att_data = AttestationData {
707+
slot: 7,
708+
head: Checkpoint {
709+
slot: 5,
710+
root: H256([5u8; 32]),
711+
},
712+
target: Checkpoint {
713+
slot: 5,
714+
root: H256([5u8; 32]),
715+
},
716+
source: Checkpoint {
717+
slot: FINALIZED_SLOT,
718+
root: H256([4u8; 32]),
719+
},
720+
};
721+
722+
// Supermajority (3 of 4) so the entry crosses 2/3.
723+
let proofs = vec![AggregatedSignatureProof::empty(make_bits(&[0, 1, 2]))];
724+
725+
let (score, _) = score_entry(
726+
&att_data,
727+
&proofs,
728+
&HashMap::new(),
729+
FINALIZED_SLOT,
730+
NUM_VALIDATORS,
731+
)
732+
.expect("entry contributes new voters");
733+
734+
assert_eq!(
735+
score.tier,
736+
Tier::Justify,
737+
"source at the finalized boundary must justify, not finalize"
738+
);
739+
}
740+
691741
/// Regression test for https://github.com/lambdaclass/ethlambda/issues/259
692742
///
693743
/// Simulates a stall scenario by populating the payload pool with 50

crates/blockchain/state_transition/src/lib.rs

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,16 +387,25 @@ fn is_valid_vote(state: &State, data: &AttestationData) -> bool {
387387

388388
/// Attempt to advance finalization from source to target.
389389
///
390-
/// Finalization succeeds when there are no justifiable slots between
391-
/// source.slot and target.slot (exclusive). When finalization advances,
392-
/// shifts the justified_slots window and prunes stale justifications.
390+
/// Finalization advances only when the source lies past the old finalized point
391+
/// and there are no justifiable slots between source.slot and target.slot
392+
/// (exclusive). A source at or behind the finalized boundary is already final:
393+
/// it may justify a newer target, but it must not re-finalize or scan below the
394+
/// finalized boundary. When finalization advances, shifts the justified_slots
395+
/// window and prunes stale justifications.
393396
fn try_finalize(
394397
state: &mut State,
395398
source: Checkpoint,
396399
target: Checkpoint,
397400
justifications: &mut HashMap<H256, Vec<bool>>,
398401
root_to_slot: &HashMap<H256, u64>,
399402
) {
403+
// A stale or boundary source is already finalized; advancing from it would
404+
// rewind the finalized checkpoint and scan slots below the boundary.
405+
if source.slot <= state.latest_finalized.slot {
406+
return;
407+
}
408+
400409
// Consider whether finalization can advance.
401410
if ((source.slot + 1)..target.slot)
402411
.any(|slot| slot_is_justifiable_after(slot, state.latest_finalized.slot))
@@ -683,4 +692,72 @@ mod tests {
683692
);
684693
assert_eq!(state.latest_justified.root, r9);
685694
}
695+
696+
/// Regression (leanSpec #802): a supermajority attestation whose source sits
697+
/// at or behind the finalized boundary may justify a newer target, but must
698+
/// never advance (rewind) finalization below that boundary.
699+
///
700+
/// Setup: finalized = justified = slot 4. A supermajority votes from a stale
701+
/// source at slot 1 to a fresh target at slot 6 (Δ=2 is pronic, so the target
702+
/// is justifiable). The target should become justified while the finalized
703+
/// checkpoint stays pinned at slot 4.
704+
#[test]
705+
fn stale_finalized_source_justifies_without_rewinding_finalization() {
706+
const NUM_VALIDATORS: usize = 4;
707+
let r1 = H256([1u8; 32]);
708+
let r4 = H256([4u8; 32]);
709+
let r6 = H256([6u8; 32]);
710+
711+
let mut hashes: Vec<H256> = vec![H256::ZERO; 7];
712+
hashes[1] = r1;
713+
hashes[4] = r4;
714+
hashes[6] = r6;
715+
716+
let validators = make_validators(NUM_VALIDATORS);
717+
// Window is relative to finalized=4; cover up to slot 6 (slots 5 and 6).
718+
let mut justified_slots = JustifiedSlots::new();
719+
justified_slots_ops::extend_to_slot(&mut justified_slots, 4, 6);
720+
721+
let mut state = State {
722+
config: ChainConfig { genesis_time: 0 },
723+
slot: 7,
724+
latest_block_header: BlockHeader {
725+
slot: 6,
726+
proposer_index: 0,
727+
parent_root: H256::ZERO,
728+
state_root: H256::ZERO,
729+
body_root: BlockBody::default().hash_tree_root(),
730+
},
731+
latest_justified: Checkpoint { slot: 4, root: r4 },
732+
latest_finalized: Checkpoint { slot: 4, root: r4 },
733+
historical_block_hashes: SszList::try_from(hashes).unwrap(),
734+
justified_slots,
735+
validators: SszList::try_from(validators).unwrap(),
736+
justifications_roots: Default::default(),
737+
justifications_validators: JustificationValidators::new(),
738+
};
739+
740+
// Supermajority (3 of 4) attesting from the stale source (slot 1) to the
741+
// fresh target (slot 6). Source slot 1 <= finalized 4, so it is implicitly
742+
// justified and passes is_valid_vote.
743+
let atts: Vec<AggregatedAttestation> = vec![make_attestation(
744+
7,
745+
(1, r1),
746+
(6, r6),
747+
(6, r6),
748+
&[0, 1, 2],
749+
NUM_VALIDATORS,
750+
)];
751+
let atts: AggregatedAttestations = atts.try_into().unwrap();
752+
753+
process_attestations(&mut state, &atts).expect("process_attestations should succeed");
754+
755+
// The target is justified.
756+
assert_eq!(state.latest_justified.slot, 6);
757+
assert_eq!(state.latest_justified.root, r6);
758+
// Finalization stays pinned at the boundary: the stale source must not
759+
// rewind or re-finalize.
760+
assert_eq!(state.latest_finalized.slot, 4);
761+
assert_eq!(state.latest_finalized.root, r4);
762+
}
686763
}

0 commit comments

Comments
 (0)