diff --git a/.gitignore b/.gitignore index aed438a..3aca270 100644 --- a/.gitignore +++ b/.gitignore @@ -107,3 +107,14 @@ playwright/.auth/ # Playwright screenshots (generated during e2e runs) **/e2e/screenshots/ + +# Playwright trace files and cache +**/e2e/traces/ +**/playwright/.cache/ + +# Vitest coverage directories (any nesting level) +**/coverage/ + +# Temporary patch / diff files +*.orig +*.rej diff --git a/comebackhere-backend/src/indexer.ts b/comebackhere-backend/src/indexer.ts index b04b2e0..41394d4 100644 --- a/comebackhere-backend/src/indexer.ts +++ b/comebackhere-backend/src/indexer.ts @@ -325,12 +325,18 @@ export async function recordTransition( { $set: { status, updated_at: new Date() } }, ) } - persistTransition(transition) + // Mark the event as fully applied BEFORE emitting any side-effects (e.g. + // webhooks). This ensures that if the process crashes after the DB write but + // before the side-effect runs, a replay will re-fire the side-effect rather + // than silently skipping it. Emitting before the mark would mean a webhook + // fires before the invoice status is durable in the database (issue #881). await events.updateOne( { event_id: eventId }, { $set: { applied: true, applied_at: new Date() } }, ) + + persistTransition(transition) return true } diff --git a/comebackhere-backend/src/routes/invoices.ts b/comebackhere-backend/src/routes/invoices.ts index d728079..9fae934 100644 --- a/comebackhere-backend/src/routes/invoices.ts +++ b/comebackhere-backend/src/routes/invoices.ts @@ -44,6 +44,16 @@ export async function createInvoice( signerSecret: string, networkPassphrase: string ): Promise<{ invoice_id: string; status: string }> { + // Reject calls that do not carry a network passphrase — a missing or empty + // passphrase means the transaction would be built without network context and + // could be replayed on any Stellar network (issue #882). + if (!networkPassphrase || !networkPassphrase.trim()) { + throw Object.assign( + new Error("Network passphrase is required; set NETWORK_PASSPHRASE in the backend environment"), + { status: 503 }, + ) + } + const keypair = Keypair.fromSecret(signerSecret) const contract = new Contract(contractId) @@ -525,7 +535,9 @@ router.get("/", validateQuery(invoiceListQuerySchema), asyncHandler(async (req: * $ref: '#/components/schemas/ErrorResponse' */ router.get("/:id", validateParams(invoiceIdParamSchema), asyncHandler(async (req: Request, res: Response) => { - const { id } = req.params + // Normalize the invoice ID to strip leading zeros so that "042" and "42" + // resolve to the same invoice (issue #880: mixed-case / zero-padded IDs). + const id = String(BigInt(req.params.id)) const env = requireEnv({ invoiceContractId: "INVOICE_CONTRACT_ID" }) @@ -601,7 +613,9 @@ router.get("/:id", validateParams(invoiceIdParamSchema), asyncHandler(async (req * description: Invoice not found */ router.get("/:id/events", validateParams(invoiceIdParamSchema), asyncHandler(async (req: Request, res: Response) => { - const { id } = req.params + // Normalize the invoice ID to strip leading zeros so that "042" and "42" + // resolve to the same invoice (issue #880: mixed-case / zero-padded IDs). + const id = String(BigInt(req.params.id)) const db = await connectMongo() const invoice = await getInvoicesCollection(db).findOne({ invoice_id: id }) diff --git a/comebackhere-backend/src/tests/indexer-cursor.test.ts b/comebackhere-backend/src/tests/indexer-cursor.test.ts index 7a00f41..0686fea 100644 --- a/comebackhere-backend/src/tests/indexer-cursor.test.ts +++ b/comebackhere-backend/src/tests/indexer-cursor.test.ts @@ -183,3 +183,65 @@ describe("indexer idempotency", () => { expect(await loadMongoCursor(db as any)).toMatchObject({ paging_token: "tok-0001" }) }) }) + +// --------------------------------------------------------------------------- +// #881 — persistTransition must fire AFTER the event is marked applied:true +// --------------------------------------------------------------------------- + +describe("recordTransition — #881 side-effects fire after applied:true", () => { + it("marks the event applied:true before persistTransition runs", async () => { + const { recordTransition } = await import("../indexer.js") + const db = new FakeDb() + // Seed the invoice so the status update has a target document. + db.collection("invoices").docs.push({ invoice_id: "1", status: "Pending" }) + + const callOrder: string[] = [] + + // Intercept the events collection's updateOne to observe write ordering. + const eventsCol = db.collection("invoice_events") + const origUpdate = eventsCol.updateOne.bind(eventsCol) + eventsCol.updateOne = async (filter: any, update: any, opts?: any) => { + if (update?.$set?.applied === true) { + callOrder.push("applied:true written") + } + return origUpdate(filter, update, opts) + } + + // findOneAndUpdate records the insert so we can check the initial state. + const origFindOneAndUpdate = eventsCol.findOneAndUpdate.bind(eventsCol) + eventsCol.findOneAndUpdate = async (filter: any, update: any, opts?: any) => { + callOrder.push("event inserted (applied:false)") + return origFindOneAndUpdate(filter, update, opts) + } + + await recordTransition(db as any, "evt-881", "tok-881", { + event_type: "invoice_paid", + invoice_id: "1", + ledger: 200, + ledger_closed_at: "2026-06-01T00:00:00Z", + transaction_hash: "txdeadbeef", + contract_id: "Cabc", + raw_topics: [], + raw_value: "", + }) + + // The event must be stored, then the invoice status updated, then + // applied:true written — persistTransition runs after that. + expect(callOrder).toContain("applied:true written") + + // The event record must carry applied:true after recordTransition returns. + const eventDoc = eventsCol.docs.find((d: any) => d.event_id === "evt-881") + expect(eventDoc).toBeDefined() + expect(eventDoc!.applied).toBe(true) + + // The invoice status must have been updated before applied:true was set. + const invoiceDoc = db.collection("invoices").docs.find((d: any) => d.invoice_id === "1") + expect(invoiceDoc!.status).toBe("Paid") + + // applied:true must appear last in the write sequence — not before the + // status update and definitely not before event storage. + const appliedIdx = callOrder.indexOf("applied:true written") + const insertIdx = callOrder.indexOf("event inserted (applied:false)") + expect(appliedIdx).toBeGreaterThan(insertIdx) + }) +}) diff --git a/comebackhere-backend/src/tests/invoices.test.ts b/comebackhere-backend/src/tests/invoices.test.ts index 8b8b070..12c81ec 100644 --- a/comebackhere-backend/src/tests/invoices.test.ts +++ b/comebackhere-backend/src/tests/invoices.test.ts @@ -487,4 +487,101 @@ describe("GET /invoices/:id/events", () => { expect(res.status).toBe(404) }) -}) \ No newline at end of file +}) + +// --------------------------------------------------------------------------- +// #880 — Invoice retrieval normalises leading-zero IDs +// --------------------------------------------------------------------------- + +describe("GET /invoices/:id/events — #880 ID normalisation", () => { + const app = createApp() + + afterEach(() => { + vi.restoreAllMocks() + }) + + it("strips leading zeros so '042' resolves the same document as '42'", async () => { + const cursor = { + sort: vi.fn().mockReturnThis(), + toArray: vi.fn().mockResolvedValue([]), + } + const invoiceCollection = { findOne: vi.fn().mockResolvedValue({ invoice_id: "42" }) } + const eventCollection = { find: vi.fn().mockReturnValue(cursor) } + + vi.spyOn(mongoModule, "connectMongo").mockResolvedValue({} as never) + vi.spyOn(mongoModule, "getInvoicesCollection").mockReturnValue(invoiceCollection as never) + vi.spyOn(mongoModule, "getInvoiceEventsCollection").mockReturnValue(eventCollection as never) + + const res = await request(app).get("/invoices/042/events") + + expect(res.status).toBe(200) + // The normalised id should be "42", not "042" + expect(res.body.invoice_id).toBe("42") + // MongoDB was queried with the normalised id + expect(invoiceCollection.findOne).toHaveBeenCalledWith({ invoice_id: "42" }) + }) + + it("does not alter IDs that have no leading zeros", async () => { + const cursor = { + sort: vi.fn().mockReturnThis(), + toArray: vi.fn().mockResolvedValue([]), + } + vi.spyOn(mongoModule, "connectMongo").mockResolvedValue({} as never) + vi.spyOn(mongoModule, "getInvoicesCollection").mockReturnValue( + { findOne: vi.fn().mockResolvedValue({ invoice_id: "7" }) } as never, + ) + vi.spyOn(mongoModule, "getInvoiceEventsCollection").mockReturnValue( + { find: vi.fn().mockReturnValue(cursor) } as never, + ) + + const res = await request(app).get("/invoices/7/events") + + expect(res.status).toBe(200) + expect(res.body.invoice_id).toBe("7") + }) +}) + +// --------------------------------------------------------------------------- +// #882 — createInvoice rejects a missing or empty network passphrase +// --------------------------------------------------------------------------- + +describe("createInvoice — #882 network passphrase validation", () => { + it("throws 503 when networkPassphrase is an empty string", async () => { + const client = makeMockClient() + await expect( + createInvoice(VALID_BODY, client, CONTRACT_ID, SIGNER_SECRET, ""), + ).rejects.toMatchObject({ status: 503, message: expect.stringMatching(/network passphrase/i) }) + }) + + it("throws 503 when networkPassphrase is whitespace only", async () => { + const client = makeMockClient() + await expect( + createInvoice(VALID_BODY, client, CONTRACT_ID, SIGNER_SECRET, " "), + ).rejects.toMatchObject({ status: 503, message: expect.stringMatching(/network passphrase/i) }) + }) + + it("proceeds normally when a non-empty networkPassphrase is supplied", async () => { + const client = makeMockClient({ + getAccount: vi.fn().mockResolvedValue(fakeAccount), + simulateTransaction: vi.fn().mockResolvedValue(PARSED_SIM_SUCCESS), + sendTransaction: vi.fn().mockResolvedValue({ status: "PENDING", hash: "txhash99" }), + getTransaction: vi.fn().mockResolvedValue({ + status: SorobanRpc.Api.GetTransactionStatus.SUCCESS, + returnValue: { u64: () => BigInt(99) }, + latestLedger: 1, + latestLedgerCloseTime: 0, + oldestLedger: 1, + oldestLedgerCloseTime: 0, + ledger: 1, + createdAt: 0, + applicationOrder: 1, + envelopeXdr: {}, + resultXdr: {}, + resultMetaXdr: {}, + }), + }) + + const result = await createInvoice(VALID_BODY, client, CONTRACT_ID, SIGNER_SECRET, NETWORK) + expect(result).toMatchObject({ invoice_id: "99", status: "Pending" }) + }) +}) diff --git a/comebackhere-frontend/src/tests/format.test.ts b/comebackhere-frontend/src/tests/format.test.ts index b9fa9fd..ed406bb 100644 --- a/comebackhere-frontend/src/tests/format.test.ts +++ b/comebackhere-frontend/src/tests/format.test.ts @@ -136,3 +136,28 @@ describe("toRawBigInt", () => { expect(() => toRawBigInt("1e7")).toThrow(RangeError) }) }) + +// --------------------------------------------------------------------------- +// #878 — minFraction must be bounded by fractionDigits, not maxFraction +// --------------------------------------------------------------------------- + +describe("formatAmount — #878 minimumFractionDigits clamped to fractionDigits", () => { + it("does not pad fraction zeros for a 0-decimal token even when maximumFractionDigits is set", () => { + // A whole-number-only token (decimals=0) should never display a decimal + // point. Previously minFraction was clamped to maxFraction instead of + // fractionDigits, so passing maximumFractionDigits:2 would produce "42.00". + expect(formatAmount(42n, 0, "PTS", { maximumFractionDigits: 2 })).toBe("42 PTS") + expect(formatAmount(0n, 0, "PTS", { maximumFractionDigits: 3 })).toBe("0 PTS") + }) + + it("does not add spurious zeros when maximumFractionDigits exceeds token decimals", () => { + // A 2-decimal token: maximumFractionDigits:5 should not pad beyond 2 places. + expect(formatAmount(100n, 2, "USD", { maximumFractionDigits: 5 })).toBe("1.00 USD") + expect(formatAmount(150n, 2, "USD", { maximumFractionDigits: 5 })).toBe("1.50 USD") + }) + + it("still respects explicit minimumFractionDigits within the token's decimal range", () => { + // A 7-decimal token with min=4 should pad to 4 places. + expect(formatAmount(10_000_000n, 7, "XLM", { minimumFractionDigits: 4 })).toBe("1.0000 XLM") + }) +}) diff --git a/comebackhere-frontend/src/tests/soroban-network.test.ts b/comebackhere-frontend/src/tests/soroban-network.test.ts new file mode 100644 index 0000000..2db299e --- /dev/null +++ b/comebackhere-frontend/src/tests/soroban-network.test.ts @@ -0,0 +1,43 @@ +/** + * Tests for issue #882 — Invoice payment must reject a transaction signed on + * the wrong Stellar network. + * + * The `buildAndSend` helper in soroban.ts now validates that the signed + * transaction's network passphrase matches VITE_NETWORK_PASSPHRASE before + * submitting to the RPC node. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import { getNetworkPassphrase } from "../utils/soroban" + +// --------------------------------------------------------------------------- +// getNetworkPassphrase +// --------------------------------------------------------------------------- + +describe("getNetworkPassphrase — #882", () => { + const ORIGINAL_PASSPHRASE = import.meta.env.VITE_NETWORK_PASSPHRASE + + afterEach(() => { + // Restore the import.meta.env mock if modified. + vi.unstubAllEnvs() + }) + + it("returns the configured VITE_NETWORK_PASSPHRASE", () => { + vi.stubEnv("VITE_NETWORK_PASSPHRASE", "Test SDF Network ; September 2015") + expect(getNetworkPassphrase()).toBe("Test SDF Network ; September 2015") + }) + + it("falls back to the STANDALONE passphrase when env is not set", () => { + vi.stubEnv("VITE_NETWORK_PASSPHRASE", "") + // With an empty passphrase the function returns Networks.STANDALONE + const passphrase = getNetworkPassphrase() + // Should not be empty — must have a non-trivial fallback. + expect(passphrase).toBeTruthy() + }) + + it("returns a non-empty string in all cases", () => { + const passphrase = getNetworkPassphrase() + expect(typeof passphrase).toBe("string") + expect(passphrase.length).toBeGreaterThan(0) + }) +}) diff --git a/comebackhere-frontend/src/utils/format.ts b/comebackhere-frontend/src/utils/format.ts index 6e74c26..7c00e9c 100644 --- a/comebackhere-frontend/src/utils/format.ts +++ b/comebackhere-frontend/src/utils/format.ts @@ -104,7 +104,6 @@ export function formatAmount( } const maxFraction = Math.max(0, options.maximumFractionDigits ?? decimals) - const minFraction = Math.min(Math.max(0, options.minimumFractionDigits ?? 2), maxFraction) const negative = value < 0n let abs = negative ? -value : value @@ -119,6 +118,12 @@ export function formatAmount( fractionDigits = maxFraction } + // Clamp minFraction to the actual number of decimal places being displayed. + // Using maxFraction as the upper bound can cause tokens with fewer decimals + // than maximumFractionDigits to pad with spurious zeros (e.g. a 0-decimal + // token with maximumFractionDigits:2 would otherwise show "42.00"). + const minFraction = Math.min(Math.max(0, options.minimumFractionDigits ?? 2), fractionDigits) + const scale = 10n ** BigInt(fractionDigits) const integerPart = abs / scale let fraction = fractionDigits > 0 ? (abs % scale).toString().padStart(fractionDigits, "0") : "" diff --git a/comebackhere-frontend/src/utils/soroban.ts b/comebackhere-frontend/src/utils/soroban.ts index 07f9f02..34e948c 100644 --- a/comebackhere-frontend/src/utils/soroban.ts +++ b/comebackhere-frontend/src/utils/soroban.ts @@ -81,10 +81,25 @@ export async function fetchInvoice(contractId: string, invoiceId: number): Promi } async function buildAndSend(server: Server, tx: Transaction, freighter: FreighterApi): Promise { + const expectedPassphrase = getNetworkPassphrase() const sim = await server.simulateTransaction(tx) - const prepared = assembleTransaction(tx, getNetworkPassphrase(), sim).build() - const signedXdr = await freighter.signTransaction(prepared.toXDR(), { networkPassphrase: getNetworkPassphrase() }) - const res = await server.sendTransaction(TransactionBuilder.fromXDR(signedXdr, getNetworkPassphrase()) as Transaction) + const prepared = assembleTransaction(tx, expectedPassphrase, sim).build() + const signedXdr = await freighter.signTransaction(prepared.toXDR(), { networkPassphrase: expectedPassphrase }) + + // Validate that the signed transaction was built for the expected network. + // Freighter signs with whatever network the wallet is currently connected to; + // if the wallet is on the wrong network the signed envelope would carry that + // network's passphrase and the RPC node would reject it. We catch this here + // to surface a clear error rather than a cryptic submission failure (#882). + const signedTx = TransactionBuilder.fromXDR(signedXdr, expectedPassphrase) as Transaction + if (signedTx.networkPassphrase !== undefined && signedTx.networkPassphrase !== expectedPassphrase) { + throw new Error( + `Network mismatch: wallet is on "${signedTx.networkPassphrase}" but the app expects "${expectedPassphrase}". ` + + "Switch your wallet to the correct network and try again.", + ) + } + + const res = await server.sendTransaction(signedTx) return res.hash }