Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion crates/blockchain/src/block_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,11 +457,19 @@ impl ProjectedState {
if !known_block_roots.contains(&att_data.head.root) {
return Err("head_root_unknown");
}
// The projection is seeded from the head state, whose window stops at
// `head.slot - 1`, so slots between the head and the candidate block are
// legitimately untracked here. That is not a validity verdict: this is a
// pre-filter over gossip candidates, so an untracked slot reads as "not
// justified yet" and the STF stays the authority on
// `JustifiedSlotOutOfRange`.
if !justified_slots_ops::is_slot_justified(
&self.justified_slots,
self.finalized_slot,
att_data.source.slot,
) {
)
.unwrap_or(false)
{
return Err("source_not_justified");
}
if !attestation_data_matches_chain(extended_historical_block_hashes, att_data) {
Expand All @@ -471,12 +479,15 @@ impl ProjectedState {
if !is_genesis_self_vote && att_data.target.slot <= att_data.source.slot {
return Err("target_not_after_source");
}
// An untracked target slot (commonly the head block's own slot) is not yet
// justified, so it stays eligible. Same reasoning as the source check above.
if !is_genesis_self_vote
&& justified_slots_ops::is_slot_justified(
&self.justified_slots,
self.finalized_slot,
att_data.target.slot,
)
.unwrap_or(false)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Out-of-range target aborts proposal

When a pooled attestation has an in-range source but targets the proposed block's slot or later, .unwrap_or(false) treats the out-of-range target as eligible. The selected attestation then raises JustifiedSlotOutOfRange in the final state transition, aborting block construction and causing the proposer to miss the slot.

Knowledge Base Used: Blockchain core: fork choice, state transition, block building, sync

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/blockchain/src/block_builder.rs
Line: 490

Comment:
**Out-of-range target aborts proposal**

When a pooled attestation has an in-range source but targets the proposed block's slot or later, `.unwrap_or(false)` treats the out-of-range target as eligible. The selected attestation then raises `JustifiedSlotOutOfRange` in the final state transition, aborting block construction and causing the proposer to miss the slot.

**Knowledge Base Used:** [Blockchain core: fork choice, state transition, block building, sync](https://app.greptile.com/lambdaclass/-/custom-context/knowledge-base/lambdaclass/ethlambda/-/docs/blockchain-core.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

{
return Err("target_already_justified");
}
Expand Down
27 changes: 23 additions & 4 deletions crates/blockchain/state_transition/src/justified_slots_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

use ethlambda_types::state::JustifiedSlots;

use crate::Error;

/// Calculate relative index for a slot after finalization.
/// Returns None if slot <= finalized_slot (implicitly justified).
fn relative_index(target_slot: u64, finalized_slot: u64) -> Option<usize> {
Expand All @@ -16,10 +18,27 @@ fn relative_index(target_slot: u64, finalized_slot: u64) -> Option<usize> {
}

/// Check if a slot is justified (finalized slots are implicitly justified).
pub fn is_slot_justified(slots: &JustifiedSlots, finalized_slot: u64, target_slot: u64) -> bool {
relative_index(target_slot, finalized_slot)
.map(|idx| slots.get(idx).unwrap_or(false))
.unwrap_or(true) // Finalized slots are implicitly justified
///
/// A slot past the finalized boundary but beyond the tracked bitlist has no
/// justification status to report. The spec surfaces that as a domain rejection
/// (leanSpec #1023, `JUSTIFIED_SLOT_OUT_OF_RANGE`) rather than reading it as
/// "not justified", so a block carrying such a vote is invalid. Callers on the
/// state transition path must propagate the error; callers that only pre-filter
/// candidate votes may treat it as "unknown, so not justified".
pub fn is_slot_justified(
slots: &JustifiedSlots,
finalized_slot: u64,
target_slot: u64,
) -> Result<bool, Error> {
// Finalized slots are implicitly justified and need no tracked bit.
let Some(idx) = relative_index(target_slot, finalized_slot) else {
return Ok(true);
};
slots.get(idx).ok_or(Error::JustifiedSlotOutOfRange {
slot: target_slot,
finalized_slot,
tracked_length: slots.len(),
})
}

/// Mark a slot as justified. No-op if slot is finalized.
Expand Down
37 changes: 26 additions & 11 deletions crates/blockchain/state_transition/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ pub enum Error {
index: usize,
validator_count: usize,
},
#[error(
"justified slot {slot} is outside the tracked range (finalized_boundary={finalized_slot}, tracked_length={tracked_length})"
)]
JustifiedSlotOutOfRange {
slot: u64,
finalized_slot: u64,
tracked_length: usize,
},
}

/// Transition the given pre-state to the block's post-state.
Expand Down Expand Up @@ -309,7 +317,7 @@ fn process_attestations(
let source = attestation_data.source;
let target = attestation_data.target;

if !is_valid_vote(state, attestation_data) {
if !is_valid_vote(state, attestation_data)? {
continue;
}

Expand Down Expand Up @@ -392,7 +400,15 @@ fn process_attestations(
/// rejects zero-hash source or target roots)
/// 4. Target slot > source slot
/// 5. Target slot is justifiable after the finalized slot
fn is_valid_vote(state: &State, data: &AttestationData) -> bool {
///
/// A failed check drops the vote and leaves the block valid, matching the
/// spec's `continue` semantics. The exception is a source or target slot past
/// the tracked justification window: that has no justification status to read
/// at all, so it invalidates the whole block via `JustifiedSlotOutOfRange`
/// (leanSpec #1023). After `process_block_header` the window covers up to
/// `block.slot - 1`, so this is exactly a vote whose source or target slot is
/// at or beyond the importing block's own slot.
fn is_valid_vote(state: &State, data: &AttestationData) -> Result<bool, Error> {
let source = data.source;
let target = data.target;

Expand All @@ -401,37 +417,36 @@ fn is_valid_vote(state: &State, data: &AttestationData) -> bool {
&state.justified_slots,
state.latest_finalized.slot,
source.slot,
) {
// TODO: why doesn't this make the block invalid?
return false;
)? {
return Ok(false);
}

// Ignore votes for targets that have already reached consensus
if justified_slots_ops::is_slot_justified(
&state.justified_slots,
state.latest_finalized.slot,
target.slot,
) {
return false;
)? {
return Ok(false);
}

// Ensure the vote refers to blocks that actually exist on our chain;
// also rejects zero-hash source or target inline.
if !attestation_data_matches_chain(&state.historical_block_hashes, data) {
return false;
return Ok(false);
}

// Ensure time flows forward
if target.slot <= source.slot {
return false;
return Ok(false);
}

// Ensure the target falls on a slot that can be justified after the finalized one.
if !slot_is_justifiable_after(target.slot, state.latest_finalized.slot) {
return false;
return Ok(false);
}

true
Ok(true)
}

/// Attempt to advance finalization from source to target.
Expand Down