Skip to content

Commit ae6ac42

Browse files
authored
fix(state-transition): preserve monotonicity of latest_justified within a block (lambdaclass#390)
## Description We recently found a bug in the spec and made a PR patching it: leanEthereum/leanSpec#781. This PR updates our code matching those changes. ## What changed - `process_attestations` unconditionally assigned `state.latest_justified = target` on every supermajority threshold crossing. When a block carried multiple supermajority attestations whose targets were not in increasing slot order, a later attestation clobbered an earlier one and the post-state ended below the highest justified target. - The store latches `latest_justified` monotonically across all branches (`crates/blockchain/src/store.rs:485-491`). Once any branch advanced the store ahead of the canonical chain's post-state, every subsequent proposal failed `JustifiedDivergenceNotClosed` and the chain froze permanently. - Fix: guard the assignment so `latest_justified` only advances. `justified_slots` continues to be updated unconditionally — multiple slots can be independently justified within a block. ## Observed failure Devnet froze at slot 27969 (finalized) / 27978 (justified) / 27984 (head). Investigation: - Canonical head `0xef43cf66` (slot 27984) carried 3 supermajority attestations in body order targeting slots **27978, 27981, 27974** (all justifiable from `source=27972`). - After `process_attestations`, post-state `latest_justified = (27974, ...)` instead of `(27981, ...)`, because the last-applied attestation overwrote the higher justifications. - Meanwhile, fork block `0xe88cff27` at slot 27983 had advanced the store's `latest_justified` to 27978. - From then on, every proposer's `build_block` produced `justified=27974` and failed the `JustifiedDivergenceNotClosed` check at `crates/blockchain/src/store.rs:737`. **37,787** proposal failures across 4 validators over 4 days before recovery. ## Test plan - [x] New regression test `latest_justified_does_not_regress_within_block` in `state_transition/src/lib.rs`: 4-validator state, three supermajority attestations in body order targeting slots 4 → 9 → 6. Fails on `main` (ends at slot 6); passes with the fix (stays at slot 9). - [x] `cargo test -p ethlambda-state-transition --lib` — 1/1 pass - [x] `cargo test -p ethlambda-state-transition --test stf_spectests` — 51/51 pass - [x] `cargo clippy -p ethlambda-state-transition --all-targets -- -D warnings` — clean
1 parent 7bb14c3 commit ae6ac42

3 files changed

Lines changed: 151 additions & 2 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/blockchain/state_transition/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ serde.workspace = true
2222
serde_json.workspace = true
2323

2424
hex.workspace = true
25+
libssz-types.workspace = true
2526

2627
datatest-stable = "0.3.3"
2728

crates/blockchain/state_transition/src/lib.rs

Lines changed: 149 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,9 @@ fn process_attestations(
304304
// Check whether the vote count crosses the supermajority threshold
305305
let vote_count = votes.iter().filter(|voted| **voted).count();
306306
if 3 * vote_count >= 2 * validator_count {
307-
// The block becomes justified
308-
state.latest_justified = target;
307+
// If the slot is higher, update the latest justified
308+
state.latest_justified =
309+
std::cmp::max_by_key(state.latest_justified, target, |c| c.slot);
309310
justified_slots_ops::set_justified(
310311
&mut state.justified_slots,
311312
state.latest_finalized.slot,
@@ -537,3 +538,149 @@ pub fn slot_is_justifiable_after(slot: u64, finalized_slot: u64) -> bool {
537538
.and_then(|v| v.checked_add(1))
538539
.is_some_and(|val| val.isqrt().pow(2) == val && val % 2 == 1)
539540
}
541+
542+
#[cfg(test)]
543+
mod tests {
544+
use super::*;
545+
use ethlambda_types::{
546+
attestation::{AggregatedAttestation, AggregationBits, AttestationData},
547+
block::BlockBody,
548+
checkpoint::Checkpoint,
549+
primitives::H256,
550+
state::{ChainConfig, JustifiedSlots, State, Validator},
551+
};
552+
use libssz_types::SszList;
553+
554+
fn make_validators(n: usize) -> Vec<Validator> {
555+
(0..n)
556+
.map(|i| Validator {
557+
attestation_pubkey: [i as u8; 52],
558+
proposal_pubkey: [i as u8; 52],
559+
index: i as u64,
560+
})
561+
.collect()
562+
}
563+
564+
fn make_bits(set: &[usize], len: usize) -> AggregationBits {
565+
let mut b = AggregationBits::with_length(len).unwrap();
566+
for &i in set {
567+
b.set(i, true).unwrap();
568+
}
569+
b
570+
}
571+
572+
fn make_attestation(
573+
att_slot: u64,
574+
src: (u64, H256),
575+
tgt: (u64, H256),
576+
head: (u64, H256),
577+
bits_set: &[usize],
578+
bits_len: usize,
579+
) -> AggregatedAttestation {
580+
AggregatedAttestation {
581+
aggregation_bits: make_bits(bits_set, bits_len),
582+
data: AttestationData {
583+
slot: att_slot,
584+
source: Checkpoint {
585+
slot: src.0,
586+
root: src.1,
587+
},
588+
target: Checkpoint {
589+
slot: tgt.0,
590+
root: tgt.1,
591+
},
592+
head: Checkpoint {
593+
slot: head.0,
594+
root: head.1,
595+
},
596+
},
597+
}
598+
}
599+
600+
/// Regression: `process_attestations` must not let `state.latest_justified`
601+
/// regress within a single block when attestations appear in body order
602+
/// whose target slots are not monotonically increasing.
603+
///
604+
/// Observed on devnet at canonical slot 27984: a block carried three
605+
/// supermajority attestations targeting slots 27978, 27981, and 27974 (in
606+
/// that order). Each reached the supermajority threshold and the
607+
/// unconditional `state.latest_justified = target` assignment caused the
608+
/// post-state to end at `latest_justified.slot = 27974`. Because the
609+
/// store had already latched `latest_justified = 27978` from importing a
610+
/// fork block, every subsequent proposal failed
611+
/// `JustifiedDivergenceNotClosed` and the chain froze.
612+
///
613+
/// Compressed setup: finalized=0, source=3 (justified), targets in body
614+
/// order 4 / 9 / 6 — all justifiable from finalized=0 (Δ=4 ≤ 5, Δ=9=3²,
615+
/// Δ=6=2·3). With 4 validators, three votes is supermajority, so each
616+
/// attestation crosses the threshold.
617+
#[test]
618+
fn latest_justified_does_not_regress_within_block() {
619+
const NUM_VALIDATORS: usize = 4;
620+
let r3 = H256([3u8; 32]);
621+
let r4 = H256([4u8; 32]);
622+
let r6 = H256([6u8; 32]);
623+
let r9 = H256([9u8; 32]);
624+
625+
// historical_block_hashes indexed by slot. Genesis at slot 0 is ZERO
626+
// (matches the default finalized checkpoint), then we place the
627+
// canonical roots at slots 3 / 4 / 6 / 9. Other slots are empty and
628+
// are not referenced by any attestation in the test.
629+
let mut hashes: Vec<H256> = vec![H256::ZERO; 10];
630+
hashes[3] = r3;
631+
hashes[4] = r4;
632+
hashes[6] = r6;
633+
hashes[9] = r9;
634+
635+
let validators = make_validators(NUM_VALIDATORS);
636+
let mut justified_slots = JustifiedSlots::new();
637+
justified_slots_ops::extend_to_slot(&mut justified_slots, 0, 9);
638+
// Mark slot 3 as justified so source=(3, r3) passes is_valid_vote.
639+
justified_slots_ops::set_justified(&mut justified_slots, 0, 3);
640+
641+
let mut state = State {
642+
config: ChainConfig { genesis_time: 0 },
643+
slot: 10,
644+
latest_block_header: BlockHeader {
645+
slot: 9,
646+
proposer_index: 0,
647+
parent_root: H256::ZERO,
648+
state_root: H256::ZERO,
649+
body_root: BlockBody::default().hash_tree_root(),
650+
},
651+
latest_justified: Checkpoint { slot: 3, root: r3 },
652+
latest_finalized: Checkpoint {
653+
slot: 0,
654+
root: H256::ZERO,
655+
},
656+
historical_block_hashes: SszList::try_from(hashes).unwrap(),
657+
justified_slots,
658+
validators: SszList::try_from(validators).unwrap(),
659+
justifications_roots: Default::default(),
660+
justifications_validators: JustificationValidators::new(),
661+
};
662+
663+
// Three supermajority attestations (3 of 4 validators each), all from
664+
// source=(3, r3), in body order targeting slots 4 → 9 → 6.
665+
let atts: Vec<AggregatedAttestation> = vec![
666+
make_attestation(3, (3, r3), (4, r4), (4, r4), &[0, 1, 3], NUM_VALIDATORS),
667+
make_attestation(3, (3, r3), (9, r9), (9, r9), &[0, 1, 2], NUM_VALIDATORS),
668+
make_attestation(3, (3, r3), (6, r6), (6, r6), &[0, 2, 3], NUM_VALIDATORS),
669+
];
670+
let atts: AggregatedAttestations = atts.try_into().unwrap();
671+
672+
process_attestations(&mut state, &atts).expect("process_attestations should succeed");
673+
674+
// After processing, the chain's view of "latest justified" must be the
675+
// highest justified target (slot 9), not the last-processed one
676+
// (slot 6). Pre-fix this assertion fails with slot=6.
677+
assert_eq!(
678+
state.latest_justified.slot,
679+
9,
680+
"latest_justified regressed: got slot={}, root={}; expected slot=9",
681+
state.latest_justified.slot,
682+
ShortRoot(&state.latest_justified.root.0)
683+
);
684+
assert_eq!(state.latest_justified.root, r9);
685+
}
686+
}

0 commit comments

Comments
 (0)