diff --git a/backend/src/domain/invoice-settlement.ts b/backend/src/domain/invoice-settlement.ts index 6b92753..a61ad03 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 e9d7e12..282d594 100644 --- a/backend/src/routes/invoice.handlers.ts +++ b/backend/src/routes/invoice.handlers.ts @@ -39,6 +39,7 @@ import { PaymentClaimError } from '../domain/payment-attribution'; import { IllegalStateTransitionError } from '../domain/invoice-lifecycle'; import { SettlementTimeUnavailableError, + InvoiceTerminalConflictError, warningForLatePayment, } from '../domain/invoice-settlement'; import { cutoverDrainMode, simulationAllowed } from '../config/runtime'; @@ -422,10 +423,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']; @@ -433,6 +433,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); @@ -442,26 +450,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, @@ -520,6 +508,17 @@ export function createInvoiceHandlers(options: InvoiceHandlerOptions): InvoiceHa options.paymentMonitor?.unregisterWatch(req.params.id); sendSuccess(res, 200, invoice); } catch (error: any) { + 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; + } if (error instanceof IllegalStateTransitionError) { res.status(400).json({ success: false, diff --git a/backend/src/services/invoice-memory.service.ts b/backend/src/services/invoice-memory.service.ts index e5d7fb0..40565e5 100644 --- a/backend/src/services/invoice-memory.service.ts +++ b/backend/src/services/invoice-memory.service.ts @@ -234,9 +234,7 @@ export class InvoiceMemoryService { if (sellerPublicKey && existing.sellerPublicKey !== sellerPublicKey) { throw new Error('Unauthorized: only the seller can cancel this invoice'); } - if (existing.status !== 'PENDING') { - throw new IllegalStateTransitionError(existing.status, 'CANCELLED'); - } + // 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 cf6a86d..10e50b4 100644 --- a/backend/src/services/invoice.service.ts +++ b/backend/src/services/invoice.service.ts @@ -13,6 +13,7 @@ import { } from '../domain/invoice-lifecycle'; import { SettlementTimeUnavailableError, + cancelConflictForStatus, type LatePaymentWarningCode, type SettlementContext, } from '../domain/invoice-settlement'; @@ -390,21 +391,18 @@ export class InvoiceService { if (existing.rows.length === 0) { throw new Error('Invoice not found'); } - const current = existing.rows[0]; + const row = existing.rows[0]; if ( sellerPublicKey && - current.status === 'PENDING' && - current.seller_public_key !== sellerPublicKey + row.seller_public_key !== sellerPublicKey ) { throw new Error('Unauthorized: only the seller can cancel this invoice'); } - if (current.status !== 'PENDING') { - throw new IllegalStateTransitionError( - current.status as 'PENDING' | 'PAID' | 'EXPIRED' | 'CANCELLED', - 'CANCELLED' - ); - } - 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 1293088..f4306c8 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, @@ -121,14 +126,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() }); } @@ -143,14 +151,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 376b21f..3adc6fb 100644 --- a/backend/tests/invoice-cancel-payment-race.test.ts +++ b/backend/tests/invoice-cancel-payment-race.test.ts @@ -398,8 +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); @@ -604,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-dual-backend.test.ts b/backend/tests/invoice-dual-backend.test.ts index 6d9c103..cb30773 100644 --- a/backend/tests/invoice-dual-backend.test.ts +++ b/backend/tests/invoice-dual-backend.test.ts @@ -452,8 +452,13 @@ function runDualBackendSuite( await assert.rejects( () => storage.cancelInvoice(inv.id, SELLER_A), - /Invoice not found or already processed/ + { + name: 'InvoiceTerminalConflictError', + code: 'INVOICE_ALREADY_CANCELLED', + currentStatus: 'CANCELLED', + } ); + assert.equal((await storage.getInvoiceById(inv.id))?.status, 'CANCELLED'); }); it('rejects cancel when sellerPublicKey does not match (cross-seller guard)', async () => { @@ -465,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), diff --git a/backend/tests/invoice-handlers.test.ts b/backend/tests/invoice-handlers.test.ts index 8a07cce..754e04e 100644 --- a/backend/tests/invoice-handlers.test.ts +++ b/backend/tests/invoice-handlers.test.ts @@ -694,7 +694,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}`); } @@ -1004,8 +1004,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 () => { @@ -1057,10 +1059,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( @@ -1072,7 +1074,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 () => { @@ -1085,7 +1087,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( @@ -1096,8 +1098,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-service.test.ts b/backend/tests/invoice-service.test.ts index c9bd1af..fd8d245 100644 --- a/backend/tests/invoice-service.test.ts +++ b/backend/tests/invoice-service.test.ts @@ -387,8 +387,13 @@ describe('InvoiceService (Postgres) seller scoping', () => { await assert.rejects( () => service.cancelInvoice(pending.id), - /Invoice not found or already processed/ + { + name: 'InvoiceTerminalConflictError', + code: 'INVOICE_ALREADY_CANCELLED', + currentStatus: 'CANCELLED', + } ); + assert.equal((await service.getInvoiceById(pending.id))?.status, 'CANCELLED'); }); it('cancelInvoice permits cancellation when sellerPublicKey matches and rejects when mismatched', async () => { diff --git a/backend/tests/invoice-state-machine.test.ts b/backend/tests/invoice-state-machine.test.ts index fbc235c..0d3f815 100644 --- a/backend/tests/invoice-state-machine.test.ts +++ b/backend/tests/invoice-state-machine.test.ts @@ -10,6 +10,7 @@ import { isUiTerminalInvoiceStatus, } from '../src/domain/invoice-lifecycle.ts'; import type { InvoiceStatus } from '../src/domain/invoice-lifecycle.ts'; +import { InvoiceTerminalConflictError } from '../src/domain/invoice-settlement.ts'; import { createInvoiceHandlers } from '../src/routes/invoice.handlers.ts'; import { MemoryInvoiceStorage } from '../src/storage/memory-invoice-storage.ts'; @@ -162,7 +163,7 @@ describe('Invoice lifecycle domain transitions (issue #443)', () => { }); describe('Invoice lifecycle HTTP cancel guards (issue #443)', () => { - it('cancelInvoice returns 400 with INVOICE_ALREADY_PAID when invoice is already PAID', async () => { + it('cancelInvoice returns 409 with INVOICE_ALREADY_PAID when invoice is already PAID', async () => { const storage = new MemoryInvoiceStorage(); const handlers = createInvoiceHandlers({ storage }); @@ -193,12 +194,14 @@ describe('Invoice lifecycle HTTP cancel guards (issue #443)', () => { cancelRes ); - assert.equal(cancelRes.statusCode, 400); + assert.equal(cancelRes.statusCode, 409); assert.equal(cancelRes.body.success, false); assert.equal(cancelRes.body.code, 'INVOICE_ALREADY_PAID'); + assert.equal(cancelRes.body.status, 'PAID'); + assert.equal(cancelRes.body.paymentTxHash, 'a'.repeat(64)); }); - it('cancelInvoice returns 400 with INVOICE_NOT_PENDING when invoice is already CANCELLED', async () => { + it('cancelInvoice returns 409 with INVOICE_ALREADY_CANCELLED when invoice is already CANCELLED', async () => { const storage = new MemoryInvoiceStorage(); const handlers = createInvoiceHandlers({ storage }); @@ -235,12 +238,13 @@ describe('Invoice lifecycle HTTP cancel guards (issue #443)', () => { secondCancelRes ); - assert.equal(secondCancelRes.statusCode, 400); + assert.equal(secondCancelRes.statusCode, 409); assert.equal(secondCancelRes.body.success, false); - assert.equal(secondCancelRes.body.code, 'INVOICE_NOT_PENDING'); + assert.equal(secondCancelRes.body.code, 'INVOICE_ALREADY_CANCELLED'); + assert.equal(secondCancelRes.body.status, 'CANCELLED'); }); - it('cancelInvoice returns 400 with INVOICE_EXPIRED when invoice is EXPIRED', async () => { + it('cancelInvoice returns 409 with INVOICE_EXPIRED when invoice is EXPIRED', async () => { const storage = new MemoryInvoiceStorage(); const handlers = createInvoiceHandlers({ storage }); @@ -268,9 +272,10 @@ describe('Invoice lifecycle HTTP cancel guards (issue #443)', () => { cancelRes ); - assert.equal(cancelRes.statusCode, 400); + assert.equal(cancelRes.statusCode, 409); assert.equal(cancelRes.body.success, false); assert.equal(cancelRes.body.code, 'INVOICE_EXPIRED'); + assert.equal(cancelRes.body.status, 'EXPIRED'); }); it('cancelInvoice returns 404 when invoice does not exist', async () => { @@ -314,7 +319,7 @@ describe('Invoice lifecycle storage matrix (issue #443)', () => { await assert.rejects( () => storage.cancelInvoice(paid.id, SELLER_KEY), (err: any) => { - assert.equal(err instanceof IllegalStateTransitionError, true); + assert.equal(err instanceof InvoiceTerminalConflictError, true); assert.equal(err.code, 'INVOICE_ALREADY_PAID'); return true; } diff --git a/backend/tests/invoice-terminal-settlement.test.ts b/backend/tests/invoice-terminal-settlement.test.ts new file mode 100644 index 0000000..e323681 --- /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/backend/tests/memory-storage.test.ts b/backend/tests/memory-storage.test.ts index 88f6fa1..1c6a312 100644 --- a/backend/tests/memory-storage.test.ts +++ b/backend/tests/memory-storage.test.ts @@ -188,8 +188,15 @@ describe('MemoryStorage parity with Postgres invoice columns', () => { const cancelled = store.cancelInvoice(pending.id, sellerA); assert.equal(cancelled?.status, 'CANCELLED'); - const alreadyCancelled = store.cancelInvoice(pending.id, sellerA); - assert.equal(alreadyCancelled, undefined); + assert.throws( + () => store.cancelInvoice(pending.id, sellerA), + { + name: 'InvoiceTerminalConflictError', + code: 'INVOICE_ALREADY_CANCELLED', + currentStatus: 'CANCELLED', + } + ); + assert.equal(store.getInvoiceById(pending.id)?.status, 'CANCELLED'); }); it('cancelInvoice throws unauthorized when sellerPublicKey does not match', () => { diff --git a/backend/tests/stellar-service.test.ts b/backend/tests/stellar-service.test.ts index 772a74e..7802921 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; } }); }); diff --git a/docs/EXPIRY-AND-LATE-PAYMENT.md b/docs/EXPIRY-AND-LATE-PAYMENT.md index e41e5af..387a5ac 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/docs/TERMINAL_SETTLEMENT_INTEGRATION.md b/docs/TERMINAL_SETTLEMENT_INTEGRATION.md new file mode 100644 index 0000000..c515b72 --- /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. diff --git a/frontend/lib/payment-page-state.d.ts b/frontend/lib/payment-page-state.d.ts index 3106a23..4798a0d 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 8a430d1..b85e481 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 63e2326..d6f6e1f 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 e600350..22e5e65 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 4812185..4af5bf5 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',