fix(upload): report the settlement refusal that stopped a merkle payment - #195
Conversation
A merkle upload larger than one tree pays sub-batch by sub-batch, one on-chain transaction each. When a later sub-batch is refused because storers no longer accept this client's settlement version, `pay_for_merkle_multi_batch` returns a partial receipt rather than an error, so the sub-batches that already settled are not stranded. It discarded the refusal in the process. The chunks those later sub-batches never covered then reached the store path with no proof, and were reported as `N chunk(s) short of quorum after 4 attempts`. They had no quorum shortfall and were never attempted. `ant-cli` prints that reason and nothing else, so on the one run that spent money the user was given a fabricated failure mode and none of the storer's upgrade instruction. The client-wide latch makes the next run fail cleanly, so the damage is one misleading run, but it is the run that paid. Report those chunks as having no merkle proof, and name the refusal when one stopped the payment. All three exits of `upload_merkle_from_spill` are covered: the normal one and both fatal store aborts, which previously reported only the abort and left the refusal in the per-chunk messages the CLI does not print. The refusal is a parameter rather than a latch read inside the function. Of its five callers only the fresh-payment wallet path passes one, taken straight after that upload's own payment returned. The two external-signer callers deliberately leave sub-batches unpaid by supplying `None` winner hashes and never call `pay_for_merkle_batch`, and the two cached-resume callers run no payment at all; attributing a refusal there would blame it for gaps it did not cause, and the latch is shared across references to one client, so that is reachable. The wording never says a proof-less chunk went unpaid. A sub-batch settles on-chain before its proofs are generated, so a proof-generation failure leaves chunks that were charged for and still have no proof. It also scopes the storer's message before quoting it: that message ends with "nothing was charged", which is true of the sub-batch it refused and false of an upload whose earlier sub-batches settled, and the CLI prints that spend on the same line. Mirrors `settlement_refusal_after_paid_waves`, which already does this for the single-node wave path. The merkle path never got the same treatment. Scope: command-line file uploads. The in-memory `data_put` path still reports a bare missing proof, and which error surfaces there races between a proof-less chunk and an unrelated store failure. No public API change.
mickvandijke
left a comment
There was a problem hiding this comment.
Looks good for the intended CLI file-upload path. Missing proofs are now distinguished from quorum failures, and partial uploads retain the relevant upgrade instruction.
A non-blocking improvement for concurrent uploads sharing a Client: consider carrying the refusal that actually stopped the payment alongside its partial result, rather than reading the client-wide refusal latch afterwards. Another operation can set that latch while this upload is awaiting a wallet transaction; if the transaction then fails for an unrelated reason, the final message can incorrectly attribute it to a settlement refusal. Preserving the cause per payment would make reporting more precise for overlapping operations. Sequential uploads and uploads using separate client instances are unaffected by this race.
Locally validated: 645 library tests, 24 Merkle/encryption tests, the targeted 35-node external partial-payment E2E test, and formatting checks all passed.
Linear issue
Closes V2-1175
Risk tier
Only the construction of an error message the client already returns. No node
behaviour, wire format, stored-data format, payments or upgrade mechanism.
Compatibility
is a private method,
Client::upload_merkle_from_spill.Semver impact
Test evidence
Local, on this branch, against
ant-core0.8.1 and publishedant-protocol2.3.5.--all-targets --all-featuresand-D warningscargo fmt --checkcargo docwith--deny=warningsmerkle_unit,unit_self_encryptdaemon_integration,node_add_integrationFive unit tests cover the reason construction. Three mutation checks confirm they
bite: reverting the helper to the previous behaviour reddens both reason tests
with the old
N chunk(s) short of quorum after 4 attemptsstring; dropping thesentence that scopes the storer's message reddens the scoping test; reverting the
fatal-abort composition reddens the fatal test.
test_external_merkle_partial_payment_is_partial_uploadgained assertions on thereason: an externally unpaid sub-batch must be reported as having no proof, and
must not be reported as a refusal.
Not covered
No end-to-end test drives a wallet-path refusal; that needs storers that actually
refuse. The wallet half rests on the unit tests and mutation checks. The E2E above
covers the external-signer half.
New dependency
none
ADR
n/a (Tier 1)
Mitigation / rollback
Revert the single commit. Nothing outside error-message construction changes, so
the blast radius is the text of a failure that was already being returned.
Scope, stated plainly
This fixes command-line file uploads. The in-memory
data_putpath(
merkle_upload_chunks) still returns a bareMissing merkle proof for chunk <hex>, and which error surfaces there races between a proof-less chunk and anunrelated store failure. This is not an ant-client-wide fix.
Two unrelated bugs found while doing this, not fixed here
A merkle sub-batch settles on-chain before
finalize_merkle_batchgeneratesits proofs. Both of these follow from that and predate this change:
pay_for_merkle_multi_batchtreats an empty proof map as "nothing paid yet".If the first sub-batch settles and then proof generation fails, it returns
Err, the caller writes the receipt cache only onOk, and the proofs cannotbe re-derived. That spend is lost with no recovery path.
timestamp are never folded into the partial receipt, so
PartialUploadSpendand the CLI under-report what the user actually spent.
Worth their own issues.