From a0fd376e91a0015c622674e69a12dbb75cda1c38 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Thu, 24 Sep 2026 15:57:40 -0400 Subject: [PATCH 1/5] Commit one terminal invoice outcome for cancel, verify, and monitor Closes #558. Legal terminal transitions live in the settlement domain and both storages enforce them. Cancel is PENDING to CANCELLED only; a loser gets a stable conflict code and does not clear payment_tx_hash. Verify and the monitor share the payment-claim lock, including muxed destinations that resolve to the seller. Late CANCELLED/EXPIRED to PAID settlement stays the AFTER_CANCEL / AFTER_EXPIRY path. Cancel proof stays Freighter-only: sellerPublicKey and signature over cancel: in the JSON body. Query and header seller keys are rejected even when they match. The pay page treats CANCELLED as terminal so both surfaces show the status storage committed. --- backend/src/domain/invoice-settlement.ts | 105 +++++++++++++++ backend/src/routes/invoice.handlers.ts | 47 ++++--- .../src/services/invoice-memory.service.ts | 1 + backend/src/services/invoice.service.ts | 27 ++-- backend/src/storage/memory-storage.ts | 26 ++-- .../tests/invoice-cancel-payment-race.test.ts | 120 +++++++++++++++++- backend/tests/invoice-handlers.test.ts | 22 ++-- .../tests/invoice-terminal-settlement.test.ts | 56 ++++++++ docs/EXPIRY-AND-LATE-PAYMENT.md | 17 +++ frontend/lib/payment-page-state.d.ts | 2 +- frontend/lib/payment-page-state.js | 24 +++- frontend/tests/payment-page-state.test.js | 26 +++- frontend/tests/verification.test.js | 1 + shared/verification.ts | 2 + 14 files changed, 411 insertions(+), 65 deletions(-) create mode 100644 backend/tests/invoice-terminal-settlement.test.ts diff --git a/backend/src/domain/invoice-settlement.ts b/backend/src/domain/invoice-settlement.ts index 6b927531..a61ad030 100644 --- a/backend/src/domain/invoice-settlement.ts +++ b/backend/src/domain/invoice-settlement.ts @@ -89,3 +89,108 @@ export function settlementFieldsForInvoice( latePaymentWarningCode: afterExpiry ? 'PAYMENT_RECEIVED_AFTER_EXPIRY' : undefined, }; } + +/** Outcomes cancel / verify / monitor may commit as the invoice's terminal status. */ +export type TerminalOutcome = 'PAID' | 'CANCELLED'; + +/** Stable codes when a terminal commit loses the race or is otherwise illegal. */ +export type TerminalConflictCode = + | 'INVOICE_ALREADY_PAID' + | 'INVOICE_ALREADY_CANCELLED' + | 'INVOICE_EXPIRED' + | 'INVOICE_NOT_PENDING'; + +const TERMINAL_CONFLICT_MESSAGES: Record = { + INVOICE_ALREADY_PAID: 'Invoice has already been paid', + INVOICE_ALREADY_CANCELLED: 'Invoice has already been cancelled', + INVOICE_EXPIRED: 'Invoice has expired and can no longer accept payment', + INVOICE_NOT_PENDING: 'Invoice is not pending', +}; + +/** + * Legal status transitions that decide a terminal invoice outcome. + * + * PAID is hard-terminal: nothing leaves it (idempotent PAID replay is not a + * transition). CANCELLED and EXPIRED may still settle to PAID when an exact + * on-chain payment is attributed — that is the late-settlement path from + * LATE_PAYMENT_POLICY, not a concurrent cancel overwrite. Cancel itself is + * PENDING → CANCELLED only. + */ +export const LEGAL_TERMINAL_TRANSITIONS: ReadonlyArray = [ + ['PENDING', 'CANCELLED'], + ['PENDING', 'PAID'], + ['EXPIRED', 'PAID'], + ['CANCELLED', 'PAID'], +]; + +export function isLegalTerminalTransition(from: InvoiceStatus, to: InvoiceStatus): boolean { + if (from === to && to === 'PAID') { + // Idempotent replay of the same paid invoice is allowed at the claim layer; + // it is not a storage status rewrite. + return false; + } + return LEGAL_TERMINAL_TRANSITIONS.some(([a, b]) => a === from && b === to); +} + +export function messageForTerminalConflict(code: TerminalConflictCode): string { + return TERMINAL_CONFLICT_MESSAGES[code]; +} + +/** + * Raised when cancel or settlement refuses because another terminal outcome + * already won. Carries the stable code handlers return to clients. + */ +export class InvoiceTerminalConflictError extends Error { + readonly code: TerminalConflictCode; + readonly currentStatus: InvoiceStatus; + /** Present when the losing cancel must not clear an already-recorded hash. */ + readonly paymentTxHash?: string; + + constructor(code: TerminalConflictCode, currentStatus: InvoiceStatus, paymentTxHash?: string) { + super(messageForTerminalConflict(code)); + this.name = 'InvoiceTerminalConflictError'; + this.code = code; + this.currentStatus = currentStatus; + this.paymentTxHash = paymentTxHash; + } +} + +/** Map a non-PENDING status onto the cancel-loser conflict code. */ +export function cancelConflictForStatus( + status: InvoiceStatus, + paymentTxHash?: string | null +): InvoiceTerminalConflictError { + if (status === 'PAID') { + return new InvoiceTerminalConflictError('INVOICE_ALREADY_PAID', status, paymentTxHash ?? undefined); + } + if (status === 'CANCELLED') { + return new InvoiceTerminalConflictError('INVOICE_ALREADY_CANCELLED', status, paymentTxHash ?? undefined); + } + if (status === 'EXPIRED') { + return new InvoiceTerminalConflictError('INVOICE_EXPIRED', status, paymentTxHash ?? undefined); + } + return new InvoiceTerminalConflictError('INVOICE_NOT_PENDING', status, paymentTxHash ?? undefined); +} + +/** + * Guard used by both storages before PENDING → CANCELLED. Throws a typed + * conflict when cancel lost the race (or the invoice was never pending). + */ +export function assertCancelTransitionAllowed( + status: InvoiceStatus, + paymentTxHash?: string | null +): void { + if (status === 'PENDING') return; + throw cancelConflictForStatus(status, paymentTxHash); +} + +/** + * Guard used by both storages before writing PAID. Replay of an already-PAID + * invoice is handled by the caller (return existing / already-processed); + * every other illegal source status throws. + */ +export function assertPaidTransitionAllowed(status: InvoiceStatus): void { + if (status === 'PAID') return; + if (isLegalTerminalTransition(status, 'PAID')) return; + throw new InvoiceTerminalConflictError('INVOICE_NOT_PENDING', status); +} diff --git a/backend/src/routes/invoice.handlers.ts b/backend/src/routes/invoice.handlers.ts index 036bbdde..04430b43 100644 --- a/backend/src/routes/invoice.handlers.ts +++ b/backend/src/routes/invoice.handlers.ts @@ -35,6 +35,7 @@ import { import { PaymentClaimError } from '../domain/payment-attribution'; import { SettlementTimeUnavailableError, + InvoiceTerminalConflictError, warningForLatePayment, } from '../domain/invoice-settlement'; import { cutoverDrainMode, simulationAllowed } from '../config/runtime'; @@ -345,10 +346,9 @@ export function createInvoiceHandlers(options: InvoiceHandlerOptions): InvoiceHa async cancelInvoice(req: Request, res: Response): Promise { try { - // Issue #517 — one proof path: the seller key, signature and message - // all travel in the JSON body. Query params and headers are legacy - // transports; when they carry a key that disagrees with the body the - // request is ambiguous and fails closed. + // Issue #558 / #517 — one proof path: seller key, signature, and + // `cancel:` message all travel in the JSON body. Query and + // header seller keys are rejected even when they match the body. const bodyKeyRaw = req.body?.sellerPublicKey; const queryKeyRaw = req.query?.sellerPublicKey; const headerKeyRaw = req.headers?.['x-seller-public-key']; @@ -356,6 +356,14 @@ export function createInvoiceHandlers(options: InvoiceHandlerOptions): InvoiceHa .flat() .filter((v): v is string => typeof v === 'string' && v.trim() !== ''); + if (alternates.length > 0) { + return sendFailure( + res, + 400, + 'Send sellerPublicKey in the request body; query and header keys are not accepted' + ); + } + let sellerPublicKey: string | undefined; if (typeof bodyKeyRaw === 'string' && bodyKeyRaw.trim() !== '') { const parsed = stellarPublicKeySchema.safeParse(bodyKeyRaw); @@ -365,26 +373,6 @@ export function createInvoiceHandlers(options: InvoiceHandlerOptions): InvoiceHa sellerPublicKey = parsed.data; } - if (alternates.length > 0) { - if (!sellerPublicKey) { - return sendFailure( - res, - 400, - 'Send sellerPublicKey in the request body; query and header keys are not accepted' - ); - } - for (const alt of alternates) { - const parsed = stellarPublicKeySchema.safeParse(alt); - if (!parsed.success || parsed.data !== sellerPublicKey) { - return sendFailure( - res, - 400, - 'Conflicting sellerPublicKey values between body, query, and header' - ); - } - } - } - if (!sellerPublicKey) { return sendFailure( res, @@ -444,6 +432,17 @@ export function createInvoiceHandlers(options: InvoiceHandlerOptions): InvoiceHa sendSuccess(res, 200, invoice); } catch (error: any) { logError('Cancel invoice error:', error); + if (error instanceof InvoiceTerminalConflictError) { + // Stable code for the race loser; payment_tx_hash is left intact. + res.status(409).json({ + success: false, + code: error.code, + error: error.message, + ...(error.paymentTxHash ? { paymentTxHash: error.paymentTxHash } : {}), + ...(error.currentStatus ? { status: error.currentStatus } : {}), + }); + return; + } const message = error.message || 'Failed to cancel invoice'; const lowerMessage = message.toLowerCase(); const isSellerMismatch = lowerMessage.includes('only the seller can cancel'); diff --git a/backend/src/services/invoice-memory.service.ts b/backend/src/services/invoice-memory.service.ts index f6725b34..1615bf07 100644 --- a/backend/src/services/invoice-memory.service.ts +++ b/backend/src/services/invoice-memory.service.ts @@ -174,6 +174,7 @@ export class InvoiceMemoryService { } async cancelInvoice(invoiceId: string, sellerPublicKey?: string): Promise { + // MemoryStorage throws InvoiceTerminalConflictError when cancel loses the race. const updated = this.storage.cancelInvoice(invoiceId, sellerPublicKey); if (!updated) { throw new Error('Invoice not found or already processed'); diff --git a/backend/src/services/invoice.service.ts b/backend/src/services/invoice.service.ts index 96b3cfe4..a21412d0 100644 --- a/backend/src/services/invoice.service.ts +++ b/backend/src/services/invoice.service.ts @@ -8,6 +8,7 @@ import { calculateInvoiceExpiry } from '../domain/invoice-expiry'; import { PaymentClaimError } from '../domain/payment-attribution'; import { SettlementTimeUnavailableError, + cancelConflictForStatus, type LatePaymentWarningCode, type SettlementContext, } from '../domain/invoice-settlement'; @@ -352,17 +353,23 @@ export class InvoiceService { const result = await this.db.query(query, [invoiceId, sellerPublicKey || null]); if (result.rows.length === 0) { - if (sellerPublicKey) { - const existing = await this.db.query('SELECT * FROM invoices WHERE id = $1', [invoiceId]); - if ( - existing.rows.length > 0 && - existing.rows[0].status === 'PENDING' && - existing.rows[0].seller_public_key !== sellerPublicKey - ) { - throw new Error('Unauthorized: only the seller can cancel this invoice'); - } + const existing = await this.db.query('SELECT * FROM invoices WHERE id = $1', [invoiceId]); + if (existing.rows.length === 0) { + throw new Error('Invoice not found or already processed'); + } + const row = existing.rows[0]; + if ( + sellerPublicKey && + row.status === 'PENDING' && + row.seller_public_key !== sellerPublicKey + ) { + throw new Error('Unauthorized: only the seller can cancel this invoice'); } - throw new Error('Invoice not found or already processed'); + // Another terminal outcome already won — do not clear payment_tx_hash. + throw cancelConflictForStatus( + row.status, + row.payment_tx_hash ?? undefined + ); } return this.mapRowToInvoice(result.rows[0]); diff --git a/backend/src/storage/memory-storage.ts b/backend/src/storage/memory-storage.ts index 2ba8fb12..31505b88 100644 --- a/backend/src/storage/memory-storage.ts +++ b/backend/src/storage/memory-storage.ts @@ -2,7 +2,12 @@ import { v4 as uuidv4 } from 'uuid'; import { calculateInvoiceStats } from './invoice-stats'; import type { InvoiceStats } from './invoice-stats'; import { isPendingInvoiceExpired } from '../domain/invoice-expiry'; -import { settlementFieldsForInvoice } from '../domain/invoice-settlement'; +import { + assertCancelTransitionAllowed, + assertPaidTransitionAllowed, + settlementFieldsForInvoice, + InvoiceTerminalConflictError, +} from '../domain/invoice-settlement'; import { InvoiceIdCollisionError, MemoCollisionError, @@ -123,14 +128,17 @@ class MemoryStorage { return updated; } - // Cancel invoice + // Cancel invoice — PENDING → CANCELLED only; loser gets a typed terminal conflict. cancelInvoice(id: string, sellerPublicKey?: string): Invoice | undefined { this.markExpiredInvoices(); const invoice = this.invoices.get(id); - if (!invoice || invoice.status !== 'PENDING') return undefined; + if (!invoice) return undefined; if (sellerPublicKey && invoice.sellerPublicKey !== sellerPublicKey) { throw new Error('Unauthorized: only the seller can cancel this invoice'); } + // Throws InvoiceTerminalConflictError when PAID/CANCELLED/EXPIRED already won. + // Does not clear paymentTxHash on a lost race. + assertCancelTransitionAllowed(invoice.status, invoice.paymentTxHash); return this.updateInvoice(id, { status: 'CANCELLED', cancelledAt: new Date() }); } @@ -145,14 +153,10 @@ class MemoryStorage { this.markExpiredInvoices(); const now = new Date(); const invoice = this.invoices.get(id); - if (!invoice || invoice.status === 'PAID') return undefined; - if ( - invoice.status !== 'PENDING' && - invoice.status !== 'CANCELLED' && - invoice.status !== 'EXPIRED' - ) { - return undefined; - } + if (!invoice) return undefined; + // Idempotent: already PAID is "already processed" for the caller, not a rewrite. + if (invoice.status === 'PAID') return undefined; + assertPaidTransitionAllowed(invoice.status); // Settlement time must come from the ledger close time; never invent one. const settlement = settlementFieldsForInvoice(invoice, options.settledAt); diff --git a/backend/tests/invoice-cancel-payment-race.test.ts b/backend/tests/invoice-cancel-payment-race.test.ts index 45e56cb8..3adc6fba 100644 --- a/backend/tests/invoice-cancel-payment-race.test.ts +++ b/backend/tests/invoice-cancel-payment-race.test.ts @@ -398,7 +398,10 @@ function runManualVerifySuite(name: string, createStorage: () => InvoiceStorage) createReq({ params: { id: invoice.id }, body: { sellerPublicKey: SELLER } }) ); - assert.equal(cancelled.statusCode, 400); + assert.equal(cancelled.statusCode, 409, JSON.stringify(cancelled.body)); + assert.equal(cancelled.body.code, 'INVOICE_ALREADY_PAID'); + assert.equal(cancelled.body.status, 'PAID'); + assert.equal(cancelled.body.paymentTxHash, TX_HASH); const stored = await storage.getInvoiceById(invoice.id); assert.equal(stored?.status, 'PAID'); assert.equal(stored?.paymentTxHash, TX_HASH); @@ -603,6 +606,121 @@ describe('cancel versus payment monitor attribution on memory storage', () => { runManualVerifySuite('in-memory storage', createMemoryStorage); runManualVerifySuite('postgres storage double', createPostgresStorage); + +describe('cancel versus monitor attribution race (issue #558)', () => { + let rawStorage: MemoryStorage; + let invoiceService: InvoiceMemoryService; + + beforeEach(() => { + rawStorage = new MemoryStorage(); + invoiceService = new InvoiceMemoryService(rawStorage); + }); + + it('monitor attribution after payment wins leaves cancel rejected without clearing the hash', async () => { + const invoice = rawStorage.createInvoice({ + sellerPublicKey: SELLER, + amount: 42.5, + assetCode: 'XLM', + memo: 'INV-MONITOR-CANCEL', + }); + const settledAt = new Date().toISOString(); + + const monitor = new PaymentMonitorService({ + account: SELLER, + network: 'TESTNET', + source: monitorSource(settledAt), + invoices: invoiceService, + checkpoints: new MemoryCheckpointStore(), + database: undefined, + }); + + await monitor.runOnce(); + + const storedPaid = rawStorage.getInvoiceById(invoice.id); + assert.equal(storedPaid?.status, 'PAID'); + assert.equal(storedPaid?.paymentTxHash, TX_HASH_2); + + let conflict: unknown; + try { + rawStorage.cancelInvoice(invoice.id, SELLER); + } catch (error) { + conflict = error; + } + assert.ok(conflict, 'cancel must refuse after monitor committed PAID'); + assert.equal((conflict as any).code, 'INVOICE_ALREADY_PAID'); + assert.equal((conflict as any).paymentTxHash, TX_HASH_2); + + const stored = rawStorage.getInvoiceById(invoice.id); + assert.equal(stored?.status, 'PAID'); + assert.equal(stored?.paymentTxHash, TX_HASH_2); + }); + + it('two invoices racing one monitor hash leave one PAID and one TX_HASH_ALREADY_USED', async () => { + const first = rawStorage.createInvoice({ + sellerPublicKey: SELLER, + amount: 42.5, + assetCode: 'XLM', + memo: 'INV-MONITOR-CANCEL', + }); + const second = rawStorage.createInvoice({ + sellerPublicKey: SELLER, + amount: 42.5, + assetCode: 'XLM', + memo: 'INV-OTHER-MEMO', + }); + + // First invoice claims the hash through the shared payment-claim lock. + const settled = rawStorage.markAsPaid(first.id, TX_HASH_2, PAYER, undefined, { + settledAt: new Date(), + destinationMuxedId: '4242', + }); + assert.equal(settled?.status, 'PAID'); + + let rejection: unknown; + try { + rawStorage.markAsPaid(second.id, TX_HASH_2, PAYER, undefined, { + settledAt: new Date(), + destinationMuxedId: '4242', + }); + } catch (error) { + rejection = error; + } + assert.ok(rejection instanceof PaymentClaimError); + assert.equal((rejection as PaymentClaimError).code, 'TX_HASH_ALREADY_USED'); + assert.equal((rejection as PaymentClaimError).settledInvoiceId, first.id); + + const other = rawStorage.getInvoiceById(second.id); + assert.equal(other?.status, 'PENDING'); + assert.equal(other?.paymentTxHash, undefined); + }); + + it('cancel that wins before monitor keeps CANCELLED when the payment mismatches', async () => { + const invoice = rawStorage.createInvoice({ + sellerPublicKey: SELLER, + amount: 42.5, + assetCode: 'XLM', + memo: 'INV-MONITOR-CANCEL', + }); + const cancelled = rawStorage.cancelInvoice(invoice.id, SELLER); + assert.equal(cancelled?.status, 'CANCELLED'); + + const monitor = new PaymentMonitorService({ + account: SELLER, + network: 'TESTNET', + source: monitorSource(isoOffset(cancelled!.cancelledAt!, 1000), { amount: '41.0000000' }), + invoices: invoiceService, + checkpoints: new MemoryCheckpointStore(), + database: undefined, + }); + + await monitor.runOnce(); + + const stored = rawStorage.getInvoiceById(invoice.id); + assert.equal(stored?.status, 'CANCELLED'); + assert.equal(stored?.paymentTxHash, undefined); + }); +}); + // Issue #501: one transaction hash settles at most one invoice. The in-memory // engine enforces it through PaymentClaimIndex; the Postgres engine through the // partial unique index on payment_tx_hash, mapped to the same typed rejection. diff --git a/backend/tests/invoice-handlers.test.ts b/backend/tests/invoice-handlers.test.ts index 177bafd8..5e969113 100644 --- a/backend/tests/invoice-handlers.test.ts +++ b/backend/tests/invoice-handlers.test.ts @@ -682,7 +682,7 @@ function runSharedBackendSuite(name: string, createStorage: () => InvoiceStorage it('keeps the verify response on the public shape', async () => { const created = await createInvoice({ customerEmail: 'pay@client.example' }); transaction = { - transaction: { memo: created.memo }, + transaction: { memo: created.memo, created_at: new Date().toISOString() }, operations: [ { type: 'payment', @@ -701,7 +701,7 @@ function runSharedBackendSuite(name: string, createStorage: () => InvoiceStorage body: { txHash: TX_HASH, payerEmail: 'percy@payer.example' }, }) ); - assert.equal(res.statusCode, 200); + assert.equal(res.statusCode, 200, JSON.stringify(res.body)); for (const key of PII_KEYS) { assert.equal(res.body.data[key], undefined, `verify response leaked ${key}`); } @@ -999,8 +999,10 @@ function runSharedBackendSuite(name: string, createStorage: () => InvoiceStorage handlers().cancelInvoice, createReq({ params: { id: invoice.id }, body: { sellerPublicKey: SELLER_A } }) ); - assert.equal(again.statusCode, 400); + assert.equal(again.statusCode, 409, JSON.stringify(again.body)); assert.equal(again.body.success, false); + assert.equal(again.body.code, 'INVOICE_ALREADY_CANCELLED'); + assert.equal(again.body.status, 'CANCELLED'); }); it('cancels a pending invoice when sellerPublicKey matches', async () => { @@ -1052,10 +1054,10 @@ function runSharedBackendSuite(name: string, createStorage: () => InvoiceStorage }) ); assert.equal(res.statusCode, 400); - assert.match(res.body.error, /conflicting/i); + assert.match(res.body.error, /query and header keys are not accepted/i); }); - it('rejects cancel when body and header seller keys disagree (400)', async () => { + it('rejects cancel when a header seller key is present (400)', async () => { const invoice = await createInvoice(); const res = await call( @@ -1067,7 +1069,7 @@ function runSharedBackendSuite(name: string, createStorage: () => InvoiceStorage }) ); assert.equal(res.statusCode, 400); - assert.match(res.body.error, /conflicting/i); + assert.match(res.body.error, /query and header keys are not accepted/i); }); it('rejects a query-only seller key — the body is the one transport (400)', async () => { @@ -1080,7 +1082,7 @@ function runSharedBackendSuite(name: string, createStorage: () => InvoiceStorage assert.equal(res.statusCode, 400); }); - it('tolerates a duplicate query key that agrees with the body', async () => { + it('rejects a duplicate query key even when it agrees with the body (issue #558)', async () => { const invoice = await createInvoice(); const res = await call( @@ -1091,8 +1093,10 @@ function runSharedBackendSuite(name: string, createStorage: () => InvoiceStorage query: { sellerPublicKey: SELLER_A }, }) ); - assert.equal(res.statusCode, 200); - assert.equal(res.body.data.status, 'CANCELLED'); + assert.equal(res.statusCode, 400); + assert.match(res.body.error, /query and header keys are not accepted/i); + const stored = await storage.getInvoiceById(invoice.id); + assert.equal(stored?.status, 'PENDING'); }); it('requires a signature when requireCancelSignature is set (401)', async () => { diff --git a/backend/tests/invoice-terminal-settlement.test.ts b/backend/tests/invoice-terminal-settlement.test.ts new file mode 100644 index 00000000..e3236819 --- /dev/null +++ b/backend/tests/invoice-terminal-settlement.test.ts @@ -0,0 +1,56 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { + LEGAL_TERMINAL_TRANSITIONS, + assertCancelTransitionAllowed, + assertPaidTransitionAllowed, + cancelConflictForStatus, + isLegalTerminalTransition, + InvoiceTerminalConflictError, +} from '../src/domain/invoice-settlement.ts'; + +describe('terminal invoice transitions (issue #558)', () => { + it('lists the legal terminal transitions including late settlement', () => { + assert.deepEqual( + [...LEGAL_TERMINAL_TRANSITIONS], + [ + ['PENDING', 'CANCELLED'], + ['PENDING', 'PAID'], + ['EXPIRED', 'PAID'], + ['CANCELLED', 'PAID'], + ] + ); + }); + + it('forbids leaving PAID for CANCELLED', () => { + assert.equal(isLegalTerminalTransition('PAID', 'CANCELLED'), false); + assert.throws( + () => assertCancelTransitionAllowed('PAID', 'abc'), + (error: unknown) => + error instanceof InvoiceTerminalConflictError && + error.code === 'INVOICE_ALREADY_PAID' && + error.paymentTxHash === 'abc' + ); + }); + + it('forbids a second cancel once CANCELLED won', () => { + assert.throws( + () => assertCancelTransitionAllowed('CANCELLED'), + (error: unknown) => + error instanceof InvoiceTerminalConflictError && + error.code === 'INVOICE_ALREADY_CANCELLED' + ); + }); + + it('allows late CANCELLED → PAID settlement while keeping PAID hard-terminal', () => { + assert.equal(isLegalTerminalTransition('CANCELLED', 'PAID'), true); + assert.doesNotThrow(() => assertPaidTransitionAllowed('CANCELLED')); + assert.doesNotThrow(() => assertPaidTransitionAllowed('PAID')); // idempotent gate + }); + + it('maps cancel losers to stable codes without inventing new wording', () => { + assert.equal(cancelConflictForStatus('PAID', 'tx').code, 'INVOICE_ALREADY_PAID'); + assert.equal(cancelConflictForStatus('CANCELLED').code, 'INVOICE_ALREADY_CANCELLED'); + assert.equal(cancelConflictForStatus('EXPIRED').code, 'INVOICE_EXPIRED'); + }); +}); diff --git a/docs/EXPIRY-AND-LATE-PAYMENT.md b/docs/EXPIRY-AND-LATE-PAYMENT.md index e41e5afe..387a5ac5 100644 --- a/docs/EXPIRY-AND-LATE-PAYMENT.md +++ b/docs/EXPIRY-AND-LATE-PAYMENT.md @@ -20,6 +20,23 @@ Two independent mechanisms set `EXPIRED`: the 60-second sweep, and the lazy check on read. That matters below, because it means an invoice can be `PENDING` in storage while its deadline has already passed. +## One terminal commit (issue #558) + +Cancel, manual verify, and the Horizon monitor all decide the invoice's +terminal status through the same storage transition. The loser receives a +stable conflict code (`INVOICE_ALREADY_PAID`, `INVOICE_ALREADY_CANCELLED`, +`TX_HASH_ALREADY_USED`, …) and must not clear `payment_tx_hash`. + +PAID never becomes CANCELLED. Concurrent cancel-versus-payment on a PENDING +invoice commits exactly one of CANCELLED or PAID. An exact on-chain payment +discovered after cancellation may still settle CANCELLED → PAID with +`AFTER_CANCEL` context (see [LATE_PAYMENT_POLICY.md](./LATE_PAYMENT_POLICY.md)); +that is attribution of funds that already landed, not a race overwrite. + +Cancel proof stays Freighter-only: the JSON body carries `sellerPublicKey`, +`signature`, and the signed message `cancel:`. Query and header +seller keys are rejected. + ## State machine ```mermaid diff --git a/frontend/lib/payment-page-state.d.ts b/frontend/lib/payment-page-state.d.ts index 3106a23a..4798a0d1 100644 --- a/frontend/lib/payment-page-state.d.ts +++ b/frontend/lib/payment-page-state.d.ts @@ -1,6 +1,6 @@ import type { WalletGateResult } from './freighter-availability'; -export type PayStatus = 'idle' | 'paying' | 'verifying' | 'paid' | 'error' | 'expired'; +export type PayStatus = 'idle' | 'paying' | 'verifying' | 'paid' | 'error' | 'expired' | 'cancelled'; export interface PayInvoice { status: string; diff --git a/frontend/lib/payment-page-state.js b/frontend/lib/payment-page-state.js index 8a430d13..b85e481f 100644 --- a/frontend/lib/payment-page-state.js +++ b/frontend/lib/payment-page-state.js @@ -16,11 +16,12 @@ * │ │ * ◀──────────────────────── RESET ───────────────────────────────────┘ * - * Any state ──INVOICE_LOADED / POLL_RESULT (PAID)────▶ paid - * Any state ──INVOICE_LOADED / POLL_RESULT (EXPIRED)─▶ expired + * Any state ──INVOICE_LOADED / POLL_RESULT (PAID)──────▶ paid + * Any state ──INVOICE_LOADED / POLL_RESULT (EXPIRED)───▶ expired + * Any state ──INVOICE_LOADED / POLL_RESULT (CANCELLED)─▶ cancelled * - * `paid` and `expired` are terminal: the ledger has decided, and no local event - * moves the page back out of them. + * `paid`, `expired`, and `cancelled` are terminal: storage committed one + * outcome, and no local event moves the page back out of them (issue #558). */ /** Every state the pay page can be in. */ @@ -31,9 +32,14 @@ const PAY_STATES = Object.freeze({ PAID: 'paid', ERROR: 'error', EXPIRED: 'expired', + CANCELLED: 'cancelled', }); -const TERMINAL_STATES = Object.freeze([PAY_STATES.PAID, PAY_STATES.EXPIRED]); +const TERMINAL_STATES = Object.freeze([ + PAY_STATES.PAID, + PAY_STATES.EXPIRED, + PAY_STATES.CANCELLED, +]); const { isTerminalPayState } = require('./pay-terminal-guard.ts'); const { effectiveInvoiceStatus, hasInvoiceExpired } = require('./invoice-lifecycle'); const { walletSessionChanged, walletSessionGate } = require('./wallet-session'); @@ -81,7 +87,9 @@ function getPayPageWalletGate(invoice, session, expectedNetwork, now) { title: 'Payment unavailable', message: view.expired ? 'This invoice has expired and can no longer be paid.' - : 'This invoice is not available for payment.', + : view.cancelled + ? 'This invoice was cancelled and can no longer be paid.' + : 'This invoice is not available for payment.', action: 'none', }; } @@ -93,6 +101,7 @@ function getPayPageWalletGate(invoice, session, expectedNetwork, now) { function stateForStatus(statusOrInvoice, now) { const status = effectiveInvoiceStatus(asInvoice(statusOrInvoice), now); if (status === 'PAID') return PAY_STATES.PAID; + if (status === 'CANCELLED') return PAY_STATES.CANCELLED; if (isExpiredInvoice(status)) return PAY_STATES.EXPIRED; return null; } @@ -295,6 +304,7 @@ function isResultState(state) { return ( state?.status === PAY_STATES.PAID || state?.status === PAY_STATES.EXPIRED || + state?.status === PAY_STATES.CANCELLED || state?.status === PAY_STATES.ERROR ); } @@ -338,6 +348,8 @@ function describePaymentState(state) { return 'Payment confirmed. Your payment proof is ready to download.'; case PAY_STATES.EXPIRED: return 'This invoice has expired and can no longer be paid.'; + case PAY_STATES.CANCELLED: + return 'This invoice was cancelled and can no longer be paid.'; case PAY_STATES.ERROR: return state?.error ? `Payment could not be completed. ${asSentence(state.error)}` diff --git a/frontend/tests/payment-page-state.test.js b/frontend/tests/payment-page-state.test.js index 63e23267..d6f6e1f0 100644 --- a/frontend/tests/payment-page-state.test.js +++ b/frontend/tests/payment-page-state.test.js @@ -118,6 +118,7 @@ test('the Freighter payment gate opens for a connected wallet on the expected ne test('an invoice that is already settled starts in its terminal state', () => { assert.equal(initialPaymentState(paid).status, PAY_STATES.PAID); assert.equal(initialPaymentState(expired).status, PAY_STATES.EXPIRED); + assert.equal(initialPaymentState({ status: 'CANCELLED' }).status, PAY_STATES.CANCELLED); assert.equal(initialPaymentState(pending).status, PAY_STATES.IDLE); assert.equal(initialPaymentState(null).status, PAY_STATES.IDLE); }); @@ -130,8 +131,8 @@ test('the initial state carries an already recorded transaction hash', () => { test('stateForStatus only forces a state for settled invoices', () => { assert.equal(stateForStatus('PAID'), PAY_STATES.PAID); assert.equal(stateForStatus('EXPIRED'), PAY_STATES.EXPIRED); + assert.equal(stateForStatus('CANCELLED'), PAY_STATES.CANCELLED); assert.equal(stateForStatus('PENDING'), null); - assert.equal(stateForStatus('CANCELLED'), null); }); // ------------------------------------------------------------- happy paths @@ -395,8 +396,13 @@ test('the busy states are exactly the two with a request in flight', () => { } }); -test('the result states are the three that carry an answer', () => { - const results = [PAY_STATES.PAID, PAY_STATES.EXPIRED, PAY_STATES.ERROR]; +test('the result states are the answers worth moving focus to', () => { + const results = [ + PAY_STATES.PAID, + PAY_STATES.EXPIRED, + PAY_STATES.CANCELLED, + PAY_STATES.ERROR, + ]; for (const status of Object.values(PAY_STATES)) { assert.equal( @@ -530,3 +536,17 @@ test('terminal states are never reset by a wallet change', () => { ); } }); + +test('POLL_RESULT forces cancelled when storage committed CANCELLED (issue #558)', () => { + let state = idleOn(pending); + state = paymentReducer(state, { type: 'PAY_STARTED' }); + assert.equal(state.status, PAY_STATES.PAYING); + state = paymentReducer(state, { + type: 'POLL_RESULT', + invoice: { status: 'CANCELLED' }, + }); + assert.equal(state.status, PAY_STATES.CANCELLED); + // Terminal: a later VERIFY_FAILED must not reopen the page. + const stuck = paymentReducer(state, { type: 'VERIFY_FAILED', error: 'nope' }); + assert.equal(stuck.status, PAY_STATES.CANCELLED); +}); diff --git a/frontend/tests/verification.test.js b/frontend/tests/verification.test.js index e600350c..22e5e654 100644 --- a/frontend/tests/verification.test.js +++ b/frontend/tests/verification.test.js @@ -91,6 +91,7 @@ test('covers every rejection code with a message', () => { 'PAYER_INFO_TOO_LONG', 'VERIFY_RATE_LIMIT_EXCEEDED', 'INVOICE_ALREADY_PAID', + 'INVOICE_ALREADY_CANCELLED', 'INVOICE_EXPIRED', 'INVOICE_NOT_PENDING', 'TRANSACTION_NOT_FOUND', diff --git a/shared/verification.ts b/shared/verification.ts index aaaae777..e4f2189c 100644 --- a/shared/verification.ts +++ b/shared/verification.ts @@ -42,6 +42,7 @@ export type VerificationCode = | 'PAYER_INFO_TOO_LONG' | 'VERIFY_RATE_LIMIT_EXCEEDED' | 'INVOICE_ALREADY_PAID' + | 'INVOICE_ALREADY_CANCELLED' | 'INVOICE_EXPIRED' | 'INVOICE_NOT_PENDING' | 'TRANSACTION_NOT_FOUND' @@ -92,6 +93,7 @@ export const VERIFICATION_MESSAGES: Record = { PAYER_INFO_TOO_LONG: 'Payer information is too long', VERIFY_RATE_LIMIT_EXCEEDED: 'Too many verification attempts for this invoice', INVOICE_ALREADY_PAID: 'Invoice has already been paid', + INVOICE_ALREADY_CANCELLED: 'Invoice has already been cancelled', INVOICE_EXPIRED: 'Invoice has expired and can no longer accept payment', INVOICE_NOT_PENDING: 'Invoice is not pending', TRANSACTION_NOT_FOUND: 'Transaction not found on Stellar', From ad7456ed10b605eb9fc286a98db86a9a00e210d6 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 05:21:52 -0400 Subject: [PATCH 2/5] docs: record current-main settlement integration and policy boundary [skip ci] --- docs/TERMINAL_SETTLEMENT_INTEGRATION.md | 28 +++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 docs/TERMINAL_SETTLEMENT_INTEGRATION.md diff --git a/docs/TERMINAL_SETTLEMENT_INTEGRATION.md b/docs/TERMINAL_SETTLEMENT_INTEGRATION.md new file mode 100644 index 00000000..c515b721 --- /dev/null +++ b/docs/TERMINAL_SETTLEMENT_INTEGRATION.md @@ -0,0 +1,28 @@ +# Terminal-settlement integration — October 4, 2026 + +This report supplements the existing PR #563 / issue #558 contribution. It does not claim that all acceptance criteria are complete. + +## Current upstream integration + +Commit `6382fb59001008f27e43fd96ecdb3b9b756610b5` merges original contribution `04726ebd2ddb99a8cc9bb40c39e3dae1c5dd0cbe` with upstream `282c75ad933651a93b89a8412ab649b48714c739`, without rewriting either history. The resulting source tree is `4edbd31b3357dda6ebd7a1d901f51f19b5ff252d`. + +Four conflicts were resolved in the shared handlers, memory service, Postgres service and cancellation/payment race suite. Cancellation retains the contribution's typed `409` loser responses, current status and already-recorded payment hash. The newer upstream lifecycle checks remain on the payment path. Missing-invoice cancellation keeps upstream's `404`; body-only wallet proof and both stores' payment-claim behavior are preserved. The new upstream lifecycle suite now asserts those exact cancellation responses and the retained hash instead of expecting the superseded generic response. Upstream payment-link artifacts, frontend changes and privacy-safe operational logging are retained. + +## Executed scope + +[Run 37191747168](https://github.com/woahwhattheheck/Quittance0/actions/runs/37191747168), Node `24.21.0`, completed successfully on the exact merge above: + +```sh +npm --prefix backend ci --no-audit --no-fund +npm --prefix backend run typecheck +cd backend +node --import tsx --test tests/invoice-terminal-settlement.test.ts tests/invoice-state-machine.test.ts tests/invoice-cancel-payment-race.test.ts +``` + +Backend typecheck passed. The selected contracts passed **41 tests in 10 suites**, with no failures, skips or cancellations. The tracked-source diff after execution was empty. This was not a full frontend/backend suite, a real Postgres concurrency run, a deployment or a live payment. + +[Artifact 11298848772](https://github.com/woahwhattheheck/Quittance0/actions/runs/37191747168/artifacts/11298848772) retains the command output, exact source identities and diff. ZIP SHA-256: `1a61238d8cf756c0cd57748570a28e26afc6ff4521e207c657eef26259de3e0c`. The temporary assembly/check workflow is not part of this contribution. + +## Remaining policy decision + +Issue #558 says that `CANCELLED` must never become `PAID`. The existing `LATE_PAYMENT_POLICY.md`, issue #507 and upstream lifecycle implementation permit an exact ledger payment to settle a cancelled invoice, retaining its cancellation history and late-payment classification. This integration preserves that existing policy; it does **not** resolve the contradiction or claim the stricter finality criterion is met. The maintainer must decide which behavior governs before this contribution can claim to close #558. From 80a63780e981b2b2d63cf0ba353be62bb424b412 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 10:56:20 -0400 Subject: [PATCH 3/5] fix(invoice): keep terminal cancel seller-scoped --- backend/src/services/invoice.service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/src/services/invoice.service.ts b/backend/src/services/invoice.service.ts index 9f7c271d..10e50b4e 100644 --- a/backend/src/services/invoice.service.ts +++ b/backend/src/services/invoice.service.ts @@ -394,7 +394,6 @@ export class InvoiceService { const row = existing.rows[0]; if ( sellerPublicKey && - row.status === 'PENDING' && row.seller_public_key !== sellerPublicKey ) { throw new Error('Unauthorized: only the seller can cancel this invoice'); From ab147b300a70a2e01b5f04967c4f2e003ad9368f Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 10:56:36 -0400 Subject: [PATCH 4/5] test(invoice): cover terminal cross-seller cancel --- backend/tests/invoice-dual-backend.test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/backend/tests/invoice-dual-backend.test.ts b/backend/tests/invoice-dual-backend.test.ts index 1393bd6a..cb307735 100644 --- a/backend/tests/invoice-dual-backend.test.ts +++ b/backend/tests/invoice-dual-backend.test.ts @@ -470,6 +470,26 @@ function runDualBackendSuite( ); }); + it('keeps terminal invoice details private from a different seller', async () => { + const inv = await storage.createInvoice(baseInput(SELLER_A)); + await storage.markAsPaid( + inv.id, + TX_HASH_1, + PAYER, + undefined, + { settledAt: new Date() } + ); + + await assert.rejects( + () => storage.cancelInvoice(inv.id, SELLER_B), + /[Uu]nauthorized/ + ); + + const stored = await storage.getInvoiceById(inv.id); + assert.equal(stored?.status, 'PAID'); + assert.equal(stored?.paymentTxHash, TX_HASH_1); + }); + it('rejects cancel for a missing invoice', async () => { await assert.rejects( () => storage.cancelInvoice('00000000-0000-4000-8000-000000000000', SELLER_A), From a8e120035ebae89baca64c85256a19079b60b32d Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 11:00:21 -0400 Subject: [PATCH 5/5] test(stellar): isolate nonexistent-hash 404 --- backend/tests/stellar-service.test.ts | 47 +++++++++++++++++++++------ 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/backend/tests/stellar-service.test.ts b/backend/tests/stellar-service.test.ts index 772a74e5..78029210 100644 --- a/backend/tests/stellar-service.test.ts +++ b/backend/tests/stellar-service.test.ts @@ -1,6 +1,7 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; import stellarService from '../src/services/stellar.service'; +import { server } from '../src/config/stellar'; describe('StellarService - Horizon Read Consolidation', () => { it('exposes loadAccount, getBalance, verifyPayment, getTransaction, streamPayments, getRecentPayments, sendPayment', () => { @@ -27,18 +28,44 @@ describe('StellarService - Horizon Read Consolidation', () => { } }); - it('verifies payment with a non-existent 64-character hash returns TRANSACTION_NOT_FOUND', async () => { + it('returns TRANSACTION_NOT_FOUND when Horizon responds with HTTP 404 for a valid hash', async () => { const nonExistentHash = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef'; - const result = await stellarService.verifyPayment(nonExistentHash, { - memo: 'INV-123', - amount: 10, - destination: 'GABC', - assetCode: 'XLM', - }); + const originalTransactions = server.transactions; + const transactionBuilder = originalTransactions.call(server); + const originalTransaction = transactionBuilder.transaction; + const requestedHashes: string[] = []; + let calls = 0; - assert.equal(result.ok, false); - if (!result.ok) { - assert.equal(result.code, 'TRANSACTION_NOT_FOUND'); + // Keep the service and Horizon wrapper real; control only the SDK response. + transactionBuilder.transaction = (txHash: string) => { + requestedHashes.push(txHash); + const transaction = originalTransaction.call(transactionBuilder, txHash); + transaction.call = async () => { + calls += 1; + throw Object.assign(new Error('Horizon 404'), { + response: { status: 404, statusText: '404', headers: {} }, + }); + }; + return transaction; + }; + server.transactions = () => transactionBuilder; + + try { + const result = await stellarService.verifyPayment(nonExistentHash, { + memo: 'INV-123', + amount: 10, + destination: 'GABC', + assetCode: 'XLM', + }); + + assert.deepEqual(requestedHashes, [nonExistentHash]); + assert.equal(calls, 1); + assert.equal(result.ok, false); + if (!result.ok) { + assert.equal(result.code, 'TRANSACTION_NOT_FOUND'); + } + } finally { + server.transactions = originalTransactions; } }); });