Skip to content

Document on_block() validate-then-mutate structure and resolve stale TODOs #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pablodeymo wants to merge 2 commits into from
Closed

Document on_block() validate-then-mutate structure and resolve stale TODOs #154

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
083b34f
Document on_block() validate-then-mutate structure and resolve attest…
pablodeymo Feb 26, 2026
4122c73
Fix inaccurate attestation failure comments in on_block()
pablodeymo Feb 26, 2026
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
41 changes: 25 additions & 16 deletions crates/blockchain/src/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,18 @@ fn on_attestation(

/// Process a new block and update the forkchoice state.
///
/// This method integrates a block into the forkchoice store by:
/// 1. Validating the block's parent exists
/// 2. Computing the post-state via the state transition function
/// 3. Processing attestations included in the block body (on-chain)
/// 4. Updating the forkchoice head
/// 5. Processing the proposer's attestation (as if gossiped)
/// This method integrates a block into the forkchoice store in two phases:
///
/// **Phase 1 — Validation (read-only):**
/// 1. Verifying the block's parent state exists
/// 2. Validating cryptographic signatures (when enabled)
/// 3. Computing the post-state via the state transition function
///
/// **Phase 2 — Mutations (only if all validation passes):**
/// 4. Storing the block, state, and updating checkpoints
/// 5. Processing block body attestations into fork choice (best-effort)
/// 6. Updating the forkchoice head
/// 7. Processing the proposer's attestation (as pending)
pub fn on_block(
store: &mut Store,
signed_block: SignedBlockWithAttestation,
Expand Down Expand Up @@ -349,8 +355,6 @@ pub fn on_block(
})?;

// Validate cryptographic signatures
// TODO: extract signature verification to a pre-checks function
// to avoid the need for this
if cfg!(not(feature = "skip-signature-verification")) {
verify_signatures(&parent_state, &signed_block)?;
}
Expand All @@ -363,6 +367,8 @@ pub fn on_block(
let state_root = block.state_root;
post_state.latest_block_header.state_root = state_root;

// === Store mutations (only reached if all validation passes) ===

// Update justified/finalized checkpoints if they have higher slots
let justified = (post_state.latest_justified.slot > store.latest_justified().slot)
.then_some(post_state.latest_justified);
Expand All @@ -381,25 +387,26 @@ pub fn on_block(
let aggregated_attestations = &block.body.attestations;
let attestation_signatures = &signed_block.signature.attestation_signatures;

// Process block body attestations.
// TODO: fail the block if an attestation is invalid. Right now we
// just log a warning.
for (att, proof) in aggregated_attestations
.iter()
.zip(attestation_signatures.iter())
{
let validator_ids = aggregation_bits_to_validator_indices(&att.aggregation_bits);

for validator_id in validator_ids {
// Update Proof Map - Store the proof so future block builders can reuse this aggregation
// Store the proof so future block builders can reuse this aggregation
store.insert_aggregated_payload(&att.data, validator_id, proof.clone());

// Update Fork Choice - Register the vote immediately (historical/on-chain)
// Register the vote immediately (historical/on-chain)
let attestation = Attestation {
validator_id,
data: att.data.clone(),
};
// TODO: validate attestations before processing
// Block body attestations can fail validation for legitimate reasons
// (e.g., referencing blocks from forks we haven't seen, or source
// exceeding target when the attester's justified checkpoint advanced
// past their chosen target). Invalid attestations are skipped rather
// than rejecting the block.
let _ = on_attestation(store, attestation, true)
.inspect(|_| metrics::inc_attestations_valid("block"))
.inspect_err(|err| {
Expand Down Expand Up @@ -430,8 +437,10 @@ pub fn on_block(
);
}

// Process proposer attestation (enters "new" stage, not "known")
// TODO: validate attestations before processing
// Process proposer attestation (enters "new" stage, not "known").
// Like block body attestations, this can fail validation for legitimate
// reasons (e.g., referencing the block being processed, or stale checkpoint
// data). Errors are logged rather than rejecting the block.
let _ = on_attestation(store, proposer_attestation, false)
.inspect(|_| metrics::inc_attestations_valid("gossip"))
.inspect_err(|err| {
Expand Down