Pay the 3x settlement multiplier on the merkle batch path - #161
Conversation
A chunk stored through a merkle batch earned its node one third of what the identical chunk earned through a single-node upload. The single-node path pays the median-priced issuer 3x its quote; the merkle path copied the bare quoted price into the on-chain payable amount, so the contract's median16(amount) x 2^depth settled 1x the median per leaf. Apply the multiplier where the payable amount is built. The signed candidate prices and the pool hash are untouched: the hash is the key a storer resolves the on-chain payment record under, and it must keep committing to the quotes the nodes actually signed. Batch uploads get ~3x more expensive. The alternative, lowering the single-node path to 1x, would cut per-chunk revenue network-wide by two thirds instead.
Two follow-ups to paying the 3x multiplier on the merkle path. Read the multiplier from the single-node constant instead of a second literal 3. The defect being fixed was the two payment paths disagreeing about the multiplier, so they now read it from one place. Bill the estimate for padded leaves. The vault charges median16 x 2^depth per batch and the tree rounds its leaf count up to a power of two, so a 65-chunk batch pays for 128 leaves. Estimating from the raw chunk count was already wrong before this change, but it erred high and was harmless; now that merkle settles at 3x it errs low, by up to 2x on a non-power-of-two batch. An under-quote is the harmful direction, because a caller sizing its wallet from the estimate runs dry mid-upload.
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Reviewed exact head b8c1a35c398aad3be4dc8766cf4d343870000984 alongside WithAutonomi/ant-node#188.
Verdict: changes requested. The 3× payable construction is correct, but the new padded-leaf estimator exposes a deterministic 256n+1 payment-path defect and currently claims an executable path that does not exist.
What is correct
pool_commitment_with_payment_multiplierapplies the shared client-side 3× constant to on-chain candidate amounts with checked arithmetic.- Signed candidate prices and
pool_hashremain unchanged. - The focused multiplier/hash tests pass, as do the real forced-Merkle upload/download tests against released
ant-node 0.15.0.
Blocker: 256n+1 becomes a paid partial failure
merkle_billable_leaves(257) returns 258 and the test says the final singleton is billed as a two-leaf tree (file.rs:3506-3510). The execution path does not do that:
pay_for_merkle_multi_batchpartitions withaddresses.chunks(MAX_LEAVES)(merkle.rs:647-665), so 257 becomes[256, 1].- The first 256-address sub-batch can settle on-chain.
- The one-address remainder is passed unchanged to
pay_for_merkle_single_batch;MerkleTreerequires at least two leaves, so it fails. - Because earlier payment already succeeded, the function returns a partial result with 256 proofs (
merkle.rs:681-699), and the user-facing upload reports the final address failed.
Preflight can leave any arbitrary to_upload length, so this affects exact 257/513/etc. uploads and larger uploads whose already-stored filtering leaves such a count. The external-signer path also does not split/rebalance an oversized set.
Suggested fix: use one shared partitioning routine for execution and estimation, guaranteeing every Merkle partition has 2..=MAX_LEAVES entries. Rebalance singleton remainders (257 → [255, 2]; 513 → [256, 255, 2]) rather than inventing a duplicate leaf. Add regressions for 257, 513, post-preflight 257, and external signing—or explicitly route the singleton through a documented compatible payment path.
Test-evidence correction
The PR body overstates current estimate evidence:
e2e_cost_estimate::test_estimate_matches_actual_costuses Auto mode on files producing about three chunks, so it exercises the Single path, not Merkle padding.- Current Merkle E2E uses released
ant-node 0.15.0; it proves new-client → old-node compatibility, not new-client →ant-node#188enforcement. - No estimate-versus-actual test covers 65/129/257/300 chunks.
Also rename/reword merkle_per_chunk_settlement_matches_single_node_multiplier: it proves 3× settlement per padded leaf against the Merkle pool median, not equal cost per actual chunk around padding boundaries.
Verification
cargo fmt --all -- --check— passedcargo test --lib --all --no-fail-fast— 422 passed- Focused multiplier/hash/estimate tests — 5 passed
- Current client CI is green except Ubuntu general E2E still pending.
The economic release remains separately gated on @forthebux sign-off and paired cross-version evidence. No approval or merge performed.
9093311 to
6c2a604
Compare
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Re-reviewed current head 6c2a604cf203da80101b1cb6c0a237193ca746f6 after the follow-up push.
Verdict: not approved yet. The 256n+1 payment defect is fixed: execution, padded-leaf estimation and gas estimation now share the same partitioner; 257 becomes [255, 2]; focused local checks pass (66 Merkle-related library tests, 8 merkle_unit tests, and e2e_merkle compiles).
Remaining gates:
- Current CI is not green.
Merkle E2E (macos-latest)failed intest_merkle_payment_across_batch_boundary: the new 65-address payment received 15 candidates but requires 16. Ubuntu passed the same job, so this may be harness/network flakiness, but it needs a clean rerun or a deterministic fix before approval. The broad E2E jobs are still pending. - The PR body still states the disproved rollout model (“both must ship on the same train”, “either order is safe”, and “partial rollout cannot strand uploads”). The implementation/ADR now use a client-first sequence, with old clients refused by enforcing nodes after the boundary. Please update the public review record.
- API warning:
MerkleBatchTooLargeextends the public exhaustiveant_core::data::Errorenum. Downstream exhaustive matches can stop compiling. Please confirm this breaking surface is deliberate for the next release, or reuse an existing error shape.
Release remains separately gated on paired new-client → new-node evidence and @forthebux economic sign-off. No merge performed.
Payment split oversized uploads with `addresses.chunks(MAX_LEAVES)`, which respects the 256-leaf ceiling but not the two-leaf floor: 257 addresses came out as [256, 1]. The 256-address batch was paid on-chain and the singleton remainder could not build a tree, so the upload returned a paid partial failure. Every post-preflight count congruent to 1 modulo 256 hits it (257, 513, 769, ...), and preflight leaves an arbitrary count behind. Add one partitioning helper, `merkle_batch_sizes`, and route both payment and cost estimation through it. It borrows one address from the preceding batch rather than stranding it: 257 becomes [255, 2], 513 becomes [256, 255, 2]. Order is preserved and no address is duplicated or synthesised, so a partition is a plain in-order cover whose every batch is a buildable 2..=256-leaf tree. Billable leaves are now summed over those same partitions instead of a second batching model that could drift from what payment does. The leaf totals are unchanged (255 pads to 256), but they are now derived rather than restated. The external-signer path passed its whole address set to `prepare_merkle_batch_external`, which cannot span transactions: one prepared batch is one signature and one payment. It now returns a typed `MerkleBatchTooLarge` before any candidate collection, rather than failing opaquely in the tree build or silently switching payment model. Also renames `merkle_per_chunk_settlement_matches_single_node_multiplier`, which proves 3x settlement per *padded leaf* against the pool median, not equal cost per actual chunk around padding boundaries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6c2a604 to
ffb7567
Compare
dirvine
left a comment
dirvine
left a comment
There was a problem hiding this comment.
Engineering approval at head ffb7567482833b5c92699385b387e6605fb1da73. Verified the 3× payable construction, shared Merkle partition/estimate model, 257→[255,2] fix, external-signer guard, focused tests and current compile/lint checks. CI E2E jobs may finish independently. This is code-review approval only; it is not economic sign-off, release authorisation or merge authorisation.
3 participants
Pay the 3x settlement multiplier on the merkle batch path. Client half of
WithAutonomi/ant-node#188, which carries ADR-0008 and the storer-side
requirement; both must ship on the same train — see Compatibility for why
either order is safe in the meantime.
A chunk stored through a merkle batch earned its node one third of what
the identical chunk earned through a single-node upload, for the same storage and
the same replication.
SingleNodePayment::from_quotespays the median-priced issuer3 x price. Themerkle path built its pool commitments straight from
MerklePaymentCandidatePool::to_commitment(), which copies the bare quoted priceinto the on-chain payable
CandidateNode.amount, so the contract'smedian16(amount) x 2^depthsettled1 x medianper padded leaf.The multiplier now goes on the payable amount, which is the field the vault pays
out of, and it is read from the same
SINGLE_NODE_PAYMENT_MULTIPLIERthesingle-node path uses rather than a second literal
3— the defect was the twopaths disagreeing about the multiplier, so they now read it from one place. The
signed candidate prices and the pool hash are deliberately untouched: the hash is
the key a storer resolves the on-chain payment record under, and it has to keep
committing to the quotes the nodes actually signed.
Nothing detected this. The storer recomputed its expectation from the same
un-multiplied signed prices, so its
paid_amount >= expectedcheck passed, andMerklePaymentCandidatePool::verify_prices(which would compare on-chain amountsagainst signed prices) is called by neither repo.
Second commit: cost estimates now bill merkle uploads for the padded tree.
The vault charges per leaf and the tree rounds up to a power of two, so a
65-chunk batch pays for 128 leaves. Estimating from the raw chunk count was
already wrong, but it erred high and was harmless; at 3x it errs low by up to 2x,
and an under-quote is the harmful direction because a caller sizing its wallet
from the estimate runs dry mid-upload.
Linear issue
Risk tier
Client-only code, but it changes what every batch upload pays on-chain, so it is
an economics change rather than T1.
Compatibility
Client cost changes: batch uploads of 64+ chunks pay roughly 3x more. Stated
plainly because it is the point of the change, not a side effect.
Mixed-version rollout, and the one-week receipts:
An upgraded client simply overpays against an un-upgraded storer.
off the receipt's own timestamp precisely so receipts already bought at 1x keep
working until they expire, instead of being refused after the money was spent.
pinned to an old client past that boundary needs telling before the train.
Semver impact
No API change. Flagging for review that the economic impact is large even
though the semver impact is not.
Test evidence
Branched from upstream
mainat81848a0900fa9972e9af2d323bf3b49bf8d382aaandverified the diff touches only the two intended files — no workflow, manifest or
lockfile churn (
git diff --name-onlyagainst that base returns exactlyant-core/src/data/client/{file,merkle}.rs).Every CI job that does not need a chain, run locally at this head:
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo test --lib --allcargo test -p ant-core --test merkle_unitcargo test -p ant-core --test unit_self_encryptdaemon_integration+node_add_integrationcargo doc --all-features --no-depsChain-backed, against a local Anvil devnet:
e2e_cost_estimate— 6/6 pass, includingtest_estimate_matches_actual_cost,which compares the quoted estimate against what the upload actually settles.
That is the test that would have caught the padded-leaf change being wrong in
either direction.
e2e_merkle(35-node testnet) — the run that exercises a real batch paying 3xthrough the vault. Still in flight locally when this PR was opened; it is also
a required CI job here, so the authoritative result is the check on this PR.
Five new tests: every candidate's payable amount is 3x its quoted price; the
signed prices and the pool hash are unchanged by the multiplier; the median of
the payable amounts equals 3x the median quote (the per-leaf parity invariant);
padded-leaf billing is exact at powers of two and rounds up otherwise, including
the
MAX_LEAVESsplit and the two-leaf minimum; and billing never under-quotesacross 1..2000 chunks.
cargo clippy --all-targets --all-features -- -D warningsclean;cargo fmtclean;
RUSTDOCFLAGS=--deny=warnings cargo docclean.Gap, stated plainly: what is still owed for T3 is the cross-version pair —
an old client's 1x upload on a fresh receipt being refused once past
ant-node#188's boundary, and an upgraded client admitted at 3x by a node built
from #188. Both halves are local branches right now, so that matrix needs a
testnet with the two builds facing each other; the single-repo suites above
cannot cover it.
New dependency
none
ADR
Mitigation / rollback
Revert this PR and clients return to paying 1x on the batch path. Safe in either
order with the node side: if this is reverted while #188 is live, batch uploads
are refused once past its boundary, so the two reverts should be paired — or
move
MERKLE_PARITY_ENFORCED_FROM_UNIXforward, which restores acceptancewithout a client release.