diff --git a/app/__tests__/api/confirm-transaction-checked.test.ts b/app/__tests__/api/confirm-transaction-checked.test.ts new file mode 100644 index 000000000..10de58328 --- /dev/null +++ b/app/__tests__/api/confirm-transaction-checked.test.ts @@ -0,0 +1,141 @@ +/** + * GH#2517 — every `confirmTransaction()` in an API route must have its result + * checked. + * + * `Connection.confirmTransaction()` can RESOLVE with a `SignatureResult` whose + * `err` records an on-chain execution failure — awaiting it proves the RPC call + * completed, not that the transaction succeeded. Seven routes discarded that + * result and advanced success-only state anyway: persisting a mint address for + * a mint that was never created, consuming a 24-hour faucet claim, and telling + * callers that tokens were delivered. + * + * The repo already had the right answer — `assertSuccessfulConfirmation()`, + * which accepts only an explicit `value.err === null` and so also fails closed + * on a malformed or incomplete result. Two routes already used it. This is the + * guard that stops the next call site being written without it. + * + * Deliberately a source scan rather than per-route tests: the property is "no + * call site anywhere is bare", which is a statement about the whole surface. A + * test per known route would say nothing about the eighth one somebody adds. + */ + +import { describe, it, expect } from "vitest"; +import * as fs from "fs"; +import * as path from "path"; + +const API_DIR = path.resolve(__dirname, "../../app/api"); + +function routeFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) out.push(...routeFiles(full)); + else if (entry.name === "route.ts") out.push(full); + } + return out; +} + +/** Strip line comments so prose mentioning confirmTransaction() isn't counted. */ +function stripComments(src: string): string { + return src + .split("\n") + .map((l) => l.replace(/\/\/.*$/, "")) + .join("\n"); +} + +interface Site { + file: string; + line: number; + checked: boolean; +} + +/** + * Decide whether ONE call's result is checked, bound to that call rather than + * to its neighbourhood. + * + * An earlier version of this scan looked for `assertSuccessfulConfirmation` + * anywhere within ±500 characters, which CodeRabbit correctly flagged on #2519: + * a bare confirmation sitting next to a guarded one would pass. That defeats the + * point of a guard whose whole job is catching the NEXT call site. + * + * Every call in the tree is one of two shapes, so both are recognised exactly: + * + * A assertSuccessfulConfirmation(await conn.confirmTransaction(...), "...") + * B const result = await conn.confirmTransaction(...) // then result is + * // asserted or + * // result.value.err + * // is inspected + * + * Shape B is tied to the BINDING NAME, so an unrelated guarded call elsewhere + * in the file cannot vouch for it. + */ +function isChecked(code: string, callIdx: number): boolean { + const before = code.slice(0, callIdx); + + // A: the call is the argument to the assertion. Allow the receiver + // expression and an `await` between the paren and the call. + if (/assertSuccessfulConfirmation\(\s*(?:await\s+)?[\w$.]*$/.test(before)) { + return true; + } + + // B: the call's result is bound; require that binding to be checked later. + const bound = before.match(/(?:const|let|var)\s+([\w$]+)\s*(?::[^=]+)?=\s*(?:await\s+)?[\w$.]*$/); + if (bound) { + const name = bound[1]; + const after = code.slice(callIdx); + const asserted = new RegExp( + `assertSuccessfulConfirmation\\(\\s*${name}\\b`, + ).test(after); + const inspected = new RegExp(`\\b${name}\\.value\\.err\\b`).test(after); + return asserted || inspected; + } + + return false; +} + +function callSites(): Site[] { + const sites: Site[] = []; + for (const file of routeFiles(API_DIR)) { + const code = stripComments(fs.readFileSync(file, "utf8")); + for (const m of code.matchAll(/\.confirmTransaction\(/g)) { + const idx = m.index ?? 0; + sites.push({ + file: path.relative(API_DIR, file), + line: code.slice(0, idx).split("\n").length, + checked: isChecked(code, idx), + }); + } + } + return sites; +} + +describe("GH#2517: no API route discards a confirmTransaction result", () => { + it("finds the call sites at all (guards the scan itself)", () => { + // If a refactor moves these calls behind a wrapper, this test starts + // passing vacuously — so assert the scan still sees something. + expect(callSites().length).toBeGreaterThanOrEqual(10); + }); + + it("checks every one of them", () => { + const bare = callSites().filter((s) => !s.checked); + expect( + bare.map((s) => `${s.file}:${s.line}`), + "confirmTransaction() result discarded — wrap it in assertSuccessfulConfirmation()", + ).toEqual([]); + }); + + it("the seven routes GH#2517 named import the shared helper", () => { + const named = [ + "devnet-airdrop/route.ts", + "devnet-mint-token/route.ts", + "devnet-mirror-mint/route.ts", + "playground/faucet/route.ts", + ]; + for (const rel of named) { + const src = fs.readFileSync(path.join(API_DIR, rel), "utf8"); + expect(src, rel).toContain( + 'from "@/lib/transaction-confirmation"', + ); + } + }); +}); diff --git a/app/app/api/devnet-airdrop/route.ts b/app/app/api/devnet-airdrop/route.ts index 7c8a5cfca..9f5ba17b6 100644 --- a/app/app/api/devnet-airdrop/route.ts +++ b/app/app/api/devnet-airdrop/route.ts @@ -53,6 +53,7 @@ import { getConfig } from "@/lib/config"; import { getDevnetMintSigner } from "@/lib/devnet-signer"; import type { getServiceClient as _GetServiceClient } from "@/lib/supabase"; import * as Sentry from "@sentry/nextjs"; +import { assertSuccessfulConfirmation } from "@/lib/transaction-confirmation"; type SupabaseClient = ReturnType; @@ -368,7 +369,14 @@ async function resolveServerOwnedDevnetMint( try { const createSig = await connection.sendRawTransaction(signedCreateTx.serialize()); - await connection.confirmTransaction(createSig, "confirmed"); + // GH#2517: confirmTransaction() can RESOLVE with a SignatureResult whose + // `err` records an on-chain failure, so awaiting it is not proof of + // success. Without this, a mint that failed on chain still reached the + // `devnet_mints` upsert below and became the stored mapping. + assertSuccessfulConfirmation( + await connection.confirmTransaction(createSig, "confirmed"), + "Devnet mirror-mint creation", + ); } catch (e) { Sentry.captureException(e, { tags: { endpoint: "/api/devnet-airdrop", step: "resolveServerOwnedDevnetMint.createMint" }, @@ -757,7 +765,12 @@ export async function POST(req: NextRequest) { const txSig = await connection.sendRawTransaction( (signedTx as Transaction).serialize(), ); - await connection.confirmTransaction(txSig, "confirmed"); + // GH#2517: without this the 24h claim stays reserved and the + // response reports an amount even though nothing was minted. + assertSuccessfulConfirmation( + await connection.confirmTransaction(txSig, "confirmed"), + "Devnet airdrop mint", + ); return txSig; })(), 30_000, diff --git a/app/app/api/devnet-mint-token/route.ts b/app/app/api/devnet-mint-token/route.ts index 4feb48bbd..7be0e12de 100644 --- a/app/app/api/devnet-mint-token/route.ts +++ b/app/app/api/devnet-mint-token/route.ts @@ -37,6 +37,7 @@ import * as Sentry from "@sentry/nextjs"; import { getClientIp } from "@/lib/get-client-ip"; import { checkMintRateLimit } from "@/lib/devnet-mirror-mint-rate-limit"; import { tryFaucetGate, releaseFaucetClaim } from "@/lib/faucet-rate-gate"; +import { assertSuccessfulConfirmation } from "@/lib/transaction-confirmation"; export const dynamic = "force-dynamic"; @@ -310,7 +311,12 @@ export async function POST(req: NextRequest) { const airdropTxSig = await connection.sendRawTransaction( (signedAirdropTx as Transaction).serialize(), ); - await connection.confirmTransaction(airdropTxSig, "confirmed"); + // GH#2517: the thrown-error branch omits the airdrop fields, so an + // unchecked result here reports the optional airdrop as successful. + assertSuccessfulConfirmation( + await connection.confirmTransaction(airdropTxSig, "confirmed"), + "Devnet token airdrop", + ); mintSucceeded = true; return NextResponse.json({ @@ -399,7 +405,12 @@ export async function POST(req: NextRequest) { (tx as Transaction).partialSign(mintKeypair); tx = mintSigner.signTransaction(tx); const sig = await connection.sendRawTransaction((tx as Transaction).serialize()); - await connection.confirmTransaction(sig, "confirmed"); + // GH#2517: a nonexistent mint must not become the stored mapping or be + // returned as `status: "created"` to later callers. + assertSuccessfulConfirmation( + await connection.confirmTransaction(sig, "confirmed"), + "Devnet mint creation", + ); const devnetMint = mintKeypair.publicKey.toBase58(); diff --git a/app/app/api/devnet-mirror-mint/route.ts b/app/app/api/devnet-mirror-mint/route.ts index 9dc4a6f1f..85e573203 100644 --- a/app/app/api/devnet-mirror-mint/route.ts +++ b/app/app/api/devnet-mirror-mint/route.ts @@ -45,6 +45,7 @@ import { getServiceClient } from "@/lib/supabase"; import { getDevnetMintSigner } from "@/lib/devnet-signer"; import { validateTokenMetadata, validateDexScreenerResponse, validateJupiterTokenResponse } from "@/lib/token-metadata-validators"; import * as Sentry from "@sentry/nextjs"; +import { assertSuccessfulConfirmation } from "@/lib/transaction-confirmation"; export const dynamic = "force-dynamic"; @@ -359,7 +360,12 @@ export async function POST(req: NextRequest) { let sig: string; try { sig = await connection.sendRawTransaction(tx.serialize()); - await connection.confirmTransaction(sig, "confirmed"); + // GH#2517: with no pre-existing canonical row this route would otherwise + // persist or return an address whose mint creation failed. + assertSuccessfulConfirmation( + await connection.confirmTransaction(sig, "confirmed"), + "Devnet mirror-mint creation", + ); } catch (e) { Sentry.captureException(e, { tags: { endpoint: "/api/devnet-mirror-mint", step: "sendAndConfirm" }, diff --git a/app/app/api/playground/faucet/route.ts b/app/app/api/playground/faucet/route.ts index bddb008a9..80edc944a 100644 --- a/app/app/api/playground/faucet/route.ts +++ b/app/app/api/playground/faucet/route.ts @@ -45,6 +45,7 @@ import { } from "@solana/spl-token"; import { getDevnetMintSigner } from "@/lib/devnet-signer"; import * as Sentry from "@sentry/nextjs"; +import { assertSuccessfulConfirmation } from "@/lib/transaction-confirmation"; export const dynamic = "force-dynamic"; @@ -232,9 +233,15 @@ export async function POST(req: NextRequest) { (signedTx as Transaction).serialize(), { skipPreflight: false }, ); - await connection.confirmTransaction( - { signature: usdcSig, blockhash, lastValidBlockHeight }, - "confirmed", + // GH#2517: the catch below releases the durable claim slot, so throwing + // here is what stops a failed mint from recording the claim and telling + // the caller 10,000 Sim-USDC was delivered. + assertSuccessfulConfirmation( + await connection.confirmTransaction( + { signature: usdcSig, blockhash, lastValidBlockHeight }, + "confirmed", + ), + "Playground USDC faucet mint", ); } catch (mintErr) { // Release the durable claim slot so a mint failure doesn't lock the wallet. @@ -268,7 +275,12 @@ export async function POST(req: NextRequest) { const airdropSig: string = await Promise.race([ (async () => { const s = await pubConn.requestAirdrop(walletPk, SOL_AIRDROP_AMOUNT); - await pubConn.confirmTransaction(s, "confirmed"); + // GH#2517: an unchecked result advertises SOL success and stops the + // RPC fallback loop from trying the next endpoint. + assertSuccessfulConfirmation( + await pubConn.confirmTransaction(s, "confirmed"), + "Playground SOL airdrop", + ); return s; })(), new Promise((_, reject) =>