diff --git a/typescript/src/compat/mpp.ts b/typescript/src/compat/mpp.ts index bdd114d..7deefd4 100644 --- a/typescript/src/compat/mpp.ts +++ b/typescript/src/compat/mpp.ts @@ -539,6 +539,75 @@ export function decodeMppCredential( */ const BLOCKCHAIN_CHARGE_METHODS = new Set(['tempo', 'evm', 'solana', 'lightning', 'stellar']); +/** + * Methods whose OWN spec elevates `recipient` to REQUIRED. + * + * `draft-payment-intent-charge-00` §Shared Fields lists `recipient` under + * OPTIONAL Fields, and notes only in passing that "Payment methods MAY elevate + * OPTIONAL fields to REQUIRED in their method specification (e.g. `recipient` + * and `expires` are REQUIRED for blockchain methods)". That parenthetical is an + * ILLUSTRATION OF A MECHANISM, not a blanket rule — and the mechanism it + * describes is per-method. Reading it as a rule over the whole blockchain set + * inverts the spec: it lets a prose example in the core document override the + * method specs that are actually normative. + * + * Requirement levels below are read from each method's own request schema + * (tempoxyz/mpp-specs @ f9506cd): + * + * evm REQUIRED draft-evm-charge-00.md:264 + * tempo REQUIRED draft-tempo-charge-00.md:155 + * stellar REQUIRED draft-stellar-charge-00.md:258 + * solana REQUIRED draft-solana-charge-00.md + * lightning OPTIONAL draft-lightning-charge-00.md:206 — "Lightning + * implementations typically do not use this field; + * the invoice payee is implied by the BOLT11 invoice." + * + * Lightning is the lone exception among the five, which is why the blanket rule + * survived: it was correct four times out of five. Any method added to + * {@link BLOCKCHAIN_CHARGE_METHODS} must be classified here from its own spec. + */ +const RECIPIENT_REQUIRED_METHODS = new Set(['tempo', 'evm', 'solana', 'stellar']); + +/** + * Resolve the s402 `payTo` for a Charge request, per method. + * + * Every branch either returns a real, payable destination or throws. There is + * deliberately no path that yields an empty string: requirements carrying + * `payTo: ""` would look structurally valid and could never be settled, which + * is strictly worse than a rejection at the boundary. + */ +function resolvePayTo( + method: string, + request: { recipient?: string; methodDetails?: Record }, +): string { + if (typeof request.recipient === 'string' && request.recipient.length > 0) { + return request.recipient; + } + + if (RECIPIENT_REQUIRED_METHODS.has(method)) { + throw new s402Error('INVALID_PAYLOAD', + `MPP "${method}" Charge request is missing "recipient" — the ${method} method spec's ` + + `own request schema marks it REQUIRED (mpp-specs draft-${method}-charge-00)`); + } + + if (method === 'lightning') { + // draft-lightning-charge-00 §Method Details: `invoice` is REQUIRED and + // "This field is authoritative; all other payment parameters are derived + // from it." The payee is implied by the BOLT11 invoice, so the invoice IS + // the payment destination when `recipient` is absent. + const invoice = request.methodDetails?.invoice; + if (typeof invoice === 'string' && invoice.length > 0) return invoice; + throw new s402Error('INVALID_PAYLOAD', + 'MPP "lightning" Charge request has neither "recipient" (OPTIONAL per ' + + 'draft-lightning-charge-00) nor "methodDetails.invoice" (REQUIRED) — ' + + 'no payment destination can be derived'); + } + + throw new s402Error('INVALID_PAYLOAD', + `MPP "${method}" Charge request is missing "recipient" and the method has no ` + + `documented fallback destination — cannot resolve payTo`); +} + /** * Network identifier resolution for MPP Charge requests per method. * @@ -571,9 +640,9 @@ function resolveNetwork(method: string, methodDetails: Record | * requires — keep those on the MPP path. * * @throws {s402Error} `INVALID_PAYLOAD` if the method is not a known - * blockchain-style Charge method, if the request is missing a recipient - * (REQUIRED for blockchain methods per charge spec), or if the challenge - * has expired at `now`. + * blockchain-style Charge method, if no `payTo` can be resolved for that + * method (see {@link resolvePayTo} — `recipient` is REQUIRED per METHOD, not + * across the whole set), or if the challenge has expired at `now`. */ export function fromMppChargeChallenge( challenge: MppChallenge, @@ -590,10 +659,7 @@ export function fromMppChargeChallenge( } const request = decodeMppChargeRequest(challenge); - if (typeof request.recipient !== 'string' || request.recipient.length === 0) { - throw new s402Error('INVALID_PAYLOAD', - 'Blockchain Charge request missing "recipient" — required by charge-intent spec for blockchain methods'); - } + const payTo = resolvePayTo(challenge.method, request); let expiresAt: number | undefined; if (challenge.expires) { @@ -615,7 +681,7 @@ export function fromMppChargeChallenge( network: resolveNetwork(challenge.method, request.methodDetails), asset: request.currency, amount: request.amount, - payTo: request.recipient, + payTo, expiresAt, extensions: { mpp: { @@ -670,12 +736,12 @@ export interface ToMppChargeInput { /** * Build an {@link MppChargeRequest} from direct input. The shape follows * `draft-payment-intent-charge-00` §Request Schema: `amount` + `currency` are - * REQUIRED across every method; `recipient` is REQUIRED for blockchain - * methods and OPTIONAL for processor methods; `methodDetails` carries - * method-specific extension data. + * REQUIRED across every method; `recipient` is REQUIRED only where a METHOD + * spec elevates it (see {@link RECIPIENT_REQUIRED_METHODS} — Lightning does + * not); `methodDetails` carries method-specific extension data. * - * @throws {s402Error} `INVALID_PAYLOAD` for malformed amount or for missing - * recipient on a known blockchain method. + * @throws {s402Error} `INVALID_PAYLOAD` for malformed amount, or for a missing + * recipient on a method whose own spec marks it REQUIRED. */ export function toMppChargeRequest(input: ToMppChargeInput): MppChargeRequest { if (!isValidAmount(input.amount)) { @@ -686,9 +752,15 @@ export function toMppChargeRequest(input: ToMppChargeInput): MppChargeRequest { throw new s402Error('INVALID_PAYLOAD', 'MPP Charge "currency" is required and must be a string'); } const method = normalizeChargeMethod(input.method); - if (BLOCKCHAIN_CHARGE_METHODS.has(method) && (!input.recipient || typeof input.recipient !== 'string')) { + // The write-path twin of the same defect (DAN-854). Keyed off the blanket + // blockchain set, this refused to EMIT a spec-legal Lightning charge — one + // carrying an authoritative BOLT11 invoice and no recipient. Same misreading, + // opposite direction, so it shares the one table rather than growing a second. + if (RECIPIENT_REQUIRED_METHODS.has(method) && (!input.recipient || typeof input.recipient !== 'string')) { throw new s402Error('INVALID_PAYLOAD', - `MPP method "${method}" is a blockchain method and requires "recipient" — processor methods (stripe, card) route internally and may omit it`); + `MPP method "${method}" requires "recipient" — its own method spec marks it REQUIRED ` + + `(mpp-specs draft-${method}-charge-00). Lightning omits it by design (the BOLT11 invoice ` + + `implies the payee); processor methods (stripe, card) route internally.`); } const request: MppChargeRequest = { diff --git a/typescript/test/compat-mpp-recipient-optionality.test.ts b/typescript/test/compat-mpp-recipient-optionality.test.ts new file mode 100644 index 0000000..cdd3ab2 --- /dev/null +++ b/typescript/test/compat-mpp-recipient-optionality.test.ts @@ -0,0 +1,119 @@ +/** + * DAN-854 — `recipient` is REQUIRED per METHOD, not across the whole + * blockchain-method set. + * + * The defect: `fromMppChargeChallenge` rejected every Charge request lacking + * `recipient`, citing the charge-intent spec. That spec lists `recipient` under + * OPTIONAL Fields and only notes, parenthetically, that a method spec MAY + * elevate it. The elevation is per-method — and Lightning exercises the + * mechanism in the opposite direction. + * + * Requirement levels read from tempoxyz/mpp-specs @ f9506cd, each from that + * method's own Shared Fields table or field list: + * + * evm REQUIRED specs/methods/evm/draft-evm-charge-00.md:264 + * tempo REQUIRED specs/methods/tempo/draft-tempo-charge-00.md:155 + * stellar REQUIRED specs/methods/stellar/draft-stellar-charge-00.md:258 + * solana REQUIRED specs/methods/solana/draft-solana-charge-00.md + * lightning OPTIONAL specs/methods/lightning/draft-lightning-charge-00.md:206 + * + * Lightning is the lone exception among the five, which is exactly why a + * blanket rule survived review: it is correct four times out of five. + */ +import { describe, it, expect } from 'vitest'; +import { fromMppChargeChallenge, type MppChallenge } from '../src/compat/mpp.js'; +import { s402Error } from '../src/errors.js'; + +function base64url(input: string): string { + const b64 = Buffer.from(input, 'utf-8').toString('base64'); + return b64.replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, ''); +} + +function challenge(method: string, request: unknown): MppChallenge { + return { + id: 'kM9xPqWvT2nJrHsY4aDfEb', + realm: 'api.example.com', + method, + intent: 'charge', + request: base64url(JSON.stringify(request)), + } as MppChallenge; +} + +/** + * draft-lightning-charge-00.md § Examples, "Decoded `request`", VERBATIM. + * Note what is absent: there is no `recipient` key. That is the whole ticket. + */ +const LIGHTNING_SPEC_EXAMPLE = { + amount: '100', + currency: 'sat', + description: 'Weather report for 94107', + methodDetails: { + invoice: 'lnbc1u1p...', + paymentHash: 'bc230847...', + network: 'mainnet', + }, +}; + +describe('DAN-854 · lightning: recipient is OPTIONAL', () => { + it("accepts the Lightning spec's own charge example, unmodified", () => { + expect(() => fromMppChargeChallenge(challenge('lightning', LIGHTNING_SPEC_EXAMPLE))) + .not.toThrow(); + }); + + it('maps payTo to the BOLT11 invoice, which the spec calls authoritative', () => { + // draft-lightning-charge-00.md:231-234 — `invoice` is REQUIRED and "This + // field is authoritative; all other payment parameters are derived from + // it", with the payee "implied by the BOLT11 invoice". So the invoice is + // the honest payment destination when `recipient` is absent. + const req = fromMppChargeChallenge(challenge('lightning', LIGHTNING_SPEC_EXAMPLE)); + expect(req.payTo).toBe('lnbc1u1p...'); + }); + + it('NEVER emits an empty payTo — an unpayable destination must throw instead', () => { + // The failure mode this ticket explicitly forbids. Without `recipient` AND + // without `invoice` there is no destination, and silently emitting "" would + // produce requirements that look valid and can never be settled. + const noDestination = { amount: '100', currency: 'sat', methodDetails: { network: 'mainnet' } }; + expect(() => fromMppChargeChallenge(challenge('lightning', noDestination))) + .toThrow(s402Error); + }); + + it('still prefers an explicit recipient when Lightning supplies one', () => { + // The non-canonical shape. `recipient` is OPTIONAL, not forbidden, so a + // request carrying one must not have it silently discarded in favour of the + // invoice. + const withRecipient = { ...LIGHTNING_SPEC_EXAMPLE, recipient: '03abc...node' }; + const req = fromMppChargeChallenge(challenge('lightning', withRecipient)); + expect(req.payTo).toBe('03abc...node'); + }); +}); + +describe('DAN-854 · methods whose own spec makes recipient REQUIRED', () => { + const REQUIRED_METHODS: Array<[string, Record]> = [ + ['evm', { amount: '1000000', currency: '0xA0b86991c62181', methodDetails: { chainId: 8453 } }], + ['tempo', { amount: '1000000', currency: '0x20c0000000000000', methodDetails: { chainId: 42431 } }], + ['solana', { amount: '1000000', currency: 'EPjFWdd5AufqSSqeM2q', methodDetails: {} }], + ['stellar', { amount: '10000000', currency: 'CBIELTK6YBZJU5UP2WWQ', methodDetails: {} }], + ]; + + for (const [method, request] of REQUIRED_METHODS) { + it(`${method}: rejects a charge with no recipient`, () => { + expect(() => fromMppChargeChallenge(challenge(method, request))).toThrow(s402Error); + }); + } + + it('names the METHOD spec as the authority, not the charge-intent spec', () => { + // Criterion 3. The old message read "required by charge-intent spec for + // blockchain methods" — citing, as the authority for a per-method rule, the + // one document that lists the field as OPTIONAL. A wrong reason is what the + // next agent acts on. + let message = ''; + try { + fromMppChargeChallenge(challenge('evm', REQUIRED_METHODS[0][1])); + } catch (e) { + message = (e as Error).message; + } + expect(message).toContain('evm'); + expect(message).not.toContain('charge-intent spec'); + }); +});