From 8fbf237aeb9c2f15b4f66611f7cbb7f71bce2bfe Mon Sep 17 00:00:00 2001 From: vladimir-ea Date: Thu, 17 Sep 2026 13:51:58 +0100 Subject: [PATCH 1/5] serve partial cell requests --- crates/bin/src/main.rs | 2 +- crates/columns/src/cell_store.rs | 1 + crates/common/src/cell_store/mod.rs | 1 + crates/common/src/gossip.rs | 6 +- crates/common/src/lib.rs | 5 +- crates/common/src/spine/messages.rs | 17 + crates/config/src/lib.rs | 9 +- crates/control/src/cell_allocator.rs | 4 + crates/control/src/cell_ingress.rs | 55 +- crates/control/src/counters.rs | 12 + crates/control/src/lib.rs | 1 + crates/control/src/partial_exchange.rs | 455 ++++++++++++++++ .../control/src/partial_exchange/headers.rs | 87 ++++ .../src/partial_exchange/peer_column.rs | 52 ++ .../control/src/partial_exchange/response.rs | 136 +++++ crates/control/src/partial_exchange/tests.rs | 491 ++++++++++++++++++ .../src/partial_exchange/tests/allocations.rs | 58 +++ crates/control/src/tile.rs | 92 +++- crates/control/src/tile/tests.rs | 2 + crates/control/src/tile/tests/partial.rs | 139 +++++ crates/gossip/src/control.rs | 47 +- crates/gossip/src/handler.rs | 32 +- crates/gossip/src/lib.rs | 3 +- crates/gossip/src/message.rs | 73 ++- crates/gossip/src/partial.rs | 41 +- crates/gossip/src/partial/metadata.rs | 115 ++++ crates/network/src/p2p/mod.rs | 10 +- crates/network/src/p2p/quic/gossip_frame.rs | 31 +- .../src/p2p/quic/gossip_frame/tests.rs | 10 +- crates/network/src/p2p/quic/mod.rs | 1 + crates/network/src/p2p/quic/peer.rs | 43 +- crates/network/src/p2p/quic/send_receipts.rs | 126 +++++ crates/network/src/p2p/streams/gossip_out.rs | 23 +- crates/network/src/p2p/streams/state.rs | 7 +- crates/network/src/tile.rs | 22 +- crates/peer/src/lib.rs | 2 +- crates/peer/src/manager/fork_tests.rs | 4 +- crates/peer/src/manager/mod.rs | 16 +- crates/peer/src/manager/partial.rs | 53 ++ crates/peer/src/manager/promises.rs | 73 ++- crates/ssz/src/ssz_view/partial_column.rs | 11 +- 41 files changed, 2263 insertions(+), 105 deletions(-) create mode 100644 crates/control/src/partial_exchange.rs create mode 100644 crates/control/src/partial_exchange/headers.rs create mode 100644 crates/control/src/partial_exchange/peer_column.rs create mode 100644 crates/control/src/partial_exchange/response.rs create mode 100644 crates/control/src/partial_exchange/tests.rs create mode 100644 crates/control/src/partial_exchange/tests/allocations.rs create mode 100644 crates/control/src/tile/tests/partial.rs create mode 100644 crates/gossip/src/partial/metadata.rs create mode 100644 crates/network/src/p2p/quic/send_receipts.rs create mode 100644 crates/peer/src/manager/partial.rs diff --git a/crates/bin/src/main.rs b/crates/bin/src/main.rs index 6b5596375..5e5ce87f7 100644 --- a/crates/bin/src/main.rs +++ b/crates/bin/src/main.rs @@ -257,7 +257,7 @@ fn main() -> Result<(), Box> { } } - // Validation only: a non-Off mode fails startup rather than being ignored. + // Partial receiving remains gated by configuration validation. let partial_columns = config.partial_columns().map_err(|error| format!("partial columns config: {error:?}"))?; diff --git a/crates/columns/src/cell_store.rs b/crates/columns/src/cell_store.rs index c152c7cfe..f9ff3f6e9 100644 --- a/crates/columns/src/cell_store.rs +++ b/crates/columns/src/cell_store.rs @@ -378,6 +378,7 @@ impl CellStore { block_root: *root, column, slot: context.context.slot, + blob_count: context.context.blob_count, domain: context.domain, available: entry.admitted.0, full: entry.full.as_ref().map(|f| (f.read, f.cell_offset, f.proof_offset)), diff --git a/crates/common/src/cell_store/mod.rs b/crates/common/src/cell_store/mod.rs index 816914a49..c7b272087 100644 --- a/crates/common/src/cell_store/mod.rs +++ b/crates/common/src/cell_store/mod.rs @@ -211,6 +211,7 @@ pub struct ColumnAvailability { pub block_root: [u8; 32], pub column: usize, pub slot: u64, + pub blob_count: usize, pub domain: GossipDomain, pub available: u128, pub full: Option<(TCacheRead, usize, usize)>, diff --git a/crates/common/src/gossip.rs b/crates/common/src/gossip.rs index 2498efd1c..63843b5f9 100644 --- a/crates/common/src/gossip.rs +++ b/crates/common/src/gossip.rs @@ -59,10 +59,12 @@ impl GossipDomain { /// Gossipsub 1.3 extensions announcement, sent as the first RPC on /// every stream we write to after negotiating meshsub 1.3. Length -/// prefix 4, then `RPC { control(3) { extensions(6) {} } }` — empty -/// because no extension is enabled yet. +/// prefix 4, then `RPC { control(3) { extensions(6) {} } }` when no +/// extensions are enabled. pub const GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME: &[u8] = &[4, 0x1A, 2, 0x32, 0]; +pub const GOSSIP_PARTIAL_EXTENSIONS_ANNOUNCEMENT_FRAME: &[u8] = &[6, 0x1A, 4, 0x32, 2, 0x50, 1]; + /// Eth2 gossipsub topic name. Wire topic is /// `/eth2/{fork_digest_hex}/{name}/ssz_snappy`; this enum covers the `{name}` /// portion. Subnet ids travel inline. diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index 5776ce49c..7ee1a89e4 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -8,8 +8,9 @@ pub use spine::{ pub use crate::{ error::Error, gossip::{ - ATTESTATION_SUBNETS, GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME, GOSSIP_TOPIC_COUNTER_SLOTS, - GossipDomain, GossipTopic, MAX_GOSSIP_COMPRESSED_PAYLOAD_SIZE, MAX_GOSSIP_FRAME_SIZE, + ATTESTATION_SUBNETS, GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME, + GOSSIP_PARTIAL_EXTENSIONS_ANNOUNCEMENT_FRAME, GOSSIP_TOPIC_COUNTER_SLOTS, GossipDomain, + GossipTopic, MAX_GOSSIP_COMPRESSED_PAYLOAD_SIZE, MAX_GOSSIP_FRAME_SIZE, MAX_GOSSIP_UNCOMPRESSED_PAYLOAD_SIZE, MESSAGE_ID_LEN, MessageId, MessageIdHasher, SYNC_COMMITTEE_SUBNETS, gossip_topic_for_counter_slot, msg_id_invalid_snappy, msg_id_valid_snappy, diff --git a/crates/common/src/spine/messages.rs b/crates/common/src/spine/messages.rs index ed1e448b3..4a64b3baa 100644 --- a/crates/common/src/spine/messages.rs +++ b/crates/common/src/spine/messages.rs @@ -386,6 +386,7 @@ impl RpcOutbound { #[repr(C, u8)] #[allow(clippy::large_enum_variant)] pub enum PeerEvent { + SegmentedGossipResult(GossipFrameResult), /// Peer_id_full contains the secp256k1 pubkey and can be used to derive /// discovery id P2pNewConnection { @@ -792,6 +793,22 @@ pub enum P2pSend { Rpc(RpcOutbound), } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum GossipFrameOutcome { + /// Entire frame accepted by the transport, not acknowledged by the peer. + Written { + stream_id: P2pStreamId, + }, + Dropped, +} + +#[derive(Clone, Copy, Debug)] +pub struct GossipFrameResult { + pub p2p_peer: usize, + pub frame_seq: u64, + pub outcome: GossipFrameOutcome, +} + impl P2pSend { pub fn peer_id(&self) -> usize { match self { diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index b5b8a45af..6ada01824 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -546,11 +546,11 @@ impl Config { self.attestation_subnet_count } - /// Validated partial-columns mode. Non-Off modes are unsupported - /// and rejected rather than silently ignored. + /// Receiving partial columns remains disabled until validation and request + /// scheduling are connected. pub fn partial_columns(&self) -> Result { match self.partial_columns { - PartialColumnsMode::Off => Ok(PartialColumnsMode::Off), + PartialColumnsMode::Off | PartialColumnsMode::SendOnly => Ok(self.partial_columns), mode => Err(Error::ConfigError(format!("partial_columns {mode:?} is not supported"))), } } @@ -598,7 +598,6 @@ mod tests { assert_eq!(cfg.partial_columns().unwrap(), PartialColumnsMode::Off); } - /// Non-Off partial modes are unsupported. #[test] fn partial_columns_modes_are_validated() { let base = r#" @@ -608,6 +607,8 @@ mod tests { "#; let cfg: Config = toml::from_str(&format!("{base}partial_columns = \"send_only\"")).unwrap(); + assert_eq!(cfg.partial_columns().unwrap(), PartialColumnsMode::SendOnly); + let cfg: Config = toml::from_str(&format!("{base}partial_columns = \"enabled\"")).unwrap(); let err = format!("{:?}", cfg.partial_columns().unwrap_err()); assert!(err.contains("not supported"), "{err}"); } diff --git a/crates/control/src/cell_allocator.rs b/crates/control/src/cell_allocator.rs index f84b66bbb..183d2dbd1 100644 --- a/crates/control/src/cell_allocator.rs +++ b/crates/control/src/cell_allocator.rs @@ -58,6 +58,10 @@ impl CellAllocator { &self.producer } + pub fn slot_window(&self) -> (u64, Instant) { + (self.slot, self.slot_end) + } + pub fn allocate(&mut self, request: AssemblyRequest) -> Result { let context = request.context; if context.slot < self.min_slot { diff --git a/crates/control/src/cell_ingress.rs b/crates/control/src/cell_ingress.rs index a210f7638..9d0d226f4 100644 --- a/crates/control/src/cell_ingress.rs +++ b/crates/control/src/cell_ingress.rs @@ -1,10 +1,10 @@ -use std::time::Instant; +use std::{ptr, time::Instant}; use flux::spine::SpineProducers; use fxhash::FxHashMap; use silver_common::{ - ColumnOrigin, DataColumnsEvent, GossipTopic, PeerControl, SilverSpineProducers, SszCache, - TProducer, TRandomAccess, + ColumnOrigin, DataColumnsEvent, ForkName, GossipTopic, PeerControl, PeerEvent, + SilverSpineProducers, SszCache, TProducer, TRandomAccess, cell_store::{ CellKey, CellStoreConfig, CellStoreEvent, ColumnAvailability, PendingCell, StoreError, }, @@ -122,15 +122,22 @@ impl CellIngress { CellStoreEvent::Available(update) if now < update.expires && update.slot >= self.min_slot => { - let key = (update.block_root, update.column); - if self.available.contains_key(&key) || self.available.len() < self.capacity { - self.available.insert(key, update); - } + self.update_availability(update, now); } _ => {} } } + pub(crate) fn update_availability(&mut self, update: ColumnAvailability, now: Instant) { + let key = (update.block_root, update.column); + if now < update.expires && + update.slot >= self.min_slot && + (self.available.contains_key(&key) || self.available.len() < self.capacity) + { + self.available.insert(key, update); + } + } + pub fn availability( &self, root: &[u8; 32], @@ -143,6 +150,40 @@ impl CellIngress { .filter(|update| now < update.expires && update.slot >= self.min_slot) } + pub fn slot_window(&self) -> (u64, Instant) { + self.allocator.slot_window() + } + + pub fn columns(&self, now: Instant) -> impl Iterator + '_ { + self.available + .values() + .copied() + .filter(move |column| now < column.expires && column.slot >= self.min_slot) + } + + pub fn serving_column(&self, event: &PeerEvent, now: Instant) -> Option { + let (topic, digest, full) = match event { + PeerEvent::SendGossip { + topic, domain, ssz, ssz_cache: SszCache::DataColumns, .. + } => (*topic, domain.digest(), Some(*ssz)), + PeerEvent::OutboundIHave { topic, digest, .. } => (*topic, *digest, None), + _ => return None, + }; + let GossipTopic::DataColumnSidecar(index) = topic else { return None }; + self.columns(now).find(|column| { + column.column == index as usize && + column.domain.digest() == digest && + column.available != 0 && + (column.domain.format() != ForkName::Fulu || column.header.is_some()) && + full.is_none_or(|read| { + column.full.is_some_and(|(source, ..)| { + source.seq() == read.seq() && + ptr::eq(&*source.cache_ref(), &*read.cache_ref()) + }) + }) + }) + } + pub fn stage_cell( &self, key: CellKey, diff --git a/crates/control/src/counters.rs b/crates/control/src/counters.rs index 4579d70ca..62a27f890 100644 --- a/crates/control/src/counters.rs +++ b/crates/control/src/counters.rs @@ -8,5 +8,17 @@ silver_common::declare_counters! { RootNeedsStalled, RootNeedsTracked, RootNeedsRefused, + PartialMetadataReceived, + PartialMetadataReplaced, + PartialMetadataIgnored, + PartialStateLimited, + PartialFramesQueued, + PartialFramesWritten, + PartialFramesDropped, + PartialCellsServed, + PartialWithdrawals, + PartialExchanges, + PartialPendingFrames, + PartialResponsesSent, } } diff --git a/crates/control/src/lib.rs b/crates/control/src/lib.rs index 1864d1d94..7f4a2a126 100644 --- a/crates/control/src/lib.rs +++ b/crates/control/src/lib.rs @@ -2,6 +2,7 @@ pub mod cell_allocator; pub mod cell_ingress; pub mod cluster; mod counters; +mod partial_exchange; pub mod sync_engine; mod tile; diff --git a/crates/control/src/partial_exchange.rs b/crates/control/src/partial_exchange.rs new file mode 100644 index 000000000..5ff2f75b5 --- /dev/null +++ b/crates/control/src/partial_exchange.rs @@ -0,0 +1,455 @@ +use std::{ + collections::VecDeque, + time::{Duration, Instant}, +}; + +use fxhash::FxHashMap; +use silver_common::{ + ForkName, GossipFrameOutcome, GossipFrameResult, GossipTopic, P2pSend, PeerEvent, TProducer, + cell_store::{CellStoreConfig, ColumnAvailability}, +}; +use silver_gossip::{ColumnGroupKey, PartialMetadataReceived, PartsMetadata}; +use silver_peer::PeerManager; + +use self::{ + headers::HeaderTracker, + peer_column::{ExchangeKey, PeerColumnExchange}, + response::PartialResponse, +}; +use crate::{ControlCounters, cell_ingress::CellIngress}; + +mod headers; +mod peer_column; +mod response; +#[cfg(test)] +mod tests; + +const WORK_PER_SPIN: usize = 64; +const MAX_PENDING: usize = 128; +const MAX_FRAMES_PER_GROUP: u8 = 32; +const HEARTBEAT: Duration = Duration::from_millis(500); +const RETRY: Duration = Duration::from_millis(100); + +struct PendingPartialSend { + key: ExchangeKey, + rows: u128, + available: u128, + header: bool, +} + +pub(crate) struct PartialExchange { + exchanges: FxHashMap, + peer_counts: FxHashMap, + pending: FxHashMap<(usize, u64), PendingPartialSend>, + headers: HeaderTracker, + ready: VecDeque, + capacity: usize, + peer_capacity: usize, + max_rows: usize, + columns: u128, + slot: u64, + next_heartbeat: Instant, +} + +impl PartialExchange { + pub fn new(config: &CellStoreConfig, slot: u64, now: Instant) -> Self { + let peer_capacity = config.column_capacity().max(16); + let capacity = (peer_capacity * 32).min(8192); + Self { + exchanges: FxHashMap::with_capacity_and_hasher(capacity, Default::default()), + peer_counts: FxHashMap::with_capacity_and_hasher(capacity, Default::default()), + pending: FxHashMap::with_capacity_and_hasher(MAX_PENDING, Default::default()), + headers: HeaderTracker::new(capacity), + ready: VecDeque::with_capacity(capacity), + capacity, + peer_capacity, + max_rows: config.max_blobs(), + columns: config.columns(), + slot, + next_heartbeat: now, + } + } + + fn admit( + &mut self, + key: ExchangeKey, + slot: u64, + rows: usize, + expires: Instant, + now: Instant, + ) -> bool { + if self.exchanges.contains_key(&key) { + return true; + } + let count = self.peer_counts.get(&key.peer).copied().unwrap_or(0); + if self.exchanges.len() >= self.capacity || count >= self.peer_capacity { + ControlCounters::PartialStateLimited.inc(); + return false; + } + self.exchanges.insert(key, PeerColumnExchange::new(slot, rows, expires, now)); + self.peer_counts.insert(key.peer, count + 1); + true + } + + fn schedule(&mut self, key: ExchangeKey) { + if let Some(exchange) = self.exchanges.get_mut(&key) && + !exchange.scheduled + { + exchange.scheduled = true; + self.ready.push_back(key); + } + } + + pub fn available( + &mut self, + column: ColumnAvailability, + peers: &PeerManager, + now: Instant, + lazy: bool, + ) { + if now >= column.expires || column.slot != self.slot { + return; + } + let group = ColumnGroupKey::from(column); + let topic = GossipTopic::DataColumnSidecar(group.column); + let mut lazy_remaining = if lazy { peers.lazy_gossip_limit() } else { 0 }; + for peer in peers.partial_peers(topic, group.domain.digest()) { + let key = ExchangeKey { peer: peer.connection, group }; + if !peer.meshed && !self.exchanges.contains_key(&key) { + if lazy_remaining == 0 { + continue; + } + lazy_remaining -= 1; + } + if !self.admit(key, column.slot, column.blob_count, column.expires, now) { + continue; + } + let exchange = self.exchanges.get_mut(&key).unwrap(); + exchange.slot = column.slot; + exchange.n_rows = column.blob_count; + exchange.expires = column.expires; + if exchange.remote.is_some_and(|remote| remote.n_rows != column.blob_count) || + exchange.remote_slot.is_some_and(|slot| slot != column.slot) + { + exchange.remote = None; + exchange.remote_slot = None; + ControlCounters::PartialMetadataIgnored.inc(); + } + if exchange.remote.is_some() && group.domain.format() == ForkName::Fulu { + self.headers.known(key.peer, group); + } + self.schedule(key); + } + } + + pub fn metadata( + &mut self, + message: PartialMetadataReceived, + ingress: &CellIngress, + peers: &PeerManager, + now: Instant, + ) { + ControlCounters::PartialMetadataReceived.inc(); + let group = message.group; + let (slot, expires) = ingress.slot_window(); + if group.column >= 128 || + self.columns & (1u128 << group.column) == 0 || + message.metadata.n_rows == 0 || + message.metadata.n_rows > self.max_rows || + message.slot.is_some_and(|s| s != slot) || + peers + .partial_peer( + message.stream_id.peer(), + GossipTopic::DataColumnSidecar(group.column), + group.domain.digest(), + ) + .is_none() + { + ControlCounters::PartialMetadataIgnored.inc(); + return; + } + let column = ingress.availability(&group.block_root, group.column as usize, now); + if column.is_some_and(|c| { + c.domain != group.domain || + c.blob_count != message.metadata.n_rows || + message.slot.is_some_and(|s| s != c.slot) + }) { + ControlCounters::PartialMetadataIgnored.inc(); + return; + } + let key = ExchangeKey { peer: message.stream_id.peer(), group }; + if !self.admit(key, slot, message.metadata.n_rows, expires, now) { + return; + } + let exchange = self.exchanges.get_mut(&key).unwrap(); + if exchange.remote.is_some() { + ControlCounters::PartialMetadataReplaced.inc(); + } + exchange.replace(message.metadata, message.slot); + if column.is_some() && group.domain.format() == ForkName::Fulu { + self.headers.known(key.peer, group); + } + self.schedule(key); + } + + pub fn peer_event(&mut self, event: &PeerEvent, now: Instant) { + match *event { + PeerEvent::SegmentedGossipResult(result) => self.completed(result, now), + PeerEvent::P2pDisconnect { p2p_peer, .. } | + PeerEvent::P2pGossipExtensions { p2p_peer, .. } => self.remove_peer(p2p_peer), + PeerEvent::P2pNewConnection { p2p_peer_id, .. } => self.remove_peer(p2p_peer_id), + PeerEvent::P2pGossipTopicUnsubscribe { + p2p_peer, + topic: GossipTopic::DataColumnSidecar(column), + digest, + } | + PeerEvent::P2pGossipPartialCaps { p2p_peer, subnet: column, digest, .. } => { + self.remove_where(|key| { + key.peer == p2p_peer && + key.group.column == column && + key.group.domain.digest() == digest + }); + } + PeerEvent::P2pStreamClosed { stream_id } if stream_id.protocol().is_gossip() => { + self.headers.reset_stream(stream_id); + self.remove_peer(stream_id.peer()); + } + _ => {} + } + } + + fn completed(&mut self, result: GossipFrameResult, now: Instant) { + let Some(pending) = self.pending.remove(&(result.p2p_peer, result.frame_seq)) else { + return + }; + let written = match result.outcome { + GossipFrameOutcome::Written { stream_id } => Some(stream_id), + GossipFrameOutcome::Dropped => None, + }; + if pending.header { + self.headers.complete(result.p2p_peer, pending.key.group, result.frame_seq, written); + } + if let Some(exchange) = self.exchanges.get_mut(&pending.key) { + exchange.pending = false; + if written.is_some() { + exchange.sent |= pending.rows; + exchange.advertised = Some(pending.available); + ControlCounters::PartialFramesWritten.inc(); + if pending.rows != 0 { + ControlCounters::PartialResponsesSent.inc(); + ControlCounters::PartialCellsServed.add(pending.rows.count_ones() as u64); + } + } else { + exchange.retry_at = now + RETRY; + ControlCounters::PartialFramesDropped.inc(); + } + self.schedule(pending.key); + } + } + + fn remove_where(&mut self, mut predicate: impl FnMut(&ExchangeKey) -> bool) { + self.exchanges.retain(|key, _| { + if !predicate(key) { + return true; + } + let count = self.peer_counts.get_mut(&key.peer).unwrap(); + *count -= 1; + if *count == 0 { + self.peer_counts.remove(&key.peer); + } + false + }); + self.pending.retain(|&(peer, seq), pending| { + if self.exchanges.contains_key(&pending.key) { + return true; + } + if pending.header { + self.headers.complete(peer, pending.key.group, seq, None); + } + false + }); + self.ready.retain(|key| self.exchanges.contains_key(key)); + } + + fn remove_peer(&mut self, peer: usize) { + self.remove_where(|key| key.peer == peer); + self.headers.remove_peer(peer); + } + + pub fn reject(&mut self, root: &[u8; 32]) { + self.remove_where(|key| &key.group.block_root == root); + self.headers.remove_root(root); + } + + pub fn validated_sender(&mut self, peer: usize, column: ColumnAvailability) { + if column.domain.format() == ForkName::Fulu { + self.headers.known(peer, column.into()); + } + } + + pub fn spin( + &mut self, + ingress: &CellIngress, + peers: &PeerManager, + producer: &mut TProducer, + now: Instant, + emit: &mut impl FnMut(P2pSend), + ) -> bool { + self.advance_slot(ingress, peers, producer, now, emit); + if now >= self.next_heartbeat { + self.next_heartbeat = now + HEARTBEAT; + self.remove_where(|key| { + peers + .partial_peer( + key.peer, + GossipTopic::DataColumnSidecar(key.group.column), + key.group.domain.digest(), + ) + .is_none() + }); + for column in ingress.columns(now) { + self.available(column, peers, now, true); + } + } + let mut did_work = false; + for _ in 0..self.ready.len().min(WORK_PER_SPIN) { + if self.pending.len() >= MAX_PENDING { + break; + } + let key = self.ready.pop_front().unwrap(); + let exchange = self.exchanges.get_mut(&key).unwrap(); + exchange.scheduled = false; + if exchange.pending || + now < exchange.retry_at || + now >= exchange.expires || + exchange.frames_sent >= MAX_FRAMES_PER_GROUP + { + continue; + } + let Some(peer) = peers.partial_peer( + key.peer, + GossipTopic::DataColumnSidecar(key.group.column), + key.group.domain.digest(), + ) else { + continue; + }; + let Some(column) = ingress + .availability(&key.group.block_root, key.group.column as usize, now) + .filter(|c| c.domain == key.group.domain) + else { + continue; + }; + if exchange.remote.is_some_and(|r| r.n_rows != column.blob_count) || + exchange.remote_slot.is_some_and(|s| s != column.slot) + { + continue; + } + let rows = if peer.requests { exchange.requested(column.available) } else { 0 }; + let header = peer.requests && + key.group.domain.format() == ForkName::Fulu && + column.header.is_some() && + self.headers.needed(key.peer, key.group); + if rows == 0 && !header && exchange.advertised == Some(column.available) { + continue; + } + let response = PartialResponse { + group: key.group, + slot: column.slot, + metadata: PartsMetadata { + available: column.available, + requests: 0, + n_rows: column.blob_count, + }, + column: Some(column), + rows, + header, + }; + let frame = match response.write(producer, column.expires) { + Ok(frame) => frame, + Err(_) => { + exchange.retry_at = now + RETRY; + ControlCounters::PartialFramesDropped.inc(); + continue; + } + }; + let seq = frame.read().seq(); + exchange.pending = true; + exchange.frames_sent += 1; + self.pending.insert((key.peer, seq), PendingPartialSend { + key, + rows, + available: column.available, + header, + }); + if header { + self.headers.queued(key.peer, key.group, seq); + } + ControlCounters::PartialFramesQueued.inc(); + emit(P2pSend::SegmentedGossip { peer_id: key.peer, frame }); + did_work = true; + } + ControlCounters::PartialExchanges.set(self.exchanges.len() as u64); + ControlCounters::PartialPendingFrames.set(self.pending.len() as u64); + did_work + } + + pub fn advance_slot( + &mut self, + ingress: &CellIngress, + peers: &PeerManager, + producer: &mut TProducer, + now: Instant, + emit: &mut impl FnMut(P2pSend), + ) { + let (slot, _) = ingress.slot_window(); + if slot != self.slot { + self.expire(slot, peers, producer, now, emit); + } + } + + fn expire( + &mut self, + slot: u64, + peers: &PeerManager, + producer: &mut TProducer, + now: Instant, + emit: &mut impl FnMut(P2pSend), + ) { + for (key, exchange) in self + .exchanges + .iter() + .filter(|(_, e)| e.advertised.is_some_and(|a| a != 0)) + .take(WORK_PER_SPIN) + { + if peers + .partial_peer( + key.peer, + GossipTopic::DataColumnSidecar(key.group.column), + key.group.domain.digest(), + ) + .is_none() + { + continue; + } + let response = PartialResponse { + group: key.group, + slot: exchange.slot, + metadata: PartsMetadata { available: 0, requests: 0, n_rows: exchange.n_rows }, + column: None, + rows: 0, + header: false, + }; + if let Ok(frame) = response.write(producer, now + RETRY) { + emit(P2pSend::SegmentedGossip { peer_id: key.peer, frame }); + ControlCounters::PartialWithdrawals.inc(); + } + } + self.exchanges.clear(); + self.peer_counts.clear(); + self.pending.clear(); + self.headers.clear(); + self.ready.clear(); + self.slot = slot; + self.next_heartbeat = now; + } +} diff --git a/crates/control/src/partial_exchange/headers.rs b/crates/control/src/partial_exchange/headers.rs new file mode 100644 index 000000000..b889091e5 --- /dev/null +++ b/crates/control/src/partial_exchange/headers.rs @@ -0,0 +1,87 @@ +use fxhash::FxHashMap; +use silver_common::{GossipDomain, P2pStreamId}; +use silver_gossip::ColumnGroupKey; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +struct HeaderKey { + peer: usize, + domain: GossipDomain, + root: [u8; 32], +} + +#[derive(Clone, Copy)] +enum HeaderState { + Queued(u64), + Written(P2pStreamId), + Known, +} + +pub(super) struct HeaderTracker { + entries: FxHashMap, + capacity: usize, +} + +impl HeaderTracker { + pub fn new(capacity: usize) -> Self { + Self { + entries: FxHashMap::with_capacity_and_hasher(capacity, Default::default()), + capacity, + } + } + + fn key(peer: usize, group: ColumnGroupKey) -> HeaderKey { + HeaderKey { peer, domain: group.domain, root: group.block_root } + } + + pub fn needed(&self, peer: usize, group: ColumnGroupKey) -> bool { + !self.entries.contains_key(&Self::key(peer, group)) + } + + pub fn queued(&mut self, peer: usize, group: ColumnGroupKey, seq: u64) { + let key = Self::key(peer, group); + if self.entries.len() < self.capacity || self.entries.contains_key(&key) { + self.entries.insert(key, HeaderState::Queued(seq)); + } + } + + pub fn known(&mut self, peer: usize, group: ColumnGroupKey) { + let key = Self::key(peer, group); + if self.entries.len() < self.capacity || self.entries.contains_key(&key) { + self.entries.insert(key, HeaderState::Known); + } + } + + pub fn complete( + &mut self, + peer: usize, + group: ColumnGroupKey, + seq: u64, + stream: Option, + ) { + let key = Self::key(peer, group); + if matches!(self.entries.get(&key), Some(HeaderState::Queued(pending)) if *pending == seq) { + if let Some(stream) = stream { + self.entries.insert(key, HeaderState::Written(stream)); + } else { + self.entries.remove(&key); + } + } + } + + pub fn reset_stream(&mut self, stream: P2pStreamId) { + self.entries + .retain(|_, state| !matches!(state, HeaderState::Written(sent) if *sent == stream)); + } + + pub fn remove_peer(&mut self, peer: usize) { + self.entries.retain(|key, _| key.peer != peer); + } + + pub fn remove_root(&mut self, root: &[u8; 32]) { + self.entries.retain(|key, _| &key.root != root); + } + + pub fn clear(&mut self) { + self.entries.clear(); + } +} diff --git a/crates/control/src/partial_exchange/peer_column.rs b/crates/control/src/partial_exchange/peer_column.rs new file mode 100644 index 000000000..3f0821a67 --- /dev/null +++ b/crates/control/src/partial_exchange/peer_column.rs @@ -0,0 +1,52 @@ +use std::time::Instant; + +use silver_gossip::{ColumnGroupKey, PartsMetadata}; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(super) struct ExchangeKey { + pub peer: usize, + pub group: ColumnGroupKey, +} + +pub(super) struct PeerColumnExchange { + pub remote: Option, + pub remote_slot: Option, + pub advertised: Option, + pub sent: u128, + pub pending: bool, + pub scheduled: bool, + pub retry_at: Instant, + pub expires: Instant, + pub slot: u64, + pub n_rows: usize, + pub frames_sent: u8, +} + +impl PeerColumnExchange { + pub fn new(slot: u64, n_rows: usize, expires: Instant, now: Instant) -> Self { + Self { + remote: None, + remote_slot: None, + advertised: None, + sent: 0, + pending: false, + scheduled: false, + retry_at: now, + expires, + slot, + n_rows, + frames_sent: 0, + } + } + + pub fn replace(&mut self, metadata: PartsMetadata, slot: Option) { + // Repeated snapshots do not bypass the retransmission budget. + self.sent &= metadata.requests & !metadata.available; + self.remote = Some(metadata); + self.remote_slot = slot; + } + + pub fn requested(&self, available: u128) -> u128 { + self.remote.map_or(0, |remote| available & remote.requests & !remote.available & !self.sent) + } +} diff --git a/crates/control/src/partial_exchange/response.rs b/crates/control/src/partial_exchange/response.rs new file mode 100644 index 000000000..535f8b14b --- /dev/null +++ b/crates/control/src/partial_exchange/response.rs @@ -0,0 +1,136 @@ +use std::{ + io::{Cursor, Write}, + str, + time::Instant, +}; + +use silver_common::{ + CacheFrameError, CacheFrameRef, CacheSegment, ForkName, TProducer, + cell_store::{CellSource, ColumnAvailability}, + ssz_view::{ + BYTES_PER_CELL, BYTES_PER_KZG_COMMITMENT, BYTES_PER_KZG_PROOF, + partial_column::{ + PARTIAL_HEADER_FIXED, PartialLayout, PartialSidecarPlan, fulu_group_id, gloas_group_id, + }, + }, +}; +use silver_gossip::{ColumnGroupKey, PartialFrame, PartsMetadata}; + +pub(super) struct PartialResponse { + pub group: ColumnGroupKey, + pub slot: u64, + pub metadata: PartsMetadata, + pub column: Option, + pub rows: u128, + pub header: bool, +} + +impl PartialResponse { + pub fn write( + &self, + producer: &mut TProducer, + expires: Instant, + ) -> Result { + let mut topic = Cursor::new([0u8; 96]); + let digest = self.group.domain.digest(); + write!( + topic, + "/eth2/{:02x}{:02x}{:02x}{:02x}/data_column_sidecar_{}/ssz_snappy", + digest[0], digest[1], digest[2], digest[3], self.group.column + ) + .unwrap(); + let topic = str::from_utf8(&topic.get_ref()[..topic.position() as usize]).unwrap(); + let fulu = self.group.domain.format() == ForkName::Fulu; + let mut group = gloas_group_id(&self.group.block_root, self.slot); + let group = if fulu { + group[..33].copy_from_slice(&fulu_group_id(&self.group.block_root)); + &group[..33] + } else { + &group[..] + }; + let header = self.header.then(|| self.column.and_then(|column| column.header)).flatten(); + if self.header && (!fulu || header.is_none()) { + return Err(CacheFrameError::InvalidDescriptor); + } + let header_bytes = if header.is_some() { + PARTIAL_HEADER_FIXED + self.metadata.n_rows * BYTES_PER_KZG_COMMITMENT + } else { + 0 + }; + let plan = if self.rows != 0 || header.is_some() { + if self.column.is_none_or(|c| { + self.rows & !c.available != 0 || + (self.rows != 0 && c.full.is_none() && c.assembly.is_none()) + }) { + return Err(CacheFrameError::InvalidDescriptor); + } + let layout = + if fulu { PartialLayout::Fulu { header_bytes } } else { PartialLayout::Gloas }; + Some( + PartialSidecarPlan::new(layout, self.rows, self.metadata.n_rows) + .ok_or(CacheFrameError::InvalidDescriptor)?, + ) + } else { + None + }; + let frame = PartialFrame { + topic, + group_id: group, + plan, + header: header.map(|read| CacheSegment::DataColumns { + read, + offset: 0, + length: header_bytes, + }), + metadata: Some(self.metadata), + }; + frame.write( + producer, + CellSegments { column: self.column, rows: self.rows, proof: false }, + CellSegments { column: self.column, rows: self.rows, proof: true }, + expires, + ) + } +} + +#[derive(Clone)] +struct CellSegments { + column: Option, + rows: u128, + proof: bool, +} + +impl Iterator for CellSegments { + type Item = CacheSegment; + + fn next(&mut self) -> Option { + if self.rows == 0 { + return None; + } + let row = self.rows.trailing_zeros() as usize; + self.rows &= self.rows - 1; + let cell = self.column?.cell(row)?; + let length = if self.proof { BYTES_PER_KZG_PROOF } else { BYTES_PER_CELL }; + Some(match cell.source { + CellSource::Full { read, cell, proof } => CacheSegment::DataColumns { + read, + offset: if self.proof { proof } else { cell }, + length, + }, + CellSource::Assembly { reservation, row } => CacheSegment::Shared { + reservation, + part: row, + second: self.proof, + offset: 0, + length, + }, + }) + } + + fn size_hint(&self) -> (usize, Option) { + let count = self.rows.count_ones() as usize; + (count, Some(count)) + } +} + +impl ExactSizeIterator for CellSegments {} diff --git a/crates/control/src/partial_exchange/tests.rs b/crates/control/src/partial_exchange/tests.rs new file mode 100644 index 000000000..1ba62452f --- /dev/null +++ b/crates/control/src/partial_exchange/tests.rs @@ -0,0 +1,491 @@ +use std::{io::Write, net::IpAddr, sync::Arc}; + +use buffa::MessageView; +use silver_chain_spec::SpecConfig; +use silver_columns::cell_store::CellStore; +use silver_common::{ + CacheFrameRef, GossipDomain, Keypair, P2pStreamId, PeerId, StreamProtocol, TCache, + TCacheProducer, TRandomAccess, + cell_store::{CommitmentContext, ContextData, FuluContextSource}, + column_util::push_data_column_sidecar_prefix, + ssz_view::{ + BYTES_PER_CELL, BYTES_PER_KZG_PROOF, METADATA_SIZE, + partial_column::{ + PartialDataColumnPartsMetadataView, PartialDataColumnSidecarFuluView, + PartialDataColumnSidecarGloasView, + }, + }, +}; +use silver_peer::SyncingConfig; + +use super::*; + +#[path = "../../../gossip/src/generated/protobuf.gossipsub.rs"] +#[allow(dead_code, clippy::all)] +#[rustfmt::skip] +mod protobuf; + +mod allocations; + +const ROOT: [u8; 32] = [7; 32]; +const ROWS: usize = 4; + +struct Rig { + columns: Box, + network: Box, + outbound: Box, + store: CellStore, + ingress: CellIngress, + output: TProducer, + exchange: PartialExchange, + peers: PeerManager, + domain: GossipDomain, + now: Instant, +} + +impl Rig { + fn new(format: ForkName) -> Self { + let now = Instant::now(); + let spec = Arc::new(SpecConfig { + fulu_fork_epoch: 0, + gloas_fork_epoch: if format == ForkName::Gloas { 0 } else { u64::MAX }, + max_blobs_per_block_electra: ROWS as u64, + blob_schedule: Vec::new(), + ..SpecConfig::mainnet() + }); + let config = CellStoreConfig::new(spec, 3, Duration::from_secs(11)).unwrap(); + let producer = TCache::producer("", config.cache_capacity()); + let columns = Box::new(producer.cache_ref().retained_random_access("").unwrap()); + let network = Box::new(producer.cache_ref().retained_random_access("").unwrap()); + let output = TCache::producer("", 1 << 20); + let outbound = Box::new(output.cache_ref().strict_random_access("", true).unwrap()); + let exchange = PartialExchange::new(&config, 0, now); + let mut ingress = CellIngress::new(config.clone(), producer, 0, now).unwrap(); + let mut store = CellStore::new(config, 0, now).unwrap(); + let domain = GossipDomain::new([0; 4], format); + let header = [0; 208]; + let proof = [0x33; 128]; + let commitments = [0x44; ROWS * 48]; + let context_data = if format == ForkName::Fulu { + ContextData::Fulu { + signed_header: &header, + inclusion_proof: &proof, + commitments: &commitments, + } + } else { + ContextData::Gloas { commitments: &commitments } + }; + let source = if format == ForkName::Fulu { + let mut reservation = + ingress.producer_mut().reserve(context_data.encoded_len(), false).unwrap(); + context_data.write(reservation.buffer().unwrap()); + reservation.flush().unwrap(); + Some(FuluContextSource::Header(reservation.read())) + } else { + None + }; + let context = CommitmentContext { block_root: ROOT, slot: 0, format, blob_count: ROWS }; + store.admit_context(context, domain, context_data, source).unwrap(); + let request = store.request_assemblies(&ROOT).unwrap(); + let set = ingress.allocator_mut().allocate(request).unwrap(); + let mut columns = columns; + store.install(set, &mut columns).unwrap(); + let mut full = Vec::new(); + if format == ForkName::Fulu { + push_data_column_sidecar_prefix(&mut full, 0, ROWS, &header, &proof); + } else { + full.extend_from_slice(&0u64.to_le_bytes()); + full.extend_from_slice(&56u32.to_le_bytes()); + full.extend_from_slice(&((56 + ROWS * BYTES_PER_CELL) as u32).to_le_bytes()); + full.extend_from_slice(&0u64.to_le_bytes()); + full.extend_from_slice(&ROOT); + } + for row in 0..ROWS { + full.extend_from_slice(&[row as u8; BYTES_PER_CELL]); + } + if format == ForkName::Fulu { + full.extend_from_slice(&commitments); + } + for row in 0..ROWS { + full.extend_from_slice(&[0x10 + row as u8; BYTES_PER_KZG_PROOF]); + } + let mut reservation = ingress.producer_mut().reserve(full.len(), false).unwrap(); + reservation.write_all(&full).unwrap(); + reservation.flush().unwrap(); + // These tests start at the verified availability handoff; Columns owns + // cryptographic validation. + store.retain_full(&ROOT, 0, reservation.read(), &mut columns).unwrap(); + let peers = PeerManager::new( + PeerId::default(), + vec![], + vec![GossipTopic::DataColumnSidecar(0), GossipTopic::DataColumnSidecar(1)], + Default::default(), + SyncingConfig::default(), + [0; 4], + [0; METADATA_SIZE], + 3, + ); + Self { columns, network, outbound, store, ingress, output, exchange, peers, domain, now } + } + + fn connect(&mut self, peer: usize, requests: bool, mesh: bool) { + let peer_id = Keypair::from_secret(&[peer as u8; 32]).unwrap().peer_id(); + let events = [ + PeerEvent::P2pNewConnection { + p2p_peer_id: peer, + peer_id_full: peer_id, + ip: "127.0.0.1".parse::().unwrap().into(), + port: 9000, + local_dial: false, + }, + PeerEvent::P2pGossipExtensions { p2p_peer: peer, partial_messages: true }, + ]; + for event in events { + self.peers.handle_event(event, self.now, &mut |_| {}); + } + for column in 0..2 { + self.peers.handle_event( + PeerEvent::P2pGossipTopicSubscribe { + p2p_peer: peer, + topic: GossipTopic::DataColumnSidecar(column), + digest: [0; 4], + }, + self.now, + &mut |_| {}, + ); + self.peers.handle_event( + PeerEvent::P2pGossipPartialCaps { + p2p_peer: peer, + digest: [0; 4], + subnet: column, + requests, + supports_sending: true, + }, + self.now, + &mut |_| {}, + ); + if !mesh { + self.peers.handle_event( + PeerEvent::P2pGossipTopicPrune { + p2p_peer: peer, + topic: GossipTopic::DataColumnSidecar(column), + digest: [0; 4], + backoff_seconds: Some(60), + }, + self.now, + &mut |_| {}, + ); + } + } + } + + fn publish(&mut self, column: usize) { + let column = self.store.availability(&ROOT, column).unwrap(); + self.ingress.update_availability(column, self.now); + self.exchange.available(column, &self.peers, self.now, false); + } + + fn request(&mut self, peer: usize, available: u128, requests: u128) { + let message = self.message(peer, available, requests); + self.exchange.metadata(message, &self.ingress, &self.peers, self.now); + } + + fn message(&self, peer: usize, available: u128, requests: u128) -> PartialMetadataReceived { + PartialMetadataReceived { + stream_id: P2pStreamId::new(peer, 3, StreamProtocol::GossipSubV13, true), + group: ColumnGroupKey { domain: self.domain, block_root: ROOT, column: 0 }, + slot: (self.domain.format() == ForkName::Gloas).then_some(0), + metadata: PartsMetadata { available, requests, n_rows: ROWS }, + } + } + + fn spin(&mut self) -> Vec<(usize, CacheFrameRef)> { + let mut frames = Vec::new(); + self.exchange.spin(&self.ingress, &self.peers, &mut self.output, self.now, &mut |event| { + let P2pSend::SegmentedGossip { peer_id, frame } = event else { + panic!("unexpected send") + }; + frames.push((peer_id, frame)); + }); + frames + } + + fn wire(&mut self, frame: CacheFrameRef) -> Vec { + let view = frame.acquire(&mut self.outbound, self.now).unwrap(); + let descriptor = view.descriptor_range(); + let mut wire = Vec::new(); + for segment in view.segments() { + if let Some(range) = segment.framing_range() { + wire.extend_from_slice(&descriptor.as_ref()[range]); + } else { + let range = segment.acquire(&mut self.outbound, Some(&mut self.network)).unwrap(); + wire.extend_from_slice(range.as_ref()); + } + } + assert_eq!(wire.len(), view.wire_len()); + wire + } + + fn complete(&mut self, peer: usize, frame: CacheFrameRef, written: bool) { + self.exchange.peer_event( + &PeerEvent::SegmentedGossipResult(GossipFrameResult { + p2p_peer: peer, + frame_seq: frame.read().seq(), + outcome: if written { + GossipFrameOutcome::Written { + stream_id: P2pStreamId::new(peer, 4, StreamProtocol::GossipSubV13, false), + } + } else { + GossipFrameOutcome::Dropped + }, + }), + self.now, + ); + } +} + +#[test] +fn retained_full_columns_serve_requested_rows_for_both_forks_and_non_mesh_peers() { + for format in [ForkName::Fulu, ForkName::Gloas] { + let mut rig = Rig::new(format); + rig.connect(1, true, false); + rig.request(1, 0b0101, 0b1111); + assert!(rig.spin().is_empty()); + rig.publish(0); + let frames = rig.spin(); + assert_eq!(frames.len(), 1); + let wire = rig.wire(frames[0].1); + let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); + let partial = rpc.partial.as_option().unwrap(); + assert_eq!(partial.group_id.unwrap().len(), if format == ForkName::Fulu { 33 } else { 41 }); + let payload = partial.partial_message.unwrap(); + let (cells, proofs) = if format == ForkName::Fulu { + assert_eq!(PartialDataColumnSidecarFuluView::check_size(payload, ROWS), Some(0b1010)); + assert!(PartialDataColumnSidecarFuluView::header(payload).is_empty()); + ( + PartialDataColumnSidecarFuluView::cells(payload), + PartialDataColumnSidecarFuluView::proofs(payload), + ) + } else { + assert_eq!(PartialDataColumnSidecarGloasView::check_size(payload, ROWS), Some(0b1010)); + ( + PartialDataColumnSidecarGloasView::cells(payload), + PartialDataColumnSidecarGloasView::proofs(payload), + ) + }; + assert_eq!(&cells[..BYTES_PER_CELL], &[1; BYTES_PER_CELL]); + assert_eq!(&cells[BYTES_PER_CELL..], &[3; BYTES_PER_CELL]); + assert_eq!(&proofs[..BYTES_PER_KZG_PROOF], &[0x11; BYTES_PER_KZG_PROOF]); + assert_eq!(&proofs[BYTES_PER_KZG_PROOF..], &[0x13; BYTES_PER_KZG_PROOF]); + rig.complete(1, frames[0].1, true); + rig.request(1, 0b0101, 0b1111); + assert!(rig.spin().is_empty(), "repeated request must not resend written rows"); + } +} + +#[test] +fn snapshots_replace_and_available_request_bits_do_not_request_data() { + let mut rig = Rig::new(ForkName::Gloas); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 0b1100); + rig.request(1, 0, 0b0010); + let frames = rig.spin(); + let wire = rig.wire(frames[0].1); + let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); + assert_eq!( + PartialDataColumnSidecarGloasView::check_size( + rpc.partial.as_option().unwrap().partial_message.unwrap(), + ROWS + ), + Some(0b0010) + ); + rig.complete(1, frames[0].1, true); + rig.request(1, 0b0100, 0b0100); + assert!(rig.spin().is_empty()); +} + +#[test] +fn send_only_peers_receive_metadata_but_never_partial_payloads() { + let mut rig = Rig::new(ForkName::Fulu); + rig.connect(1, false, false); + rig.publish(0); + rig.request(1, 0, 0b1111); + let frames = rig.spin(); + let wire = rig.wire(frames[0].1); + let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); + let partial = rpc.partial.as_option().unwrap(); + assert!(partial.partial_message.is_none()); + assert_eq!( + PartialDataColumnPartsMetadataView::check_size(partial.parts_metadata.unwrap(), ROWS), + Some((15, 0)) + ); +} + +#[test] +fn fulu_header_is_shared_across_topics_and_failed_send_is_retried() { + let mut rig = Rig::new(ForkName::Fulu); + rig.connect(1, true, true); + rig.publish(0); + rig.publish(1); + let frames = rig.spin(); + assert_eq!(frames.len(), 2); + let mut headers = 0; + for (peer, frame) in frames { + let wire = rig.wire(frame); + let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); + let has_header = rpc.partial.as_option().unwrap().partial_message.is_some(); + headers += usize::from(has_header); + rig.complete(peer, frame, !has_header); + } + assert_eq!(headers, 1); + rig.now += HEARTBEAT; + let frames = rig.spin(); + assert_eq!(frames.len(), 1); + let wire = rig.wire(frames[0].1); + let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); + assert!(rpc.partial.as_option().unwrap().partial_message.is_some()); + rig.complete(1, frames[0].1, true); + rig.publish(0); + rig.publish(1); + assert!(rig.spin().is_empty()); +} + +#[test] +fn malformed_context_metadata_and_floods_do_not_allocate_cell_storage() { + let mut rig = Rig::new(ForkName::Gloas); + rig.connect(1, true, false); + rig.publish(0); + let mut message = rig.message(1, 0, 1); + message.metadata.n_rows = 3; + rig.exchange.metadata(message, &rig.ingress, &rig.peers, rig.now); + assert!(rig.exchange.exchanges.is_empty()); + message.metadata.n_rows = ROWS; + message.slot = Some(1); + rig.exchange.metadata(message, &rig.ingress, &rig.peers, rig.now); + assert!(rig.exchange.exchanges.is_empty()); + let seq = rig.ingress.producer_mut().next_seq(); + message.slot = Some(0); + for i in 0..100u8 { + message.group.block_root = [i; 32]; + rig.exchange.metadata(message, &rig.ingress, &rig.peers, rig.now); + } + assert_eq!(rig.exchange.exchanges.len(), rig.exchange.peer_capacity); + assert_eq!(rig.ingress.producer_mut().next_seq(), seq); + assert!(rig.exchange.ready.len() <= rig.exchange.peer_capacity); +} + +#[test] +fn expiry_withdraws_without_reading_expired_payloads() { + let mut rig = Rig::new(ForkName::Gloas); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 1); + let frames = rig.spin(); + rig.complete(1, frames[0].1, true); + rig.now += Duration::from_secs(12); + let event = rig.ingress.allocator_mut().advance(rig.now, 0).unwrap(); + rig.network.advance_retention(event.retain_from); + rig.columns.advance_retention(event.retain_from); + let frames = rig.spin(); + assert_eq!(frames.len(), 1); + let wire = rig.wire(frames[0].1); + let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); + let partial = rpc.partial.as_option().unwrap(); + assert!(partial.partial_message.is_none()); + assert_eq!( + PartialDataColumnPartsMetadataView::check_size(partial.parts_metadata.unwrap(), ROWS), + Some((0, 0)) + ); + assert!(rig.exchange.exchanges.is_empty()); + assert!(rig.exchange.pending.is_empty()); +} + +#[test] +fn disconnect_clears_pending_state_and_late_results_cannot_mark_a_header_sent() { + let mut rig = Rig::new(ForkName::Fulu); + rig.connect(1, true, true); + rig.publish(0); + let frames = rig.spin(); + assert_eq!(frames.len(), 1); + let group = rig.message(1, 0, 0).group; + assert!(!rig.exchange.headers.needed(1, group)); + rig.exchange + .peer_event(&PeerEvent::P2pDisconnect { p2p_peer: 1, peer_id: PeerId::default() }, rig.now); + assert!(rig.exchange.pending.is_empty()); + rig.complete(1, frames[0].1, true); + assert!(rig.exchange.headers.needed(1, group)); +} + +#[test] +fn withdrawn_capabilities_discard_pending_headers_and_late_feedback() { + let mut rig = Rig::new(ForkName::Fulu); + rig.connect(1, true, true); + rig.publish(0); + let frames = rig.spin(); + let event = PeerEvent::P2pGossipPartialCaps { + p2p_peer: 1, + subnet: 0, + digest: rig.domain.digest(), + requests: false, + supports_sending: false, + }; + rig.exchange.peer_event(&event, rig.now); + rig.peers.handle_event(event, rig.now, &mut |_| {}); + rig.complete(1, frames[0].1, true); + assert!(rig.exchange.headers.needed(1, rig.message(1, 0, 0).group)); + rig.request(1, 0, 1); + assert!(rig.spin().is_empty()); + assert!(rig.exchange.exchanges.is_empty()); +} + +#[test] +fn stale_requests_after_an_ignored_withdrawal_never_revive_expired_cells() { + for format in [ForkName::Fulu, ForkName::Gloas] { + let mut rig = Rig::new(format); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 1); + let frames = rig.spin(); + rig.complete(1, frames[0].1, true); + rig.now += Duration::from_secs(12); + rig.ingress.allocator_mut().advance(rig.now, 0).unwrap(); + assert_eq!(rig.spin().len(), 1); + let seq = rig.ingress.producer_mut().next_seq(); + // An additive metadata implementation can keep requesting old cells + // after receiving the all-zero withdrawal. + for _ in 0..100 { + rig.request(1, 0, 15); + assert!(rig.spin().is_empty()); + } + assert_eq!(rig.ingress.producer_mut().next_seq(), seq); + assert!(rig.exchange.pending.is_empty()); + } +} + +#[test] +fn queued_frames_keep_their_original_domain_across_fork_and_digest_changes() { + let mut rig = Rig::new(ForkName::Fulu); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 1); + let frames = rig.spin(); + for next in + [GossipDomain::new([1; 4], ForkName::Fulu), GossipDomain::new([2; 4], ForkName::Gloas)] + { + rig.peers.set_active_domains(next.digest(), Some(rig.domain.digest()), &mut |_| {}); + let wire = rig.wire(frames[0].1); + let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); + let partial = rpc.partial.as_option().unwrap(); + assert_eq!(partial.topic_id, Some(&b"/eth2/00000000/data_column_sidecar_0/ssz_snappy"[..])); + assert_eq!(partial.group_id.unwrap().len(), 33); + assert_eq!( + PartialDataColumnSidecarFuluView::check_size(partial.partial_message.unwrap(), ROWS), + Some(1) + ); + } + rig.peers.set_active_domains([2; 4], None, &mut |_| {}); + rig.now += HEARTBEAT; + assert!(rig.spin().is_empty()); + assert!(rig.exchange.exchanges.is_empty()); +} diff --git a/crates/control/src/partial_exchange/tests/allocations.rs b/crates/control/src/partial_exchange/tests/allocations.rs new file mode 100644 index 000000000..3d4db355d --- /dev/null +++ b/crates/control/src/partial_exchange/tests/allocations.rs @@ -0,0 +1,58 @@ +use std::{ + alloc::{GlobalAlloc, Layout, System}, + cell::Cell, +}; + +use super::*; + +thread_local! { + static ALLOCATIONS: Cell = const { Cell::new(0) }; +} + +struct CountingAllocator; + +unsafe impl GlobalAlloc for CountingAllocator { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + ALLOCATIONS.with(|count| count.set(count.get() + 1)); + unsafe { System.alloc(layout) } + } + + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + ALLOCATIONS.with(|count| count.set(count.get() + 1)); + unsafe { System.alloc_zeroed(layout) } + } + + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + ALLOCATIONS.with(|count| count.set(count.get() + 1)); + unsafe { System.realloc(ptr, layout, new_size) } + } + + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + unsafe { System.dealloc(ptr, layout) } + } +} + +#[global_allocator] +static ALLOCATOR: CountingAllocator = CountingAllocator; + +#[test] +fn serving_and_feedback_allocate_nothing_after_construction() { + for format in [ForkName::Fulu, ForkName::Gloas] { + let mut rig = Rig::new(format); + rig.connect(1, true, true); + let before = ALLOCATIONS.with(Cell::get); + rig.publish(0); + for attempt in 0..MAX_FRAMES_PER_GROUP { + let mut frame = None; + rig.exchange.spin(&rig.ingress, &rig.peers, &mut rig.output, rig.now, &mut |send| { + let P2pSend::SegmentedGossip { frame: next, .. } = send else { + panic!("unexpected send") + }; + assert!(frame.replace(next).is_none()); + }); + rig.complete(1, frame.unwrap(), true); + rig.request(1, 0, 1 << (attempt % ROWS as u8)); + } + assert_eq!(ALLOCATIONS.with(Cell::get) - before, 0); + } +} diff --git a/crates/control/src/tile.rs b/crates/control/src/tile.rs index 2f4da3722..3566fb79a 100644 --- a/crates/control/src/tile.rs +++ b/crates/control/src/tile.rs @@ -21,6 +21,7 @@ use self::{attestation_cluster::AttestationClusterHandler, gossip_schedule::Goss use crate::{ cell_ingress::{CellIngress, handle_data_column_event}, cluster::{AttestationClusterConfig, ClusterError}, + partial_exchange::PartialExchange, sync_engine::{SyncAction, SyncEngine}, }; @@ -62,6 +63,7 @@ pub struct Controller { /// forwards until then. Drained into the PM on the first transition. pending_subnet_topics: Vec, cell_ingress: Option, + partial_exchange: Option, /// Chain schedule, used to resolve the active gossip fork domain from /// the wall slot. spec: Arc, @@ -108,6 +110,7 @@ impl Controller { auto_ping: true, pending_subnet_topics: Vec::new(), cell_ingress: None, + partial_exchange: None, spec, gossip_schedule: None, }) @@ -120,6 +123,8 @@ impl Controller { slot: u64, slot_start: Instant, ) -> Result { + self.partial_exchange = Some(PartialExchange::new(&config, slot, slot_start)); + self.gossip_handler.enable_partial_sending(); self.cell_ingress = Some(CellIngress::new(config, producer, slot, slot_start)?); Ok(self) } @@ -251,8 +256,32 @@ impl Tile for Controller { self.attestation_cluster.free(); if let Some(ingress) = &mut self.cell_ingress { ingress.spin(now, &adapter.producers); + if let Some(exchange) = &mut self.partial_exchange { + exchange.advance_slot( + ingress, + &self.peer_manager, + &mut self.gossip_handler.mcache_publish, + now, + &mut |send| adapter.produce(send), + ); + } adapter.consume(|event: CellStoreEvent, producers| { ingress.handle(event, now, producers); + if let Some(exchange) = &mut self.partial_exchange { + match event { + CellStoreEvent::Available(column) => { + if let Some(column) = + ingress.availability(&column.block_root, column.column, now) + { + exchange.available(column, &self.peer_manager, now, false); + } + } + CellStoreEvent::RejectedContext { block_root } => { + exchange.reject(&block_root) + } + _ => {} + } + } }); } @@ -325,7 +354,21 @@ impl Tile for Controller { self.gossip_handler.mcache_insert(*msg_hash, *topic, *domain, *protobuf); } - self.peer_manager.handle_event(event, now, &mut |evt| { + if let Some(exchange) = &mut self.partial_exchange { + exchange.peer_event(&event, now); + } + let serving_column = + self.cell_ingress.as_ref().and_then(|ingress| ingress.serving_column(&event, now)); + if let ( + Some(exchange), + Some(column), + PeerEvent::SendGossip { originator_stream_id, .. }, + ) = (&mut self.partial_exchange, serving_column, event) + { + exchange.validated_sender(originator_stream_id.peer(), column); + } + let partial_serving = serving_column.is_some(); + self.peer_manager.handle_event_with_partial(event, now, partial_serving, &mut |evt| { handle_peer_control( &mut self.gossip_handler, &mut self.rpc_producer, @@ -508,16 +551,36 @@ impl Tile for Controller { } while let Some(event) = self.gossip_handler.pop_event() { match event { + GossipHandlerEvent::PartialMetadata(metadata) => { + if let (Some(exchange), Some(ingress)) = + (&mut self.partial_exchange, &self.cell_ingress) + { + exchange.metadata(metadata, ingress, &self.peer_manager, now); + } + } GossipHandlerEvent::PeerEvent(peer_event) => { self.sync_engine.on_peer_event(peer_event, self.peer_manager.our_fork_digest()); - self.peer_manager.handle_event(peer_event, now, &mut |evt| { - handle_peer_control( - &mut self.gossip_handler, - &mut self.rpc_producer, - evt, - &mut adapter.producers, - ) - }); + if let Some(exchange) = &mut self.partial_exchange { + exchange.peer_event(&peer_event, now); + } + let partial_serving = self + .cell_ingress + .as_ref() + .and_then(|ingress| ingress.serving_column(&peer_event, now)) + .is_some(); + self.peer_manager.handle_event_with_partial( + peer_event, + now, + partial_serving, + &mut |evt| { + handle_peer_control( + &mut self.gossip_handler, + &mut self.rpc_producer, + evt, + &mut adapter.producers, + ) + }, + ); } GossipHandlerEvent::NewGossip(new_gossip_msg) => adapter.produce(new_gossip_msg), GossipHandlerEvent::SendGossip(gossip_msg_out) => { @@ -525,6 +588,17 @@ impl Tile for Controller { } } } + if let (Some(exchange), Some(ingress)) = (&mut self.partial_exchange, &self.cell_ingress) && + exchange.spin( + ingress, + &self.peer_manager, + &mut self.gossip_handler.mcache_publish, + now, + &mut |send| adapter.produce(send), + ) + { + adapter.mark_work(); + } } fn try_init(&mut self, _adapter: &mut SpineAdapter) -> bool { diff --git a/crates/control/src/tile/tests.rs b/crates/control/src/tile/tests.rs index 940323225..70544d9e9 100644 --- a/crates/control/src/tile/tests.rs +++ b/crates/control/src/tile/tests.rs @@ -14,6 +14,8 @@ fn test_domain() -> silver_common::GossipDomain { use super::*; +mod partial; + struct GossipPublications { controller: Controller, adapter: SpineAdapter, diff --git a/crates/control/src/tile/tests/partial.rs b/crates/control/src/tile/tests/partial.rs new file mode 100644 index 000000000..b368a0ebe --- /dev/null +++ b/crates/control/src/tile/tests/partial.rs @@ -0,0 +1,139 @@ +use buffa::{Message, MessageView}; +use silver_common::{ + GossipFrameOutcome, GossipFrameResult, + cell_store::{CellStoreEvent, ColumnAvailability}, + ssz_view::{ + BYTES_PER_CELL, BYTES_PER_KZG_PROOF, + partial_column::{ + PartialDataColumnSidecarGloasView, gloas_group_id, parts_metadata_len, + write_parts_metadata, + }, + }, +}; + +use super::*; + +#[path = "../../../../gossip/src/generated/protobuf.gossipsub.rs"] +#[allow(dead_code, clippy::all)] +#[rustfmt::skip] +mod protobuf; + +#[test] +fn metadata_crosses_ingress_control_and_segmented_send_spine_queues() { + let topic = GossipTopic::DataColumnSidecar(0); + let mut capture = GossipPublications::new(topic, &[]); + let now = Instant::now(); + let spec = Arc::new(SpecConfig { + fulu_fork_epoch: 0, + gloas_fork_epoch: 0, + max_blobs_per_block_electra: 2, + blob_schedule: Vec::new(), + ..SpecConfig::mainnet() + }); + let config = CellStoreConfig::new(spec.clone(), 1, Duration::from_secs(11)).unwrap(); + let mut columns = TCache::producer("", config.cache_capacity()); + let mut columns_reader = Box::new(columns.cache_ref().retained_random_access("").unwrap()); + let mut outgoing_reader = Box::new( + capture + .controller + .gossip_handler + .mcache_publish + .cache_ref() + .strict_random_access("", true) + .unwrap(), + ); + let mut bytes = vec![0; 56]; + bytes[8..12].copy_from_slice(&56u32.to_le_bytes()); + bytes[12..16].copy_from_slice(&((56 + 2 * BYTES_PER_CELL) as u32).to_le_bytes()); + bytes[24..56].fill(7); + bytes.extend_from_slice(&[0x11; BYTES_PER_CELL]); + bytes.extend_from_slice(&[0x22; BYTES_PER_CELL]); + bytes.extend_from_slice(&[0x33; BYTES_PER_KZG_PROOF]); + bytes.extend_from_slice(&[0x44; BYTES_PER_KZG_PROOF]); + let full = write_bytes(&mut columns, &bytes); + capture.controller.spec = spec; + capture.controller = + capture.controller.with_data_columns_cache(config, columns, 0, now).unwrap(); + let domain = GossipDomain::new([0; 4], ForkName::Gloas); + capture.controller.gossip_handler.set_domains(domain, None); + capture.crank(); + capture.observer.produce(CellStoreEvent::Available(ColumnAvailability { + block_root: [7; 32], + column: 0, + slot: 0, + blob_count: 2, + domain, + available: 3, + full: Some((full, 56, 56 + 2 * BYTES_PER_CELL)), + assembly: None, + header: None, + expires: now + Duration::from_secs(12), + })); + let mut metadata = vec![0; parts_metadata_len(2)]; + write_parts_metadata(1, 2, 2, &mut metadata); + let rpc = protobuf::RPC { + subscriptions: vec![protobuf::rpc::SubOpts { + subscribe: Some(true), + topic_id: Some(topic.to_wire("00000000")), + requests_partial: Some(true), + supports_sending_partial: Some(false), + ..Default::default() + }], + control: buffa::MessageField::some(protobuf::ControlMessage { + extensions: buffa::MessageField::some(protobuf::ControlExtensions { + partial_messages: Some(true), + ..Default::default() + }), + ..Default::default() + }), + partial: buffa::MessageField::some(protobuf::PartialMessagesExtension { + topic_id: Some(topic.to_wire("00000000").into_bytes()), + group_id: Some(gloas_group_id(&[7; 32], 0).to_vec()), + parts_metadata: Some(metadata), + ..Default::default() + }), + ..Default::default() + }; + let stream = P2pStreamId::new(1, 3, StreamProtocol::GossipSubV13, true); + let mut incoming = stream.as_ref().to_vec(); + incoming.extend_from_slice(&rpc.encode_to_vec()); + let read = write_bytes(&mut capture.incoming, &incoming); + capture.observer.produce(GossipMsgIn { p2p_id: stream, tcache: read }); + capture.crank(); + let mut sent = None; + capture.observer.consume(|event: P2pSend, _| { + if let P2pSend::SegmentedGossip { peer_id, frame } = event { + assert_eq!(peer_id, 1); + assert!(sent.replace(frame).is_none()); + } + }); + let frame = sent.expect("metadata must reach the exchange coordinator"); + let view = frame.acquire(&mut outgoing_reader, Instant::now()).unwrap(); + let descriptor = view.descriptor_range(); + let mut wire = Vec::new(); + for segment in view.segments() { + if let Some(range) = segment.framing_range() { + wire.extend_from_slice(&descriptor.as_ref()[range]); + } else { + wire.extend_from_slice( + segment.acquire(&mut outgoing_reader, Some(&mut columns_reader)).unwrap().as_ref(), + ); + } + } + let decoded = protobuf::RPCView::decode_view(&wire).unwrap(); + let payload = decoded.partial.as_option().unwrap().partial_message.unwrap(); + assert_eq!(PartialDataColumnSidecarGloasView::check_size(payload, 2), Some(2)); + assert_eq!(PartialDataColumnSidecarGloasView::cells(payload), &[0x22; BYTES_PER_CELL]); + assert_eq!(PartialDataColumnSidecarGloasView::proofs(payload), &[0x44; BYTES_PER_KZG_PROOF]); + capture.observer.produce(PeerEvent::SegmentedGossipResult(GossipFrameResult { + p2p_peer: 1, + frame_seq: frame.read().seq(), + outcome: GossipFrameOutcome::Written { + stream_id: P2pStreamId::new(1, 4, StreamProtocol::GossipSubV13, false), + }, + })); + capture.crank(); + capture + .observer + .consume(|event: P2pSend, _| assert!(!matches!(event, P2pSend::SegmentedGossip { .. }))); +} diff --git a/crates/gossip/src/control.rs b/crates/gossip/src/control.rs index 8e261625d..23c727902 100644 --- a/crates/gossip/src/control.rs +++ b/crates/gossip/src/control.rs @@ -336,7 +336,15 @@ pub fn copy_subscribes_to_protobuf_output( producer: &mut TProducer, topics: &[&str], ) -> Result { - encode_sub_opts(producer, topics, true) + copy_subscriptions(producer, topics, false) +} + +pub(crate) fn copy_subscriptions( + producer: &mut TProducer, + topics: &[&str], + supports_partial: bool, +) -> Result { + encode_sub_opts(producer, topics, true, supports_partial) } /// As `copy_subscribes_to_protobuf_output` but with `subscribe = false` @@ -345,7 +353,7 @@ pub fn copy_unsubscribes_to_protobuf_output( producer: &mut TProducer, topics: &[&str], ) -> Result { - encode_sub_opts(producer, topics, false) + encode_sub_opts(producer, topics, false, false) } /// Subscribe / unsubscribe share wire shape; only the bool differs. @@ -353,6 +361,7 @@ fn encode_sub_opts( producer: &mut TProducer, topics: &[&str], subscribe: bool, + supports_partial: bool, ) -> Result { // RPC.subscriptions = field 1 (LD, repeated SubOpts). // SubOpts.subscribe = field 1 (varint), topic_id = field 2 (string). @@ -363,7 +372,11 @@ fn encode_sub_opts( let total: usize = topics .iter() .map(|t| { - let inner = TAG_LEN + BOOL_LEN + TAG_LEN + string_encoded_len(t); + let inner = TAG_LEN + + BOOL_LEN + + TAG_LEN + + string_encoded_len(t) + + usize::from(supports_partial) * 4; TAG_LEN + varint_len(inner as u64) + inner }) .sum(); @@ -373,7 +386,11 @@ fn encode_sub_opts( let mut cursor: &mut [u8] = &mut out[..total]; for topic in topics { - let inner = TAG_LEN + BOOL_LEN + TAG_LEN + string_encoded_len(topic); + let inner = TAG_LEN + + BOOL_LEN + + TAG_LEN + + string_encoded_len(topic) + + usize::from(supports_partial) * 4; // RPC.subscriptions (field 1, LD) — one wrap per entry. Tag::new(1, WireType::LengthDelimited).encode(&mut cursor); encode_varint(inner as u64, &mut cursor); @@ -383,6 +400,12 @@ fn encode_sub_opts( // SubOpts.topic_id (field 2, string). Tag::new(2, WireType::LengthDelimited).encode(&mut cursor); encode_string(topic, &mut cursor); + if supports_partial { + Tag::new(3, WireType::Varint).encode(&mut cursor); + encode_varint(0, &mut cursor); + Tag::new(4, WireType::Varint).encode(&mut cursor); + encode_varint(1, &mut cursor); + } } reservation.increment_offset(total); @@ -579,6 +602,22 @@ mod tests { } } + #[test] + fn send_only_subscription_advertises_sending_without_requesting() { + let mut producer = TCache::producer("", 1 << 14); + let read = copy_subscriptions( + &mut producer, + &["/eth2/00000000/data_column_sidecar_3/ssz_snappy"], + true, + ) + .unwrap(); + let bytes = read_bytes(read, &producer); + let rpc = RPCView::decode_view(&bytes).unwrap(); + let subscription = rpc.subscriptions.iter().next().unwrap(); + assert_eq!(subscription.requests_partial, Some(false)); + assert_eq!(subscription.supports_sending_partial, Some(true)); + } + #[test] fn unsubscribes_round_trip() { let mut producer = TCache::producer("control_test", 1 << 14); diff --git a/crates/gossip/src/handler.rs b/crates/gossip/src/handler.rs index 38785c9ea..047889aac 100644 --- a/crates/gossip/src/handler.rs +++ b/crates/gossip/src/handler.rs @@ -10,7 +10,7 @@ use silver_common::{ }; use crate::{ - GossipHandlerEvent, + GossipHandlerEvent, PartialMetadataReceived, control::{ self, copy_idontwants_to_protobuf_output, copy_ihaves_to_protobuf_output, handle_grafts, handle_idontwants, handle_ihaves, handle_iwants, handle_prunes, handle_subscriptions, @@ -48,6 +48,7 @@ pub struct GossipHandler { snap_scratch: Vec, extensions: ExtensionTracker, + send_partial_columns: bool, events: VecDeque, } @@ -71,6 +72,7 @@ impl GossipHandler { mcache_publish: protobuf_gossip_publish, mcache, extensions: ExtensionTracker::default(), + send_partial_columns: false, iwant_buffer: Vec::with_capacity(256), snap_encoder: snap::raw::Encoder::new(), snap_scratch: Vec::new(), @@ -264,6 +266,10 @@ impl GossipHandler { self.events.pop_front() } + pub fn enable_partial_sending(&mut self) { + self.send_partial_columns = true; + } + fn handle_peer_control_inner( &mut self, peer_control: PeerControl, @@ -272,9 +278,11 @@ impl GossipHandler { match peer_control { PeerControl::P2pGossipSubscribe { p2p: _, p2p_connection, topic, digest } => { let wire = topic.to_wire(&hex::encode(digest)); - if let Ok(tcache) = - control::copy_subscribes_to_protobuf_output(&mut self.mcache_publish, &[&wire]) - { + if let Ok(tcache) = control::copy_subscriptions( + &mut self.mcache_publish, + &[&wire], + self.send_partial_columns && matches!(topic, GossipTopic::DataColumnSidecar(_)), + ) { tracing::debug!(p2p_connection, ?topic, "Emit new gossip subscribe"); emit(GossipHandlerEvent::SendGossip(GossipMsgOut { peer_id: p2p_connection, @@ -394,6 +402,14 @@ impl GossipHandler { } handle_subscriptions(stream_id, gossip_proto.subscriptions, &self.domains, emit); + if self.send_partial_columns && + let Some(partial) = gossip_proto.partial.as_option() && + let Some(metadata) = + PartialMetadataReceived::decode(partial, *stream_id, &self.domains) + { + emit(GossipHandlerEvent::PartialMetadata(metadata)); + } + if let Some(control) = gossip_proto.control.as_option() { handle_grafts(stream_id, &control.graft, &self.domains, emit); handle_prunes(stream_id, &control.prune, &self.domains, emit); @@ -657,7 +673,9 @@ mod tests { .expect("new message"); let message = match handler.pop_event().expect("new gossip event") { GossipHandlerEvent::NewGossip(message) => message, - GossipHandlerEvent::PeerEvent(_) | GossipHandlerEvent::SendGossip(_) => { + GossipHandlerEvent::PartialMetadata(_) | + GossipHandlerEvent::PeerEvent(_) | + GossipHandlerEvent::SendGossip(_) => { panic!("unexpected local injection event") } }; @@ -673,7 +691,9 @@ mod tests { assert_eq!(handler.inject_local(topic, &ssz, Nanos::now()).unwrap(), Some(msg_id)); let duplicate = match handler.pop_event().expect("duplicate local gossip event") { GossipHandlerEvent::NewGossip(message) => message, - GossipHandlerEvent::PeerEvent(_) | GossipHandlerEvent::SendGossip(_) => { + GossipHandlerEvent::PartialMetadata(_) | + GossipHandlerEvent::PeerEvent(_) | + GossipHandlerEvent::SendGossip(_) => { panic!("unexpected duplicate local injection event") } }; diff --git a/crates/gossip/src/lib.rs b/crates/gossip/src/lib.rs index 374ab783c..c2af5e1c2 100644 --- a/crates/gossip/src/lib.rs +++ b/crates/gossip/src/lib.rs @@ -15,12 +15,13 @@ pub use control::{ copy_subscribes_to_protobuf_output, copy_unsubscribes_to_protobuf_output, }; pub use handler::GossipHandler; -pub use partial::{PartialFrame, PartsMetadata}; +pub use partial::{ColumnGroupKey, PartialFrame, PartialMetadataReceived, PartsMetadata}; use silver_common::{GossipMsgOut, NewGossipMsg, PeerEvent}; /// Events emitted by the GossipHandler. #[allow(clippy::large_enum_variant)] pub enum GossipHandlerEvent { + PartialMetadata(PartialMetadataReceived), PeerEvent(PeerEvent), NewGossip(NewGossipMsg), SendGossip(GossipMsgOut), diff --git a/crates/gossip/src/message.rs b/crates/gossip/src/message.rs index 7d10163ea..d8bd3ed1b 100644 --- a/crates/gossip/src/message.rs +++ b/crates/gossip/src/message.rs @@ -54,16 +54,6 @@ pub(super) fn handle_incoming( let (topic, domain) = domains.parse(topic_string)?; tracing::trace!(?stream_id, ?topic, "Gossip message received"); - // Data column sidecars decompress into the data-columns cache so the - // cell store can retain them by reference; everything else, and the - // fallback when no cell store is configured, uses the ssz-gossip cache. - let (publish, ssz_cache) = match data_columns_publish { - Some(dc) if matches!(topic, GossipTopic::DataColumnSidecar(_)) => { - (dc, SszCache::DataColumns) - } - _ => (incoming_gossip_publish, SszCache::Gossip), - }; - // Decompress: block snappy. let len = read_message_length(snappy_data, &topic).inspect_err(|_| { let hash = msg_id_invalid_snappy(topic_string, snappy_data); @@ -76,11 +66,24 @@ pub(super) fn handle_incoming( } })?; - // Alloc into downstream tcache - SSZ message bytes - let mut reservation = - publish.reserve(len, false).ok_or(Error::BufferTooSmall).inspect_err(|e| { - tracing::error!(?e, len, topic_string, "failed to reserve incoming gossip SSZ"); - })?; + // Serving retention must not prevent ordinary full-sidecar validation. + let retained = data_columns_publish + .filter(|_| matches!(topic, GossipTopic::DataColumnSidecar(_))) + .and_then(|producer| { + producer.reserve(len, false).map(|reservation| (producer, reservation)) + }); + let (publish, ssz_cache, mut reservation) = match retained { + Some((producer, reservation)) => (producer, SszCache::DataColumns, reservation), + None => { + let reservation = incoming_gossip_publish + .reserve(len, false) + .ok_or(Error::BufferTooSmall) + .inspect_err(|e| { + tracing::error!(?e, len, topic_string, "failed to reserve incoming gossip SSZ"); + })?; + (incoming_gossip_publish, SszCache::Gossip, reservation) + } + }; let msg_id = decompress_to_reservation(publish, snappy_data, &mut reservation, topic_string) .inspect_err(|e| { @@ -221,7 +224,7 @@ fn read_message_length(msg: &[u8], gossip_topic: &GossipTopic) -> Result usize { #[cfg(test)] mod tests { use buffa::{Message, MessageField, MessageView}; + use silver_common::{ + GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME, GOSSIP_PARTIAL_EXTENSIONS_ANNOUNCEMENT_FRAME, + }; use crate::generated::{ ControlExtensions, ControlMessage, PartialMessagesExtension, RPC, RPCView, rpc::SubOpts, }; - /// The constant announcement frame is a canonical encoding of - /// `RPC { control { extensions {} } }` with its length prefix. #[test] fn extensions_announcement_frame_decodes() { - use silver_common::GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME as FRAME; - assert_eq!(FRAME[0] as usize, FRAME.len() - 1); - let reference = RPC { - control: MessageField::some(ControlMessage { - extensions: MessageField::some(ControlExtensions::default()), + for (frame, partial_messages) in [ + (GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME, None), + (GOSSIP_PARTIAL_EXTENSIONS_ANNOUNCEMENT_FRAME, Some(true)), + ] { + assert_eq!(frame[0] as usize, frame.len() - 1); + let reference = RPC { + control: MessageField::some(ControlMessage { + extensions: MessageField::some(ControlExtensions { + partial_messages, + ..Default::default() + }), + ..Default::default() + }), ..Default::default() - }), - ..Default::default() + } + .encode_to_vec(); + assert_eq!(&frame[1..], reference); + let view = RPCView::decode_view(&frame[1..]).unwrap(); + let control = view.control.as_option().unwrap(); + assert_eq!(control.extensions.as_option().unwrap().partial_messages, partial_messages); } - .encode_to_vec(); - assert_eq!(&FRAME[1..], reference); - let view = RPCView::decode_view(&FRAME[1..]).unwrap(); - let control = view.control.as_option().unwrap(); - assert!(control.extensions.is_set()); - assert_eq!(control.extensions.as_option().unwrap().partial_messages, None); } /// Registry wire numbers: SubOpts.requestsPartial=3, diff --git a/crates/gossip/src/partial/metadata.rs b/crates/gossip/src/partial/metadata.rs new file mode 100644 index 000000000..152eef953 --- /dev/null +++ b/crates/gossip/src/partial/metadata.rs @@ -0,0 +1,115 @@ +use std::str; + +use silver_common::{ + ForkName, GossipDomain, GossipTopic, P2pStreamId, + cell_store::ColumnAvailability, + ssz_view::partial_column::{ + FULU_GROUP_ID_SIZE, GLOAS_GROUP_ID_SIZE, PARTIAL_COLUMNS_VERSION_BYTE, + PartialDataColumnPartsMetadataView, + }, +}; + +use super::PartsMetadata; +use crate::{generated::PartialMessagesExtensionView, handler::ActiveDomains}; + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub struct ColumnGroupKey { + pub domain: GossipDomain, + pub block_root: [u8; 32], + pub column: u64, +} + +impl From for ColumnGroupKey { + fn from(column: ColumnAvailability) -> Self { + Self { domain: column.domain, block_root: column.block_root, column: column.column as u64 } + } +} + +#[derive(Clone, Copy, Debug)] +pub struct PartialMetadataReceived { + pub stream_id: P2pStreamId, + pub group: ColumnGroupKey, + pub slot: Option, + pub metadata: PartsMetadata, +} + +impl PartialMetadataReceived { + pub(crate) fn decode( + partial: &PartialMessagesExtensionView<'_>, + stream_id: P2pStreamId, + domains: &ActiveDomains, + ) -> Option { + let (topic, domain) = domains.parse(str::from_utf8(partial.topic_id?).ok()?).ok()?; + let GossipTopic::DataColumnSidecar(column) = topic else { return None }; + if column >= 128 { + return None; + } + let group_id = partial.group_id?; + if group_id.first().copied()? != PARTIAL_COLUMNS_VERSION_BYTE { + return None; + } + let slot = match domain.format() { + ForkName::Fulu if group_id.len() == FULU_GROUP_ID_SIZE => None, + ForkName::Gloas if group_id.len() == GLOAS_GROUP_ID_SIZE => { + Some(u64::from_le_bytes(group_id[33..].try_into().ok()?)) + } + _ => return None, + }; + let (available, requests, n_rows) = + PartialDataColumnPartsMetadataView::decode(partial.parts_metadata?)?; + Some(Self { + stream_id, + group: ColumnGroupKey { domain, block_root: group_id[1..33].try_into().ok()?, column }, + slot, + metadata: PartsMetadata { available, requests, n_rows }, + }) + } +} + +#[cfg(test)] +mod tests { + use silver_common::{ + StreamProtocol, + ssz_view::partial_column::{ + fulu_group_id, gloas_group_id, parts_metadata_len, write_parts_metadata, + }, + }; + + use super::*; + + #[test] + fn decodes_both_group_formats_and_rejects_wrong_domains_versions_and_lengths() { + for format in [ForkName::Fulu, ForkName::Gloas] { + let domain = GossipDomain::new([1, 2, 3, 4], format); + let domains = ActiveDomains::new(Some(domain)); + let mut metadata = [0; parts_metadata_len(4)]; + write_parts_metadata(0b0101, 0b1110, 4, &mut metadata); + let root = [7; 32]; + let fulu = fulu_group_id(&root); + let gloas = gloas_group_id(&root, 123); + let group = if format == ForkName::Fulu { &fulu[..] } else { &gloas[..] }; + let mut partial = PartialMessagesExtensionView { + topic_id: Some(b"/eth2/01020304/data_column_sidecar_7/ssz_snappy"), + group_id: Some(group), + parts_metadata: Some(&metadata), + ..Default::default() + }; + let stream = P2pStreamId::new(1, 3, StreamProtocol::GossipSubV13, true); + let decoded = PartialMetadataReceived::decode(&partial, stream, &domains).unwrap(); + assert_eq!(decoded.group.domain, domain); + assert_eq!(decoded.group.column, 7); + assert_eq!(decoded.group.block_root, root); + assert_eq!(decoded.metadata, PartsMetadata { available: 5, requests: 14, n_rows: 4 }); + assert_eq!(decoded.slot, (format == ForkName::Gloas).then_some(123)); + partial.topic_id = Some(b"/eth2/00000000/data_column_sidecar_7/ssz_snappy"); + assert!(PartialMetadataReceived::decode(&partial, stream, &domains).is_none()); + partial.topic_id = Some(b"/eth2/01020304/data_column_sidecar_7/ssz_snappy"); + partial.group_id = Some(&group[..group.len() - 1]); + assert!(PartialMetadataReceived::decode(&partial, stream, &domains).is_none()); + let mut unknown = group.to_vec(); + unknown[0] = 1; + partial.group_id = Some(&unknown); + assert!(PartialMetadataReceived::decode(&partial, stream, &domains).is_none()); + } + } +} diff --git a/crates/network/src/p2p/mod.rs b/crates/network/src/p2p/mod.rs index 0780100e4..b8621e0c7 100644 --- a/crates/network/src/p2p/mod.rs +++ b/crates/network/src/p2p/mod.rs @@ -18,8 +18,9 @@ pub(crate) use quic::{Peer, create_client_config}; pub use quic::{SendResult, create_endpoint, create_server_config}; use quinn_proto::{ConnectionHandle, DatagramEvent, Endpoint}; use silver_common::{ - CacheFrameRef, ClusterMsgOut, GossipMsgOut, Identify, Keypair, P2pConnectionStats, P2pStreamId, - PeerId, ProtoIdentify, ProtoIdentifyView, RpcOutbound, RpcRequestOutbound, TCacheRead, + CacheFrameRef, ClusterMsgOut, GossipFrameResult, GossipMsgOut, Identify, Keypair, + P2pConnectionStats, P2pStreamId, PeerId, ProtoIdentify, ProtoIdentifyView, RpcOutbound, + RpcRequestOutbound, TCacheRead, }; use crate::{ @@ -58,6 +59,7 @@ pub fn p2p_spin( #[derive(Debug, Clone)] #[allow(clippy::large_enum_variant)] pub enum NetEvent { + GossipFrameResult(GossipFrameResult), /// A peer connection has been established and its PeerId verified. PeerConnected { peer: RemotePeer, @@ -364,6 +366,10 @@ impl P2p { NetworkCounters::P2pConnections.set(self.peers.len() as u64); if let Some(limits) = &self.segmented_limits { limits.publish_gauges(); + while let Some(result) = limits.pop_result() { + did_work = true; + on_event(NetEvent::GossipFrameResult(result)); + } } did_work } diff --git a/crates/network/src/p2p/quic/gossip_frame.rs b/crates/network/src/p2p/quic/gossip_frame.rs index 8ee153353..bea10f429 100644 --- a/crates/network/src/p2p/quic/gossip_frame.rs +++ b/crates/network/src/p2p/quic/gossip_frame.rs @@ -2,10 +2,15 @@ use std::{cell::Cell, ptr::NonNull, time::Instant}; use bytes::Bytes; use silver_common::{ - AcquiredCacheFrame, AcquiredCacheSegment, AcquiredRange, CacheFrameView, TRead, + AcquiredCacheFrame, AcquiredCacheSegment, AcquiredRange, CacheFrameView, GossipFrameResult, + P2pStreamId, TRead, }; -use super::{Leased, leased::OutboundLeaseWheel}; +use super::{ + Leased, + leased::OutboundLeaseWheel, + send_receipts::{SendReceipt, SendReceipts}, +}; use crate::{NetworkCounters, p2p::Context}; const MAX_RETAINED_BYTES: usize = 64 * 1024 * 1024; @@ -18,6 +23,7 @@ pub(crate) enum OutboundGossip { } pub(crate) struct SegmentedGossipLimits { + receipts: SendReceipts, frames: Cell, owners: Cell, retained_bytes: Cell, @@ -35,6 +41,7 @@ impl Default for SegmentedGossipLimits { impl SegmentedGossipLimits { pub(crate) fn new(max_frames: usize) -> Self { Self { + receipts: SendReceipts::new(max_frames), frames: Cell::new(0), owners: Cell::new(0), retained_bytes: Cell::new(0), @@ -70,7 +77,7 @@ impl SegmentedGossipLimits { )?; NetworkCounters::CacheSegmentedAdmitted.inc(); NetworkCounters::CacheSegmentedSegments.add(frame.segment_count() as u64); - Some(SegmentedFrame { segments: wheel.leased(frame, now), budget }) + Some(SegmentedFrame { segments: wheel.leased(frame, now), budget, receipt: None }) } pub(crate) fn publish_gauges(&self) { @@ -78,6 +85,10 @@ impl SegmentedGossipLimits { NetworkCounters::CacheSegmentedOwners.set(self.owners.get() as u64); NetworkCounters::CacheSegmentedRetainedBytes.set(self.retained_bytes.get() as u64); } + + pub(crate) fn pop_result(&self) -> Option { + self.receipts.pop() + } } impl Drop for SegmentedGossipLimits { @@ -90,11 +101,16 @@ impl Drop for SegmentedGossipLimits { #[derive(Debug)] pub(crate) struct SegmentedFrame { + receipt: Option, segments: Leased, budget: FrameBudget, } impl SegmentedFrame { + pub(crate) fn track(&mut self, limits: &SegmentedGossipLimits, peer: usize, seq: u64) -> bool { + self.receipt = limits.receipts.acquire(peer, seq, self.wire_len()); + self.receipt.is_some() + } pub(crate) fn wire_len(&self) -> usize { self.segments.wire_len() } @@ -116,7 +132,7 @@ pub(crate) struct SegmentedWriter { impl SegmentedWriter { pub(crate) fn chunk(&mut self) -> Option<&mut Bytes> { if self.current.is_empty() { - let SegmentedFrame { segments, budget } = &mut self.frame; + let SegmentedFrame { segments, budget, .. } = &mut self.frame; self.current = match segments.take_next()? { AcquiredCacheSegment::Framing(range) => { if self.descriptor.is_empty() { @@ -135,6 +151,13 @@ impl SegmentedWriter { self.remaining -= bytes; self.remaining == 0 } + + pub(crate) fn complete(&mut self, stream_id: P2pStreamId) { + assert_eq!(self.remaining, 0); + if let Some(receipt) = &mut self.frame.receipt { + receipt.written(stream_id); + } + } } #[derive(Debug)] diff --git a/crates/network/src/p2p/quic/gossip_frame/tests.rs b/crates/network/src/p2p/quic/gossip_frame/tests.rs index 0672f2e3f..594f26ae8 100644 --- a/crates/network/src/p2p/quic/gossip_frame/tests.rs +++ b/crates/network/src/p2p/quic/gossip_frame/tests.rs @@ -8,8 +8,8 @@ use std::{ use quinn_proto::StreamId; use silver_common::{ - AcquiredWithOffset, CacheFrameRef, CacheSegment, P2pStreamId, StreamProtocol, SubLayout, - SubReservationRef, TCache, TCacheProducer, TProducer, + AcquiredWithOffset, CacheFrameRef, CacheSegment, GossipFrameOutcome, P2pStreamId, + StreamProtocol, SubLayout, SubReservationRef, TCache, TCacheProducer, TProducer, }; use super::*; @@ -257,7 +257,8 @@ fn segments_are_allocated_lazily_and_blocked_retries_survive_expiry() { let warm = h.acquire(reference).unwrap(); drop(warm); let before = ALLOCATIONS.with(Cell::get); - let frame = h.acquire(reference).unwrap(); + let mut frame = h.acquire(reference).unwrap(); + assert!(frame.track(&h.limits, 0, reference.read().seq())); assert_eq!(ALLOCATIONS.with(Cell::get) - before, 0); let cell_ptr = assembly .acquire(h.context.data_columns_consumer.as_deref_mut().unwrap()) @@ -305,6 +306,9 @@ fn segments_are_allocated_lazily_and_blocked_retries_survive_expiry() { assert_eq!(&io.written[69..], &[0xcd; 8]); assert_eq!(io.retained[1].as_ptr(), cell_ptr); assert_eq!(h.limits.frames.get(), 0); + let result = h.limits.pop_result().unwrap(); + assert_eq!(result.frame_seq, reference.read().seq()); + assert_eq!(result.outcome, GossipFrameOutcome::Written { stream_id: stream() }); assert!(h.columns.reserve(8192, true).is_none()); assert!(h.wheel.expire(h.now + Duration::from_secs(11)).is_some()); io.retained.clear(); diff --git a/crates/network/src/p2p/quic/mod.rs b/crates/network/src/p2p/quic/mod.rs index 6e689aa03..3dcb9cf89 100644 --- a/crates/network/src/p2p/quic/mod.rs +++ b/crates/network/src/p2p/quic/mod.rs @@ -11,6 +11,7 @@ use super::tls; mod gossip_frame; mod leased; mod peer; +mod send_receipts; mod stream; pub(crate) use gossip_frame::{OutboundGossip, SegmentedGossipLimits, SegmentedWriter}; diff --git a/crates/network/src/p2p/quic/peer.rs b/crates/network/src/p2p/quic/peer.rs index cec42ea88..9dbe1b6db 100644 --- a/crates/network/src/p2p/quic/peer.rs +++ b/crates/network/src/p2p/quic/peer.rs @@ -193,14 +193,18 @@ impl Peer { .acquire(&mut context.gossip_consumer, now) .ok() .and_then(|view| limits.acquire(view, context, &self.outbound_lease_wheel, now)); - let Some(frame) = acquired else { + let Some(mut acquired) = acquired else { crate::NetworkCounters::CacheSegmentedRejected.inc(); return SendResult::MessageDropped; }; - self.queue_gossip(OutboundGossip::Segmented(frame)) + if !acquired.track(limits, self.handle.0, frame.read().seq()) { + return SendResult::MessageDropped; + } + self.queue_gossip(OutboundGossip::Segmented(acquired)) } fn queue_gossip(&mut self, msg: OutboundGossip) -> SendResult { + let tracked = matches!(msg, OutboundGossip::Segmented(_)); self.dirty = true; let stream_id = match self.outbound_gossip { Some(id) => id, @@ -216,7 +220,11 @@ impl Peer { if let OutboundBuffer::Gossip(buffer) = &mut stream.out_buffer { let dropped = buffer.add_msg(msg); stream.needs_spin = true; - return if dropped { SendResult::MessageDropped } else { SendResult::Ok }; + return if dropped && !tracked { + SendResult::MessageDropped + } else { + SendResult::Ok + }; } } SendResult::StreamCreationError @@ -1301,6 +1309,23 @@ mod tests { assert!(buf.is_empty()); } + #[test] + fn queue_eviction_reports_the_evicted_frame_not_its_replacement() { + let receipts = Box::new(super::super::send_receipts::SendReceipts::new(4)); + let mut buffer = OutBuffer::new(2); + assert!(!buffer.add_msg(receipts.acquire(1, 10, 100).unwrap())); + assert!(!buffer.add_msg(receipts.acquire(1, 11, 100).unwrap())); + assert!(buffer.add_msg(receipts.acquire(1, 12, 100).unwrap())); + let dropped = receipts.pop().unwrap(); + assert_eq!(dropped.frame_seq, 10); + assert_eq!(dropped.outcome, silver_common::GossipFrameOutcome::Dropped); + assert!(receipts.pop().is_none()); + drop(buffer); + let dropped: Vec<_> = + std::iter::from_fn(|| receipts.pop()).map(|result| result.frame_seq).collect(); + assert_eq!(dropped, [12, 11]); + } + #[test] fn outbound_lease_wheel_drop_cancels_lease_after_box_moves() { let t0 = Instant::now(); @@ -2240,7 +2265,9 @@ mod tests { .advance_retention(columns.next_seq()); wait_for(&mut pair, &mut client_h, &mut server_h, 200, |_, s| !s.received.is_empty()); let wire: Vec<_> = server_h.received.values().flatten().copied().collect(); - assert_eq!(wire, announced(payload)); + let mut expected = vec![0x1a, 4, 0x32, 2, 0x50, 1]; + expected.extend_from_slice(payload); + assert_eq!(wire, expected); let now = Instant::now(); for i in 0..200 { if pair.client_peer.outbound_lease_wheel.active_count() == 0 { @@ -2279,12 +2306,16 @@ mod tests { let id = peer.outbound_gossip.unwrap(); peer.streams.get_mut(&id).unwrap().out_buffer = OutboundBuffer::Gossip(OutBuffer::new(1)); assert_eq!(peer.outbound_lease_wheel.active_count(), 0); - for expected in [SendResult::Ok, SendResult::MessageDropped, SendResult::MessageDropped] { + assert_eq!(limits.pop_result().unwrap().frame_seq, frame.read().seq()); + for i in 0..3 { assert_eq!( peer.send_segmented_gossip(frame, &mut h.context, &limits, &mut h.rpc_codec_pool), - expected + SendResult::Ok ); assert_eq!(peer.outbound_lease_wheel.active_count(), 1); + if i > 0 { + assert_eq!(limits.pop_result().unwrap().frame_seq, frame.read().seq()); + } } peer.clear_streams(&mut h.rpc_codec_pool); assert_eq!(peer.outbound_lease_wheel.active_count(), 0); diff --git a/crates/network/src/p2p/quic/send_receipts.rs b/crates/network/src/p2p/quic/send_receipts.rs new file mode 100644 index 000000000..56457e000 --- /dev/null +++ b/crates/network/src/p2p/quic/send_receipts.rs @@ -0,0 +1,126 @@ +use std::{ + cell::{Cell, RefCell}, + collections::VecDeque, + ptr::NonNull, +}; + +use fxhash::FxHashMap; +use silver_common::{GossipFrameOutcome, GossipFrameResult, P2pStreamId}; + +const MAX_PEER_FRAMES: usize = 16; +const MAX_PEER_BYTES: usize = 4 * 1024 * 1024; + +pub(super) struct SendReceipts { + pending: Cell, + peers: RefCell>, + completed: RefCell>, + capacity: usize, +} + +impl SendReceipts { + pub(super) fn new(capacity: usize) -> Self { + Self { + pending: Cell::new(0), + peers: RefCell::new(FxHashMap::with_capacity_and_hasher(capacity, Default::default())), + completed: RefCell::new(VecDeque::with_capacity(capacity)), + capacity, + } + } + + pub(super) fn acquire(&self, peer: usize, frame_seq: u64, bytes: usize) -> Option { + if self.pending.get() + self.completed.borrow().len() >= self.capacity { + return None; + } + let mut peers = self.peers.borrow_mut(); + let (frames, queued_bytes) = peers.get(&peer).copied().unwrap_or_default(); + if frames >= MAX_PEER_FRAMES || bytes > MAX_PEER_BYTES.saturating_sub(queued_bytes) { + return None; + } + peers.insert(peer, (frames + 1, queued_bytes + bytes)); + self.pending.set(self.pending.get() + 1); + Some(SendReceipt { + receipts: NonNull::from(self), + result: GossipFrameResult { + p2p_peer: peer, + frame_seq, + outcome: GossipFrameOutcome::Dropped, + }, + bytes, + }) + } + + pub(super) fn pop(&self) -> Option { + self.completed.borrow_mut().pop_front() + } +} + +#[derive(Debug)] +pub(super) struct SendReceipt { + receipts: NonNull, + result: GossipFrameResult, + bytes: usize, +} + +// Receipts remain on NetworkTile; the boxed limits outlive every queued frame. +unsafe impl Send for SendReceipt {} + +impl SendReceipt { + pub(super) fn written(&mut self, stream_id: P2pStreamId) { + self.result.outcome = GossipFrameOutcome::Written { stream_id }; + } +} + +impl Drop for SendReceipt { + fn drop(&mut self) { + let receipts = unsafe { self.receipts.as_ref() }; + let mut peers = receipts.peers.borrow_mut(); + let (frames, bytes) = peers.get_mut(&self.result.p2p_peer).unwrap(); + *frames -= 1; + *bytes -= self.bytes; + if *frames == 0 { + peers.remove(&self.result.p2p_peer); + } + receipts.pending.set(receipts.pending.get() - 1); + receipts.completed.borrow_mut().push_back(self.result); + } +} + +#[cfg(test)] +mod tests { + use silver_common::StreamProtocol; + + use super::*; + + #[test] + fn written_and_abandoned_frames_release_their_own_receipts() { + let receipts = Box::new(SendReceipts::new(2)); + let mut first = receipts.acquire(1, 10, 1024).unwrap(); + let second = receipts.acquire(2, 20, 2048).unwrap(); + assert!(receipts.acquire(1, 30, 1).is_none()); + let stream_id = P2pStreamId::new(1, 4, StreamProtocol::GossipSubV13, false); + first.written(stream_id); + drop(first); + assert!(receipts.acquire(1, 30, 1).is_none(), "undrained feedback reserves its capacity"); + let result = receipts.pop().unwrap(); + assert_eq!(result.frame_seq, 10); + assert_eq!(result.outcome, GossipFrameOutcome::Written { stream_id }); + drop(second); + let result = receipts.pop().unwrap(); + assert_eq!(result.p2p_peer, 2); + assert_eq!(result.frame_seq, 20); + assert_eq!(result.outcome, GossipFrameOutcome::Dropped); + assert!(receipts.peers.borrow().is_empty()); + assert_eq!(receipts.pending.get(), 0); + } + + #[test] + fn peer_byte_limit_does_not_block_other_peers() { + let receipts = Box::new(SendReceipts::new(4)); + let first = receipts.acquire(1, 1, MAX_PEER_BYTES).unwrap(); + assert!(receipts.acquire(1, 2, 1).is_none()); + let second = receipts.acquire(2, 3, 1).unwrap(); + drop(first); + drop(second); + assert!(receipts.peers.borrow().is_empty()); + } +} diff --git a/crates/network/src/p2p/streams/gossip_out.rs b/crates/network/src/p2p/streams/gossip_out.rs index 4c680f2fb..a231678bf 100644 --- a/crates/network/src/p2p/streams/gossip_out.rs +++ b/crates/network/src/p2p/streams/gossip_out.rs @@ -1,7 +1,8 @@ use std::slice; use silver_common::{ - GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME, MAX_GOSSIP_FRAME_SIZE, P2pStreamId, TRead, + GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME, GOSSIP_PARTIAL_EXTENSIONS_ANNOUNCEMENT_FRAME, + MAX_GOSSIP_FRAME_SIZE, P2pStreamId, TRead, }; use crate::{ @@ -20,6 +21,7 @@ pub(crate) enum GossipWriteState { /// stream recreation restarts from here. Announcing { written: usize, + partial_columns: bool, }, Idle, WritingLength { @@ -64,16 +66,18 @@ impl GossipWriteState { p2p_id: &P2pStreamId, ) -> Result { match self { - Self::Announcing { mut written } => { - let n = io.write_to_stream( - p2p_id.stream_id(), - &GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME[written..], - )?; + Self::Announcing { mut written, partial_columns } => { + let announcement: &[u8] = if partial_columns { + GOSSIP_PARTIAL_EXTENSIONS_ANNOUNCEMENT_FRAME + } else { + GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME + }; + let n = io.write_to_stream(p2p_id.stream_id(), &announcement[written..])?; written += n; - if written == GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME.len() { + if written == announcement.len() { Ok(Spin::Next(Self::Idle)) } else { - Ok(Spin::Ok(Self::Announcing { written })) + Ok(Spin::Ok(Self::Announcing { written, partial_columns })) } } Self::Idle => match io.gossip_next() { @@ -129,6 +133,7 @@ impl GossipWriteState { let n = io.write_chunks(p2p_id.stream_id(), slice::from_mut(chunk))?; let chunk_complete = chunk.is_empty(); if frame.written(n) { + frame.complete(*p2p_id); Ok(Spin::Next(Self::Idle)) } else if chunk_complete { Ok(Spin::Next(Self::WritingSegments(frame))) @@ -278,7 +283,7 @@ mod tests { }; let p2p_id = P2pStreamId::new(0, 4, StreamProtocol::GossipSub, false); - let mut state = GossipWriteState::Announcing { written: 0 }; + let mut state = GossipWriteState::Announcing { written: 0, partial_columns: false }; for _ in 0..64 { state = state.spin(&mut io, &p2p_id).unwrap(); } diff --git a/crates/network/src/p2p/streams/state.rs b/crates/network/src/p2p/streams/state.rs index 3d885e3ca..685f4164f 100644 --- a/crates/network/src/p2p/streams/state.rs +++ b/crates/network/src/p2p/streams/state.rs @@ -262,7 +262,12 @@ impl StreamState { Ok(Self::Gossip { read: GossipReadState::default(), write: if announce { - GossipWriteState::Announcing { written: 0 } + GossipWriteState::Announcing { + written: 0, + partial_columns: context + .data_columns_consumer + .is_some(), + } } else { GossipWriteState::Idle }, diff --git a/crates/network/src/tile.rs b/crates/network/src/tile.rs index db5d3934c..b259139db 100644 --- a/crates/network/src/tile.rs +++ b/crates/network/src/tile.rs @@ -4,15 +4,19 @@ use std::{ time::{Duration, Instant}, }; -use flux::{spine::SpineAdapter, tile::Tile, tracing}; +use flux::{ + spine::{SpineAdapter, SpineProducers}, + tile::Tile, + tracing, +}; use flux_profiler::timed; use mio::{Events, Poll, Token}; use quinn_proto::Transmit; use secp256k1::PublicKey; use silver_common::{ - BeaconStateEvent, ClusterIn, ClusterMsgIn, ClusterMsgOut, GossipMsgIn, GossipMsgOut, P2pSend, - PeerControl, PeerEvent, PeerStats, RpcInbound, RpcOutbound, SLOTS_PER_EPOCH, SilverSpine, - cell_store::RetentionEvent, + BeaconStateEvent, ClusterIn, ClusterMsgIn, ClusterMsgOut, GossipFrameOutcome, + GossipFrameResult, GossipMsgIn, GossipMsgOut, P2pSend, PeerControl, PeerEvent, PeerStats, + RpcInbound, RpcOutbound, SLOTS_PER_EPOCH, SilverSpine, cell_store::RetentionEvent, }; use silver_discovery::{DiscV5, Discovery, DiscoveryEvent}; @@ -150,6 +154,9 @@ impl NetworkTile { let mut on_event = |event| match event { Event::P2pNet(net_event) => match net_event { + NetEvent::GossipFrameResult(result) => { + adapter.produce(PeerEvent::SegmentedGossipResult(result)); + } NetEvent::PeerConnected { peer, addr, local_dialler } => { let port = addr.port(); adapter.produce(PeerEvent::P2pNewConnection { @@ -236,6 +243,13 @@ impl NetworkTile { self.inner.enqueue_rpc_out(rpc_outbound) }, }; + if result != p2p::SendResult::Ok && let P2pSend::SegmentedGossip { peer_id, frame } = msg { + producers.produce(PeerEvent::SegmentedGossipResult(GossipFrameResult { + p2p_peer: peer_id, + frame_seq: frame.read().seq(), + outcome: GossipFrameOutcome::Dropped, + })); + } match result { p2p::SendResult::Ok => {} p2p::SendResult::StreamCreationError | p2p::SendResult::StreamGone => { diff --git a/crates/peer/src/lib.rs b/crates/peer/src/lib.rs index bedc85bf7..8a46bf11d 100644 --- a/crates/peer/src/lib.rs +++ b/crates/peer/src/lib.rs @@ -4,7 +4,7 @@ mod manager; mod scoring; mod state; -pub use manager::{PeerManager, RejectedRoots}; +pub use manager::{PartialPeer, PeerManager, RejectedRoots}; pub use silver_config::SyncingConfig; silver_common::declare_counters! { diff --git a/crates/peer/src/manager/fork_tests.rs b/crates/peer/src/manager/fork_tests.rs index 154739418..b6df0f37e 100644 --- a/crates/peer/src/manager/fork_tests.rs +++ b/crates/peer/src/manager/fork_tests.rs @@ -104,13 +104,13 @@ fn mesh_refill_and_ihave_use_exact_domain_subscriptions() { let protobuf = reservation.read(); reservation.increment_offset(1); captured.0.clear(); - manager.on_outbound_ihave(TOPIC, NEW, protobuf, &mut |event| captured.0.push(event)); + manager.on_outbound_ihave(TOPIC, NEW, protobuf, false, &mut |event| captured.0.push(event)); assert!( matches!(captured.0.as_slice(), [PeerControl::P2pSend(P2pSend::Gossip(message))] if message.peer_id == 1) ); manager.on_unsubscribe(1, TOPIC, NEW, now, &mut |_| {}); captured.0.clear(); - manager.on_outbound_ihave(TOPIC, NEW, protobuf, &mut |event| captured.0.push(event)); + manager.on_outbound_ihave(TOPIC, NEW, protobuf, false, &mut |event| captured.0.push(event)); assert!(captured.0.is_empty()); } diff --git a/crates/peer/src/manager/mod.rs b/crates/peer/src/manager/mod.rs index 7948d0e8d..4f40ac5e8 100644 --- a/crates/peer/src/manager/mod.rs +++ b/crates/peer/src/manager/mod.rs @@ -24,8 +24,10 @@ use crate::{ pub(crate) mod admission; pub(crate) mod attempts; pub(crate) mod mesh; +mod partial; pub(crate) mod peers; pub(crate) mod promises; +pub use partial::PartialPeer; pub(crate) mod rpc; pub(crate) mod sync; @@ -295,8 +297,19 @@ impl PeerManager { event: PeerEvent, now: Instant, emit: &mut impl FnMut(PeerControl), + ) { + self.handle_event_with_partial(event, now, false, emit); + } + + pub fn handle_event_with_partial( + &mut self, + event: PeerEvent, + now: Instant, + partial_serving: bool, + emit: &mut impl FnMut(PeerControl), ) { match event { + PeerEvent::SegmentedGossipResult(_) => {} PeerEvent::P2pNewConnection { p2p_peer_id, peer_id_full, ip, port, local_dial } => { self.on_connected(p2p_peer_id, peer_id_full, ip, port, now, emit, local_dial); } @@ -445,7 +458,7 @@ impl PeerManager { self.on_new_gossip(p2p_peer, topic, msg_hash, recv_ts, idontwant, emit); } PeerEvent::OutboundIHave { topic, digest, msg_count: _, protobuf } => { - self.on_outbound_ihave(topic, digest, protobuf, emit); + self.on_outbound_ihave(topic, digest, protobuf, partial_serving, emit); } PeerEvent::OutboundIWant { p2p_peer, iwant } => { self.on_outbound_iwant(p2p_peer, iwant, emit); @@ -467,6 +480,7 @@ impl PeerManager { topic, domain.digest(), protobuf, + partial_serving, emit, ); } diff --git a/crates/peer/src/manager/partial.rs b/crates/peer/src/manager/partial.rs new file mode 100644 index 000000000..02742b7cd --- /dev/null +++ b/crates/peer/src/manager/partial.rs @@ -0,0 +1,53 @@ +use silver_common::GossipTopic; + +use super::PeerManager; + +#[derive(Clone, Copy, Debug)] +pub struct PartialPeer { + pub connection: usize, + pub requests: bool, + pub meshed: bool, +} + +impl PeerManager { + pub fn partial_peer( + &self, + connection: usize, + topic: GossipTopic, + digest: [u8; 4], + ) -> Option { + if !self.active_gossip_digests.contains(&Some(digest)) || !self.our_topics.contains(&topic) + { + return None; + } + let peer = self.peers.get(&connection)?; + let caps = peer.subscriptions.get(&(digest, topic))?; + if !peer.partial_extensions || + !caps.supports_sending || + peer.gossip_gate_score() < self.params.gossip_threshold + { + return None; + } + Some(PartialPeer { + connection, + requests: caps.requests, + meshed: self + .mesh + .get(&topic) + .and_then(|m| m.get(digest)) + .is_some_and(|m| m.peers.contains(&connection)), + }) + } + + pub fn partial_peers( + &self, + topic: GossipTopic, + digest: [u8; 4], + ) -> impl Iterator + '_ { + self.peers.keys().filter_map(move |&peer| self.partial_peer(peer, topic, digest)) + } + + pub fn lazy_gossip_limit(&self) -> usize { + self.params.d_lazy as usize + } +} diff --git a/crates/peer/src/manager/promises.rs b/crates/peer/src/manager/promises.rs index 9363cd4de..d5584b0c6 100644 --- a/crates/peer/src/manager/promises.rs +++ b/crates/peer/src/manager/promises.rs @@ -213,7 +213,7 @@ impl PeerManager { } let local = LOCAL_GOSSIP_STREAM_ID.peer(); let SelfBuiltGossip { msg_id, domain, protobuf, idontwant } = built; - self.on_send_gossip(local, msg_id, topic, domain.digest(), protobuf, emit); + self.on_send_gossip(local, msg_id, topic, domain.digest(), protobuf, false, emit); self.fan_out_idontwant(topic, local, idontwant, emit); } @@ -255,6 +255,7 @@ impl PeerManager { topic: GossipTopic, digest: [u8; 4], protobuf: TCacheRead, + partial_serving: bool, emit: &mut impl FnMut(PeerControl), ) { let mesh_for_topic = self.mesh.get(&topic).and_then(|meshes| meshes.get(digest)); @@ -273,6 +274,12 @@ impl PeerManager { if peer.gossip_gate_score() < self.params.gossip_threshold { continue; } + if partial_serving && + peer.partial_extensions && + peer.subscriptions.get(&(digest, topic)).is_some_and(|caps| caps.requests) + { + continue; + } emit(PeerControl::P2pSend(P2pSend::Gossip(GossipMsgOut { peer_id: *conn, tcache: protobuf, @@ -303,6 +310,7 @@ impl PeerManager { } #[timed] + #[allow(clippy::too_many_arguments)] pub(super) fn on_send_gossip( &mut self, sender: usize, @@ -310,6 +318,7 @@ impl PeerManager { topic: GossipTopic, digest: [u8; 4], tcache: TCacheRead, + partial_serving: bool, emit: &mut impl FnMut(PeerControl), ) { let Some(meshed_peers) = self.mesh.get(&topic).and_then(|m| m.get(digest)) else { @@ -330,6 +339,12 @@ impl PeerManager { // dontwant continue; } + if partial_serving && + peer_state.partial_extensions && + peer_state.subscriptions.get(&(digest, topic)).is_some_and(|caps| caps.requests) + { + continue; + } peer_state.topic_stats.entry(topic).or_default().fanout_sent += 1; crate::counters::GossipTopicCounters::sent(topic); emit(PeerControl::P2pSend(P2pSend::Gossip(GossipMsgOut { peer_id: *peer, tcache }))); @@ -374,7 +389,7 @@ impl PeerManager { mod tests { use std::{io::Write as _, time::Duration}; - use silver_common::{PeerEvent, SyncUpdate, TCache, TCacheProducer}; + use silver_common::{PeerEvent, SszCache, SyncUpdate, TCache, TCacheProducer}; use silver_config::ScoreParams; use super::*; @@ -1264,4 +1279,58 @@ mod tests { .collect(); assert_eq!(recipients, vec![1, 2, 3]); } + + #[test] + fn partial_routing_requires_a_servable_source_and_exact_topic_capabilities() { + let now = Instant::now(); + let params = ScoreParams { d: 0, d_low: 0, d_high: 8, ..Default::default() }; + let topic = GossipTopic::DataColumnSidecar(4); + let (mut manager, mut captured) = fixture(vec![topic], params); + for id in 1..=3u8 { + connect(&mut manager, &mut captured, id as usize, id, now); + manager.handle_event( + PeerEvent::P2pGossipTopicSubscribe { p2p_peer: id as usize, topic, digest: [0; 4] }, + now, + &mut |_| {}, + ); + manager.handle_event( + PeerEvent::P2pGossipExtensions { p2p_peer: id as usize, partial_messages: true }, + now, + &mut |_| {}, + ); + manager.handle_event( + PeerEvent::P2pGossipPartialCaps { + p2p_peer: id as usize, + subnet: 4, + digest: [0; 4], + requests: id == 1, + supports_sending: id != 3, + }, + now, + &mut |_| {}, + ); + manager.test_mesh_extend(topic, [id as usize]); + } + let event = PeerEvent::SendGossip { + originator_stream_id: LOCAL_GOSSIP_STREAM_ID, + topic, + domain: test_domain(), + ssz_cache: SszCache::DataColumns, + msg_hash: MessageId { id: [0xcd; 20] }, + recv_ts: Nanos::now(), + protobuf: mk_tcache_read(), + ssz: mk_tcache_read(), + }; + for (servable, expected) in [(true, vec![2, 3]), (false, vec![1, 2, 3])] { + let mut recipients = Vec::new(); + manager.handle_event_with_partial(event, now, servable, &mut |event| { + if let PeerControl::P2pSend(P2pSend::Gossip(message)) = event { + recipients.push(message.peer_id); + } + }); + assert_eq!(recipients, expected); + } + assert!(manager.partial_peer(1, topic, [1; 4]).is_none()); + assert!(manager.partial_peer(1, GossipTopic::DataColumnSidecar(5), [0; 4]).is_none()); + } } diff --git a/crates/ssz/src/ssz_view/partial_column.rs b/crates/ssz/src/ssz_view/partial_column.rs index 549bb1830..b6dd87bd9 100644 --- a/crates/ssz/src/ssz_view/partial_column.rs +++ b/crates/ssz/src/ssz_view/partial_column.rs @@ -228,6 +228,11 @@ impl PartialDataColumnPartsMetadataView { /// Returns (available, requests); both bitlists must have exactly /// the trusted `n_rows` bits. pub fn check_size(buf: &[u8], n_rows: usize) -> Option<(u128, u128)> { + let (available, requests, rows) = Self::decode(buf)?; + (rows == n_rows).then_some((available, requests)) + } + + pub fn decode(buf: &[u8]) -> Option<(u128, u128, usize)> { if buf.len() < 10 { return None; } @@ -236,9 +241,9 @@ impl PartialDataColumnPartsMetadataView { if o0 != 8 || o1 < o0 || o1 > buf.len() { return None; } - let (available, n) = bitlist_u128(&buf[o0..o1], n_rows)?; - let (requests, n_req) = bitlist_u128(&buf[o1..], n_rows)?; - (n == n_rows && n_req == n_rows).then_some((available, requests)) + let (available, n) = bitlist_u128(&buf[o0..o1], 128)?; + let (requests, n_req) = bitlist_u128(&buf[o1..], 128)?; + (n == n_req).then_some((available, requests, n)) } } From 88dc02cbc2919fbad72cfa0ca4d60e321c71099a Mon Sep 17 00:00:00 2001 From: vladimir-ea Date: Fri, 18 Sep 2026 10:54:24 +0100 Subject: [PATCH 2/5] add partial requests counter --- crates/control/src/counters.rs | 1 + crates/control/src/partial_exchange.rs | 22 ++++--- .../src/partial_exchange/peer_column.rs | 57 ++++++++++++++++++- 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/crates/control/src/counters.rs b/crates/control/src/counters.rs index 62a27f890..38d2f88f9 100644 --- a/crates/control/src/counters.rs +++ b/crates/control/src/counters.rs @@ -20,5 +20,6 @@ silver_common::declare_counters! { PartialExchanges, PartialPendingFrames, PartialResponsesSent, + PartialCellsRequested, } } diff --git a/crates/control/src/partial_exchange.rs b/crates/control/src/partial_exchange.rs index 5ff2f75b5..d048861b2 100644 --- a/crates/control/src/partial_exchange.rs +++ b/crates/control/src/partial_exchange.rs @@ -156,18 +156,19 @@ impl PartialExchange { self.columns & (1u128 << group.column) == 0 || message.metadata.n_rows == 0 || message.metadata.n_rows > self.max_rows || - message.slot.is_some_and(|s| s != slot) || - peers - .partial_peer( - message.stream_id.peer(), - GossipTopic::DataColumnSidecar(group.column), - group.domain.digest(), - ) - .is_none() + message.slot.is_some_and(|s| s != slot) { ControlCounters::PartialMetadataIgnored.inc(); return; } + let Some(peer) = peers.partial_peer( + message.stream_id.peer(), + GossipTopic::DataColumnSidecar(group.column), + group.domain.digest(), + ) else { + ControlCounters::PartialMetadataIgnored.inc(); + return; + }; let column = ingress.availability(&group.block_root, group.column as usize, now); if column.is_some_and(|c| { c.domain != group.domain || @@ -185,7 +186,10 @@ impl PartialExchange { if exchange.remote.is_some() { ControlCounters::PartialMetadataReplaced.inc(); } - exchange.replace(message.metadata, message.slot); + let requested = exchange.replace(message.metadata, message.slot); + if peer.requests && requested != 0 { + ControlCounters::PartialCellsRequested.add(requested as u64); + } if column.is_some() && group.domain.format() == ForkName::Fulu { self.headers.known(key.peer, group); } diff --git a/crates/control/src/partial_exchange/peer_column.rs b/crates/control/src/partial_exchange/peer_column.rs index 3f0821a67..ff2665f25 100644 --- a/crates/control/src/partial_exchange/peer_column.rs +++ b/crates/control/src/partial_exchange/peer_column.rs @@ -39,14 +39,67 @@ impl PeerColumnExchange { } } - pub fn replace(&mut self, metadata: PartsMetadata, slot: Option) { + /// Returns newly requested cells, excluding cells the peer already has. + pub fn replace(&mut self, metadata: PartsMetadata, slot: Option) -> u32 { + let requested = metadata.requests & !metadata.available; + let previous = self.remote.map_or(0, |remote| remote.requests & !remote.available); // Repeated snapshots do not bypass the retransmission budget. - self.sent &= metadata.requests & !metadata.available; + self.sent &= requested; self.remote = Some(metadata); self.remote_slot = slot; + (requested & !previous).count_ones() } pub fn requested(&self, available: u128) -> u128 { self.remote.map_or(0, |remote| available & remote.requests & !remote.available & !self.sent) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn exchange() -> PeerColumnExchange { + let now = Instant::now(); + PeerColumnExchange::new(0, 128, now, now) + } + + fn metadata(available: u128, requests: u128) -> PartsMetadata { + PartsMetadata { available, requests, n_rows: 128 } + } + + #[test] + fn counts_new_requests_not_repeated_snapshots_or_available_cells() { + let mut exchange = exchange(); + assert_eq!(exchange.replace(metadata(0b0101, 0b1111), None), 2); + assert_eq!(exchange.replace(metadata(0b0101, 0b1111), None), 0); + exchange.sent = 0b1010; + assert_eq!(exchange.replace(metadata(0b0101, 0b1111), None), 0); + assert_eq!(exchange.sent, 0b1010); + assert_eq!(exchange.replace(metadata(0b0101, 0b1_1111), None), 1); + assert_eq!(exchange.requested(u128::MAX), 0b1_0000); + } + + #[test] + fn cancellation_then_rerequest_counts_new_demand() { + let mut exchange = exchange(); + assert_eq!(exchange.replace(metadata(0, 1), None), 1); + exchange.sent = 1; + assert_eq!(exchange.replace(metadata(0, 0), None), 0); + assert_eq!(exchange.sent, 0); + assert_eq!(exchange.replace(metadata(0, 1), None), 1); + assert_eq!(exchange.requested(1), 1); + assert_eq!(exchange.replace(metadata(1, 1), None), 0); + assert_eq!(exchange.replace(metadata(0, 1), None), 1); + } + + #[test] + fn counts_requests_before_local_availability_and_in_the_highest_row() { + let mut exchange = exchange(); + assert_eq!(exchange.replace(metadata(0, u128::MAX), Some(0)), 128); + assert_eq!(exchange.requested(0), 0); + assert_eq!(exchange.replace(metadata(u128::MAX, u128::MAX), Some(0)), 0); + assert_eq!(exchange.replace(metadata(0, 1 << 127), Some(0)), 1); + assert_eq!(exchange.requested(1 << 127), 1 << 127); + } +} From 4bccfea4cde98a4f85d63b292a07a8ef3e2c2c18 Mon Sep 17 00:00:00 2001 From: vladimir-ea Date: Fri, 18 Sep 2026 10:58:23 +0100 Subject: [PATCH 3/5] fix test --- crates/control/src/partial_exchange/tests/allocations.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/control/src/partial_exchange/tests/allocations.rs b/crates/control/src/partial_exchange/tests/allocations.rs index 3d4db355d..06cac15c5 100644 --- a/crates/control/src/partial_exchange/tests/allocations.rs +++ b/crates/control/src/partial_exchange/tests/allocations.rs @@ -37,6 +37,7 @@ static ALLOCATOR: CountingAllocator = CountingAllocator; #[test] fn serving_and_feedback_allocate_nothing_after_construction() { + ControlCounters::init().unwrap(); for format in [ForkName::Fulu, ForkName::Gloas] { let mut rig = Rig::new(format); rig.connect(1, true, true); From ea07dd7aa53ebfb3d673cca06e81b9cbca18ae96 Mon Sep 17 00:00:00 2001 From: vladimir-ea Date: Fri, 18 Sep 2026 17:56:06 +0100 Subject: [PATCH 4/5] remove receipts mechanism in favour of generalised 'SendDropped' event --- crates/bin/src/main.rs | 1 - crates/common/src/spine/messages.rs | 27 +- crates/common/src/spine/tcache/cache_frame.rs | 14 +- .../src/spine/tcache/cache_frame/tests.rs | 4 + crates/control/src/counters.rs | 10 +- crates/control/src/partial_exchange.rs | 221 ++++++------- .../control/src/partial_exchange/headers.rs | 30 +- .../src/partial_exchange/peer_column.rs | 2 - .../src/partial_exchange/peer_exchange.rs | 81 +++++ .../control/src/partial_exchange/response.rs | 12 +- crates/control/src/partial_exchange/tests.rs | 230 ++++++++++++-- .../src/partial_exchange/tests/allocations.rs | 5 +- crates/control/src/tile/tests/partial.rs | 12 +- crates/gossip/src/partial.rs | 78 +++-- crates/network/benches/quic_basic.rs | 2 +- crates/network/benches/quic_pingpong.rs | 12 +- crates/network/src/lib.rs | 4 + crates/network/src/p2p/mod.rs | 40 ++- crates/network/src/p2p/quic/gossip_frame.rs | 47 +-- .../src/p2p/quic/gossip_frame/tests.rs | 9 +- crates/network/src/p2p/quic/mod.rs | 24 +- crates/network/src/p2p/quic/peer.rs | 292 ++++++++++++++---- crates/network/src/p2p/quic/send_receipts.rs | 126 -------- crates/network/src/p2p/streams/gossip_out.rs | 2 +- crates/network/src/p2p/streams/rpc/mod.rs | 72 ++++- crates/network/src/p2p/streams/rpc/tests.rs | 49 +++ crates/network/src/tile.rs | 42 ++- crates/peer/src/manager/admission.rs | 6 +- crates/peer/src/manager/mod.rs | 8 +- crates/peer/src/manager/rpc.rs | 31 ++ 30 files changed, 987 insertions(+), 506 deletions(-) create mode 100644 crates/control/src/partial_exchange/peer_exchange.rs delete mode 100644 crates/network/src/p2p/quic/send_receipts.rs create mode 100644 crates/network/src/p2p/streams/rpc/tests.rs diff --git a/crates/bin/src/main.rs b/crates/bin/src/main.rs index 5e5ce87f7..b89382138 100644 --- a/crates/bin/src/main.rs +++ b/crates/bin/src/main.rs @@ -257,7 +257,6 @@ fn main() -> Result<(), Box> { } } - // Partial receiving remains gated by configuration validation. let partial_columns = config.partial_columns().map_err(|error| format!("partial columns config: {error:?}"))?; diff --git a/crates/common/src/spine/messages.rs b/crates/common/src/spine/messages.rs index 4a64b3baa..18dfe7db9 100644 --- a/crates/common/src/spine/messages.rs +++ b/crates/common/src/spine/messages.rs @@ -386,7 +386,6 @@ impl RpcOutbound { #[repr(C, u8)] #[allow(clippy::large_enum_variant)] pub enum PeerEvent { - SegmentedGossipResult(GossipFrameResult), /// Peer_id_full contains the secp256k1 pubkey and can be used to derive /// discovery id P2pNewConnection { @@ -444,7 +443,7 @@ pub enum PeerEvent { P2pOutboundMessageDropped { p2p_peer: usize, protocol: StreamProtocol, - rpc_request: bool, + msg: P2pSend, }, P2pGossipTopicSubscribe { p2p_peer: usize, @@ -788,27 +787,17 @@ pub enum RpcSeverity { #[allow(clippy::large_enum_variant)] pub enum P2pSend { Gossip(GossipMsgOut), - SegmentedGossip { peer_id: usize, frame: CacheFrameRef }, + SegmentedGossip { + peer_id: usize, + frame: CacheFrameRef, + /// Count cells when Network finishes writing a partial response. + /// None for generic frames and best-effort availability withdrawals. + partial_cells: Option, + }, Identify(usize), Rpc(RpcOutbound), } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum GossipFrameOutcome { - /// Entire frame accepted by the transport, not acknowledged by the peer. - Written { - stream_id: P2pStreamId, - }, - Dropped, -} - -#[derive(Clone, Copy, Debug)] -pub struct GossipFrameResult { - pub p2p_peer: usize, - pub frame_seq: u64, - pub outcome: GossipFrameOutcome, -} - impl P2pSend { pub fn peer_id(&self) -> usize { match self { diff --git a/crates/common/src/spine/tcache/cache_frame.rs b/crates/common/src/spine/tcache/cache_frame.rs index cec657448..ff9a41c57 100644 --- a/crates/common/src/spine/tcache/cache_frame.rs +++ b/crates/common/src/spine/tcache/cache_frame.rs @@ -167,7 +167,14 @@ impl CacheFrameRef { return Err(CacheFrameError::InvalidDescriptor); } let descriptor_len = buffer.len(); - let view = CacheFrameView { read, count, wire_len, framing_start, descriptor_len }; + let view = CacheFrameView { + read, + expires: self.expires, + count, + wire_len, + framing_start, + descriptor_len, + }; let mut total = 0usize; for segment in view.segments() { if segment.length == 0 || @@ -192,6 +199,7 @@ impl CacheFrameRef { #[derive(Debug)] pub struct CacheFrameView { read: AcquiredRead, + expires: Instant, count: usize, wire_len: usize, framing_start: usize, @@ -199,6 +207,10 @@ pub struct CacheFrameView { } impl CacheFrameView { + pub fn reference(&self) -> CacheFrameRef { + CacheFrameRef { descriptor: self.read.read, expires: self.expires } + } + pub fn wire_len(&self) -> usize { self.wire_len } diff --git a/crates/common/src/spine/tcache/cache_frame/tests.rs b/crates/common/src/spine/tcache/cache_frame/tests.rs index 928e58c69..771f5fa4d 100644 --- a/crates/common/src/spine/tcache/cache_frame/tests.rs +++ b/crates/common/src/spine/tcache/cache_frame/tests.rs @@ -35,6 +35,10 @@ fn copy_handle_round_trips_framing_and_source_ranges() { ) .unwrap(); let view = frame.acquire(&mut consumer, now).unwrap(); + let restored = view.reference(); + assert_eq!(restored.read().seq(), frame.read().seq()); + assert_eq!(restored.read().cache_ref().cache, frame.read().cache_ref().cache); + assert_eq!(restored.expires, frame.expires); assert_eq!(view.wire_len(), 8); assert_eq!(view.segment_count(), 3); let descriptor = view.descriptor_range(); diff --git a/crates/control/src/counters.rs b/crates/control/src/counters.rs index 38d2f88f9..915c4f71d 100644 --- a/crates/control/src/counters.rs +++ b/crates/control/src/counters.rs @@ -1,4 +1,5 @@ silver_common::declare_counters! { + #[allow(non_camel_case_types)] pub ControlCounters => "control" { TailUnavailable, RangesIssued, @@ -13,13 +14,14 @@ silver_common::declare_counters! { PartialMetadataIgnored, PartialStateLimited, PartialFramesQueued, - PartialFramesWritten, + _Reserved_PartialFramesWritten, PartialFramesDropped, - PartialCellsServed, + _Reserved_PartialCellsServed, PartialWithdrawals, PartialExchanges, - PartialPendingFrames, - PartialResponsesSent, + _Reserved_PartialPendingFrames, + _Reserved_PartialResponsesSent, PartialCellsRequested, + PartialRateLimited, } } diff --git a/crates/control/src/partial_exchange.rs b/crates/control/src/partial_exchange.rs index d048861b2..9cf8d2d4e 100644 --- a/crates/control/src/partial_exchange.rs +++ b/crates/control/src/partial_exchange.rs @@ -5,7 +5,7 @@ use std::{ use fxhash::FxHashMap; use silver_common::{ - ForkName, GossipFrameOutcome, GossipFrameResult, GossipTopic, P2pSend, PeerEvent, TProducer, + ForkName, GossipTopic, P2pSend, PeerEvent, TProducer, cell_store::{CellStoreConfig, ColumnAvailability}, }; use silver_gossip::{ColumnGroupKey, PartialMetadataReceived, PartsMetadata}; @@ -14,33 +14,26 @@ use silver_peer::PeerManager; use self::{ headers::HeaderTracker, peer_column::{ExchangeKey, PeerColumnExchange}, + peer_exchange::PeerExchange, response::PartialResponse, }; use crate::{ControlCounters, cell_ingress::CellIngress}; mod headers; mod peer_column; +mod peer_exchange; mod response; #[cfg(test)] mod tests; const WORK_PER_SPIN: usize = 64; -const MAX_PENDING: usize = 128; const MAX_FRAMES_PER_GROUP: u8 = 32; const HEARTBEAT: Duration = Duration::from_millis(500); const RETRY: Duration = Duration::from_millis(100); -struct PendingPartialSend { - key: ExchangeKey, - rows: u128, - available: u128, - header: bool, -} - pub(crate) struct PartialExchange { exchanges: FxHashMap, - peer_counts: FxHashMap, - pending: FxHashMap<(usize, u64), PendingPartialSend>, + peer_exchanges: FxHashMap, headers: HeaderTracker, ready: VecDeque, capacity: usize, @@ -57,8 +50,7 @@ impl PartialExchange { let capacity = (peer_capacity * 32).min(8192); Self { exchanges: FxHashMap::with_capacity_and_hasher(capacity, Default::default()), - peer_counts: FxHashMap::with_capacity_and_hasher(capacity, Default::default()), - pending: FxHashMap::with_capacity_and_hasher(MAX_PENDING, Default::default()), + peer_exchanges: FxHashMap::with_capacity_and_hasher(capacity, Default::default()), headers: HeaderTracker::new(capacity), ready: VecDeque::with_capacity(capacity), capacity, @@ -81,13 +73,16 @@ impl PartialExchange { if self.exchanges.contains_key(&key) { return true; } - let count = self.peer_counts.get(&key.peer).copied().unwrap_or(0); - if self.exchanges.len() >= self.capacity || count >= self.peer_capacity { + let peer = self.peer_exchanges.get(&key.peer); + if self.exchanges.len() >= self.capacity || + peer.is_some_and(|p| p.columns >= self.peer_capacity) || + (peer.is_none() && self.peer_exchanges.len() >= self.capacity) + { ControlCounters::PartialStateLimited.inc(); return false; } self.exchanges.insert(key, PeerColumnExchange::new(slot, rows, expires, now)); - self.peer_counts.insert(key.peer, count + 1); + self.peer_exchanges.entry(key.peer).or_default().columns += 1; true } @@ -198,9 +193,28 @@ impl PartialExchange { pub fn peer_event(&mut self, event: &PeerEvent, now: Instant) { match *event { - PeerEvent::SegmentedGossipResult(result) => self.completed(result, now), - PeerEvent::P2pDisconnect { p2p_peer, .. } | - PeerEvent::P2pGossipExtensions { p2p_peer, .. } => self.remove_peer(p2p_peer), + PeerEvent::P2pOutboundMessageDropped { + msg: P2pSend::SegmentedGossip { peer_id, frame, partial_cells }, + .. + } => { + if partial_cells.is_some() { + ControlCounters::PartialFramesDropped.inc(); + } + // Only inspect the descriptor's sequence: its TCache bytes may + // already have expired. One failure resets this peer's batch. + if self + .peer_exchanges + .get_mut(&peer_id) + .is_some_and(|peer| peer.dropped(frame.read().seq())) + { + self.retry_peer(peer_id, now); + } + } + PeerEvent::P2pDisconnect { p2p_peer, .. } => self.remove_peer(p2p_peer), + PeerEvent::P2pGossipExtensions { p2p_peer, .. } => { + self.remove_where(|key| key.peer == p2p_peer); + self.headers.remove_peer(p2p_peer); + } PeerEvent::P2pNewConnection { p2p_peer_id, .. } => self.remove_peer(p2p_peer_id), PeerEvent::P2pGossipTopicUnsubscribe { p2p_peer, @@ -215,39 +229,30 @@ impl PartialExchange { }); } PeerEvent::P2pStreamClosed { stream_id } if stream_id.protocol().is_gossip() => { - self.headers.reset_stream(stream_id); - self.remove_peer(stream_id.peer()); + self.retry_peer(stream_id.peer(), now); } _ => {} } } - fn completed(&mut self, result: GossipFrameResult, now: Instant) { - let Some(pending) = self.pending.remove(&(result.p2p_peer, result.frame_seq)) else { - return - }; - let written = match result.outcome { - GossipFrameOutcome::Written { stream_id } => Some(stream_id), - GossipFrameOutcome::Dropped => None, - }; - if pending.header { - self.headers.complete(result.p2p_peer, pending.key.group, result.frame_seq, written); + fn retry_peer(&mut self, peer: usize, now: Instant) { + self.headers.retry_peer(peer); + if let Some(exchange) = self.peer_exchanges.get_mut(&peer) { + exchange.reset_sends(); } - if let Some(exchange) = self.exchanges.get_mut(&pending.key) { - exchange.pending = false; - if written.is_some() { - exchange.sent |= pending.rows; - exchange.advertised = Some(pending.available); - ControlCounters::PartialFramesWritten.inc(); - if pending.rows != 0 { - ControlCounters::PartialResponsesSent.inc(); - ControlCounters::PartialCellsServed.add(pending.rows.count_ones() as u64); - } - } else { - exchange.retry_at = now + RETRY; - ControlCounters::PartialFramesDropped.inc(); + for (key, exchange) in &mut self.exchanges { + if key.peer != peer { + continue; + } + // Preserve the latest remote requests, but no longer assume our + // availability, cells or headers reached this peer. No quota refund. + exchange.sent = 0; + exchange.advertised = None; + exchange.retry_at = now + RETRY; + if !exchange.scheduled { + exchange.scheduled = true; + self.ready.push_back(*key); } - self.schedule(pending.key); } } @@ -256,20 +261,10 @@ impl PartialExchange { if !predicate(key) { return true; } - let count = self.peer_counts.get_mut(&key.peer).unwrap(); - *count -= 1; - if *count == 0 { - self.peer_counts.remove(&key.peer); - } - false - }); - self.pending.retain(|&(peer, seq), pending| { - if self.exchanges.contains_key(&pending.key) { - return true; - } - if pending.header { - self.headers.complete(peer, pending.key.group, seq, None); - } + // Keep the spent per-peer budget until the next heartbeat, even + // when unsubscribing removes the peer's last column exchange. + self.peer_exchanges.get_mut(&key.peer).unwrap().columns -= 1; + self.headers.forget_sent(key.peer, key.group); false }); self.ready.retain(|key| self.exchanges.contains_key(key)); @@ -278,6 +273,7 @@ impl PartialExchange { fn remove_peer(&mut self, peer: usize) { self.remove_where(|key| key.peer == peer); self.headers.remove_peer(peer); + self.peer_exchanges.remove(&peer); } pub fn reject(&mut self, root: &[u8; 32]) { @@ -300,34 +296,17 @@ impl PartialExchange { emit: &mut impl FnMut(P2pSend), ) -> bool { self.advance_slot(ingress, peers, producer, now, emit); - if now >= self.next_heartbeat { - self.next_heartbeat = now + HEARTBEAT; - self.remove_where(|key| { - peers - .partial_peer( - key.peer, - GossipTopic::DataColumnSidecar(key.group.column), - key.group.domain.digest(), - ) - .is_none() - }); - for column in ingress.columns(now) { - self.available(column, peers, now, true); - } - } let mut did_work = false; for _ in 0..self.ready.len().min(WORK_PER_SPIN) { - if self.pending.len() >= MAX_PENDING { - break; - } let key = self.ready.pop_front().unwrap(); let exchange = self.exchanges.get_mut(&key).unwrap(); exchange.scheduled = false; - if exchange.pending || - now < exchange.retry_at || - now >= exchange.expires || - exchange.frames_sent >= MAX_FRAMES_PER_GROUP - { + if now < exchange.retry_at { + exchange.scheduled = true; + self.ready.push_back(key); + continue; + } + if now >= exchange.expires || exchange.frames_sent >= MAX_FRAMES_PER_GROUP { continue; } let Some(peer) = peers.partial_peer( @@ -368,32 +347,37 @@ impl PartialExchange { rows, header, }; - let frame = match response.write(producer, column.expires) { - Ok(frame) => frame, - Err(_) => { - exchange.retry_at = now + RETRY; - ControlCounters::PartialFramesDropped.inc(); - continue; - } - }; + let budget = self.peer_exchanges.get_mut(&key.peer).unwrap(); + let (frame, bytes) = + match response.write(producer, column.expires, budget.remaining_bytes()) { + Ok(Some(frame)) => frame, + Ok(None) => { + ControlCounters::PartialRateLimited.inc(); + continue; + } + Err(_) => { + exchange.retry_at = now + RETRY; + ControlCounters::PartialFramesDropped.inc(); + continue; + } + }; let seq = frame.read().seq(); - exchange.pending = true; + budget.record(seq, bytes); + exchange.sent |= rows; + exchange.advertised = Some(column.available); exchange.frames_sent += 1; - self.pending.insert((key.peer, seq), PendingPartialSend { - key, - rows, - available: column.available, - header, - }); if header { - self.headers.queued(key.peer, key.group, seq); + self.headers.sent(key.peer, key.group); } ControlCounters::PartialFramesQueued.inc(); - emit(P2pSend::SegmentedGossip { peer_id: key.peer, frame }); + emit(P2pSend::SegmentedGossip { + peer_id: key.peer, + frame, + partial_cells: Some(rows.count_ones() as u8), + }); did_work = true; } ControlCounters::PartialExchanges.set(self.exchanges.len() as u64); - ControlCounters::PartialPendingFrames.set(self.pending.len() as u64); did_work } @@ -405,10 +389,32 @@ impl PartialExchange { now: Instant, emit: &mut impl FnMut(P2pSend), ) { + let heartbeat = now >= self.next_heartbeat; + if heartbeat { + self.next_heartbeat = now + HEARTBEAT; + self.peer_exchanges.retain(|_, peer| { + peer.heartbeat(); + peer.columns != 0 + }); + } let (slot, _) = ingress.slot_window(); if slot != self.slot { self.expire(slot, peers, producer, now, emit); } + if heartbeat { + self.remove_where(|key| { + peers + .partial_peer( + key.peer, + GossipTopic::DataColumnSidecar(key.group.column), + key.group.domain.digest(), + ) + .is_none() + }); + for column in ingress.columns(now) { + self.available(column, peers, now, true); + } + } } fn expire( @@ -443,17 +449,22 @@ impl PartialExchange { rows: 0, header: false, }; - if let Ok(frame) = response.write(producer, now + RETRY) { - emit(P2pSend::SegmentedGossip { peer_id: key.peer, frame }); + let budget = self.peer_exchanges.get_mut(&key.peer).unwrap(); + if let Ok(Some((frame, bytes))) = + response.write(producer, now + RETRY, budget.remaining_bytes()) + { + budget.record(frame.read().seq(), bytes); + emit(P2pSend::SegmentedGossip { peer_id: key.peer, frame, partial_cells: None }); ControlCounters::PartialWithdrawals.inc(); } } self.exchanges.clear(); - self.peer_counts.clear(); - self.pending.clear(); + for peer in self.peer_exchanges.values_mut() { + peer.columns = 0; + peer.reset_sends(); + } self.headers.clear(); self.ready.clear(); self.slot = slot; - self.next_heartbeat = now; } } diff --git a/crates/control/src/partial_exchange/headers.rs b/crates/control/src/partial_exchange/headers.rs index b889091e5..595f8d6a5 100644 --- a/crates/control/src/partial_exchange/headers.rs +++ b/crates/control/src/partial_exchange/headers.rs @@ -1,5 +1,5 @@ use fxhash::FxHashMap; -use silver_common::{GossipDomain, P2pStreamId}; +use silver_common::GossipDomain; use silver_gossip::ColumnGroupKey; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -11,8 +11,7 @@ struct HeaderKey { #[derive(Clone, Copy)] enum HeaderState { - Queued(u64), - Written(P2pStreamId), + Sent, Known, } @@ -37,10 +36,10 @@ impl HeaderTracker { !self.entries.contains_key(&Self::key(peer, group)) } - pub fn queued(&mut self, peer: usize, group: ColumnGroupKey, seq: u64) { + pub fn sent(&mut self, peer: usize, group: ColumnGroupKey) { let key = Self::key(peer, group); if self.entries.len() < self.capacity || self.entries.contains_key(&key) { - self.entries.insert(key, HeaderState::Queued(seq)); + self.entries.entry(key).or_insert(HeaderState::Sent); } } @@ -51,26 +50,15 @@ impl HeaderTracker { } } - pub fn complete( - &mut self, - peer: usize, - group: ColumnGroupKey, - seq: u64, - stream: Option, - ) { + pub fn forget_sent(&mut self, peer: usize, group: ColumnGroupKey) { let key = Self::key(peer, group); - if matches!(self.entries.get(&key), Some(HeaderState::Queued(pending)) if *pending == seq) { - if let Some(stream) = stream { - self.entries.insert(key, HeaderState::Written(stream)); - } else { - self.entries.remove(&key); - } + if matches!(self.entries.get(&key), Some(HeaderState::Sent)) { + self.entries.remove(&key); } } - pub fn reset_stream(&mut self, stream: P2pStreamId) { - self.entries - .retain(|_, state| !matches!(state, HeaderState::Written(sent) if *sent == stream)); + pub fn retry_peer(&mut self, peer: usize) { + self.entries.retain(|key, state| key.peer != peer || matches!(state, HeaderState::Known)); } pub fn remove_peer(&mut self, peer: usize) { diff --git a/crates/control/src/partial_exchange/peer_column.rs b/crates/control/src/partial_exchange/peer_column.rs index ff2665f25..24270f7d6 100644 --- a/crates/control/src/partial_exchange/peer_column.rs +++ b/crates/control/src/partial_exchange/peer_column.rs @@ -13,7 +13,6 @@ pub(super) struct PeerColumnExchange { pub remote_slot: Option, pub advertised: Option, pub sent: u128, - pub pending: bool, pub scheduled: bool, pub retry_at: Instant, pub expires: Instant, @@ -29,7 +28,6 @@ impl PeerColumnExchange { remote_slot: None, advertised: None, sent: 0, - pending: false, scheduled: false, retry_at: now, expires, diff --git a/crates/control/src/partial_exchange/peer_exchange.rs b/crates/control/src/partial_exchange/peer_exchange.rs new file mode 100644 index 000000000..e154bad69 --- /dev/null +++ b/crates/control/src/partial_exchange/peer_exchange.rs @@ -0,0 +1,81 @@ +const MAX_FRAMES: usize = 16; +const MAX_BYTES: usize = 4 * 1024 * 1024; + +#[derive(Default)] +pub(super) struct PeerExchange { + pub columns: usize, + frames: usize, + bytes: usize, + sent: Option<(u64, u64)>, +} + +impl PeerExchange { + pub fn remaining_bytes(&self) -> usize { + if self.frames == MAX_FRAMES { 0 } else { MAX_BYTES - self.bytes } + } + + pub fn record(&mut self, seq: u64, bytes: usize) { + assert!(self.frames < MAX_FRAMES && bytes <= MAX_BYTES - self.bytes); + self.frames += 1; + self.bytes += bytes; + let range = self.sent.get_or_insert((seq, seq)); + range.1 = seq; + } + + pub fn dropped(&mut self, seq: u64) -> bool { + if !self.sent.is_some_and(|(first, last)| (first..=last).contains(&seq)) { + return false; + } + // One peer-wide reset covers every frame submitted before this failure. + // Later failures from that batch must not invalidate its replacements. + self.sent = None; + true + } + + pub fn reset_sends(&mut self) { + self.sent = None; + } + + pub fn heartbeat(&mut self) { + self.frames = 0; + self.bytes = 0; + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn limits_count_all_sends_without_refunding_drops() { + let mut peer = PeerExchange::default(); + for seq in 0..MAX_FRAMES { + assert!(peer.remaining_bytes() > 0); + peer.record(seq as u64, 1); + assert!(peer.dropped(seq as u64)); + } + assert_eq!(peer.remaining_bytes(), 0); + peer.heartbeat(); + assert_eq!(peer.remaining_bytes(), MAX_BYTES); + peer.record(100, MAX_BYTES); + assert_eq!(peer.remaining_bytes(), 0); + peer.heartbeat(); + assert_eq!(peer.remaining_bytes(), MAX_BYTES); + } + + #[test] + fn failure_batches_and_stale_drops_do_not_reset_new_sends() { + let mut peer = PeerExchange::default(); + peer.record(10, 1); + peer.record(20, 1); + assert!(peer.dropped(10)); + assert!(!peer.dropped(20)); + peer.record(30, 1); + assert!(!peer.dropped(20)); + assert!(!peer.dropped(40)); + assert!(peer.dropped(30)); + peer.record(40, 1); + peer.reset_sends(); + assert!(!peer.dropped(40)); + } +} diff --git a/crates/control/src/partial_exchange/response.rs b/crates/control/src/partial_exchange/response.rs index 535f8b14b..a4eec2872 100644 --- a/crates/control/src/partial_exchange/response.rs +++ b/crates/control/src/partial_exchange/response.rs @@ -30,7 +30,8 @@ impl PartialResponse { &self, producer: &mut TProducer, expires: Instant, - ) -> Result { + max_bytes: usize, + ) -> Result, CacheFrameError> { let mut topic = Cursor::new([0u8; 96]); let digest = self.group.domain.digest(); write!( @@ -84,12 +85,17 @@ impl PartialResponse { }), metadata: Some(self.metadata), }; - frame.write( + let wire_len = frame.wire_len(); + if wire_len > max_bytes { + return Ok(None); + } + let frame = frame.write( producer, CellSegments { column: self.column, rows: self.rows, proof: false }, CellSegments { column: self.column, rows: self.rows, proof: true }, expires, - ) + )?; + Ok(Some((frame, wire_len))) } } diff --git a/crates/control/src/partial_exchange/tests.rs b/crates/control/src/partial_exchange/tests.rs index 1ba62452f..02ec301fe 100644 --- a/crates/control/src/partial_exchange/tests.rs +++ b/crates/control/src/partial_exchange/tests.rs @@ -202,7 +202,7 @@ impl Rig { fn spin(&mut self) -> Vec<(usize, CacheFrameRef)> { let mut frames = Vec::new(); self.exchange.spin(&self.ingress, &self.peers, &mut self.output, self.now, &mut |event| { - let P2pSend::SegmentedGossip { peer_id, frame } = event else { + let P2pSend::SegmentedGossip { peer_id, frame, .. } = event else { panic!("unexpected send") }; frames.push((peer_id, frame)); @@ -226,19 +226,13 @@ impl Rig { wire } - fn complete(&mut self, peer: usize, frame: CacheFrameRef, written: bool) { + fn dropped(&mut self, peer: usize, frame: CacheFrameRef) { self.exchange.peer_event( - &PeerEvent::SegmentedGossipResult(GossipFrameResult { + &PeerEvent::P2pOutboundMessageDropped { p2p_peer: peer, - frame_seq: frame.read().seq(), - outcome: if written { - GossipFrameOutcome::Written { - stream_id: P2pStreamId::new(peer, 4, StreamProtocol::GossipSubV13, false), - } - } else { - GossipFrameOutcome::Dropped - }, - }), + protocol: StreamProtocol::GossipSub, + msg: P2pSend::SegmentedGossip { peer_id: peer, frame, partial_cells: Some(0) }, + }, self.now, ); } @@ -277,9 +271,8 @@ fn retained_full_columns_serve_requested_rows_for_both_forks_and_non_mesh_peers( assert_eq!(&cells[BYTES_PER_CELL..], &[3; BYTES_PER_CELL]); assert_eq!(&proofs[..BYTES_PER_KZG_PROOF], &[0x11; BYTES_PER_KZG_PROOF]); assert_eq!(&proofs[BYTES_PER_KZG_PROOF..], &[0x13; BYTES_PER_KZG_PROOF]); - rig.complete(1, frames[0].1, true); rig.request(1, 0b0101, 0b1111); - assert!(rig.spin().is_empty(), "repeated request must not resend written rows"); + assert!(rig.spin().is_empty(), "repeated request must not resend submitted rows"); } } @@ -300,7 +293,6 @@ fn snapshots_replace_and_available_request_bits_do_not_request_data() { ), Some(0b0010) ); - rig.complete(1, frames[0].1, true); rig.request(1, 0b0100, 0b0100); assert!(rig.spin().is_empty()); } @@ -336,16 +328,28 @@ fn fulu_header_is_shared_across_topics_and_failed_send_is_retried() { let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); let has_header = rpc.partial.as_option().unwrap().partial_message.is_some(); headers += usize::from(has_header); - rig.complete(peer, frame, !has_header); + if has_header { + rig.dropped(peer, frame); + } } assert_eq!(headers, 1); - rig.now += HEARTBEAT; + rig.now += RETRY; let frames = rig.spin(); - assert_eq!(frames.len(), 1); - let wire = rig.wire(frames[0].1); - let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); - assert!(rpc.partial.as_option().unwrap().partial_message.is_some()); - rig.complete(1, frames[0].1, true); + assert_eq!(frames.len(), 2, "a drop resets all optimistic state for this peer"); + let headers = frames + .into_iter() + .filter(|(_, frame)| { + let wire = rig.wire(*frame); + protobuf::RPCView::decode_view(&wire) + .unwrap() + .partial + .as_option() + .unwrap() + .partial_message + .is_some() + }) + .count(); + assert_eq!(headers, 1); rig.publish(0); rig.publish(1); assert!(rig.spin().is_empty()); @@ -381,8 +385,7 @@ fn expiry_withdraws_without_reading_expired_payloads() { rig.connect(1, true, false); rig.publish(0); rig.request(1, 0, 1); - let frames = rig.spin(); - rig.complete(1, frames[0].1, true); + assert_eq!(rig.spin().len(), 1); rig.now += Duration::from_secs(12); let event = rig.ingress.allocator_mut().advance(rig.now, 0).unwrap(); rig.network.advance_retention(event.retain_from); @@ -398,11 +401,11 @@ fn expiry_withdraws_without_reading_expired_payloads() { Some((0, 0)) ); assert!(rig.exchange.exchanges.is_empty()); - assert!(rig.exchange.pending.is_empty()); + assert!(rig.exchange.ready.is_empty()); } #[test] -fn disconnect_clears_pending_state_and_late_results_cannot_mark_a_header_sent() { +fn disconnect_clears_state_and_late_drops_do_not_restore_it() { let mut rig = Rig::new(ForkName::Fulu); rig.connect(1, true, true); rig.publish(0); @@ -412,13 +415,14 @@ fn disconnect_clears_pending_state_and_late_results_cannot_mark_a_header_sent() assert!(!rig.exchange.headers.needed(1, group)); rig.exchange .peer_event(&PeerEvent::P2pDisconnect { p2p_peer: 1, peer_id: PeerId::default() }, rig.now); - assert!(rig.exchange.pending.is_empty()); - rig.complete(1, frames[0].1, true); + assert!(rig.exchange.exchanges.is_empty()); + assert!(rig.exchange.peer_exchanges.is_empty()); + rig.dropped(1, frames[0].1); assert!(rig.exchange.headers.needed(1, group)); } #[test] -fn withdrawn_capabilities_discard_pending_headers_and_late_feedback() { +fn withdrawn_capabilities_discard_sent_headers_and_late_drops() { let mut rig = Rig::new(ForkName::Fulu); rig.connect(1, true, true); rig.publish(0); @@ -432,7 +436,7 @@ fn withdrawn_capabilities_discard_pending_headers_and_late_feedback() { }; rig.exchange.peer_event(&event, rig.now); rig.peers.handle_event(event, rig.now, &mut |_| {}); - rig.complete(1, frames[0].1, true); + rig.dropped(1, frames[0].1); assert!(rig.exchange.headers.needed(1, rig.message(1, 0, 0).group)); rig.request(1, 0, 1); assert!(rig.spin().is_empty()); @@ -446,8 +450,7 @@ fn stale_requests_after_an_ignored_withdrawal_never_revive_expired_cells() { rig.connect(1, true, false); rig.publish(0); rig.request(1, 0, 1); - let frames = rig.spin(); - rig.complete(1, frames[0].1, true); + assert_eq!(rig.spin().len(), 1); rig.now += Duration::from_secs(12); rig.ingress.allocator_mut().advance(rig.now, 0).unwrap(); assert_eq!(rig.spin().len(), 1); @@ -459,7 +462,7 @@ fn stale_requests_after_an_ignored_withdrawal_never_revive_expired_cells() { assert!(rig.spin().is_empty()); } assert_eq!(rig.ingress.producer_mut().next_seq(), seq); - assert!(rig.exchange.pending.is_empty()); + assert!(rig.exchange.ready.is_empty()); } } @@ -489,3 +492,164 @@ fn queued_frames_keep_their_original_domain_across_fork_and_digest_changes() { assert!(rig.spin().is_empty()); assert!(rig.exchange.exchanges.is_empty()); } + +#[test] +fn drops_retry_latest_requests_only_and_coalesce_without_disturbing_other_peers() { + let mut rig = Rig::new(ForkName::Fulu); + rig.connect(1, true, false); + rig.connect(2, true, false); + rig.publish(0); + rig.request(1, 0, 1); + rig.request(2, 0, 1); + let first = rig.spin(); + assert_eq!(first.len(), 2); + let dropped = first.iter().find(|(peer, _)| *peer == 1).unwrap().1; + rig.request(1, 0, 2); + let second = rig.spin(); + assert_eq!(second.len(), 1); + + rig.dropped(1, dropped); + assert!(rig.spin().is_empty(), "retry waits for its backoff"); + rig.now += RETRY; + let retried = rig.spin(); + assert_eq!(retried.len(), 1); + assert_eq!(retried[0].0, 1); + let wire = rig.wire(retried[0].1); + let rpc = protobuf::RPCView::decode_view(&wire).unwrap(); + let payload = rpc.partial.as_option().unwrap().partial_message.unwrap(); + assert_eq!(PartialDataColumnSidecarFuluView::check_size(payload, ROWS), Some(2)); + assert!( + PartialDataColumnSidecarFuluView::header(payload).is_empty(), + "known headers survive retries" + ); + + rig.dropped(1, second[0].1); + rig.now += RETRY; + assert!( + rig.spin().is_empty(), + "another drop from the old batch must not reset its replacement" + ); + rig.request(2, 0, 1); + assert!(rig.spin().is_empty()); + + rig.dropped(1, retried[0].1); + rig.now += RETRY; + assert_eq!(rig.spin().len(), 1, "a dropped replacement still triggers recovery"); +} + +#[test] +fn stream_closure_preserves_requests_for_the_replacement_stream() { + let mut rig = Rig::new(ForkName::Gloas); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 3); + let first = rig.spin(); + assert_eq!(first.len(), 1); + rig.exchange.peer_event( + &PeerEvent::P2pStreamClosed { + stream_id: P2pStreamId::new(1, 4, StreamProtocol::GossipSubV13, false), + }, + rig.now, + ); + rig.now += RETRY; + let retried = rig.spin(); + assert_eq!(retried.len(), 1); + assert_eq!(rig.wire(first[0].1), rig.wire(retried[0].1)); + rig.dropped(1, first[0].1); + rig.now += RETRY; + assert!(rig.spin().is_empty()); +} + +#[test] +fn heartbeat_frame_quota_is_per_peer_and_drops_do_not_refund_it() { + let mut rig = Rig::new(ForkName::Gloas); + rig.connect(1, true, false); + rig.publish(0); + let mut last = None; + for attempt in 0..16 { + rig.request(1, 0, 1 << (attempt % ROWS)); + let frames = rig.spin(); + assert_eq!(frames.len(), 1); + last = Some(frames[0].1); + } + rig.request(1, 0, 1); + let seq = rig.output.next_seq(); + assert!(rig.spin().is_empty()); + assert_eq!( + rig.output.next_seq(), + seq, + "rate limiting must happen before reserving a descriptor" + ); + + rig.connect(2, true, false); + rig.request(2, 0, 1); + let frames = rig.spin(); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].0, 2); + + rig.dropped(1, last.unwrap()); + rig.now += RETRY; + assert!(rig.spin().is_empty()); + rig.now += HEARTBEAT - RETRY; + let frames = rig.spin(); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].0, 1); +} + +#[test] +fn heartbeat_byte_quota_checks_the_entire_frame_before_writing() { + for format in [ForkName::Fulu, ForkName::Gloas] { + let mut rig = Rig::new(format); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 1); + let frame = rig.spin()[0].1; + let wire_len = rig.wire(frame).len(); + let budget = rig.exchange.peer_exchanges.get_mut(&1).unwrap(); + budget.record(frame.read().seq(), budget.remaining_bytes() - wire_len + 1); + rig.request(1, 0, 2); + let seq = rig.output.next_seq(); + assert!(rig.spin().is_empty()); + assert_eq!(rig.output.next_seq(), seq); + let key = ExchangeKey { peer: 1, group: rig.message(1, 0, 0).group }; + assert_eq!(rig.exchange.exchanges[&key].sent, 0); + + rig.now += HEARTBEAT; + let frames = rig.spin(); + assert_eq!(frames.len(), 1); + assert_eq!(rig.wire(frames[0].1).len(), wire_len); + } +} + +#[test] +fn subscription_changes_and_slot_expiry_cannot_reset_the_heartbeat_budget() { + let mut rig = Rig::new(ForkName::Gloas); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 1); + let frame = rig.spin()[0].1; + let budget = rig.exchange.peer_exchanges.get_mut(&1).unwrap(); + budget.record(frame.read().seq(), budget.remaining_bytes()); + for event in [ + PeerEvent::P2pGossipTopicUnsubscribe { + p2p_peer: 1, + topic: GossipTopic::DataColumnSidecar(0), + digest: rig.domain.digest(), + }, + PeerEvent::P2pGossipExtensions { p2p_peer: 1, partial_messages: true }, + ] { + rig.exchange.peer_event(&event, rig.now); + rig.request(1, 0, 1); + assert!(rig.spin().is_empty()); + assert_eq!(rig.exchange.peer_exchanges[&1].remaining_bytes(), 0); + } + rig.exchange + .expire(1, &rig.peers, &mut rig.output, rig.now, &mut |_| panic!("no remaining quota")); + assert_eq!(rig.exchange.peer_exchanges[&1].remaining_bytes(), 0); + // Old-slot drops must not reset new exchange state or schedule a retry. + let key = ExchangeKey { peer: 1, group: rig.message(1, 0, 0).group }; + rig.exchange.admit(key, 1, ROWS, rig.now + Duration::from_secs(12), rig.now); + rig.dropped(1, frame); + assert_eq!(rig.exchange.exchanges[&key].retry_at, rig.now); + assert!(rig.exchange.ready.is_empty()); +} diff --git a/crates/control/src/partial_exchange/tests/allocations.rs b/crates/control/src/partial_exchange/tests/allocations.rs index 06cac15c5..cf5dd79a9 100644 --- a/crates/control/src/partial_exchange/tests/allocations.rs +++ b/crates/control/src/partial_exchange/tests/allocations.rs @@ -36,7 +36,7 @@ unsafe impl GlobalAlloc for CountingAllocator { static ALLOCATOR: CountingAllocator = CountingAllocator; #[test] -fn serving_and_feedback_allocate_nothing_after_construction() { +fn serving_and_drop_recovery_allocate_nothing_after_construction() { ControlCounters::init().unwrap(); for format in [ForkName::Fulu, ForkName::Gloas] { let mut rig = Rig::new(format); @@ -51,8 +51,9 @@ fn serving_and_feedback_allocate_nothing_after_construction() { }; assert!(frame.replace(next).is_none()); }); - rig.complete(1, frame.unwrap(), true); + rig.dropped(1, frame.unwrap()); rig.request(1, 0, 1 << (attempt % ROWS as u8)); + rig.now += RETRY; } assert_eq!(ALLOCATIONS.with(Cell::get) - before, 0); } diff --git a/crates/control/src/tile/tests/partial.rs b/crates/control/src/tile/tests/partial.rs index b368a0ebe..3a41e3944 100644 --- a/crates/control/src/tile/tests/partial.rs +++ b/crates/control/src/tile/tests/partial.rs @@ -1,6 +1,5 @@ use buffa::{Message, MessageView}; use silver_common::{ - GossipFrameOutcome, GossipFrameResult, cell_store::{CellStoreEvent, ColumnAvailability}, ssz_view::{ BYTES_PER_CELL, BYTES_PER_KZG_PROOF, @@ -102,8 +101,9 @@ fn metadata_crosses_ingress_control_and_segmented_send_spine_queues() { capture.crank(); let mut sent = None; capture.observer.consume(|event: P2pSend, _| { - if let P2pSend::SegmentedGossip { peer_id, frame } = event { + if let P2pSend::SegmentedGossip { peer_id, frame, partial_cells } = event { assert_eq!(peer_id, 1); + assert_eq!(partial_cells, Some(1)); assert!(sent.replace(frame).is_none()); } }); @@ -125,13 +125,7 @@ fn metadata_crosses_ingress_control_and_segmented_send_spine_queues() { assert_eq!(PartialDataColumnSidecarGloasView::check_size(payload, 2), Some(2)); assert_eq!(PartialDataColumnSidecarGloasView::cells(payload), &[0x22; BYTES_PER_CELL]); assert_eq!(PartialDataColumnSidecarGloasView::proofs(payload), &[0x44; BYTES_PER_KZG_PROOF]); - capture.observer.produce(PeerEvent::SegmentedGossipResult(GossipFrameResult { - p2p_peer: 1, - frame_seq: frame.read().seq(), - outcome: GossipFrameOutcome::Written { - stream_id: P2pStreamId::new(1, 4, StreamProtocol::GossipSubV13, false), - }, - })); + // No successful-send feedback is needed to suppress duplicate responses. capture.crank(); capture .observer diff --git a/crates/gossip/src/partial.rs b/crates/gossip/src/partial.rs index 564c5f2cf..d58be8b05 100644 --- a/crates/gossip/src/partial.rs +++ b/crates/gossip/src/partial.rs @@ -55,6 +55,27 @@ pub struct PartialFrame<'a> { } impl PartialFrame<'_> { + /// Protobuf RPC length, excluding the stream's length prefix. + pub fn wire_len(&self) -> usize { + let (fields, tail) = self.field_lengths(); + let extension = fields + self.plan.map_or(0, |plan| plan.ssz_len()) + tail; + 1 + varint_len(extension as u64) + extension + } + + fn field_lengths(&self) -> (usize, usize) { + let fields = 1 + + string_encoded_len(self.topic) + + 1 + + varint_len(self.group_id.len() as u64) + + self.group_id.len() + + self.plan.map_or(0, |plan| 1 + varint_len(plan.ssz_len() as u64)); + let tail = self.metadata.as_ref().map_or(0, |meta| { + let len = parts_metadata_len(meta.n_rows); + 1 + varint_len(len as u64) + len + }); + (fields, tail) + } + /// Write the frame descriptor into the outgoing gossip cache. /// `cells` and `proofs` are the selected rows' ranges in ascending /// row order; counts and lengths must match the plan. @@ -85,13 +106,7 @@ impl PartialFrame<'_> { let ssz_len = self.plan.map_or(0, |plan| plan.ssz_len()); let meta_len = self.metadata.as_ref().map(|meta| parts_metadata_len(meta.n_rows)); - let fields_len = 1 + - string_encoded_len(self.topic) + - 1 + - varint_len(self.group_id.len() as u64) + - self.group_id.len() + - self.plan.map_or(0, |_| 1 + varint_len(ssz_len as u64)); - let tail = meta_len.map_or(0, |m| 1 + varint_len(m as u64) + m); + let (fields_len, tail) = self.field_lengths(); let ext_len = fields_len + ssz_len + tail; // Framing layout: [lead | header list prefix | metadata field]. @@ -409,26 +424,28 @@ mod tests { let group = fulu_group_id(&[0xab; 32]); let plan = PartialSidecarPlan::new(PartialLayout::Fulu { header_bytes }, 0b101, 3).unwrap(); - let frame = PartialFrame { + let partial = PartialFrame { topic: TOPIC, group_id: &group, plan: Some(plan), header, metadata: Some(PartsMetadata { available: 0b101, requests: 0b010, n_rows: 3 }), - } - .write( - &mut producer, - cells.iter().copied(), - proofs.iter().copied(), - now + Duration::from_secs(1), - ) - .unwrap(); + }; + let frame = partial + .write( + &mut producer, + cells.iter().copied(), + proofs.iter().copied(), + now + Duration::from_secs(1), + ) + .unwrap(); let mut metadata = vec![0u8; parts_metadata_len(3)]; write_parts_metadata(0b101, 0b010, 3, &mut metadata); let reference = reference_rpc(&group, reference_ssz(&plan, &source, header_bytes), Some(metadata)); + assert_eq!(partial.wire_len(), reference.len()); assert_eq!(reassemble(frame, &mut consumer, now), reference); } @@ -481,16 +498,17 @@ mod tests { }); let group = fulu_group_id(&[0xab; 32]); let now = Instant::now(); - let frame = PartialFrame { + let partial = PartialFrame { topic: TOPIC, group_id: &group, plan: Some(plan), header, metadata: Some(PartsMetadata { available: 0x109, requests: 0xf6, n_rows: 9 }), - } - .write(&mut producer, cells, proofs, now + Duration::from_secs(1)) - .unwrap(); + }; + let frame = + partial.write(&mut producer, cells, proofs, now + Duration::from_secs(1)).unwrap(); let wire = reassemble(frame, &mut consumer, now); + assert_eq!(partial.wire_len(), wire.len()); let metadata = vec![8, 0, 0, 0, 10, 0, 0, 0, 9, 3, 0xf6, 2]; assert_eq!(wire, reference_rpc(&group, reference, Some(metadata))); } @@ -506,22 +524,24 @@ mod tests { let group = gloas_group_id(&[0xcd; 32], 123_456); let plan = PartialSidecarPlan::new(PartialLayout::Gloas, 0b10, 2).unwrap(); - let frame = PartialFrame { + let partial = PartialFrame { topic: TOPIC, group_id: &group, plan: Some(plan), header: None, metadata: None, - } - .write( - &mut producer, - cells.iter().copied(), - proofs.iter().copied(), - now + Duration::from_secs(1), - ) - .unwrap(); + }; + let frame = partial + .write( + &mut producer, + cells.iter().copied(), + proofs.iter().copied(), + now + Duration::from_secs(1), + ) + .unwrap(); let reference = reference_rpc(&group, reference_ssz(&plan, &source, 0), None); + assert_eq!(partial.wire_len(), reference.len()); assert_eq!(reassemble(frame, &mut consumer, now), reference); } diff --git a/crates/network/benches/quic_basic.rs b/crates/network/benches/quic_basic.rs index be6207ece..c751b9692 100644 --- a/crates/network/benches/quic_basic.rs +++ b/crates/network/benches/quic_basic.rs @@ -201,7 +201,7 @@ pub fn broadcast(c: &mut Criterion) { let Some(msg) = msgs.last() { let result = client.enqueue_gossip(*msg); - if result == SendResult::Ok { + if matches!(result, SendResult::Ok) { msgs.pop(); //println!("enqueue_gossip failed: // {result:?}"); diff --git a/crates/network/benches/quic_pingpong.rs b/crates/network/benches/quic_pingpong.rs index 79433bac2..dbd0592c6 100644 --- a/crates/network/benches/quic_pingpong.rs +++ b/crates/network/benches/quic_pingpong.rs @@ -103,11 +103,13 @@ pub fn broadcast(c: &mut Criterion) { reservation .write_all(&read[size_of::()..]) .unwrap(); - if server_tile.enqueue_gossip(GossipMsgOut { - peer_id: id.peer(), - tcache: reservation.read(), - }) != SendResult::Ok - { + if !matches!( + server_tile.enqueue_gossip(GossipMsgOut { + peer_id: id.peer(), + tcache: reservation.read(), + }), + SendResult::Ok + ) { println!("send failed!"); } break; diff --git a/crates/network/src/lib.rs b/crates/network/src/lib.rs index 076fe04b4..4d0c380f6 100644 --- a/crates/network/src/lib.rs +++ b/crates/network/src/lib.rs @@ -51,6 +51,10 @@ silver_common::declare_counters! { CacheSegmentedOwners, // Reserved ranges per recipient, including descriptors; not unique cache backing bytes. CacheSegmentedRetainedBytes, + // Complete partial frames accepted by Quinn, not yet necessarily ACKed. + PartialFramesWritten, + PartialResponsesSent, + PartialCellsServed, } } diff --git a/crates/network/src/p2p/mod.rs b/crates/network/src/p2p/mod.rs index b8621e0c7..c24ff372e 100644 --- a/crates/network/src/p2p/mod.rs +++ b/crates/network/src/p2p/mod.rs @@ -18,9 +18,9 @@ pub(crate) use quic::{Peer, create_client_config}; pub use quic::{SendResult, create_endpoint, create_server_config}; use quinn_proto::{ConnectionHandle, DatagramEvent, Endpoint}; use silver_common::{ - CacheFrameRef, ClusterMsgOut, GossipFrameResult, GossipMsgOut, Identify, Keypair, - P2pConnectionStats, P2pStreamId, PeerId, ProtoIdentify, ProtoIdentifyView, RpcOutbound, - RpcRequestOutbound, TCacheRead, + CacheFrameRef, ClusterMsgOut, GossipMsgOut, Identify, Keypair, P2pConnectionStats, P2pSend, + P2pStreamId, PeerId, ProtoIdentify, ProtoIdentifyView, RpcOutbound, RpcRequestOutbound, + TCacheRead, }; use crate::{ @@ -59,7 +59,6 @@ pub fn p2p_spin( #[derive(Debug, Clone)] #[allow(clippy::large_enum_variant)] pub enum NetEvent { - GossipFrameResult(GossipFrameResult), /// A peer connection has been established and its PeerId verified. PeerConnected { peer: RemotePeer, @@ -366,50 +365,59 @@ impl P2p { NetworkCounters::P2pConnections.set(self.peers.len() as u64); if let Some(limits) = &self.segmented_limits { limits.publish_gauges(); - while let Some(result) = limits.pop_result() { - did_work = true; - on_event(NetEvent::GossipFrameResult(result)); - } } did_work } pub fn enqueue_gossip(&mut self, msg: GossipMsgOut, context: &mut Context) -> SendResult { - match context.gossip_consumer.acquire_strict(msg.into()) { + let result = match context.gossip_consumer.acquire_strict(msg.into()) { Some(acquired) => match self.peers.get_mut(&ConnectionHandle(msg.peer_id)) { Some(peer) => peer.send_gossip(acquired, &mut self.rpc_codec_pool), None => SendResult::UnknownPeer, }, - None => SendResult::MessageDropped, - } + None => SendResult::Dropped(None), + }; + result.map_dropped(|dropped| { + dropped.map_or(P2pSend::Gossip(msg), |old| old.into_message(msg.peer_id)) + }) } pub fn enqueue_segmented_gossip( &mut self, peer_id: usize, frame: CacheFrameRef, + partial_cells: Option, context: &mut Context, ) -> SendResult { - match self.peers.get_mut(&ConnectionHandle(peer_id)) { + let result = match self.peers.get_mut(&ConnectionHandle(peer_id)) { Some(peer) => peer.send_segmented_gossip( frame, + partial_cells, context, self.segmented_limits.get_or_insert_with(Box::default), &mut self.rpc_codec_pool, ), None => SendResult::UnknownPeer, - } + }; + result.map_dropped(|dropped| { + dropped.map_or(P2pSend::SegmentedGossip { peer_id, frame, partial_cells }, |old| { + old.into_message(peer_id) + }) + }) } pub fn enqueue_rpc_out(&mut self, msg: RpcOutbound, context: &mut Context) -> SendResult { - match self.peers.get_mut(&ConnectionHandle(msg.peer_id())) { + let result = match self.peers.get_mut(&ConnectionHandle(msg.peer_id())) { Some(peer) => { tracing::debug!(protocol=?msg.protocol(), peer=msg.peer_id(), "enqueue outbound rpc request"); let acquired_msg = AcquiredRpcOutbound::from((msg, &mut context.rpc_consumer)); peer.send_rpc(acquired_msg) } None => SendResult::UnknownPeer, - } + }; + result.map_dropped(|dropped| { + P2pSend::Rpc(dropped.map_or(msg, |old| old.into_message(msg.peer_id()))) + }) } pub fn enqueue_cluster_out(&mut self, msg: ClusterMsgOut, context: &mut Context) -> SendResult { @@ -419,7 +427,7 @@ impl P2p { { Some(peer) => match context.cluster_outbound_consumer.acquire_strict(msg.data) { Some(acquired) => peer.send_cluster(acquired, &mut self.rpc_codec_pool), - None => SendResult::MessageDropped, + None => SendResult::Dropped(None), }, None => SendResult::UnknownPeer, } diff --git a/crates/network/src/p2p/quic/gossip_frame.rs b/crates/network/src/p2p/quic/gossip_frame.rs index bea10f429..35e5db5ee 100644 --- a/crates/network/src/p2p/quic/gossip_frame.rs +++ b/crates/network/src/p2p/quic/gossip_frame.rs @@ -2,15 +2,11 @@ use std::{cell::Cell, ptr::NonNull, time::Instant}; use bytes::Bytes; use silver_common::{ - AcquiredCacheFrame, AcquiredCacheSegment, AcquiredRange, CacheFrameView, GossipFrameResult, - P2pStreamId, TRead, + AcquiredCacheFrame, AcquiredCacheSegment, AcquiredRange, CacheFrameView, GossipMsgOut, P2pSend, + TRead, }; -use super::{ - Leased, - leased::OutboundLeaseWheel, - send_receipts::{SendReceipt, SendReceipts}, -}; +use super::{Leased, leased::OutboundLeaseWheel}; use crate::{NetworkCounters, p2p::Context}; const MAX_RETAINED_BYTES: usize = 64 * 1024 * 1024; @@ -22,8 +18,20 @@ pub(crate) enum OutboundGossip { Segmented(SegmentedFrame), } +impl OutboundGossip { + pub(crate) fn into_message(self, peer_id: usize) -> P2pSend { + match self { + Self::Contiguous(read) => P2pSend::Gossip(GossipMsgOut { peer_id, tcache: read.read }), + Self::Segmented(frame) => P2pSend::SegmentedGossip { + peer_id, + frame: frame.segments.reference(), + partial_cells: frame.partial_cells, + }, + } + } +} + pub(crate) struct SegmentedGossipLimits { - receipts: SendReceipts, frames: Cell, owners: Cell, retained_bytes: Cell, @@ -41,7 +49,6 @@ impl Default for SegmentedGossipLimits { impl SegmentedGossipLimits { pub(crate) fn new(max_frames: usize) -> Self { Self { - receipts: SendReceipts::new(max_frames), frames: Cell::new(0), owners: Cell::new(0), retained_bytes: Cell::new(0), @@ -77,7 +84,7 @@ impl SegmentedGossipLimits { )?; NetworkCounters::CacheSegmentedAdmitted.inc(); NetworkCounters::CacheSegmentedSegments.add(frame.segment_count() as u64); - Some(SegmentedFrame { segments: wheel.leased(frame, now), budget, receipt: None }) + Some(SegmentedFrame { segments: wheel.leased(frame, now), budget, partial_cells: None }) } pub(crate) fn publish_gauges(&self) { @@ -85,10 +92,6 @@ impl SegmentedGossipLimits { NetworkCounters::CacheSegmentedOwners.set(self.owners.get() as u64); NetworkCounters::CacheSegmentedRetainedBytes.set(self.retained_bytes.get() as u64); } - - pub(crate) fn pop_result(&self) -> Option { - self.receipts.pop() - } } impl Drop for SegmentedGossipLimits { @@ -101,16 +104,12 @@ impl Drop for SegmentedGossipLimits { #[derive(Debug)] pub(crate) struct SegmentedFrame { - receipt: Option, + pub(crate) partial_cells: Option, segments: Leased, budget: FrameBudget, } impl SegmentedFrame { - pub(crate) fn track(&mut self, limits: &SegmentedGossipLimits, peer: usize, seq: u64) -> bool { - self.receipt = limits.receipts.acquire(peer, seq, self.wire_len()); - self.receipt.is_some() - } pub(crate) fn wire_len(&self) -> usize { self.segments.wire_len() } @@ -152,10 +151,14 @@ impl SegmentedWriter { self.remaining == 0 } - pub(crate) fn complete(&mut self, stream_id: P2pStreamId) { + pub(crate) fn complete(&mut self) { assert_eq!(self.remaining, 0); - if let Some(receipt) = &mut self.frame.receipt { - receipt.written(stream_id); + if let Some(cells) = self.frame.partial_cells.take() { + NetworkCounters::PartialFramesWritten.inc(); + if cells != 0 { + NetworkCounters::PartialResponsesSent.inc(); + NetworkCounters::PartialCellsServed.add(cells as u64); + } } } } diff --git a/crates/network/src/p2p/quic/gossip_frame/tests.rs b/crates/network/src/p2p/quic/gossip_frame/tests.rs index 594f26ae8..9fb3d4cf3 100644 --- a/crates/network/src/p2p/quic/gossip_frame/tests.rs +++ b/crates/network/src/p2p/quic/gossip_frame/tests.rs @@ -8,8 +8,8 @@ use std::{ use quinn_proto::StreamId; use silver_common::{ - AcquiredWithOffset, CacheFrameRef, CacheSegment, GossipFrameOutcome, P2pStreamId, - StreamProtocol, SubLayout, SubReservationRef, TCache, TCacheProducer, TProducer, + AcquiredWithOffset, CacheFrameRef, CacheSegment, P2pStreamId, StreamProtocol, SubLayout, + SubReservationRef, TCache, TCacheProducer, TProducer, }; use super::*; @@ -258,7 +258,7 @@ fn segments_are_allocated_lazily_and_blocked_retries_survive_expiry() { drop(warm); let before = ALLOCATIONS.with(Cell::get); let mut frame = h.acquire(reference).unwrap(); - assert!(frame.track(&h.limits, 0, reference.read().seq())); + frame.partial_cells = Some(1); assert_eq!(ALLOCATIONS.with(Cell::get) - before, 0); let cell_ptr = assembly .acquire(h.context.data_columns_consumer.as_deref_mut().unwrap()) @@ -306,9 +306,6 @@ fn segments_are_allocated_lazily_and_blocked_retries_survive_expiry() { assert_eq!(&io.written[69..], &[0xcd; 8]); assert_eq!(io.retained[1].as_ptr(), cell_ptr); assert_eq!(h.limits.frames.get(), 0); - let result = h.limits.pop_result().unwrap(); - assert_eq!(result.frame_seq, reference.read().seq()); - assert_eq!(result.outcome, GossipFrameOutcome::Written { stream_id: stream() }); assert!(h.columns.reserve(8192, true).is_none()); assert!(h.wheel.expire(h.now + Duration::from_secs(11)).is_some()); io.retained.clear(); diff --git a/crates/network/src/p2p/quic/mod.rs b/crates/network/src/p2p/quic/mod.rs index 3dcb9cf89..fffd0548b 100644 --- a/crates/network/src/p2p/quic/mod.rs +++ b/crates/network/src/p2p/quic/mod.rs @@ -4,14 +4,13 @@ use quinn_proto::{ ClientConfig, Endpoint, EndpointConfig, ServerConfig, crypto::rustls::{QuicClientConfig, QuicServerConfig}, }; -use silver_common::{Keypair, PeerId}; +use silver_common::{Keypair, P2pSend, PeerId}; use super::tls; mod gossip_frame; mod leased; mod peer; -mod send_receipts; mod stream; pub(crate) use gossip_frame::{OutboundGossip, SegmentedGossipLimits, SegmentedWriter}; @@ -65,16 +64,31 @@ pub fn create_server_config(keypair: &Keypair) -> Result { Ok(config) } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum SendResult { +#[derive(Clone, Copy, Debug)] +#[allow(clippy::large_enum_variant)] +pub enum SendResult { Ok, StreamCreationError, /// RPC response targeted a stream no longer in the map (closed/reset /// before the response was enqueued). StreamGone, - MessageDropped, + /// Evicting an older message does not reject the newly enqueued message. + Dropped(Option), UnknownPeer, /// Connection is closing/draining: nothing sent on it can be delivered, /// and opening a stream would misreport as credit exhaustion. ConnectionClosing, } + +impl SendResult { + pub(crate) fn map_dropped(self, map: impl FnOnce(Option) -> U) -> SendResult { + match self { + Self::Ok => SendResult::Ok, + Self::StreamCreationError => SendResult::StreamCreationError, + Self::StreamGone => SendResult::StreamGone, + Self::Dropped(msg) => SendResult::Dropped(Some(map(msg))), + Self::UnknownPeer => SendResult::UnknownPeer, + Self::ConnectionClosing => SendResult::ConnectionClosing, + } + } +} diff --git a/crates/network/src/p2p/quic/peer.rs b/crates/network/src/p2p/quic/peer.rs index 9dbe1b6db..83183511a 100644 --- a/crates/network/src/p2p/quic/peer.rs +++ b/crates/network/src/p2p/quic/peer.rs @@ -163,7 +163,7 @@ impl Peer { &mut self, msg: TRead, rpc_codec_pool: &mut RpcCodecPool, - ) -> SendResult { + ) -> SendResult { if self.connection.is_closed() { return SendResult::ConnectionClosing; } @@ -178,10 +178,11 @@ impl Peer { pub(crate) fn send_segmented_gossip( &mut self, frame: CacheFrameRef, + partial_cells: Option, context: &mut Context, limits: &SegmentedGossipLimits, rpc_codec_pool: &mut RpcCodecPool, - ) -> SendResult { + ) -> SendResult { if self.connection.is_closed() { return SendResult::ConnectionClosing; } @@ -195,16 +196,13 @@ impl Peer { .and_then(|view| limits.acquire(view, context, &self.outbound_lease_wheel, now)); let Some(mut acquired) = acquired else { crate::NetworkCounters::CacheSegmentedRejected.inc(); - return SendResult::MessageDropped; + return SendResult::Dropped(None); }; - if !acquired.track(limits, self.handle.0, frame.read().seq()) { - return SendResult::MessageDropped; - } + acquired.partial_cells = partial_cells; self.queue_gossip(OutboundGossip::Segmented(acquired)) } - fn queue_gossip(&mut self, msg: OutboundGossip) -> SendResult { - let tracked = matches!(msg, OutboundGossip::Segmented(_)); + fn queue_gossip(&mut self, msg: OutboundGossip) -> SendResult { self.dirty = true; let stream_id = match self.outbound_gossip { Some(id) => id, @@ -220,11 +218,7 @@ impl Peer { if let OutboundBuffer::Gossip(buffer) = &mut stream.out_buffer { let dropped = buffer.add_msg(msg); stream.needs_spin = true; - return if dropped && !tracked { - SendResult::MessageDropped - } else { - SendResult::Ok - }; + return dropped.map_or(SendResult::Ok, |msg| SendResult::Dropped(Some(msg))); } } SendResult::StreamCreationError @@ -258,7 +252,7 @@ impl Peer { if let OutboundBuffer::Cluster(buffer) = &mut stream.out_buffer { let dropped = buffer.add_msg(msg); stream.needs_spin = true; - return if dropped { SendResult::MessageDropped } else { SendResult::Ok }; + return dropped.map_or(SendResult::Ok, |_| SendResult::Dropped(None)); } } SendResult::StreamCreationError @@ -281,7 +275,7 @@ impl Peer { false } - pub(crate) fn send_rpc(&mut self, msg: AcquiredRpcOutbound) -> SendResult { + pub(crate) fn send_rpc(&mut self, msg: AcquiredRpcOutbound) -> SendResult { if self.connection.is_closed() { return SendResult::ConnectionClosing; } @@ -314,7 +308,7 @@ impl Peer { if let OutboundBuffer::Rpc(buffer) = &mut stream.out_buffer { let dropped = buffer.add_msg(msg); stream.needs_spin = true; - return if dropped { SendResult::MessageDropped } else { SendResult::Ok }; + return dropped.map_or(SendResult::Ok, |msg| SendResult::Dropped(Some(msg))); } SendResult::StreamCreationError } @@ -1223,18 +1217,15 @@ impl OutBuffer { seq & (self.len - 1) } - /// Returns `true` if adding the new message dropped the oldest queued - /// message. - fn add_msg(&mut self, msg: T) -> bool { - let dropped = self.head - self.tail == self.msgs.len(); - if dropped { + fn add_msg(&mut self, msg: T) -> Option { + if self.head - self.tail == self.msgs.len() { // Full: pos(head) == pos(tail), so the overwrite below replaces // the oldest message. Advance tail with it — otherwise head/tail // desync and is_empty() reports non-empty while pop() yields // None, leaving the stream flagged needs_spin forever. self.tail += 1; } - self.msgs[self.pos(self.head)].replace(msg); + let dropped = self.msgs[self.pos(self.head)].replace(msg); self.head += 1; dropped } @@ -1269,7 +1260,8 @@ mod tests { use mio::{Poll, Token}; use quinn_proto::{DatagramEvent, Endpoint, EndpointConfig}; use silver_common::{ - CacheSegment, Enr, GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME, Keypair, TCache, TCacheProducer, + CacheFrameError, CacheSegment, Enr, GOSSIP_EXTENSIONS_ANNOUNCEMENT_FRAME, GossipMsgOut, + Keypair, P2pSend, RpcOutbound, RpcResponse, RpcResponseOutbound, TCache, TCacheProducer, TConsumer, TProducer, }; @@ -1290,12 +1282,12 @@ mod tests { let mut buf = OutBuffer::new(4); for i in 0..4usize { - assert!(!buf.add_msg(Item(i))); + assert!(buf.add_msg(Item(i)).is_none()); } // Two overwrites drop the two oldest messages. - assert!(buf.add_msg(Item(4))); - assert!(buf.add_msg(Item(5))); + assert_eq!(buf.add_msg(Item(4)).map(|item| item.0), Some(0)); + assert_eq!(buf.add_msg(Item(5)).map(|item| item.0), Some(1)); assert_eq!(buf.len(), 4); let drained: Vec<_> = std::iter::from_fn(|| buf.pop()).map(|u| u.0).collect(); @@ -1304,28 +1296,11 @@ mod tests { assert!(buf.pop().is_none()); // Buffer must remain usable after an overflow episode. - assert!(!buf.add_msg(Item(6))); + assert!(buf.add_msg(Item(6)).is_none()); assert_eq!(buf.pop().map(|u| u.0), Some(6)); assert!(buf.is_empty()); } - #[test] - fn queue_eviction_reports_the_evicted_frame_not_its_replacement() { - let receipts = Box::new(super::super::send_receipts::SendReceipts::new(4)); - let mut buffer = OutBuffer::new(2); - assert!(!buffer.add_msg(receipts.acquire(1, 10, 100).unwrap())); - assert!(!buffer.add_msg(receipts.acquire(1, 11, 100).unwrap())); - assert!(buffer.add_msg(receipts.acquire(1, 12, 100).unwrap())); - let dropped = receipts.pop().unwrap(); - assert_eq!(dropped.frame_seq, 10); - assert_eq!(dropped.outcome, silver_common::GossipFrameOutcome::Dropped); - assert!(receipts.pop().is_none()); - drop(buffer); - let dropped: Vec<_> = - std::iter::from_fn(|| receipts.pop()).map(|result| result.frame_seq).collect(); - assert_eq!(dropped, [12, 11]); - } - #[test] fn outbound_lease_wheel_drop_cancels_lease_after_box_moves() { let t0 = Instant::now(); @@ -1415,12 +1390,15 @@ mod tests { let wheel = Box::new(OutboundLeaseWheel::new(t0)); let mut buffer = OutBuffer::new(2); - assert!(!buffer.add_msg(wheel.leased(1u8, t0))); - assert!(!buffer.add_msg(wheel.leased(2u8, t0))); + assert!(buffer.add_msg(wheel.leased(1u8, t0)).is_none()); + assert!(buffer.add_msg(wheel.leased(2u8, t0)).is_none()); assert_eq!(wheel.active_count(), 2); - assert!(buffer.add_msg(wheel.leased(3u8, t0))); - assert_eq!(wheel.active_count(), 2, "overwrite must drop the oldest root lease"); + let evicted = buffer.add_msg(wheel.leased(3u8, t0)).unwrap(); + assert_eq!(*evicted, 1); + assert_eq!(wheel.active_count(), 3); + drop(evicted); + assert_eq!(wheel.active_count(), 2, "dropping the evicted message releases its lease"); drop(buffer); assert_eq!(wheel.active_count(), 0); @@ -1526,6 +1504,13 @@ mod tests { } impl PeerPair { + fn into_client(self) -> P2p { + let keypair = Keypair::from_secret(&[2u8; 32]).unwrap(); + let mut endpoint = P2p::new(keypair, self.client_ep, 16, FxHashSet::default()); + endpoint.peers.insert(self.client_peer.handle, self.client_peer); + endpoint + } + fn new() -> Self { let server_addr: SocketAddr = "127.0.0.1:5000".parse().unwrap(); let client_addr: SocketAddr = "127.0.0.1:5001".parse().unwrap(); @@ -1838,7 +1823,7 @@ mod tests { let mut pair = PeerPair::new(); let now = Instant::now(); - assert_eq!(client_h.send_cluster(b"first", &mut pair.client_peer), SendResult::Ok); + assert!(matches!(client_h.send_cluster(b"first", &mut pair.client_peer), SendResult::Ok)); let stream = pair.client_peer.cluster_stream.unwrap(); assert_eq!(pair.client_peer.outbound_lease_wheel.active_count(), 1); @@ -1859,7 +1844,7 @@ mod tests { assert_eq!(pair.client_peer.cluster_stream, None); assert_eq!(pair.client_peer.outbound_lease_wheel.active_count(), 0); - assert_eq!(client_h.send_cluster(b"second", &mut pair.client_peer), SendResult::Ok); + assert!(matches!(client_h.send_cluster(b"second", &mut pair.client_peer), SendResult::Ok)); let replacement = pair.client_peer.cluster_stream.unwrap(); assert_ne!(replacement, stream); assert!(pair.client_peer.streams.contains_key(&replacement)); @@ -2248,15 +2233,16 @@ mod tests { .into_iter(), ) .unwrap(); - assert_eq!( + assert!(matches!( pair.client_peer.send_segmented_gossip( frame, + None, &mut client_h.context, &limits, &mut client_h.rpc_codec_pool ), SendResult::Ok - ); + )); client_h .context .data_columns_consumer @@ -2299,28 +2285,210 @@ mod tests { ) .unwrap(); let peer = &mut pair.client_peer; - assert_eq!( - peer.send_segmented_gossip(frame, &mut h.context, &limits, &mut h.rpc_codec_pool), + assert!(matches!( + peer.send_segmented_gossip(frame, None, &mut h.context, &limits, &mut h.rpc_codec_pool), SendResult::Ok - ); + )); let id = peer.outbound_gossip.unwrap(); peer.streams.get_mut(&id).unwrap().out_buffer = OutboundBuffer::Gossip(OutBuffer::new(1)); assert_eq!(peer.outbound_lease_wheel.active_count(), 0); - assert_eq!(limits.pop_result().unwrap().frame_seq, frame.read().seq()); + let mut previous = frame; for i in 0..3 { - assert_eq!( - peer.send_segmented_gossip(frame, &mut h.context, &limits, &mut h.rpc_codec_pool), - SendResult::Ok + let frame = CacheFrameRef::write( + &mut h.gossip_out_producer, + Instant::now() + Duration::from_secs(1), + b"payload", + [CacheSegment::Framing { offset: 0, length: 7 }].into_iter(), + ) + .unwrap(); + let result = peer.send_segmented_gossip( + frame, + None, + &mut h.context, + &limits, + &mut h.rpc_codec_pool, ); - assert_eq!(peer.outbound_lease_wheel.active_count(), 1); - if i > 0 { - assert_eq!(limits.pop_result().unwrap().frame_seq, frame.read().seq()); + if i == 0 { + assert!(matches!(result, SendResult::Ok)); + } else { + let SendResult::Dropped(Some(dropped)) = result else { + panic!("expected the evicted segmented frame"); + }; + let P2pSend::SegmentedGossip { frame: dropped, .. } = + dropped.into_message(peer.handle.0) + else { + panic!("expected a segmented message"); + }; + assert_eq!(dropped.read().seq(), previous.read().seq()); } + assert_eq!(peer.outbound_lease_wheel.active_count(), 1); + previous = frame; } peer.clear_streams(&mut h.rpc_codec_pool); assert_eq!(peer.outbound_lease_wheel.active_count(), 0); } + #[test] + fn endpoint_gossip_overflow_returns_the_evicted_variant_and_descriptor() { + let mut h = PeerHarness::new(); + h.context.gossip_consumer = + h.gossip_out_producer.cache_ref().strict_random_access("", true).unwrap(); + let pair = PeerPair::new(); + let handle = pair.client_peer.handle; + let mut endpoint = pair.into_client(); + let peer = endpoint.peers.get_mut(&handle).unwrap(); + let stream = peer.open_stream(StreamProtocol::GossipSubV13).unwrap(); + peer.outbound_gossip = Some(stream); + peer.streams.get_mut(&stream).unwrap().out_buffer = + OutboundBuffer::Gossip(OutBuffer::new(1)); + + let mut reservation = h.gossip_out_producer.reserve(5, true).unwrap(); + reservation.write_all(b"first").unwrap(); + let first = GossipMsgOut { peer_id: handle.0, tcache: reservation.read() }; + assert!(matches!(endpoint.enqueue_gossip(first, &mut h.context), SendResult::Ok)); + + let expires = Instant::now() + Duration::from_secs(1); + let frame = CacheFrameRef::write( + &mut h.gossip_out_producer, + expires, + b"second", + [CacheSegment::Framing { offset: 0, length: 6 }].into_iter(), + ) + .unwrap(); + let SendResult::Dropped(Some(P2pSend::Gossip(dropped))) = + endpoint.enqueue_segmented_gossip(handle.0, frame, Some(7), &mut h.context) + else { + panic!("expected the evicted contiguous message"); + }; + assert_eq!(dropped.peer_id, handle.0); + assert_eq!(dropped.tcache.seq(), first.tcache.seq()); + + let mut reservation = h.gossip_out_producer.reserve(5, true).unwrap(); + reservation.write_all(b"third").unwrap(); + let third = GossipMsgOut { peer_id: handle.0, tcache: reservation.read() }; + let SendResult::Dropped(Some(P2pSend::SegmentedGossip { + peer_id, + frame: dropped, + partial_cells, + })) = endpoint.enqueue_gossip(third, &mut h.context) + else { + panic!("expected the evicted segmented message"); + }; + assert_eq!(peer_id, handle.0); + assert_eq!(partial_cells, Some(7)); + assert_eq!(dropped.read().seq(), frame.read().seq()); + assert!(matches!( + dropped.acquire(&mut h.context.gossip_consumer, expires), + Err(CacheFrameError::Expired) + )); + + let peer = endpoint.peers.get_mut(&handle).unwrap(); + assert_eq!(peer.outbound_lease_wheel.active_count(), 1); + let OutboundBuffer::Gossip(buffer) = &mut peer.streams.get_mut(&stream).unwrap().out_buffer + else { + panic!("expected gossip buffer"); + }; + let P2pSend::Gossip(queued) = buffer.pop().unwrap().into_message(handle.0) else { + panic!("expected the replacement message to remain queued"); + }; + assert_eq!(queued.tcache.seq(), third.tcache.seq()); + assert!(buffer.is_empty()); + assert_eq!(peer.outbound_lease_wheel.active_count(), 0); + } + + #[test] + fn endpoint_rejected_gossip_returns_the_attempted_message() { + let mut h = PeerHarness::new(); + h.context.gossip_consumer = + h.gossip_out_producer.cache_ref().strict_random_access("", true).unwrap(); + let pair = PeerPair::new(); + let handle = pair.client_peer.handle; + let mut endpoint = pair.into_client(); + + let mut reservation = h.gossip_out_producer.reserve(5, false).unwrap(); + reservation.write_all(b"first").unwrap(); + let msg = GossipMsgOut { peer_id: handle.0, tcache: reservation.read() }; + let SendResult::Dropped(Some(P2pSend::Gossip(dropped))) = + endpoint.enqueue_gossip(msg, &mut h.context) + else { + panic!("expected rejection of the incomplete message"); + }; + assert_eq!(dropped.peer_id, handle.0); + assert_eq!(dropped.tcache.seq(), msg.tcache.seq()); + + let frame = CacheFrameRef::write( + &mut h.gossip_out_producer, + Instant::now(), + b"expired", + [CacheSegment::Framing { offset: 0, length: 7 }].into_iter(), + ) + .unwrap(); + let SendResult::Dropped(Some(P2pSend::SegmentedGossip { + peer_id, + frame: dropped, + partial_cells, + })) = endpoint.enqueue_segmented_gossip(handle.0, frame, Some(3), &mut h.context) + else { + panic!("expected rejection of the expired frame"); + }; + assert_eq!(peer_id, handle.0); + assert_eq!(partial_cells, Some(3)); + assert_eq!(dropped.read().seq(), frame.read().seq()); + assert!(endpoint.peers[&handle].outbound_gossip.is_none()); + assert_eq!(endpoint.peers[&handle].outbound_lease_wheel.active_count(), 0); + } + + #[test] + fn endpoint_rpc_overflow_returns_the_evicted_response() { + let mut h = PeerHarness::new(); + let mut producer = TCache::producer("", TCACHE_BYTES); + h.context.rpc_consumer = producer.cache_ref().strict_random_access("", true).unwrap(); + let pair = PeerPair::new(); + let handle = pair.client_peer.handle; + let mut endpoint = pair.into_client(); + let peer = endpoint.peers.get_mut(&handle).unwrap(); + let stream = peer.open_stream(StreamProtocol::BeaconBlocksByRoot).unwrap(); + peer.streams.get_mut(&stream).unwrap().out_buffer = OutboundBuffer::Rpc(OutBuffer::new(1)); + let stream_id = + P2pStreamId::new(handle.0, stream.into(), StreamProtocol::BeaconBlocksByRoot, false); + + let mut reservation = producer.reserve(5, true).unwrap(); + reservation.write_all(b"block").unwrap(); + let read = reservation.read(); + let first = RpcOutbound::Response(RpcResponseOutbound { + stream_id, + response: RpcResponse::BeaconBlock { fork_digest: [1; 4], ssz: read }, + }); + assert!(matches!(endpoint.enqueue_rpc_out(first, &mut h.context), SendResult::Ok)); + + let complete = RpcOutbound::Response(RpcResponseOutbound { + stream_id, + response: RpcResponse::Complete, + }); + let SendResult::Dropped(Some(P2pSend::Rpc(RpcOutbound::Response(dropped)))) = + endpoint.enqueue_rpc_out(complete, &mut h.context) + else { + panic!("expected the evicted RPC response"); + }; + assert_eq!(dropped.stream_id, stream_id); + let RpcResponse::BeaconBlock { fork_digest, ssz } = dropped.response else { + panic!("expected the old block response, not its terminator"); + }; + assert_eq!(fork_digest, [1; 4]); + assert_eq!(ssz.seq(), read.seq()); + + let peer = endpoint.peers.get_mut(&handle).unwrap(); + let OutboundBuffer::Rpc(buffer) = &mut peer.streams.get_mut(&stream).unwrap().out_buffer + else { + panic!("expected RPC buffer"); + }; + assert!(matches!( + buffer.pop().unwrap().into_message(handle.0), + RpcOutbound::Response(RpcResponseOutbound { response: RpcResponse::Complete, .. }) + )); + assert!(buffer.is_empty()); + } + #[test] fn bidirectional_data_transfer() { let mut client_h = PeerHarness::new(); diff --git a/crates/network/src/p2p/quic/send_receipts.rs b/crates/network/src/p2p/quic/send_receipts.rs deleted file mode 100644 index 56457e000..000000000 --- a/crates/network/src/p2p/quic/send_receipts.rs +++ /dev/null @@ -1,126 +0,0 @@ -use std::{ - cell::{Cell, RefCell}, - collections::VecDeque, - ptr::NonNull, -}; - -use fxhash::FxHashMap; -use silver_common::{GossipFrameOutcome, GossipFrameResult, P2pStreamId}; - -const MAX_PEER_FRAMES: usize = 16; -const MAX_PEER_BYTES: usize = 4 * 1024 * 1024; - -pub(super) struct SendReceipts { - pending: Cell, - peers: RefCell>, - completed: RefCell>, - capacity: usize, -} - -impl SendReceipts { - pub(super) fn new(capacity: usize) -> Self { - Self { - pending: Cell::new(0), - peers: RefCell::new(FxHashMap::with_capacity_and_hasher(capacity, Default::default())), - completed: RefCell::new(VecDeque::with_capacity(capacity)), - capacity, - } - } - - pub(super) fn acquire(&self, peer: usize, frame_seq: u64, bytes: usize) -> Option { - if self.pending.get() + self.completed.borrow().len() >= self.capacity { - return None; - } - let mut peers = self.peers.borrow_mut(); - let (frames, queued_bytes) = peers.get(&peer).copied().unwrap_or_default(); - if frames >= MAX_PEER_FRAMES || bytes > MAX_PEER_BYTES.saturating_sub(queued_bytes) { - return None; - } - peers.insert(peer, (frames + 1, queued_bytes + bytes)); - self.pending.set(self.pending.get() + 1); - Some(SendReceipt { - receipts: NonNull::from(self), - result: GossipFrameResult { - p2p_peer: peer, - frame_seq, - outcome: GossipFrameOutcome::Dropped, - }, - bytes, - }) - } - - pub(super) fn pop(&self) -> Option { - self.completed.borrow_mut().pop_front() - } -} - -#[derive(Debug)] -pub(super) struct SendReceipt { - receipts: NonNull, - result: GossipFrameResult, - bytes: usize, -} - -// Receipts remain on NetworkTile; the boxed limits outlive every queued frame. -unsafe impl Send for SendReceipt {} - -impl SendReceipt { - pub(super) fn written(&mut self, stream_id: P2pStreamId) { - self.result.outcome = GossipFrameOutcome::Written { stream_id }; - } -} - -impl Drop for SendReceipt { - fn drop(&mut self) { - let receipts = unsafe { self.receipts.as_ref() }; - let mut peers = receipts.peers.borrow_mut(); - let (frames, bytes) = peers.get_mut(&self.result.p2p_peer).unwrap(); - *frames -= 1; - *bytes -= self.bytes; - if *frames == 0 { - peers.remove(&self.result.p2p_peer); - } - receipts.pending.set(receipts.pending.get() - 1); - receipts.completed.borrow_mut().push_back(self.result); - } -} - -#[cfg(test)] -mod tests { - use silver_common::StreamProtocol; - - use super::*; - - #[test] - fn written_and_abandoned_frames_release_their_own_receipts() { - let receipts = Box::new(SendReceipts::new(2)); - let mut first = receipts.acquire(1, 10, 1024).unwrap(); - let second = receipts.acquire(2, 20, 2048).unwrap(); - assert!(receipts.acquire(1, 30, 1).is_none()); - let stream_id = P2pStreamId::new(1, 4, StreamProtocol::GossipSubV13, false); - first.written(stream_id); - drop(first); - assert!(receipts.acquire(1, 30, 1).is_none(), "undrained feedback reserves its capacity"); - let result = receipts.pop().unwrap(); - assert_eq!(result.frame_seq, 10); - assert_eq!(result.outcome, GossipFrameOutcome::Written { stream_id }); - drop(second); - let result = receipts.pop().unwrap(); - assert_eq!(result.p2p_peer, 2); - assert_eq!(result.frame_seq, 20); - assert_eq!(result.outcome, GossipFrameOutcome::Dropped); - assert!(receipts.peers.borrow().is_empty()); - assert_eq!(receipts.pending.get(), 0); - } - - #[test] - fn peer_byte_limit_does_not_block_other_peers() { - let receipts = Box::new(SendReceipts::new(4)); - let first = receipts.acquire(1, 1, MAX_PEER_BYTES).unwrap(); - assert!(receipts.acquire(1, 2, 1).is_none()); - let second = receipts.acquire(2, 3, 1).unwrap(); - drop(first); - drop(second); - assert!(receipts.peers.borrow().is_empty()); - } -} diff --git a/crates/network/src/p2p/streams/gossip_out.rs b/crates/network/src/p2p/streams/gossip_out.rs index a231678bf..1aa6ca4a9 100644 --- a/crates/network/src/p2p/streams/gossip_out.rs +++ b/crates/network/src/p2p/streams/gossip_out.rs @@ -133,7 +133,7 @@ impl GossipWriteState { let n = io.write_chunks(p2p_id.stream_id(), slice::from_mut(chunk))?; let chunk_complete = chunk.is_empty(); if frame.written(n) { - frame.complete(*p2p_id); + frame.complete(); Ok(Spin::Next(Self::Idle)) } else if chunk_complete { Ok(Spin::Next(Self::WritingSegments(frame))) diff --git a/crates/network/src/p2p/streams/rpc/mod.rs b/crates/network/src/p2p/streams/rpc/mod.rs index 20697c224..42d85afcf 100644 --- a/crates/network/src/p2p/streams/rpc/mod.rs +++ b/crates/network/src/p2p/streams/rpc/mod.rs @@ -5,6 +5,9 @@ mod reservation; mod response_in; mod response_out; +#[cfg(test)] +mod tests; + pub(crate) use pool::{RpcCodecDirection, RpcCodecPool}; pub use request_in::RpcReadRequest; pub use request_out::RpcWriteRequest; @@ -12,7 +15,8 @@ use reservation::{Rpc, RpcReservation, alloc_incoming_rpc}; pub use response_in::RpcReadResponse; pub use response_out::RpcWriteResponse; use silver_common::{ - P2pStreamId, RpcOutbound, RpcRequest, RpcResponse, StreamProtocol, TRandomAccess, TRead, + P2pStreamId, RpcOutbound, RpcRequest, RpcRequestOutbound, RpcResponse, RpcResponseOutbound, + StreamProtocol, TRandomAccess, TRead, rpc_rate_limit::{RPC_ERR_RATE_LIMITED, RPC_RATE_LIMITED_MSG}, ssz_view::{ BLOCKS_BY_RANGE_REQ_SIZE, DC_BY_RANGE_REQ_MAX, @@ -50,7 +54,7 @@ pub enum RpcOut { } // Consumer acquired wrapper for rpc outbound messages -#[derive(Clone)] +#[derive(Clone, Debug)] #[allow(clippy::large_enum_variant)] pub(crate) enum AcquiredRpcOutbound { Request(AcquiredRpcRequestOutbound), @@ -58,6 +62,20 @@ pub(crate) enum AcquiredRpcOutbound { } impl AcquiredRpcOutbound { + pub(crate) fn into_message(self, peer: usize) -> RpcOutbound { + match self { + Self::Request(req) => RpcOutbound::Request(RpcRequestOutbound { + application_id: req.application_id, + peer, + request: req.request.into(), + }), + Self::Response(rsp) => RpcOutbound::Response(RpcResponseOutbound { + stream_id: rsp.stream_id, + response: rsp.response.into(), + }), + } + } + pub fn protocol(&self) -> StreamProtocol { match self { Self::Request(req) => req.request.protocol(), @@ -66,13 +84,13 @@ impl AcquiredRpcOutbound { } } -#[derive(Clone)] +#[derive(Clone, Debug)] pub(crate) struct AcquiredRpcRequestOutbound { pub(crate) application_id: u64, pub(crate) request: AcquiredRpcRequest, } -#[derive(Clone)] +#[derive(Clone, Debug)] pub(crate) struct AcquiredRpcResponseOutbound { pub(crate) stream_id: P2pStreamId, pub(crate) response: AcquiredRpcResponse, @@ -141,6 +159,28 @@ impl From<(RpcResponse, &mut TRandomAccess)> for AcquiredRpcResponse { } } +impl From for RpcResponse { + fn from(rsp: AcquiredRpcResponse) -> Self { + match rsp { + AcquiredRpcResponse::StatusV1(b) => Self::StatusV1(b), + AcquiredRpcResponse::StatusV2(b) => Self::StatusV2(b), + AcquiredRpcResponse::Ping(b) => Self::Ping(b), + AcquiredRpcResponse::MetaData(b) => Self::MetaData(b), + AcquiredRpcResponse::BeaconBlock { fork_digest, ssz } => { + Self::BeaconBlock { fork_digest, ssz: ssz.read } + } + AcquiredRpcResponse::DataColumnSidecar { fork_digest, ssz } => { + Self::DataColumnSidecar { fork_digest, ssz: ssz.read } + } + AcquiredRpcResponse::ExecutionPayloadEnvelope { fork_digest, ssz } => { + Self::ExecutionPayloadEnvelope { fork_digest, ssz: ssz.read } + } + AcquiredRpcResponse::Error { error, msg, len } => Self::Error { error, msg, len }, + AcquiredRpcResponse::Complete => Self::Complete, + } + } +} + #[derive(Clone, Debug)] #[allow(clippy::large_enum_variant)] pub(crate) enum AcquiredRpcRequest { @@ -211,3 +251,27 @@ impl From<(RpcRequest, &mut TRandomAccess)> for AcquiredRpcRequest { } } } + +impl From for RpcRequest { + fn from(req: AcquiredRpcRequest) -> Self { + match req { + AcquiredRpcRequest::StatusV1(b) => Self::StatusV1(b), + AcquiredRpcRequest::StatusV2(b) => Self::StatusV2(b), + AcquiredRpcRequest::Ping(b) => Self::Ping(b), + AcquiredRpcRequest::Goodbye(b) => Self::Goodbye(b), + AcquiredRpcRequest::MetaData => Self::MetaData, + AcquiredRpcRequest::BlocksByRange(b) => Self::BlocksByRange(b), + AcquiredRpcRequest::BlockByRoot(read) => Self::BlockByRoot(read.read), + AcquiredRpcRequest::DataColumnsByRange { ssz, len } => { + Self::DataColumnsByRange { ssz, len } + } + AcquiredRpcRequest::DataColumnsByRoot(read) => Self::DataColumnsByRoot(read.read), + AcquiredRpcRequest::ExecutionPayloadEnvelopesByRange(b) => { + Self::ExecutionPayloadEnvelopesByRange(b) + } + AcquiredRpcRequest::ExecutionPayloadEnvelopesByRoot(read) => { + Self::ExecutionPayloadEnvelopesByRoot(read.read) + } + } + } +} diff --git a/crates/network/src/p2p/streams/rpc/tests.rs b/crates/network/src/p2p/streams/rpc/tests.rs new file mode 100644 index 000000000..d06f00b05 --- /dev/null +++ b/crates/network/src/p2p/streams/rpc/tests.rs @@ -0,0 +1,49 @@ +use std::io::Write; + +use silver_common::{TCache, TCacheProducer}; + +use super::*; + +#[test] +fn recovered_request_preserves_application_id_and_inline_payload() { + let producer = TCache::producer("", 1 << 16); + let mut consumer = producer.cache_ref().strict_random_access("", true).unwrap(); + let request = RpcRequest::data_columns_by_range(10, 20, u128::MAX); + let msg = RpcOutbound::Request(RpcRequestOutbound { application_id: 37, peer: 12, request }); + let recovered = AcquiredRpcOutbound::from((msg, &mut consumer)).into_message(12); + let RpcOutbound::Request(recovered) = recovered else { + panic!("expected an RPC request"); + }; + assert_eq!(recovered.application_id, 37); + assert_eq!(recovered.peer, 12); + let RpcRequest::DataColumnsByRange { ssz: expected, len: expected_len } = request else { + unreachable!(); + }; + let RpcRequest::DataColumnsByRange { ssz, len } = recovered.request else { + panic!("expected a data-column range request"); + }; + assert_eq!(ssz, expected); + assert_eq!(len, expected_len); +} + +#[test] +fn recovered_cached_requests_keep_the_original_descriptor() { + let mut producer = TCache::producer("", 1 << 16); + let mut consumer = producer.cache_ref().strict_random_access("", true).unwrap(); + let mut reservation = producer.reserve(32, true).unwrap(); + reservation.write_all(&[7; 32]).unwrap(); + let read = reservation.read(); + for request in [ + RpcRequest::BlockByRoot(read), + RpcRequest::DataColumnsByRoot(read), + RpcRequest::ExecutionPayloadEnvelopesByRoot(read), + ] { + let original = + RpcOutbound::Request(RpcRequestOutbound { application_id: 37, peer: 12, request }); + let recovered = AcquiredRpcOutbound::from((original, &mut consumer)).into_message(12); + assert_eq!(recovered.protocol(), original.protocol()); + assert_eq!(recovered.tcache_read().unwrap().seq(), read.seq()); + let acquired = consumer.acquire_strict(*recovered.tcache_read().unwrap()).unwrap(); + assert_eq!(acquired.buffer().unwrap().0, &[7; 32]); + } +} diff --git a/crates/network/src/tile.rs b/crates/network/src/tile.rs index b259139db..51f5bee0a 100644 --- a/crates/network/src/tile.rs +++ b/crates/network/src/tile.rs @@ -14,9 +14,9 @@ use mio::{Events, Poll, Token}; use quinn_proto::Transmit; use secp256k1::PublicKey; use silver_common::{ - BeaconStateEvent, ClusterIn, ClusterMsgIn, ClusterMsgOut, GossipFrameOutcome, - GossipFrameResult, GossipMsgIn, GossipMsgOut, P2pSend, PeerControl, PeerEvent, PeerStats, - RpcInbound, RpcOutbound, SLOTS_PER_EPOCH, SilverSpine, cell_store::RetentionEvent, + BeaconStateEvent, ClusterIn, ClusterMsgIn, ClusterMsgOut, GossipMsgIn, GossipMsgOut, P2pSend, + PeerControl, PeerEvent, PeerStats, RpcInbound, RpcOutbound, SLOTS_PER_EPOCH, SilverSpine, + cell_store::RetentionEvent, }; use silver_discovery::{DiscV5, Discovery, DiscoveryEvent}; @@ -154,9 +154,6 @@ impl NetworkTile { let mut on_event = |event| match event { Event::P2pNet(net_event) => match net_event { - NetEvent::GossipFrameResult(result) => { - adapter.produce(PeerEvent::SegmentedGossipResult(result)); - } NetEvent::PeerConnected { peer, addr, local_dialler } => { let port = addr.port(); adapter.produce(PeerEvent::P2pNewConnection { @@ -229,10 +226,10 @@ impl NetworkTile { tracing::debug!(peer=gossip_msg_out.peer_id, "send gossip"); self.inner.enqueue_gossip(gossip_msg_out) }, - P2pSend::SegmentedGossip { peer_id, frame } => { + P2pSend::SegmentedGossip { peer_id, frame, partial_cells } => { gossips += 1; self.inner.p2p_endpoint.enqueue_segmented_gossip( - peer_id, frame, &mut self.inner.context, + peer_id, frame, partial_cells, &mut self.inner.context, ) } P2pSend::Identify(peer) => { @@ -243,12 +240,18 @@ impl NetworkTile { self.inner.enqueue_rpc_out(rpc_outbound) }, }; - if result != p2p::SendResult::Ok && let P2pSend::SegmentedGossip { peer_id, frame } = msg { - producers.produce(PeerEvent::SegmentedGossipResult(GossipFrameResult { - p2p_peer: peer_id, - frame_seq: frame.read().seq(), - outcome: GossipFrameOutcome::Dropped, - })); + let dropped = match result { + p2p::SendResult::Ok => None, + p2p::SendResult::Dropped(msg) => Some(msg.expect("endpoint must identify dropped P2p messages")), + _ if matches!(msg, P2pSend::SegmentedGossip { .. }) => Some(msg), + _ => None, + }; + if let Some(msg) = dropped { + producers.produce(PeerEvent::P2pOutboundMessageDropped { + p2p_peer: msg.peer_id(), + protocol: msg.protocol(), + msg, + }); } match result { p2p::SendResult::Ok => {} @@ -263,16 +266,7 @@ impl NetworkTile { .into()), ); } - p2p::SendResult::MessageDropped => { - producers.peer_events.produce( - &(PeerEvent::P2pOutboundMessageDropped { - p2p_peer: msg.peer_id(), - protocol: msg.protocol(), - rpc_request, - } - .into()), - ); - } + p2p::SendResult::Dropped(_) => {} p2p::SendResult::ConnectionClosing => { tracing::debug!( peer = msg.peer_id(), diff --git a/crates/peer/src/manager/admission.rs b/crates/peer/src/manager/admission.rs index 4b65c3c18..53d940d69 100644 --- a/crates/peer/src/manager/admission.rs +++ b/crates/peer/src/manager/admission.rs @@ -1418,7 +1418,11 @@ mod tests { PeerEvent::P2pOutboundMessageDropped { p2p_peer: 1, protocol: StreamProtocol::Goodbye, - rpc_request: true, + msg: P2pSend::Rpc(RpcOutbound::Request(RpcRequestOutbound { + application_id: 0, + peer: 1, + request: RpcRequest::Goodbye(GOODBYE_TOO_MANY_PEERS.to_le_bytes()), + })), }, ] { cap.0.clear(); diff --git a/crates/peer/src/manager/mod.rs b/crates/peer/src/manager/mod.rs index 4f40ac5e8..b96cbbc1c 100644 --- a/crates/peer/src/manager/mod.rs +++ b/crates/peer/src/manager/mod.rs @@ -9,8 +9,8 @@ use std::{ }; use silver_common::{ - AgentString, Enr, GossipTopic, PeerControl, PeerEvent, PeerId, RpcSeverity, StreamProtocol, - SyncUpdate, + AgentString, Enr, GossipTopic, P2pSend, PeerControl, PeerEvent, PeerId, RpcOutbound, + RpcSeverity, StreamProtocol, SyncUpdate, ssz_view::{METADATA_SIZE, STATUS_V2_SIZE}, }; use silver_config::{ScoreParams, SyncingConfig}; @@ -309,7 +309,6 @@ impl PeerManager { emit: &mut impl FnMut(PeerControl), ) { match event { - PeerEvent::SegmentedGossipResult(_) => {} PeerEvent::P2pNewConnection { p2p_peer_id, peer_id_full, ip, port, local_dial } => { self.on_connected(p2p_peer_id, peer_id_full, ip, port, now, emit, local_dial); } @@ -337,11 +336,12 @@ impl PeerManager { } self.disconnect_after_failed_goodbye(p2p_peer, protocol, emit); } - PeerEvent::P2pOutboundMessageDropped { p2p_peer, protocol, rpc_request } => { + PeerEvent::P2pOutboundMessageDropped { p2p_peer, protocol, msg } => { // Local outbound-ring overflow — a backpressure signal, often // ours (blocked socket), not peer misbehaviour. No P7: a // stalled connection drops in bursts and the squared penalty // would graylist the whole mesh on a local uplink stall. + let rpc_request = matches!(msg, P2pSend::Rpc(RpcOutbound::Request(_))); if rpc_request { self.release_outbound_in_flight(p2p_peer, protocol); } diff --git a/crates/peer/src/manager/rpc.rs b/crates/peer/src/manager/rpc.rs index c741f2dab..feee164f5 100644 --- a/crates/peer/src/manager/rpc.rs +++ b/crates/peer/src/manager/rpc.rs @@ -760,6 +760,37 @@ mod tests { use super::*; use crate::manager::{attempts::OutboundAttempt, fixture::*}; + #[test] + fn dropped_message_releases_only_rpc_request_capacity() { + let now = Instant::now(); + let (mut mgr, mut cap) = fixture(vec![], ScoreParams::default()); + connect(&mut mgr, &mut cap, 1, 1, now); + let protocol = StreamProtocol::Ping; + let index = protocol.ordinal() as usize; + mgr.peers.get_mut(&1).unwrap().outbound_in_flight[index] = 2; + let response = RpcOutbound::Response(RpcResponseOutbound { + stream_id: P2pStreamId::new(1, 3, protocol, true), + response: RpcResponse::Ping([0; 8]), + }); + let request = RpcOutbound::Request(RpcRequestOutbound { + application_id: 7, + peer: 1, + request: RpcRequest::Ping([0; 8]), + }); + for (msg, remaining) in [(response, 2), (request, 1)] { + mgr.handle_event( + PeerEvent::P2pOutboundMessageDropped { + p2p_peer: 1, + protocol, + msg: P2pSend::Rpc(msg), + }, + now, + &mut |control| cap.0.push(control), + ); + assert_eq!(mgr.peers[&1].outbound_in_flight[index], remaining); + } + } + #[test] fn invalid_request_is_low_tolerance() { // Same severity for any protocol — peer claims our request was bad. From 295e1ec492feb0a92be8490ac122414964a49908 Mon Sep 17 00:00:00 2001 From: vladimir-ea Date: Fri, 18 Sep 2026 18:50:51 +0100 Subject: [PATCH 5/5] markups --- crates/common/src/spine/messages.rs | 5 +- crates/control/src/partial_exchange.rs | 67 ++++++++++++------- .../control/src/partial_exchange/response.rs | 5 +- crates/control/src/partial_exchange/tests.rs | 51 +++++++++++++- crates/e2e/tests/send_failures.rs | 67 +++++++++++++++++++ crates/gossip/src/partial.rs | 3 +- crates/network/src/tile.rs | 56 ++++++++-------- crates/peer/src/manager/admission.rs | 58 ++++++++++------ crates/peer/src/manager/mod.rs | 13 ++-- crates/peer/src/manager/rpc.rs | 10 +++ 10 files changed, 242 insertions(+), 93 deletions(-) create mode 100644 crates/e2e/tests/send_failures.rs diff --git a/crates/common/src/spine/messages.rs b/crates/common/src/spine/messages.rs index 18dfe7db9..5fff91919 100644 --- a/crates/common/src/spine/messages.rs +++ b/crates/common/src/spine/messages.rs @@ -417,9 +417,6 @@ pub enum PeerEvent { P2pCannotCreateStream { p2p_peer: usize, protocol: StreamProtocol, - /// Failed send was an outbound RPC request: the PM must release the - /// `outbound_in_flight` slot admitted for it, else it leaks. - rpc_request: bool, /// Response targeted a stream already closed/reset, as opposed to /// stream-credit exhaustion opening a new request stream. stream_gone: bool, @@ -440,6 +437,8 @@ pub enum PeerEvent { first_chunk_ms: u64, elapsed_ms: u64, }, + /// A send was rejected or an older queued message was evicted. + /// This event owns send-failure accounting; stream errors are diagnostics. P2pOutboundMessageDropped { p2p_peer: usize, protocol: StreamProtocol, diff --git a/crates/control/src/partial_exchange.rs b/crates/control/src/partial_exchange.rs index 9cf8d2d4e..1be53ab3a 100644 --- a/crates/control/src/partial_exchange.rs +++ b/crates/control/src/partial_exchange.rs @@ -1,5 +1,5 @@ use std::{ - collections::VecDeque, + collections::{VecDeque, hash_map::Entry}, time::{Duration, Instant}, }; @@ -69,21 +69,23 @@ impl PartialExchange { rows: usize, expires: Instant, now: Instant, - ) -> bool { - if self.exchanges.contains_key(&key) { - return true; - } - let peer = self.peer_exchanges.get(&key.peer); - if self.exchanges.len() >= self.capacity || - peer.is_some_and(|p| p.columns >= self.peer_capacity) || - (peer.is_none() && self.peer_exchanges.len() >= self.capacity) - { - ControlCounters::PartialStateLimited.inc(); - return false; + ) -> Option<&mut PeerColumnExchange> { + let full = self.exchanges.len() >= self.capacity; + match self.exchanges.entry(key) { + Entry::Occupied(entry) => Some(entry.into_mut()), + Entry::Vacant(entry) => { + let peer = self.peer_exchanges.get(&key.peer); + if full || + peer.is_some_and(|p| p.columns >= self.peer_capacity) || + (peer.is_none() && self.peer_exchanges.len() >= self.capacity) + { + ControlCounters::PartialStateLimited.inc(); + return None; + } + self.peer_exchanges.entry(key.peer).or_default().columns += 1; + Some(entry.insert(PeerColumnExchange::new(slot, rows, expires, now))) + } } - self.exchanges.insert(key, PeerColumnExchange::new(slot, rows, expires, now)); - self.peer_exchanges.entry(key.peer).or_default().columns += 1; - true } fn schedule(&mut self, key: ExchangeKey) { @@ -116,10 +118,11 @@ impl PartialExchange { } lazy_remaining -= 1; } - if !self.admit(key, column.slot, column.blob_count, column.expires, now) { + let Some(exchange) = + self.admit(key, column.slot, column.blob_count, column.expires, now) + else { continue; - } - let exchange = self.exchanges.get_mut(&key).unwrap(); + }; exchange.slot = column.slot; exchange.n_rows = column.blob_count; exchange.expires = column.expires; @@ -174,10 +177,9 @@ impl PartialExchange { return; } let key = ExchangeKey { peer: message.stream_id.peer(), group }; - if !self.admit(key, slot, message.metadata.n_rows, expires, now) { + let Some(exchange) = self.admit(key, slot, message.metadata.n_rows, expires, now) else { return; - } - let exchange = self.exchanges.get_mut(&key).unwrap(); + }; if exchange.remote.is_some() { ControlCounters::PartialMetadataReplaced.inc(); } @@ -263,7 +265,11 @@ impl PartialExchange { } // Keep the spent per-peer budget until the next heartbeat, even // when unsubscribing removes the peer's last column exchange. - self.peer_exchanges.get_mut(&key.peer).unwrap().columns -= 1; + if let Some(peer) = self.peer_exchanges.get_mut(&key.peer) { + peer.columns = peer.columns.saturating_sub(1); + } else { + tracing::error!(?key, "partial exchange has no peer budget during removal"); + } self.headers.forget_sent(key.peer, key.group); false }); @@ -298,8 +304,11 @@ impl PartialExchange { self.advance_slot(ingress, peers, producer, now, emit); let mut did_work = false; for _ in 0..self.ready.len().min(WORK_PER_SPIN) { - let key = self.ready.pop_front().unwrap(); - let exchange = self.exchanges.get_mut(&key).unwrap(); + let Some(key) = self.ready.pop_front() else { break }; + let Some(exchange) = self.exchanges.get_mut(&key) else { + tracing::error!(?key, "scheduled partial exchange is missing"); + continue; + }; exchange.scheduled = false; if now < exchange.retry_at { exchange.scheduled = true; @@ -347,7 +356,10 @@ impl PartialExchange { rows, header, }; - let budget = self.peer_exchanges.get_mut(&key.peer).unwrap(); + let Some(budget) = self.peer_exchanges.get_mut(&key.peer) else { + tracing::error!(?key, "partial exchange has no peer budget; skipping send"); + continue; + }; let (frame, bytes) = match response.write(producer, column.expires, budget.remaining_bytes()) { Ok(Some(frame)) => frame, @@ -449,7 +461,10 @@ impl PartialExchange { rows: 0, header: false, }; - let budget = self.peer_exchanges.get_mut(&key.peer).unwrap(); + let Some(budget) = self.peer_exchanges.get_mut(&key.peer) else { + tracing::error!(?key, "partial exchange has no peer budget; skipping withdrawal"); + continue; + }; if let Ok(Some((frame, bytes))) = response.write(producer, now + RETRY, budget.remaining_bytes()) { diff --git a/crates/control/src/partial_exchange/response.rs b/crates/control/src/partial_exchange/response.rs index a4eec2872..d48e43394 100644 --- a/crates/control/src/partial_exchange/response.rs +++ b/crates/control/src/partial_exchange/response.rs @@ -39,8 +39,9 @@ impl PartialResponse { "/eth2/{:02x}{:02x}{:02x}{:02x}/data_column_sidecar_{}/ssz_snappy", digest[0], digest[1], digest[2], digest[3], self.group.column ) - .unwrap(); - let topic = str::from_utf8(&topic.get_ref()[..topic.position() as usize]).unwrap(); + .map_err(|_| CacheFrameError::TooLarge)?; + let topic = str::from_utf8(&topic.get_ref()[..topic.position() as usize]) + .map_err(|_| CacheFrameError::InvalidDescriptor)?; let fulu = self.group.domain.format() == ForkName::Fulu; let mut group = gloas_group_id(&self.group.block_root, self.slot); let group = if fulu { diff --git a/crates/control/src/partial_exchange/tests.rs b/crates/control/src/partial_exchange/tests.rs index 02ec301fe..b0d38ceb7 100644 --- a/crates/control/src/partial_exchange/tests.rs +++ b/crates/control/src/partial_exchange/tests.rs @@ -238,6 +238,55 @@ impl Rig { } } +#[test] +fn missing_scheduled_exchange_is_discarded_without_sending() { + let mut rig = Rig::new(ForkName::Gloas); + assert!(rig.spin().is_empty()); + let key = ExchangeKey { peer: 1, group: rig.message(1, 0, 1).group }; + rig.exchange.ready.push_back(key); + let seq = rig.output.next_seq(); + + assert!(rig.spin().is_empty()); + assert!(rig.exchange.ready.is_empty()); + assert_eq!(rig.output.next_seq(), seq); +} + +#[test] +fn missing_peer_budget_prevents_sends_and_withdrawals() { + let mut rig = Rig::new(ForkName::Gloas); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 1); + assert_eq!(rig.spin().len(), 1); + rig.request(1, 0, 2); + rig.exchange.peer_exchanges.remove(&1); + let seq = rig.output.next_seq(); + + assert!(rig.spin().is_empty()); + let key = ExchangeKey { peer: 1, group: rig.message(1, 0, 2).group }; + assert_eq!(rig.exchange.exchanges[&key].sent, 0); + rig.exchange.expire(1, &rig.peers, &mut rig.output, rig.now, &mut |_| { + panic!("missing budget must not permit a withdrawal") + }); + assert!(rig.exchange.exchanges.is_empty()); + assert!(rig.exchange.ready.is_empty()); + assert_eq!(rig.output.next_seq(), seq); +} + +#[test] +fn removing_exchange_without_peer_budget_clears_queued_work() { + let mut rig = Rig::new(ForkName::Gloas); + rig.connect(1, true, false); + rig.publish(0); + rig.request(1, 0, 1); + assert!(!rig.exchange.exchanges.is_empty()); + rig.exchange.peer_exchanges.remove(&1); + + rig.exchange.remove_peer(1); + assert!(rig.exchange.exchanges.is_empty()); + assert!(rig.exchange.ready.is_empty()); +} + #[test] fn retained_full_columns_serve_requested_rows_for_both_forks_and_non_mesh_peers() { for format in [ForkName::Fulu, ForkName::Gloas] { @@ -648,7 +697,7 @@ fn subscription_changes_and_slot_expiry_cannot_reset_the_heartbeat_budget() { assert_eq!(rig.exchange.peer_exchanges[&1].remaining_bytes(), 0); // Old-slot drops must not reset new exchange state or schedule a retry. let key = ExchangeKey { peer: 1, group: rig.message(1, 0, 0).group }; - rig.exchange.admit(key, 1, ROWS, rig.now + Duration::from_secs(12), rig.now); + assert!(rig.exchange.admit(key, 1, ROWS, rig.now + Duration::from_secs(12), rig.now).is_some()); rig.dropped(1, frame); assert_eq!(rig.exchange.exchanges[&key].retry_at, rig.now); assert!(rig.exchange.ready.is_empty()); diff --git a/crates/e2e/tests/send_failures.rs b/crates/e2e/tests/send_failures.rs new file mode 100644 index 000000000..694a78e5e --- /dev/null +++ b/crates/e2e/tests/send_failures.rs @@ -0,0 +1,67 @@ +use std::{ + io::Write, + mem::discriminant, + time::{Duration, Instant}, +}; + +use flux::tile::Tile; +use silver_common::{ + CacheFrameRef, CacheSegment, GossipMsgOut, P2pSend, P2pStreamId, PeerEvent, RpcOutbound, + RpcRequest, RpcRequestOutbound, RpcResponse, RpcResponseOutbound, StreamProtocol, + TCacheProducer, test_util::ShmemDir, +}; +use silver_e2e::{PublisherStack, keypair_from_seed}; + +#[test] +fn unknown_peer_reports_every_send_variant_as_dropped() { + let dir = ShmemDir::new().expect("create test shared-memory directory"); + let addr = "127.0.0.1:0".parse().expect("test address is valid"); + let mut stack = + PublisherStack::new(dir.path(), "send_failures", addr, addr, keypair_from_seed(1)) + .expect("create test publisher stack"); + stack.network.loop_body(&mut stack.network_adapter); + stack.injector_adapter.consume(|_: PeerEvent, _| {}); + + let mut reservation = stack.mcache_producer.reserve(1, true).expect("reserve gossip payload"); + reservation.write_all(&[0]).expect("write gossip payload"); + let read = reservation.read(); + let frame = CacheFrameRef::write( + &mut stack.mcache_producer, + Instant::now() + Duration::from_secs(1), + &[0], + [CacheSegment::Framing { offset: 0, length: 1 }].into_iter(), + ) + .expect("write segmented gossip frame"); + let peer = 123; + for attempted in [ + P2pSend::Gossip(GossipMsgOut { peer_id: peer, tcache: read }), + P2pSend::SegmentedGossip { peer_id: peer, frame, partial_cells: Some(0) }, + P2pSend::Identify(peer), + P2pSend::Rpc(RpcOutbound::Request(RpcRequestOutbound { + peer, + application_id: 7, + request: RpcRequest::Ping([1; 8]), + })), + P2pSend::Rpc(RpcOutbound::Response(RpcResponseOutbound { + stream_id: P2pStreamId::new(peer, 3, StreamProtocol::Ping, true), + response: RpcResponse::Ping([2; 8]), + })), + ] { + stack.injector_adapter.produce(attempted); + stack.network.loop_body(&mut stack.network_adapter); + let mut dropped = None; + stack.injector_adapter.consume(|event: PeerEvent, _| { + if let PeerEvent::P2pOutboundMessageDropped { p2p_peer, protocol, msg } = event { + assert_eq!(p2p_peer, peer); + assert_eq!(protocol, attempted.protocol()); + assert!(dropped.replace(msg).is_none(), "one event per failed send"); + } + }); + let dropped = dropped.expect("every rejected send must be reported"); + assert_eq!(discriminant(&dropped), discriminant(&attempted)); + assert_eq!(dropped.peer_id(), peer); + if let (P2pSend::Rpc(dropped), P2pSend::Rpc(attempted)) = (dropped, attempted) { + assert_eq!(discriminant(&dropped), discriminant(&attempted)); + } + } +} diff --git a/crates/gossip/src/partial.rs b/crates/gossip/src/partial.rs index d58be8b05..49f10d9b7 100644 --- a/crates/gossip/src/partial.rs +++ b/crates/gossip/src/partial.rs @@ -105,7 +105,6 @@ impl PartialFrame<'_> { } let ssz_len = self.plan.map_or(0, |plan| plan.ssz_len()); - let meta_len = self.metadata.as_ref().map(|meta| parts_metadata_len(meta.n_rows)); let (fields_len, tail) = self.field_lengths(); let ext_len = fields_len + ssz_len + tail; @@ -142,7 +141,7 @@ impl PartialFrame<'_> { } if let Some(meta) = &self.metadata { let mut buf = [0u8; parts_metadata_len(128)]; - let bytes = &mut buf[..meta_len.unwrap()]; + let bytes = &mut buf[..parts_metadata_len(meta.n_rows)]; write_parts_metadata(meta.available, meta.requests, meta.n_rows, bytes); Tag::new(4, WireType::LengthDelimited).encode(&mut cursor); encode_bytes(bytes, &mut cursor); diff --git a/crates/network/src/tile.rs b/crates/network/src/tile.rs index 51f5bee0a..63f15d8a5 100644 --- a/crates/network/src/tile.rs +++ b/crates/network/src/tile.rs @@ -219,7 +219,6 @@ impl NetworkTile { } if !adapter.consume_one(|msg: P2pSend, producers| { - let rpc_request = matches!(&msg, P2pSend::Rpc(RpcOutbound::Request(_))); let result = match msg { P2pSend::Gossip(gossip_msg_out) => { gossips += 1; @@ -241,43 +240,44 @@ impl NetworkTile { }, }; let dropped = match result { - p2p::SendResult::Ok => None, - p2p::SendResult::Dropped(msg) => Some(msg.expect("endpoint must identify dropped P2p messages")), - _ if matches!(msg, P2pSend::SegmentedGossip { .. }) => Some(msg), - _ => None, - }; - if let Some(msg) = dropped { - producers.produce(PeerEvent::P2pOutboundMessageDropped { - p2p_peer: msg.peer_id(), - protocol: msg.protocol(), - msg, - }); - } - match result { - p2p::SendResult::Ok => {} - p2p::SendResult::StreamCreationError | p2p::SendResult::StreamGone => { - producers.peer_events.produce( - &(PeerEvent::P2pCannotCreateStream { - p2p_peer: msg.peer_id(), - protocol: msg.protocol(), - rpc_request, - stream_gone: matches!(result, p2p::SendResult::StreamGone), - } - .into()), + SendResult::Ok => None, + SendResult::Dropped(Some(msg)) => Some(msg), + SendResult::Dropped(None) => { + tracing::error!( + peer = msg.peer_id(), + protocol = ?msg.protocol(), + "endpoint dropped a message without identifying it" ); + None } - p2p::SendResult::Dropped(_) => {} - p2p::SendResult::ConnectionClosing => { + SendResult::StreamCreationError | SendResult::StreamGone => { + producers.produce(PeerEvent::P2pCannotCreateStream { + p2p_peer: msg.peer_id(), + protocol: msg.protocol(), + stream_gone: matches!(result, SendResult::StreamGone), + }); + Some(msg) + } + SendResult::ConnectionClosing => { tracing::debug!( peer = msg.peer_id(), protocol = ?msg.protocol(), "send refused: connection closing" ); + Some(msg) } - p2p::SendResult::UnknownPeer => { + SendResult::UnknownPeer => { // Can happen if peer has disconnected. tracing::debug!(peer=msg.peer_id(), protocol=?msg.protocol(), "Tried to send to unknown peer"); - }, + Some(msg) + } + }; + if let Some(msg) = dropped { + producers.produce(PeerEvent::P2pOutboundMessageDropped { + p2p_peer: msg.peer_id(), + protocol: msg.protocol(), + msg, + }); } }) { break; diff --git a/crates/peer/src/manager/admission.rs b/crates/peer/src/manager/admission.rs index 53d940d69..f6773adc3 100644 --- a/crates/peer/src/manager/admission.rs +++ b/crates/peer/src/manager/admission.rs @@ -1408,29 +1408,43 @@ mod tests { connect(&mut mgr, &mut cap, 1, 1, now); mgr.peers.get_mut(&1).unwrap().goodbye_sent = true; - for event in [ - PeerEvent::P2pCannotCreateStream { - p2p_peer: 1, - protocol: StreamProtocol::Goodbye, - rpc_request: true, - stream_gone: false, - }, - PeerEvent::P2pOutboundMessageDropped { - p2p_peer: 1, - protocol: StreamProtocol::Goodbye, - msg: P2pSend::Rpc(RpcOutbound::Request(RpcRequestOutbound { - application_id: 0, - peer: 1, - request: RpcRequest::Goodbye(GOODBYE_TOO_MANY_PEERS.to_le_bytes()), - })), - }, - ] { + for stream_creation_error in [false, true] { cap.0.clear(); - mgr.handle_event(event, now, &mut |control| cap.0.push(control)); - assert!(cap.0.iter().any(|control| matches!(control, PeerControl::P2pDisconnect { - p2p_connection: 1, - .. - }))); + if stream_creation_error { + mgr.handle_event( + PeerEvent::P2pCannotCreateStream { + p2p_peer: 1, + protocol: StreamProtocol::Goodbye, + stream_gone: false, + }, + now, + &mut |control| cap.0.push(control), + ); + assert!(cap.0.is_empty(), "stream diagnostics must not duplicate teardown"); + } + mgr.handle_event( + PeerEvent::P2pOutboundMessageDropped { + p2p_peer: 1, + protocol: StreamProtocol::Goodbye, + msg: P2pSend::Rpc(RpcOutbound::Request(RpcRequestOutbound { + application_id: 0, + peer: 1, + request: RpcRequest::Goodbye(GOODBYE_TOO_MANY_PEERS.to_le_bytes()), + })), + }, + now, + &mut |control| cap.0.push(control), + ); + assert_eq!( + cap.0 + .iter() + .filter(|control| matches!(control, PeerControl::P2pDisconnect { + p2p_connection: 1, + .. + })) + .count(), + 1 + ); } } diff --git a/crates/peer/src/manager/mod.rs b/crates/peer/src/manager/mod.rs index b96cbbc1c..87eca0140 100644 --- a/crates/peer/src/manager/mod.rs +++ b/crates/peer/src/manager/mod.rs @@ -322,10 +322,7 @@ impl PeerManager { self.database.dial_failed(&peer_id, now + DIAL_FAILURE_BACKOFF); } } - PeerEvent::P2pCannotCreateStream { p2p_peer, protocol, rpc_request, stream_gone } => { - if rpc_request { - self.release_outbound_in_flight(p2p_peer, protocol); - } + PeerEvent::P2pCannotCreateStream { p2p_peer, stream_gone, .. } => { if stream_gone { // Their teardown raced our (possibly late) response — // not peer misbehaviour. Counted, not penalised. @@ -334,13 +331,11 @@ impl PeerManager { crate::PeerCounters::StreamCreditExhausted.inc(); self.add_behaviour_penalty(p2p_peer, 1.0, "stream credit exhausted"); } - self.disconnect_after_failed_goodbye(p2p_peer, protocol, emit); } PeerEvent::P2pOutboundMessageDropped { p2p_peer, protocol, msg } => { - // Local outbound-ring overflow — a backpressure signal, often - // ours (blocked socket), not peer misbehaviour. No P7: a - // stalled connection drops in bursts and the squared penalty - // would graylist the whole mesh on a local uplink stall. + // Rejections and queue evictions release request capacity here. + // Drops alone aren't peer misbehaviour: squared P7 penalties + // would graylist the mesh during a local uplink stall. let rpc_request = matches!(msg, P2pSend::Rpc(RpcOutbound::Request(_))); if rpc_request { self.release_outbound_in_flight(p2p_peer, protocol); diff --git a/crates/peer/src/manager/rpc.rs b/crates/peer/src/manager/rpc.rs index feee164f5..e47dc885b 100644 --- a/crates/peer/src/manager/rpc.rs +++ b/crates/peer/src/manager/rpc.rs @@ -778,6 +778,16 @@ mod tests { request: RpcRequest::Ping([0; 8]), }); for (msg, remaining) in [(response, 2), (request, 1)] { + mgr.handle_event( + PeerEvent::P2pCannotCreateStream { + p2p_peer: 1, + protocol, + stream_gone: matches!(msg, RpcOutbound::Response(_)), + }, + now, + &mut |control| cap.0.push(control), + ); + assert_eq!(mgr.peers[&1].outbound_in_flight[index], 2); mgr.handle_event( PeerEvent::P2pOutboundMessageDropped { p2p_peer: 1,