Skip to content

wire: batched Window directive so directive traffic is O(1) in the number of live heights #191

Description

@bagelface

Consumer context: enables concurrent aggregation heights in the Gas Killer service. Full analysis: gas-killer/roadmap#18 sections A3 and A4. Service re-pins its rev in gas-killer/service#375.

Why one batched directive instead of W separate ones

This change fixes two problems at once, which is why it is worth a wire format rather than a config bump.

1. The per-peer quota cannot carry a window. DEFAULT_P2P_MESSAGES_PER_SECOND is 1.0, converted to Quota::with_period(1/rate) — a smooth quota with burst 1. LimitedSender filters rate-limited peers per key and Sender::send returns only the survivors, silently dropping to the rest (commonware-p2p src/lib.rs:149-167, src/utils/limited.rs:103-177). Sequencer::broadcast only logs when the result is entirely empty, so "2 of 3 nodes missed this Announce" is invisible.

With production settings (rebroadcastInterval: 15, p2pMessagesPerSecond: 1.0) and W separate per-height rebroadcast loops, the loops drift into alignment and burst: at W=8 that is ~7 of 8 dropped per burst, and which 7 is pseudo-random. Raising the quota is not a clean fix either, because the receive side does not drop — it sleeps the entire per-peer receive handler (commonware-p2p src/authenticated/lookup/actors/peer/actor.rs:294-298), head-of-line blocking every channel and Ping on that connection.

A batched directive makes the send rate O(1) in W: one message (or a few chunks) per rebroadcast tick regardless of window size. P2P_MESSAGES_PER_SECOND then never has to be tuned against W at all.

2. It carries the explicit floor that #192 needs, so skip rule 2 stops being inferred from message ordering.

Proposed shape

const TAG_WINDOW: u8 = 3;

pub enum WindowEntry<T: TaskData> {
    Announce { height: u64, task: T },
    Skip { height: u64 },
}

pub enum TaskDirective<T: TaskData> {
    Announce { .. },   // unchanged, still decodable
    Skip { .. },       // unchanged
    TipReport { .. },  // unchanged
    /// Every height below `base` is decided — never expect a directive for it.
    /// `entries` is the router's complete live set at broadcast time.
    /// `seq` is monotonic per router life so a delayed snapshot cannot regress state.
    Window { base: u64, seq: u64, entries: Vec<WindowEntry<T>> },
}

Notes on the design:

  • Additive tag. Do not add a base field to the existing Announce/Skip tags. An old node hitting an unknown tag decodes to Err(Error::InvalidEnum) and ingest logs and ignores it (core/src/wire/mod.rs:108, node/src/task_book.rs:403-405) — loud and safe. Changing the existing tags instead would have old nodes misparse and split digests.
  • Read::read_cfg (core/src/wire/mod.rs:98) currently reads tag then UInt(height) unconditionally. The height read has to move inside the per-tag arms.
  • base = min(inflight), not max. A height that has certified but whose execution is still running (execution can take minutes) is omitted from entries but must not raise base, or a node would skip a live neighbour.
  • Chunking is required. MAX_MESSAGE_SIZE is 1 MB and the sender panics above it; call_data alone is capped at 128 KB, so W=8 worst case is ~1 MB. Split into frames under ~256 KB, repeating base and seq in every frame. base/seq are monotone idempotent scalars and entries are disjoint per-height claims, so chunk ordering and partial delivery are both safe by construction — no reassembly protocol needed.

Acceptance

  • Codec round-trips all four variants; encode_size exact
  • Existing Announce/Skip/TipReport encodings byte-identical to today
  • An unknown tag still yields Err(InvalidEnum) (the property that makes the rollout safe)
  • Chunking test at the worst-case entry size, asserting every frame is under budget and carries base+seq
  • Golden-bytes test proving a one-height window at DIRECTIVE_FORMAT=v1 emits legacy bytes

Rollout is nodes first — see gas-killer/service#373 for why. Paired with #192 (decoder + skip rules) and #193 (the emitter).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestpriority:highMust be addressed soonthroughputTask throughput / parallel pipeline (roadmap#18, #19)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions