fix: x402 payment fallback fails closed on ambiguous outcomes (#583) - #592
Conversation
pr-reviewer Summary for #60bd2ef✅ No issues found The code review completed successfully with no findings. Review effort: 3/5 (Moderate) SummaryThis 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:
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 |
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-Signatureis transmitted,_x402Retrycollapsed every outcome that wasn't a clean 2xx into a singlenull— 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 anynullas "safe to try the next payment option or provider."That's unsafe whenever the outcome doesn't actually prove the payment was rejected:
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.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"nullsentinel — so a real success could be misread as a rejection, triggering an unnecessary second payment.Fix
X402_PAYMENT_REJECTED, a dedicatedSymbolsentinel (exported fromapi.js), returned by_x402Retryonly when the response is non-5xx and its body is readable — the one case that actually proves a clean rejection without settlement.ErrorCode.PAYMENT_AMBIGUOUS._x402Retrynow throwsNansenError(..., ErrorCode.PAYMENT_AMBIGUOUS)for: a transport failure during the paidfetch, any 5xx, an unreadable rejection body, or an unreadable success body.result !== X402_PAYMENT_REJECTEDinstead of!== null, and re-throw aPAYMENT_AMBIGUOUSerror 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
The 5 failures are pre-existing, in
src/__tests__/doctor.test.js, and reproduce identically onmainwithout this change — they rely onchmod-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_x402Retryinx402-api-retry.test.js, 2 integration tests on the full fallback chain inapi.test.js), all passing.Clean, no output.
Manual verification of every
_x402Retrybranch:Integration-level: confirmed a 5xx on the paid retry after a local-wallet signature does not trigger a WalletConnect attempt (
handleX402Paymentnever called), and a 5xx on the paid WalletConnect retry surfacesPAYMENT_AMBIGUOUSwith exactly 2 total fetch calls (no repeated payment).Checklist
npm testpasses (output above; unrelated pre-existing failures noted)npm run lintpassesconsole.login corepatch— bug fix, prevents duplicate payment authorizations)src/schema.json— no changes needed (no new commands/options; this is internal API-client behavior)