diff --git a/.changeset/fix-x402-ambiguous-payment-fallback.md b/.changeset/fix-x402-ambiguous-payment-fallback.md new file mode 100644 index 00000000..d1b10376 --- /dev/null +++ b/.changeset/fix-x402-ambiguous-payment-fallback.md @@ -0,0 +1,7 @@ +--- +"nansen-cli": patch +--- + +Fix the x402 auto-payment fallback in `src/api.js` generating multiple payment authorizations for the same request after an ambiguous outcome (issue #583). After a signed `Payment-Signature` was transmitted, `_x402Retry` previously collapsed every non-ok response — a clean rejection, a 5xx, an unreadable body — and every transport failure into a single `null`, and callers treated any `null` as "safe to try the next payment option/provider". That meant a 5xx, a timeout, or an unparseable response (any of which could mean the server already received and settled the payment) triggered signing and transmitting a *second* independent payment for the same logical request. Separately, a genuine successful response whose JSON body happened to be `null` was indistinguishable from a rejection, risking a second payment for an already-settled call. + +`_x402Retry` now returns a dedicated `X402_PAYMENT_REJECTED` sentinel only for a provably clean rejection (a non-5xx status with a readable body), and throws `NansenError` with the new `PAYMENT_AMBIGUOUS` code for anything else — a transport failure, a 5xx, or an unreadable body on either a rejection or a success. All three fallback call sites (Privy, local wallet, WalletConnect) now check against the sentinel instead of `null`, and re-throw a `PAYMENT_AMBIGUOUS` error immediately instead of silently moving on to the next provider. diff --git a/src/__tests__/api.test.js b/src/__tests__/api.test.js index a587123a..5958aa19 100644 --- a/src/__tests__/api.test.js +++ b/src/__tests__/api.test.js @@ -3469,6 +3469,131 @@ describe('NansenAPI', () => { vi.doUnmock('../walletconnect-x402.js'); }); + + // Issue #583: an ambiguous outcome after a signed payment was already + // transmitted (a 5xx, a transport failure, or an unreadable body) must + // not be treated as an ordinary rejection — the server may have already + // settled it, so signing and sending another payment would risk paying + // twice for the same logical request. + describe('ambiguous outcomes after transmission (issue #583)', () => { + it('does not fall back to WalletConnect after an ambiguous local-wallet outcome', async () => { + if (LIVE_TEST) return; + + const paymentReqs = { + accepts: [{ + scheme: 'exact', + asset: '0xUSDC', + payTo: '0xR', + amount: '1', + network: 'base', + extra: { name: 'X', version: '1', chainId: 1 }, + }], + }; + const paymentHeader = btoa(JSON.stringify(paymentReqs)); + + const errorResponse = { + ok: false, + status: 402, + json: async () => ({ message: 'Payment required' }), + headers: { get: (h) => h === 'payment-required' ? paymentHeader : null }, + }; + // The paid retry fails with a 5xx — ambiguous, not a clean rejection. + const ambiguousRetryResponse = { + ok: false, + status: 503, + json: async () => ({ error: 'internal error' }), + }; + mockFetch + .mockResolvedValueOnce(errorResponse) + .mockResolvedValueOnce(ambiguousRetryResponse); + + const mockHandleX402Payment = vi.fn().mockResolvedValue('walletconnect-sig'); + vi.resetModules(); + // Skip real wallet/crypto setup: yield one already-built local signature. + vi.doMock('../x402.js', () => ({ + createPaymentSignatures: async function* () { + yield { signature: 'local-sig', network: 'eip155:8453', asset: '0xUSDC' }; + }, + checkX402Balance: vi.fn().mockResolvedValue(null), + })); + vi.doMock('../walletconnect-x402.js', () => ({ handleX402Payment: mockHandleX402Payment })); + + const autoPayApi = new NansenAPI('test-key', 'https://api.nansen.ai'); + + let thrownError; + try { + await autoPayApi.smartMoneyNetflow({}); + } catch (err) { + thrownError = err; + } + + expect(thrownError).toBeDefined(); + expect(thrownError.code).toBe(ErrorCode.PAYMENT_AMBIGUOUS); + // Exactly the initial 402 + the one ambiguous paid retry — no second + // payment attempt via WalletConnect. + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(mockHandleX402Payment).not.toHaveBeenCalled(); + + vi.doUnmock('../x402.js'); + vi.doUnmock('../walletconnect-x402.js'); + }); + + it('propagates PAYMENT_AMBIGUOUS instead of a generic failure message when the WalletConnect paid retry is ambiguous', async () => { + if (LIVE_TEST) return; + + const paymentReqs = { + accepts: [{ + scheme: 'exact', + asset: '0xUSDC', + payTo: '0xR', + amount: '1', + network: 'base', + extra: { name: 'X', version: '1', chainId: 1 }, + }], + }; + const paymentHeader = btoa(JSON.stringify(paymentReqs)); + + const errorResponse = { + ok: false, + status: 402, + json: async () => ({ message: 'Payment required' }), + headers: { get: (h) => h === 'payment-required' ? paymentHeader : null }, + }; + const ambiguousRetryResponse = { + ok: false, + status: 503, + json: async () => ({ error: 'internal error' }), + }; + mockFetch + .mockResolvedValueOnce(errorResponse) + .mockResolvedValueOnce(ambiguousRetryResponse); + + // No local wallet configured (empty temp HOME) — falls straight + // through to WalletConnect, which signs successfully. + const mockHandleX402Payment = vi.fn().mockResolvedValue('walletconnect-sig'); + vi.resetModules(); + vi.doMock('../walletconnect-x402.js', () => ({ handleX402Payment: mockHandleX402Payment })); + + const autoPayApi = new NansenAPI('test-key', 'https://api.nansen.ai'); + + let thrownError; + try { + await autoPayApi.smartMoneyNetflow({}); + } catch (err) { + thrownError = err; + } + + expect(thrownError).toBeDefined(); + expect(thrownError.code).toBe(ErrorCode.PAYMENT_AMBIGUOUS); + expect(thrownError.message).toMatch(/not attempting another payment/i); + // Only the one paid attempt via WalletConnect — no retry loop, no + // second signature. + expect(mockHandleX402Payment).toHaveBeenCalledTimes(1); + expect(mockFetch).toHaveBeenCalledTimes(2); + + vi.doUnmock('../walletconnect-x402.js'); + }); + }); }); // =================== Smart Alert Endpoints =================== diff --git a/src/__tests__/x402-api-retry.test.js b/src/__tests__/x402-api-retry.test.js index 27ddbceb..f88ba336 100644 --- a/src/__tests__/x402-api-retry.test.js +++ b/src/__tests__/x402-api-retry.test.js @@ -12,12 +12,21 @@ * non-JSON body may not propagate cleanly through all runtime environments * without an explicit await in the async function body. * + * Issue #583 — after a signed payment was transmitted, a non-ok response no + * longer collapses everything (5xx, transport failures, unreadable bodies, + * clean rejections) into a single `null`. Only a readable, legible rejection + * body on a non-5xx status returns the X402_PAYMENT_REJECTED sentinel (safe + * to try another payment option); anything else throws NansenError with code + * PAYMENT_AMBIGUOUS, because the server may have already settled the payment + * and trying another option would risk paying twice. The sentinel is also + * distinct from a genuine successful response whose JSON body is `null`. + * * These tests pin the contract so future changes to _x402Retry are caught * before they reach CI. */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { NansenAPI, RESPONSE_META } from '../api.js'; +import { NansenAPI, RESPONSE_META, X402_PAYMENT_REJECTED, ErrorCode, NansenError } from '../api.js'; function makeApi() { return new NansenAPI('test-key', 'https://api.nansen.ai'); @@ -35,25 +44,84 @@ describe('NansenAPI._x402Retry', () => { vi.unstubAllGlobals(); }); - it('returns null when the paid response is not ok', async () => { - // Core contract: a rejected payment (non-ok retry) yields null so the - // caller can fall through to the next payment option. + it('returns the X402_PAYMENT_REJECTED sentinel for a readable, non-5xx rejection', async () => { + // Core contract: a rejection the server can prove (a legible, non-5xx + // body) is safe to treat as "try the next payment option". mockFetch.mockResolvedValue({ ok: false, status: 402, json: async () => ({ error: 'payment rejected' }), }); + const api = makeApi(); + const result = await api._x402Retry( + 'test-sig', null, null, 'https://api.nansen.ai/test', {}, + ); + expect(result).toBe(X402_PAYMENT_REJECTED); + }); + + it('throws PAYMENT_AMBIGUOUS on a 5xx after the payment was transmitted (issue #583)', async () => { + // A 5xx doesn't prove the payment was rejected — the server could have + // settled it before failing to respond. Must not be treated the same as + // a clean rejection, or the caller would sign and send another payment. + mockFetch.mockResolvedValue({ + ok: false, + status: 503, + json: async () => ({ error: 'internal error' }), + }); + + const api = makeApi(); + await expect( + api._x402Retry('test-sig', null, null, 'https://api.nansen.ai/test', {}), + ).rejects.toMatchObject({ code: ErrorCode.PAYMENT_AMBIGUOUS }); + }); + + it('throws PAYMENT_AMBIGUOUS when a non-5xx rejection body is unreadable (issue #583)', async () => { + // A 4xx we can't even parse doesn't prove a clean rejection either. + mockFetch.mockResolvedValue({ + ok: false, + status: 402, + json: vi.fn().mockRejectedValue(new SyntaxError('Unexpected token < in JSON')), + }); + + const api = makeApi(); + await expect( + api._x402Retry('test-sig', null, null, 'https://api.nansen.ai/test', {}), + ).rejects.toMatchObject({ code: ErrorCode.PAYMENT_AMBIGUOUS }); + }); + + it('throws PAYMENT_AMBIGUOUS when the fetch itself fails after transmission (issue #583)', async () => { + // The signature was already on the wire when the connection dropped — + // the server may have received and settled it. + mockFetch.mockRejectedValue(new TypeError('fetch failed')); + + const api = makeApi(); + await expect( + api._x402Retry('test-sig', null, null, 'https://api.nansen.ai/test', {}), + ).rejects.toMatchObject({ code: ErrorCode.PAYMENT_AMBIGUOUS }); + }); + + it('returns a genuine null success body as null, not the rejection sentinel (issue #583)', async () => { + // A successful (ok) response whose JSON body happens to be `null` must + // not be confused with a clean rejection — that would make the caller + // sign and transmit a second payment for an already-settled request. + mockFetch.mockResolvedValue({ + ok: true, + json: async () => null, + headers: new Map(), + }); + const api = makeApi(); const result = await api._x402Retry( 'test-sig', null, null, 'https://api.nansen.ai/test', {}, ); expect(result).toBeNull(); + expect(result).not.toBe(X402_PAYMENT_REJECTED); }); it('returns the parsed JSON body with response metadata when the paid response is ok', async () => { // Regression for e918bdd: the resolved JSON value (not a Promise) is - // returned so callers can use strict !== null to detect success. + // returned so callers can use strict !== sentinel to detect success. const responseData = { data: { token: 'ETH', value: 1234 } }; mockFetch.mockResolvedValue({ ok: true, @@ -79,7 +147,7 @@ describe('NansenAPI._x402Retry', () => { it('returns a falsy-but-valid JSON body as-is (regression for 3c25a0d)', async () => { // Before 3c25a0d the callers used `if (result)` — a valid response of // `false` would be misread as payment failure and the request would fall - // through to the next payment option. The fix uses `!== null` instead. + // through to the next payment option. The fix uses `!== sentinel` instead. mockFetch.mockResolvedValue({ ok: true, json: async () => false, @@ -89,7 +157,7 @@ describe('NansenAPI._x402Retry', () => { const result = await api._x402Retry( 'test-sig', null, null, 'https://api.nansen.ai/test', {}, ); - // false is a valid (if unusual) API response — must not be treated as null + // false is a valid (if unusual) API response — must not be treated as rejected expect(result).toBe(false); }); @@ -165,9 +233,11 @@ describe('NansenAPI._x402Retry', () => { expect(requestInit.headers['Content-Type']).toBe('application/json'); }); - it('propagates json() rejection when the paid response body is not valid JSON', async () => { - // Regression for e918bdd: the explicit await ensures a parsing failure - // surfaces as a clean rejection from _x402Retry rather than being lost. + it('throws PAYMENT_AMBIGUOUS when an ok response body is not valid JSON (issue #583)', async () => { + // Regression for e918bdd, updated for #583: the payment was accepted + // (ok) — it settled — so a parse failure now surfaces as PAYMENT_AMBIGUOUS + // rather than a bare SyntaxError, and must not be treated as a rejection + // (that would risk paying again for an already-settled request). mockFetch.mockResolvedValue({ ok: true, json: vi.fn().mockRejectedValue(new SyntaxError('Unexpected token < in JSON')), @@ -176,6 +246,19 @@ describe('NansenAPI._x402Retry', () => { const api = makeApi(); await expect( api._x402Retry('test-sig', null, null, 'https://api.nansen.ai/test', {}), - ).rejects.toThrow('Unexpected token'); + ).rejects.toMatchObject({ code: ErrorCode.PAYMENT_AMBIGUOUS }); + }); + + it('every PAYMENT_AMBIGUOUS error is a NansenError with an actionable message', async () => { + mockFetch.mockRejectedValue(new TypeError('network down')); + const api = makeApi(); + try { + await api._x402Retry('test-sig', null, null, 'https://api.nansen.ai/test', {}); + expect.unreachable('should have thrown'); + } catch (err) { + expect(err).toBeInstanceOf(NansenError); + expect(err.code).toBe(ErrorCode.PAYMENT_AMBIGUOUS); + expect(err.message).toMatch(/not attempting another payment/i); + } }); }); diff --git a/src/api.js b/src/api.js index aa14f0f9..846dceba 100644 --- a/src/api.js +++ b/src/api.js @@ -19,6 +19,16 @@ import { readResponseMeta } from './response-meta.js'; */ export const RESPONSE_META = Symbol('nansenResponseMeta'); +/** + * Sentinel returned by _x402Retry to mean "this payment option was cleanly + * rejected without settlement, safe to try the next option" — distinct from + * a genuine successful response whose JSON body happens to be `null`. + * Using `null` for both (the previous behavior) made a legitimate null-body + * success indistinguishable from a clean rejection, so the caller would sign + * and transmit ANOTHER payment for a request that had already succeeded. + */ +export const X402_PAYMENT_REJECTED = Symbol('x402PaymentRejected'); + const __dirname = path.dirname(fileURLToPath(import.meta.url)); export function telemetryHeaders() { @@ -65,6 +75,7 @@ export const ErrorCode = { // Client Errors NETWORK_ERROR: 'NETWORK_ERROR', // Connection failed TIMEOUT: 'TIMEOUT', // Request timed out + PAYMENT_AMBIGUOUS: 'PAYMENT_AMBIGUOUS', // x402 payment outcome unknown after transmission — do not retry with another payment // Generic UNKNOWN: 'UNKNOWN', // Unclassified error @@ -530,7 +541,14 @@ export class NansenAPI { /** * Retry a POST request with a payment signature. - * Returns parsed JSON if the paid request succeeds, or null if still rejected. + * Returns parsed JSON if the paid request succeeds, or the X402_PAYMENT_REJECTED + * sentinel if the server cleanly, legibly rejected it without settling. + * Throws a NansenError(PAYMENT_AMBIGUOUS) — instead of returning the sentinel — + * for any outcome that doesn't prove the payment was rejected: a transport + * failure after transmission, an HTTP 5xx, or a response body that can't be + * parsed. In all of those cases the server may already have received and + * settled the payment, so the caller must not treat it as safe to retry with + * a different option — that would risk paying twice for the same request. * Logs the payment and warns about low balance when walletLabel and network are given. * * @param {string} signature - Payment-Signature header value @@ -539,7 +557,7 @@ export class NansenAPI { * @param {string} url - Request URL * @param {object} body - Request body (will be cleaned) * @param {object} [options={}] - Request options (may include .method, .headers) - * @returns {Promise} Parsed JSON on success, null if rejected + * @returns {Promise} * * TODO: full fix — extract the entire x402 provider dispatch from request() into * an attemptX402Payment() method so adding a new payment provider only requires @@ -550,21 +568,52 @@ export class NansenAPI { // POST burned a payment signature then hit the wrong route for GET/DELETE/PATCH. const method = options.method || 'POST'; const isGet = method === 'GET'; - const paidResponse = await fetch(url, { - method, - redirect: 'error', - headers: { - ...(!isGet && { 'Content-Type': 'application/json' }), - 'X-Client-Type': 'nansen-cli', - 'X-Client-Version': packageVersion, - ...telemetryHeaders(), - 'Payment-Signature': signature, - ...this.defaultHeaders, - ...options.headers, - }, - ...(!isGet && method !== 'DELETE' && { body: JSON.stringify(NansenAPI.cleanBody(body)) }), - }); - if (!paidResponse.ok) return null; + let paidResponse; + try { + paidResponse = await fetch(url, { + method, + redirect: 'error', + headers: { + ...(!isGet && { 'Content-Type': 'application/json' }), + 'X-Client-Type': 'nansen-cli', + 'X-Client-Version': packageVersion, + ...telemetryHeaders(), + 'Payment-Signature': signature, + ...this.defaultHeaders, + ...options.headers, + }, + ...(!isGet && method !== 'DELETE' && { body: JSON.stringify(NansenAPI.cleanBody(body)) }), + }); + } catch (err) { + // The signature was already on the wire when the connection failed — + // the server may have received and settled it before we lost the + // response. Fail closed rather than let the caller sign and send a + // second payment for the same logical request. + throw new NansenError( + `x402 payment outcome unknown: request failed after the signed payment was transmitted (${err.message}). Not attempting another payment for the same request.`, + ErrorCode.PAYMENT_AMBIGUOUS, + ); + } + if (!paidResponse.ok) { + // A 5xx doesn't prove the payment was rejected — the server could have + // processed it before failing to respond. Only a readable non-5xx + // rejection body is safe to treat as "try the next option". + if (paidResponse.status >= 500) { + throw new NansenError( + `x402 payment outcome unknown: server returned ${paidResponse.status} after the signed payment was transmitted. Not attempting another payment for the same request.`, + ErrorCode.PAYMENT_AMBIGUOUS, + ); + } + try { + await paidResponse.json(); + } catch (err) { + throw new NansenError( + `x402 payment outcome unknown: rejection response body was unreadable (${err.message}). Not attempting another payment for the same request.`, + ErrorCode.PAYMENT_AMBIGUOUS, + ); + } + return X402_PAYMENT_REJECTED; + } if (walletLabel) { console.error(`[x402] Paid via ${walletLabel}${network ? ` (${network})` : ''}`); } @@ -577,7 +626,18 @@ export class NansenAPI { } } catch { /* balance check is best-effort */ } } - const data = await paidResponse.json(); + let data; + try { + data = await paidResponse.json(); + } catch (err) { + // The payment was accepted (2xx) — it settled. We just can't read the + // response body, so surface that plainly rather than silently treating + // it as a rejection and paying again. + throw new NansenError( + `x402 payment succeeded but its response body was unreadable (${err.message}). The payment was not repeated.`, + ErrorCode.PAYMENT_AMBIGUOUS, + ); + } const meta = readResponseMeta(paidResponse); this.lastResponseMeta = meta; if (meta && data !== null && typeof data === 'object') data[RESPONSE_META] = meta; @@ -712,9 +772,14 @@ export class NansenAPI { const { createPrivyPaymentSignatures } = await import('./privy.js'); for await (const { signature, network } of createPrivyPaymentSignatures(response, url)) { const result = await this._x402Retry(signature, `Privy wallet ${defaultWalletName}`, network, url, body, options); - if (result !== null) return result; + if (result !== X402_PAYMENT_REJECTED) return result; } } catch (privyErr) { + // An ambiguous outcome (transport failure, 5xx, unreadable body) + // after a signed payment was already transmitted must not be + // treated as an ordinary payment failure — there is no other + // provider to fall back to here, and retrying could double-pay. + if (privyErr instanceof NansenError && privyErr.code === ErrorCode.PAYMENT_AMBIGUOUS) throw privyErr; message = `x402 Privy payment failed: ${privyErr.message}`; } } else { @@ -724,10 +789,17 @@ export class NansenAPI { const { createPaymentSignatures } = await import('./x402.js'); for await (const { signature, network, asset } of createPaymentSignatures(response, url)) { const result = await this._x402Retry(signature, `local wallet ${defaultWalletName}`, network, url, body, options, asset); - if (result !== null) return result; - // This payment option was rejected, try next + if (result !== X402_PAYMENT_REJECTED) return result; + // This payment option was cleanly rejected without settling, try next } - } catch { /* local wallet unavailable, try WalletConnect */ } + } catch (localErr) { + // An ambiguous outcome here means a signed payment may already + // be in flight or settled server-side. Do NOT fall through to + // WalletConnect below — that would sign and transmit a second, + // independent payment authorization for the same request. + if (localErr instanceof NansenError && localErr.code === ErrorCode.PAYMENT_AMBIGUOUS) throw localErr; + /* local wallet unavailable for any other reason, try WalletConnect */ + } // 2. Fall back to WalletConnect (walletconnect-x402.js) { @@ -749,8 +821,13 @@ export class NansenAPI { const { handleX402Payment } = await import('./walletconnect-x402.js'); const paymentSignature = await handleX402Payment(paymentRequirements); const result = await this._x402Retry(paymentSignature, 'WalletConnect', null, url, body, options); - if (result !== null) return result; + if (result !== X402_PAYMENT_REJECTED) return result; } catch (x402Err) { + // WalletConnect is the last resort in this chain — an + // ambiguous outcome here still must not be reported as an + // ordinary "payment failed" that invites the caller to + // retry the whole request (and sign yet another payment). + if (x402Err instanceof NansenError && x402Err.code === ErrorCode.PAYMENT_AMBIGUOUS) throw x402Err; if (!this.apiKey) { message = 'No API key configured. Three ways to authenticate:\n' + ' 1. API key: run `nansen login --human` or set NANSEN_API_KEY (get key at https://app.nansen.ai/auth/agent-setup)\n' +