Skip to content

feat(core): type a wallet balance shortfall as Error::InsufficientFunds (#152) - #224

Open
Nic-dorman wants to merge 3 commits into
mainfrom
fix/typed-insufficient-funds
Open

Nic-dorman wants to merge 3 commits into
mainfrom
fix/typed-insufficient-funds

Conversation

@Nic-dorman

@Nic-dorman Nic-dorman commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Fixes #152. Upstream half of V2-601 (mobile SDK).

A wallet-backed payment with too few payment tokens failed with the generic data::Error::Payment(String). Consumers such as ant-ffi, the mobile SDK and the CLI could only tell "top up your wallet" apart from any other payment failure by matching message text.

Why it isn't the fix #152 describes

#152 proposed matching evmlib::wallet::Error::InsufficientTokensForQuotes(have, need) by type before the generic stringify. That's no longer possible on any path:

  • File and data uploads now go through evmlib's journaled API (native_payment.rs → Wallet::prepare_payment). It returns Result<_, String> and already calls .to_string() on that variant (evmlib-0.10.0/src/wallet/journal.rs:214).
  • ant-protocol re-exports neither evmlib::wallet::Error nor its variant, only PayForQuotesError and Wallet. ant-core's Cargo.toml forbids a direct evmlib dependency, so even the paths that still hold the typed error can't name it.

That leaves evmlib's report text as the only contract, so the fix pins it with a test.

Change

  • New data::Error::InsufficientFunds { have: Amount, need: Amount } (atto). Display: insufficient funds: wallet has {have} atto, payment needs {need} atto.
  • Error::from_payment_failure(String) parses evmlib's Insufficient tokens to pay for quotes. Have: … atto, need: … atto wherever it appears in the message (callers prefix context). Anything else stays Payment with the message verbatim, and truncated or malformed reports are not guessed at.
  • It's used at every wallet-backed payment site:
    • the journaled native path (native_payment.rs, used by file and data uploads)
    • wave-batch pay_for_quotes (batch.rs ×2)
    • Merkle pay_for_merkle_tree (merkle.rs)
    • the non-native upload adapter (upload.rs ×2)
    • single-chunk payment: payment.rs now passes the typed error through instead of re-wrapping it as Payment("On-chain payment failed: …")
  • classify_error treats it as ApplicationError, so the adaptive limiter isn't pushed down. The exhaustiveness guard and the covers-all-variants table are extended.
  • Behaviour to note:
    • Nothing has been paid when it's returned, because evmlib checks the balance before signing or sending.
    • A shortfall after earlier batches of the same upload were paid still surfaces as PartialUpload, so the spend and the stored chunks are still reported. The error is kept typed in the new PartialUpload::cause field (Option<Box<Error>>). That also holds when storing the already-paid Merkle batches fails as well. reason keeps the text, so the CLI output is unchanged.
    • A gas (ETH) shortfall isn't pre-checked by evmlib and stays Payment, as noted out of scope in Payment errors collapse insufficient-funds into generic Error::Payment(String) — add a typed InsufficientFunds variant #152.
    • The browser/JS-wallet adapters are untouched.

Linear issue

Closes V2-1507

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.

Client-side error typing only. The payment flow, amounts, wire messages and node interaction are unchanged.

Compatibility

  • Wire: none
  • Storage: none
  • API: breaking for ant_core::data::Error, which is not #[non_exhaustive].
    • It gains a variant, so a downstream exhaustive match must add an arm.
    • PartialUpload gains a cause field, so code that builds it, or destructures it without .., must add it. The CLI, ant-ffi and antd's From impl all match with .., so they are unaffected. antd's test core_partial_upload_is_not_retryable_and_says_re_prepare builds one, so it needs cause: None at antd's next ant-core bump.
    • ant-ffi (ant-sdk/ffi/rust/ant-ffi/src/lib.rs From<ant_core::data::Error>) has an other => catch-all, so it still compiles. But until it adds an arm, a shortfall maps to ClientError::InternalError where it used to map to PaymentError. That arm (→ a new ClientError::InsufficientFunds) is V2-601's follow-up.
    • The CLI message changes from payment error: Insufficient tokens to pay for quotes. Have: … to insufficient funds: wallet has … atto, payment needs … atto.

Semver impact

  • breaking
  • feature
  • fix

Test evidence

Review fix d36eb64 (2026-10-27): the shortfall stays typed in PartialUpload::cause. Run on Linux (dev2, stable, --locked), on 388c3814 plus this commit:

  • cargo fmt --all -- --check and cargo clippy --all-targets --all-features -- -D warnings: pass.
  • New unit tests in data::client::upload::tests (6), the data::error tests and the settlement-refusal test: 30/30. The whole ant-core lib: 877 passed, 0 failed.
  • e2e_native_payment (Anvil): 4/4, including insufficient_funds_then_funding_unblocks_native_payment.
  • Prod compat smoke on mainnet, re-run with this build:
    • ant file download of the canary file (3cba93c8…, 5,277,908 B): pass, and the byte count matches.
    • Upload of a random 3 MiB file with a fresh zero-balance wallet:
      • --no-merkle: File upload failed: insufficient funds: wallet has 0 atto, payment needs 220115689453125000 atto
      • --merkle: File upload failed: insufficient funds: wallet has 0 atto, payment needs 288523605468750000 atto
    • Both exit 1 before any transaction is sent.

Rebased 2026-10-23 onto main at 50bc653 (0.3.12 promoted), 66 commits newer. Two changes came with it:

Every direct wallet payment call site on the new main still maps errors through Error::from_payment_failure. Re-run on Linux (dev2, stable):

  • cargo fmt --all -- --check, cargo clippy --all-targets --all-features -- -D warnings and the browser-wasm clippy: pass.
  • cargo test -p ant-core --lib data: pass.
  • e2e_native_payment: 4 passed, including insufficient_funds_then_funding_unblocks_native_payment.

Original evidence follows.

  • Real-chain guard: e2e_native_payment::insufficient_funds_then_funding_unblocks_native_payment (Anvil) now asserts Err(InsufficientFunds { have: 0, need: 100 }) for an unfunded wallet through the real journaled path. With the old map_err(Error::Payment) restored, it fails with expected InsufficientFunds, got Err(Payment("Insufficient tokens to pay for quotes. Have: 0 atto, need: 100 atto")). That confirms both evmlib's live wording and that the test catches a regression. cargo test -p ant-core --features native,test-utils --test e2e_native_payment: 4/4 pass.

  • Unit tests:

    • Display.
    • Parsing the report bare and under each caller's context prefix, with have/need above u64::MAX to prove the U256 round-trip.
    • Non-shortfall messages (nonce errors, an RPC gas "insufficient funds for gas", truncated or malformed reports) stay Payment verbatim.
    • classify_error_covers_all_variants.
  • cargo fmt --all -- --check ✓ · cargo clippy --all-targets --all-features -- -D warnings ✓ · cargo test -p ant-core --lib: 804 passed · cargo test -p ant-cli: 30 passed. All run on Windows 11.

  • Prod compat smoke on mainnet (2026-10-23), rebased head 388c3814 against main ccd4106d. Run on Linux (dev2, stable 1.99.1) with --locked release builds.

    • The wallet is a freshly generated key, and ant wallet balance reports 0. It holds neither tokens nor gas, so nothing can be paid.

    • Uploading a random 3 MiB file (3 chunks) collects real mainnet quotes and then fails at payment:

      Path main this branch
      --no-merkle File upload failed: payment error: Insufficient tokens to pay for quotes. Have: 0 atto, need: 228374654296875000 atto File upload failed: insufficient funds: wallet has 0 atto, payment needs 228374654296875000 atto
      --merkle File upload failed: payment error: Insufficient tokens to pay for quotes. Have: 0 atto, need: 185711296875000000 atto File upload failed: insufficient funds: wallet has 0 atto, payment needs 294520417968750000 atto
    • Both builds exit 1 before sending any transaction. The need amounts differ between runs because quotes vary.

    • This confirms that evmlib's wording on the live network still matches what from_payment_failure parses, on both payment paths.

    • A mainnet ant file download of a public canary file (3cba93c8…, 5,277,908 B) with this build also succeeded, and the byte count matches.

New dependency

none

ADR

n/a

Mitigation / rollback

Revert the commit. Consumers that already match InsufficientFunds go back to seeing Payment(String).

🤖 Generated with Claude Code

@Nic-dorman
Nic-dorman force-pushed the fix/typed-insufficient-funds branch from f6fc3ab to 795606a Compare October 23, 2026 09:36
…ds (#152)

A wallet without enough payment tokens failed with the generic
Error::Payment(String), so consumers (the mobile FFI/SDK, the CLI) could
only tell "top up your wallet" from any other payment failure by
matching message text.

Since #152 was filed, file and data uploads moved to evmlib's journaled
payment API, which returns InsufficientTokensForQuotes already rendered
as a String, and ant-protocol re-exports neither evmlib::wallet::Error
nor its variant (and ant-core may not depend on evmlib directly). So the
variant cannot be matched by type on any path; evmlib's report text is
the contract. Error::from_payment_failure parses its have/need amounts
wherever the report sits in the message and is used at every
wallet-backed payment site: the journaled native path, wave-batch and
Merkle batch payment, the non-native upload adapter, and single-chunk
payment (whose caller no longer re-wraps the typed error).

The e2e_native_payment Anvil test now asserts InsufficientFunds { have:
0, need: 100 } for an unfunded wallet, pinning evmlib's wording through
the real path: with the old mapping it fails with the Payment string.
classify_error treats the new variant as an application error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Nic-dorman
Nic-dorman force-pushed the fix/typed-insufficient-funds branch from 795606a to 388c381 Compare October 23, 2026 18:52

@MaidSafe-Hermes MaidSafe-Hermes 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.

REQUEST_CHANGES — #224 at head 388c3814e08dad8e713affbedb9579015ba98db9.

Coverage: 2/2 independent seats reviewed this head — sol-1 (gpt-6.1-sol, xhigh) returned UNCERTAIN; glm-1 (z-ai/glm-5.3) returned CHANGES_REQUIRED. The material objection below is source-confirmed; the seats did not agree.

Finding — ant-core/src/data/client/upload.rs, upload_unique_records_inner (the match combining preparation and storage results): when a later payment preparation returns Error::InsufficientFunds while storage of already-paid batches also fails, the PartialUpload arm keeps the payment error only as formatted reason text. The other both-fail arm wraps it as Error::Payment. In either case the caller loses the typed InsufficientFunds { have, need } cause and must parse display text, defeating the API's stated top-up use case for multi-batch partial uploads. This path is reachable because the function runs store_upload_merkle after preparation fails, to handle already-paid batches; the PR itself documents the PartialUpload case.

Please preserve a structured insufficient-funds cause when returning PartialUpload (while retaining the partial spend and outer partial-upload status), and add a regression test for insufficient funds on a later batch combined with a storage failure. Cover the ordinary both-fail branch as well if it remains reachable.

Other gates: all reported GitHub checks are green. Locally, cargo test --locked -p ant-core --lib data::error::tests -- --nocapture passed (23 tests). The focused e2e_native_payment test could not start locally because the Anvil executable is absent (SpawnFailed: No such file or directory); CI reports the E2E jobs passing. The public data::Error enum addition is source-breaking because it is not #[non_exhaustive]; the PR discloses this and marks it breaking.

`PartialUpload` gains `cause: Option<Box<Error>>`, the typed error that
stopped the upload before every chunk was tried (for example
`InsufficientFunds` on a later batch). Three places that flattened it to
`reason` text now keep it:

- a failed payment preparation combined with a partial storage of the
  already-paid batches (`payment_and_storage`);
- the same with any other storage error, which became `Error::Payment`
  text;
- the upload-level wrapper that turns an error after spend into a
  `PartialUpload` (`after_spend`), with `upload_records` carrying the
  field through.

Storage-failure constructors set `cause: None`. `reason` and the CLI
output are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Nic-dorman

Copy link
Copy Markdown
Member Author

Addressed in d36eb64.

Finding: the typed shortfall is lost when a partial upload is returned. PartialUpload gains a cause: Option<Box<Error>> field. It holds the error that stopped the upload before every chunk was tried. For example, Some(InsufficientFunds { have, need }) when the wallet couldn't pay for a later batch. It is None when chunks failed to store. The text in reason is unchanged, so the CLI output is the same. It is boxed to keep Error small.

Three places used to flatten the error to text. They now keep it typed:

  1. Both fail, with a partial upload from storage (the case you flagged, in upload_unique_records_inner). It still returns PartialUpload with the stored chunks, spend and counts from storage, and cause now holds the payment error.
  2. Both fail, with any other storage error. This no longer becomes Error::Payment(text). It returns a PartialUpload built from the outcome so far, with the payment error as cause.
    • The branch can't be reached today: store_upload_merkle only fails with PartialUpload, because merkle_store_with_retry has no error exit and merkle_deferred_retry only forwards its result.
    • It is handled anyway, so that a future change can't bring back the text-only error.
  3. The upload-level wrapper (upload_unique_records). This one wasn't in your finding, but it does the same thing in the more common case.
    • Any error that arrives after earlier batches were paid or stored becomes a PartialUpload there. A single-node wave whose payment fails with InsufficientFunds after earlier waves settled is the typical example.
    • It now keeps the error as cause, and upload_records carries cause through when it re-maps a partial upload to the caller's original addresses.

The combining logic now lives in two small functions, payment_and_storage and after_spend, with unit tests in data::client::upload::tests:

  • a_later_shortfall_with_failed_storage_keeps_what_was_stored_and_the_typed_cause: the case you flagged. The stored chunks, the spend (100 atto, 7 wei), the failed count and both messages in reason are kept, and cause is InsufficientFunds { have: 5, need: 300 }.
  • a_later_shortfall_with_another_storage_error_is_a_partial_upload_with_the_typed_cause: the both-fail case with another storage error.
  • a_shortfall_after_an_earlier_batch_settled_is_a_partial_upload_caused_by_it: the wrapper.
  • a_shortfall_before_anything_was_spent_stays_insufficient_funds, a_shortfall_with_nothing_to_store_is_returned_as_it_is and a_partial_upload_keeps_its_cause_after_spend: the cases that must not be re-wrapped.

All other PartialUpload constructors are storage failures and set cause: None.

Compatibility: adding the field breaks code that builds PartialUpload, or destructures it without ... This PR is already marked breaking.

  • The CLI, ant-ffi and antd's From impl use .., so they are unaffected.
  • One antd test builds a PartialUpload, so it will need cause: None at antd's next ant-core bump.
  • The CHANGELOG entry and the PR body say so.

There is no new E2E test. It would need a wallet that is funded for the first Merkle batch and then runs short, combined with a storage failure in the same upload. The devnet tests can't set that up reliably, so the combining logic is unit-tested directly instead.

Verified (Linux, dev2, stable, --locked):

  • fmt, and clippy --all-targets --all-features -D warnings.
  • The 6 new tests, the data::error tests and the settlement-refusal test: 30/30. The whole ant-core lib: 877/877.
  • e2e_native_payment (Anvil): 4/4, including insufficient_funds_then_funding_unblocks_native_payment.
  • The prod compat smoke on mainnet was re-run with this build, and the PR body has the details:
    • The canary download passes, with the byte count matching.
    • A zero-balance wallet fails on both --no-merkle and --merkle with insufficient funds: wallet has 0 atto, payment needs … atto, before any transaction is sent.

CI on d36eb64: all jobs pass except Security Audit. The 6 new tests pass on ubuntu and macOS, and the lib passes 877/877 on both. e2e_native_payment passes 4/4 on both.

Security Audit: this job fails only on RUSTSEC-2026-0387 (crypto-bigint 0.5.5, reached through alloy/k256 and webrtc/dtls). The same failure is on main, and it is tracked as V2-1743. This PR doesn't change Cargo.toml or Cargo.lock.

@MaidSafe-Hermes MaidSafe-Hermes 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.

APPROVE — focused re-review of #224 at head d36eb644183381a412e4da19d1e757e00762f8f6, limited to the fix-up after prior review 388c3814.

The prior blocker is resolved. payment_and_storage now preserves the payment error as PartialUpload::cause in both combined-failure arms, while retaining stored/failed chunk counts, spend, and reason text. after_spend carries the cause through the upload-level partial wrapper, and the retry/result-expansion path plus Error::duplicate() preserve it. The new tests cover both combined failures, prior spend, passthrough, and cause preservation. I found no remaining material blocker in the reviewed delta.

Coverage: 2/2 focused independent seats reviewed this delta — sol-1 (gpt-6.1-sol, xhigh) and glm-1 (z-ai/glm-5.3) both returned APPROVE. This was a focused fix re-review, not a fresh review of the entire PR.

Verification: current-head cargo fmt --all -- --check passed; cargo test --locked -p ant-core --lib data::client::upload::tests -- --nocapture passed (6 tests); GitHub CI Unit Tests, Clippy, Format, builds, E2E, Merkle E2E, WASM/browser and PR checks pass. The local full ant-core lib command exceeded the local tool's 420-second limit, so I do not claim a local full-suite pass.

Separate CI note: Security Audit fails on RUSTSEC-2026-0387 (crypto-bigint 0.5.5). The Cargo.lock object is identical at the base, prior head and current head; the same audit command also fails at base on that advisory. The repository ruleset requires only linear-link and pr-template, both passing. Treat this as an inherited audit issue, not a blocker introduced by this PR. The new public PartialUpload::cause field is source-breaking for downstream constructors/exhaustive patterns; the changelog documents this impact.

Approval is not a merge authorization.

#238 rotated the old [Unreleased] into the 0.3.7 to 0.3.12 sections, so
this PR's entry moves to the new, empty [Unreleased], under the same
heading it had before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

Payment errors collapse insufficient-funds into generic Error::Payment(String) — add a typed InsufficientFunds variant

2 participants