Merge branch 'main' into devnet4

Author: pablodeymo

crates/blockchain/src/lib.rs

@@ -213,14 +213,16 @@ impl BlockChainServer {
         info!(%slot, %validator_id, "We are the proposer for this slot");
 
         // Build the block with attestation signatures
-        let Ok((block, attestation_signatures)) =
+        let Ok((block, attestation_signatures, post_checkpoints)) =
             store::produce_block_with_signatures(&mut self.store, slot, validator_id)
                 .inspect_err(|err| error!(%slot, %validator_id, %err, "Failed to build block"))
         else {
             return;
         };
 
-        // Create proposer's attestation (attests to the new block)
+        // Create proposer's attestation using post-block checkpoints because
+        // the block's attestations may have advanced justification/finalization
+        // but the block hasn't been imported into the store yet.
         let proposer_attestation = Attestation {
             validator_id,
             data: AttestationData {
@@ -229,8 +231,12 @@ impl BlockChainServer {
                     root: block.hash_tree_root(),
                     slot: block.slot,
                 },
-                target: store::get_attestation_target(&self.store),
-                source: self.store.latest_justified(),
+                target: store::get_attestation_target_with_checkpoints(
+                    &self.store,
+                    post_checkpoints.justified,
+                    post_checkpoints.finalized,
+                ),
+                source: post_checkpoints.justified,
             },
         };
 

crates/blockchain/src/store.rs

@@ -35,6 +35,17 @@ const JUSTIFICATION_LOOKBACK_SLOTS: u64 = 3;
 /// See: https://github.com/lambdaclass/ethlambda/issues/259
 const MAX_ATTESTATION_PROOF_BYTES: usize = 9 * 1024 * 1024;
 
+/// Post-block checkpoints extracted from the state transition in `build_block`.
+///
+/// When building a block, the state transition processes attestations that may
+/// advance justification/finalization. These checkpoints reflect the post-state
+/// values, which the proposer needs for its attestation (since the block hasn't
+/// been imported into the store yet).
+pub struct PostBlockCheckpoints {
+    pub justified: Checkpoint,
+    pub finalized: Checkpoint,
+}
+
 /// Accept new aggregated payloads, promoting them to known for fork choice.
 fn accept_new_attestations(store: &mut Store, log_tree: bool) {
     store.promote_new_aggregated_payloads();
@@ -633,6 +644,28 @@ fn on_block_core(
 ///
 /// NOTE: this assumes that we have all the blocks from the head back to the latest finalized.
 pub fn get_attestation_target(store: &Store) -> Checkpoint {
+    get_attestation_target_with_checkpoints(
+        store,
+        store.latest_justified(),
+        store.latest_finalized(),
+    )
+}
+
+/// Calculate target checkpoint using the provided justified and finalized checkpoints
+/// instead of reading them from the store.
+///
+/// This is needed by the proposer, whose block hasn't been imported yet but whose
+/// state transition may have advanced justification/finalization.
+///
+/// Note: the walk-back still starts from `store.head()` (the pre-import head), not
+/// the new block. This is correct because the new block is only 1 slot ahead with less than 2/3 of votes — the
+/// walk-back immediately reaches the same chain. The important fix is using the
+/// post-state justified/finalized for the justifiability check and clamping guard.
+pub fn get_attestation_target_with_checkpoints(
+    store: &Store,
+    justified: Checkpoint,
+    finalized: Checkpoint,
+) -> Checkpoint {
     // Start from current head
     let mut target_block_root = store.head();
     let mut target_header = store
@@ -659,7 +692,7 @@ pub fn get_attestation_target(store: &Store) -> Checkpoint {
         }
     }
 
-    let finalized_slot = store.latest_finalized().slot;
+    let finalized_slot = finalized.slot;
 
     // Ensure target is in justifiable slot range
     //
@@ -673,7 +706,7 @@ pub fn get_attestation_target(store: &Store) -> Checkpoint {
             .get_block_header(&target_block_root)
             .expect("parent block exists");
     }
-    // Guard: clamp target to latest_justified (not in the spec).
+    // Guard: clamp target to justified (not in the spec).
     //
     // The spec's walk-back has no lower bound, so it can produce attestations
     // where target.slot < source.slot (source = latest_justified). These would
@@ -685,14 +718,13 @@ pub fn get_attestation_target(store: &Store) -> Checkpoint {
     // justified checkpoint.
     //
     // See https://github.com/blockblaz/zeam/blob/697c293879e922942965cdb1da3c6044187ae00e/pkgs/node/src/forkchoice.zig#L654-L659
-    let latest_justified = store.latest_justified();
-    if target_header.slot < latest_justified.slot {
+    if target_header.slot < justified.slot {
         warn!(
             target_slot = target_header.slot,
-            justified_slot = latest_justified.slot,
+            justified_slot = justified.slot,
             "Attestation target walked behind justified source, clamping to justified"
         );
-        return latest_justified;
+        return justified;
     }
 
     Checkpoint {
@@ -769,7 +801,7 @@ pub fn produce_block_with_signatures(
     store: &mut Store,
     slot: u64,
     validator_index: u64,
-) -> Result<(Block, Vec<AggregatedSignatureProof>), StoreError> {
+) -> Result<(Block, Vec<AggregatedSignatureProof>, PostBlockCheckpoints), StoreError> {
     // Get parent block and state to build upon
     let head_root = get_proposal_head(store, slot);
     let head_state = store
@@ -794,7 +826,7 @@ pub fn produce_block_with_signatures(
 
     let known_block_roots = store.get_block_roots();
 
-    let (block, signatures) = build_block(
+    let (block, signatures, post_checkpoints) = build_block(
         &head_state,
         slot,
         validator_index,
@@ -803,7 +835,7 @@ pub fn produce_block_with_signatures(
         &aggregated_payloads,
     )?;
 
-    Ok((block, signatures))
+    Ok((block, signatures, post_checkpoints))
 }
 
 /// Errors that can occur during Store operations.
@@ -1102,7 +1134,7 @@ fn build_block(
     parent_root: H256,
     known_block_roots: &HashSet<H256>,
     aggregated_payloads: &HashMap<H256, (AttestationData, Vec<AggregatedSignatureProof>)>,
-) -> Result<(Block, Vec<AggregatedSignatureProof>), StoreError> {
+) -> Result<(Block, Vec<AggregatedSignatureProof>, PostBlockCheckpoints), StoreError> {
     let mut aggregated_attestations: Vec<AggregatedAttestation> = Vec::new();
     let mut aggregated_signatures: Vec<AggregatedSignatureProof> = Vec::new();
     let mut accumulated_proof_bytes: usize = 0;
@@ -1206,7 +1238,12 @@ fn build_block(
     process_block(&mut post_state, &final_block)?;
     final_block.state_root = post_state.hash_tree_root();
 
-    Ok((final_block, aggregated_signatures))
+    let post_checkpoints = PostBlockCheckpoints {
+        justified: post_state.latest_justified,
+        finalized: post_state.latest_finalized,
+    };
+
+    Ok((final_block, aggregated_signatures, post_checkpoints))
 }
 
 /// Verify all signatures in a signed block.
@@ -1522,7 +1559,7 @@ mod tests {
         }
 
         // Build the block; this should succeed (the bug: no size guard)
-        let (block, signatures) = build_block(
+        let (block, signatures, _post_checkpoints) = build_block(
             &head_state,
             slot,
             proposer_index,

crates/storage/src/store.rs

@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet, VecDeque};
+use std::collections::{BTreeMap, HashMap, HashSet, VecDeque};
 use std::sync::{Arc, LazyLock, Mutex};
 
 /// The tree hash root of an empty block body.
@@ -205,17 +205,15 @@ impl PayloadBuffer {
     }
 }
 
-/// Individual validator signature received via gossip.
-#[derive(Clone)]
-struct GossipSignatureEntry {
-    validator_id: u64,
-    signature: ValidatorSignature,
-}
-
 /// Gossip signatures grouped by attestation data.
+///
+/// Signatures are stored in a `BTreeMap` keyed by validator_id to guarantee
+/// ascending iteration order. XMSS aggregate proofs are order-dependent:
+/// verification reconstructs pubkeys from the participation bitfield (low-to-high),
+/// so aggregation must produce them in the same ascending order.
 struct GossipDataEntry {
     data: AttestationData,
-    signatures: Vec<GossipSignatureEntry>,
+    signatures: BTreeMap<u64, ValidatorSignature>,
 }
 
 /// Gossip signatures grouped by attestation data (via data_root).
@@ -260,8 +258,7 @@ pub struct Store {
     backend: Arc<dyn StorageBackend>,
     new_payloads: Arc<Mutex<PayloadBuffer>>,
     known_payloads: Arc<Mutex<PayloadBuffer>>,
-    /// In-memory gossip signatures: data_root → (AttestationData, Vec<GossipSignatureEntry>).
-    /// Transient data consumed at interval 2 aggregation.
+    /// In-memory gossip signatures, consumed at interval 2 aggregation.
     gossip_signatures: Arc<Mutex<GossipSignatureMap>>,
 }
 
@@ -947,7 +944,7 @@ impl Store {
     // Gossip signatures are individual validator signatures received via P2P.
     // They're transient (consumed at interval 2 aggregation) so stored in-memory.
     // Keyed by AttestationData (via data_root) matching the leanSpec structure:
-    //   gossip_signatures: dict[AttestationData, set[GossipSignatureEntry]]
+    //   gossip_signatures: dict[AttestationData, set[GossipSignature]]
 
     /// Delete gossip entries for the given (validator_id, data_root) pairs.
     pub fn delete_gossip_signatures(&mut self, keys: &[(u64, H256)]) {
@@ -957,7 +954,7 @@ impl Store {
         let mut gossip = self.gossip_signatures.lock().unwrap();
         for &(vid, data_root) in keys {
             if let Some(entry) = gossip.get_mut(&data_root) {
-                entry.signatures.retain(|e| e.validator_id != vid);
+                entry.signatures.remove(&vid);
                 if entry.signatures.is_empty() {
                     gossip.remove(&data_root);
                 }
@@ -974,7 +971,7 @@ impl Store {
                 let sigs: Vec<_> = entry
                     .signatures
                     .iter()
-                    .map(|e| (e.validator_id, e.signature.clone()))
+                    .map(|(&vid, sig)| (vid, sig.clone()))
                     .collect();
                 (HashedAttestationData::new(entry.data.clone()), sigs)
             })
@@ -992,19 +989,9 @@ impl Store {
         let mut gossip = self.gossip_signatures.lock().unwrap();
         let entry = gossip.entry(data_root).or_insert_with(|| GossipDataEntry {
             data: att_data,
-            signatures: Vec::new(),
+            signatures: BTreeMap::new(),
         });
-        // Avoid duplicates for same validator
-        if !entry
-            .signatures
-            .iter()
-            .any(|e| e.validator_id == validator_id)
-        {
-            entry.signatures.push(GossipSignatureEntry {
-                validator_id,
-                signature,
-            });
-        }
+        entry.signatures.entry(validator_id).or_insert(signature);
     }
 
     // ============ Derived Accessors ============