Skip to content
Merged
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
11 changes: 6 additions & 5 deletions crates/blockchain/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1356,13 +1356,14 @@ fn reorg_depth(old_head: H256, new_head: H256, store: &Store) -> Option<u64> {
mod tests {
use super::*;
use ethlambda_types::{
attestation::{AggregatedAttestation, AggregationBits, AttestationData, XmssSignature},
attestation::{
AggregatedAttestation, AggregationBits, AttestationData, blank_xmss_signature,
},
block::{
AggregatedSignatureProof, AttestationSignatures, BlockBody, BlockSignatures,
SignedBlock,
},
checkpoint::Checkpoint,
signature::SIGNATURE_SIZE,
state::State,
};

Expand Down Expand Up @@ -1407,7 +1408,7 @@ mod tests {
},
signature: BlockSignatures {
attestation_signatures,
proposer_signature: XmssSignature::try_from(vec![0u8; SIGNATURE_SIZE]).unwrap(),
proposer_signature: blank_xmss_signature(),
},
};

Expand Down Expand Up @@ -1561,7 +1562,7 @@ mod tests {
message: block,
signature: BlockSignatures {
attestation_signatures: AttestationSignatures::try_from(attestation_sigs).unwrap(),
proposer_signature: XmssSignature::try_from(vec![0u8; SIGNATURE_SIZE]).unwrap(),
proposer_signature: blank_xmss_signature(),
},
};

Expand Down Expand Up @@ -1858,7 +1859,7 @@ mod tests {
},
signature: BlockSignatures {
attestation_signatures,
proposer_signature: XmssSignature::try_from(vec![0u8; SIGNATURE_SIZE]).unwrap(),
proposer_signature: blank_xmss_signature(),
},
};

Expand Down
6 changes: 2 additions & 4 deletions crates/common/test-fixtures/src/fork_choice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ use crate::{
AggregationBits, AttestationData, Block, BlockBody, Checkpoint, TestInfo, TestState,
deser_xmss_hex,
};
use ethlambda_types::attestation::XmssSignature;
use ethlambda_types::attestation::{XmssSignature, blank_xmss_signature};
use ethlambda_types::block::{
AggregatedSignatureProof, AttestationSignatures, BlockSignatures, SignedBlock,
};
use ethlambda_types::primitives::H256;
use ethlambda_types::signature::SIGNATURE_SIZE;
use serde::{Deserialize, Deserializer};
use std::collections::HashMap;
use std::path::Path;
Expand Down Expand Up @@ -171,8 +170,7 @@ impl BlockStepData {
SignedBlock {
message: block,
signature: BlockSignatures {
proposer_signature: XmssSignature::try_from(vec![0u8; SIGNATURE_SIZE])
.expect("zero-filled signature has the correct length"),
proposer_signature: blank_xmss_signature(),
attestation_signatures: AttestationSignatures::try_from(proofs)
.expect("attestation proofs within limit"),
},
Expand Down
65 changes: 64 additions & 1 deletion crates/common/types/src/attestation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,40 @@ pub struct SignedAttestation {
pub signature: XmssSignature,
}

/// XMSS signature as a fixed-length byte vector (3112 bytes).
/// XMSS signature as a fixed-length byte vector (`SIGNATURE_SIZE` bytes).
pub type XmssSignature = SszVector<u8, SIGNATURE_SIZE>;

/// SSZ offset (in bytes) of the `path` body inside an XMSS `Signature` container.
///
/// Layout: 4-byte path offset + 28-byte rho + 4-byte hashes offset = 36.
const SIGNATURE_PATH_OFFSET: u32 = 36;

/// SSZ offset (in bytes) of the `hashes` body inside an XMSS `Signature`.
///
/// `path` body is 4-byte siblings offset + LOG_LIFETIME (32) siblings × 32-byte
/// digest = 1028, starting at byte 36, so hashes start at 36 + 1028 = 1064.
const SIGNATURE_HASHES_OFFSET: u32 = 1064;

/// SSZ offset (in bytes) of the `siblings` list inside the `path` container.
const SIGNATURE_PATH_SIBLINGS_OFFSET: u32 = 4;

/// Build a placeholder XMSS signature that decodes as a structurally valid
/// leanSpec `Signature` container of all-zero hashes.
///
/// Used for genesis-style anchor blocks that were never proposed and therefore
/// have no real signature. Parent containers inline this as an opaque
/// `SIGNATURE_SIZE`-byte blob; consumers that decode the inner `Signature`
/// container see `path = HashTreeOpening { siblings = [0; 32] }`, `rho = 0`,
/// `hashes = [0; 46]`. Matches ream's `Signature::blank()` so the wire format
/// is byte-identical across clients.
pub fn blank_xmss_signature() -> XmssSignature {
let mut bytes = vec![0u8; SIGNATURE_SIZE];
bytes[..4].copy_from_slice(&SIGNATURE_PATH_OFFSET.to_le_bytes());
bytes[32..36].copy_from_slice(&SIGNATURE_HASHES_OFFSET.to_le_bytes());
bytes[36..40].copy_from_slice(&SIGNATURE_PATH_SIBLINGS_OFFSET.to_le_bytes());
XmssSignature::try_from(bytes).expect("size matches SIGNATURE_SIZE")
}

/// Aggregated attestation consisting of participation bits and message.
#[derive(Debug, Clone, Serialize, SszEncode, SszDecode, HashTreeRoot)]
pub struct AggregatedAttestation {
Expand Down Expand Up @@ -275,4 +306,36 @@ mod tests {
let b = bits(24, &[0, 1, 16]);
assert!(!bits_is_subset(&a, &b));
}

/// Guard the three SSZ offsets at fixed byte positions so a one-off in any
/// constant doesn't silently produce a blob that still has the right outer
/// length but decodes incorrectly at the inner `Signature` container level.
#[test]
fn blank_xmss_signature_has_expected_ssz_offsets() {
let sig = blank_xmss_signature();
let bytes: Vec<u8> = sig.into_iter().collect();

assert_eq!(bytes.len(), SIGNATURE_SIZE);
assert_eq!(
u32::from_le_bytes(bytes[0..4].try_into().unwrap()),
SIGNATURE_PATH_OFFSET,
);
assert_eq!(
u32::from_le_bytes(bytes[32..36].try_into().unwrap()),
SIGNATURE_HASHES_OFFSET,
);
assert_eq!(
u32::from_le_bytes(bytes[36..40].try_into().unwrap()),
SIGNATURE_PATH_SIBLINGS_OFFSET,
);

// Everything outside the three offset slots must be zero — the
// placeholder is "all-zero hashes" once the offsets locate them.
for (i, b) in bytes.iter().enumerate() {
let in_offset_slot = matches!(i, 0..4 | 32..36 | 36..40);
if !in_offset_slot {
assert_eq!(*b, 0, "non-offset byte at index {i} should be zero");
}
}
}
}
5 changes: 2 additions & 3 deletions crates/net/p2p/src/req_resp/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,8 @@ mod tests {
use super::*;
use ethlambda_storage::{ForkCheckpoints, backend::InMemoryBackend};
use ethlambda_types::{
attestation::XmssSignature,
attestation::blank_xmss_signature,
block::{Block, BlockBody, BlockSignatures},
signature::SIGNATURE_SIZE,
state::State,
};
use libssz_types::SszList;
Expand All @@ -419,7 +418,7 @@ mod tests {
},
signature: BlockSignatures {
attestation_signatures: SszList::new(),
proposer_signature: XmssSignature::try_from(vec![0u8; SIGNATURE_SIZE]).unwrap(),
proposer_signature: blank_xmss_signature(),
},
}
}
Expand Down
45 changes: 37 additions & 8 deletions crates/net/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,11 +494,10 @@ mod tests {
#[tokio::test]
async fn test_get_latest_finalized_block() {
use ethlambda_types::{
attestation::XmssSignature,
attestation::blank_xmss_signature,
block::{Block, BlockBody, BlockSignatures, SignedBlock},
checkpoint::Checkpoint,
primitives::{H256, HashTreeRoot as _},
signature::SIGNATURE_SIZE,
};
use libssz::SszEncode;

Expand All @@ -519,7 +518,7 @@ mod tests {
message: block,
signature: BlockSignatures {
attestation_signatures: Default::default(),
proposer_signature: XmssSignature::try_from(vec![0u8; SIGNATURE_SIZE]).unwrap(),
proposer_signature: blank_xmss_signature(),
},
};

Expand Down Expand Up @@ -559,14 +558,37 @@ mod tests {
}

#[tokio::test]
async fn test_get_latest_finalized_block_returns_404_when_absent() {
// Genesis-anchored store: init_store writes header + state but no
// BlockSignatures entry, so get_signed_block(genesis_root) returns None
// and the endpoint must report 404 rather than panic.
async fn test_get_latest_finalized_block_serves_genesis_with_placeholder_signature() {
use ethlambda_types::{
attestation::blank_xmss_signature,
block::{BlockSignatures, SignedBlock},
};
use libssz::SszEncode;

// Genesis-anchored store: `init_store` writes the header + state but no
// `BlockSignatures` row. `get_signed_block` synthesizes an empty
// `BlockSignatures` so peers can still receive the genesis block on
// BlocksByRoot; the HTTP endpoint stays consistent and returns 200
// rather than 404.
let state = create_test_state();
let backend = Arc::new(InMemoryBackend::new());
let store = Store::from_anchor_state(backend, state);

// The body the endpoint serves must round-trip to a `SignedBlock`
// matching the genesis header paired with the synthetic blank
// signatures — same shape `get_signed_block` builds in storage.
let genesis_block = store
.get_signed_block(&store.latest_finalized().root)
.expect("genesis served via get_signed_block");
let expected = SignedBlock {
message: genesis_block.message.clone(),
signature: BlockSignatures {
attestation_signatures: Default::default(),
proposer_signature: blank_xmss_signature(),
},
};
let expected_ssz = expected.to_ssz();

let app = build_api_router(store);

let response = app
Expand All @@ -579,6 +601,13 @@ mod tests {
.await
.unwrap();

assert_eq!(response.status(), StatusCode::NOT_FOUND);
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.headers().get(header::CONTENT_TYPE).unwrap(),
SSZ_CONTENT_TYPE
);

let body = response.into_body().collect().await.unwrap().to_bytes();
assert_eq!(body.as_ref(), expected_ssz.as_slice());
}
Comment on lines 560 to 612
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.

P2 New genesis test only asserts status code

test_get_latest_finalized_block_serves_genesis_with_placeholder_signature verifies StatusCode::OK but does not check the response body or Content-Type header. The adjacent test (test_get_latest_finalized_block) asserts both, and their symmetry is the best guard that the serialisation path actually works end-to-end for the genesis case. Without a body check, a regression that returns 200 with empty or malformed SSZ bytes would pass this test silently.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/net/rpc/src/lib.rs
Line: 560-584

Comment:
**New genesis test only asserts status code**

`test_get_latest_finalized_block_serves_genesis_with_placeholder_signature` verifies `StatusCode::OK` but does not check the response body or `Content-Type` header. The adjacent test (`test_get_latest_finalized_block`) asserts both, and their symmetry is the best guard that the serialisation path actually works end-to-end for the genesis case. Without a body check, a regression that returns 200 with empty or malformed SSZ bytes would pass this test silently.

How can I resolve this? If you propose a fix, please make it concise.

}
104 changes: 91 additions & 13 deletions crates/storage/src/store.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
use std::collections::{BTreeMap, HashMap, HashSet, VecDeque};
use std::sync::{Arc, LazyLock, Mutex};

/// The tree hash root of an empty block body.
///
/// Used to detect genesis/anchor blocks that have no attestations,
/// allowing us to skip storing empty bodies and reconstruct them on read.
static EMPTY_BODY_ROOT: LazyLock<H256> = LazyLock::new(|| BlockBody::default().hash_tree_root());

use crate::api::{StorageBackend, StorageWriteBatch, Table};

use ethlambda_types::{
attestation::{AttestationData, HashedAttestationData, bits_is_subset},
attestation::{AttestationData, HashedAttestationData, bits_is_subset, blank_xmss_signature},
block::{
AggregatedSignatureProof, Block, BlockBody, BlockHeader, BlockSignatures, SignedBlock,
AggregatedSignatureProof, AttestationSignatures, Block, BlockBody, BlockHeader,
BlockSignatures, SignedBlock,
},
checkpoint::Checkpoint,
primitives::{H256, HashTreeRoot as _},
Expand All @@ -36,6 +31,24 @@ pub enum GetForkchoiceStoreError {
},
}

/// The tree hash root of an empty block body.
///
/// Used to detect genesis/anchor blocks that have no attestations,
/// allowing us to skip storing empty bodies and reconstruct them on read.
static EMPTY_BODY_ROOT: LazyLock<H256> = LazyLock::new(|| BlockBody::default().hash_tree_root());

/// Build a placeholder `BlockSignatures` for blocks that were never signed.
///
/// Genesis-style anchor blocks have no proposer signature and no per-attestation
/// proofs (no attestations exist). `get_signed_block` returns this so peers can
/// still receive the block in BlocksByRoot responses.
fn empty_block_signatures() -> BlockSignatures {
BlockSignatures {
attestation_signatures: AttestationSignatures::default(),
proposer_signature: blank_xmss_signature(),
}
}

/// Checkpoints to update in the forkchoice store.
///
/// Used with `Store::update_checkpoints` to update head and optionally
Expand Down Expand Up @@ -990,15 +1003,21 @@ impl Store {

/// Get a signed block by combining header, body, and signatures.
///
/// Returns None if any of the components are not found.
/// Note: Genesis block has no entry in BlockSignatures table.
/// Returns None if the header or body (for non-empty bodies) is missing,
/// or if the signature row is missing for any block other than the
/// slot-0 anchor.
///
/// Signatures are absent for genesis-style anchor blocks (no proposer
/// ever signed them). To keep BlocksByRoot symmetric with the
/// fork-choice view for peers, synthesize empty `BlockSignatures` for
/// the slot-0 case only; for any other slot the missing-signature
/// state is treated as storage corruption and surfaces as `None`
/// rather than as a fabricated block.
pub fn get_signed_block(&self, root: &H256) -> Option<SignedBlock> {
let view = self.backend.begin_read().expect("read view");
let key = root.to_ssz();

let header_bytes = view.get(Table::BlockHeaders, &key).expect("get")?;
let sig_bytes = view.get(Table::BlockSignatures, &key).expect("get")?;

let header = BlockHeader::from_ssz_bytes(&header_bytes).expect("valid header");

// Use empty body if header indicates empty, otherwise fetch from DB
Expand All @@ -1009,8 +1028,19 @@ impl Store {
BlockBody::from_ssz_bytes(&body_bytes).expect("valid body")
};

let signature = match view.get(Table::BlockSignatures, &key).expect("get") {
Some(sig_bytes) => {
BlockSignatures::from_ssz_bytes(&sig_bytes).expect("valid signatures")
}
// Synthesis only covers the genesis-style anchor (slot 0). Any other
// missing-signature case is a storage corruption that should surface
// as `None` rather than fabricating a block whose `attestation_signatures`
// list is empty regardless of what the body actually carries.
None if header.slot == 0 => empty_block_signatures(),
None => return None,
};
Comment on lines +1031 to +1041
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.

P2 Fallback applies to any block missing signatures, not just genesis

The synthesized-signature path triggers whenever Table::BlockSignatures has no row for a given root — including non-genesis blocks whose signature write was skipped or failed silently. The doc comment scopes the intent to "genesis-style anchor blocks," but the code has no guard (e.g., header.slot == 0) to enforce that. A non-genesis block with a missing signature row would now be served to peers with a blank XMSS placeholder rather than being treated as absent, making the gap invisible to callers. In the current write path this can't happen, but the invariant is implicit rather than structural.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/src/store.rs
Line: 1005-1010

Comment:
**Fallback applies to any block missing signatures, not just genesis**

The synthesized-signature path triggers whenever `Table::BlockSignatures` has no row for a given root — including non-genesis blocks whose signature write was skipped or failed silently. The doc comment scopes the intent to "genesis-style anchor blocks," but the code has no guard (e.g., `header.slot == 0`) to enforce that. A non-genesis block with a missing signature row would now be served to peers with a blank XMSS placeholder rather than being treated as absent, making the gap invisible to callers. In the current write path this can't happen, but the invariant is implicit rather than structural.

How can I resolve this? If you propose a fix, please make it concise.


let block = Block::from_header_and_body(header, body);
let signature = BlockSignatures::from_ssz_bytes(&sig_bytes).expect("valid signatures");

Some(SignedBlock {
message: block,
Expand Down Expand Up @@ -2313,4 +2343,52 @@ mod tests {
assert_eq!(buf.total_signatures(), 2); // slot 2 (1) + slot 3 (1)
assert_eq!(buf.len(), 2);
}

/// `Store::from_anchor_state` writes the header but no `BlockSignatures`
/// row for the slot-0 anchor. `get_signed_block` must synthesize an empty
/// `BlockSignatures` so the genesis block can still be served on
/// BlocksByRoot / `/lean/v0/blocks/finalized`.
#[test]
fn get_signed_block_synthesizes_blank_signatures_for_genesis_anchor() {
let backend: Arc<dyn StorageBackend> = Arc::new(InMemoryBackend::new());
let store = Store::from_anchor_state(backend, State::from_genesis(0, vec![]));

let head_root = store.head();
let signed = store
.get_signed_block(&head_root)
.expect("genesis block must be retrievable with synthetic signatures");

assert_eq!(signed.message.slot, 0);
assert_eq!(signed.signature.proposer_signature, blank_xmss_signature());
assert_eq!(signed.signature.attestation_signatures.len(), 0);
}

/// The synthesis branch must be confined to the slot-0 anchor: a
/// non-genesis block whose `BlockSignatures` row is missing is treated
/// as storage corruption and surfaces as `None`, not a fabricated block.
#[test]
fn get_signed_block_returns_none_for_non_genesis_with_missing_signatures() {
let backend: Arc<dyn StorageBackend> = Arc::new(InMemoryBackend::new());

// Hand-insert a slot-1 header (and empty body, via `EMPTY_BODY_ROOT`)
// but skip the `BlockSignatures` row. This mimics the corruption case
// the guard is meant to catch, without going through the normal
// `insert_signed_block` write path which always writes all three rows.
let header = BlockHeader {
slot: 1,
proposer_index: 0,
parent_root: H256::ZERO,
state_root: H256::ZERO,
body_root: *EMPTY_BODY_ROOT,
};
let root = header.hash_tree_root();
let mut batch = backend.begin_write().expect("write batch");
batch
.put_batch(Table::BlockHeaders, vec![(root.to_ssz(), header.to_ssz())])
.expect("put header");
batch.commit().expect("commit");

let store = Store::from_anchor_state(backend, State::from_genesis(0, vec![]));
assert!(store.get_signed_block(&root).is_none());
}
}