feat(chunk): declare a settlement version on quote requests - #23
feat(chunk): declare a settlement version on quote requests#23grumbach wants to merge 5 commits into
Conversation
ADR-0008 raised the merkle settlement multiplier to 3x in client code and enforced it in node code, but changed no wire type. Nothing tied the two together, so a client built before that change kept collecting quotes, paid 1x on-chain, and had the upload refused by every storer only after the payment had settled. Merkle receipts are not refundable, so each occurrence destroyed the user's money with no diagnosis beyond a paid/expected mismatch in node logs. Add a settlement version to the quote request so a storer can refuse to quote a client it knows cannot pay correctly, before that client spends anything. It is tracked separately from PROTOCOL_VERSION deliberately: that constant describes what a peer can parse, this one describes how a client turns a signed quote into an on-chain payment, and the two move independently. The change is additive. ChunkQuoteRequestV2 and MerkleCandidateQuoteRequestV2 arrive as appended ChunkMessageBody variants rather than fields on the existing structs, because ChunkMessage is postcard-encoded and postcard is not self-describing: a new field silently changes how every existing peer reads the message, while a new variant is rejected cleanly as an unknown discriminant. ProtocolError gains an appended ClientUpdateRequired variant carrying an upgrade instruction written for an end user rather than an operator, including that nothing was charged. A regression test pins the wire discriminants of the existing message and error variants. The append-only property is what makes this safe to deploy incrementally, and it holds only while declaration order is preserved, so it is now enforced rather than assumed. Enforcement policy lives in ant-node. This crate supplies the vocabulary.
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Reviewed exact head 19845c8c20e1a3505cfbfc446b7e2c26bf5b0726 as part of the coordinated protocol/node/client change.
Blocking concern:
settlement_version_is_supportedaccepts every version greater than or equal toMIN_SUPPORTED_SETTLEMENT_VERSION(src/chunk.rs:69-76). That does not preserve the stated pre-payment safety property for a future settlement change. A v1 node cannot know whether a v2 client will interpret its quote or construct payment in a way the v1 verifier accepts. Serving the quote therefore allows payment before compatibility is established—the failure this mechanism is intended to prevent. Please define an explicit supported range/capability (normallyMIN <= version <= CURRENT) or document and test a mechanism that proves a future-version client cannot burn payment against an older node.
Release gates also remain open: the Security Audit job fails on RUSTSEC-2026-0220 (ruint 1.19.0; fixed in 1.20.0), although that dependency is unchanged from main. Local cargo test --all-targets passed 85/85 tests.
This is T3 wire/payment admission, so I would not merge the coordinated set until the compatibility policy is captured in the superseding ADR and exercised on a mixed-version dev testnet.
Review raised that accepting every version at or above the minimum does not give the pre-payment guarantee this mechanism exists for. A node cannot know whether a client settling under rules it has never seen will construct a payment its verifier accepts, and by the time the verifier says no, the client has settled on-chain and cannot be refunded. The earlier reasoning held only for settlement changes that raise what is paid. ADR-0008's 3x cleared an old node's 1x minimum, so old nodes accepted new clients for free, and that case was mistaken for the general one. A change that redefines the median rule, or which field the contract pays from, breaks it. Replace settlement_version_is_supported with settlement_compatibility, returning Compatible, ClientTooOld or NodeTooOld over an inclusive MIN..=CURRENT range. The two refusals stay distinct on the wire because they need opposite handling. ClientUpdateRequired is terminal and the user must upgrade. StorerUpdateRequired says nothing about the client, which should use a different storer and tell its user nothing. Collapsing them would either tell up-to-date users to upgrade or strand new clients whenever the node fleet lags, and a lagging fleet is the normal state during the client-first rollout ADR-0008 prescribes. StorerUpdateRequired is appended, so existing discriminants are unchanged and the pinning test covers it.
Lockfile only, semver-compatible. The advisory predates this branch, but it fails the Security Audit release gate, so clearing it here avoids blocking the coordinated change on an unrelated pre-existing finding.
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Follow-up review of exact head 439ed83d70ab3bbd14c1031c2e319f793141ea5a across the coordinated protocol/node/client set.
The blocking protocol concern from my previous review is resolved. settlement_compatibility() now bounds both ends of MIN..=CURRENT; a future client version is classified as NodeTooOld, and the two refusal directions have distinct append-only wire variants. Existing postcard discriminants remain pinned by tests.
The inherited ruint advisory is also fixed, local cargo test --lib --locked passed 87/87, and all 11 checks are green, including security audit and publish dry-run.
No protocol-code blocker remains. The PR body still describes the superseded behaviour in places (future versions accepted and stale test names); please refresh that before merge so the review record matches the implementation. Publication of 2.4.0 must precede repinning the coordinated node/client PRs.
Versioning is the release train's call, driven by the PR's Semver impact box, not something a feature PR should pre-empt. Restores version 2.3.2 and retitles the changelog entry as Unreleased, since the number is not known at PR time. No functional change.
The regression test that makes the append-only rollout safe covered only request bodies 0/2/4/6 and the endpoints of ProtocolError. Reordering a response variant, or any error variant between 1 and 6, would have passed while breaking every peer built against the current release. Responses read the same discriminant space as requests, so pin all of them.
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Re-reviewed exact head e97901694cba5cad84da0b99ee8af02a791df2cf. The incremental changes complete the wire regression coverage by pinning the pre-existing response and ProtocolError discriminants, while correctly leaving the crate version bump to the release train. Bounded settlement compatibility and the two typed refusal directions remain sound. Local verification: cargo test --lib --locked passed 87/87 and formatting passed; all 11 GitHub checks are green. Approved.
2 participants
Linear issue
V2-975 — https://linear.app/autonominetwork/issue/V2-975/refuse-to-quote-clients-that-cannot-settle-correctly-instead-of
Risk tier
Wire types and the payment-admission path. T3 even though this crate only supplies the vocabulary and enforces nothing, because the vocabulary is the wire contract.
Compatibility
ChunkMessageBodyvariants and oneProtocolErrorvariant are appended. Every existing discriminant keeps its value, so a peer built against2.3.xdecodes current traffic unchanged. A peer that does not know the new variants rejects them as an unknown discriminant rather than misreading a variant it does know.CHUNK_PROTOCOL_IDis unchanged atautonomi.ant.chunk.v1. Consequence to plan for: a client sending V2 gets no answer from a node built on2.3.x, so nodes must be upgraded before clients start sending V2. The client PR handles the mixed fleet with a per-peer fallback.CURRENT_SETTLEMENT_VERSION,MIN_SUPPORTED_SETTLEMENT_VERSION,settlement_version_is_supported,client_update_required_message,ChunkQuoteRequestV2,MerkleCandidateQuoteRequestV2, and the two enum variants. Nothing is renamed, removed, or retyped. Both enums were already#[non_exhaustive], so downstreammatcharms are unaffected.Semver impact
No version bump is taken in this PR; the release train owns that. Minor per the crate guide: "Add a new
ChunkMessageBodyvariant (existing variants unchanged): minor".Test evidence
cargo test --lib— 23 chunk tests pass, 0 failed.cargo clippy --all-targets --all-features -- -D warningsclean.cargo fmt --allclean.cargo docbuilds.Five tests added, one of which is load-bearing:
appending_v2_variants_leaves_existing_discriminants_untouched— encodes each existing message body and asserts its wire discriminant byte (PutRequest0,GetRequest2,QuoteRequest4,MerkleCandidateQuoteRequest6), then asserts the new variants take 8 and 9. This is the test the whole incremental-rollout design rests on: postcard assigns discriminants by declaration order, so inserting a variant anywhere but the end would make every older peer silently misread every message from a newer one. That property was previously assumed; it is now enforced.client_update_required_is_appended_to_protocol_error— same guarantee forProtocolError, which rides inside quote responses.v2_quote_request_round_trips_with_the_settlement_versionandv2_merkle_candidate_request_round_trips_with_the_settlement_version— encode/decode of both new requests.settlement_gate_accepts_current_and_refuses_anything_older— including that a future settlement version is accepted, so an old node cannot veto a newer rule set it has never heard of.update_required_message_tells_the_user_how_to_fix_it— asserts the wording carriesant update, "too old", and "nothing was charged". The wording is the deliverable for an end user, so it is pinned rather than left to drift.No behavioural test here beyond encoding: this crate defines the vocabulary and enforces nothing. Enforcement evidence lives on the ant-node PR.
New dependency
none
ADR
ADR-0010: Settlement version and pre-payment compatibility (Proposed) — added on the ant-node branch in response to review.
https://github.com/WithAutonomi/ant-node/blob/settlement-version-quote-gate/docs/adr/ADR-0010-settlement-version-and-pre-payment-compatibility.md
Records the inclusive
MIN..=CURRENTrange and why the upper bound is load-bearing, the two refusal directions and why they must stay distinct during a client-first rollout, the unversioned retry as a bounded downgrade path with a compile-time cutover rule, and the residual that merkle storers are not exactly the peers a client quotes.Builds on ADR-0008, whose re-open trigger ("a rise in refused batch uploads after the boundary") is what fired here.
Release readiness: coordinated set, do not merge alone
This crate is inert by itself: the new variants are never constructed unless a client sends them and a node acts on them, which are separate PRs. But merging publishes it, and publication is what unblocks the other two, so it should not land until the coordinated set is fleet-ready.
The open gates live on ant-node #204 and ant-client #171 and are enumerated there: mixed-version proof over a real connection, deployment ordering (nodes before clients, which has live latency impact if reversed), observability of the adoption counter, NAT/canary, rollback rehearsal and fleet safety.
Approved on the code, not a statement that the change set is production ready.
Mitigation / rollback
Revert the commit. Nothing in this crate changes behaviour on its own: the new variants are never constructed unless a client chooses to send them and a node chooses to act on them, and both of those are separate PRs. Until the ant-client PR ships, this release is inert on the wire.