Skip to content

fix(replication)!: bound monetized first-audit scheduling (ADR-0004 Amendment 2)#177

Open
grumbach wants to merge 13 commits into
WithAutonomi:mainfrom
grumbach:fix/first-audit-scheduler
Open

fix(replication)!: bound monetized first-audit scheduling (ADR-0004 Amendment 2)#177
grumbach wants to merge 13 commits into
WithAutonomi:mainfrom
grumbach:fix/first-audit-scheduler

Conversation

@grumbach

@grumbach grumbach commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

fix(replication)!: bound monetized first-audit scheduling (ADR-0004 Amendment 2)

Production symptom

ADR-0004 (#149, released in v0.14.3 on 2026-07-08) degraded production:
downloads ~31% slower across the daily 20-item runs, replication ERROR volume
rising from ~53k/day to 547k/day by Jul 13 — 545,046 of which were audit
reason=Timeout failures across 73 hosts / 996 services. The DEV-01/DEV-02
matched ablation attributed the regression to the deterministic monetized-pin
first-audit path: detaching just that sender cut generic one-key timeout
failures 99.955%, storage-commitment audit traffic 97.7%, responsible-audit
send failures 65.5%, node network bytes ~23% both directions, and mean peak
service RSS 40.4%, while keeping 100% upload/download success.

Root cause

Fleet-wide first-audit launch pressure scaled as
uploads x pinned-quotes-per-proof x verifying-storers, unbounded:

  1. Nomination was per quote, not per payment. Every pinned quote in a
    verified single-node proof (up to 7 — six of which were never paid) and
    every merkle pool candidate (16, of which only depth were paid) was
    nominated, by every storer that verified the proof (~5-9 per chunk).
  2. Pin-level dedup is defeated hourly. Commitment rotation mints a fresh
    pin, re-arming every observer against every peer every hour.
  3. No launch-rate bound. The drainer launched an audit for every pending
    peer whose 30-min per-peer cooldown was open — all concurrently, in one
    pass. The per-peer cooldown bounds one observer against one peer; nothing
    bounded the aggregate. (DEV-03 also showed per-service concurrency caps
    alone don't help: 744,674 launches in 3.17h, 77.7% timing out.)
  4. Every launch is expensive for the target. A subtree audit's round 1
    reads and hashes the full bytes of ~sqrt(N) committed chunks; round 2
    ships 3-5 full chunks. Storers of one chunk all verify the same payment at
    the same instant, so unjittered first audits arrived as same-millisecond
    bursts that tripped the target's per-peer responder admission cap — and
    drop-on-overload is recorded by every auditor as a peer Timeout.

What changed

Payments nominate, the clock launches. The upload path cannot be removed
as a trigger entirely — a commitment delivered only as a quote sidecar never
enters gossip caches, so the ADR-0002 lottery can never audit it — but it no
longer drives launch rate:

  • Paid-only nomination (verifier). Single-node proofs nominate only the
    candidate whose on-chain settlement verified (the paid median issuer);
    merkle proofs nominate only the contract-paid paid_node_addresses
    indices. The ADR-0004 cross-check (arithmetic re-check + mismatch evidence)
    still runs on every quote/candidate.
  • Launch limiter (drainer). A token bucket (burst
    FIRST_AUDIT_BUDGET_BURST = 2, one token per
    FIRST_AUDIT_LAUNCH_INTERVAL = 5 min; sustained 12 launches/hour/node,
    plus at most the burst of 2 in the first hour), an in-flight cap
    (FIRST_AUDIT_MAX_INFLIGHT = 2, drop-guarded so a panicking or cancelled
    audit task can never leak a slot and wedge the cap), and a uniform
    0-30s launch jitter. Fleet-wide pressure becomes nodes x refill-rate,
    independent of upload volume. Budget deferral is penalty-free: the pin
    stays pending (newest-per-peer, bounded) and retries as tokens refill;
    a deferral never burns the peer's cooldown stamp and consumes nothing
    until the launch actually commits. Launch passes alternate between
    newest-first (freshest answerability windows) and oldest-first
    (starvation resistance: a stream of fresh decoy settlements cannot keep
    an aging pin from ever launching).
  • Settlement-redirect rejection (verifier). "Paid" now means recorded
    FOR the quote's rewards address, not merely under its quote hash: the
    vault stores whatever (rewardsAddress, amount) the payer supplied, so
    the previous amount-only check accepted a payment the client redirected
    to its own wallet while the issuer was treated as paid. The verifier now
    compares the vault's bytes16 payee prefix against the quote's address
    (validated against the real contract on the Anvil testnet workload).
    Honest clients build the payment from the quote, so nothing legit is
    rejected.
  • Per-peer re-audit window (drainer). After a first audit launches at a
    peer, further nominations for that peer are dropped for
    FIRST_AUDIT_PEER_REAUDIT_INTERVAL = 2h (inside the 3h answerability TTL)
    — unless the new pin's committed key count exceeds the audited one by

    1.5x, which re-nominates immediately. The jump override preserves the
    anti-inflation property (sidecar-only pins are visible to payment
    verifiers alone); stable-count rotations stop re-arming the fleet.

  • Observability (extends feat(replication): add first-audit scheduler observability #173, labels append-only). New
    rate_deferred_attempts, window_deduped, ingress_dropped counters and a
    tokens gauge in the scheduler summary; rate_deferred / window_deduped
    outcome labels. ingress_dropped (nominations dropped at the full channel)
    is the recommended rollout gate alongside responder admission drops and the
    timeout rate.
  • ADR-0004 Amendment 2 documents first audits as bounded best-effort
    sampling and restates the coverage guarantee: the FIRST commitment earning
    money for a peer, and any materially-jumped successor, faces a prompt
    audit; stable rotations rely on the gossip lottery for re-audits.

Security argument

  • A peer's first monetized pin is nominated at every storer that verified the
    payment; with burst 2 + 12/h/node budgets, the first payment still yields
    an audit within minutes from at least one observer, and audit consequences
    (confirmed failure, credit revocation) are unchanged for launched audits.
  • The count-jump override closes the "pass an audit small, then mint an
    inflated sidecar-only commitment" path.
  • Unpaid quotes lose deterministic coverage; they earned nothing, and their
    gossiped commitments remain under the ADR-0002 lottery (p=0.2 per
    observing neighbour per rotation, cooldown-gated).

Validation

  • 697 lib tests pass (14 new: launch-limiter budget/refill/inflight/window/
    count-jump semantics; per-launch alternating starvation lane; paid-only
    nomination on both payment paths; settlement-redirect rejection).
  • cargo fmt --check, both clippy passes (--all-targets --all-features,
    -D warnings), and RUSTDOCFLAGS="--deny=warnings" cargo doc clean.
  • ADR governance check passes. Payment-path and audit-path e2e suites
    (subtree_audit_testnet, merkle/legacy payment) pass unchanged.
  • Adversarial codex xhigh (gpt-5.6-sol) review to convergence, including a
    full go/no-go against the production-incident dossier. Findings found and
    fixed across rounds: settlement-address binding, drop-guarded in-flight
    slot, bounded nomination ingress channel, post-jitter answerability
    re-screen, and a starvation lane that had to move from per-pass to
    per-launch alternation. Final verdict: GO for staging, no open findings.
  • Disclosed, pre-existing, out of node-side reach: the vault's
    payForQuotes unconditionally overwrites completedPayments[quoteHash].
    A third party who learns a quote hash can overwrite the record and grief
    the PUT — this already defeats the long-standing amount check with a
    1-wei payment on every node version, and equivalently defeats the new
    address binding. Needs a contract-side fix (reject or accumulate on
    existing entries); tracked as follow-up. This PR neither introduces nor
    can fix it.
  • Local A/B testnet (tests/e2e/first_audit_ab.rs, env-gated,
    ugly_first_audit/run_ab.sh): identical production-shaped workload
    (20 nodes, real Anvil settlement, quote sidecars with pins on 280/280
    quotes, median payment, close-group PUTs, 57 commitment rotations per arm,
    65 paid uploads + 15 downloads, 100% success both arms) against baseline
    origin/main and this branch:

Three runs of the same A/B (the audit counters are stable across all three;
transfer latencies on a laptop-hosted 20-node net are noisy, so all three
are shown rather than the best):

Signal baseline main this branch
First-audit launches 183 / 185 / 181 38 / 38 / 38
Audit failures (all reason=Timeout) 19 / 19 / 19 6 / 5 / 5
Rate-deferrals absorbed by the budget 0 / 0 / 0 911 / 647 / 769
Rotated pins suppressed by the re-audit window 0 / 0 / 0 0 / 28 / 28
Upload p50 (ms) 190 / 241 / 250 184 / 467 / 247
Download p50 (ms) 22 / 37 / 52 21 / 93 / 38
Responder admission drops 0 0

Launches are cut ~79% and, more importantly, capped: the baseline's
launch count grows with upload volume, this branch's does not. Timeout
failures fall by two thirds with the same timeout policy, confirming they
were self-inflicted overload rather than slow peers. Transfers are at
parity in runs 1 and 3; run 2's arm-B upload p50 (467 ms) is an outlier
that did not reproduce and has no candidate cause in the diff (the payment
hot path makes the same single contract call as before; the scheduler
changes run in detached tasks), so it is reported as host noise rather
than explained away.

A 20-node local net cannot reproduce the full production melt (it lacks
the 997-service fan-in that saturates responder pools — hence zero
admission drops in both arms), but the direction and magnitude match the
DEV-01/DEV-02 ablation. The decisive test is Chris's matched staging
testnet against the preserved DEV-02 baseline.

Out of scope (the other ADR-0004 findings, tracked separately)

This PR fixes the perf/error regression Chris reported. The investigation's
other findings are distinct problems on their own tracks:

  • Quote-price drop (Jae's chart): pricing now counts the strict closest-7
    commitment while storage intake reaches width 20, so a node prices ~35%
    (7/20) of what it holds. This is a deliberate ADR-0004 semantic change
    (price = logical responsibility), not a bug in this path. The open decision
    is whether pricing should track physical storage pressure instead; it needs
    its own ADR revision, not this PR.
  • Cheap-onboarding / low-price-while-disk-full (Jae): the unpriced
    retained set. Shrinks once the prune fixes land, and is the subject of the
    receiver-side price-floor work below.
  • Prune liveness + restart-reset hysteresis + prune telemetry: fix(replication): batch record prune audit challenges #170/fix(pruning): make prune candidacy independent of repair-hint history #175
    (already on rc-2026.7.1) plus the still-pending restart-persistent
    hysteresis.
  • LMDB capacity/write-liveness (Jae's "fundamental problem"): the
    free-space guard rejects writes before LMDB reuses freed pages, and there
    is no compaction path. Needs a capacity-guard fix; no PR yet.
  • Receiver-side price floor (from the ant-client perf(replication): reduce neighbor sync CPU spikes #150 discussion): restore
    a node floor priced from the committed key count, node-first rollout.
  • fix(replication): drain priority sync queue and recover from routing-event lag #165 shared audit coordinator / false-timeout elimination (Mick).

Review update (post-jitter suppression)

Addressing the review-team finding: the scheduler previously committed the
suppression state (first_audited + the 2h per-peer window + token + launch
count) at launch time, then jittered and re-checked answerability in the child,
so a quote near the answerability boundary could obtain full "already audited"
suppression and drop a same-count successor for two hours despite no audit
sending. The first-audit path is now a drainer-owned reserve / promote /
cancel
scheduler: a nomination reserves a token and an in-flight slot but
stamps NO suppression; a jitter timer fires and the authoritative answerability

  • shared-cooldown check-and-stamp runs under the cooldown write lock
    immediately before the send; only that promotion stamps first_audited/window,
    flips the lane, and counts the launch. A cancelled reservation refunds the
    token, releases the slot, and leaves nothing behind (no rollback). At most one
    reservation is outstanding (preserving lane alternation), and a same-peer
    successor arriving during a reservation is retained. Design and implementation
    were taken to convergence with adversarial review.

Review update 2 (security-aware coalescing)

Addressing the re-review: the per-peer pending coalescing replaced same-peer
work by arrival order only, so a peer could erase an inflated (audit-worthy)
sidecar-only pin for the cost of one cheaper same-peer settlement (the
count-jump override only compares against the last audited count, and
sidecar-only pins have no lottery backstop). Coalescing is now
highest-count-per-peer (newest on an equal-count tie): a strictly-lower
nomination never displaces a higher-count pending pin, and a suppressed lower
nomination does not disturb the retained pin's queue position. The same rule
governs the cooldown-race requeue of a reservation, which also now counts a
capacity eviction in the observability funnel. ADR-0004 Amendment 2 documents
the rule and the accepted residual (a single per-peer slot may sacrifice the
lower fallback if the retained higher pin later ages out of the answerability
window). The non-blocking notes are addressed in the ADR: the single-node
tied-median nomination limit and the launch-time (not outcome-time) window
stamp are documented as staging-measured coverage limits.

Rollout

Load is now a fixed function of fleet size: steady-state 12 first-audits per
node per hour (~12k/hour across ~1000 nodes, versus the old
grows-with-uploads storm). No wire change, no persistent state, so upgraded
and v0.14.3 nodes interoperate and the change reverts cleanly; legacy nodes
keep generating their old unbounded load until upgraded, so relief is
proportional to the upgraded share. Canary node-first and gate widening on
the timeout rate, responder admission drops, ingress_dropped, and node
CPU/bandwidth. The decisive validation is a matched staging run at fleet
scale against the preserved DEV-02 baseline.

Semver

No wire changes. The ReplicationConfig public surface is unchanged (all
new knobs are module constants). Marked ! for the behavioral contract
change: first audits become best-effort bounded sampling (ADR-0004
Amendment 2), and payment verification additionally requires the settlement
to be recorded for the quote's rewards address.

grumbach added 7 commits July 14, 2026 11:47
…mendment 2)

Production v0.14.3 showed the deterministic first-audit path amplifying
fleet-wide: launches scaled as uploads x pinned-quotes-per-proof x
verifying-storers, with hourly pin rotation defeating dedup and no
aggregate launch bound. The overflow tripped responder admission caps and
was recorded as peer Timeouts (545k/24h), degrading downloads ~31%.

Payments now NOMINATE pins; the clock LAUNCHES them:

- verifier: nominate only the settlement-verified paid candidate
  (single-node median issuer; merkle paid_node_addresses indices), not
  every quote in the bundle / candidate in the pool
- drainer: launch limiter with a token bucket (burst 2, one launch per
  5 min), an in-flight cap (2), and 0-30s launch jitter so one chunk's
  storers do not challenge the paid peer simultaneously
- per-peer re-audit window (2h) that survives pin rotation, with a >1.5x
  committed-count jump override so inflated sidecar-only pins are still
  re-nominated immediately
- budget deferrals are penalty-free and consume nothing; newest-monetized
  pins get budget first
- scheduler summary gains rate_deferred_attempts / window_deduped /
  tokens; outcome labels stay append-only vs WithAutonomi#173
- ADR-0004 Amendment 2 documents first audits as bounded best-effort
  sampling and restates the coverage guarantee
- tests/e2e/first_audit_ab.rs: env-gated production-shaped A/B workload
  driver (real Anvil settlement, quote sidecars, median payment,
  close-group PUTs, periodic rotation) for baseline-vs-fix comparisons
…limiter accounting

Review-round fixes for the first-audit scheduler change:

- Settlement-redirect rejection: completedPayments stores whatever
  (rewardsAddress, amount) the payer supplied for a quote hash, so the
  amount-only check accepted a payment the client redirected to its own
  wallet while the median issuer was treated (and first-audited) as paid.
  The verifier now compares the vault's bytes16 payee prefix against the
  quote's rewards address; validated against the real contract via the
  Anvil workload (honest pay_for_quotes settlements still verify).
- Drop-guarded in-flight slot: a panicking or cancelled first-audit task
  can no longer leak an in-flight slot and wedge the cap shut.
- Correct the launch-rate wording (sustained 12/h plus at most the burst
  of 2 in the first hour) in config docs and ADR-0004 Amendment 2.
- A/B workload driver only runs when FIRST_AUDIT_AB=1 exactly.
…fter jitter

Final review-round hardenings against the production-incident context:

- The verifier-to-drainer monetized-pin channel is now bounded
  (FIRST_AUDIT_INGRESS_CAPACITY = 1024); producers try_send and drop on
  full. Every stage of the first-audit pipeline is now capacity-limited:
  ingress queue, pending LRU, and launch-rate token bucket. A dropped
  nomination is penalty-free and the peer stays covered by the lottery.
- The A1 answerability screen runs again after the 0-30s launch jitter,
  so a pin can never be challenged outside its window regardless of how
  deferral time, jitter, and the skew margin compose. No false-conviction
  window remains by construction.
- ADR-0004 Amendment 2 documents the accepted residuals: the
  budget-exhaustion starvation economics (bounded by real settled payment
  costs, lottery backstop) and the PRE-EXISTING vault-level
  completedPayments overwrite race (already defeats the amount check with
  a 1-wei payment on all node versions; needs a contract-side fix,
  tracked as follow-up).
Anti-starvation lane for the launch budget: passes alternate between
newest-first (freshest answerability windows) and oldest-first, so a
stream of fresh settled decoy nominations cannot keep an aging pin from
launching before its eligibility window closes. Suppressing a pin now
additionally requires pre-aged pending decoys at every observer, which
the oldest lane itself drains and the per-peer re-audit window blocks
from refreshing.

Also restate the ADR-0004 Amendment 2 starvation residual with accurate
economics (decoy settlements hit a whole storer cohort at once, merkle
settlements nominate every paid index, principal recycles through sybil
reward addresses, and sidecar-only pins have no lottery backstop) and
correct over-claimed lottery-coverage comments: a dropped nomination is
re-covered by the peer's next settled payment, the lottery covers only
gossiped commitments.
… per pass

The alternating newest/oldest lane flipped on every drainer pass, but most
passes spend no token (the bucket is empty between refills) and a pass is
triggered by incoming nominations as well as by the retry tick. An attacker
could therefore inject a nomination to force a barren scan, flip lane parity,
and keep every token-bearing pass on the newest lane — starving an aging pin
with nothing but fresh decoy settlements, no pre-aged decoys required.

The lane now advances only when a launch token is actually spent, so
consecutive LAUNCHES strictly alternate whatever the scan pattern in between,
and the oldest lane cannot be skipped. Adds a regression test modelling the
barren-scan attack, and restates the Amendment 2 residual: the remaining
suppression routes (pre-aged decoys at every observer, or evicting the target
from the 4096-entry pending LRU) both cost real settled payments at scale.
… per pass

The lane still advanced once per launching pass, so a full burst spent both
tokens in the same lane: the launch sequence could be newest, newest, oldest
instead of a strict alternation, and the anti-starvation argument did not
hold for burst passes.

The launch loop now pops candidates from a deque — front for the newest lane,
back for the oldest — and flips the lane at every committed launch, so
consecutive launches strictly alternate including within one burst pass.
Deferred entries carry their snapshot index and are re-inserted oldest-first,
restoring LRU recency regardless of which end they were popped from.

The regression test now models the real loop (deque + multi-token passes) and
rejects both wrong flip rules: per pass (attacker steers parity with barren
scans) and once per launching pass (a burst keeps one lane).

ADR-0004 Amendment 2 restated accordingly, and the residual no longer claims
suppression cannot be indefinite: evicting a target from an observer's
4096-entry pending LRU permanently drops that nomination, after which
coverage depends on the peer's next settled payment (or the ADR-0002 lottery
for a gossiped commitment).
…d telemetry

Prod-readiness review follow-ups (both minor/nit, no behavior change):

- Count nominations dropped at the bounded ingress channel when it is full
  (Full only, not Closed) in a process-global counter, surfaced as
  ingress_dropped in the 5-minute scheduler summary. A non-zero value is the
  rollout signal that nomination ingress is saturating — benign but the thing
  to watch when widening the rollout.
- Document that launched counts SCHEDULED audits (token spent); a pin that
  ages out at the post-jitter answerability re-screen is counted under
  outside_answerability_window and sends nothing, so actual wire challenges
  are launched minus post-jitter aborts, reconciled by the terminal counters.
@dirvine

dirvine commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Review-team summary — changes requested

Reviewed exact head 80769409a86a938f535c5eaeef9d88bd5fb38b09 with six independent lenses (five completed; one adversarial pass timed out), plus a local code/test pass. The paid-only targeting, settlement-address binding, token bucket, in-flight cap and bounded ingress are sound. CI is fully green, and I reproduced 697/697 library tests plus the focused payment/replication suites.

Blocking: post-jitter abort is committed as an audit

At src/replication/mod.rs:1393-1416, the scheduler consumes a token, stamps the two-hour per-peer window, inserts the pin into first_audited, flips the lane and increments launched. The child then sleeps for jitter and may fail the answerability re-screen at :1434-1457, returning without sending a challenge.

That means a quote near the eligibility boundary can obtain the full suppression state despite no audit occurring: the pin is treated as audited and a same-count successor is dropped for two hours. This is introduced by this PR's jitter/window composition and conflicts with the Amendment 2 wording that only audits which actually launch have consequences.

Please commit the first_audited/peer-window state only after the post-jitter screen, immediately before the wire challenge, and add a paused-time regression proving a post-jitter abort neither marks the pin audited nor suppresses the next nomination.

Follow-ups / rollout caveats (not classified as regressions in this PR)

  • Backdated signed quotes can already pass payment verification and be discarded by the pre-launch answerability screen; that behaviour exists on main and needs a separate security decision.
  • Newest-per-peer coalescing can already replace a pending high-count pin with a later lower-count pin; also pre-existing, but worth a focused ADR follow-up because sidecar-only pins have no lottery backstop.
  • The new A/B workload is opt-in/informational, not a CI gate. The PR's own fleet-scale matched staging comparison remains necessary before production widening.
  • Scheduler counters are cumulative; rollout dashboards should use restart-aware deltas. rate_deferred_attempts counts repeated assessments, and pre-/post-jitter expiry currently share one counter.

Verdict: fix the post-jitter state-ordering issue, then re-review for staging. No merge or approval performed.

grumbach added 2 commits July 15, 2026 13:26
…ppression

Blocking review finding (dirvine): the drainer committed durable suppression
(first_audited + the 2h per-peer window + token + launched) at LAUNCH time,
then the child slept for jitter and re-checked answerability; a quote near the
answerability boundary could obtain full 'already audited' suppression and drop
a same-count successor for two hours despite no audit ever sending.

Reworked to a drainer-owned reserve/promote/cancel scheduler (consensus design
with codex xhigh over four rounds):

- Payments NOMINATE; the drainer holds at most ONE reservation at a time
  (preserving per-launch lane alternation), consuming a token and an in-flight
  slot but stamping NO suppression.
- A jitter timer (select arm) fires at the reservation's ready_at; the
  AUTHORITATIVE answerability + shared-cooldown check-and-stamp runs then, under
  the cooldown write lock, immediately before the send. Only on that pass are
  first_audited/recent stamped, the lane flipped, and launched counted.
- A cancelled reservation (answerability lapsed during jitter, or a gossip
  audit won the cooldown) refunds the token, releases the slot, and leaves NO
  durable state — nothing to roll back. A cooldown-race requeues the event.
- A same-peer successor arriving during a reservation is retained (window
  bypass for the reserved peer), never dropped, and becomes the next
  reservation if the first cancels.
- B horizon prefilter: a nomination is only admitted if it stays answerable
  through now + max_jitter + slack, so a committed reservation can still be
  challenged at send; the authoritative check remains at promotion so A1
  holds regardless of jitter-vs-window sizing.

All scheduler state mutation stays single-threaded in the drainer; the spawned
child only runs the audit I/O and holds the moved-in in-flight slot.

Tests: horizon-prefilter boundary; answerability-cancel is state-neutral and
retains a same-count successor (the reviewer's exact hole); per-launch lane
alternation driven through reserve/resolve. 699 lib tests + cfd green; e2e
smoke on a live anvil testnet exercises reserve->promote end to end.
…ination

Codex review: resolving a reservation that loses the shared-cooldown race
requeued the reserved event unconditionally, which could overwrite a newer
same-peer successor (a count jump) already pending, losing coverage. Requeue
only when no same-peer entry is pending; the successor arrived later so it is
the newer nomination. Adds a regression test.
@grumbach grumbach requested a review from dirvine July 15, 2026 04:38
…granularity in test

- pending_len/tokens are used only by the scheduler summary log and unit
  tests, both absent from cargo build --release --no-default-features, so they
  tripped dead_code under RUSTFLAGS=-D warnings. Gate the lint the same way the
  crate already gates logging-only unused items.
- first_audit_horizon_prefilter_boundary used a 1ns epsilon; Windows SystemTime
  has 100ns (FILETIME) granularity, so the nanosecond step rounded to the same
  instant and the boundary assertion failed there. Use 1µs, exact on every
  platform.
@grumbach grumbach marked this pull request as ready for review July 15, 2026 05:25
Copilot AI review requested due to automatic review settings July 15, 2026 05:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR mitigates the ADR-0004 v0.14.3 production regression by bounding monetized “first-audit” scheduling pressure (rate/inflight/jitter + per-peer re-audit window), narrowing deterministic nomination to paid candidates only, and tightening payment verification to reject settlement-redirected payments. It also updates ADR-0004 with Amendment 2 and adds an env-gated E2E A/B workload driver for measuring the change.

Changes:

  • Replace the deterministic first-audit drainer with a bounded scheduler (token bucket + inflight cap + jitter + per-peer re-audit window + observability counters).
  • Restrict first-audit nominations to settlement-verified paid candidates (single-node median and merkle paid indices), and make nomination ingress bounded with drop accounting.
  • Bind “paid” verification to the recorded rewards-address prefix in the on-chain settlement; document the semantics in ADR-0004 Amendment 2 and add an E2E A/B driver.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/e2e/mod.rs Registers the new env-gated A/B workload module in the E2E test harness.
tests/e2e/first_audit_ab.rs Adds an env-gated E2E A/B workload driver to measure first-audit launch/failure rates under a production-shaped paid-upload workload.
src/replication/mod.rs Implements the bounded first-audit scheduler (reserve/promote/cancel), bounded ingress drop tracking, and expanded summary observability.
src/replication/config.rs Introduces constants for first-audit budget, inflight cap, jitter, re-audit window, count-jump override, and bounded ingress capacity.
src/payment/verifier.rs Nominates only paid candidates for first-audits, switches to bounded nomination channel with drop accounting, and rejects redirected settlements via rewards-address binding.
docs/adr/ADR-0004-commitment-bound-quote-pricing.md Documents Amendment 2: bounded best-effort first audits, paid-only nomination, and the new coverage/restated guarantees.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/e2e/first_audit_ab.rs

@dirvine dirvine left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — changes requested

Reviewed exact head acb36302f1b122e5d76c62c787600f688b4bbb1a against main, including the incremental fix from 80769409a8.

The original blocker is fixed: reservation now consumes only a token/in-flight slot, post-jitter cancellation refunds/releases without stamping recent, first_audited, shared cooldown, lane state or launched, and durable suppression is committed only after the authoritative promotion checks (src/replication/mod.rs:601-673). The focused cancellation/successor tests exercise that state transition.

Blocking: a lower-count same-peer payment can erase the inflated pin before audit

queue_first_audit_event still replaces pending work solely by peer and arrival order (src/replication/mod.rs:168-180). FirstAuditScheduler::enqueue applies that replacement without comparing key_count or the security coverage of the two pins (:483-512). With the new five-minute launch budget and serial reservation, this is now a practical deterministic suppression window:

  1. monetise an inflated sidecar-only commitment;
  2. before it is reserved, monetise a lower-count commitment from the same peer;
  3. the lower-count event replaces the inflated pin, which is forgotten without audit;
  4. the count-jump override never sees the displaced high count, and sidecar-only pins have no gossip-lottery backstop.

The reservation/cooldown-race path has the related form: resolve discards the reserved event whenever any same-peer successor exists (:651-665). “Newer” does not mean it subsumes the older pin's security coverage.

This route needs only one additional same-peer settlement, whereas Amendment 2 documents the accepted permanent-loss routes as pre-aged decoys or 4,096 distinct-peer nominations. Please define a security-aware coalescing rule (at minimum, a lower-count successor must not displace a higher-count monetised pin), apply it to both pending replacement and cooldown-race requeue, and add regressions for:

  • pending high count → later lower count;
  • reserved high count → later lower-count successor → cooldown-race cancellation;
  • the selected event retaining the intended pin/count and remaining launchable.

Non-blocking adversarial/rollout notes

  • Tied median quotes: single-node verification stops at the first settlement-valid median candidate (src/payment/verifier.rs:843-851), so an arbitrary client that settles multiple tied candidates causes only one paid pin to be nominated. The standard client pays one, but the implementation/ADR should state this limitation or collect all settled tied candidates if “all paid issuers” is intended. The Merkle path correctly nominates all contract-paid indices.
  • Promotion stamps the two-hour window before the audit outcome. A deliberate Transient/silence reaches the fully-graced timeout lane and is not retried by this scheduler. This appears consistent with “launched audit” semantics, but it materially weakens first-audit coverage and should be measured in the isolated staging run.
  • A cooldown-race requeue can evict another entry from a full pending LRU, but resolve ignores FirstAuditQueueOutcome; that eviction is absent from capacity_evicted, despite the ADR saying capacity loss is observable.
  • Existing, not introduced here: issuer-controlled stale/future quote timestamps can bypass sidecar-only first audits; and unauthenticated non-median legacy quotes are still passed through mismatch cross-checking. Keep QUOTE_COMMITMENT_MISMATCH_TRUST_ENABLED off until the latter is fixed.
  • The A/B workload is opt-in/informational. The matched testnet comparison remains required before rollout.

Verification

  • cargo test --lib --no-fail-fast700 passed
  • cargo test --features test-utils payment::verifier::tests --lib --no-fail-fast97 passed
  • cargo test --features test-utils replication::tests::first_audit --lib --no-fail-fast13 passed
  • cargo fmt --all -- --check — clean
  • cargo clippy --all-features -- -D clippy::panic -D clippy::unwrap_used -D clippy::expect_used — clean
  • cargo check --no-default-features — clean
  • A/B test target with test-utils — compiles; workload skipped without FIRST_AUDIT_AB=1
  • GitHub CI — all 12 checks green on this head

Review-team reconciliation: two independent detailed reviewers agreed the old blocker is fixed and independently identified the high→low same-peer coalescing bypass; one also raised tied-settlement and timeout-suppression concerns. One runtime/observability reviewer timed out. Two local-model passes produced no additional independently verifiable blocker. The requested GLM-5.2 seat could not run because the configured Z.AI account has no remaining balance.

No merge performed.

grumbach added 3 commits July 15, 2026 15:26
…cannot erase an inflated one

Re-review blocker (dirvine): per-peer pending coalescing replaced same-peer
work by arrival order only, so a peer could suppress an inflated (audit-worthy)
sidecar-only commitment for the cost of one cheaper same-peer settlement — the
count-jump override only compares against the last AUDITED count and sidecar-
only pins have no gossip-lottery backstop.

Coalescing is now highest-count-per-peer (newest on an equal-count tie): a
strictly-lower-count nomination never displaces a higher-count pending pin, and
a suppressed lower nomination leaves the retained pin's LRU recency untouched
(so low-count floods cannot steer the lane). The same rule governs the
cooldown-race requeue of a reserved event (incoming_is_newer=false, so a
higher-count reserved event beats a lower successor), which now also counts a
capacity eviction in the funnel (previously unobserved).

ADR-0004 Amendment 2 restated (highest-count coalescing + accepted residual:
a single per-peer slot may drop the lower fallback if the retained higher pin
later ages out). Non-blocking notes documented: single-node tied-median
nominates only the first settled candidate (sidecar-only extras are best-
effort residual; gossiped extras keep the lottery); the re-audit window is
stamped at launch not outcome (Transient/silence is not auto-relaunched) — both
to be quantified in the matched staging run.

Tests: coalescing keeps highest count + exposes eviction; pending high vs later
lower; reserved high vs cooldown-race lower successor. 702 lib tests + cfd
green.
…d edge tests

Review nits (both non-blocking): add a dedicated suppressed_lower funnel
counter so a burst of attempted cheaper-pin self-erasure (a lower-count
same-peer nomination dropped so a higher-count pin survives) is visible in the
scheduler summary, on both the enqueue and cooldown-race requeue paths; add
edge tests for (a) a suppressed-lower flood leaving the retained pin's LRU
recency/lane position unchanged (peek, no push) and (b) the cooldown-race
requeue counting a different-peer capacity eviction. ADR-0004 funnel list
updated. 704 lib tests + cfd green.
…he funnel

Codex review: SuppressedLower was returned for both a strictly-lower incoming
(the attack signal) and an equal-count freshness tie, so the requeue counted a
benign tie as an attempted suppression. Add a distinct RetainedOnTie outcome;
only a strictly-lower drop increments suppressed_lower. Assert the counter
through the enqueue path in the pending-lower test.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@dirvine dirvine left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the three commits after my requested-changes review at exact head 413d18ae90726b0a534fce0510a4e5bc1854139a.

The blocker is addressed:

  • pending coalescing now retains the highest committed count per peer;
  • a lower-count successor cannot erase the higher-count pin;
  • cooldown-race requeue uses the same security-aware ordering;
  • capacity eviction and suppressed-lower outcomes are observable;
  • the ADR now records the rule and accepted residuals;
  • focused regressions cover pending and reserved high→low cases, tie behaviour, recency and requeue eviction accounting.

Verified on this head:

  • cargo test --lib --no-fail-fast — 704 passed
  • cargo test --features test-utils replication::tests::first_audit --lib --no-fail-fast — 17 passed
  • incremental diff check — clean
  • GitHub CI — all 12 checks green
  • unresolved review threads — 0 (the ThreadRng comment was verified as a false positive against local/CI compilation and resolved)

No remaining code-review blocker found. The matched isolated testnet remains the next rollout gate; approval is not a merge or deployment.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants