Skip to content

feat(quote): declare the settlement version when requesting quotes - #171

Open
grumbach wants to merge 14 commits into
WithAutonomi:mainfrom
grumbach:settlement-version-quote-gate
Open

feat(quote): declare the settlement version when requesting quotes#171
grumbach wants to merge 14 commits into
WithAutonomi:mainfrom
grumbach:settlement-version-quote-gate

Conversation

@grumbach

@grumbach grumbach commented Aug 13, 2026

Copy link
Copy Markdown
Member

Linear issue

V2-975 — https://linear.app/autonominetwork/issue/V2-975/refuse-to-quote-clients-that-cannot-settle-correctly-instead-of

Risk tier

  • T0 — docs / tooling / CI / pure UX-output. Repo CI only.
  • T1 — client-only, no network-facing behavior change. CI + prod compat smoke.
  • T2 — node/client logic with behavioral surface, no protocol/format/economics change. Dev testnet + ADR.
  • T3 — protocol / storage format / payments / routing. T2 evidence + adversarial testing.

Changes the request shape on the quote path, which is the step before payment.

What this is for

A merkle batch pays on-chain before any storer sees a PUT, and merkle receipts are not refundable, so a client settling under superseded rules has its upload refused after the money is gone. Declaring the settlement version in the quote request moves the refusal to the last point where it costs nothing.

Change

Send QuoteRequestV2 / MerkleCandidateQuoteRequestV2 on both paths, and treat the two possible refusals differently:

  • ClientUpdateRequired is terminal. It aborts before payment and surfaces the storer's own wording, which tells the user to run ant update and that nothing was charged.
  • StorerUpdateRequired is skippable. The storer is the old side; nothing is wrong with this client, so the upload uses other peers and says nothing to the user. If too few remain it fails for lack of quotes, which is correct: it fails before payment rather than after.

Changes since the last review

  • Merged current main (22 commits, through the explicit EVM network selection merge). Git reported no conflicts.
  • Security Audit is green again. It was failing on RUSTSEC-2026-0258 (h2 unbounded empty DATA frames), which is a main problem rather than one this branch introduced; main cleared it in chore(deps): bump h2 0.4.14 -> 0.4.17 to clear RUSTSEC-2026-0258 (V2-1045) #177 by taking h2 0.4.17, and the merge brings that lockfile entry across. Verified locally with the workflow's own cargo audit invocation: no vulnerabilities, 7 allowed warnings.
  • The node-side ADR is now ADR-0013, renumbered because main took 0010 for an unrelated decision while this set was open. Link below updated, and it now points at the fork branch that actually carries the file rather than a WithAutonomi path that 404s.

Three defects found by an independent adversarial review pass, two of them on the payment path. Worth re-reading before this merges, since it moved code rather than comments.

  • The wave payment path did not honour the refusal latch. pay_for_storage refuses to spend once a refusal is corroborated, because the verdict is about this build and not about one operation. prepare_chunk_payment and batch_pay never consulted it, and batch_pay reaches wallet.pay_for_quotes. An upload that latched a refusal did not stop the next one paying by a different route, and the external signer could be handed a prepared chunk, which is telling a user to pay. Now checked at preparation and again immediately before the spend, since waves are pipelined and a chunk can be prepared before a refusal lands. Nothing refuses this build today, because MIN and CURRENT are equal, so this opens on the first settlement bump that makes MIN load-bearing, which is the case the gate exists for.
  • The merkle path took a refusal at face value. The single-node path rejects a ClientUpdateRequired whose echoed version is not ours or whose stated minimum this client already meets, and treats that sender as an ordinary bad peer. The merkle path converted every refusal straight through. Since two distinct peers corroborate and latch for the rest of the run, two faulty or hostile candidates could deny every upload with a refusal about some other client. Both paths now share one validation.
  • A refusal in flight could be cancelled. build_candidate_pools propagated the first pool error with ?, dropping the future set and cancelling every pool still running, so a pool one refusal short of corroboration lost it. Pools are drained before an error is reported now, which is the rule the single-node collector already follows, and a refusal outranks an ordinary failure so an exhausted pool cannot mask another pool refusing this client.

The second and third carry tests that fail if the fix is reverted, verified by reverting each one. The first is not unit-testable at the same level, for the same reason the existing pay_for_storage check is not: constructing a Client needs a live P2PNode. It mirrors that check exactly.

Changes since first review

All four blockers from the two review rounds are addressed.

A refusal now reaches the caller. Both collectors previously flattened it into their per-peer failure list, so the single-node path could still reach quorum from the remaining peers and pay, and the merkle path returned InsufficientPeers with the instruction buried in a diagnostic string.

A refusal no longer depends on who answered first. This was the subtler half, and the first fix missed it. The witnessed collector stopped as soon as it had enough quotes, discarding peers still in flight, and its overall-timeout arm falls through by design so fast peers' quotes stay usable. Either path dropped a refusal that had not arrived yet. Now the collector stops launching at the target but keeps draining what it already launched, and the verdict is recorded in a slot outside the timeout, checked after both collection branches. The launch budget already returns zero once the close group is covered, which is what lets the drain terminate instead of recruiting replacements.

Both downgrade paths are guarded. The compile-time cutover assertion previously sat beside the merkle fallback only, while the independent single-node retry was unguarded and ADR-0013 claimed otherwise. It is now one shared constant referenced from each site, so deleting one path cannot orphan the check for the other.

Structured responses never trigger the legacy retry. Silence is not proof a peer cannot parse a versioned request, so the fallback is a downgrade path; it fires only on transport-level silence, never on a refusal, and the predicate is pinned by test.

Compatibility

  • Wire: nodes should ship first. A node on the current published ant-protocol cannot decode versioned requests. The per-peer fallback makes this survivable rather than fatal, so a client on this build works against a fully legacy fleet, a fully upgraded fleet, or any mix.
  • Storage: none.
  • API: additive. Two new Error variants; the crate's own exhaustive match is updated here.

Semver impact

  • breaking
  • feature
  • fix

Adding Error variants is technically breaking for a downstream matching exhaustively on ant_core::data::Error. Marked feature because the enum is effectively open in practice. Happy to be overruled at review. No version bump is taken in this PR; the release train owns that.

Test evidence

cargo test --lib443 passed, 0 failed. cfd clean across ant-core and ant-cli.

  • an_update_refusal_is_surfaced_with_its_upgrade_instruction — the refusal arrives typed, carrying ant update and "nothing was charged".
  • a_storer_that_is_behind_is_not_reported_as_the_clients_fault — and does not carry an upgrade instruction.
  • a_refusal_aborts_quote_collection_instead_of_counting_as_one_bad_peer — stops collection rather than joining the failure list.
  • a_refusal_is_recorded_where_the_collection_timeout_cannot_discard_it — the ordering fix from round two.
  • a_lagging_storer_does_not_populate_the_refusal_slot — one behind-the-times peer must not abort an upload the close group can serve.
  • meeting_the_target_stops_launching_without_stopping_collection — the launch budget reaches zero, which is what bounds the drain.
  • only_silence_triggers_the_legacy_retry — timeouts and send failures fall back; refusals of either kind do not.

Mixed-version validation, and the regression it caught

The merkle E2E suite spawns a 35-node testnet from the published ant-node, so CI has been running the new-client-against-old-fleet case all along. It passed functionally and failed on cost, blowing the 60-minute cap with 4 of 7 tests done against a 24–38 minute baseline on main.

Cause: a peer that cannot decode a versioned request never answers, so the client waited a full quote timeout before falling back, and paid that on every request. The suite runs quote_timeout_secs = 120 and a merkle pool asks sixteen candidates.

Two bounds, both guarded against becoming downgrades:

  • Remember the answer. A silent peer is asked in the legacy shape from then on. Recorded only on a timeout, never on a send failure (which means the request never arrived and teaches nothing), and never for a peer already seen answering a versioned request, so one lost response cannot pin an upgraded peer to the legacy shape.
  • Cap the probe. VERSIONED_QUOTE_PROBE_CEILING bounds the versioned attempt. Production's 10s timeout is already below it, so it only binds in test configurations.

Cost, measured on one test against a real 35-node devnet plus anvil, same machine and same test each time:

Client behaviour test_attack_merkle_proof_for_wrong_chunk
Remember the answer only (probe waits the full 120s timeout) 295s
plus a 15s probe ceiling 268s
plus a 5s probe ceiling 191s (reverted, see below)

The node-side logs confirm what is being measured: every peer rejects the versioned shape and answers the legacy one, so this is the mixed-version path end to end.

The cost falls with the ceiling, at roughly eight sequential probe rounds per test. At 15s macOS came in under the cap at 42m55s while the slower ubuntu runner was cancelled with 6 of 7 done.

The 5s ceiling was reverted after independent review, and this is the part worth reading. It would have been a correctness defect, not a tuning win. The probe wait is the only window in which a peer can refuse, and the fallback re-asks under a new request id, so a refusal arriving after the ceiling answers a request nobody is listening to: it never counts toward corroboration, never sets the client-wide latch, and the unversioned request it raced can return a quote the client then pays against. With production's 10s quote timeout, every legitimate 5-to-10 second refusal would take that path. Neither the never-demote rule nor the compile-time guard covers it, and the clients at risk are the ones already released.

So the ceiling stays at 15s, and the rule is now written into the constant and ADR-0013: keep it at or above the largest production quote timeout, so it never binds on a production client.

Merkle E2E now passes on both platforms

Latest run on the merged head, against the raised cap main now carries:

Job Result Cap
Merkle E2E (macos-latest) pass, ~55m 90m
Merkle E2E (ubuntu-latest) pass, ~65m 90m

The thin-margin warning this section used to carry is out of date, and main is why. It previously read that ubuntu cleared a 60-minute cap by 1m45s and should be treated as thin. #178 raised the Merkle E2E timeout to 90 minutes for exactly this reason (V2-1046, "60-min cap breached by real runtime"), and merging main brings that in. Ubuntu now finishes with roughly 25 minutes of headroom rather than under two, so runner variance no longer sits on the edge of a cancellation.

The probe cost itself is unchanged and still mine to explain:

  • Pre-existing. The cap was sized for a suite with one fewer test than it now has, and had already been raised 20 → 40 → 60 as it grew, now 90.
  • This PR. Roughly two minutes per test of probe cost, present only because the suite's devnet still speaks the pre-versioned dialect. That is a consequence of the temporary fork-branch protocol pin: once the pin resolves the fleet under test answers versioned requests, there are no probes to pay for, and this contribution disappears entirely.

I deliberately did not buy margin by shortening the probe ceiling, for the reason above. That trade is no longer needed either way, now the cap is 90.

Still outstanding: old client against an upgraded node, and a genuinely mixed fleet. Both need a testnet built from the ant-node branch, which does not exist until the set lands. Structured refusal, lost refusal and send failure are pinned at unit level only.

New dependency

none. ant-protocol is temporarily repointed at the review branch for WithAutonomi/ant-protocol#23, and reverts to a published version pin once that merges and the release train publishes it. That repin is a merge-order dependency, not something this PR can take.

ADR

ADR-0013: Settlement version and pre-payment compatibility (Proposed), on the ant-node branch.

https://github.com/grumbach/ant-node/blob/settlement-version-quote-gate/docs/adr/ADR-0013-settlement-version-and-pre-payment-compatibility.md

Release readiness: NOT production ready

Merging puts this in the next release, so the bar is fleet-ready rather than code-complete. CI being green is evidence for the code gate only. Do not merge while any row below is open.

Gate Status What closes it
Code / CI Proven. 448 lib tests, merkle E2E 7/7 on both platforms, three Codex xhigh rounds adjudicated
Dependency Open. ant-core/Cargo.toml pins a mutable fork branch, and the lockfile carries both registry and git copies of ant-protocol ant-protocol #23 merged and published, then repin + regenerate lockfile
Mixed-version Partial. New-client/old-fleet is proven, and is what the E2E actually runs. The gate itself has never been exercised over a real connection, because the devnet speaks the pre-versioned dialect A devnet built from the ant-node branch
Deployment ordering Open, and this one has live user impact — see below Node fleet upgraded before this client is released
Observability Open. Adoption is measured by a node-side counter never read in production Canary evidence
NAT / canary Open. No canary. The added first-contact probe lands hardest on relayed and NAT'd peers Canary with quote latency observed
Rollback Argued, not rehearsed. Reverting restores the legacy request shape exactly A stated revert order
Fleet safety Open. The corroboration quorum and latch have never met a real fleet Canary evidence that no honest upload is halted

The deployment-ordering gate is not theoretical

Unlike the rest of this change, the client behaviour is not inert. Against a fleet that cannot answer a versioned request, every first contact with a peer costs a probe wait before the fallback, bounded by the peer timeout (10s in production, since the 15s ceiling does not bind there).

The fleet is currently entirely in that state. A merkle upload performs on the order of eight sequential quote rounds, so shipping this client before the nodes could add roughly a minute-plus to a cold upload, decaying to zero as the fleet upgrades and per-peer capability is learned. The merkle E2E measured the same effect: ~2 min/test against a legacy devnet.

That is a real user-facing latency regression if the order is reversed, and it is the strongest reason this must not ship ahead of the node rollout.

What is genuinely low risk today

The gate is inert on arrival. MIN_SUPPORTED_SETTLEMENT_VERSION and CURRENT_SETTLEMENT_VERSION are both the first declarable version, so no client in existence can be refused for being too old and no node can be refused for being behind. The refusal machinery, the corroboration quorum and the latch cannot fire until a future settlement bump.

That is a reason the risk is low. It is not evidence the gates are closed, and it does not make the set fleet-ready.

Mitigation / rollback

Revert. Until nodes enforce a minimum above the first declarable version, no storer can refuse any client on these grounds, so the observable change here is the request shape plus one extra timeout against legacy peers. Reverting restores the legacy request shape exactly.

Send the versioned quote requests on both the single-node and merkle paths,
so a storer can refuse a client that cannot settle correctly before that
client pays. A merkle batch settles on-chain before any storer sees a PUT,
and merkle receipts are not refundable, so a refusal at PUT time refuses
money that is already gone. A refusal at quote time costs nothing: no quote
means no pool commitment, which means no payment.

A storer that predates the versioned request cannot decode it and never
answers, so each peer falls back once to the legacy request shape. The
fallback is keyed on transport-level silence only. A storer that returned a
structured refusal has understood the request, and retrying that without the
version would talk it into quoting a client that cannot pay, which is
exactly the failure being removed. Both paths share one response mapper so
the two request shapes cannot be interpreted differently, and the fallback
can be deleted once the fleet answers versioned requests.

ClientUpdateRequired is lifted out of the generic protocol error into its own
terminal variant. It is a verdict about this build rather than about one
peer, it carries wording aimed at the person running the upload, and folding
it into per-peer quote failures would bury the upgrade instruction. It
classifies as an application error so it cannot push the adaptive limiter
down: the link is healthy and no retry rate clears it.

Pins ant-protocol to the branch carrying the wire types while
WithAutonomi/ant-protocol#23 is in review.

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed exact head 64ca62425dc1fdb791260d8dd56b3a0cac47a23a with the protocol and node PRs.

Two coupled blockers:

  1. is_version_unaware treats every Error::Network and Error::Timeout as proof that the peer cannot decode V2 (quote.rs:451-455, merkle.rs:203-212). It is not proof: Network includes failure to send, and Timeout also covers packet loss, a lost structured refusal, overload, event lag, or a malicious peer dropping V2. Retrying the same peer with an unversioned request then bypasses the quote gate. This is harmless only while CURRENT == MIN; at the next settlement cutover it can again obtain a quote and burn payment. Capability negotiation, a rollout cutover that removes fallback before raising MIN, or another downgrade-resistant signal is needed.

  2. ClientUpdateRequired is described as terminal and user-facing, but both collectors flatten it into ordinary peer failures (quote.rs:478-484; merkle.rs:1264-1276). The single-node path can still succeed using other/legacy quotes, while the merkle path ultimately returns InsufficientPeers with the refusal embedded in a diagnostic string. The typed upgrade error therefore does not reliably reach the caller and does not abort before payment when any peer has explicitly declared this client incompatible. Please propagate the typed refusal immediately (or establish and test an explicit quorum policy that is safe to pay).

The predicate unit test passes locally, but it only pins the unsafe classification; it does not exercise the request/response fallback. Please add integration tests for legacy silence, upgraded success, structured refusal, lost refusal/timeout and send failure.

Local focused test passed. CI unit/build checks pass, but all four E2E jobs are still pending. The manifest's contributor-branch pin must also be replaced by the published protocol crate before merge.

Addresses both coupled blockers from review.

The typed refusal never reached the caller. Both collectors folded it into
their per-peer failure list, so the single-node path could still reach a
quorum from the remaining peers and pay, and the merkle path returned
InsufficientPeers with the upgrade instruction buried in a diagnostic
string. A storer saying this client cannot settle is not one bad peer among
many: no number of further quotes makes paying safe. Both collectors now
return it immediately, which aborts before payment and puts the storer's own
wording in front of the user. record_store_quote_result becomes fallible to
carry that, and both call sites already propagate.

The legacy retry remains a downgrade path. Silence is not proof a peer
cannot parse a versioned request: a dropped response, packet loss, an
overloaded peer, or one deliberately discarding versioned requests are
indistinguishable from here, so the retry can be provoked. It is harmless
only while no client can be refused on version grounds. Rather than leave
that as a comment, a compile-time assertion fails the build if
MIN_SUPPORTED_SETTLEMENT_VERSION is raised while the fallback still exists,
so the cutover cannot be forgotten.

Handle the new StorerUpdateRequired as a skippable peer rather than a client
fault. It must not abort, or one lagging member of a close group would fail
an upload the rest could serve, and it must not be retried unversioned,
because that peer understood the request and quoting it would produce a
payment it cannot verify.
Keeps the lockfile at the commit CI resolves for the branch pin. No source
change; picks up the ruint advisory bump made on the protocol branch.

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Follow-up review of exact head 08e419771317928c14a57ed56936eb42e8db8e1f across the coordinated protocol/node/client set.

The headline fixes are real: typed ClientUpdateRequired now propagates when observed; StorerUpdateRequired is skippable; Merkle collection drains responses and aborts on a client refusal; structured responses never trigger legacy retry. The three focused tests passed locally.

Two single-node gaps remain:

  1. collect_store_quotes_from_remote_peers stops the witnessed loop as soon as quotes.len() >= target_quote_count (ant-core/src/data/client/quote.rs:1273) and its overall-timeout arm falls through (:1296-1305). Any slower in-flight ClientUpdateRequired is then dropped; if enough quotes have arrived, :1405 proceeds with the quote plan and payment. The new unit test exercises record_store_quote_result directly, so it does not cover this collector-level race. Please make the terminal policy deterministic for all launched peers, or document/test a different safe quorum rule.

  2. The compile-time MIN_SUPPORTED_SETTLEMENT_VERSION == 1 retirement guard exists only in merkle.rs:231, while the independent single-node legacy retry remains in quote.rs:378-413. ADR-0010 claims the unversioned retry is build-enforced before MIN can rise, but only one of two downgrade paths is guarded. Please put both paths behind one shared guard (or add the equivalent assertion beside the quote path) and test it.

Release gates also remain: replace the personal-fork branch pin with published ant-protocol 2.4.0 (which also removes the dual 2.3.2/2.4.0 lockfile copies), complete the pending CI/E2E jobs, and provide the mixed-fleet validation recorded as outstanding in ADR-0010.

Panel dissent: one maintainability/operations reviewer considered the staged early exit non-blocking because MIN is currently 1. Four independent code/state-machine reviewers considered the single-node asymmetries real invariant gaps. I agree they should be closed before this T3 change merges.

Versioning is the release train's call, so the comment now points at 'a
published version pin' rather than naming one that has not been decided.
Lockfile follows the protocol branch, which no longer carries a bump.
Closes the two single-node gaps raised in follow-up review.

The witnessed collector stopped as soon as it had enough quotes, discarding
peers still in flight, and its overall-timeout arm fell through by design so
that quotes from fast peers stay usable. Either path dropped a refusal that
had not arrived yet, and the upload then went on to pay. That made a verdict
about whether this client can settle at all depend on which peers answered
first.

The collector now stops launching new peers at the target but keeps draining
those already launched, so every peer it asked gets to be heard. The launch
budget already returns zero once the close group is covered, which is what
lets the drain terminate rather than recruiting replacements. Surplus quotes
are discarded; a refusal among them is not.

The refusal is also recorded in a slot outside the timeout and checked after
both collection branches, so the elapsed arm can no longer discard it. A
storer being behind (StorerUpdateRequired) deliberately does not populate
that slot: it is not a verdict about this client, and treating it as one
would abort uploads the rest of the close group could serve.

The compile-time cutover guard moves to a single shared constant referenced
from both fallback sites. It previously existed only beside the merkle path
while the independent single-node retry was unguarded, so ADR-0010's claim
that the downgrade path is build-enforced held for only one of the two.
…holes

CI ran the mixed-version case for real and it failed on cost. ant-client's
merkle E2E spawns a 35-node testnet from the published ant-node, which
cannot decode a versioned request and so never answers. The client waited a
full quote timeout before falling back, on every request rather than once
per peer, and the suite went from a 24-38 minute baseline to exceeding the
60-minute CI cap with 4 of 7 tests done.

Two bounds fix that. A peer that stays silent is remembered and asked in the
legacy shape from then on, and the versioned attempt is capped by
VERSIONED_QUOTE_PROBE_CEILING because a capability probe does not need the
patience of a real quote. Production's 10s timeout is already below that
ceiling, so it only binds in test configurations.

Neither bound may be allowed to become a downgrade. Only a timeout records a
peer: a send failure means the request never arrived and teaches nothing,
and caching it would strand a peer over one flaky send. A peer that has ever
answered a versioned request is never demoted, so a single lost response
cannot pin an upgraded peer to the legacy shape for the session.

The cutover guard now bounds CURRENT as well as MIN. Raising CURRENT alone
creates the node-behind refusal, and the unversioned retry routes around
that just as it would route around a raised MIN; guarding only MIN left it
open.

A refusal in a later merkle sub-batch is no longer folded into a partial
success. Batches above MAX_LEAVES settle sequentially, so sub-batch two's
refusal arrived after sub-batch one had paid and was being reported as Ok,
hiding the upgrade instruction and leaving the caller to rediscover it.

Also corrects two comments that claimed more than the code delivers: any
recognised response blocks the retry, but a reply carrying an unrecognised
body still ends in a timeout and takes the fallback.
Second review pass found the refusal was simultaneously too weak and too
strong.

Too weak, because it lived in one collector's local state. A refusal
observed by one in-flight upload said nothing to another that was about to
submit a payment, and merkle payments cannot be undone. The verdict is about
this build, not this upload, so it is now held on the client and every
payment entry point checks it before spending.

Too strong, because nothing authenticates a refusal. One hostile or
misconfigured peer answering ClientUpdateRequired to every query would have
aborted every upload, turning an over-query design that tolerates many bad
peers into one that tolerates none. A refusal is believed only once
SETTLEMENT_REFUSAL_QUORUM distinct peers agree, and one that does not
describe this client, wrong echoed version or a minimum this client already
meets, is discarded as a bad peer instead of counted. A genuine
incompatibility clears the threshold at once because every enforcing peer
refuses.

Because the verdict now survives the call, the merkle multi-batch path no
longer fails after earlier sub-batches have paid. The caller writes the
receipt cache only on the success path, so returning an error there
discarded proofs for money already settled on-chain, which is exactly the
destruction this work exists to prevent. It returns those proofs and lets
the latch stop the next payment instead.

Two capability-cache defects also fixed. The legacy and capable sets are
updated under separate locks, so a slow probe could insert into the legacy
set after a concurrent request had already proved the peer capable;
capability now wins when both hold an entry. And capability is recorded on
any recognised answer rather than only a successful quote, since a
structured error proves the peer parsed the versioned shape just as well.

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed exact head 0f925789edd02b4fea5fe9a499b47fc86a6d1b8b. The two prior client blockers are resolved: collectors stop launching but drain already-launched peers, refusals are recorded outside the timeout arm, and one shared compile-time guard is referenced by both the single-node and Merkle legacy fallbacks. The capability cache gives known V2 support precedence over raced timeout evidence; corroborated refusals are coherent, distinct-peer, sticky and checked at all three payment entry points. Local ant-core library tests passed 448/448 and formatting passed.\n\nOne merge blocker remains: ant-core/Cargo.toml still pins the mutable personal-fork protocol branch. The lockfile consequently contains both the registry and git copies of ant-protocol 2.3.2; the optional devnet path uses the old registry dialect and therefore does not exercise the V2 gate. Please merge/release ant-protocol #23, repin to the published registry version, and regenerate the lockfile. CI is also not complete: Ubuntu Merkle E2E was cancelled at the 60-minute cap after six passing tests, while the final E2E job remains in progress; macOS Merkle E2E passed all seven.

A shorter ceiling was tried, 15s to 5s, to bring the slower CI runner under
its 60-minute job cap. Independent review showed it would be a defect, and
the reasoning that made it look safe was wrong.

The probe wait is the only window in which a peer can refuse. Abandoning it
early does not merely mislabel a slow peer as legacy: the fallback re-asks
under a new request id, so a refusal arriving after the ceiling answers a
request nobody is listening to. It never counts toward corroboration, never
sets the client-wide latch, and the unversioned request it raced can return
a quote the client then pays against. At 5s, with production's 10s quote
timeout, every legitimate 5-to-10 second refusal would take that path.

Neither existing safeguard covers it. The never-demote rule only stops a
peer being cached as legacy after it has answered once; it does not stop the
request in flight from falling back. The compile-time guard binds future
builds, while the clients at risk are the ones already released.

So the ceiling stays at 15s and the rule is now written down: keep it at or
above the largest production quote timeout, so it never binds on a
production client and nothing real is truncated. It exists only to bound
configurations that set a timeout far above any real answer time, which in
practice means test harnesses.

Also corrects three claims the code does not deliver. The probe is not a
cheap parse check, it runs the peer's whole quote handler and abandoning it
does not cancel that work. Concurrent first contacts are not single-flighted,
so a peer can be probed by a few in-flight requests before any records the
answer, measured at about two per peer. And the cache guarantees later
rounds do not re-probe, not that a probe is paid exactly once.

The remaining suite cost is an artifact of the temporary fork-branch
protocol pin: once the devnet under test can answer a versioned request
there are no probes to pay for.

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed exact head 7080fa3c277959046369deeb1790ce0d633a1c4d. The incremental diff changes comments only; the previously reviewed implementation and 15-second ceiling are unchanged. Local formatting and ant-core library tests pass 448/448. All GitHub checks now pass, including both Merkle E2Es, so the prior CI blocker is closed. No executable client-code blocker remains.

One merge blocker remains unchanged: ant-core/Cargo.toml still uses the mutable personal-fork protocol branch, and Cargo.lock still contains separate registry and git copies of ant-protocol 2.3.2. This leaves the optional devnet node on the old wire dialect. Please merge/publish protocol #23, repin to the registry release and regenerate the lockfile before final sign-off.

Non-blocking documentation correction: the new-request-ID explanation applies to the Merkle fallback; the single-node fallback reuses its request ID. The stated safety conclusion still holds, but the prose should scope that mechanism accurately.

The probe-ceiling doc claimed the legacy fallback reissues under a new
request id. That holds for the merkle path, which allocates one via
next_request_id, but the single-node path reuses the original id.

Both still fail to observe a refusal that arrives after the ceiling, by
different mechanisms: merkle discards it on the id mismatch, while the
single-node retry has already dropped the await that would have matched
it and only sees a late refusal if it lands after the retry resubscribes.
The safety conclusion is unchanged.

Comment-only, no behaviour change.

@dirvine dirvine left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

APPROVE — reviewed exact head 8a2b3e52c4d3d4d90bcf1b2ef33030e3b23e4edb.

No material blockers found. The client selects the versioned quote path safely, corroborates client-too-old refusals, and treats node-too-old peers distinctly without enabling downgrade overpayment. Local verification: formatting and all 448 ant-core library tests passed.

The audit failure is inherited (h2, RUSTSEC-2026-0258); the macOS transport setup failure and cancelled long E2E job are not failures in this change. Merge/release sequencing still requires ant-protocol#25 to be merged and published first. Release version bumps remain with the release owner.

Picks up the Merkle E2E timeout raise from WithAutonomi#178 (60 -> 90 min), which is
what killed the ubuntu leg of this branch's last CI run at 60m18s with no
test failure. Also brings in the beta upgrade channel work from WithAutonomi#173.

No conflicts; the branch's own diff against main is byte-for-byte
unchanged by this merge.
ant-protocol#23 was reverted in ant-protocol#24, and ant-protocol#25
re-applies it. WithAutonomi#25 is the pull request this pin is waiting on, so the
comment should name it. Comment only; the pin itself is unchanged.
`pay_for_storage` refuses to spend once a refusal has been corroborated,
because the verdict is about this build rather than about one operation.
The wave path did not. `prepare_chunk_payment` and `batch_pay` never
consulted the latch, and `batch_pay` reaches `wallet.pay_for_quotes`, so
an upload that latched a refusal did not stop the next one paying through
a different path. A wave upload whose close group happened to answer, or
an external signer handed a prepared chunk, would still spend.

Nothing refuses this build today, since the minimum and current settlement
versions are equal, so there is no behaviour to observe yet. The hole
opens on the first settlement bump that makes the minimum load-bearing,
which is the case the gate exists for.

Checked in both places. At preparation, because a prepared chunk is what
the external signer is given and handing one out is telling a user to pay.
Again immediately before the spend, because waves are pipelined and chunks
quoted for the next wave can be prepared before a refusal lands.
Two gaps on the merkle path, both about refusals that the single-node path
already handles correctly.

A refusal was taken at face value. The single-node path checks that a
peer's `ClientUpdateRequired` actually describes this client, rejecting one
whose echoed version is not ours or whose stated minimum this client
already meets, and treats the sender as an ordinary bad peer. The merkle
path converted every refusal straight through. Because two distinct peers
corroborate a refusal and latch it for the rest of the run, two faulty or
hostile candidates could deny every upload with a refusal about some other
client entirely. Both paths now share one validation.

A refusal in flight could be cancelled. `build_candidate_pools` propagated
the first pool error with `?`, which drops the future set and cancels every
pool still running. A pool one refusal short of corroboration would lose
it, and the caller can fall back to wave payment. Pools are now drained
before an error is reported, which is the rule the single-node collector
already follows: stop making progress, but never drop a verdict already in
flight. A refusal outranks an ordinary failure when both occur, so a pool
running out of peers cannot mask another pool declaring this client unable
to settle.

Both are covered by tests that fail if the fix is reverted.
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.

2 participants