Skip to content

Commit fd2cc63

Browse files
Use post-block checkpoints for proposer attestation source and target (#268)
## Motivation A bug was found during devnet3 testing in the Qlean client (C++ implementation) where **finality was lagging more than 3 slots behind head**. The root cause was identified through structured log analysis: the proposer's attestation was computed **before** the block was imported into fork choice, causing the attestation to carry stale `source` (justified) and `target` checkpoint values. ### The ordering bug across clients When a proposer builds a block containing attestations that advance justification/finalization, the state transition updates these checkpoints. The question is: does the proposer's own attestation reflect the **pre-import** or **post-import** checkpoint values? **Qlean (buggy ordering):** ``` produceBlockWithSignatures ├─ produceAttestation ← attestation BEFORE import │ ├─ getHead ← stale head │ └─ getLatestJustified ← stale justified └─ onBlock ← import updates head/justified ``` **Zeam (correct ordering):** ``` validator.maybeDoProposal ├─ chain.produceBlock │ ├─ forkChoice.onBlock ← import FIRST │ └─ forkChoice.updateHead ← head/justified updated └─ chain.constructAttestationData ├─ forkChoice.getHead ← fresh head └─ forkChoice.getLatestJustified ← fresh justified ``` **ethlambda (was buggy, same as Qlean):** ``` propose_block() ├─ produce_block_with_signatures() ← build block ├─ Attestation { source: store.latest_justified(), ... } ← stale! ├─ sign attestation ├─ assemble SignedBlockWithAttestation └─ process_block() → on_block() ← import AFTER attestation ``` ### Why we can't just reorder In ethlambda (and Qlean), the proposer attestation is embedded inside `SignedBlockWithAttestation` → `BlockWithAttestation`. It's part of the block's hash tree root and required for signature verification during import. So unlike Zeam's architecture (where block and attestation are separate), we **cannot** import the block first and then create the attestation — it's a chicken-and-egg problem. ## Description The fix exploits the fact that `build_block()` already runs the full state transition to compute `state_root`. The resulting `post_state` contains `latest_justified` and `latest_finalized` after processing the block's attestations — but these values were previously discarded. ### Changes 1. **`PostBlockCheckpoints` struct** — carries the two checkpoints (`justified` + `finalized`) from `build_block` back to the caller. 2. **`build_block` / `produce_block_with_signatures`** — return `PostBlockCheckpoints` alongside the block, extracted from the post-state transition result. No extra computation — just plumbing values that were already computed. 3. **`get_attestation_target_with_checkpoints`** — parameterized version of `get_attestation_target` that accepts justified/finalized checkpoints instead of reading from the store. The existing `get_attestation_target` becomes a thin wrapper that delegates with store values. 4. **`propose_block`** — uses `post_checkpoints.justified` as `source` and calls `get_attestation_target_with_checkpoints` with post-block checkpoints for `target`. ### Behavior - **When block has no attestations** (or attestations don't advance justification): `post_state.latest_justified == head_state.latest_justified`, so behavior is identical to before. No regression. - **When block attestations advance justification**: proposer's attestation now correctly reflects the post-import justified checkpoint, preventing finality lag. ## How to Test ```bash make fmt # passes make lint # passes make test # all 97 tests pass ``` Full devnet validation is recommended to verify finality behavior under load. --------- Co-authored-by: Tomás Grüner <47506558+MegaRedHand@users.noreply.github.com>
1 parent 65c2907 commit fd2cc63

2 files changed

Lines changed: 59 additions & 16 deletions

File tree

crates/blockchain/src/lib.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,14 +213,16 @@ impl BlockChainServer {
213213
info!(%slot, %validator_id, "We are the proposer for this slot");
214214

215215
// Build the block with attestation signatures
216-
let Ok((block, attestation_signatures)) =
216+
let Ok((block, attestation_signatures, post_checkpoints)) =
217217
store::produce_block_with_signatures(&mut self.store, slot, validator_id)
218218
.inspect_err(|err| error!(%slot, %validator_id, %err, "Failed to build block"))
219219
else {
220220
return;
221221
};
222222

223-
// Create proposer's attestation (attests to the new block)
223+
// Create proposer's attestation using post-block checkpoints because
224+
// the block's attestations may have advanced justification/finalization
225+
// but the block hasn't been imported into the store yet.
224226
let proposer_attestation = Attestation {
225227
validator_id,
226228
data: AttestationData {
@@ -229,8 +231,12 @@ impl BlockChainServer {
229231
root: block.hash_tree_root(),
230232
slot: block.slot,
231233
},
232-
target: store::get_attestation_target(&self.store),
233-
source: self.store.latest_justified(),
234+
target: store::get_attestation_target_with_checkpoints(
235+
&self.store,
236+
post_checkpoints.justified,
237+
post_checkpoints.finalized,
238+
),
239+
source: post_checkpoints.justified,
234240
},
235241
};
236242

crates/blockchain/src/store.rs

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,17 @@ const JUSTIFICATION_LOOKBACK_SLOTS: u64 = 3;
3535
/// See: https://github.com/lambdaclass/ethlambda/issues/259
3636
const MAX_ATTESTATION_PROOF_BYTES: usize = 9 * 1024 * 1024;
3737

38+
/// Post-block checkpoints extracted from the state transition in `build_block`.
39+
///
40+
/// When building a block, the state transition processes attestations that may
41+
/// advance justification/finalization. These checkpoints reflect the post-state
42+
/// values, which the proposer needs for its attestation (since the block hasn't
43+
/// been imported into the store yet).
44+
pub struct PostBlockCheckpoints {
45+
pub justified: Checkpoint,
46+
pub finalized: Checkpoint,
47+
}
48+
3849
/// Accept new aggregated payloads, promoting them to known for fork choice.
3950
fn accept_new_attestations(store: &mut Store, log_tree: bool) {
4051
store.promote_new_aggregated_payloads();
@@ -621,6 +632,28 @@ fn on_block_core(
621632
///
622633
/// NOTE: this assumes that we have all the blocks from the head back to the latest finalized.
623634
pub fn get_attestation_target(store: &Store) -> Checkpoint {
635+
get_attestation_target_with_checkpoints(
636+
store,
637+
store.latest_justified(),
638+
store.latest_finalized(),
639+
)
640+
}
641+
642+
/// Calculate target checkpoint using the provided justified and finalized checkpoints
643+
/// instead of reading them from the store.
644+
///
645+
/// This is needed by the proposer, whose block hasn't been imported yet but whose
646+
/// state transition may have advanced justification/finalization.
647+
///
648+
/// Note: the walk-back still starts from `store.head()` (the pre-import head), not
649+
/// the new block. This is correct because the new block is only 1 slot ahead with less than 2/3 of votes — the
650+
/// walk-back immediately reaches the same chain. The important fix is using the
651+
/// post-state justified/finalized for the justifiability check and clamping guard.
652+
pub fn get_attestation_target_with_checkpoints(
653+
store: &Store,
654+
justified: Checkpoint,
655+
finalized: Checkpoint,
656+
) -> Checkpoint {
624657
// Start from current head
625658
let mut target_block_root = store.head();
626659
let mut target_header = store
@@ -647,7 +680,7 @@ pub fn get_attestation_target(store: &Store) -> Checkpoint {
647680
}
648681
}
649682

650-
let finalized_slot = store.latest_finalized().slot;
683+
let finalized_slot = finalized.slot;
651684

652685
// Ensure target is in justifiable slot range
653686
//
@@ -661,7 +694,7 @@ pub fn get_attestation_target(store: &Store) -> Checkpoint {
661694
.get_block_header(&target_block_root)
662695
.expect("parent block exists");
663696
}
664-
// Guard: clamp target to latest_justified (not in the spec).
697+
// Guard: clamp target to justified (not in the spec).
665698
//
666699
// The spec's walk-back has no lower bound, so it can produce attestations
667700
// where target.slot < source.slot (source = latest_justified). These would
@@ -673,14 +706,13 @@ pub fn get_attestation_target(store: &Store) -> Checkpoint {
673706
// justified checkpoint.
674707
//
675708
// See https://github.com/blockblaz/zeam/blob/697c293879e922942965cdb1da3c6044187ae00e/pkgs/node/src/forkchoice.zig#L654-L659
676-
let latest_justified = store.latest_justified();
677-
if target_header.slot < latest_justified.slot {
709+
if target_header.slot < justified.slot {
678710
warn!(
679711
target_slot = target_header.slot,
680-
justified_slot = latest_justified.slot,
712+
justified_slot = justified.slot,
681713
"Attestation target walked behind justified source, clamping to justified"
682714
);
683-
return latest_justified;
715+
return justified;
684716
}
685717

686718
Checkpoint {
@@ -757,7 +789,7 @@ pub fn produce_block_with_signatures(
757789
store: &mut Store,
758790
slot: u64,
759791
validator_index: u64,
760-
) -> Result<(Block, Vec<AggregatedSignatureProof>), StoreError> {
792+
) -> Result<(Block, Vec<AggregatedSignatureProof>, PostBlockCheckpoints), StoreError> {
761793
// Get parent block and state to build upon
762794
let head_root = get_proposal_head(store, slot);
763795
let head_state = store
@@ -782,7 +814,7 @@ pub fn produce_block_with_signatures(
782814

783815
let known_block_roots = store.get_block_roots();
784816

785-
let (block, signatures) = build_block(
817+
let (block, signatures, post_checkpoints) = build_block(
786818
&head_state,
787819
slot,
788820
validator_index,
@@ -791,7 +823,7 @@ pub fn produce_block_with_signatures(
791823
&aggregated_payloads,
792824
)?;
793825

794-
Ok((block, signatures))
826+
Ok((block, signatures, post_checkpoints))
795827
}
796828

797829
/// Errors that can occur during Store operations.
@@ -999,7 +1031,7 @@ fn build_block(
9991031
parent_root: H256,
10001032
known_block_roots: &HashSet<H256>,
10011033
aggregated_payloads: &HashMap<H256, (AttestationData, Vec<AggregatedSignatureProof>)>,
1002-
) -> Result<(Block, Vec<AggregatedSignatureProof>), StoreError> {
1034+
) -> Result<(Block, Vec<AggregatedSignatureProof>, PostBlockCheckpoints), StoreError> {
10031035
let mut aggregated_attestations: Vec<AggregatedAttestation> = Vec::new();
10041036
let mut aggregated_signatures: Vec<AggregatedSignatureProof> = Vec::new();
10051037
let mut accumulated_proof_bytes: usize = 0;
@@ -1099,7 +1131,12 @@ fn build_block(
10991131
process_block(&mut post_state, &final_block)?;
11001132
final_block.state_root = post_state.hash_tree_root();
11011133

1102-
Ok((final_block, aggregated_signatures))
1134+
let post_checkpoints = PostBlockCheckpoints {
1135+
justified: post_state.latest_justified,
1136+
finalized: post_state.latest_finalized,
1137+
};
1138+
1139+
Ok((final_block, aggregated_signatures, post_checkpoints))
11031140
}
11041141

11051142
/// Verify all signatures in a signed block.
@@ -1415,7 +1452,7 @@ mod tests {
14151452
}
14161453

14171454
// Build the block; this should succeed (the bug: no size guard)
1418-
let (block, signatures) = build_block(
1455+
let (block, signatures, _post_checkpoints) = build_block(
14191456
&head_state,
14201457
slot,
14211458
proposer_index,

0 commit comments

Comments
 (0)