Skip to content

Commit 02ff520

Browse files
authored
fix(fork-choice): reject attestation slot before head block slot (lambdaclass#442)
## What Ports the head-consistency guard from [leanSpec PR #1020](leanEthereum/leanSpec#1020) into `validate_attestation_data`. A gossip attestation's slot records *when* the head was observed, so it cannot precede the slot of the head block it claims to have seen. We did not enforce this lower bound: the wire `slot` field was untethered from any real block, free to be set arbitrarily relative to the observed head. ## Why | Property | Risk without the guard | |----------|------------------------| | Soundness | An unbounded slot violates 3SF attestation semantics: the slot is meant to anchor when the head was seen. | | Liveness (spec context) | In leanSpec a crafted near-`2**64` slot overflowed the slot→interval constructor and crashed the node (remote DoS). | **Note on the overflow half:** ethlambda's time check already uses `saturating_mul` (`data.slot.saturating_mul(INTERVALS_PER_SLOT)`), so a near-`u64::MAX` slot saturates to `u64::MAX` and is rejected cleanly as too-far-in-future rather than panicking. The Rust port was never crashable the way the unchecked Python constructor was, so no change to the time-check arithmetic is needed; the spec's switch to slot-unit comparison is mathematically equivalent to the existing interval comparison. This PR adds the genuinely missing piece: the semantic lower bound. ## Change - New `StoreError::AttestationSlotBeforeHead { attestation_slot, head_slot }` rejection. - Check `data.slot < data.head.slot` after the ancestry checks, before the time check (mirrors the spec's ordering). - Two unit tests: a vote whose slot precedes its head (rejected as `AttestationSlotBeforeHead`), and a `u64::MAX` ceiling slot (rejected as `AttestationTooFarInFuture`, confirming no overflow). Applies to both gossip paths via the shared `validate_attestation_data` (`on_gossip_attestation` and `on_gossip_aggregated_attestation`). ## Testing ``` cargo test -p ethlambda-blockchain --lib validate_attestation # 5 passed cargo fmt --all && cargo clippy -p ethlambda-blockchain --lib # clean ``` https://claude.ai/code/session_018pvnQJkom3DobK9oSbK42T
1 parent 2aa1d0e commit 02ff520

1 file changed

Lines changed: 92 additions & 1 deletion

File tree

crates/blockchain/src/store.rs

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ fn checkpoint_is_ancestor(
163163
/// 3. The head must be at least as recent as source and target.
164164
/// 4. Checkpoint slots must match the actual block slots.
165165
/// 5. Source, target, and head must lie on one parent chain.
166-
/// 6. The vote's slot must have started locally (a small disparity margin is allowed).
166+
/// 6. The vote's slot cannot precede the slot of the head it claims to have seen.
167+
/// 7. The vote's slot must have started locally (a small disparity margin is allowed).
167168
fn validate_attestation_data(store: &Store, data: &AttestationData) -> Result<(), StoreError> {
168169
let _timing = metrics::time_attestation_validation();
169170

@@ -222,6 +223,15 @@ fn validate_attestation_data(store: &Store, data: &AttestationData) -> Result<()
222223
return Err(StoreError::TargetNotAncestorOfHead);
223224
}
224225

226+
// Head Consistency Check - A vote cannot have observed its head before that
227+
// head existed, so the vote's slot must not precede the head block's slot.
228+
if data.slot < data.head.slot {
229+
return Err(StoreError::AttestationSlotBeforeHead {
230+
attestation_slot: data.slot,
231+
head_slot: data.head.slot,
232+
});
233+
}
234+
225235
// Time Check - Honest validators emit votes only after their slot has begun.
226236
// Allow a small disparity margin for clock skew between peers.
227237
//
@@ -858,6 +868,12 @@ pub enum StoreError {
858868
#[error("Target checkpoint must be ancestor of head")]
859869
TargetNotAncestorOfHead,
860870

871+
#[error("Attestation slot {attestation_slot} precedes head block slot {head_slot}")]
872+
AttestationSlotBeforeHead {
873+
attestation_slot: u64,
874+
head_slot: u64,
875+
},
876+
861877
#[error(
862878
"Attestation slot {attestation_slot} is too far in future (store time: {store_time} intervals)"
863879
)]
@@ -1273,6 +1289,81 @@ mod tests {
12731289
);
12741290
}
12751291

1292+
/// leanSpec #1020: a vote whose slot precedes the slot of the head block it
1293+
/// claims to have seen must be rejected. The vote cannot have observed its
1294+
/// head before that head existed.
1295+
#[test]
1296+
fn validate_attestation_rejects_slot_before_head() {
1297+
let mut store = new_test_store();
1298+
let genesis = store.head();
1299+
1300+
let b1 = H256([1u8; 32]);
1301+
let b2 = H256([2u8; 32]);
1302+
let b3 = H256([3u8; 32]);
1303+
insert_test_block(&mut store, b1, 1, genesis);
1304+
insert_test_block(&mut store, b2, 2, b1);
1305+
insert_test_block(&mut store, b3, 3, b2);
1306+
store.set_time(3 * INTERVALS_PER_SLOT);
1307+
1308+
// head=b3 at slot 3, but the vote's own slot is 2: it claims to have seen
1309+
// a head that did not yet exist when the vote was cast.
1310+
let data = AttestationData {
1311+
slot: 2,
1312+
source: Checkpoint {
1313+
root: genesis,
1314+
slot: 0,
1315+
},
1316+
target: Checkpoint { root: b2, slot: 2 },
1317+
head: Checkpoint { root: b3, slot: 3 },
1318+
};
1319+
1320+
let result = validate_attestation_data(&store, &data);
1321+
assert!(
1322+
matches!(
1323+
result,
1324+
Err(StoreError::AttestationSlotBeforeHead {
1325+
attestation_slot: 2,
1326+
head_slot: 3,
1327+
})
1328+
),
1329+
"Expected AttestationSlotBeforeHead, got: {result:?}"
1330+
);
1331+
}
1332+
1333+
/// leanSpec #1020: a vote whose slot sits at the unsigned 64-bit ceiling must
1334+
/// be rejected cleanly as too-far-in-future, without overflowing the
1335+
/// slot-to-interval multiplication. `saturating_mul` clamps the product at
1336+
/// `u64::MAX` rather than panicking on overflow.
1337+
#[test]
1338+
fn validate_attestation_rejects_ceiling_slot_without_overflow() {
1339+
let mut store = new_test_store();
1340+
let genesis = store.head();
1341+
1342+
let b1 = H256([1u8; 32]);
1343+
let b2 = H256([2u8; 32]);
1344+
insert_test_block(&mut store, b1, 1, genesis);
1345+
insert_test_block(&mut store, b2, 2, b1);
1346+
store.set_time(2 * INTERVALS_PER_SLOT);
1347+
1348+
// A crafted gossip vote with a near-`u64::MAX` slot. The head-consistency
1349+
// check passes (slot >= head.slot), so this exercises the time check.
1350+
let data = AttestationData {
1351+
slot: u64::MAX,
1352+
source: Checkpoint {
1353+
root: genesis,
1354+
slot: 0,
1355+
},
1356+
target: Checkpoint { root: b1, slot: 1 },
1357+
head: Checkpoint { root: b2, slot: 2 },
1358+
};
1359+
1360+
let result = validate_attestation_data(&store, &data);
1361+
assert!(
1362+
matches!(result, Err(StoreError::AttestationTooFarInFuture { .. })),
1363+
"Expected AttestationTooFarInFuture, got: {result:?}"
1364+
);
1365+
}
1366+
12761367
/// A vote whose source, target, and head form one parent chain validates.
12771368
#[test]
12781369
fn validate_attestation_accepts_proper_ancestor_chain() {

0 commit comments

Comments
 (0)