Skip to content

fix: x402 payment fallback fails closed on ambiguous outcomes (#583) - #592

Merged
kome12 merged 1 commit into
nansen-ai:mainfrom
ygd58:fix/x402-ambiguous-payment-fallback
Sep 7, 2026
Merged

fix: x402 payment fallback fails closed on ambiguous outcomes (#583)#592
kome12 merged 1 commit into
nansen-ai:mainfrom
ygd58:fix/x402-ambiguous-payment-fallback

Conversation

@ygd58

@ygd58 ygd58 commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #583.

Related: #575 applies the same fail-closed principle to ambiguous swap/bridge broadcasts, but that's a separate code path — this covers src/api.js's x402 auto-payment fallback.

Problem

After a signed Payment-Signature is transmitted, _x402Retry collapsed every outcome that wasn't a clean 2xx into a single null — a provably clean rejection, a 5xx, an unreadable response body, and any transport failure/timeout all looked identical. Every fallback call site (Privy, local wallet with network fallback, WalletConnect) treated any null as "safe to try the next payment option or provider."

That's unsafe whenever the outcome doesn't actually prove the payment was rejected:

  • A 5xx after transmission doesn't mean the server rejected the payment — it could have settled it and then failed to respond.
  • A transport error/timeout after fetch() sent the signed payment carries the same ambiguity, and previously this exception wasn't even caught by _x402Retry — it propagated up and was swallowed by the generic "local wallet unavailable" catch in the caller, which then proceeded to sign and send a second, independent payment via WalletConnect for the same request.
  • An unreadable/invalid response body (rejection or success) is likewise unprovable either way.

In all of these cases, continuing the fallback chain means signing and transmitting another payment authorization for a request that may have already been paid.

Separately: a genuine successful response whose JSON body happens to be null (a legitimate value) was indistinguishable from the internal "rejected" null sentinel — so a real success could be misread as a rejection, triggering an unnecessary second payment.

Fix

  • Added X402_PAYMENT_REJECTED, a dedicated Symbol sentinel (exported from api.js), returned by _x402Retry only when the response is non-5xx and its body is readable — the one case that actually proves a clean rejection without settlement.
  • Added ErrorCode.PAYMENT_AMBIGUOUS. _x402Retry now throws NansenError(..., ErrorCode.PAYMENT_AMBIGUOUS) for: a transport failure during the paid fetch, any 5xx, an unreadable rejection body, or an unreadable success body.
  • All three fallback call sites (Privy loop, local-wallet loop, WalletConnect single call) now check result !== X402_PAYMENT_REJECTED instead of !== null, and re-throw a PAYMENT_AMBIGUOUS error immediately — the local-wallet catch no longer swallows it as "wallet unavailable, try WalletConnect", and the WalletConnect catch no longer folds it into the generic "x402 auto-payment failed" message. Each failure mode now stops the whole fallback chain instead of continuing to another provider.

Testing

npm test
Test Files  1 failed | 65 passed (66)
     Tests  5 failed | 2687 passed | 16 skipped (2708)

The 5 failures are pre-existing, in src/__tests__/doctor.test.js, and reproduce identically on main without this change — they rely on chmod-based unreadable-file simulation, which doesn't apply when tests run as root. Unrelated to this fix. 9 new tests added (7 unit tests on _x402Retry in x402-api-retry.test.js, 2 integration tests on the full fallback chain in api.test.js), all passing.

npm run lint

Clean, no output.

Manual verification of every _x402Retry branch:

clean 402 rejection, readable body -> REJECTED_SENTINEL (safe to try next)
5xx ambiguous                      -> throws PAYMENT_AMBIGUOUS
4xx unreadable body                -> throws PAYMENT_AMBIGUOUS
transport failure                  -> throws PAYMENT_AMBIGUOUS
success with null body             -> returns real null (not the rejection sentinel)
success with real data             -> returns the data, unchanged

Integration-level: confirmed a 5xx on the paid retry after a local-wallet signature does not trigger a WalletConnect attempt (handleX402Payment never called), and a 5xx on the paid WalletConnect retry surfaces PAYMENT_AMBIGUOUS with exactly 2 total fetch calls (no repeated payment).

Checklist

  • npm test passes (output above; unrelated pre-existing failures noted)
  • npm run lint passes
  • New code paths have tests (9 added: 7 unit + 2 integration)
  • No console.log in core
  • Error messages are actionable (name the ambiguous condition, state that no further payment was attempted)
  • Changeset added (patch — bug fix, prevents duplicate payment authorizations)
  • src/schema.json — no changes needed (no new commands/options; this is internal API-client behavior)

@nansen-pr-reviewer

Copy link
Copy Markdown

pr-reviewer Summary for #60bd2ef

No issues found

The code review completed successfully with no findings.

Review effort: 3/5 (Moderate)

Summary

This is a well-reasoned, carefully implemented safety fix for a real double-payment risk. The core logic is correct throughout, the test coverage is thorough, and all project conventions are followed.

What was reviewed:

  • src/api.js_x402Retry rewrite: The new X402_PAYMENT_REJECTED Symbol sentinel is the right tool here — it's unambiguous by design (unlike the previous null, which was aliased with genuine null-body API responses). The 5xx boundary (status >= 500) is the correct conservative cutoff: 5xx is treated as ambiguous, 4xx with a readable body is treated as provably rejected, 4xx with an unreadable body is treated as ambiguous. Transport failures and redirect: 'error'-triggered throw paths both surface as PAYMENT_AMBIGUOUS. No path left open.

  • Three call sites (Privy, local wallet, WalletConnect): All three now check !== X402_PAYMENT_REJECTED and re-throw PAYMENT_AMBIGUOUS immediately instead of falling through. The local-wallet catch no longer swallows ambiguous errors as "wallet unavailable" — the exact bug that allowed the WalletConnect double-payment. The WalletConnect clean-rejection path (line 824 falls through to line 848) correctly surfaces as a 402 error with paymentRequirements attached, which is the right behavior for WalletConnect being the last resort.

  • Tests: 7 unit tests on _x402Retry pin every new branch precisely. The 2 integration tests verify the two highest-risk scenarios (local-wallet ambiguity not triggering WalletConnect, and WalletConnect ambiguity surfacing PAYMENT_AMBIGUOUS with exactly 2 fetch calls). All existing tests updated correctly to reflect the sentinel replacing null.

  • 4xx body parse result discarded (lines 607–615): paidResponse.json() is called solely as a readability probe — the parsed value is intentionally discarded. This is correct; the only purpose is confirming the rejection is legible.

  • No AGENTS.md/CLAUDE.md violations. No BigInt arithmetic, no interactive prompts, no hardcoded keys, no live network calls in tests. Changeset correctly typed as patch. schema.json unchanged (no new commands/options). No .github/workflows/ or CODEOWNERS changes.


Token usage: 1,157 input, 4,656 output, 645,062 cache read, 47,006 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.

@kome12
kome12 merged commit 8c6f4cc into nansen-ai:main Sep 7, 2026
9 checks passed
@github-actions github-actions Bot mentioned this pull request Sep 7, 2026
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.

x402 fallback may generate multiple payment authorizations after an ambiguous response

2 participants