From 425dc2b8485809dc6f95649b8e2f0a141c5e8154 Mon Sep 17 00:00:00 2001 From: redshift <213178690+1ftredsh@users.noreply.github.com> Date: Fri, 2 Oct 2026 02:11:29 +0800 Subject: [PATCH] fix(wallet): address mint recovery review and clarify scope --- docs/wallet-mint-recovery.md | 71 +++++++++++++++++++ src/cli.ts | 19 +++-- src/daemon/http/index.ts | 33 ++++++--- src/daemon/http/wallet-recovery.test.ts | 49 +++++++++++++ src/daemon/wallet/cleanup.test.ts | 13 +++- src/daemon/wallet/cleanup.ts | 18 ++++- src/daemon/wallet/coco-client.test.ts | 42 +++++++++++ src/daemon/wallet/coco-client.ts | 35 ++++++--- src/daemon/wallet/cocod-client.ts | 6 +- .../mint-quote-recovery.fake-mint.test.ts | 30 ++++++++ src/daemon/wallet/mint-quote-recovery.ts | 11 +-- 11 files changed, 296 insertions(+), 31 deletions(-) create mode 100644 docs/wallet-mint-recovery.md create mode 100644 src/daemon/http/wallet-recovery.test.ts diff --git a/docs/wallet-mint-recovery.md b/docs/wallet-mint-recovery.md new file mode 100644 index 0000000..cb344ef --- /dev/null +++ b/docs/wallet-mint-recovery.md @@ -0,0 +1,71 @@ +# Mint quote recovery: scope and troubleshooting + +`routstrd wallet recover` explicitly retries mint operations through coco using +**their existing stored outputs**. It can restore signatures when a quote is +already issued, and reopen failed operations when explicitly requested: + +```sh +routstrd history --json +routstrd wallet recover --op --include-failed +``` + +Failed operations require explicit IDs over both HTTP and the CLI. A successful +re-run on an already finalized operation is a no-op. Requests that exceed their +wait budget are not cancelled; explicit retries skip the operation while the +underlying work is outstanding. + +## What this fixes—and what it does not + +Coco already checks pending quotes on startup and the daemon periodically +refreshes them. A quote paid while the daemon was offline does not, by itself, +require a new issuance implementation. + +This change makes normal cleanup confirm UNPAID with the mint before failing an +expired quote. PAID, ISSUED and unverified quotes remain pending. It also gives +operators a recovery path for operations previously marked failed. + +It does **not** replace rejected outputs with fresh outputs on an active keyset. +An inactive-keyset rejection can therefore remain retryable with zero recovery. +Recovery reports coco's persisted mint error when available, rather than only a +generic “remains pending” error. + +Do not infer that the production incidents were caused by keyset retirement. +Before claiming those incidents are fixed, collect: + +- The affected operation IDs, quote IDs, state and persisted `error`. +- A fresh remote quote state and, where provided, paid/issued amounts. +- The keyset IDs in the stored outputs and the mint's current keyset metadata. +- A reproduction showing existing recovery fails and the proposed fix succeeds. + +Inspect persisted operation data through a read-only database copy; do not edit +rows or run recovery scripts concurrently with a daemon against the same wallet. +Never share the mnemonic, output secrets, or full wallet database in a PR. + +A future fresh-output path must preserve original outputs for uncertain issuance +and NUT-09 restore, allocate fresh deterministic counters safely, and coordinate +with coco's watcher/processor. It needs its own integration tests before handling +real funds. + +## Cleanup preview and force + +`wallet cleanup --dry-run` is local-only: it reports `mintQuoteCandidates`, not +confirmed failures. `failedMintQuotes` and `leftForRecovery` are zero because no +mint check or cleanup transition was performed. Send/melt counts remain planned +cleanup counts in dry-run mode. + +`--force` deliberately bypasses mint confirmation and can strand paid sats in a +failed operation. Prefer normal cleanup. Forced operations can be retried with +`--op --include-failed`, but recovery still depends on the mint +accepting their stored outputs or restoring their signatures. + +## Integration and release notes + +The reopen helper uses private coco-core 1.0.1 methods. Retain real-Manager and +HTTP fake-mint coverage, use frozen dependency installs, and re-run integration +tests on coco upgrades. A controlled low-value live-mint smoke test remains +recommended before release. + +PR #118 removes `cocod-client.ts`. When integrating that change, move recovery +and cleanup contracts into its replacement `wallet-client.ts`, rename HTTP error +references accordingly, and make recovery mandatory for the in-process client. +This follow-up does not pull in #118's unrelated removal. diff --git a/src/cli.ts b/src/cli.ts index 21c6d54..b81efd6 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -2045,7 +2045,9 @@ walletCmd }); const answer = await new Promise((resolve) => { rl.question( - "This will fail expired mint quotes confirmed unpaid, reclaim old pending sends, and cancel prepared melts. Continue? [y/N] ", + options.force + ? "WARNING: --force fails expired mint quotes WITHOUT checking the mint and may strand paid sats. It also reclaims old pending sends and cancels prepared melts. Continue? [y/N] " + : "This will fail expired mint quotes confirmed unpaid, reclaim old pending sends, and cancel prepared melts. Continue? [y/N] ", (value: string) => { rl.close(); resolve(value.trim().toLowerCase()); @@ -2080,6 +2082,7 @@ walletCmd | { dryRun?: boolean; failedMintQuotes?: number; + mintQuoteCandidates?: number; leftForRecovery?: number; reclaimedSends?: number; cancelledMelts?: number; @@ -2091,9 +2094,13 @@ walletCmd if (output) { const prefix = output.dryRun ? "Would clean up:" : "Cleaned up:"; console.log(prefix); - console.log( - ` Expired mint quotes failed: ${output.failedMintQuotes ?? 0}`, - ); + if (output.dryRun) { + console.log( + ` Expired mint quote candidates (not checked with mint): ${output.mintQuoteCandidates ?? 0}`, + ); + } else { + console.log(` Expired mint quotes failed: ${output.failedMintQuotes ?? 0}`); + } console.log( ` Expired quotes kept for recovery (paid/issued/unverified): ${output.leftForRecovery ?? 0}`, ); @@ -2129,11 +2136,11 @@ walletCmd walletCmd .command("recover") .description( - "Re-issue PAID mint quotes whose sats were never claimed by checking each quote with its mint", + "Retry mint quotes using their stored outputs (does not replace rejected outputs)", ) .option( "--op ", - "Recover only this operation id (repeatable; required to target failed operations)", + "Recover this operation id (repeatable; find IDs with routstrd history --json)", (value: string, previous: string[]) => [...previous, value], [] as string[], ) diff --git a/src/daemon/http/index.ts b/src/daemon/http/index.ts index 7cee3d5..adb8ea6 100644 --- a/src/daemon/http/index.ts +++ b/src/daemon/http/index.ts @@ -289,10 +289,13 @@ function optionalStringArrayField( ): string[] | undefined { const value = body[field]; if (value === undefined) return undefined; - if (!Array.isArray(value) || value.some((item) => typeof item !== "string")) { - throw new CocodHttpError(400, `'${field}' must be an array of strings.`); + if ( + !Array.isArray(value) || + value.some((item) => typeof item !== "string" || !item.trim()) + ) { + throw new CocodHttpError(400, `'${field}' must be an array of non-empty strings.`); } - return value as string[]; + return value.map((item: string) => item.trim()); } function getCurrentMode(deps: DaemonDeps): ClientMode { @@ -502,13 +505,27 @@ export function createDaemonRequestHandler(deps: { } const body = await readJsonBody(req); + const operationIds = optionalStringArrayField(body, "operationIds"); + if (body.includeFailed === true && !operationIds?.length) { + throw new CocodHttpError( + 400, + "'includeFailed' requires non-empty 'operationIds'.", + ); + } + if ( + body.timeoutMs !== undefined && + (typeof body.timeoutMs !== "number" || + !Number.isFinite(body.timeoutMs) || body.timeoutMs <= 0) + ) { + throw new CocodHttpError( + 400, + "'timeoutMs' must be a positive finite number.", + ); + } const result = await deps.walletClient.recoverMintQuotes({ - operationIds: optionalStringArrayField(body, "operationIds"), + operationIds, includeFailed: body.includeFailed === true, - timeoutMs: - typeof body.timeoutMs === "number" && Number.isFinite(body.timeoutMs) - ? body.timeoutMs - : undefined, + timeoutMs: body.timeoutMs as number | undefined, }); return { output: result }; }); diff --git a/src/daemon/http/wallet-recovery.test.ts b/src/daemon/http/wallet-recovery.test.ts new file mode 100644 index 0000000..b49a984 --- /dev/null +++ b/src/daemon/http/wallet-recovery.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, mock } from "bun:test"; +import { EventEmitter } from "node:events"; +import { createDaemonRequestHandler } from "./index"; + +async function recover(body: unknown) { + const recoverMintQuotes = mock(async (_options: unknown) => ({ recovered: 0 })); + const handler = createDaemonRequestHandler({ walletClient: { recoverMintQuotes } } as never); + const req = new EventEmitter() as any; + Object.assign(req, { method: "POST", url: "/wallet/recover", headers: { host: "localhost" } }); + const res = { + status: 0, body: "", + writeHead(status: number) { this.status = status; }, + end(chunk: string) { this.body = chunk; }, + }; + setImmediate(() => { + req.emit("data", Buffer.from(JSON.stringify(body))); + req.emit("end"); + }); + await handler(req, res as never); + return { res, recoverMintQuotes }; +} + +describe("POST /wallet/recover validation", () => { + it.each([{}, { operationIds: [] }])("rejects includeFailed without explicit IDs: %j", async (body) => { + const { res, recoverMintQuotes } = await recover({ ...body, includeFailed: true }); + expect(res.status).toBe(400); + expect(recoverMintQuotes).not.toHaveBeenCalled(); + }); + it.each([[""], [" "], [42]])("rejects invalid operation IDs: %j", async (operationIds) => { + const { res, recoverMintQuotes } = await recover({ operationIds }); + expect(res.status).toBe(400); + expect(recoverMintQuotes).not.toHaveBeenCalled(); + }); + it.each([0, -1, "1000"])("rejects invalid timeout %j", async (timeoutMs) => { + const { res, recoverMintQuotes } = await recover({ timeoutMs }); + expect(res.status).toBe(400); + expect(recoverMintQuotes).not.toHaveBeenCalled(); + }); + it("passes normalized explicit IDs and a positive timeout", async () => { + const { res, recoverMintQuotes } = await recover({ operationIds: [" op-1 "], includeFailed: true, timeoutMs: 1000 }); + expect(res.status).toBe(200); + expect(recoverMintQuotes).toHaveBeenCalledWith({ operationIds: ["op-1"], includeFailed: true, timeoutMs: 1000 }); + }); + it("still permits checking pending quotes without IDs", async () => { + const { res, recoverMintQuotes } = await recover({}); + expect(res.status).toBe(200); + expect(recoverMintQuotes).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/daemon/wallet/cleanup.test.ts b/src/daemon/wallet/cleanup.test.ts index 381ab4e..b7a8568 100644 --- a/src/daemon/wallet/cleanup.test.ts +++ b/src/daemon/wallet/cleanup.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { selectCleanupOperations } from "./cleanup"; +import { selectCleanupOperations, summarizeMintCleanup } from "./cleanup"; const NOW_MS = 1_800_000_000_000; const DAY_MS = 24 * 60 * 60 * 1000; @@ -166,3 +166,14 @@ describe("selectCleanupOperations", () => { expect(result.meltsToCancel).toEqual([]); }); }); + +describe("mint cleanup reporting", () => { + it("reports dry-run candidates, not confirmed failures", () => { + expect(summarizeMintCleanup({ dryRun: true, candidates: 3, failed: 0, leftForRecovery: 0 })) + .toEqual({ mintQuoteCandidates: 3, failedMintQuotes: 0, leftForRecovery: 0 }); + }); + it("reports only actual failures in a real run", () => { + expect(summarizeMintCleanup({ dryRun: false, candidates: 3, failed: 1, leftForRecovery: 2 })) + .toEqual({ mintQuoteCandidates: 3, failedMintQuotes: 1, leftForRecovery: 2 }); + }); +}); diff --git a/src/daemon/wallet/cleanup.ts b/src/daemon/wallet/cleanup.ts index bbf24c2..1ff48ea 100644 --- a/src/daemon/wallet/cleanup.ts +++ b/src/daemon/wallet/cleanup.ts @@ -64,8 +64,8 @@ export interface CleanupSelection< * can have happened before expiry while the daemon was down, leaving no * local observation. Callers that fail quotes automatically at startup must * therefore confirm UNPAID with the mint first (see - * settleExpiredMintQuotes in coco-client.ts); only the explicit, - * user-invoked cleanup command may fail candidates purely locally. + * failExpiredMintQuoteIfUnpaid in coco-client.ts). Explicit cleanup follows + * the same rule unless the operator opts into unsafe `--force` behaviour. * - Pending sends are reclaimed (rolled back) only when they are older than * `minAgeMs`, so we never roll back a token that a receiver might still * legitimately claim. @@ -102,3 +102,17 @@ export function selectCleanupOperations< return { mintsToFail, sendsToReclaim, meltsToCancel }; } + +/** Keep a local-only dry-run preview distinct from mint-confirmed outcomes. */ +export function summarizeMintCleanup(input: { + dryRun: boolean; + candidates: number; + failed: number; + leftForRecovery: number; +}) { + return { + mintQuoteCandidates: input.candidates, + failedMintQuotes: input.dryRun ? 0 : input.failed, + leftForRecovery: input.dryRun ? 0 : input.leftForRecovery, + }; +} diff --git a/src/daemon/wallet/coco-client.test.ts b/src/daemon/wallet/coco-client.test.ts index fc03b5e..3711300 100644 --- a/src/daemon/wallet/coco-client.test.ts +++ b/src/daemon/wallet/coco-client.test.ts @@ -1333,6 +1333,28 @@ describe("runMintQuoteRecovery", () => { expect(result.errors).toHaveLength(1); }); + it("falls back to the original failure when diagnostic lookup fails", async () => { + const { source } = fakeSource([mintOp()], { + observe: async () => ({ category: "ready" }), + finalize: async () => { throw new Error("original failure"); }, + }); + source.ops.mint.get = async () => { throw new Error("lookup failed"); }; + const result = await runMintQuoteRecovery(source); + expect(result).toMatchObject({ retryable: 1, recovered: 0 }); + expect(result.errors).toEqual([{ operationId: "op-1", error: "original failure" }]); + }); + + it("bounds a hung diagnostic lookup after finalize fails", async () => { + const { source } = fakeSource([mintOp()], { + observe: async () => ({ category: "ready" }), + finalize: async () => { throw new Error("original failure"); }, + }); + source.ops.mint.get = () => new Promise(() => {}); + const result = await runMintQuoteRecovery(source, { timeoutMs: 20 }); + expect(result).toMatchObject({ retryable: 1, recovered: 0 }); + expect(result.errors).toEqual([{ operationId: "op-1", error: "original failure" }]); + }); + it("bounds finalize so one hung mint cannot block recovery", async () => { const { source } = fakeSource([mintOp()], { observe: async () => ({ category: "ready" }), @@ -1369,6 +1391,26 @@ describe("runMintQuoteRecovery", () => { expect(reopenFailedOperation).not.toHaveBeenCalled(); }); + it("requires explicit IDs when including failed operations", async () => { + const { source, reopenFailedOperation, observePendingOperation } = fakeSource([]); + await expect(runMintQuoteRecovery(source, { includeFailed: true })).rejects.toThrow( + "includeFailed requires explicit operationIds", + ); + await expect(runMintQuoteRecovery(source, { includeFailed: true, operationIds: [] })).rejects.toThrow( + "includeFailed requires explicit operationIds", + ); + expect(reopenFailedOperation).not.toHaveBeenCalled(); + expect(observePendingOperation).not.toHaveBeenCalled(); + }); + + it.each([0, -1, NaN, Infinity])("rejects invalid recovery timeout %s", async (timeoutMs) => { + const { source, observePendingOperation } = fakeSource([]); + await expect(runMintQuoteRecovery(source, { timeoutMs })).rejects.toThrow( + "timeoutMs must be a positive finite number", + ); + expect(observePendingOperation).not.toHaveBeenCalled(); + }); + it("re-opens a named failed operation, then mints it", async () => { const { source, reopenFailedOperation, finalize } = fakeSource( [mintOp({ state: "failed", lastObservedRemoteState: "PAID" })], diff --git a/src/daemon/wallet/coco-client.ts b/src/daemon/wallet/coco-client.ts index ebfec2d..900088d 100644 --- a/src/daemon/wallet/coco-client.ts +++ b/src/daemon/wallet/coco-client.ts @@ -36,7 +36,7 @@ import type { WalletCleanupResult, WalletRecoveryProgress, } from "./cocod-client"; -import { selectCleanupOperations } from "./cleanup"; +import { selectCleanupOperations, summarizeMintCleanup } from "./cleanup"; import { classifyMintQuoteObservation, selectMintQuotesForRecovery, @@ -1032,7 +1032,13 @@ export async function runMintQuoteRecovery( options: MintQuoteRecoveryOptions = {}, onProgress?: (message: string) => void, ): Promise { + if (options.includeFailed && !options.operationIds?.length) { + throw new Error("includeFailed requires explicit operationIds"); + } const timeoutMs = options.timeoutMs ?? MINT_QUOTE_RECOVERY_TIMEOUT_MS; + if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) { + throw new Error("timeoutMs must be a positive finite number"); + } const outstanding = options.outstanding ?? new Map>(); const result: MintQuoteRecoveryResult = { @@ -1215,7 +1221,18 @@ export async function runMintQuoteRecovery( onProgress?.(`${label}: another recovery is working on it; skipped`); } else { result.retryable++; - onProgress?.(`${label}: could not finish recovery: ${messageOf(error)}`); + // finalize can throw a generic "remains pending" error after coco has + // persisted the actionable mint rejection (for example inactive keyset). + const current = await withTimeout( + source.ops.mint.get(operationId), + remaining(), + ).catch(() => null); + const detail = current?.state === "pending" && current.error + ? current.error + : messageOf(error); + result.errors.push({ operationId, error: detail }); + onProgress?.(`${label}: could not finish recovery: ${detail}`); + return; } result.errors.push({ operationId, error: messageOf(error) }); return; @@ -2337,11 +2354,14 @@ export async function createCocoClient( } } - const failedMintQuoteCount = dryRun - ? selection.mintsToFail.length - : failedMintQuotes; + const mintSummary = summarizeMintCleanup({ + dryRun, + candidates: selection.mintsToFail.length, + failed: failedMintQuotes, + leftForRecovery, + }); const actedOn = - failedMintQuoteCount + + (dryRun ? mintSummary.mintQuoteCandidates : mintSummary.failedMintQuotes) + leftForRecovery + selection.sendsToReclaim.length + selection.meltsToCancel.length; @@ -2351,8 +2371,7 @@ export async function createCocoClient( return { dryRun, - failedMintQuotes: failedMintQuoteCount, - leftForRecovery, + ...mintSummary, reclaimedSends: selection.sendsToReclaim.length, cancelledMelts: selection.meltsToCancel.length, skipped, diff --git a/src/daemon/wallet/cocod-client.ts b/src/daemon/wallet/cocod-client.ts index b53bf88..1caee85 100644 --- a/src/daemon/wallet/cocod-client.ts +++ b/src/daemon/wallet/cocod-client.ts @@ -94,9 +94,11 @@ export interface WalletCleanupOptions { /** Summary of a wallet cleanup run. */ export interface WalletCleanupResult { dryRun: boolean; - /** Number of expired pending mint quotes marked as failed. */ + /** Expired quotes selected for checking; dry runs do not contact the mint. */ + mintQuoteCandidates: number; + /** Number actually marked failed (always zero in a dry run). */ failedMintQuotes: number; - /** Expired quotes whose mint reported PAID/ISSUED, left for recovery. */ + /** Expired quotes kept pending because they are paid/issued or unverified. */ leftForRecovery: number; /** Number of stale pending send operations reclaimed. */ reclaimedSends: number; diff --git a/src/daemon/wallet/mint-quote-recovery.fake-mint.test.ts b/src/daemon/wallet/mint-quote-recovery.fake-mint.test.ts index 20715b5..2554665 100644 --- a/src/daemon/wallet/mint-quote-recovery.fake-mint.test.ts +++ b/src/daemon/wallet/mint-quote-recovery.fake-mint.test.ts @@ -134,6 +134,36 @@ describe("PAID mint quote recovery with a real Manager and mint", () => { expect(await booted.spendable()).toBe(210_000); }); + it("existing coco recovery already issues expired paid pending quotes", async () => { + booted = await boot({ quoteExpiry: -60 }); + const op = await prepareQuote(booted, 100); + booted.mint.markPaid(op.quoteId as string); + await booted.manager.recoverPendingMintOperations(); + expect(await booted.spendable()).toBe(100); + expect(booted.mint.getQuote(op.quoteId as string)?.state).toBe("ISSUED"); + }); + + it("keeps rejected stored outputs and reports the actionable mint error", async () => { + booted = await boot({ quoteExpiry: -60 }); + const op = await prepareQuote(booted, 100); + const outputs = outputsOf(op); + booted.mint.markPaid(op.quoteId as string); + // Model the mint refusing the stored outputs, not invoice expiry. This is + // not evidence that the production quotes used an inactive keyset. + booted.mint.mintError = { code: 12001, detail: "keyset id inactive." }; + await booted.manager.recoverPendingMintOperations(); + expect(await booted.spendable()).toBe(0); + const result = await runMintQuoteRecovery(booted.source() as never, { + operationIds: [op.id as string], + }); + expect(result).toMatchObject({ recovered: 0, retryable: 1 }); + expect(result.errors.some((entry) => entry.error.includes("keyset id inactive"))).toBe(true); + expect(await booted.spendable()).toBe(0); + expect(booted.mint.getQuote(op.quoteId as string)?.state).toBe("PAID"); + expect(outputsOf(await booted.manager.ops.mint.get(op.id as string) as unknown as AnyRecord)).toEqual(outputs); + for (const request of booted.mint.requests) expect(request.outputs).toEqual(outputs); + }); + it("restores proofs for a quote already issued at the mint", async () => { booted = await boot({ quoteExpiry: null }); const op = await prepareQuote(booted, 210_000); diff --git a/src/daemon/wallet/mint-quote-recovery.ts b/src/daemon/wallet/mint-quote-recovery.ts index be53b95..eb620b2 100644 --- a/src/daemon/wallet/mint-quote-recovery.ts +++ b/src/daemon/wallet/mint-quote-recovery.ts @@ -5,11 +5,14 @@ * `pending` (the Lightning payment landed before expiry while the daemon was * down, so no local observation was ever recorded) or even terminally * `failed` (coco gives up when the mint refuses to sign, for example after the - * invoice expiry). The paid sats are claimable either way: NUT-04 lets the - * holder submit outputs for any quote id while `amount_issued < amount_paid`. + * invoice expiry). Claimability still depends on the mint accepting issuance. + * This feature retries the stored outputs or restores their signatures; it + * does not regenerate outputs rejected by the mint (for example an inactive + * keyset). coco already reconciles pending paid quotes at startup and in the + * periodic sweep. The new capability is operator-targeted recovery, including + * explicitly reopening failed operations, alongside safer cleanup. * - * Recovery therefore has to ask the mint what it thinks, then re-issue the - * quote. These helpers decide *what* to do from a remote observation; the + * Recovery asks the mint what it thinks, then retries issuance or restore. These helpers decide *what* to do from a remote observation; the * actual state transitions are applied by the in-process coco wallet client * so coco-core's operation services emit their normal events and release * proof reservations. Keeping the decisions here makes them unit testable