fix(core): harden exchange send path from full-codebase review
- Cut every fragment of a stream from one encoding (age frozen at the first fragment) and bounds-check the slice: re-encoding mid-stream corrupted fragments and could panic when the tick went backwards. - Fold the bloom summary to fit the MTU instead of sending a ~540-byte frame that small-MTU links reject; drop the sim's summary MTU exemption that hid this. - Finish a peer's in-flight stream before starting another, and count an addressed message as delivered only after its last fragment. - Alternate custody and broadcast per peer, rotate through addressed candidates, and evict the oldest bounce record when full, so neither path starves. - Add Exchange::on_send_error; Node::poll uses it so Busy/Unreachable sends are retried rather than silently dropped. - Clear bounce records per contact, record duplicates against their sender, and skip addressed messages that cannot fit the MTU. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+84
-11
@@ -9,6 +9,12 @@
|
||||
//!
|
||||
//! This type is `core`-only: it owns a fixed `[u8; SUMMARY_BYTES]` array and
|
||||
//! never touches the heap, so it compiles in `no_std` without `alloc`.
|
||||
//!
|
||||
//! A filter can be *folded* to any power-of-two size up to `SUMMARY_BYTES` so a
|
||||
//! summary fits a narrow link's MTU. Folding ORs the upper half onto the lower
|
||||
//! half; because every capacity is a power of two, a bit index reduced modulo
|
||||
//! the full capacity and then modulo the folded one lands on the same bit, so
|
||||
//! a folded filter keeps every member at the cost of more false positives.
|
||||
|
||||
use crate::consts;
|
||||
use crate::observation::ObservationId;
|
||||
@@ -18,6 +24,8 @@ use crate::observation::ObservationId;
|
||||
pub struct Bloom {
|
||||
pub salt: u32,
|
||||
bits: [u8; consts::SUMMARY_BYTES],
|
||||
/// Bytes in use: a power of two in `1..=SUMMARY_BYTES`.
|
||||
len: usize,
|
||||
}
|
||||
|
||||
impl Bloom {
|
||||
@@ -26,39 +34,74 @@ impl Bloom {
|
||||
Bloom {
|
||||
salt,
|
||||
bits: [0u8; consts::SUMMARY_BYTES],
|
||||
len: consts::SUMMARY_BYTES,
|
||||
}
|
||||
}
|
||||
|
||||
/// Sets the `BLOOM_K` bits for `id`.
|
||||
pub fn insert(&mut self, id: &ObservationId) {
|
||||
for idx in positions(self.salt, id) {
|
||||
for idx in positions(self.salt, id, self.len) {
|
||||
self.bits[idx / 8] |= 1 << (idx % 8);
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns `true` if `id` may have been inserted (all of its bits are set).
|
||||
pub fn might_contain(&self, id: &ObservationId) -> bool {
|
||||
positions(self.salt, id)
|
||||
positions(self.salt, id, self.len)
|
||||
.iter()
|
||||
.all(|idx| self.bits[idx / 8] & (1 << (idx % 8)) != 0)
|
||||
}
|
||||
|
||||
/// Rebuilds a `Bloom` from its raw parts.
|
||||
///
|
||||
/// Returns `None` unless `bits` is exactly `SUMMARY_BYTES` long, so a caller
|
||||
/// deserializing from an untrusted source cannot construct a filter from a
|
||||
/// truncated or oversized backing array.
|
||||
/// Returns `None` unless `bits.len()` is a power of two no larger than
|
||||
/// `SUMMARY_BYTES` (a full or folded filter), so a caller deserializing
|
||||
/// from an untrusted source cannot construct a filter from a truncated or
|
||||
/// oversized backing array.
|
||||
pub fn from_parts(salt: u32, bits: &[u8]) -> Option<Self> {
|
||||
let bytes: [u8; consts::SUMMARY_BYTES] = bits.try_into().ok()?;
|
||||
Some(Bloom { salt, bits: bytes })
|
||||
if !valid_len(bits.len()) {
|
||||
return None;
|
||||
}
|
||||
let mut bytes = [0u8; consts::SUMMARY_BYTES];
|
||||
bytes[..bits.len()].copy_from_slice(bits);
|
||||
Some(Bloom {
|
||||
salt,
|
||||
bits: bytes,
|
||||
len: bits.len(),
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns this filter folded down to `bytes` bytes, or `None` unless
|
||||
/// `bytes` is a power of two no larger than the current size.
|
||||
///
|
||||
/// Every id the filter contains is still contained after folding.
|
||||
pub fn fold(&self, bytes: usize) -> Option<Bloom> {
|
||||
if !valid_len(bytes) || bytes > self.len {
|
||||
return None;
|
||||
}
|
||||
let mut out = [0u8; consts::SUMMARY_BYTES];
|
||||
for (i, b) in self.bits[..self.len].iter().enumerate() {
|
||||
out[i % bytes] |= b;
|
||||
}
|
||||
Some(Bloom {
|
||||
salt: self.salt,
|
||||
bits: out,
|
||||
len: bytes,
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns the raw backing bit array as a slice.
|
||||
pub fn bits(&self) -> &[u8] {
|
||||
&self.bits
|
||||
&self.bits[..self.len]
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns `true` for a filter size in bytes that `positions` supports: a power
|
||||
/// of two no larger than `SUMMARY_BYTES`.
|
||||
fn valid_len(bytes: usize) -> bool {
|
||||
bytes.is_power_of_two() && bytes <= consts::SUMMARY_BYTES
|
||||
}
|
||||
|
||||
/// Computes the `BLOOM_K` bit indices for `id` under `salt` using the pinned
|
||||
/// double-hash scheme shared with the simulation.
|
||||
///
|
||||
@@ -72,9 +115,12 @@ impl Bloom {
|
||||
/// h1 = fmix64(a)
|
||||
/// h2 = fmix64(b) | 1
|
||||
/// capacity = SUMMARY_BYTES * 8
|
||||
/// bit_index = (h1 + k * h2) % capacity
|
||||
/// bit_index = ((h1 + k * h2) % capacity) % (len * 8)
|
||||
/// ```
|
||||
fn positions(salt: u32, id: &ObservationId) -> [usize; consts::BLOOM_K] {
|
||||
///
|
||||
/// `len` is the filter size in bytes; the second reduction is a no-op for a
|
||||
/// full-size filter and matches [`Bloom::fold`] for a folded one.
|
||||
fn positions(salt: u32, id: &ObservationId, len: usize) -> [usize; consts::BLOOM_K] {
|
||||
fn fmix64(mut x: u64) -> u64 {
|
||||
x ^= x >> 33;
|
||||
x = x.wrapping_mul(0xff51afd7ed558ccd);
|
||||
@@ -95,7 +141,7 @@ fn positions(salt: u32, id: &ObservationId) -> [usize; consts::BLOOM_K] {
|
||||
let capacity = (consts::SUMMARY_BYTES * 8) as u64;
|
||||
let mut out = [0usize; consts::BLOOM_K];
|
||||
for (k, slot) in out.iter_mut().enumerate() {
|
||||
*slot = ((h1.wrapping_add((k as u64).wrapping_mul(h2))) % capacity) as usize;
|
||||
*slot = ((h1.wrapping_add((k as u64).wrapping_mul(h2))) % capacity) as usize % (len * 8);
|
||||
}
|
||||
out
|
||||
}
|
||||
@@ -229,4 +275,31 @@ mod tests {
|
||||
assert_eq!(bloom.salt, 9);
|
||||
assert_eq!(bloom.bits(), &raw[..]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn folded_filter_keeps_members_and_round_trips() {
|
||||
let mut bloom = Bloom::new(11);
|
||||
let mut rng = Rng::new(6);
|
||||
let ids: [ObservationId; 50] = std::array::from_fn(|_| rng.id());
|
||||
for id in &ids {
|
||||
bloom.insert(id);
|
||||
}
|
||||
for bytes in [consts::SUMMARY_BYTES, 256, 128, 64, 1] {
|
||||
let folded = bloom.fold(bytes).expect("power-of-two fold");
|
||||
assert_eq!(folded.bits().len(), bytes);
|
||||
// A folded filter decoded from the wire answers the same way.
|
||||
let wire = Bloom::from_parts(11, folded.bits()).expect("folded length accepted");
|
||||
for id in &ids {
|
||||
assert!(folded.might_contain(id), "fold to {bytes} lost a member");
|
||||
assert!(
|
||||
wire.might_contain(id),
|
||||
"decoded fold to {bytes} lost a member"
|
||||
);
|
||||
}
|
||||
}
|
||||
// Folding is only defined for power-of-two sizes no larger than the filter.
|
||||
assert!(bloom.fold(0).is_none());
|
||||
assert!(bloom.fold(100).is_none());
|
||||
assert!(bloom.fold(consts::SUMMARY_BYTES * 2).is_none());
|
||||
}
|
||||
}
|
||||
|
||||
+738
-200
File diff suppressed because it is too large
Load Diff
@@ -140,10 +140,10 @@ impl<S: Store, T: Transport> Node<S, T> {
|
||||
/// Drains all pending transport events, storing any newly received
|
||||
/// observations, then emits at most one outbound frame.
|
||||
///
|
||||
/// Returns the number of *new* observations stored this round. A
|
||||
/// [`crate::transport::SendError`] is ignored: the observation stays
|
||||
/// queued only when the transport was `Busy`, otherwise it is dropped and
|
||||
/// re-queued by the next summary round.
|
||||
/// Returns the number of *new* observations stored this round. If the
|
||||
/// transport refuses the frame with `Busy` or `Unreachable`, the exchange
|
||||
/// rolls back and offers the same frame on a later poll; a `TooLarge`
|
||||
/// frame is dropped (see [`Exchange::on_send_error`]).
|
||||
pub fn poll(&mut self, now: Tick) -> usize {
|
||||
let mut stored = 0usize;
|
||||
while let Some(ev) = self.transport.poll() {
|
||||
@@ -158,7 +158,9 @@ impl<S: Store, T: Transport> Node<S, T> {
|
||||
.exchange
|
||||
.next_frame(&self.store, mtu, now, &mut self.out)
|
||||
{
|
||||
let _ = self.transport.send(&addr, &self.out[..n]);
|
||||
if let Err(err) = self.transport.send(&addr, &self.out[..n]) {
|
||||
self.exchange.on_send_error(&err);
|
||||
}
|
||||
}
|
||||
stored
|
||||
}
|
||||
|
||||
@@ -308,3 +308,52 @@ fn belief_includes_addressed_to_me() {
|
||||
assert_eq!(belief.payload, Payload::from_slice(b"for-me").unwrap());
|
||||
assert_eq!(belief.observer, [1; 16]);
|
||||
}
|
||||
|
||||
/// A `PairTransport` that reports `Busy` for every other send, dropping it.
|
||||
struct Flaky {
|
||||
inner: PairTransport,
|
||||
sends: usize,
|
||||
}
|
||||
|
||||
impl Transport for Flaky {
|
||||
type Addr = u8;
|
||||
|
||||
fn poll(&mut self) -> Option<Event<Self::Addr>> {
|
||||
self.inner.poll()
|
||||
}
|
||||
|
||||
fn send(&mut self, to: &Self::Addr, frame: &[u8]) -> Result<(), SendError> {
|
||||
self.sends += 1;
|
||||
if self.sends % 2 == 1 {
|
||||
return Err(SendError::Busy);
|
||||
}
|
||||
self.inner.send(to, frame)
|
||||
}
|
||||
|
||||
fn mtu(&self) -> usize {
|
||||
self.inner.mtu()
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn busy_sends_are_retried() {
|
||||
let (a, b) = PairTransport::pair(1500);
|
||||
let mut na = Node::new([1u8; 16], 0, MemStore::new(), Flaky { inner: a, sends: 0 });
|
||||
let mut nb = Node::new([2u8; 16], 0, MemStore::new(), Flaky { inner: b, sends: 0 });
|
||||
na.observe(b"door/1", b"open", None, 0).unwrap();
|
||||
na.observe(b"door/2", b"closed", None, 0).unwrap();
|
||||
nb.observe(b"bat/1", b"up", None, 0).unwrap();
|
||||
na.observe_to([2; 16], 60_000, b"task", b"go", None, 0)
|
||||
.unwrap();
|
||||
na.transport_mut().inner.connect();
|
||||
nb.transport_mut().inner.connect();
|
||||
|
||||
// The clock stands still, so no summary is ever re-sent: a frame lost to
|
||||
// Busy is only delivered if the exchange retries it.
|
||||
for _ in 0..40 {
|
||||
na.poll(0);
|
||||
nb.poll(0);
|
||||
}
|
||||
assert_eq!(na.store().len(), 4);
|
||||
assert_eq!(nb.store().len(), 4);
|
||||
}
|
||||
|
||||
@@ -11,7 +11,6 @@ use rand::Rng;
|
||||
use std::cell::RefCell;
|
||||
use std::rc::Rc;
|
||||
use tines_core::transport::{Event, FrameBuf, SendError, Transport};
|
||||
use tines_core::Frame;
|
||||
|
||||
/// A node's view of a shared `SimWorld`.
|
||||
pub struct SimTransport {
|
||||
@@ -39,17 +38,12 @@ impl Transport for SimTransport {
|
||||
/// Order of checks: reachability, then MTU, then the loss roll. A frame that
|
||||
/// survives loss is enqueued; a surviving frame is duplicated with the dup
|
||||
/// roll (counting each enqueue toward `frames_sent`).
|
||||
///
|
||||
/// Summary frames are control messages that carry no wire fragmentation: the
|
||||
/// core emits a fixed-size bloom summary that can exceed a small link MTU,
|
||||
/// so control frames are exempt from the per-frame MTU check that governs
|
||||
/// data (`Obs`/`Frag`) frames.
|
||||
fn send(&mut self, to: &Self::Addr, frame: &[u8]) -> Result<(), SendError> {
|
||||
let mut w = self.world.borrow_mut();
|
||||
if !w.trace.in_contact(self.me, *to, w.prev_t) {
|
||||
return Err(SendError::Unreachable);
|
||||
}
|
||||
if frame.len() > w.cfg.mtu && !matches!(Frame::decode(frame), Ok(Frame::Summary { .. })) {
|
||||
if frame.len() > w.cfg.mtu {
|
||||
return Err(SendError::TooLarge);
|
||||
}
|
||||
if w.rng.gen::<f64>() < w.cfg.loss {
|
||||
|
||||
Reference in New Issue
Block a user