From 9f2d77c5e977050d21b63cd8c06946c73b758655 Mon Sep 17 00:00:00 2001 From: Daniel Ahn Date: Sun, 16 Aug 2026 16:04:56 -0700 Subject: [PATCH] fix(DAN-854): recipient is REQUIRED per METHOD, not across the blockchain set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fromMppChargeChallenge rejected every Charge request without "recipient", citing the charge-intent spec as its authority. That spec lists recipient under OPTIONAL Fields; the line the message leaned on is a parenthetical illustrating that a method spec MAY elevate it. The mechanism is per-method, and Lightning exercises it in the other direction. Read from each method spec at mpp-specs f9506cd: evm REQUIRED · tempo REQUIRED · solana REQUIRED · stellar REQUIRED lightning OPTIONAL — "the invoice payee is implied by the BOLT11 invoice" Lightning is the lone exception among the five, which is why a blanket rule survived: it is correct four times out of five. The Lightning drafts own canonical example threw before reaching resolveNetwork. payTo for Lightning resolves to methodDetails.invoice, which the spec calls authoritative and from which all other payment parameters derive. Every branch of resolvePayTo returns a payable destination or throws — there is no path that emits an empty string, because requirements carrying payTo:"" look valid and can never settle. Fixes the write-path twin in the same change and off the same table: toMppChargeRequest used the identical blanket set, so it refused to EMIT a spec-legal Lightning charge. Same misreading, opposite direction. Error messages now name the method spec that actually requires the field. The wrong reason is what the next agent acts on. Criterion 5, confirmed not changed: the misreading did not reach "expires". Every reference is to challenge.expires, the auth-param, never request.expires — which is what the charge spec requires. Gates: tsc --noEmit clean · 1111 tests / 29 files pass. No Python MPP compat module exists, so there is no cross-language twin (checked, not assumed). --- typescript/src/compat/mpp.ts | 102 ++++++++++++--- .../compat-mpp-recipient-optionality.test.ts | 119 ++++++++++++++++++ 2 files changed, 206 insertions(+), 15 deletions(-) create mode 100644 typescript/test/compat-mpp-recipient-optionality.test.ts 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'); + }); +});