From c84282437d9da57366e290847302d3340822d1e8 Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 10:37:19 +0100 Subject: [PATCH 01/11] check block size --- crates/beacon_state/tile/src/tile/block.rs | 25 +++++++--- crates/beacon_state/tile/src/tile/tests.rs | 58 ++++++++++++++++++++-- crates/columns/src/tile.rs | 51 ++++++++++++++++--- 3 files changed, 114 insertions(+), 20 deletions(-) diff --git a/crates/beacon_state/tile/src/tile/block.rs b/crates/beacon_state/tile/src/tile/block.rs index 1d333f01..d4ddc63c 100644 --- a/crates/beacon_state/tile/src/tile/block.rs +++ b/crates/beacon_state/tile/src/tile/block.rs @@ -46,6 +46,11 @@ impl BeaconStateTile { producers: &mut Producers, mut send_gossip: impl FnMut(&mut Producers), ) -> Feedback { + if let Err(e) = Self::check_block_size(data) { + tracing::warn!(?source, "{e}"); + return e.feedback(); + } + let block_slot = SignedBeaconBlockView::slot(data); let parsed = match self.parse_and_verify_block(data, pre_verified) { Ok(parsed) => { @@ -427,14 +432,20 @@ impl BeaconStateTile { self.maybe_finalize(); } - fn precheck_block(&self, data: &[u8]) -> Result { - if !SignedBeaconBlockView::check_size(data) { - return Err(PrecheckError::SizeMismatch { - expected_min: ssz_view::SIGNED_BEACON_BLOCK_MIN, - expected_max: ssz_view::SIGNED_BEACON_BLOCK_MAX, - got: data.len(), - }); + fn check_block_size(data: &[u8]) -> Result<(), PrecheckError> { + if SignedBeaconBlockView::check_size(data) { + return Ok(()); } + Err(PrecheckError::SizeMismatch { + expected_min: ssz_view::SIGNED_BEACON_BLOCK_MIN, + expected_max: ssz_view::SIGNED_BEACON_BLOCK_MAX, + got: data.len(), + }) + } + + fn precheck_block(&self, data: &[u8]) -> Result { + Self::check_block_size(data)?; + let block_slot = SignedBeaconBlockView::slot(data); let block_epoch = block_slot / SLOTS_PER_EPOCH; let finalized_epoch = self.fork_choice.finalized_checkpoint.epoch; diff --git a/crates/beacon_state/tile/src/tile/tests.rs b/crates/beacon_state/tile/src/tile/tests.rs index fca16c4d..c511cc99 100644 --- a/crates/beacon_state/tile/src/tile/tests.rs +++ b/crates/beacon_state/tile/src/tile/tests.rs @@ -8,15 +8,16 @@ use silver_beacon_state_data::{ StateReadView, ValSeed, Withdrawals, }; use silver_common::{ - GossipTopic, MessageId, P2pStreamId, StreamProtocol, TCache, TCacheProducer, TProducer, + GossipTopic, MessageId, P2pStreamId, StreamProtocol, TCache, TCacheProducer, TCacheRead, + TProducer, ssz_view::{ ATTESTATION_DATA_SIZE, AttestationView, PROPOSER_SLASHING_SIZE, SIGNED_AGG_PROOF_MIN, - SIGNED_BLS_CHANGE_SIZE, SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, SIGNED_VOLUNTARY_EXIT_SIZE, - SINGLE_ATT_SIZE, SignedAggregateAndProofView, SingleAttestationView, StatusView, - SyncCommitteeContributionView, + SIGNED_BEACON_BLOCK_MIN, SIGNED_BLS_CHANGE_SIZE, SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, + SIGNED_VOLUNTARY_EXIT_SIZE, SINGLE_ATT_SIZE, SignedAggregateAndProofView, + SingleAttestationView, StatusView, }, }; -use silver_ssz::ssz_view::EXECUTION_PAYLOAD_ENVELOPE_MIN; +use silver_ssz::ssz_view::{EXECUTION_PAYLOAD_ENVELOPE_MIN, SyncCommitteeContributionView}; use super::*; use crate::{ @@ -385,6 +386,53 @@ fn block_unknown_parent_rejected() { assert_eq!(tile.fork_choice.nodes.len(), nodes_before, "no node added"); } +/// A gossip block shorter than the fixed prefix must not reach any +/// `SignedBeaconBlockView` accessor: they slice compile-time offsets and +/// `unwrap`, and `release-prod` aborts on panic. +#[test] +fn short_gossip_block_rejected_before_any_field_read() { + let (mut tile, mut gp, _rp, _spine, mut adapter) = tile_with_producers(200); + seed_tile(&mut tile, 4, 10); + tile.sync_target = SyncUpdate::Following; + + for len in [0, 1, 100, 107, SIGNED_BEACON_BLOCK_MIN - 1] { + let (data, read) = publish_block_bytes(&mut gp, &vec![0u8; len]); + let feedback = tile.apply_block( + &data, + read, + BlockSource::Gossip, + false, + &mut adapter.producers, + |_| panic!("a malformed block must never be relayed"), + ); + assert!(matches!(feedback, Feedback::Reject(None)), "len {len}: {feedback:?}"); + } + + // Control: at the minimum length the gate lets the block through, so the + // assertions above are about the bound and not about a blanket reject. + let mut bytes = vec![0u8; SIGNED_BEACON_BLOCK_MIN]; + bytes[116] = 0xFF; // unknown parent_root + let (data, read) = publish_block_bytes(&mut gp, &bytes); + let feedback = + tile.apply_block(&data, read, BlockSource::Gossip, false, &mut adapter.producers, |_| { + panic!("an unimportable block must never be relayed") + }); + assert!(matches!(feedback, Feedback::RequestParent { .. }), "{feedback:?}"); +} + +/// The tile reads gossip blocks out of its own tcache, so a test buffer has +/// to be published there to be readable back as `apply_block` sees it. +fn publish_block_bytes(producer: &mut TProducer, bytes: &[u8]) -> (Vec, TCacheRead) { + let mut r = producer.reserve(bytes.len().max(1), true).expect("reserve"); + if let Ok(buf) = r.buffer() { + buf[..bytes.len()].copy_from_slice(bytes); + } + r.increment_offset(bytes.len()); + let read = r.read(); + producer.publish_head(); + (bytes.to_vec(), read) +} + // ── pending-block bounds ── #[test] diff --git a/crates/columns/src/tile.rs b/crates/columns/src/tile.rs index ab71f994..b1e8cebd 100644 --- a/crates/columns/src/tile.rs +++ b/crates/columns/src/tile.rs @@ -128,6 +128,8 @@ impl DataColumnsTile { } }; + debug_assert!(SignedBeaconBlockView::check_size(buffer)); + let slot = SignedBeaconBlockView::slot(buffer); if slot <= self.sync_state.data_availability_floor() { return None; @@ -377,11 +379,19 @@ impl DataColumnsTile { stream_id: P2pStreamId, producers: &mut SilverSpineProducers, ) { - let parent_root = t_read - .buffer() - .ok() - .filter(|(buf, _)| SignedBeaconBlockView::check_size(buf)) - .map(|(buf, _)| *SignedBeaconBlockView::parent_root(buf)); + let parent_root = match t_read.buffer() { + Ok((buf, _)) if SignedBeaconBlockView::check_size(buf) => { + *SignedBeaconBlockView::parent_root(buf) + } + Ok((buf, _)) => { + tracing::warn!(?stream_id, len = buf.len(), "malformed beacon block"); + return; + } + Err(e) => { + tracing::error!(?e, ?stream_id, "failed to read beacon block cache buffer"); + return; + } + }; let root = self.beacon_block(stream_id, t_read, producers); @@ -390,9 +400,7 @@ impl DataColumnsTile { { self.drain_pending_gloas_columns(block_root, producers); } - if let Some(parent_root) = parent_root { - self.drain_parent_pending_columns(parent_root, producers); - } + self.drain_parent_pending_columns(parent_root, producers); } #[timed] @@ -679,6 +687,7 @@ mod tests { use silver_beacon_state_data::BeaconStateOwner; use silver_common::{ EngineReq, P2pStreamId, StreamProtocol, TCache, TCacheProducer, TCacheRead, + ssz_view::SIGNED_BEACON_BLOCK_MIN, }; use tempfile::TempDir; @@ -842,6 +851,32 @@ mod tests { } } + /// `handle_beacon_block` is the gossip and RPC entry, so it owns the size + /// gate: every `SignedBeaconBlockView` accessor slices a compile-time + /// offset and `unwrap`s, and `release-prod` aborts on panic. + #[test] + fn short_block_stops_at_the_entry_gate() { + for len in [0, 1, 100, 107, SIGNED_BEACON_BLOCK_MIN - 1] { + let mut rig = Rig::new(CUSTODY_COLUMNS); + rig.tile.sync_state.set_sync_target(SyncUpdate::Following); + let (mut consumer, ssz) = produce_block(&vec![0u8; len], "short_block_cons"); + let read = consumer.acquire(ssz); + + rig.tile.handle_beacon_block( + read, + P2pStreamId::new(2, 2, StreamProtocol::GossipSub, true), + &mut rig.conn.producers, + ); + let out = rig.drain(); + + assert_eq!( + out.available + out.persisted + out.engine + out.missing.len(), + 0, + "len {len}: a malformed block says nothing" + ); + } + } + /// A block at or below the DA floor owes no columns, and coverage there is /// unobservable — so nothing is emitted at all. Per-block events no /// consumer can act on are pure spam on the live path. From be620ca9bd79333051f3aea959b0e052f273dfa3 Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 10:51:05 +0100 Subject: [PATCH 02/11] check gossip msgs sizes --- crates/common/src/error.rs | 1 + crates/common/src/gossip.rs | 40 +++++++++++++++++++++++++++++------- crates/gossip/src/message.rs | 3 +++ 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/crates/common/src/error.rs b/crates/common/src/error.rs index d61ac11b..938a7401 100644 --- a/crates/common/src/error.rs +++ b/crates/common/src/error.rs @@ -20,6 +20,7 @@ pub enum Error { BufferTooSmall, GossipFrameTooLarge, GossipPayloadTooLarge, + GossipPayloadTooSmall, ParseTopicError, InvalidSnappy, IoError(#[from] std::io::Error), diff --git a/crates/common/src/gossip.rs b/crates/common/src/gossip.rs index 9e928241..69dbf29f 100644 --- a/crates/common/src/gossip.rs +++ b/crates/common/src/gossip.rs @@ -3,13 +3,16 @@ use std::fmt; use crate::{ Error, ssz_view::{ - ATTESTER_SLASHING_MAX, AttesterSlashingView, DATA_COLUMN_SIDECAR_MAX, - DataColumnSidecarFuluView, LIGHT_CLIENT_FINALITY_UPDATE_MAX, - LIGHT_CLIENT_OPTIMISTIC_UPDATE_MAX, LightClientFinalityUpdateView, - LightClientOptimisticUpdateView, MAX_PAYLOAD_SIZE, PAYLOAD_ATTESTATION_MESSAGE_SIZE, - PROPOSER_SLASHING_SIZE, PayloadAttestationMessageView, ProposerSlashingView, - SIGNED_AGG_PROOF_MAX, SIGNED_BLS_CHANGE_SIZE, SIGNED_CONTRIBUTION_AND_PROOF_SIZE, - SIGNED_EXECUTION_PAYLOAD_BID_MAX, SIGNED_PROPOSER_PREFERENCES_SIZE, + ATTESTER_SLASHING_MAX, ATTESTER_SLASHING_MIN, AttesterSlashingView, + DATA_COLUMN_SIDECAR_GLOAS_MIN, DATA_COLUMN_SIDECAR_MAX, DataColumnSidecarFuluView, + LIGHT_CLIENT_FINALITY_UPDATE_MAX, LIGHT_CLIENT_FINALITY_UPDATE_MIN, + LIGHT_CLIENT_OPTIMISTIC_UPDATE_MAX, LIGHT_CLIENT_OPTIMISTIC_UPDATE_MIN, + LightClientFinalityUpdateView, LightClientOptimisticUpdateView, MAX_PAYLOAD_SIZE, + PAYLOAD_ATTESTATION_MESSAGE_SIZE, PROPOSER_SLASHING_SIZE, PayloadAttestationMessageView, + ProposerSlashingView, SIGNED_AGG_PROOF_MAX, SIGNED_AGG_PROOF_MIN, SIGNED_BEACON_BLOCK_MIN, + SIGNED_BLS_CHANGE_SIZE, SIGNED_CONTRIBUTION_AND_PROOF_SIZE, + SIGNED_EXECUTION_PAYLOAD_BID_MAX, SIGNED_EXECUTION_PAYLOAD_BID_MIN, + SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, SIGNED_PROPOSER_PREFERENCES_SIZE, SIGNED_VOLUNTARY_EXIT_SIZE, SINGLE_ATT_SIZE, SYNC_COMMITTEE_MSG_SIZE, SignedAggregateAndProofView, SignedBeaconBlockView, SignedBlsToExecutionChangeView, SignedContributionAndProofView, SignedExecutionPayloadBidView, @@ -181,6 +184,29 @@ impl GossipTopic { } } + pub fn min_uncompressed_size(self) -> usize { + match self { + Self::BeaconBlock => SIGNED_BEACON_BLOCK_MIN, + Self::ExecutionPayload => SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, + Self::BeaconAggregateAndProof => SIGNED_AGG_PROOF_MIN, + Self::BeaconAttestation(_) => SINGLE_ATT_SIZE, + Self::VoluntaryExit => SIGNED_VOLUNTARY_EXIT_SIZE, + Self::ProposerSlashing => PROPOSER_SLASHING_SIZE, + Self::AttesterSlashing => ATTESTER_SLASHING_MIN, + Self::SyncCommitteeContributionAndProof => SIGNED_CONTRIBUTION_AND_PROOF_SIZE, + Self::SyncCommittee(_) => SYNC_COMMITTEE_MSG_SIZE, + Self::LightClientFinalityUpdate => LIGHT_CLIENT_FINALITY_UPDATE_MIN, + Self::LightClientOptimisticUpdate => LIGHT_CLIENT_OPTIMISTIC_UPDATE_MIN, + Self::BlsToExecutionChange => SIGNED_BLS_CHANGE_SIZE, + // Both sidecar layouts share the topic, and the gloas one is + // shorter (its commitments live on the bid, not the sidecar). + Self::DataColumnSidecar(_) => DATA_COLUMN_SIDECAR_GLOAS_MIN, + Self::ExecutionPayloadBid => SIGNED_EXECUTION_PAYLOAD_BID_MIN, + Self::PayloadAttestationMessage => PAYLOAD_ATTESTATION_MESSAGE_SIZE, + Self::ProposerPreferences => SIGNED_PROPOSER_PREFERENCES_SIZE, + } + } + /// Parse the full wire topic `/eth2/{fork_digest_hex}/{name}/ssz_snappy`. /// Verifies the envelope and that the fork digest matches /// `fork_digest_hex`. diff --git a/crates/gossip/src/message.rs b/crates/gossip/src/message.rs index 804efce2..a660704f 100644 --- a/crates/gossip/src/message.rs +++ b/crates/gossip/src/message.rs @@ -205,6 +205,9 @@ fn read_message_length(msg: &[u8], gossip_topic: &GossipTopic) -> Result gossip_topic.max_uncompressed_size() { return Err(Error::GossipPayloadTooLarge); } + if len < gossip_topic.min_uncompressed_size() { + return Err(Error::GossipPayloadTooSmall); + } Ok(len) } From f880390193da0f39732bd456e5d5e26826345669 Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 11:07:21 +0100 Subject: [PATCH 03/11] verify msgs offsets in check_size --- crates/beacon_state/tile/src/tile/tests.rs | 36 +++++++++++++---- .../tile/tests/ssz_view_fixtures.rs | 39 +++++++++++++++++++ crates/ssz/src/ssz_view.rs | 37 ++++++++++++++---- crates/storage/src/store.rs | 16 +++----- 4 files changed, 102 insertions(+), 26 deletions(-) diff --git a/crates/beacon_state/tile/src/tile/tests.rs b/crates/beacon_state/tile/src/tile/tests.rs index c511cc99..470d7514 100644 --- a/crates/beacon_state/tile/src/tile/tests.rs +++ b/crates/beacon_state/tile/src/tile/tests.rs @@ -30,6 +30,26 @@ use crate::{ const MAX_EFFECTIVE_BALANCE: u64 = 32_000_000_000; const ANCHOR_ROOT: B256 = [0x01u8; 32]; +/// Zeroed `SignedBeaconBlock` with the offsets `SignedBeaconBlockView` pins: +/// message at 100, body at 184. Fields go in at their fixed offsets. +fn empty_block(len: usize) -> Vec { + assert!(len >= SIGNED_BEACON_BLOCK_MIN); + let mut bytes = vec![0u8; len]; + bytes[0..4].copy_from_slice(&100u32.to_le_bytes()); + bytes[180..184].copy_from_slice(&84u32.to_le_bytes()); + bytes +} + +/// Zeroed `SignedAggregateAndProof` with the three offsets +/// `SignedAggregateAndProofView` pins. +fn empty_aggregate() -> Vec { + let mut buf = vec![0u8; SIGNED_AGG_PROOF_MIN]; + buf[0..4].copy_from_slice(&100u32.to_le_bytes()); + buf[108..112].copy_from_slice(&108u32.to_le_bytes()); + buf[208..212].copy_from_slice(&236u32.to_le_bytes()); + buf +} + fn make_tile() -> BeaconStateTile { make_tile_at_wall_slot(1) } @@ -97,7 +117,7 @@ fn make_tile_with_gossip(wall_slot: u64) -> (BeaconStateTile, TProducer, TProduc /// Publish a minimal block (slot at offset 100) into `producer` and wrap it /// as a buffered gossip orphan whose slot the tile can read back. fn gossip_pending(producer: &mut TProducer, slot: u64) -> PendingBlock { - let mut bytes = vec![0u8; 200]; + let mut bytes = empty_block(200); bytes[100..108].copy_from_slice(&slot.to_le_bytes()); let mut r = producer.reserve(bytes.len(), true).expect("reserve"); if let Ok(buf) = r.buffer() { @@ -369,7 +389,7 @@ fn block_unknown_parent_rejected() { // Minimal SignedBeaconBlock: message at fixed offset 100 (4-byte // offset + 96-byte signature), parent_root @ 116 set to an unknown // root so precheck bails with ParentMissing before any state change. - let mut buf = vec![0u8; 200]; + let mut buf = empty_block(200); buf[100..108].copy_from_slice(&11u64.to_le_bytes()); // slot buf[108..116].copy_from_slice(&0u64.to_le_bytes()); // proposer_index buf[116] = 0xFF; // parent_root[0] @@ -410,7 +430,7 @@ fn short_gossip_block_rejected_before_any_field_read() { // Control: at the minimum length the gate lets the block through, so the // assertions above are about the bound and not about a blanket reject. - let mut bytes = vec![0u8; SIGNED_BEACON_BLOCK_MIN]; + let mut bytes = empty_block(SIGNED_BEACON_BLOCK_MIN); bytes[116] = 0xFF; // unknown parent_root let (data, read) = publish_block_bytes(&mut gp, &bytes); let feedback = @@ -495,7 +515,7 @@ fn buffer_orphan_idx( /// Signed block just well-formed enough to reach the parent lookup: the /// message's slot sits at [100..108) and its parent root at [116..148). fn rpc_block(producer: &mut TProducer, slot: u64, parent_root: B256) -> silver_common::TCacheRead { - let mut bytes = vec![0u8; 200]; + let mut bytes = empty_block(200); bytes[100..108].copy_from_slice(&slot.to_le_bytes()); bytes[116..148].copy_from_slice(&parent_root); let mut r = producer.reserve(bytes.len(), true).expect("reserve"); @@ -892,7 +912,7 @@ fn block_known_parent_bad_sig_rejected() { // Valid structure, zeroed BLS signature → precheck reaches and fails // signature verification, so no fork-choice node is added. - let mut buf = vec![0u8; 200]; + let mut buf = empty_block(200); buf[100..108].copy_from_slice(&11u64.to_le_bytes()); // slot buf[108..116].copy_from_slice(&0u64.to_le_bytes()); // proposer_index buf[116..148].copy_from_slice(&parent_root); // parent_root @@ -1705,7 +1725,7 @@ fn justified_balances_rebuilt_on_checkpoint_change_only() { fn agg_multi_committee_bits_rejected() { let mut tile = make_tile(); seed_tile(&mut tile, 4, 0); - let mut buf = vec![0u8; SIGNED_AGG_PROOF_MIN]; + let mut buf = empty_aggregate(); buf[436] = 0b0000_0011; // two committee bits assert_eq!(tile.handle_aggregate_and_proof(&buf), Feedback::Reject(None)); } @@ -1714,7 +1734,7 @@ fn agg_multi_committee_bits_rejected() { fn agg_unknown_block_root_ignored() { let mut tile = make_tile(); seed_tile(&mut tile, 4, 0); - let mut buf = vec![0u8; SIGNED_AGG_PROOF_MIN]; + let mut buf = empty_aggregate(); buf[436] = 0b0000_0001; // single committee bit buf[228] = 0xFF; // beacon_block_root not in fork choice assert_eq!(tile.handle_aggregate_and_proof(&buf), Feedback::Ignore); @@ -1857,7 +1877,7 @@ fn agg_slot_too_old_ignored() { fn agg_slot_too_future_ignored() { let mut tile = make_tile_at_wall_slot(0); seed_tile(&mut tile, 128, 0); - let mut buf = vec![0u8; SIGNED_AGG_PROOF_MIN]; + let mut buf = empty_aggregate(); buf[436] = 0b0000_0001; buf[212] = 5; // slot = 5 > wall (0) assert_eq!(tile.handle_aggregate_and_proof(&buf), Feedback::Ignore); diff --git a/crates/beacon_state/tile/tests/ssz_view_fixtures.rs b/crates/beacon_state/tile/tests/ssz_view_fixtures.rs index c2770393..2f3fbcdc 100644 --- a/crates/beacon_state/tile/tests/ssz_view_fixtures.rs +++ b/crates/beacon_state/tile/tests/ssz_view_fixtures.rs @@ -244,8 +244,20 @@ fn signed_beacon_block() { let m = &v["message"]; // Outer: offset to message (==100); body offset at [180..184) (== 84). + // `check_size` enforces both, so the fixture is what proves the + // enforced constants are the ones real encodings carry. + assert!(SignedBeaconBlockView::check_size(buf), "{}", _case.display()); assert_eq!(u32::from_le_bytes(buf[0..4].try_into().unwrap()), 100); assert_eq!(u32::from_le_bytes(buf[180..184].try_into().unwrap()), 84); + for off in [0, 180] { + let mut bad = bytes.clone(); + bad[off] = bad[off].wrapping_add(1); + assert!( + !SignedBeaconBlockView::check_size(&bad), + "{}: offset at {off} not enforced", + _case.display() + ); + } assert_eq!(*SignedBeaconBlockView::signature(buf), b96(&v["signature"])); assert_eq!(SignedBeaconBlockView::slot(buf), u(&m["slot"])); @@ -266,9 +278,20 @@ fn signed_aggregate_and_proof() { // Layout invariants: outer message offset 100, inner aggregate offset // 108 (rel. to 100), aggregation_bits offset 236 (rel. to 208). + // `check_size` enforces all three. + assert!(SignedAggregateAndProofView::check_size(buf), "{}", _case.display()); assert_eq!(u32::from_le_bytes(buf[0..4].try_into().unwrap()), 100); assert_eq!(u32::from_le_bytes(buf[108..112].try_into().unwrap()), 108); assert_eq!(u32::from_le_bytes(buf[208..212].try_into().unwrap()), 236); + for off in [0, 108, 208] { + let mut bad = bytes.clone(); + bad[off] = bad[off].wrapping_add(1); + assert!( + !SignedAggregateAndProofView::check_size(&bad), + "{}: offset at {off} not enforced", + _case.display() + ); + } assert_eq!(*SignedAggregateAndProofView::signature(buf), b96(&v["signature"])); assert_eq!(SignedAggregateAndProofView::aggregator_index(buf), u(&m["aggregator_index"])); @@ -302,11 +325,27 @@ fn attester_slashing() { // Layout invariants: att_1 offset 8; att_2 offset monotonic; // each IndexedAttestation's attesting_indices offset is 228. + // `check_size` enforces all three plus 8-byte index alignment. + assert!(AttesterSlashingView::check_size(buf), "{}", _case.display()); assert_eq!(u32::from_le_bytes(buf[0..4].try_into().unwrap()), 8); let att2_off = u32::from_le_bytes(buf[4..8].try_into().unwrap()) as usize; assert!(att2_off >= 8 + 228 && att2_off <= buf.len()); assert_eq!(u32::from_le_bytes(buf[8..12].try_into().unwrap()), 228); assert_eq!(u32::from_le_bytes(buf[att2_off..att2_off + 4].try_into().unwrap()), 228); + for off in [0, 8, att2_off] { + let mut bad = bytes.clone(); + bad[off] = bad[off].wrapping_add(1); + assert!( + !AttesterSlashingView::check_size(&bad), + "{}: offset at {off} not enforced", + _case.display() + ); + } + // A misaligned att_2 offset leaves att_1's index list holding a + // partial u64, which the `chunks_exact(8)` readers would drop. + let mut bad = bytes.clone(); + bad[4..8].copy_from_slice(&(att2_off as u32 + 1).to_le_bytes()); + assert!(!AttesterSlashingView::check_size(&bad), "{}", _case.display()); let a1 = &v["attestation_1"]; let a2 = &v["attestation_2"]; diff --git a/crates/ssz/src/ssz_view.rs b/crates/ssz/src/ssz_view.rs index b180799b..52d0544f 100644 --- a/crates/ssz/src/ssz_view.rs +++ b/crates/ssz/src/ssz_view.rs @@ -600,11 +600,14 @@ impl SignedBeaconBlockView { &buf[184..] } /// Validates `buf` is large enough for every accessor above to read - /// without panicking. Accessors only read compile-time fixed offsets - /// plus `&buf[184..]`, so a bare length bound suffices. + /// without panicking, and that the two offsets those accessors hardcode + /// carry their canonical values. #[inline] pub fn check_size(buf: &[u8]) -> bool { - buf.len() >= SIGNED_BEACON_BLOCK_MIN && buf.len() <= SIGNED_BEACON_BLOCK_MAX + buf.len() >= SIGNED_BEACON_BLOCK_MIN && + buf.len() <= SIGNED_BEACON_BLOCK_MAX && + u32_le(buf, 0) as usize == 100 && + u32_le(buf, 180) as usize == 84 } #[inline] @@ -842,11 +845,16 @@ impl SignedAggregateAndProofView { pub fn aggregate(buf: &[u8]) -> &[u8] { &buf[208..] } - /// All accessors read compile-time fixed offsets plus `&buf[444..]`; - /// a length bound suffices. + /// Bounds the length and pins the three nested offsets the accessors + /// hardcode. None of them reaches the attestation data root, so an + /// honestly signed aggregate stays verifiable with any of them rewritten. #[inline] pub fn check_size(buf: &[u8]) -> bool { - buf.len() >= SIGNED_AGG_PROOF_MIN && buf.len() <= SIGNED_AGG_PROOF_MAX + buf.len() >= SIGNED_AGG_PROOF_MIN && + buf.len() <= SIGNED_AGG_PROOF_MAX && + u32_le(buf, 0) as usize == 100 && + u32_le(buf, 108) as usize == 108 && + u32_le(buf, 208) as usize == 236 } } @@ -972,17 +980,30 @@ impl AttesterSlashingView { } /// Validates: /// - `buf.len()` within [MIN, MAX]; + /// - the outer att_1 offset and both inner `attesting_indices` offsets + /// carry their canonical values (8, 228, 228); /// - `att2_off` at least 236 (att_1 fixed ends there) so /// `&buf[236..att2_off]` is a valid slice; /// - `att2_off + 228 <= buf.len()` so att_2's fixed reads and the - /// `&buf[att2_off+228..]` tail are in-bounds. + /// `&buf[att2_off+228..]` tail are in-bounds; + /// - both `attesting_indices` regions hold whole `u64` elements, which + /// the `chunks_exact(8)` readers downstream would otherwise truncate + /// past silently. #[inline] pub fn check_size(buf: &[u8]) -> bool { if buf.len() < ATTESTER_SLASHING_MIN || buf.len() > ATTESTER_SLASHING_MAX { return false; } + if u32_le(buf, 0) as usize != 8 || u32_le(buf, 8) as usize != 228 { + return false; + } let off2 = u32_le(buf, 4) as usize; - off2 >= 236 && off2.saturating_add(228) <= buf.len() + if off2 < 236 || off2.saturating_add(228) > buf.len() { + return false; + } + (off2 - 236).is_multiple_of(8) && + (buf.len() - off2 - 228).is_multiple_of(8) && + u32_le(buf, off2) as usize == 228 } } diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 1588a029..205f92ac 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -1507,6 +1507,8 @@ mod tests { let parent_root = [0x42; 32]; let state_root = [0x24; 32]; let mut block = vec![0u8; 184]; + block[0..4].copy_from_slice(&100u32.to_le_bytes()); + block[180..184].copy_from_slice(&84u32.to_le_bytes()); block[100..108].copy_from_slice(&slot.to_le_bytes()); block[108..116].copy_from_slice(&7u64.to_le_bytes()); block[116..148].copy_from_slice(&parent_root); @@ -1682,6 +1684,8 @@ mod tests { fn blob_block(slot: u64, parent_root: [u8; 32], state_root: [u8; 32]) -> Vec { let (body_start, body_len) = (184usize, 404usize); let mut block = vec![0u8; body_start + body_len]; + block[0..4].copy_from_slice(&100u32.to_le_bytes()); + block[180..184].copy_from_slice(&84u32.to_le_bytes()); block[100..108].copy_from_slice(&slot.to_le_bytes()); block[108..116].copy_from_slice(&11u64.to_le_bytes()); block[116..148].copy_from_slice(&parent_root); @@ -1937,15 +1941,7 @@ mod tests { let slot = 96u64; let parent_root = [0x31; 32]; let state_root = [0x13; 32]; - let body_start = 184usize; - let body_len = 404usize; - let mut block = vec![0u8; body_start + body_len]; - block[100..108].copy_from_slice(&slot.to_le_bytes()); - block[108..116].copy_from_slice(&11u64.to_le_bytes()); - block[116..148].copy_from_slice(&parent_root); - block[148..180].copy_from_slice(&state_root); - block[body_start + 388..body_start + 392].copy_from_slice(&396u32.to_le_bytes()); - block[body_start + 392..body_start + 396].copy_from_slice(&404u32.to_le_bytes()); + let block = blob_block(slot, parent_root, state_root); // Block on disk, no columns — exactly the post-sync state. let dir = store.finalized_slot_dir(super::Payload::Block, slot); @@ -2010,7 +2006,7 @@ mod tests { // Stand the backlog up at the cap without touching the cursor. let cb = store.history.columns.as_mut().unwrap(); - let block = vec![0u8; 184 + 404]; + let block = blob_block(0, [0u8; 32], [0u8; 32]); for i in 0..super::io::MAX_OPEN_COLUMN_NEEDS { cb.seed_block([i as u8; 32], (i as u64) + 1, &block, 0b1, &super::test_spec(u64::MAX)); } From fa378ef359b6d101733a4cf2d0ca63234301a6a6 Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 11:48:26 +0100 Subject: [PATCH 04/11] more columns checks --- .../beacon_state/tile/tests/ef_fork_choice.rs | 14 ++- crates/columns/src/tile.rs | 74 ++++++++++-- crates/columns/src/validate.rs | 35 +++++- crates/common/src/column_util.rs | 111 +++++++++++++++--- crates/storage/src/store.rs | 2 +- crates/storage/src/store/backfill.rs | 40 ++++--- crates/storage/src/store/history.rs | 3 +- 7 files changed, 224 insertions(+), 55 deletions(-) diff --git a/crates/beacon_state/tile/tests/ef_fork_choice.rs b/crates/beacon_state/tile/tests/ef_fork_choice.rs index b89e8581..6c32c880 100644 --- a/crates/beacon_state/tile/tests/ef_fork_choice.rs +++ b/crates/beacon_state/tile/tests/ef_fork_choice.rs @@ -18,7 +18,8 @@ use std::path::{Path, PathBuf}; use ef_common::{case_file, ef_tile, iter_test_cases, parse_root, snappy_decode, spec_tests_dir}; use serde_yml::{Mapping, Value}; use silver_beacon_state::BeaconStateTile; -use silver_beacon_state_data::{BeaconState, SpecConfig}; +use silver_beacon_state_data::{BeaconState, SLOTS_PER_EPOCH, SpecConfig}; +use silver_common::ssz_view::DataColumnSidecarFuluView; fn fork_choice_dir(fork: &str, handler: &str) -> PathBuf { spec_tests_dir().join("tests").join("mainnet").join(fork).join("fork_choice").join(handler) @@ -37,19 +38,22 @@ fn known_skip(name: &str) -> Option<&'static str> { /// Spec `is_data_available`: run silver's real column-sidecar verification /// (shape + inclusion proof + KZG, from the storage tile) over the columns the /// step provides. Available iff non-empty and every column verifies. -fn columns_available(dir: &Path, cols: &[Value]) -> bool { +fn columns_available(dir: &Path, cols: &[Value], spec: &SpecConfig) -> bool { !cols.is_empty() && cols.iter().all(|c| { let sc = case_file(dir, c.as_str().unwrap()); - silver_common::column_util::verify_data_column_sidecar_fulu(&sc) && + let epoch = DataColumnSidecarFuluView::slot(&sc) / SLOTS_PER_EPOCH; + let max_blobs = spec.blob_params_at(epoch).max_blobs_per_block as usize; + silver_common::column_util::verify_data_column_sidecar_fulu(&sc, max_blobs) && silver_common::column_util::verify_data_column_sidecar_inclusion_proof(&sc) && silver_common::column_util::verify_data_column_sidecar_kzg_proofs_fulu(&sc) }) } fn run_case(name: &str, dir: &Path) { + let spec = SpecConfig::mainnet(); let anchor = snappy_decode(&dir.join("anchor_state.ssz_snappy")); - let state = BeaconState::decompose(&anchor, &SpecConfig::mainnet(), None) + let state = BeaconState::decompose(&anchor, &spec, None) .unwrap_or_else(|e| panic!("{name}: decompose anchor_state: {e}")); let genesis_time = state.immutable.genesis_time; let mut tile = ef_tile(state); @@ -68,7 +72,7 @@ fn run_case(name: &str, dir: &Path) { // peerdas: spec `is_data_available` gates import on the columns // verifying. An unavailable block stays out of fork choice, so // the head is unchanged — model the gate by not importing. - Some(cols) if !columns_available(dir, cols) => { + Some(cols) if !columns_available(dir, cols, &spec) => { assert!(!valid, "{name} step {si}: block {b} unavailable but valid"); } _ => { diff --git a/crates/columns/src/tile.rs b/crates/columns/src/tile.rs index b1e8cebd..50253ff2 100644 --- a/crates/columns/src/tile.rs +++ b/crates/columns/src/tile.rs @@ -100,8 +100,8 @@ impl DataColumnsTile { Duration::from_millis(spec.slot_duration_ms()) * SLOTS_PER_EPOCH as u32; Self { consumers, + validator: ColumnValidator::new(beacon_state, spec.clone(), epoch_duration), spec, - validator: ColumnValidator::new(beacon_state, epoch_duration), kzg_batch: KzgBatch::new(), tracker: ColumnTracker::new(custody_group_columns, epoch_duration), gloas_pending_columns: Wheel::new(epoch_duration), @@ -445,13 +445,15 @@ impl DataColumnsTile { p2p_peer: stream_id.peer(), severity: RpcSeverity::Fatal, }); - producers.produce(SyncNeed::Missing { - root: block_root, - slot, - kind: DataKind::Columns, - columns: bitmask, - origin: Origin::Live, - }); + if bitmask != 0 { + producers.produce(SyncNeed::Missing { + root: block_root, + slot, + kind: DataKind::Columns, + columns: bitmask, + origin: Origin::Live, + }); + } } } @@ -687,7 +689,7 @@ mod tests { use silver_beacon_state_data::BeaconStateOwner; use silver_common::{ EngineReq, P2pStreamId, StreamProtocol, TCache, TCacheProducer, TCacheRead, - ssz_view::SIGNED_BEACON_BLOCK_MIN, + ssz_view::{DATA_COLUMN_SIDECAR_MIN, NUMBER_OF_COLUMNS, SIGNED_BEACON_BLOCK_MIN}, }; use tempfile::TempDir; @@ -851,6 +853,60 @@ mod tests { } } + /// Fulu-layout sidecar with empty lists: enough for `SidecarLayout::of` + /// and the index read, which is all these cases need. + fn synth_fulu_sidecar(index: u64, slot: u64) -> Vec { + let mut buf = vec![0u8; DATA_COLUMN_SIDECAR_MIN]; + buf[0..8].copy_from_slice(&index.to_le_bytes()); + for off in [8usize, 12, 16] { + buf[off..off + 4].copy_from_slice(&(DATA_COLUMN_SIDECAR_MIN as u32).to_le_bytes()); + } + buf[20..28].copy_from_slice(&slot.to_le_bytes()); + buf + } + + fn feed_sidecar(rig: &mut Rig, bytes: &[u8], cache: &'static str) -> ColumnDisposition { + let (mut consumer, ssz) = produce_block(bytes, cache); + let read = consumer.acquire(ssz); + rig.tile.data_columns( + PendingColumn { + stream_id: P2pStreamId::new(2, 2, StreamProtocol::DataColumnSidecarsByRange, true), + sidecar: read, + gossip_subnet: None, + recv_ts: IngestionTime::now(), + }, + RelayMeta::None, + &mut rig.conn.producers, + ) + } + + /// `1u128 << index` is only defined below 128, and `release-prod` masks an + /// over-wide shift rather than trapping — so an out-of-range index used to + /// reject while naming a different, innocent column to re-request. + #[test] + fn out_of_range_column_index_names_no_column_to_refetch() { + for index in [NUMBER_OF_COLUMNS as u64, NUMBER_OF_COLUMNS as u64 + 3, u64::MAX] { + let mut rig = Rig::new(CUSTODY_COLUMNS); + let disposition = feed_sidecar(&mut rig, &synth_fulu_sidecar(index, 7), "oor_index"); + let out = rig.drain(); + + assert!( + matches!(disposition, ColumnDisposition::Rejected { bitmask: 0, .. }), + "index {index}: rejected with no column named" + ); + assert!(out.missing.is_empty(), "index {index}: nothing to re-own"); + } + + // Control: an in-range index does name its column, so the assertions + // above are about the bound and not about a blanket empty bitmask. + let mut rig = Rig::new(CUSTODY_COLUMNS); + let disposition = feed_sidecar(&mut rig, &synth_fulu_sidecar(3, 7), "in_range_index"); + assert!( + matches!(disposition, ColumnDisposition::Rejected { bitmask, .. } if bitmask == 1 << 3), + "an in-range index is re-owed" + ); + } + /// `handle_beacon_block` is the gossip and RPC entry, so it owns the size /// gate: every `SignedBeaconBlockView` accessor slices a compile-time /// offset and `unwrap`s, and `release-prod` aborts on panic. diff --git a/crates/columns/src/validate.rs b/crates/columns/src/validate.rs index c699f09e..22d8971a 100644 --- a/crates/columns/src/validate.rs +++ b/crates/columns/src/validate.rs @@ -1,11 +1,15 @@ -use std::time::{Duration, Instant}; +use std::{ + sync::Arc, + time::{Duration, Instant}, +}; use flux_profiler::timed; -use silver_beacon_state_data::{BeaconStateReader, SLOTS_PER_EPOCH}; +use silver_beacon_state_data::{BeaconStateReader, SLOTS_PER_EPOCH, SpecConfig}; use silver_common::{ IngestionTime, P2pStreamId, StreamProtocol, TRead, Wheel, column_util as util, ssz_view::{ - DataColumnSidecarFuluView, DataColumnSidecarGloasView, SidecarLayout, SignedBeaconBlockView, + DataColumnSidecarFuluView, DataColumnSidecarGloasView, NUMBER_OF_COLUMNS, SidecarLayout, + SignedBeaconBlockView, }, }; @@ -35,6 +39,7 @@ pub(crate) enum ColumnOutcome { /// every check but KZG". Owns the caches only validation consults. pub(crate) struct ColumnValidator { beacon_state: BeaconStateReader, + spec: Arc, // Gloas sidecars carry no commitments, so column KZG verifies against these. gloas_commitments: Wheel, 4>, // Roots of persisted blocks — parent-seen checks beyond the head fork. @@ -42,14 +47,24 @@ pub(crate) struct ColumnValidator { } impl ColumnValidator { - pub fn new(beacon_state: BeaconStateReader, epoch_duration: Duration) -> Self { + pub fn new( + beacon_state: BeaconStateReader, + spec: Arc, + epoch_duration: Duration, + ) -> Self { Self { beacon_state, + spec, gloas_commitments: Wheel::new(epoch_duration), persisted_block_roots: Wheel::new(epoch_duration), } } + /// EIP-7892 `blob_schedule` entry active at `slot`'s epoch. + fn max_blobs_at(&self, slot: u64) -> usize { + self.spec.blob_params_at(slot / SLOTS_PER_EPOCH).max_blobs_per_block as usize + } + pub fn note_persisted(&mut self, block_root: BlockRoot) { self.persisted_block_roots.insert(block_root, ()); } @@ -129,6 +144,10 @@ impl ColumnValidator { let block_root = util::block_root_from_sidecar(buffer); let column_index = DataColumnSidecarFuluView::index(buffer); + if column_index >= NUMBER_OF_COLUMNS as u64 { + tracing::warn!(?stream_id, column_index, "sidecar column index out of range"); + return ColumnOutcome::Reject { block_root, slot, bitmask: 0 }; + } let column_bitmask = 1u128 << column_index; if let Some(subnet) = gossip_subnet && @@ -143,7 +162,7 @@ impl ColumnValidator { return ColumnOutcome::AlreadyHeld { block_root, column_index, slot }; } - if !util::verify_data_column_sidecar_fulu(buffer) { + if !util::verify_data_column_sidecar_fulu(buffer, self.max_blobs_at(slot)) { tracing::warn!(?stream_id, "badly formed data column sidecar"); return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; } @@ -272,6 +291,10 @@ impl ColumnValidator { let block_root = *DataColumnSidecarGloasView::beacon_block_root(buffer); let column_index = DataColumnSidecarGloasView::index(buffer); + if column_index >= NUMBER_OF_COLUMNS as u64 { + tracing::warn!(?stream_id, column_index, "sidecar column index out of range"); + return ColumnOutcome::Reject { block_root, slot, bitmask: 0 }; + } let column_bitmask = 1u128 << column_index; if let Some(subnet) = gossip_subnet && @@ -288,7 +311,7 @@ impl ColumnValidator { return ColumnOutcome::Buffer { block_root }; }; - if !util::verify_data_column_sidecar_gloas(buffer, commitments) { + if !util::verify_data_column_sidecar_gloas(buffer, commitments, self.max_blobs_at(slot)) { tracing::warn!(?stream_id, "badly formed gloas data column sidecar"); return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; } diff --git a/crates/common/src/column_util.rs b/crates/common/src/column_util.rs index 5c28ad40..7fdebebb 100644 --- a/crates/common/src/column_util.rs +++ b/crates/common/src/column_util.rs @@ -86,7 +86,7 @@ pub fn block_root_from_sidecar(sidecar: &[u8]) -> B256 { ]) } -fn check_sidecar_shape(column: &[u8], commits: &[u8], proofs: &[u8]) -> bool { +fn check_sidecar_shape(column: &[u8], commits: &[u8], proofs: &[u8], max_blobs: usize) -> bool { if !column.len().is_multiple_of(BYTES_PER_CELL) || !commits.len().is_multiple_of(BYTES_PER_KZG_COMMITMENT) || !proofs.len().is_multiple_of(BYTES_PER_KZG_PROOF) @@ -96,10 +96,13 @@ fn check_sidecar_shape(column: &[u8], commits: &[u8], proofs: &[u8]) -> bool { let n_cells = column.len() / BYTES_PER_CELL; let n_commits = commits.len() / BYTES_PER_KZG_COMMITMENT; let n_proofs = proofs.len() / BYTES_PER_KZG_PROOF; - n_cells == n_commits && n_commits == n_proofs && n_commits <= MAX_BLOB_COMMITMENTS_PER_BLOCK + n_cells == n_commits && + n_commits == n_proofs && + n_commits >= 1 && + n_commits <= max_blobs.min(MAX_BLOB_COMMITMENTS_PER_BLOCK) } -pub fn verify_data_column_sidecar_fulu(sidecar: &[u8]) -> bool { +pub fn verify_data_column_sidecar_fulu(sidecar: &[u8], max_blobs: usize) -> bool { if !DataColumnSidecarFuluView::check_size(sidecar) { return false; } @@ -110,10 +113,15 @@ pub fn verify_data_column_sidecar_fulu(sidecar: &[u8]) -> bool { DataColumnSidecarFuluView::column(sidecar), DataColumnSidecarFuluView::kzg_commitments(sidecar), DataColumnSidecarFuluView::kzg_proofs(sidecar), + max_blobs, ) } -pub fn verify_data_column_sidecar_gloas(sidecar: &[u8], commitments: &[u8]) -> bool { +pub fn verify_data_column_sidecar_gloas( + sidecar: &[u8], + commitments: &[u8], + max_blobs: usize, +) -> bool { if !DataColumnSidecarGloasView::check_size(sidecar) { return false; } @@ -124,6 +132,7 @@ pub fn verify_data_column_sidecar_gloas(sidecar: &[u8], commitments: &[u8]) -> b DataColumnSidecarGloasView::column(sidecar), commitments, DataColumnSidecarGloasView::kzg_proofs(sidecar), + max_blobs, ) } @@ -403,6 +412,10 @@ pub fn push_data_column_sidecar_prefix( #[cfg(test)] mod tests { + /// Above every count these tests build, so a case that fails does so on + /// the property it names rather than on the schedule bound. + const MAX_BLOBS: usize = 128; + use super::*; #[test] @@ -436,33 +449,57 @@ mod tests { buf } + /// Every sidecar carries at least one blob; the active schedule is the + /// upper bound, not the SSZ list limit. #[test] fn verify_shape_accepts_synthetic_sidecar() { - for n in [0usize, 1, 2, 6, 72] { + for n in [1usize, 2, 6, 72] { let buf = synth_sidecar(0, n, n, n); - assert!(verify_data_column_sidecar_fulu(&buf), "n={n}"); + assert!(verify_data_column_sidecar_fulu(&buf, n), "n={n}"); } } + #[test] + fn verify_shape_rejects_zero_blob_sidecar() { + let buf = synth_sidecar(0, 0, 0, 0); + assert!(!verify_data_column_sidecar_fulu(&buf, MAX_BLOBS)); + assert!(!verify_data_column_sidecar_gloas(&synth_gloas_sidecar(0, 0, 0), &[], MAX_BLOBS)); + } + + /// The bound tracked is the epoch's `blob_schedule` entry, so the same + /// bytes are accepted under one schedule and rejected under a tighter one. + #[test] + fn verify_shape_tracks_the_active_blob_schedule() { + let buf = synth_sidecar(0, 7, 7, 7); + assert!(verify_data_column_sidecar_fulu(&buf, 7)); + assert!(verify_data_column_sidecar_fulu(&buf, 9)); + assert!(!verify_data_column_sidecar_fulu(&buf, 6)); + + let gloas = synth_gloas_sidecar(0, 7, 7); + let commits = vec![0u8; 7 * BYTES_PER_KZG_COMMITMENT]; + assert!(verify_data_column_sidecar_gloas(&gloas, &commits, 7)); + assert!(!verify_data_column_sidecar_gloas(&gloas, &commits, 6)); + } + #[test] fn verify_shape_rejects_out_of_range_index() { let mut buf = synth_sidecar(0, 1, 1, 1); buf[0..8].copy_from_slice(&(NUMBER_OF_COLUMNS as u64).to_le_bytes()); - assert!(!verify_data_column_sidecar_fulu(&buf)); + assert!(!verify_data_column_sidecar_fulu(&buf, MAX_BLOBS)); } #[test] fn verify_shape_rejects_length_mismatch() { // 2 cells but only 1 commitment + 1 proof — count mismatch. let buf = synth_sidecar(0, 2, 1, 1); - assert!(!verify_data_column_sidecar_fulu(&buf)); + assert!(!verify_data_column_sidecar_fulu(&buf, MAX_BLOBS)); } #[test] fn verify_shape_rejects_truncated_buffer() { let mut buf = synth_sidecar(0, 1, 1, 1); buf.truncate(DATA_COLUMN_SIDECAR_MIN - 1); - assert!(!verify_data_column_sidecar_fulu(&buf)); + assert!(!verify_data_column_sidecar_fulu(&buf, MAX_BLOBS)); } #[test] @@ -500,18 +537,29 @@ mod tests { fn gloas_sidecar_shape_checks() { let commits = |n: usize| vec![0u8; n * BYTES_PER_KZG_COMMITMENT]; // 1 cell / 1 commit / 1 proof — well-formed. - assert!(verify_data_column_sidecar_gloas(&synth_gloas_sidecar(0, 1, 1), &commits(1))); - // empty. - assert!(verify_data_column_sidecar_gloas(&synth_gloas_sidecar(0, 0, 0), &commits(0))); + assert!(verify_data_column_sidecar_gloas( + &synth_gloas_sidecar(0, 1, 1), + &commits(1), + MAX_BLOBS + )); // index out of range. assert!(!verify_data_column_sidecar_gloas( &synth_gloas_sidecar(NUMBER_OF_COLUMNS as u64, 1, 1), - &commits(1) + &commits(1), + MAX_BLOBS )); // cell/proof count mismatch. - assert!(!verify_data_column_sidecar_gloas(&synth_gloas_sidecar(0, 2, 1), &commits(2))); + assert!(!verify_data_column_sidecar_gloas( + &synth_gloas_sidecar(0, 2, 1), + &commits(2), + MAX_BLOBS + )); // commitment-count mismatch. - assert!(!verify_data_column_sidecar_gloas(&synth_gloas_sidecar(0, 1, 1), &commits(2))); + assert!(!verify_data_column_sidecar_gloas( + &synth_gloas_sidecar(0, 1, 1), + &commits(2), + MAX_BLOBS + )); } /// End-to-end gloas: real cells + proofs through the gloas sidecar layout, @@ -549,7 +597,7 @@ mod tests { buf.extend_from_slice(&column); buf.extend_from_slice(&col_proofs); - assert!(verify_data_column_sidecar_gloas(&buf, &commitments), "shape j={j}"); + assert!(verify_data_column_sidecar_gloas(&buf, &commitments, MAX_BLOBS), "shape j={j}"); assert!(verify_data_column_sidecar_kzg_proofs_gloas(&buf, &commitments), "kzg j={j}"); } } @@ -623,10 +671,39 @@ mod tests { out.extend_from_slice(&col_proofs); assert_eq!(out.len(), data_column_sidecar_len(n)); - assert!(verify_data_column_sidecar_fulu(&out), "shape, col {j}"); + assert!(verify_data_column_sidecar_fulu(&out, MAX_BLOBS), "shape, col {j}"); assert_eq!(DataColumnSidecarFuluView::index(&out), j); assert!(verify_data_column_sidecar_kzg_proofs_fulu(&out), "kzg, col {j}"); assert!(verify_data_column_sidecar_inclusion_proof(&out), "inclusion, col {j}"); } } + + /// The zero-blob sidecar needs no key: take any honest blobless block, and + /// an empty commitments list re-roots to its `body_root`, so the inclusion + /// proof verifies and the KZG batch passes on zero cells. The shape check + /// is the only thing standing between that and a recorded column. + #[test] + fn zero_blob_sidecar_off_a_blobless_block_is_refused_by_shape_alone() { + use silver_common::ssz_hash::kzg_commitments_inclusion_proof; + + let body = synth_body_with_commitments(&[]); + let mut header = [0u8; 208]; + header[80..112].copy_from_slice(&hash_tree_root_body_fulu(&body)); + + let mut out = Vec::with_capacity(data_column_sidecar_len(0)); + push_data_column_sidecar_prefix( + &mut out, + 0, + 0, + &header, + &kzg_commitments_inclusion_proof(&body), + ); + assert_eq!(out.len(), data_column_sidecar_len(0)); + + // Everything downstream of the shape check waves it through. + assert!(verify_data_column_sidecar_inclusion_proof(&out), "the proof really does verify"); + assert!(verify_data_column_sidecar_kzg_proofs_fulu(&out), "the empty batch really passes"); + + assert!(!verify_data_column_sidecar_fulu(&out, MAX_BLOBS)); + } } diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 205f92ac..e5d3d8f0 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -463,7 +463,7 @@ impl Store { ) where F: FnMut(PeerEvent), { - let (verified, rejected) = self.history.add_sidecar(sidecar, peer, now); + let (verified, rejected) = self.history.add_sidecar(sidecar, peer, now, &self.spec); for bad in rejected { tracing::warn!( peer = bad.peer, diff --git a/crates/storage/src/store/backfill.rs b/crates/storage/src/store/backfill.rs index 33613673..231266b9 100644 --- a/crates/storage/src/store/backfill.rs +++ b/crates/storage/src/store/backfill.rs @@ -6,7 +6,7 @@ use std::{ }; use fxhash::FxHashMap; -use silver_beacon_state_data::SpecConfig; +use silver_beacon_state_data::{SLOTS_PER_EPOCH, SpecConfig}; use silver_common::{ TRead, column_util::{self, KzgBatchEntry, KzgScratch}, @@ -172,13 +172,15 @@ impl Expect { } impl PendingColumnBlock { - fn accepts(&self, layout: SidecarLayout, sidecar: &[u8]) -> bool { + fn accepts(&self, layout: SidecarLayout, sidecar: &[u8], spec: &SpecConfig) -> bool { + let max_blobs = + spec.blob_params_at(self.slot / SLOTS_PER_EPOCH).max_blobs_per_block as usize; match (&self.expect, layout) { ( Expect::Fulu { proposer_index, parent_root, state_root, body_root, signature }, SidecarLayout::Fulu, ) => { - column_util::verify_data_column_sidecar_fulu(sidecar) && + column_util::verify_data_column_sidecar_fulu(sidecar, max_blobs) && column_util::verify_data_column_sidecar_inclusion_proof(sidecar) && DataColumnSidecarFuluView::slot(sidecar) == self.slot && DataColumnSidecarFuluView::proposer_index(sidecar) == *proposer_index && @@ -188,7 +190,7 @@ impl PendingColumnBlock { DataColumnSidecarFuluView::block_signature(sidecar) == signature } (Expect::Gloas { commitments }, SidecarLayout::Gloas) => { - column_util::verify_data_column_sidecar_gloas(sidecar, commitments) && + column_util::verify_data_column_sidecar_gloas(sidecar, commitments, max_blobs) && DataColumnSidecarGloasView::slot(sidecar) == self.slot } _ => { @@ -303,8 +305,9 @@ impl ColumnBackfill { sidecar: TRead, peer: usize, now: Instant, + spec: &SpecConfig, ) -> (Option, Vec) { - let Some((block_root, column_index)) = self.park(&sidecar, peer, now) else { + let Some((block_root, column_index)) = self.park(&sidecar, peer, now, spec) else { return (None, Vec::new()); }; let Some(block) = self.pending.get_mut(&block_root) else { return (None, Vec::new()) }; @@ -317,7 +320,13 @@ impl ColumnBackfill { /// The per-sidecar checks: shape, header/commitment binding to a pending /// block, and dedup. `Some` = accepted for parking. - fn park(&mut self, sidecar: &TRead, peer: usize, now: Instant) -> Option<(B256, u64)> { + fn park( + &mut self, + sidecar: &TRead, + peer: usize, + now: Instant, + spec: &SpecConfig, + ) -> Option<(B256, u64)> { let buffer = match sidecar.buffer() { Ok((buffer, _)) => buffer, Err(e) => { @@ -361,7 +370,7 @@ impl ColumnBackfill { if expected.requested & column_bitmask == 0 || expected.received & column_bitmask != 0 { return None; } - if !expected.accepts(layout, buffer) { + if !expected.accepts(layout, buffer, spec) { tracing::warn!( block_root = hex::encode(block_root), column_index, @@ -591,7 +600,6 @@ impl PendingEnvelope { mod tests { use std::io::Write; - use silver_beacon_state_data::SLOTS_PER_EPOCH; use silver_common::{TCache, TCacheProducer}; use super::*; @@ -726,8 +734,8 @@ mod tests { }; // Contents are irrelevant: the layout arm is what refuses. - assert!(!fulu.accepts(SidecarLayout::Gloas, &[0u8; 512])); - assert!(!gloas.accepts(SidecarLayout::Fulu, &[0u8; 512])); + assert!(!fulu.accepts(SidecarLayout::Gloas, &[0u8; 512], &spec())); + assert!(!gloas.accepts(SidecarLayout::Fulu, &[0u8; 512], &spec())); } /// A gloas-era block links only if its root is computed with the gloas body @@ -874,12 +882,12 @@ mod tests { cb.seed_block(f.block_root, 20, &f.block, requested, &spec()); for &col in &[0usize, 1] { - let (verified, rejected) = cb.add_sidecar(tc.tread(&f.sidecars[col]), 7, now); + let (verified, rejected) = cb.add_sidecar(tc.tread(&f.sidecars[col]), 7, now, &spec()); assert!(verified.is_none() && rejected.is_empty(), "col {col} parked, not verified"); } assert_eq!(cb.owed_span(), (20, 21), "still owed until the set completes"); - let (verified, rejected) = cb.add_sidecar(tc.tread(&f.sidecars[3]), 7, now); + let (verified, rejected) = cb.add_sidecar(tc.tread(&f.sidecars[3]), 7, now, &spec()); assert!(rejected.is_empty()); let verified = verified.expect("last column completes the set"); assert_eq!((verified.block_root, verified.slot), (f.block_root, 20)); @@ -903,8 +911,8 @@ mod tests { let proofs_off = column_util::data_column_sidecar_len(2) - 2 * 48; forged[proofs_off] ^= 0x01; - cb.add_sidecar(tc.tread(&f.sidecars[0]), 7, now); - let (verified, rejected) = cb.add_sidecar(tc.tread(&forged), 9, now); + cb.add_sidecar(tc.tread(&f.sidecars[0]), 7, now, &spec()); + let (verified, rejected) = cb.add_sidecar(tc.tread(&forged), 9, now, &spec()); assert!(verified.is_none(), "a bad column holds the set back"); assert_eq!(rejected.len(), 1); assert_eq!((rejected[0].column_index, rejected[0].peer), (1, 9)); @@ -914,7 +922,7 @@ mod tests { assert_eq!(block.parked.len(), 1, "the honest column stays parked"); // The re-ask lands a good copy: the set completes. - let (verified, rejected) = cb.add_sidecar(tc.tread(&f.sidecars[1]), 11, now); + let (verified, rejected) = cb.add_sidecar(tc.tread(&f.sidecars[1]), 11, now, &spec()); assert!(rejected.is_empty()); assert_eq!(verified.expect("completes").sidecars.len(), 2); } @@ -928,7 +936,7 @@ mod tests { let mut tc = Tc::new("backfill_kzg_expire"); let mut cb = ColumnBackfill::new(1..97); cb.seed_block(f.block_root, 20, &f.block, 0b11, &spec()); - cb.add_sidecar(tc.tread(&f.sidecars[0]), 7, now); + cb.add_sidecar(tc.tread(&f.sidecars[0]), 7, now, &spec()); cb.expire_incomplete(now + INCOMPLETE_BLOCK_TIMEOUT - Duration::from_millis(1)); assert_eq!(cb.pending[&f.block_root].parked.len(), 1, "inside the window it is kept"); diff --git a/crates/storage/src/store/history.rs b/crates/storage/src/store/history.rs index a8e2ca7d..a66ba51e 100644 --- a/crates/storage/src/store/history.rs +++ b/crates/storage/src/store/history.rs @@ -202,9 +202,10 @@ impl HistoryBackfill { sidecar: TRead, peer: usize, now: Instant, + spec: &SpecConfig, ) -> (Option, Vec) { match self.columns.as_mut() { - Some(columns) => columns.add_sidecar(sidecar, peer, now), + Some(columns) => columns.add_sidecar(sidecar, peer, now, spec), None => { tracing::error!("received backfill data column with no active column backfill!"); (None, Vec::new()) From b7ced10e60a9fb9c6fb032ea7597cb2bcdfa324d Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 11:55:28 +0100 Subject: [PATCH 05/11] column sidecar parent block check --- crates/columns/src/tile.rs | 2 +- crates/columns/src/validate.rs | 53 ++++++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/crates/columns/src/tile.rs b/crates/columns/src/tile.rs index 50253ff2..8854b015 100644 --- a/crates/columns/src/tile.rs +++ b/crates/columns/src/tile.rs @@ -566,7 +566,7 @@ impl DataColumnsTile { let slot = SignedBeaconBlockView::slot(buf); let block_root = util::block_root(buf, self.spec.is_gloas_at_slot(slot)); - self.validator.note_persisted(block_root); + self.validator.note_persisted(block_root, slot); self.drain_parent_pending_columns(block_root, producers); } diff --git a/crates/columns/src/validate.rs b/crates/columns/src/validate.rs index 22d8971a..67630454 100644 --- a/crates/columns/src/validate.rs +++ b/crates/columns/src/validate.rs @@ -34,6 +34,13 @@ pub(crate) enum ColumnOutcome { Record { block_root: BlockRoot, column_index: u64, bitmask: u128, slot: u64 }, } +#[derive(Debug, PartialEq, Eq)] +enum ParentCheck { + Seen, + Unseen, + NotExtending { parent_slot: u64 }, +} + /// Per-sidecar validation, i.e. everything except the KZG cell proofs — /// those are deferred to the end-of-pass batch, so `Record` means "passed /// every check but KZG". Owns the caches only validation consults. @@ -42,8 +49,9 @@ pub(crate) struct ColumnValidator { spec: Arc, // Gloas sidecars carry no commitments, so column KZG verifies against these. gloas_commitments: Wheel, 4>, - // Roots of persisted blocks — parent-seen checks beyond the head fork. - persisted_block_roots: Wheel, + // Persisted blocks and the slot each sits at — parent-seen and + // parent-slot checks beyond the head fork. + persisted_block_roots: Wheel, } impl ColumnValidator { @@ -65,8 +73,18 @@ impl ColumnValidator { self.spec.blob_params_at(slot / SLOTS_PER_EPOCH).max_blobs_per_block as usize } - pub fn note_persisted(&mut self, block_root: BlockRoot) { - self.persisted_block_roots.insert(block_root, ()); + pub fn note_persisted(&mut self, block_root: BlockRoot, slot: u64) { + self.persisted_block_roots.insert(block_root, slot); + } + + /// Fulu requires a sidecar to be proposed strictly after its parent block. + fn check_parent(&self, parent_root: &BlockRoot, slot: u64, in_head_fork: bool) -> ParentCheck { + match self.persisted_block_roots.get(parent_root) { + Some(&parent_slot) if slot <= parent_slot => ParentCheck::NotExtending { parent_slot }, + Some(_) => ParentCheck::Seen, + None if in_head_fork => ParentCheck::Seen, + None => ParentCheck::Unseen, + } } pub fn gloas_commitments(&self, block_root: &BlockRoot) -> Option<&[u8]> { @@ -227,18 +245,21 @@ impl ColumnValidator { tracing::warn!(?stream_id, "sidecar slot at or below finalized — ignoring"); return ColumnOutcome::Skip; } - // The state view sees only the head fork's chain; the store holds - // every BS-accepted block (all forks, incl. validated children of the - // head that aren't head yet). - let parent_validated = parent_validated || self.persisted_block_roots.contains(parent_root); - if !parent_validated { - tracing::warn!( - ?stream_id, - slot, - parent_root = hex::encode(parent_root), - "sidecar parent_root not yet validated — ignoring (not penalized)" - ); - return ColumnOutcome::AwaitParent { parent_root: *parent_root }; + match self.check_parent(parent_root, slot, parent_validated) { + ParentCheck::Seen => {} + ParentCheck::Unseen => { + tracing::warn!( + ?stream_id, + slot, + parent_root = hex::encode(parent_root), + "sidecar parent_root not yet validated — ignoring (not penalized)" + ); + return ColumnOutcome::AwaitParent { parent_root: *parent_root }; + } + ParentCheck::NotExtending { parent_slot } => { + tracing::warn!(?stream_id, slot, parent_slot, "sidecar does not extend its parent"); + return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; + } } if !proposer_matches { tracing::warn!(?stream_id, "sidecar proposer_index mismatch"); From 7e47252c8495aa9f1c34175ab3a80087fcac95dc Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 12:03:37 +0100 Subject: [PATCH 06/11] rpc column checks --- crates/columns/src/tile.rs | 56 ++++++++++++++++- crates/columns/src/validate.rs | 112 ++++++++++++++++++++++----------- 2 files changed, 129 insertions(+), 39 deletions(-) diff --git a/crates/columns/src/tile.rs b/crates/columns/src/tile.rs index 8854b015..c15fa366 100644 --- a/crates/columns/src/tile.rs +++ b/crates/columns/src/tile.rs @@ -259,7 +259,7 @@ impl DataColumnsTile { } ColumnDisposition::Ignored } - ColumnOutcome::Record { block_root, column_index, bitmask, slot } => { + ColumnOutcome::Record { block_root, column_index, bitmask, slot, relay_eligible } => { let queued = self.kzg_batch.push(PendingKzg { sidecar: column.sidecar, stream_id: column.stream_id, @@ -269,7 +269,7 @@ impl DataColumnsTile { bitmask, slot, is_gloas, - relay, + relay: if relay_eligible { relay } else { RelayMeta::None }, }); if queued { ColumnDisposition::Batched } else { ColumnDisposition::Ignored } } @@ -853,6 +853,58 @@ mod tests { } } + /// A sidecar whose gossip checks could not all be completed is still + /// imported, but must not reach the mesh with us as its relayer. The relay + /// is dropped at batch time, so the flush has nothing to send. + #[test] + fn relay_ineligible_column_is_batched_without_a_relay() { + for (relay_eligible, want_relay, cache) in + [(true, true, "relay_ok"), (false, false, "relay_gated")] + { + // `consumer` is declared before `rig` so it outlives the batched + // `TRead` that points back at it. + let (mut consumer, ssz) = produce_block(&blob_block_bytes(7), cache); + let mut rig = Rig::new(CUSTODY_COLUMNS); + let read = consumer.acquire(ssz); + + let disposition = rig.tile.handle_column( + ColumnOutcome::Record { + block_root: [4u8; 32], + column_index: 3, + bitmask: 1 << 3, + slot: 7, + relay_eligible, + }, + PendingColumn { + stream_id: P2pStreamId::new( + 2, + 2, + StreamProtocol::DataColumnSidecarsByRange, + true, + ), + sidecar: read, + gossip_subnet: None, + recv_ts: IngestionTime::now(), + }, + false, + RelayMeta::Rpc { ssz }, + &mut rig.conn.producers, + ); + + assert!( + matches!(disposition, ColumnDisposition::Batched), + "relay_eligible={relay_eligible}: imported either way" + ); + let queued = rig.tile.kzg_batch.pending.first().expect("batched"); + assert_eq!( + !matches!(queued.relay, RelayMeta::None), + want_relay, + "relay_eligible={relay_eligible}" + ); + rig.tile.kzg_batch.pending.clear(); + } + } + /// Fulu-layout sidecar with empty lists: enough for `SidecarLayout::of` /// and the index read, which is all these cases need. fn synth_fulu_sidecar(index: u64, slot: u64) -> Vec { diff --git a/crates/columns/src/validate.rs b/crates/columns/src/validate.rs index 67630454..408b54ea 100644 --- a/crates/columns/src/validate.rs +++ b/crates/columns/src/validate.rs @@ -6,7 +6,7 @@ use std::{ use flux_profiler::timed; use silver_beacon_state_data::{BeaconStateReader, SLOTS_PER_EPOCH, SpecConfig}; use silver_common::{ - IngestionTime, P2pStreamId, StreamProtocol, TRead, Wheel, column_util as util, + IngestionTime, P2pStreamId, TRead, Wheel, column_util as util, ssz_view::{ DataColumnSidecarFuluView, DataColumnSidecarGloasView, NUMBER_OF_COLUMNS, SidecarLayout, SignedBeaconBlockView, @@ -27,11 +27,39 @@ pub(crate) struct PendingColumn { pub(crate) enum ColumnOutcome { Skip, - AlreadyHeld { block_root: BlockRoot, column_index: u64, slot: u64 }, - Reject { block_root: BlockRoot, slot: u64, bitmask: u128 }, - Buffer { block_root: BlockRoot }, - AwaitParent { parent_root: BlockRoot }, - Record { block_root: BlockRoot, column_index: u64, bitmask: u128, slot: u64 }, + AlreadyHeld { + block_root: BlockRoot, + column_index: u64, + slot: u64, + }, + Reject { + block_root: BlockRoot, + slot: u64, + bitmask: u128, + }, + Buffer { + block_root: BlockRoot, + }, + AwaitParent { + parent_root: BlockRoot, + }, + /// `relay_eligible` is false when a check the gossip rules require could + /// not be completed — the sidecar is still imported, but it must not enter + /// the mesh on our authority. + Record { + block_root: BlockRoot, + column_index: u64, + bitmask: u128, + slot: u64, + relay_eligible: bool, + }, +} + +#[derive(Debug, PartialEq, Eq)] +enum ProposerCheck { + Matches, + Mismatch, + Unresolvable, } #[derive(Debug, PartialEq, Eq)] @@ -174,8 +202,6 @@ impl ColumnValidator { return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; } - let do_parent_checks = stream_id.protocol() == StreamProtocol::GossipSub; - if tracker.has_any(&block_root, column_bitmask) { return ColumnOutcome::AlreadyHeld { block_root, column_index, slot }; } @@ -199,26 +225,18 @@ impl ColumnValidator { let claimed_proposer_index = DataColumnSidecarFuluView::proposer_index(buffer); let checks = self.beacon_state.read(&|v| { let state_epoch = v.slot.current_epoch(); - - let (proposer_matches, parent_validated, is_above_finalized) = if do_parent_checks { - // proposer_lookahead is anchored to `state_epoch` and covers - // current+next epochs (PROPOSER_LOOKAHEAD_SIZE = 64). Slots - // outside that window we cannot resolve here. - let lookahead_idx = slot.wrapping_sub(state_epoch * SLOTS_PER_EPOCH) as usize; - let expected_proposer = v.epoch.proposer_at(lookahead_idx); - let parent_validated = parent_root == sync_state.head_root() || - v.block_roots.contains(parent_root, v.slot.slot_number()); - let is_above_finalized = - util::is_above_finalized(buffer, v.epoch.state().finalized_checkpoint.epoch); - ( - expected_proposer == Some(claimed_proposer_index), - parent_validated, - is_above_finalized, - ) - } else { - // Sync / RPC blocks cannot validate proposer shuffling. - (true, true, true) + // proposer_lookahead is anchored to `state_epoch` and covers + // current+next epochs (PROPOSER_LOOKAHEAD_SIZE = 64). + let lookahead_idx = slot.wrapping_sub(state_epoch * SLOTS_PER_EPOCH) as usize; + let proposer = match v.epoch.proposer_at(lookahead_idx) { + Some(expected) if expected == claimed_proposer_index => ProposerCheck::Matches, + Some(_) => ProposerCheck::Mismatch, + None => ProposerCheck::Unresolvable, }; + let parent_in_head_fork = parent_root == sync_state.head_root() || + v.block_roots.contains(parent_root, v.slot.slot_number()); + let is_above_finalized = + util::is_above_finalized(buffer, v.epoch.state().finalized_checkpoint.epoch); let idx = claimed_proposer_index as usize; let pubkey = @@ -226,15 +244,15 @@ impl ColumnValidator { ( is_above_finalized, - parent_validated, - proposer_matches, + parent_in_head_fork, + proposer, pubkey, v.epoch.fork().current_version, // TODO for backfill v.imm.genesis_validators_root, ) }); // No snapshot yet (pre-bootstrap): nothing can be validated. - let Some((above_finalized, parent_validated, proposer_matches, pubkey, fork_version, gvr)) = + let Some((above_finalized, parent_in_head_fork, proposer, pubkey, fork_version, gvr)) = checks else { tracing::warn!(?stream_id, "sidecar before first beacon state snapshot"); @@ -245,7 +263,7 @@ impl ColumnValidator { tracing::warn!(?stream_id, "sidecar slot at or below finalized — ignoring"); return ColumnOutcome::Skip; } - match self.check_parent(parent_root, slot, parent_validated) { + match self.check_parent(parent_root, slot, parent_in_head_fork) { ParentCheck::Seen => {} ParentCheck::Unseen => { tracing::warn!( @@ -261,10 +279,18 @@ impl ColumnValidator { return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; } } - if !proposer_matches { - tracing::warn!(?stream_id, "sidecar proposer_index mismatch"); - return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; - } + let relay_eligible = match proposer { + ProposerCheck::Matches => true, + ProposerCheck::Mismatch => { + tracing::warn!(?stream_id, "sidecar proposer_index mismatch"); + return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; + } + // Spec answer is IGNORE. + ProposerCheck::Unresolvable => { + tracing::debug!(?stream_id, slot, "sidecar proposer unresolvable — not relayed"); + false + } + }; // BLS verify cache: skip the ~1 ms verify iff the sidecar's // signature bytes match a previously-validated signature for @@ -283,7 +309,13 @@ impl ColumnValidator { tracker.set_signature(block_root, sig_bytes); } - ColumnOutcome::Record { block_root, column_index, bitmask: column_bitmask, slot } + ColumnOutcome::Record { + block_root, + column_index, + bitmask: column_bitmask, + slot, + relay_eligible, + } } #[timed] @@ -337,6 +369,12 @@ impl ColumnValidator { return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; } - ColumnOutcome::Record { block_root, column_index, bitmask: column_bitmask, slot } + ColumnOutcome::Record { + block_root, + column_index, + bitmask: column_bitmask, + slot, + relay_eligible: true, + } } } From cea283448e4f1bdfac0400588479070936387aae Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 13:29:49 +0100 Subject: [PATCH 07/11] future slot gate --- crates/beacon_api/src/config.rs | 3 +- crates/beacon_state/tile/src/error.rs | 6 +-- crates/beacon_state/tile/src/tile/block.rs | 9 ++-- .../beacon_state/tile/tests/ef_fork_choice.rs | 26 ++--------- crates/common/src/ticker.rs | 46 +++---------------- 5 files changed, 20 insertions(+), 70 deletions(-) diff --git a/crates/beacon_api/src/config.rs b/crates/beacon_api/src/config.rs index a6a7bd8a..54dbd9bf 100644 --- a/crates/beacon_api/src/config.rs +++ b/crates/beacon_api/src/config.rs @@ -18,6 +18,7 @@ use silver_common::{ MAX_BLOB_COMMITMENTS_PER_BLOCK, MAX_COMMITTEES_PER_SLOT, MAX_PAYLOAD_SIZE, MAX_REQUEST_BLOCKS_DENEB, MAX_VALIDATORS_PER_COMMITTEE, NUMBER_OF_COLUMNS, }, + ticker::MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS, }; use crate::json::Json; @@ -192,7 +193,7 @@ const NETWORK_CONFIG: &[(&str, u64)] = &[ ("EPOCHS_PER_SUBNET_SUBSCRIPTION", EPOCHS_PER_SUBNET_SUBSCRIPTION), ("MIN_EPOCHS_FOR_BLOCK_REQUESTS", 33_024), ("ATTESTATION_PROPAGATION_SLOT_RANGE", 32), - ("MAXIMUM_GOSSIP_CLOCK_DISPARITY", 500), + ("MAXIMUM_GOSSIP_CLOCK_DISPARITY", MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS), ("SUBNETS_PER_NODE", SUBNETS_PER_NODE as u64), ("ATTESTATION_SUBNET_COUNT", 64), ("ATTESTATION_SUBNET_EXTRA_BITS", 0), diff --git a/crates/beacon_state/tile/src/error.rs b/crates/beacon_state/tile/src/error.rs index 46898688..3304c3a1 100644 --- a/crates/beacon_state/tile/src/error.rs +++ b/crates/beacon_state/tile/src/error.rs @@ -25,10 +25,8 @@ pub enum PrecheckError { PastSlot { block_slot: Slot, parent_slot: Slot }, #[error("block already imported: block_root=0x{}", b256_hex(block_root))] AlreadyKnown { block_root: B256 }, - #[error( - "block ticker slot precheck failed: block_slot={block_slot} ticker={wall_slot_plus_one}" - )] - FutureSlot { block_slot: Slot, wall_slot_plus_one: Slot }, + #[error("block ticker slot precheck failed: block_slot={block_slot} wall_slot={wall_slot}")] + FutureSlot { block_slot: Slot, wall_slot: Slot }, #[error( "block proposer lookahead precheck failed: expected={expected} got={got} \ block_root=0x{}", diff --git a/crates/beacon_state/tile/src/tile/block.rs b/crates/beacon_state/tile/src/tile/block.rs index d4ddc63c..ee9c39dc 100644 --- a/crates/beacon_state/tile/src/tile/block.rs +++ b/crates/beacon_state/tile/src/tile/block.rs @@ -508,10 +508,11 @@ impl BeaconStateTile { return Err(PrecheckError::PastSlot { block_slot, parent_slot }); } - // Spec gossip rule: IGNORE blocks whose slot exceeds wall slot. - let wall_slot_plus_one = self.ticker.current_slot() + 1; - if block_slot > wall_slot_plus_one { - return Err(PrecheckError::FutureSlot { block_slot, wall_slot_plus_one }); + if self.ticker.is_future_slot(block_slot) { + return Err(PrecheckError::FutureSlot { + block_slot, + wall_slot: self.ticker.current_slot(), + }); } let parent_epoch = parent_slot / SLOTS_PER_EPOCH; diff --git a/crates/beacon_state/tile/tests/ef_fork_choice.rs b/crates/beacon_state/tile/tests/ef_fork_choice.rs index 6c32c880..8b505ec0 100644 --- a/crates/beacon_state/tile/tests/ef_fork_choice.rs +++ b/crates/beacon_state/tile/tests/ef_fork_choice.rs @@ -4,12 +4,10 @@ //! asserts head / justified / finalized / proposer_boost_root after each step. //! //! Runs the follower-relevant handlers (`ex_ante`, `get_head`, `on_block`, -//! including the `peerdas` data-availability cases — `is_data_available` is +//! including the `future_block` and `peerdas` cases — `is_data_available` is //! modeled by running silver's real column-sidecar verification and importing -//! only when it passes). Skipped, logged not silent: the proposer-only -//! `get_proposer_head` / `should_override_forkchoice_update` handlers, and -//! `future_block` (silver's gossip next-slot tolerance vs the strict store -//! rule). +//! only when it passes). The proposer-only `get_proposer_head` / +//! `should_override_forkchoice_update` handlers are not driven at all. mod ef_common; @@ -25,16 +23,6 @@ fn fork_choice_dir(fork: &str, handler: &str) -> PathBuf { spec_tests_dir().join("tests").join("mainnet").join(fork).join("fork_choice").join(handler) } -/// Cases we knowingly don't cover yet — logged, never silently passed. -fn known_skip(name: &str) -> Option<&'static str> { - if name.contains("future_block") { - // silver permits a next-slot block (gossip clock-disparity tolerance); - // the EF on_block handler enforces the strict store rule. - return Some("silver allows next-slot blocks (gossip tolerance)"); - } - None -} - /// Spec `is_data_available`: run silver's real column-sidecar verification /// (shape + inclusion proof + KZG, from the storage tile) over the columns the /// step provides. Available iff non-empty and every column verifies. @@ -175,16 +163,10 @@ fn run_checks(name: &str, si: usize, tile: &BeaconStateTile, checks: &Mapping) { fn run_handler(fork: &str, handler: &str) { let cases = iter_test_cases(&fork_choice_dir(fork, handler)); assert!(!cases.is_empty(), "{fork}/{handler}: no fork_choice cases found"); - let mut skipped = 0; for (name, path) in &cases { - if let Some(reason) = known_skip(name) { - eprintln!("SKIP {fork}/{handler}/{name}: {reason}"); - skipped += 1; - continue; - } run_case(&format!("{fork}/{handler}/{name}"), path); } - eprintln!("{fork}/{handler}: {} run, {skipped} skipped", cases.len() - skipped); + eprintln!("{fork}/{handler}: {} run", cases.len()); } #[test] diff --git a/crates/common/src/ticker.rs b/crates/common/src/ticker.rs index 080fce29..0bce28f8 100644 --- a/crates/common/src/ticker.rs +++ b/crates/common/src/ticker.rs @@ -2,6 +2,8 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; type Slot = u64; +pub const MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS: u64 = 500; + // slot/24 before next slot = 500ms on mainnet. const FORK_CHOICE_LOOKAHEAD_DIVISOR: u64 = 24; // slot/4 before next slot = 3s on mainnet, state advance fires at 3/4. @@ -109,24 +111,11 @@ impl SlotTicker { self.millis_since_genesis() / self.slot_ms } - /// Whether `slot` is current after extending both ends of its wall-clock - /// interval by `disparity`. Consensus gossip validation uses this for its - /// permitted clock skew around slot boundaries. - pub fn is_current_slot_with_disparity(&self, slot: Slot, disparity: Duration) -> bool { - let now = self.millis_since_genesis(); - let disparity_ms = u64::try_from(disparity.as_millis()).unwrap_or(u64::MAX); - let start = slot.saturating_mul(self.slot_ms); - let end = slot.saturating_add(1).saturating_mul(self.slot_ms); - now.saturating_add(disparity_ms) >= start && now <= end.saturating_add(disparity_ms) - } - - /// The newest slot admitted by [`Self::is_current_slot_with_disparity`]. - /// A two-slot seen-cache can rotate to this value and retain every slot - /// that is valid at the current instant, including across the boundary. - pub fn latest_slot_with_disparity(&self, disparity: Duration) -> Slot { - let wall = self.current_slot(); - let next = wall.saturating_add(1); - if self.is_current_slot_with_disparity(next, disparity) { next } else { wall } + /// Spec gossip rule: a block or sidecar from a future slot is IGNOREd, + /// with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance. + pub fn is_future_slot(&self, slot: Slot) -> bool { + slot.saturating_mul(self.slot_ms) > + self.millis_since_genesis() + MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS } pub fn is_before_attesting_interval(&self, is_gloas: bool) -> bool { @@ -242,25 +231,4 @@ mod tests { let ev = t.tick(); assert!(matches!(ev, TickEvent::SlotStart(3))); } - - #[test] - fn current_slot_disparity_covers_both_boundary_sides() { - let mut t = SlotTicker::new(0, Duration::from_secs(12), Duration::from_secs(4)); - let disparity = Duration::from_millis(500); - - t.set_since_genesis_ms(12_000 + 499); - assert!(t.is_current_slot_with_disparity(0, disparity)); - assert!(t.is_current_slot_with_disparity(1, disparity)); - assert!(!t.is_current_slot_with_disparity(2, disparity)); - assert_eq!(t.latest_slot_with_disparity(disparity), 1); - - t.set_since_genesis_ms(24_000 - 500); - assert!(t.is_current_slot_with_disparity(1, disparity)); - assert!(t.is_current_slot_with_disparity(2, disparity)); - assert_eq!(t.latest_slot_with_disparity(disparity), 2); - - t.set_since_genesis_ms(24_000 - 501); - assert!(!t.is_current_slot_with_disparity(2, disparity)); - assert_eq!(t.latest_slot_with_disparity(disparity), 1); - } } From e95f199116c5205d643e10b1e66b0ee962c7e192 Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 13:46:35 +0100 Subject: [PATCH 08/11] fulu block precheck --- .../data/src/beacon_block_body.rs | 10 +++ crates/beacon_state/tile/src/error.rs | 23 ++++- crates/beacon_state/tile/src/tile/block.rs | 45 +++++++++- crates/beacon_state/tile/src/tile/tests.rs | 85 ++++++++++++++++++- 4 files changed, 157 insertions(+), 6 deletions(-) diff --git a/crates/beacon_state/data/src/beacon_block_body.rs b/crates/beacon_state/data/src/beacon_block_body.rs index cabe9a69..31618561 100644 --- a/crates/beacon_state/data/src/beacon_block_body.rs +++ b/crates/beacon_state/data/src/beacon_block_body.rs @@ -143,6 +143,16 @@ impl<'a> BodyOffsets<'a> { ) .unwrap_or(&[]) } + + #[inline] + pub fn blob_commitments_fulu(&self) -> &'a [u8] { + self.slice( + BeaconBlockBodyFuluView::blob_kzg_commitments_offset(self.body), + BeaconBlockBodyFuluView::execution_requests_offset(self.body), + ) + .unwrap_or(&[]) + } + #[inline] pub fn execution_requests(&self) -> &'a [u8] { self.slice( diff --git a/crates/beacon_state/tile/src/error.rs b/crates/beacon_state/tile/src/error.rs index 3304c3a1..7d8ba3d0 100644 --- a/crates/beacon_state/tile/src/error.rs +++ b/crates/beacon_state/tile/src/error.rs @@ -45,6 +45,24 @@ pub enum PrecheckError { b256_hex(block_root) )] UnverifiedParentPayload { parent_root: B256, block_root: B256 }, + #[error( + "block execution payload timestamp precheck failed: expected={expected} got={got} \ + block_root=0x{}", + b256_hex(block_root) + )] + PayloadTimestamp { expected: u64, got: u64, block_root: B256 }, + #[error( + "block execution payload too short for its fork: len={len} min={min} \ + block_root=0x{}", + b256_hex(block_root) + )] + PayloadTooShort { len: usize, min: usize, block_root: B256 }, + #[error( + "block blob commitment count precheck failed: got={got} max={max} \ + block_root=0x{}", + b256_hex(block_root) + )] + TooManyCommitments { got: usize, max: usize, block_root: B256 }, #[error("invalid block signature: block_root=0x{}", b256_hex(block_root))] InvalidSignature { block_root: B256 }, } @@ -65,7 +83,10 @@ impl PrecheckError { } Self::ParentInvalid { block_root, .. } | Self::ProposerLookaheadMismatch { block_root, .. } | - Self::ProposerIndexTooBig { block_root, .. } => Feedback::Reject(Some(block_root)), + Self::ProposerIndexTooBig { block_root, .. } | + Self::PayloadTimestamp { block_root, .. } | + Self::PayloadTooShort { block_root, .. } | + Self::TooManyCommitments { block_root, .. } => Feedback::Reject(Some(block_root)), Self::InvalidSignature { block_root } => Feedback::Reject(Some(block_root)), } } diff --git a/crates/beacon_state/tile/src/tile/block.rs b/crates/beacon_state/tile/src/tile/block.rs index ee9c39dc..d20dd7f3 100644 --- a/crates/beacon_state/tile/src/tile/block.rs +++ b/crates/beacon_state/tile/src/tile/block.rs @@ -1,7 +1,8 @@ use flux::spine::SpineProducers; use flux_profiler::timed; use silver_beacon_state_data::{ - B256, BeaconBlockHeader, Checkpoint, Epoch, SLOTS_PER_EPOCH, StateId, + B256, BeaconBlockHeader, BodyFork, BodyOffsets, Checkpoint, Epoch, SLOTS_PER_EPOCH, Slot, + StateId, StateReadView, }; use silver_common::{ BeaconStateEvent, BlockSource, BlockStage, EngineFcuReq, EngineNewPayloadReq, EngineReq, @@ -541,6 +542,10 @@ impl BeaconStateTile { }); } + if !is_gloas { + self.precheck_fulu_payload(body, block_slot, block_epoch, &rv, block_root)?; + } + let has_data_columns = SignedBeaconBlockView::has_data_columns(data, is_gloas); Ok(ParsedBlock { @@ -553,6 +558,44 @@ impl BeaconStateTile { }) } + /// The Fulu gossip rules require the execution timestamp and the active + /// blob limit before propagation. The STF re-checks both, but that runs + /// after the relay has already gone out. + fn precheck_fulu_payload( + &self, + body: &[u8], + block_slot: Slot, + block_epoch: Epoch, + rv: &StateReadView<'_>, + block_root: B256, + ) -> Result<(), PrecheckError> { + let Some(offsets) = BodyOffsets::new(body, BodyFork::Fulu) else { + return Ok(()); // body_root hashing already tolerates a short body + }; + + let payload = offsets.payload(); + if payload.len() < ssz_view::EXECUTION_PAYLOAD_FIXED { + return Err(PrecheckError::PayloadTooShort { + len: payload.len(), + min: ssz_view::EXECUTION_PAYLOAD_FIXED, + block_root, + }); + } + let expected = rv.imm.genesis_time + block_slot * self.spec.seconds_per_slot(); + let got = ssz_view::ExecutionPayloadView::timestamp(payload); + if got != expected { + return Err(PrecheckError::PayloadTimestamp { expected, got, block_root }); + } + + let max = self.spec.blob_params_at(block_epoch).max_blobs_per_block as usize; + let got = offsets.blob_commitments_fulu().len() / ssz_view::BYTES_PER_KZG_COMMITMENT; + if got > max { + return Err(PrecheckError::TooManyCommitments { got, max, block_root }); + } + + Ok(()) + } + fn verify_block_signature(&self, data: &[u8], parsed: &ParsedBlock) -> bool { let block_epoch = parsed.header.slot / SLOTS_PER_EPOCH; let rv = self.state.read_view(parsed.parent_state_id); diff --git a/crates/beacon_state/tile/src/tile/tests.rs b/crates/beacon_state/tile/src/tile/tests.rs index 470d7514..baaede1c 100644 --- a/crates/beacon_state/tile/src/tile/tests.rs +++ b/crates/beacon_state/tile/src/tile/tests.rs @@ -11,10 +11,10 @@ use silver_common::{ GossipTopic, MessageId, P2pStreamId, StreamProtocol, TCache, TCacheProducer, TCacheRead, TProducer, ssz_view::{ - ATTESTATION_DATA_SIZE, AttestationView, PROPOSER_SLASHING_SIZE, SIGNED_AGG_PROOF_MIN, - SIGNED_BEACON_BLOCK_MIN, SIGNED_BLS_CHANGE_SIZE, SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, - SIGNED_VOLUNTARY_EXIT_SIZE, SINGLE_ATT_SIZE, SignedAggregateAndProofView, - SingleAttestationView, StatusView, + ATTESTATION_DATA_SIZE, AttestationView, BYTES_PER_KZG_COMMITMENT, EXECUTION_PAYLOAD_FIXED, + PROPOSER_SLASHING_SIZE, SIGNED_AGG_PROOF_MIN, SIGNED_BEACON_BLOCK_MIN, + SIGNED_BLS_CHANGE_SIZE, SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, SIGNED_VOLUNTARY_EXIT_SIZE, + SINGLE_ATT_SIZE, SignedAggregateAndProofView, SingleAttestationView, StatusView, }, }; use silver_ssz::ssz_view::{EXECUTION_PAYLOAD_ENVELOPE_MIN, SyncCommitteeContributionView}; @@ -453,6 +453,83 @@ fn publish_block_bytes(producer: &mut TProducer, bytes: &[u8]) -> (Vec, TCac (bytes.to_vec(), read) } +/// Fulu requires the execution timestamp and the active blob limit before a +/// block is propagated. The STF checks both, but that runs after the relay. +#[test] +fn payload_timestamp_and_blob_count_are_checked_before_relay() { + const PARENT_SLOT: u64 = 10; + let slot = PARENT_SLOT + 1; + + let spec = SpecConfig::mainnet(); + let max_blobs = spec.blob_params_at(0).max_blobs_per_block as usize; + // The test state's genesis_time is 0, so the stamp is purely slot-derived. + let good_stamp = slot * spec.seconds_per_slot(); + + for (stamp, commitments, want_accepted) in [ + (good_stamp, max_blobs, true), + (good_stamp + 1, 0, false), + (good_stamp - 1, 0, false), + (good_stamp, max_blobs + 1, false), + ] { + let (mut tile, mut gp, _rp, _spine, mut adapter) = tile_with_producers(PARENT_SLOT + 2); + seed_tile(&mut tile, 4, PARENT_SLOT); + tile.sync_target = SyncUpdate::Following; + + let bytes = fulu_block_with_payload(slot, stamp, commitments); + let (data, read) = publish_block_bytes(&mut gp, &bytes); + + let mut relayed = false; + let feedback = tile.apply_block( + &data, + read, + BlockSource::Gossip, + true, // pre_verified: the signature is not what these cases are about + &mut adapter.producers, + |_| relayed = true, + ); + + assert_eq!( + !matches!(feedback, Feedback::Reject(_)), + want_accepted, + "stamp={stamp} commitments={commitments}: {feedback:?}" + ); + assert_eq!( + relayed, want_accepted, + "stamp={stamp} commitments={commitments}: relay must follow the precheck" + ); + } +} + +/// Fulu block whose body is long enough for `BodyOffsets`, carrying an +/// execution payload at its fixed size followed by `n` blob commitments. +/// Every other variable field is empty. +fn fulu_block_with_payload(slot: u64, timestamp: u64, n_commitments: usize) -> Vec { + const BODY: usize = 184; + const FIXED: usize = 396; + let payload_end = FIXED + EXECUTION_PAYLOAD_FIXED; + let body_len = payload_end + n_commitments * BYTES_PER_KZG_COMMITMENT; + + let mut b = empty_block(BODY + body_len); + b[100..108].copy_from_slice(&slot.to_le_bytes()); + b[116..148].copy_from_slice(&ANCHOR_ROOT); + + // Empty lists all point at the end of the fixed part. + for off in [200usize, 204, 208, 212, 216] { + b[BODY + off..BODY + off + 4].copy_from_slice(&(FIXED as u32).to_le_bytes()); + } + let put = |b: &mut Vec, off: usize, v: usize| { + b[BODY + off..BODY + off + 4].copy_from_slice(&(v as u32).to_le_bytes()); + }; + put(&mut b, 380, FIXED); // execution_payload + put(&mut b, 384, payload_end); // bls_to_execution_changes + put(&mut b, 388, payload_end); // blob_kzg_commitments + put(&mut b, 392, body_len); // execution_requests + + let payload = BODY + FIXED; + b[payload + 428..payload + 436].copy_from_slice(×tamp.to_le_bytes()); + b +} + // ── pending-block bounds ── #[test] From 2dd42874b2150872f906bb200f1e024f13bbfbf1 Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 13:53:46 +0100 Subject: [PATCH 09/11] gossip block check --- crates/beacon_state/tile/src/tile.rs | 1 + crates/beacon_state/tile/src/tile/block.rs | 30 ++++++++++----- crates/beacon_state/tile/src/tile/gossip.rs | 4 +- crates/beacon_state/tile/src/tile/tests.rs | 42 +++++++++++++++++++++ 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/crates/beacon_state/tile/src/tile.rs b/crates/beacon_state/tile/src/tile.rs index b92920b0..c6863048 100644 --- a/crates/beacon_state/tile/src/tile.rs +++ b/crates/beacon_state/tile/src/tile.rs @@ -111,6 +111,7 @@ struct ParsedBlock { parent_state_id: StateId, is_gloas: bool, parent_payload_status: PayloadStatus, + relay_eligible: bool, } pub struct BeaconStateTile { diff --git a/crates/beacon_state/tile/src/tile/block.rs b/crates/beacon_state/tile/src/tile/block.rs index d20dd7f3..42dc54cd 100644 --- a/crates/beacon_state/tile/src/tile/block.rs +++ b/crates/beacon_state/tile/src/tile/block.rs @@ -55,7 +55,9 @@ impl BeaconStateTile { let block_slot = SignedBeaconBlockView::slot(data); let parsed = match self.parse_and_verify_block(data, pre_verified) { Ok(parsed) => { - send_gossip(producers); + if parsed.relay_eligible { + send_gossip(producers); + } parsed } Err(e) => { @@ -517,21 +519,28 @@ impl BeaconStateTile { } let parent_epoch = parent_slot / SLOTS_PER_EPOCH; - // Fulu proposer selection via `proposer_lookahead` (current + next - // epoch, 64 slots), fixed at the parent's prior epoch boundary — read - // it from the parent post-state. - if block_epoch == parent_epoch || block_epoch == parent_epoch + 1 { - let lookahead_idx = (block_slot - parent_epoch * SLOTS_PER_EPOCH) as usize; - if let Some(expected) = rv.epoch.proposer_at(lookahead_idx) && - proposer_index != expected - { + // Fulu proposer selection via `proposer_lookahead`, fixed at the + // parent's prior epoch boundary and covering only its current + next + // epoch. + let lookahead_idx = (block_slot - parent_epoch * SLOTS_PER_EPOCH) as usize; + let relay_eligible = match rv.epoch.proposer_at(lookahead_idx) { + Some(expected) if proposer_index != expected => { return Err(PrecheckError::ProposerLookaheadMismatch { expected, got: proposer_index, block_root, }); } - } + Some(_) => true, + None => { + tracing::debug!( + block_slot, + parent_slot, + "proposer lookahead does not reach this block — not relayed" + ); + false + } + }; let validator_count = rv.validators.count(); if proposer_index as usize >= validator_count { @@ -555,6 +564,7 @@ impl BeaconStateTile { parent_state_id, is_gloas, parent_payload_status, + relay_eligible, }) } diff --git a/crates/beacon_state/tile/src/tile/gossip.rs b/crates/beacon_state/tile/src/tile/gossip.rs index 3d713691..e49b21c6 100644 --- a/crates/beacon_state/tile/src/tile/gossip.rs +++ b/crates/beacon_state/tile/src/tile/gossip.rs @@ -1167,7 +1167,9 @@ impl BeaconStateTile { let feedback = match m.topic { GossipTopic::BeaconBlock if !self.sync_target.is_following() => { match self.parse_and_verify_block(data, pre_verified) { - Ok(_) if do_relay => Self::relay_gossip(&m, producers), + Ok(parsed) if do_relay && parsed.relay_eligible => { + Self::relay_gossip(&m, producers) + } Err(err) if matches!(err.feedback(), Feedback::Reject(_)) => { producers.produce(PeerEvent::P2pGossipInvalidMsg { p2p_peer: m.stream_id.peer(), diff --git a/crates/beacon_state/tile/src/tile/tests.rs b/crates/beacon_state/tile/src/tile/tests.rs index baaede1c..b02d35fb 100644 --- a/crates/beacon_state/tile/src/tile/tests.rs +++ b/crates/beacon_state/tile/src/tile/tests.rs @@ -530,6 +530,48 @@ fn fulu_block_with_payload(slot: u64, timestamp: u64, n_commitments: usize) -> V b } +/// `proposer_lookahead` reaches only the parent's current and next epoch. A +/// block past a long skipped-slot run has no entry, so silver cannot check its +/// proposer — it still imports, but must not go on the wire as if it had. +#[test] +fn block_past_the_lookahead_window_imports_but_is_not_relayed() { + const PARENT_SLOT: u64 = 10; + + // The parent sits in epoch 0, so the lookahead index is the slot itself + // and the window is `[0, PROPOSER_LOOKAHEAD_SIZE)`. + let last_in_window = PROPOSER_LOOKAHEAD_SIZE as u64 - 1; + for (slot, want_relay) in [ + (PARENT_SLOT + 1, true), + (last_in_window, true), + (last_in_window + 1, false), + (last_in_window + SLOTS_PER_EPOCH, false), + ] { + let (mut tile, mut gp, _rp, _spine, mut adapter) = tile_with_producers(slot); + seed_tile(&mut tile, 4, PARENT_SLOT); + tile.sync_target = SyncUpdate::Following; + + let mut bytes = empty_block(200); + bytes[100..108].copy_from_slice(&slot.to_le_bytes()); + bytes[116..148].copy_from_slice(&ANCHOR_ROOT); + let (data, read) = publish_block_bytes(&mut gp, &bytes); + + let mut relayed = false; + let feedback = tile.apply_block( + &data, + read, + BlockSource::Gossip, + true, + &mut adapter.producers, + |_| relayed = true, + ); + + assert_eq!(relayed, want_relay, "slot {slot}: {feedback:?}"); + // Either way the block is not thrown away: the proposer window is our + // limit, not grounds to refuse the block. + assert!(!matches!(feedback, Feedback::Ignore), "slot {slot}: {feedback:?}"); + } +} + // ── pending-block bounds ── #[test] From f471cb47b8d59075a580b4eb20b2ab16aa337457 Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 14:02:09 +0100 Subject: [PATCH 10/11] msgs bounds check + future slot check for columns --- crates/columns/src/sync.rs | 30 +++++++- crates/columns/src/validate.rs | 4 +- .../src/p2p/streams/rpc/reservation.rs | 70 +++++++++++++++++-- .../src/p2p/streams/rpc/response_in.rs | 6 +- 4 files changed, 99 insertions(+), 11 deletions(-) diff --git a/crates/columns/src/sync.rs b/crates/columns/src/sync.rs index a87490de..cf23213b 100644 --- a/crates/columns/src/sync.rs +++ b/crates/columns/src/sync.rs @@ -1,14 +1,31 @@ +use std::time::Instant; + use silver_beacon_state_data::SLOTS_PER_EPOCH; -use silver_common::{SyncUpdate, merkle::B256, ssz_view::StatusView}; +use silver_common::{ + SyncUpdate, merkle::B256, ssz_view::StatusView, ticker::MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS, +}; -#[derive(Debug, Default)] +#[derive(Debug)] pub(crate) struct SyncStatus { sync_target: SyncUpdate, wall_slot: u64, + wall_slot_at: Instant, head_root: B256, finalized_slot: u64, } +impl Default for SyncStatus { + fn default() -> Self { + Self { + sync_target: SyncUpdate::default(), + wall_slot: 0, + wall_slot_at: Instant::now(), + head_root: B256::default(), + finalized_slot: 0, + } + } +} + impl SyncStatus { pub(crate) fn is_synced(&self) -> bool { self.sync_target.is_following() @@ -30,8 +47,17 @@ impl SyncStatus { &self.head_root } + pub(crate) fn is_future_slot(&self, slot: u64, slot_ms: u64) -> bool { + let ms_into_slot = self.wall_slot_at.elapsed().as_millis() as u64; + slot.saturating_mul(slot_ms) > + self.wall_slot.saturating_mul(slot_ms) + + ms_into_slot + + MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS + } + pub(crate) fn update(&mut self, ssz: [u8; 92], wall_slot: u64) { self.wall_slot = wall_slot; + self.wall_slot_at = Instant::now(); self.head_root = *StatusView::head_root(&ssz); self.finalized_slot = StatusView::finalized_epoch(&ssz) * SLOTS_PER_EPOCH; } diff --git a/crates/columns/src/validate.rs b/crates/columns/src/validate.rs index 408b54ea..8f4f6af1 100644 --- a/crates/columns/src/validate.rs +++ b/crates/columns/src/validate.rs @@ -178,7 +178,7 @@ impl ColumnValidator { ); } - if sync_state.is_synced() && slot > sync_state.wall_slot().saturating_add(1) { + if sync_state.is_synced() && sync_state.is_future_slot(slot, self.spec.slot_duration_ms()) { tracing::debug!( ?stream_id, slot, @@ -329,7 +329,7 @@ impl ColumnValidator { let PendingColumn { stream_id, gossip_subnet, .. } = *column; let slot = DataColumnSidecarGloasView::slot(buffer); - if sync_state.is_synced() && slot > sync_state.wall_slot().saturating_add(1) { + if sync_state.is_synced() && sync_state.is_future_slot(slot, self.spec.slot_duration_ms()) { tracing::debug!( ?stream_id, slot, diff --git a/crates/network/src/p2p/streams/rpc/reservation.rs b/crates/network/src/p2p/streams/rpc/reservation.rs index b7943aa3..0b41d161 100644 --- a/crates/network/src/p2p/streams/rpc/reservation.rs +++ b/crates/network/src/p2p/streams/rpc/reservation.rs @@ -1,19 +1,81 @@ -use std::io::{Error, ErrorKind}; +use std::{ + io::{Error, ErrorKind}, + ops::RangeInclusive, +}; use silver_common::{ P2pStreamId, RpcRequest, RpcResponse, StreamProtocol, TCacheProducer, TProducer, TReservation, ssz_view::{ - BLOCKS_BY_RANGE_REQ_SIZE, DC_BY_RANGE_REQ_MAX, - EXECUTION_PAYLOAD_ENVELOPES_BY_RANGE_REQ_SIZE, GOODBYE_SIZE, METADATA_SIZE, PING_SIZE, - STATUS_V1_SIZE, STATUS_V2_SIZE, + BLOCKS_BY_RANGE_REQ_SIZE, DATA_COLUMN_SIDECAR_GLOAS_MIN, DATA_COLUMN_SIDECAR_MAX, + DC_BY_RANGE_REQ_MAX, DC_BY_RANGE_REQ_MIN, DC_BY_ROOT_SINGLE_SIZE, + EXECUTION_PAYLOAD_ENVELOPES_BY_RANGE_REQ_SIZE, GOODBYE_SIZE, MAX_PAYLOAD_SIZE, + METADATA_SIZE, PING_SIZE, SIGNED_BEACON_BLOCK_MAX, SIGNED_BEACON_BLOCK_MIN, + SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, STATUS_V1_SIZE, STATUS_V2_SIZE, }, }; +fn payload_bounds(id: &P2pStreamId) -> RangeInclusive { + let exact = |n: usize| n..=n; + if id.is_incoming() { + // Requests. + match id.protocol() { + StreamProtocol::StatusV1 => exact(STATUS_V1_SIZE), + StreamProtocol::StatusV2 => exact(STATUS_V2_SIZE), + StreamProtocol::Ping => exact(PING_SIZE), + StreamProtocol::Goodbye => exact(GOODBYE_SIZE), + StreamProtocol::Metadata => 0..=0, + StreamProtocol::BeaconBlocksByRange => exact(BLOCKS_BY_RANGE_REQ_SIZE), + StreamProtocol::ExecutionPayloadEnvelopesByRange => { + exact(EXECUTION_PAYLOAD_ENVELOPES_BY_RANGE_REQ_SIZE) + } + StreamProtocol::DataColumnSidecarsByRange => DC_BY_RANGE_REQ_MIN..=DC_BY_RANGE_REQ_MAX, + // `List[Root, MAX_REQUEST_BLOCKS]` and its column/envelope + // siblings: whole elements, and the tcache caps the total. + StreamProtocol::BeaconBlocksByRoot | + StreamProtocol::ExecutionPayloadEnvelopesByRoot => 32..=MAX_PAYLOAD_SIZE, + StreamProtocol::DataColumnSidecarsByRoot => DC_BY_ROOT_SINGLE_SIZE..=MAX_PAYLOAD_SIZE, + _ => 0..=0, + } + } else { + // Responses. + match id.protocol() { + StreamProtocol::StatusV1 => exact(STATUS_V1_SIZE), + StreamProtocol::StatusV2 => exact(STATUS_V2_SIZE), + StreamProtocol::Ping => exact(PING_SIZE), + StreamProtocol::Metadata => exact(METADATA_SIZE), + StreamProtocol::BeaconBlocksByRange | StreamProtocol::BeaconBlocksByRoot => { + SIGNED_BEACON_BLOCK_MIN..=SIGNED_BEACON_BLOCK_MAX + } + // Both sidecar layouts share the protocol; gloas is the shorter. + StreamProtocol::DataColumnSidecarsByRange | + StreamProtocol::DataColumnSidecarsByRoot => { + DATA_COLUMN_SIDECAR_GLOAS_MIN..=DATA_COLUMN_SIDECAR_MAX + } + StreamProtocol::ExecutionPayloadEnvelopesByRange | + StreamProtocol::ExecutionPayloadEnvelopesByRoot => { + SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN..=MAX_PAYLOAD_SIZE + } + _ => 0..=0, + } + } +} + pub fn alloc_incoming_rpc( rpc_in: &mut TProducer, id: &P2pStreamId, len: usize, ) -> Result { + let bounds = payload_bounds(id); + if !bounds.contains(&len) { + tracing::warn!( + ?id, + len, + min = bounds.start(), + max = bounds.end(), + "rpc chunk length outside the protocol's bounds" + ); + return Err(ErrorKind::InvalidData.into()); + } let (inbound, tcache) = if id.is_incoming() { // incoming rpc = request match id.protocol() { diff --git a/crates/network/src/p2p/streams/rpc/response_in.rs b/crates/network/src/p2p/streams/rpc/response_in.rs index d39adca3..c4d0734b 100644 --- a/crates/network/src/p2p/streams/rpc/response_in.rs +++ b/crates/network/src/p2p/streams/rpc/response_in.rs @@ -326,7 +326,7 @@ mod tests { use std::net::SocketAddr; use quinn_proto::StreamId; - use silver_common::{StreamProtocol, TCache, TRead}; + use silver_common::{StreamProtocol, TCache, TRead, ssz_view::DATA_COLUMN_SIDECAR_GLOAS_MIN}; use super::*; use crate::p2p::streams::{rpc::AcquiredRpcOutbound, snappy::SnappyEncoder}; @@ -449,8 +449,8 @@ mod tests { const STREAM_IDENTIFIER: [u8; 10] = [0xff, 0x06, 0x00, 0x00, b's', b'N', b'a', b'P', b'p', b'Y']; - let declared = 8usize; // SSZ length advertised to the reader - let data_len = 100usize; // actual uncompressed frame data — overshoots + let declared = DATA_COLUMN_SIDECAR_GLOAS_MIN; // SSZ length advertised + let data_len = declared + 44; // actual uncompressed frame data — overshoots // [status=0][fork_digest:4][varint declared][stream id][uncompressed frame] let mut wire = vec![0u8]; From 65fdbecbcd6465ef1c149c83112964566411f549 Mon Sep 17 00:00:00 2001 From: Nina Date: Fri, 4 Sep 2026 14:40:34 +0100 Subject: [PATCH 11/11] better --- crates/beacon_api/src/config.rs | 4 +- crates/beacon_state/data/src/column/roots.rs | 17 +- crates/beacon_state/data/src/column/tests.rs | 23 ++- crates/beacon_state/tile/src/error.rs | 17 +- crates/beacon_state/tile/src/tile.rs | 7 +- crates/beacon_state/tile/src/tile/block.rs | 38 ++-- crates/beacon_state/tile/src/tile/tests.rs | 187 +++++++++++++----- .../tile/tests/ssz_view_fixtures.rs | 12 +- crates/columns/src/sync.rs | 30 +-- crates/columns/src/validate.rs | 40 ++-- crates/common/src/ticker.rs | 47 ++++- .../src/p2p/streams/rpc/reservation.rs | 31 ++- crates/ssz/src/ssz_hash_gloas/block_body.rs | 36 +++- .../src/ssz_hash_gloas/execution_requests.rs | 21 +- crates/ssz/src/ssz_view.rs | 164 ++++++++++++++- 15 files changed, 508 insertions(+), 166 deletions(-) diff --git a/crates/beacon_api/src/config.rs b/crates/beacon_api/src/config.rs index 54dbd9bf..8672da5b 100644 --- a/crates/beacon_api/src/config.rs +++ b/crates/beacon_api/src/config.rs @@ -18,7 +18,7 @@ use silver_common::{ MAX_BLOB_COMMITMENTS_PER_BLOCK, MAX_COMMITTEES_PER_SLOT, MAX_PAYLOAD_SIZE, MAX_REQUEST_BLOCKS_DENEB, MAX_VALIDATORS_PER_COMMITTEE, NUMBER_OF_COLUMNS, }, - ticker::MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS, + ticker::MAXIMUM_GOSSIP_CLOCK_DISPARITY, }; use crate::json::Json; @@ -193,7 +193,7 @@ const NETWORK_CONFIG: &[(&str, u64)] = &[ ("EPOCHS_PER_SUBNET_SUBSCRIPTION", EPOCHS_PER_SUBNET_SUBSCRIPTION), ("MIN_EPOCHS_FOR_BLOCK_REQUESTS", 33_024), ("ATTESTATION_PROPAGATION_SLOT_RANGE", 32), - ("MAXIMUM_GOSSIP_CLOCK_DISPARITY", MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS), + ("MAXIMUM_GOSSIP_CLOCK_DISPARITY", MAXIMUM_GOSSIP_CLOCK_DISPARITY.as_millis() as u64), ("SUBNETS_PER_NODE", SUBNETS_PER_NODE as u64), ("ATTESTATION_SUBNET_COUNT", 64), ("ATTESTATION_SUBNET_EXTRA_BITS", 0), diff --git a/crates/beacon_state/data/src/column/roots.rs b/crates/beacon_state/data/src/column/roots.rs index 11f2ea07..3230878f 100644 --- a/crates/beacon_state/data/src/column/roots.rs +++ b/crates/beacon_state/data/src/column/roots.rs @@ -39,13 +39,16 @@ impl RootsView<'_, BlockRoots> { self.get(slot as usize % SLOTS_PER_HISTORICAL_ROOT) } - /// Whether the ring holds `root` at or below `from_slot`. Fork choice is - /// what writes a root here, so membership means "seen and validated". - /// Walks most-recent-first: a queried parent is virtually always a slot or - /// two back, while a miss costs the whole ring either way. - pub fn contains(&self, root: &B256, from_slot: Slot) -> bool { - (0..SLOTS_PER_HISTORICAL_ROOT as u64) - .any(|back| self.at_slot(from_slot.saturating_sub(back)) == *root) + /// Slot of the block with `root`, if the ring holds it at or below + /// `from_slot`. Fork choice is what writes a root here, so a hit means + /// "seen and validated". + pub fn slot_of(&self, root: &B256, from_slot: Slot) -> Option { + let oldest = from_slot.saturating_sub(SLOTS_PER_HISTORICAL_ROOT as u64 - 1); + let mut slot = (oldest..=from_slot).rev().find(|&s| self.at_slot(s) == *root)?; + while slot > oldest && self.at_slot(slot - 1) == *root { + slot -= 1; + } + Some(slot) } } diff --git a/crates/beacon_state/data/src/column/tests.rs b/crates/beacon_state/data/src/column/tests.rs index 60e2ef88..27628ea2 100644 --- a/crates/beacon_state/data/src/column/tests.rs +++ b/crates/beacon_state/data/src/column/tests.rs @@ -433,22 +433,29 @@ fn block_roots_ring_wraps_and_hashes_as_a_vector() { assert_eq!(wv.hash_root(), hash_b256_vector(&expected)); } -/// `contains` walks back from `from_slot`, so it sees every written bucket but -/// nothing written above the slot it is asked about. +/// `slot_of` walks back from `from_slot`, so it sees every written bucket but +/// nothing written above the slot it is asked about. A block at slot 100 +/// followed by two empty slots occupies buckets 100..=102, and the slot +/// reported is the block's own. #[test] -fn block_roots_contains_scans_back_from_the_given_slot() { +fn block_roots_slot_of_scans_back_from_the_given_slot() { let mut g = BlockRootsGroup::zeroed_vector(); let id = { let mut wv = g.roll_fresh(); - wv.set(100, [0xAA; 32]); - wv.set(101, [0xBB; 32]); + wv.set(99, [0x99; 32]); + for slot in 100..=102 { + wv.set(slot, [0xAA; 32]); + } + wv.set(103, [0xBB; 32]); wv.commit() }; let reader = g.view(id); - assert!(reader.contains(&[0xAA; 32], 101)); - assert!(reader.contains(&[0xBB; 32], 101)); - assert!(!reader.contains(&[0xCC; 32], 101)); + assert_eq!(reader.slot_of(&[0xAA; 32], 103), Some(100)); + assert_eq!(reader.slot_of(&[0xAA; 32], 101), Some(100)); + assert_eq!(reader.slot_of(&[0xBB; 32], 103), Some(103)); + assert_eq!(reader.slot_of(&[0xBB; 32], 102), None, "written above the asked slot"); + assert_eq!(reader.slot_of(&[0xCC; 32], 103), None); } /// A block's reveal accumulates into the current epoch's bucket; the boundary diff --git a/crates/beacon_state/tile/src/error.rs b/crates/beacon_state/tile/src/error.rs index 7d8ba3d0..492a563e 100644 --- a/crates/beacon_state/tile/src/error.rs +++ b/crates/beacon_state/tile/src/error.rs @@ -1,4 +1,4 @@ -use silver_beacon_state_data::{B256, BLSPubkey, BlockBodyError, Epoch, Slot}; +use silver_beacon_state_data::{B256, BLSPubkey, BlockBodyError, Slot}; use thiserror::Error; use crate::tile::Feedback; @@ -19,8 +19,10 @@ pub enum PrecheckError { b256_hex(block_root) )] ParentInvalid { parent_root: B256, block_root: B256 }, - #[error("past block: block_epoch={block_epoch} finalized_epoch={finalized_epoch}")] - PreFinalized { block_epoch: Epoch, finalized_epoch: Epoch }, + #[error("past block: block_slot={block_slot} finalized_slot={finalized_slot}")] + PreFinalized { block_slot: Slot, finalized_slot: Slot }, + #[error("block body is not canonical SSZ: block_slot={block_slot} body_len={body_len}")] + NonCanonicalBody { block_slot: Slot, body_len: usize }, #[error("block past-slot precheck failed: block_slot={block_slot} parent_slot={parent_slot}")] PastSlot { block_slot: Slot, parent_slot: Slot }, #[error("block already imported: block_root=0x{}", b256_hex(block_root))] @@ -51,12 +53,6 @@ pub enum PrecheckError { b256_hex(block_root) )] PayloadTimestamp { expected: u64, got: u64, block_root: B256 }, - #[error( - "block execution payload too short for its fork: len={len} min={min} \ - block_root=0x{}", - b256_hex(block_root) - )] - PayloadTooShort { len: usize, min: usize, block_root: B256 }, #[error( "block blob commitment count precheck failed: got={got} max={max} \ block_root=0x{}", @@ -70,7 +66,7 @@ pub enum PrecheckError { impl PrecheckError { pub fn feedback(self) -> Feedback { match self { - Self::SizeMismatch { .. } => Feedback::Reject(None), + Self::SizeMismatch { .. } | Self::NonCanonicalBody { .. } => Feedback::Reject(None), Self::ParentMissing { parent_root, block_root, .. } => { Feedback::RequestParent { parent_root, block_root } } @@ -85,7 +81,6 @@ impl PrecheckError { Self::ProposerLookaheadMismatch { block_root, .. } | Self::ProposerIndexTooBig { block_root, .. } | Self::PayloadTimestamp { block_root, .. } | - Self::PayloadTooShort { block_root, .. } | Self::TooManyCommitments { block_root, .. } => Feedback::Reject(Some(block_root)), Self::InvalidSignature { block_root } => Feedback::Reject(Some(block_root)), } diff --git a/crates/beacon_state/tile/src/tile.rs b/crates/beacon_state/tile/src/tile.rs index c6863048..c050c356 100644 --- a/crates/beacon_state/tile/src/tile.rs +++ b/crates/beacon_state/tile/src/tile.rs @@ -1,4 +1,4 @@ -use std::{fmt::Debug, sync::Arc, time::Duration}; +use std::{fmt::Debug, sync::Arc}; use flux::{ spine::{FluxSpine, SpineAdapter, SpineProducers}, @@ -14,7 +14,7 @@ use silver_common::{ NewGossipMsg, Origin, ReplayBlock, RequestId, RpcInbound, RpcResponse, RpcResponseInbound, SilverSpine, SyncUpdate, TRandomAccess, TRead, hex32, ssz_view::{MAX_ATTESTATIONS_ELECTRA, MAX_ATTESTING_INDICES, STATUS_V2_SIZE}, - ticker::{SlotTicker, TickEvent}, + ticker::{MAXIMUM_GOSSIP_CLOCK_DISPARITY, SlotTicker, TickEvent}, }; use silver_config::{PendingBounds, SyncingConfig}; @@ -45,9 +45,6 @@ mod seen_validators; mod shuffling_cache; mod sync_contribution_pool; -/// Consensus-spec clock-skew allowance for slot-scoped gossip validation. -const MAXIMUM_GOSSIP_CLOCK_DISPARITY: Duration = Duration::from_millis(500); - #[derive(Clone, Copy, PartialEq, Eq)] pub enum Feedback { Accept(Option), diff --git a/crates/beacon_state/tile/src/tile/block.rs b/crates/beacon_state/tile/src/tile/block.rs index 42dc54cd..073fbe8e 100644 --- a/crates/beacon_state/tile/src/tile/block.rs +++ b/crates/beacon_state/tile/src/tile/block.rs @@ -7,10 +7,13 @@ use silver_beacon_state_data::{ use silver_common::{ BeaconStateEvent, BlockSource, BlockStage, EngineFcuReq, EngineNewPayloadReq, EngineReq, SyncUpdate, TCacheRead, hex32, - ssz_view::{self, SignedBeaconBlockView}, + ssz_view::{self, BeaconBlockBodyFuluView, BeaconBlockBodyGloasView, SignedBeaconBlockView}, }; -use super::{BeaconStateTile, Feedback, ParsedBlock, Producers, gossip::EnvelopeCheck}; +use super::{ + BeaconStateTile, Feedback, MAXIMUM_GOSSIP_CLOCK_DISPARITY, ParsedBlock, Producers, + gossip::EnvelopeCheck, +}; use crate::{ bls, error::PrecheckError, @@ -451,11 +454,6 @@ impl BeaconStateTile { let block_slot = SignedBeaconBlockView::slot(data); let block_epoch = block_slot / SLOTS_PER_EPOCH; - let finalized_epoch = self.fork_choice.finalized_checkpoint.epoch; - if block_epoch < finalized_epoch { - return Err(PrecheckError::PreFinalized { block_epoch, finalized_epoch }); - } - let proposer_index = SignedBeaconBlockView::proposer_index(data); let parent_root = *SignedBeaconBlockView::parent_root(data); let state_root = *SignedBeaconBlockView::state_root(data); @@ -463,6 +461,16 @@ impl BeaconStateTile { let body = SignedBeaconBlockView::body(data); let is_gloas = self.spec.is_gloas_at(block_epoch); + + let canonical = if is_gloas { + BeaconBlockBodyGloasView::check_canonical(body) + } else { + BeaconBlockBodyFuluView::check_canonical(body) + }; + if !canonical { + return Err(PrecheckError::NonCanonicalBody { block_slot, body_len: body.len() }); + } + let body_root = ssz_hash::hash_tree_root_body(body, is_gloas); let block_header = BeaconBlockHeader { @@ -478,6 +486,11 @@ impl BeaconStateTile { return Err(PrecheckError::AlreadyKnown { block_root }); } + let finalized_slot = self.fork_choice.finalized_checkpoint.epoch * SLOTS_PER_EPOCH; + if block_slot <= finalized_slot { + return Err(PrecheckError::PreFinalized { block_slot, finalized_slot }); + } + let Some(parent_idx) = self.fork_choice.find_node_idx(&parent_root) else { let last_applied_slot = self.head_state_slot(); return Err(PrecheckError::ParentMissing { @@ -511,7 +524,7 @@ impl BeaconStateTile { return Err(PrecheckError::PastSlot { block_slot, parent_slot }); } - if self.ticker.is_future_slot(block_slot) { + if block_slot > self.ticker.latest_slot_with_disparity(MAXIMUM_GOSSIP_CLOCK_DISPARITY) { return Err(PrecheckError::FutureSlot { block_slot, wall_slot: self.ticker.current_slot(), @@ -580,16 +593,11 @@ impl BeaconStateTile { block_root: B256, ) -> Result<(), PrecheckError> { let Some(offsets) = BodyOffsets::new(body, BodyFork::Fulu) else { - return Ok(()); // body_root hashing already tolerates a short body + return Err(PrecheckError::NonCanonicalBody { block_slot, body_len: body.len() }); }; - let payload = offsets.payload(); if payload.len() < ssz_view::EXECUTION_PAYLOAD_FIXED { - return Err(PrecheckError::PayloadTooShort { - len: payload.len(), - min: ssz_view::EXECUTION_PAYLOAD_FIXED, - block_root, - }); + return Err(PrecheckError::NonCanonicalBody { block_slot, body_len: body.len() }); } let expected = rv.imm.genesis_time + block_slot * self.spec.seconds_per_slot(); let got = ssz_view::ExecutionPayloadView::timestamp(payload); diff --git a/crates/beacon_state/tile/src/tile/tests.rs b/crates/beacon_state/tile/src/tile/tests.rs index b02d35fb..e7a22a3a 100644 --- a/crates/beacon_state/tile/src/tile/tests.rs +++ b/crates/beacon_state/tile/src/tile/tests.rs @@ -11,10 +11,11 @@ use silver_common::{ GossipTopic, MessageId, P2pStreamId, StreamProtocol, TCache, TCacheProducer, TCacheRead, TProducer, ssz_view::{ - ATTESTATION_DATA_SIZE, AttestationView, BYTES_PER_KZG_COMMITMENT, EXECUTION_PAYLOAD_FIXED, - PROPOSER_SLASHING_SIZE, SIGNED_AGG_PROOF_MIN, SIGNED_BEACON_BLOCK_MIN, - SIGNED_BLS_CHANGE_SIZE, SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, SIGNED_VOLUNTARY_EXIT_SIZE, - SINGLE_ATT_SIZE, SignedAggregateAndProofView, SingleAttestationView, StatusView, + ATTESTATION_DATA_SIZE, AttestationView, BEACON_BLOCK_BODY_FIXED, BYTES_PER_KZG_COMMITMENT, + EXECUTION_PAYLOAD_FIXED, EXECUTION_REQUESTS_FULU_FIXED, PROPOSER_SLASHING_SIZE, + SIGNED_AGG_PROOF_MIN, SIGNED_BEACON_BLOCK_MIN, SIGNED_BLS_CHANGE_SIZE, + SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, SIGNED_VOLUNTARY_EXIT_SIZE, SINGLE_ATT_SIZE, + SignedAggregateAndProofView, SingleAttestationView, StatusView, }, }; use silver_ssz::ssz_view::{EXECUTION_PAYLOAD_ENVELOPE_MIN, SyncCommitteeContributionView}; @@ -22,7 +23,7 @@ use silver_ssz::ssz_view::{EXECUTION_PAYLOAD_ENVELOPE_MIN, SyncCommitteeContribu use super::*; use crate::{ fork_choice::{BlockImport, PayloadStatus}, - merkle, + merkle, ssz_hash, stf::AttestationVote, test_signing, }; @@ -30,23 +31,46 @@ use crate::{ const MAX_EFFECTIVE_BALANCE: u64 = 32_000_000_000; const ANCHOR_ROOT: B256 = [0x01u8; 32]; -/// Zeroed `SignedBeaconBlock` with the offsets `SignedBeaconBlockView` pins: -/// message at 100, body at 184. Fields go in at their fixed offsets. -fn empty_block(len: usize) -> Vec { - assert!(len >= SIGNED_BEACON_BLOCK_MIN); - let mut bytes = vec![0u8; len]; - bytes[0..4].copy_from_slice(&100u32.to_le_bytes()); - bytes[180..184].copy_from_slice(&84u32.to_le_bytes()); - bytes +/// Byte position of the body inside a `SignedBeaconBlock`. +const BODY: usize = SIGNED_BEACON_BLOCK_MIN; + +fn put_u32(b: &mut [u8], at: usize, v: usize) { + b[at..at + 4].copy_from_slice(&(v as u32).to_le_bytes()); +} + +/// Smallest canonical Fulu `SignedBeaconBlock`: every list empty, the +/// execution payload at its fixed size, and every offset table pointing where +/// a strict decoder expects. Fields go in at their fixed offsets. +fn empty_block() -> Vec { + let payload_end = BEACON_BLOCK_BODY_FIXED + EXECUTION_PAYLOAD_FIXED; + let mut b = vec![0u8; BODY + payload_end + EXECUTION_REQUESTS_FULU_FIXED]; + put_u32(&mut b, 0, 100); + put_u32(&mut b, 180, 84); + for off in [200, 204, 208, 212, 216, 380] { + put_u32(&mut b, BODY + off, BEACON_BLOCK_BODY_FIXED); + } + for off in [384, 388, 392] { + put_u32(&mut b, BODY + off, payload_end); + } + let payload = BODY + BEACON_BLOCK_BODY_FIXED; + for off in [436, 504, 508] { + put_u32(&mut b, payload + off, EXECUTION_PAYLOAD_FIXED); + } + let requests = BODY + payload_end; + for off in [0, 4, 8] { + put_u32(&mut b, requests + off, EXECUTION_REQUESTS_FULU_FIXED); + } + b } /// Zeroed `SignedAggregateAndProof` with the three offsets -/// `SignedAggregateAndProofView` pins. +/// `SignedAggregateAndProofView` pins and an empty, well-formed bitlist. fn empty_aggregate() -> Vec { - let mut buf = vec![0u8; SIGNED_AGG_PROOF_MIN]; + let mut buf = vec![0u8; SIGNED_AGG_PROOF_MIN + 1]; buf[0..4].copy_from_slice(&100u32.to_le_bytes()); buf[108..112].copy_from_slice(&108u32.to_le_bytes()); buf[208..212].copy_from_slice(&236u32.to_le_bytes()); + buf[SIGNED_AGG_PROOF_MIN] = 0b1; // bitlist sentinel buf } @@ -117,7 +141,7 @@ fn make_tile_with_gossip(wall_slot: u64) -> (BeaconStateTile, TProducer, TProduc /// Publish a minimal block (slot at offset 100) into `producer` and wrap it /// as a buffered gossip orphan whose slot the tile can read back. fn gossip_pending(producer: &mut TProducer, slot: u64) -> PendingBlock { - let mut bytes = empty_block(200); + let mut bytes = empty_block(); bytes[100..108].copy_from_slice(&slot.to_le_bytes()); let mut r = producer.reserve(bytes.len(), true).expect("reserve"); if let Ok(buf) = r.buffer() { @@ -389,7 +413,7 @@ fn block_unknown_parent_rejected() { // Minimal SignedBeaconBlock: message at fixed offset 100 (4-byte // offset + 96-byte signature), parent_root @ 116 set to an unknown // root so precheck bails with ParentMissing before any state change. - let mut buf = empty_block(200); + let mut buf = empty_block(); buf[100..108].copy_from_slice(&11u64.to_le_bytes()); // slot buf[108..116].copy_from_slice(&0u64.to_le_bytes()); // proposer_index buf[116] = 0xFF; // parent_root[0] @@ -428,9 +452,10 @@ fn short_gossip_block_rejected_before_any_field_read() { assert!(matches!(feedback, Feedback::Reject(None)), "len {len}: {feedback:?}"); } - // Control: at the minimum length the gate lets the block through, so the - // assertions above are about the bound and not about a blanket reject. - let mut bytes = empty_block(SIGNED_BEACON_BLOCK_MIN); + // Control: a well-formed block gets through the gate, so the assertions + // above are about the bound and not about a blanket reject. + let mut bytes = empty_block(); + bytes[100..108].copy_from_slice(&11u64.to_le_bytes()); bytes[116] = 0xFF; // unknown parent_root let (data, read) = publish_block_bytes(&mut gp, &bytes); let feedback = @@ -500,36 +525,105 @@ fn payload_timestamp_and_blob_count_are_checked_before_relay() { } } -/// Fulu block whose body is long enough for `BodyOffsets`, carrying an -/// execution payload at its fixed size followed by `n` blob commitments. -/// Every other variable field is empty. +/// `empty_block` on the anchor at `slot`, its payload stamped `timestamp` and +/// followed by `n_commitments` (zeroed) blob commitments. fn fulu_block_with_payload(slot: u64, timestamp: u64, n_commitments: usize) -> Vec { - const BODY: usize = 184; - const FIXED: usize = 396; - let payload_end = FIXED + EXECUTION_PAYLOAD_FIXED; - let body_len = payload_end + n_commitments * BYTES_PER_KZG_COMMITMENT; - - let mut b = empty_block(BODY + body_len); + let mut b = empty_block(); b[100..108].copy_from_slice(&slot.to_le_bytes()); b[116..148].copy_from_slice(&ANCHOR_ROOT); - // Empty lists all point at the end of the fixed part. - for off in [200usize, 204, 208, 212, 216] { - b[BODY + off..BODY + off + 4].copy_from_slice(&(FIXED as u32).to_le_bytes()); - } - let put = |b: &mut Vec, off: usize, v: usize| { - b[BODY + off..BODY + off + 4].copy_from_slice(&(v as u32).to_le_bytes()); - }; - put(&mut b, 380, FIXED); // execution_payload - put(&mut b, 384, payload_end); // bls_to_execution_changes - put(&mut b, 388, payload_end); // blob_kzg_commitments - put(&mut b, 392, body_len); // execution_requests - - let payload = BODY + FIXED; + let payload = BODY + BEACON_BLOCK_BODY_FIXED; b[payload + 428..payload + 436].copy_from_slice(×tamp.to_le_bytes()); + + let commitments_at = payload + EXECUTION_PAYLOAD_FIXED; + let commitments_len = n_commitments * BYTES_PER_KZG_COMMITMENT; + b.splice(commitments_at..commitments_at, std::iter::repeat_n(0u8, commitments_len)); + put_u32(&mut b, BODY + 392, commitments_at - BODY + commitments_len); // execution_requests + b +} + +/// Non-canonical re-encoding of a Fulu block with the same field contents: +/// a 4-byte gap between the body's fixed part and its first list, every body +/// offset shifted past it. +fn body_with_gap(block: &[u8]) -> Vec { + let mut b = block.to_vec(); + let gap = BODY + BEACON_BLOCK_BODY_FIXED; + b.splice(gap..gap, [0u8; 4]); + for off in [200, 204, 208, 212, 216, 380, 384, 388, 392] { + let at = BODY + off; + let v = u32::from_le_bytes(b[at..at + 4].try_into().unwrap()) as usize; + put_u32(&mut b, at, v + 4); + } b } +/// The gap re-encoding hashes to the honest body root, so the proposer's +/// signature still verifies over it; only the canonical check stands between +/// it and a relay that every strict peer would penalise. +#[test] +fn non_canonical_body_is_rejected_before_relay() { + const PARENT_SLOT: u64 = 10; + let slot = PARENT_SLOT + 1; + let stamp = slot * SpecConfig::mainnet().seconds_per_slot(); + + let honest = fulu_block_with_payload(slot, stamp, 0); + let gapped = body_with_gap(&honest); + assert_eq!( + ssz_hash::hash_tree_root_body_fulu(&honest[BODY..]), + ssz_hash::hash_tree_root_body_fulu(&gapped[BODY..]), + "the attack premise: the gap is invisible to the hash" + ); + + for (bytes, want_reject) in [(honest, false), (gapped, true)] { + let (mut tile, mut gp, _rp, _spine, mut adapter) = tile_with_producers(PARENT_SLOT + 2); + seed_tile(&mut tile, 4, PARENT_SLOT); + tile.sync_target = SyncUpdate::Following; + let (data, read) = publish_block_bytes(&mut gp, &bytes); + + let mut relayed = false; + let feedback = tile.apply_block( + &data, + read, + BlockSource::Gossip, + true, + &mut adapter.producers, + |_| relayed = true, + ); + + assert_eq!(matches!(feedback, Feedback::Reject(None)), want_reject, "{feedback:?}"); + assert_eq!(relayed, !want_reject, "relay must follow the canonical check"); + } +} + +/// Spec: a block must sit strictly after the finalized checkpoint's start +/// slot, not merely in or after its epoch. +#[test] +fn block_at_the_finalized_start_slot_is_ignored() { + const PARENT_SLOT: u64 = 31; + let finalized_slot = SLOTS_PER_EPOCH; + + for (slot, want_ignore) in [(finalized_slot, true), (finalized_slot + 1, false)] { + let (mut tile, mut gp, _rp, _spine, mut adapter) = tile_with_producers(slot + 1); + seed_tile(&mut tile, 4, PARENT_SLOT); + tile.sync_target = SyncUpdate::Following; + tile.fork_choice.finalized_checkpoint.epoch = 1; + + let stamp = slot * SpecConfig::mainnet().seconds_per_slot(); + let bytes = fulu_block_with_payload(slot, stamp, 0); + let (data, read) = publish_block_bytes(&mut gp, &bytes); + let feedback = tile.apply_block( + &data, + read, + BlockSource::Gossip, + true, + &mut adapter.producers, + |_| {}, + ); + + assert_eq!(matches!(feedback, Feedback::Ignore), want_ignore, "slot {slot}: {feedback:?}"); + } +} + /// `proposer_lookahead` reaches only the parent's current and next epoch. A /// block past a long skipped-slot run has no entry, so silver cannot check its /// proposer — it still imports, but must not go on the wire as if it had. @@ -550,9 +644,8 @@ fn block_past_the_lookahead_window_imports_but_is_not_relayed() { seed_tile(&mut tile, 4, PARENT_SLOT); tile.sync_target = SyncUpdate::Following; - let mut bytes = empty_block(200); - bytes[100..108].copy_from_slice(&slot.to_le_bytes()); - bytes[116..148].copy_from_slice(&ANCHOR_ROOT); + let stamp = slot * SpecConfig::mainnet().seconds_per_slot(); + let bytes = fulu_block_with_payload(slot, stamp, 0); let (data, read) = publish_block_bytes(&mut gp, &bytes); let mut relayed = false; @@ -634,7 +727,7 @@ fn buffer_orphan_idx( /// Signed block just well-formed enough to reach the parent lookup: the /// message's slot sits at [100..108) and its parent root at [116..148). fn rpc_block(producer: &mut TProducer, slot: u64, parent_root: B256) -> silver_common::TCacheRead { - let mut bytes = empty_block(200); + let mut bytes = empty_block(); bytes[100..108].copy_from_slice(&slot.to_le_bytes()); bytes[116..148].copy_from_slice(&parent_root); let mut r = producer.reserve(bytes.len(), true).expect("reserve"); @@ -1031,7 +1124,7 @@ fn block_known_parent_bad_sig_rejected() { // Valid structure, zeroed BLS signature → precheck reaches and fails // signature verification, so no fork-choice node is added. - let mut buf = empty_block(200); + let mut buf = empty_block(); buf[100..108].copy_from_slice(&11u64.to_le_bytes()); // slot buf[108..116].copy_from_slice(&0u64.to_le_bytes()); // proposer_index buf[116..148].copy_from_slice(&parent_root); // parent_root diff --git a/crates/beacon_state/tile/tests/ssz_view_fixtures.rs b/crates/beacon_state/tile/tests/ssz_view_fixtures.rs index 2f3fbcdc..a8b67e96 100644 --- a/crates/beacon_state/tile/tests/ssz_view_fixtures.rs +++ b/crates/beacon_state/tile/tests/ssz_view_fixtures.rs @@ -21,8 +21,16 @@ fn snappy_decode(path: &Path) -> Vec { } fn cases_for(container: &str) -> Vec<(PathBuf, Vec, Value)> { - let dir = - spec_tests_dir().join("tests/mainnet/fulu/ssz_static").join(container).join("ssz_random"); + cases_for_fork("fulu", container) +} + +fn cases_for_fork(fork: &str, container: &str) -> Vec<(PathBuf, Vec, Value)> { + let dir = spec_tests_dir() + .join("tests/mainnet") + .join(fork) + .join("ssz_static") + .join(container) + .join("ssz_random"); let mut dirs: Vec = fs::read_dir(&dir) .unwrap_or_else(|e| panic!("{}: {e}", dir.display())) .filter_map(|e| e.ok().map(|e| e.path()).filter(|p| p.is_dir())) diff --git a/crates/columns/src/sync.rs b/crates/columns/src/sync.rs index cf23213b..a87490de 100644 --- a/crates/columns/src/sync.rs +++ b/crates/columns/src/sync.rs @@ -1,31 +1,14 @@ -use std::time::Instant; - use silver_beacon_state_data::SLOTS_PER_EPOCH; -use silver_common::{ - SyncUpdate, merkle::B256, ssz_view::StatusView, ticker::MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS, -}; +use silver_common::{SyncUpdate, merkle::B256, ssz_view::StatusView}; -#[derive(Debug)] +#[derive(Debug, Default)] pub(crate) struct SyncStatus { sync_target: SyncUpdate, wall_slot: u64, - wall_slot_at: Instant, head_root: B256, finalized_slot: u64, } -impl Default for SyncStatus { - fn default() -> Self { - Self { - sync_target: SyncUpdate::default(), - wall_slot: 0, - wall_slot_at: Instant::now(), - head_root: B256::default(), - finalized_slot: 0, - } - } -} - impl SyncStatus { pub(crate) fn is_synced(&self) -> bool { self.sync_target.is_following() @@ -47,17 +30,8 @@ impl SyncStatus { &self.head_root } - pub(crate) fn is_future_slot(&self, slot: u64, slot_ms: u64) -> bool { - let ms_into_slot = self.wall_slot_at.elapsed().as_millis() as u64; - slot.saturating_mul(slot_ms) > - self.wall_slot.saturating_mul(slot_ms) + - ms_into_slot + - MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS - } - pub(crate) fn update(&mut self, ssz: [u8; 92], wall_slot: u64) { self.wall_slot = wall_slot; - self.wall_slot_at = Instant::now(); self.head_root = *StatusView::head_root(&ssz); self.finalized_slot = StatusView::finalized_epoch(&ssz) * SLOTS_PER_EPOCH; } diff --git a/crates/columns/src/validate.rs b/crates/columns/src/validate.rs index 8f4f6af1..9c9f6e52 100644 --- a/crates/columns/src/validate.rs +++ b/crates/columns/src/validate.rs @@ -69,6 +69,13 @@ enum ParentCheck { NotExtending { parent_slot: u64 }, } +impl ParentCheck { + /// Fulu requires a sidecar to be proposed strictly after its parent block. + fn extending(slot: u64, parent_slot: u64) -> Self { + if slot > parent_slot { Self::Seen } else { Self::NotExtending { parent_slot } } + } +} + /// Per-sidecar validation, i.e. everything except the KZG cell proofs — /// those are deferred to the end-of-pass batch, so `Record` means "passed /// every check but KZG". Owns the caches only validation consults. @@ -105,16 +112,6 @@ impl ColumnValidator { self.persisted_block_roots.insert(block_root, slot); } - /// Fulu requires a sidecar to be proposed strictly after its parent block. - fn check_parent(&self, parent_root: &BlockRoot, slot: u64, in_head_fork: bool) -> ParentCheck { - match self.persisted_block_roots.get(parent_root) { - Some(&parent_slot) if slot <= parent_slot => ParentCheck::NotExtending { parent_slot }, - Some(_) => ParentCheck::Seen, - None if in_head_fork => ParentCheck::Seen, - None => ParentCheck::Unseen, - } - } - pub fn gloas_commitments(&self, block_root: &BlockRoot) -> Option<&[u8]> { self.gloas_commitments.get(block_root).map(|c| c.as_ref()) } @@ -178,7 +175,7 @@ impl ColumnValidator { ); } - if sync_state.is_synced() && sync_state.is_future_slot(slot, self.spec.slot_duration_ms()) { + if sync_state.is_synced() && slot > sync_state.wall_slot().saturating_add(1) { tracing::debug!( ?stream_id, slot, @@ -223,6 +220,7 @@ impl ColumnValidator { // BLS verify runs OUTSIDE the closure (slow; would hold the // notional read lock too long otherwise). let claimed_proposer_index = DataColumnSidecarFuluView::proposer_index(buffer); + let persisted_parent_slot = self.persisted_block_roots.get(parent_root).copied(); let checks = self.beacon_state.read(&|v| { let state_epoch = v.slot.current_epoch(); // proposer_lookahead is anchored to `state_epoch` and covers @@ -233,8 +231,14 @@ impl ColumnValidator { Some(_) => ProposerCheck::Mismatch, None => ProposerCheck::Unresolvable, }; - let parent_in_head_fork = parent_root == sync_state.head_root() || - v.block_roots.contains(parent_root, v.slot.slot_number()); + let parent = match persisted_parent_slot { + Some(parent_slot) => ParentCheck::extending(slot, parent_slot), + None if parent_root == sync_state.head_root() => ParentCheck::Seen, + None => match v.block_roots.slot_of(parent_root, v.slot.slot_number()) { + Some(parent_slot) => ParentCheck::extending(slot, parent_slot), + None => ParentCheck::Unseen, + }, + }; let is_above_finalized = util::is_above_finalized(buffer, v.epoch.state().finalized_checkpoint.epoch); @@ -244,7 +248,7 @@ impl ColumnValidator { ( is_above_finalized, - parent_in_head_fork, + parent, proposer, pubkey, v.epoch.fork().current_version, // TODO for backfill @@ -252,9 +256,7 @@ impl ColumnValidator { ) }); // No snapshot yet (pre-bootstrap): nothing can be validated. - let Some((above_finalized, parent_in_head_fork, proposer, pubkey, fork_version, gvr)) = - checks - else { + let Some((above_finalized, parent, proposer, pubkey, fork_version, gvr)) = checks else { tracing::warn!(?stream_id, "sidecar before first beacon state snapshot"); return ColumnOutcome::Reject { block_root, slot, bitmask: column_bitmask }; }; @@ -263,7 +265,7 @@ impl ColumnValidator { tracing::warn!(?stream_id, "sidecar slot at or below finalized — ignoring"); return ColumnOutcome::Skip; } - match self.check_parent(parent_root, slot, parent_in_head_fork) { + match parent { ParentCheck::Seen => {} ParentCheck::Unseen => { tracing::warn!( @@ -329,7 +331,7 @@ impl ColumnValidator { let PendingColumn { stream_id, gossip_subnet, .. } = *column; let slot = DataColumnSidecarGloasView::slot(buffer); - if sync_state.is_synced() && sync_state.is_future_slot(slot, self.spec.slot_duration_ms()) { + if sync_state.is_synced() && slot > sync_state.wall_slot().saturating_add(1) { tracing::debug!( ?stream_id, slot, diff --git a/crates/common/src/ticker.rs b/crates/common/src/ticker.rs index 0bce28f8..37eafca6 100644 --- a/crates/common/src/ticker.rs +++ b/crates/common/src/ticker.rs @@ -2,7 +2,8 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; type Slot = u64; -pub const MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS: u64 = 500; +/// Consensus-spec clock-skew allowance for slot-scoped gossip validation. +pub const MAXIMUM_GOSSIP_CLOCK_DISPARITY: Duration = Duration::from_millis(500); // slot/24 before next slot = 500ms on mainnet. const FORK_CHOICE_LOOKAHEAD_DIVISOR: u64 = 24; @@ -111,11 +112,24 @@ impl SlotTicker { self.millis_since_genesis() / self.slot_ms } - /// Spec gossip rule: a block or sidecar from a future slot is IGNOREd, - /// with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance. - pub fn is_future_slot(&self, slot: Slot) -> bool { - slot.saturating_mul(self.slot_ms) > - self.millis_since_genesis() + MAXIMUM_GOSSIP_CLOCK_DISPARITY_MS + /// Whether `slot` is current after extending both ends of its wall-clock + /// interval by `disparity`. Consensus gossip validation uses this for its + /// permitted clock skew around slot boundaries. + pub fn is_current_slot_with_disparity(&self, slot: Slot, disparity: Duration) -> bool { + let now = self.millis_since_genesis(); + let disparity_ms = u64::try_from(disparity.as_millis()).unwrap_or(u64::MAX); + let start = slot.saturating_mul(self.slot_ms); + let end = slot.saturating_add(1).saturating_mul(self.slot_ms); + now.saturating_add(disparity_ms) >= start && now <= end.saturating_add(disparity_ms) + } + + /// The newest slot admitted by [`Self::is_current_slot_with_disparity`]. + /// A two-slot seen-cache can rotate to this value and retain every slot + /// that is valid at the current instant, including across the boundary. + pub fn latest_slot_with_disparity(&self, disparity: Duration) -> Slot { + let wall = self.current_slot(); + let next = wall.saturating_add(1); + if self.is_current_slot_with_disparity(next, disparity) { next } else { wall } } pub fn is_before_attesting_interval(&self, is_gloas: bool) -> bool { @@ -231,4 +245,25 @@ mod tests { let ev = t.tick(); assert!(matches!(ev, TickEvent::SlotStart(3))); } + + #[test] + fn current_slot_disparity_covers_both_boundary_sides() { + let mut t = SlotTicker::new(0, Duration::from_secs(12), Duration::from_secs(4)); + let disparity = Duration::from_millis(500); + + t.set_since_genesis_ms(12_000 + 499); + assert!(t.is_current_slot_with_disparity(0, disparity)); + assert!(t.is_current_slot_with_disparity(1, disparity)); + assert!(!t.is_current_slot_with_disparity(2, disparity)); + assert_eq!(t.latest_slot_with_disparity(disparity), 1); + + t.set_since_genesis_ms(24_000 - 500); + assert!(t.is_current_slot_with_disparity(1, disparity)); + assert!(t.is_current_slot_with_disparity(2, disparity)); + assert_eq!(t.latest_slot_with_disparity(disparity), 2); + + t.set_since_genesis_ms(24_000 - 501); + assert!(!t.is_current_slot_with_disparity(2, disparity)); + assert_eq!(t.latest_slot_with_disparity(disparity), 1); + } } diff --git a/crates/network/src/p2p/streams/rpc/reservation.rs b/crates/network/src/p2p/streams/rpc/reservation.rs index 0b41d161..6cb2aeae 100644 --- a/crates/network/src/p2p/streams/rpc/reservation.rs +++ b/crates/network/src/p2p/streams/rpc/reservation.rs @@ -6,11 +6,12 @@ use std::{ use silver_common::{ P2pStreamId, RpcRequest, RpcResponse, StreamProtocol, TCacheProducer, TProducer, TReservation, ssz_view::{ - BLOCKS_BY_RANGE_REQ_SIZE, DATA_COLUMN_SIDECAR_GLOAS_MIN, DATA_COLUMN_SIDECAR_MAX, - DC_BY_RANGE_REQ_MAX, DC_BY_RANGE_REQ_MIN, DC_BY_ROOT_SINGLE_SIZE, - EXECUTION_PAYLOAD_ENVELOPES_BY_RANGE_REQ_SIZE, GOODBYE_SIZE, MAX_PAYLOAD_SIZE, - METADATA_SIZE, PING_SIZE, SIGNED_BEACON_BLOCK_MAX, SIGNED_BEACON_BLOCK_MIN, - SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, STATUS_V1_SIZE, STATUS_V2_SIZE, + BLOCKS_BY_RANGE_REQ_SIZE, BLOCKS_BY_ROOT_REQ_MAX, DATA_COLUMN_SIDECAR_GLOAS_MIN, + DATA_COLUMN_SIDECAR_MAX, DC_BY_RANGE_REQ_MAX, DC_BY_RANGE_REQ_MIN, DC_BY_ROOT_SINGLE_SIZE, + EXECUTION_PAYLOAD_ENVELOPES_BY_RANGE_REQ_SIZE, EXECUTION_PAYLOAD_ENVELOPES_BY_ROOT_REQ_MAX, + GOODBYE_SIZE, MAX_PAYLOAD_SIZE, METADATA_SIZE, PING_SIZE, SIGNED_BEACON_BLOCK_MAX, + SIGNED_BEACON_BLOCK_MIN, SIGNED_EXECUTION_PAYLOAD_ENVELOPE_MIN, STATUS_V1_SIZE, + STATUS_V2_SIZE, }, }; @@ -29,10 +30,11 @@ fn payload_bounds(id: &P2pStreamId) -> RangeInclusive { exact(EXECUTION_PAYLOAD_ENVELOPES_BY_RANGE_REQ_SIZE) } StreamProtocol::DataColumnSidecarsByRange => DC_BY_RANGE_REQ_MIN..=DC_BY_RANGE_REQ_MAX, - // `List[Root, MAX_REQUEST_BLOCKS]` and its column/envelope - // siblings: whole elements, and the tcache caps the total. - StreamProtocol::BeaconBlocksByRoot | - StreamProtocol::ExecutionPayloadEnvelopesByRoot => 32..=MAX_PAYLOAD_SIZE, + StreamProtocol::BeaconBlocksByRoot => 32..=BLOCKS_BY_ROOT_REQ_MAX, + StreamProtocol::ExecutionPayloadEnvelopesByRoot => { + 32..=EXECUTION_PAYLOAD_ENVELOPES_BY_ROOT_REQ_MAX + } + // Variable-size identifiers, so only the tcache caps the total. StreamProtocol::DataColumnSidecarsByRoot => DC_BY_ROOT_SINGLE_SIZE..=MAX_PAYLOAD_SIZE, _ => 0..=0, } @@ -60,13 +62,22 @@ fn payload_bounds(id: &P2pStreamId) -> RangeInclusive { } } +/// Requests that are a `List[Root, _]`: the length must be whole roots. +fn is_root_list_request(id: &P2pStreamId) -> bool { + id.is_incoming() && + matches!( + id.protocol(), + StreamProtocol::BeaconBlocksByRoot | StreamProtocol::ExecutionPayloadEnvelopesByRoot + ) +} + pub fn alloc_incoming_rpc( rpc_in: &mut TProducer, id: &P2pStreamId, len: usize, ) -> Result { let bounds = payload_bounds(id); - if !bounds.contains(&len) { + if !bounds.contains(&len) || (is_root_list_request(id) && !len.is_multiple_of(32)) { tracing::warn!( ?id, len, diff --git a/crates/ssz/src/ssz_hash_gloas/block_body.rs b/crates/ssz/src/ssz_hash_gloas/block_body.rs index aa908a14..1449ca55 100644 --- a/crates/ssz/src/ssz_hash_gloas/block_body.rs +++ b/crates/ssz/src/ssz_hash_gloas/block_body.rs @@ -6,9 +6,13 @@ use crate::{ ssz_hash::{hash_eth1_data_bytes, hash_sync_aggregate}, ssz_hash_gloas::ExecutionRequestsView, ssz_view::{ - AttestationView, AttesterSlashingView, BeaconBlockBodyGloasView, DepositView, - PayloadAttestationView, ProposerSlashingView, SignedBlsToExecutionChangeView, - SignedExecutionPayloadBidView, SignedVoluntaryExitView, + AttestationView, AttesterSlashingView, BEACON_BLOCK_BODY_FIXED, BYTES_PER_KZG_COMMITMENT, + BeaconBlockBodyFuluView, BeaconBlockBodyGloasView, DEPOSIT_SIZE, DepositView, + MAX_BLOB_COMMITMENTS_PER_BLOCK, PAYLOAD_ATTESTATION_SIZE, PROPOSER_SLASHING_SIZE, + PayloadAttestationView, ProposerSlashingView, SIGNED_BLS_CHANGE_SIZE, + SIGNED_EXECUTION_PAYLOAD_BID_MIN, SIGNED_VOLUNTARY_EXIT_SIZE, + SignedBlsToExecutionChangeView, SignedExecutionPayloadBidView, SignedVoluntaryExitView, + fixed_list_ok, offsets_ok, variable_field, variable_list_ok, }, }; @@ -17,6 +21,32 @@ impl ProgressiveContainer for BeaconBlockBodyGloasView { } impl BeaconBlockBodyGloasView { + pub fn check_canonical(body: &[u8]) -> bool { + const OFFSETS: [usize; 9] = BeaconBlockBodyFuluView::VARIABLE_OFFSETS; + const UNCAPPED: usize = usize::MAX; + if body.len() < BEACON_BLOCK_BODY_FIXED || + !offsets_ok(body, &OFFSETS, BEACON_BLOCK_BODY_FIXED) + { + return false; + } + let field = |i: usize| variable_field(body, &OFFSETS, i); + let bid = field(6); + fixed_list_ok(field(0), PROPOSER_SLASHING_SIZE, UNCAPPED) && + variable_list_ok(field(1), UNCAPPED, AttesterSlashingView::check_size) && + variable_list_ok(field(2), UNCAPPED, AttestationView::check_canonical) && + fixed_list_ok(field(3), DEPOSIT_SIZE, UNCAPPED) && + fixed_list_ok(field(4), SIGNED_VOLUNTARY_EXIT_SIZE, UNCAPPED) && + fixed_list_ok(field(5), SIGNED_BLS_CHANGE_SIZE, UNCAPPED) && + SignedExecutionPayloadBidView::check_size(bid) && + fixed_list_ok( + &bid[SIGNED_EXECUTION_PAYLOAD_BID_MIN..], + BYTES_PER_KZG_COMMITMENT, + MAX_BLOB_COMMITMENTS_PER_BLOCK, + ) && + fixed_list_ok(field(7), PAYLOAD_ATTESTATION_SIZE, UNCAPPED) && + ExecutionRequestsView::check_canonical(field(8)) + } + #[timed] pub fn hash_tree_root(body: &[u8]) -> B256 { match Self::field_roots(body) { diff --git a/crates/ssz/src/ssz_hash_gloas/execution_requests.rs b/crates/ssz/src/ssz_hash_gloas/execution_requests.rs index 015464e1..ece90fdf 100644 --- a/crates/ssz/src/ssz_hash_gloas/execution_requests.rs +++ b/crates/ssz/src/ssz_hash_gloas/execution_requests.rs @@ -7,8 +7,9 @@ use crate::{ progressive::{ProgressiveContainer, ProgressiveHasher, packed_active_fields}, ssz_view::{ BUILDER_DEPOSIT_REQUEST_SIZE, BUILDER_EXIT_REQUEST_SIZE, BuilderDepositRequestView, - BuilderExitRequestView, ConsolidationRequestView, DepositRequestView, - WithdrawalRequestView, + BuilderExitRequestView, CONSOLIDATION_REQUEST_SIZE, ConsolidationRequestView, + DEPOSIT_REQUEST_SIZE, DepositRequestView, WITHDRAWAL_REQUEST_SIZE, WithdrawalRequestView, + fixed_list_ok, offsets_ok, variable_field, }, }; @@ -45,6 +46,22 @@ impl ExecutionRequestsView { out } + pub fn check_canonical(data: &[u8]) -> bool { + const OFFSETS: [usize; 5] = [0, 4, 8, 12, 16]; + const ELEMENT_SIZES: [usize; 5] = [ + DEPOSIT_REQUEST_SIZE, + WITHDRAWAL_REQUEST_SIZE, + CONSOLIDATION_REQUEST_SIZE, + BUILDER_DEPOSIT_REQUEST_SIZE, + BUILDER_EXIT_REQUEST_SIZE, + ]; + data.len() >= 20 && + offsets_ok(data, &OFFSETS, 20) && + ELEMENT_SIZES.iter().enumerate().all(|(i, &elem)| { + fixed_list_ok(variable_field(data, &OFFSETS, i), elem, usize::MAX) + }) + } + #[timed] pub fn hash_tree_root(data: &[u8]) -> B256 { let [deposits, withdrawals, consolidations, builder_deposits, builder_exits] = diff --git a/crates/ssz/src/ssz_view.rs b/crates/ssz/src/ssz_view.rs index 52d0544f..7d859046 100644 --- a/crates/ssz/src/ssz_view.rs +++ b/crates/ssz/src/ssz_view.rs @@ -49,6 +49,64 @@ fn fixed(buf: &[u8], off: usize) -> &[u8; N] { buf[off..off + N].try_into().unwrap() } +// -- Canonical-encoding checks ----------------------------------------- +// +// SSZ has exactly one encoding per value, and strict decoders (the other +// clients) reject anything else. Hashing does not: a gap after an offset table +// or a padded list hashes like the honest bytes, so a message can keep a valid +// signature while every strict peer rejects it and penalises whoever relayed +// it. These helpers pin what the hash leaves loose. + +/// `List[T, max]` of fixed-size elements: whole elements, at most `max`. +#[inline] +pub(crate) fn fixed_list_ok(data: &[u8], elem: usize, max: usize) -> bool { + data.len().is_multiple_of(elem) && data.len() / elem <= max +} + +/// `List[T, max]` of variable-size elements: the first offset is the table's +/// own length, offsets never decrease, the last element ends at the end of +/// `data`, and every element passes `elem_ok`. +pub(crate) fn variable_list_ok(data: &[u8], max: usize, elem_ok: impl Fn(&[u8]) -> bool) -> bool { + if data.is_empty() { + return true; + } + if data.len() < 4 { + return false; + } + let table_len = u32_le(data, 0) as usize; + if table_len == 0 || !table_len.is_multiple_of(4) || table_len > data.len() { + return false; + } + let count = table_len / 4; + if count > max { + return false; + } + let mut start = table_len; + for i in 1..=count { + let end = if i < count { u32_le(data, i * 4) as usize } else { data.len() }; + if end < start || end > data.len() || !elem_ok(&data[start..end]) { + return false; + } + start = end; + } + true +} + +/// A container's variable-field offsets, read at `positions` in field order: +/// the first equals the fixed part's length, they never decrease, and none +/// points past `buf`. +pub(crate) fn offsets_ok(buf: &[u8], positions: &[usize], fixed_len: usize) -> bool { + let mut prev = fixed_len; + for (i, &pos) in positions.iter().enumerate() { + let off = u32_le(buf, pos) as usize; + if (i == 0 && off != fixed_len) || off < prev || off > buf.len() { + return false; + } + prev = off; + } + true +} + // -- Spec size bounds (Fulu) ------------------------------------------ // // Global cap on any uncompressed gossip/RPC payload. Per-type SSZ bounds @@ -243,6 +301,16 @@ impl AttestationView { pub fn aggregation_bits(buf: &[u8]) -> &[u8] { &buf[ATTESTATION_FIXED..] } + + pub fn check_canonical(buf: &[u8]) -> bool { + if buf.len() <= ATTESTATION_FIXED || u32_le(buf, 0) as usize != ATTESTATION_FIXED { + return false; + } + let bits = &buf[ATTESTATION_FIXED..]; + let last = bits[bits.len() - 1]; + last != 0 && + (bits.len() - 1) * 8 + 7 - last.leading_zeros() as usize <= MAX_ATTESTING_INDICES + } } // -- ProposerSlashing (proposer_slashing) ---------------------------- @@ -740,6 +808,43 @@ impl BeaconBlockBodyFuluView { pub fn execution_requests_offset(buf: &[u8]) -> u32 { u32_le(buf, 392) } + + pub const VARIABLE_OFFSETS: [usize; 9] = [200, 204, 208, 212, 216, 380, 384, 388, 392]; + + pub fn check_canonical(body: &[u8]) -> bool { + if body.len() < BEACON_BLOCK_BODY_FIXED || + !offsets_ok(body, &Self::VARIABLE_OFFSETS, BEACON_BLOCK_BODY_FIXED) + { + return false; + } + let field = |i: usize| variable_field(body, &Self::VARIABLE_OFFSETS, i); + fixed_list_ok(field(0), PROPOSER_SLASHING_SIZE, MAX_PROPOSER_SLASHINGS) && + variable_list_ok( + field(1), + MAX_ATTESTER_SLASHINGS_ELECTRA, + AttesterSlashingView::check_size, + ) && + variable_list_ok( + field(2), + MAX_ATTESTATIONS_ELECTRA, + AttestationView::check_canonical, + ) && + fixed_list_ok(field(3), DEPOSIT_SIZE, MAX_DEPOSITS) && + fixed_list_ok(field(4), SIGNED_VOLUNTARY_EXIT_SIZE, MAX_VOLUNTARY_EXITS) && + ExecutionPayloadView::check_canonical(field(5)) && + fixed_list_ok(field(6), SIGNED_BLS_CHANGE_SIZE, MAX_BLS_TO_EXECUTION_CHANGES) && + fixed_list_ok(field(7), BYTES_PER_KZG_COMMITMENT, MAX_BLOB_COMMITMENTS_PER_BLOCK) && + ExecutionRequestsFuluView::check_canonical(field(8)) + } +} + +/// Field `i` of a container whose offset table `offsets_ok` has already +/// accepted; the last field runs to the end of `buf`. +#[inline] +pub(crate) fn variable_field<'a>(buf: &'a [u8], positions: &[usize], i: usize) -> &'a [u8] { + let start = u32_le(buf, positions[i]) as usize; + let end = positions.get(i + 1).map_or(buf.len(), |&p| u32_le(buf, p) as usize); + &buf[start..end] } // -- SignedAggregateAndProof (beacon_aggregate_and_proof) ------------- @@ -854,7 +959,7 @@ impl SignedAggregateAndProofView { buf.len() <= SIGNED_AGG_PROOF_MAX && u32_le(buf, 0) as usize == 100 && u32_le(buf, 108) as usize == 108 && - u32_le(buf, 208) as usize == 236 + AttestationView::check_canonical(&buf[208..]) } } @@ -2422,6 +2527,43 @@ impl DepositView { } } +// -- ExecutionRequests (Fulu) ----------------------------------------- +// +// Variable. Three bounded Lists of fixed-size elements, so the fixed part +// is the 12B offset table: +// [0..4) offset: deposits (== 12) +// [4..8) offset: withdrawals +// [8..12) offset: consolidations + +pub const EXECUTION_REQUESTS_FULU_FIXED: usize = 12; + +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] +#[repr(C)] +pub struct ExecutionRequestsFuluView; + +impl ExecutionRequestsFuluView { + pub fn check_canonical(buf: &[u8]) -> bool { + const OFFSETS: [usize; 3] = [0, 4, 8]; + buf.len() >= EXECUTION_REQUESTS_FULU_FIXED && + offsets_ok(buf, &OFFSETS, EXECUTION_REQUESTS_FULU_FIXED) && + fixed_list_ok( + variable_field(buf, &OFFSETS, 0), + DEPOSIT_REQUEST_SIZE, + MAX_DEPOSIT_REQUESTS_PER_PAYLOAD, + ) && + fixed_list_ok( + variable_field(buf, &OFFSETS, 1), + WITHDRAWAL_REQUEST_SIZE, + MAX_WITHDRAWAL_REQUESTS_PER_PAYLOAD, + ) && + fixed_list_ok( + variable_field(buf, &OFFSETS, 2), + CONSOLIDATION_REQUEST_SIZE, + MAX_CONSOLIDATION_REQUESTS_PER_PAYLOAD, + ) + } +} + // -- DepositRequest (192B; ExecutionRequests.deposits element) -------- // [0..48) pubkey // [48..80) withdrawal_credentials @@ -2662,6 +2804,26 @@ impl ExecutionPayloadView { pub fn withdrawals_offset(buf: &[u8]) -> u32 { u32_le(buf, 508) } + + pub fn check_canonical(buf: &[u8]) -> bool { + const OFFSETS: [usize; 3] = [436, 504, 508]; + if buf.len() < EXECUTION_PAYLOAD_FIXED || + !offsets_ok(buf, &OFFSETS, EXECUTION_PAYLOAD_FIXED) + { + return false; + } + variable_field(buf, &OFFSETS, 0).len() <= MAX_EXTRA_DATA_BYTES && + variable_list_ok( + variable_field(buf, &OFFSETS, 1), + MAX_TRANSACTIONS_PER_PAYLOAD, + |tx| tx.len() <= MAX_BYTES_PER_TRANSACTION, + ) && + fixed_list_ok( + variable_field(buf, &OFFSETS, 2), + WITHDRAWAL_SIZE, + MAX_WITHDRAWALS_PER_PAYLOAD, + ) + } #[inline] pub fn block_access_list_offset(buf: &[u8]) -> u32 { u32_le(buf, 528)