From ddc982ce5b811139648eadcd501a2c33314d7b8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 4 Aug 2026 21:00:24 +0000 Subject: [PATCH] a rescan can be told how deep to look, and a node wallet cannot be deleted blind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the gap limit was 1000, hardcoded, with the comment noting that raising it costs node CPU and not correctness — and no way to raise it. it is the one thing about a scan only the owner can know: how many addresses their old wallet handed out and never had paid. RescanOptions threads from the POST body through the backend and the source to utxos/scan, capped at 100k because past that the scan takes longer than anyone waits. the card gets a "search depth" field beside the button, blank meaning the default. deleting a wallet with no seed took one unconfirmed request. the dialog asked for the wallet's name and then threw the answer away, so the check existed only for whoever went through the dialog — a node wallet still holds the credential, the labels and the freezes. confirmName now travels with the request and the route enforces it. a bodyless DELETE is told which field is missing rather than that its JSON did not parse. Co-Authored-By: Claude Opus 5 --- .../sidecar/wallet/backends/onchain.ts | 5 ++- .../sidecar/wallet/chain-source-nbxplorer.ts | 20 ++++++--- src/servers/sidecar/wallet/chain-source.ts | 7 ++- src/servers/sidecar/wallet/routes.ts | 24 +++++++++- src/servers/sidecar/wallet/types.ts | 13 +++++- .../officerdev/src/apps/Wallet/RescanCard.tsx | 45 +++++++++++++++---- .../Wallet/dialogs/DeleteWalletDialog.tsx | 10 ++++- .../src/apps/Wallet/useWalletData.ts | 15 +++++-- 8 files changed, 112 insertions(+), 27 deletions(-) diff --git a/src/servers/sidecar/wallet/backends/onchain.ts b/src/servers/sidecar/wallet/backends/onchain.ts index 32902a41..1f86e329 100644 --- a/src/servers/sidecar/wallet/backends/onchain.ts +++ b/src/servers/sidecar/wallet/backends/onchain.ts @@ -61,6 +61,7 @@ import { type NewAddressRequest, type NodeInfo, type OnchainTx, + type RescanOptions, type RescanState, type SendCoinsRequest, type SendCoinsResult, @@ -326,10 +327,10 @@ export class OnchainBackend extends BaseBackend { * nothing visible until the snapshot in front of it is rebuilt, and without this the owner would be * staring at the same zero until the TTL happened to expire. */ - async startRescan(): Promise { + async startRescan(opts: RescanOptions = {}): Promise { const start = this.chain.startRescan?.bind(this.chain); if (!start) this.notSupported('rescanning the chain'); - const { state, done } = start(this.scanContext().accounts); + const { state, done } = start(this.scanContext().accounts, opts); void done .then(() => this.refresh()) // The rescan's own failure is already on its state and reported through getSyncState; a failed diff --git a/src/servers/sidecar/wallet/chain-source-nbxplorer.ts b/src/servers/sidecar/wallet/chain-source-nbxplorer.ts index 3f26a4fc..0757ee49 100644 --- a/src/servers/sidecar/wallet/chain-source-nbxplorer.ts +++ b/src/servers/sidecar/wallet/chain-source-nbxplorer.ts @@ -36,6 +36,7 @@ import { type BitcoinNetwork, type FeeEstimates, type OnchainTx, + type RescanOptions, type RescanState, } from './types'; @@ -184,7 +185,7 @@ export class NbxplorerChainSource implements WalletChainSource { * * A second call while one is running returns the same handle rather than queueing a duplicate. */ - startRescan(accounts: readonly ScanAccount[]): RescanHandle { + startRescan(accounts: readonly ScanAccount[], opts: RescanOptions = {}): RescanHandle { const existing = this.rescan; if (existing?.running && this.rescanRun) return { state: existing, done: this.rescanRun }; @@ -199,7 +200,7 @@ export class NbxplorerChainSource implements WalletChainSource { error: null, }; this.rescan = state; - this.rescanRun = this.runRescan(accounts, state); + this.rescanRun = this.runRescan(accounts, state, opts); return { state, done: this.rescanRun }; } @@ -239,15 +240,20 @@ export class NbxplorerChainSource implements WalletChainSource { error: null, }; this.rescan = state; - this.rescanRun = this.runRescan(accounts, state, false); + this.rescanRun = this.runRescan(accounts, state, null); return { state, done: this.rescanRun }; } - private async runRescan(accounts: readonly ScanAccount[], state: RescanState, start = true): Promise { + /** `opts` null means adopt: poll the variants without ever queueing one. */ + private async runRescan( + accounts: readonly ScanAccount[], + state: RescanState, + opts: RescanOptions | null, + ): Promise { try { for (const account of accounts) { state.current = account.type; - state.found += start ? await this.scanVariant(account) : await this.awaitVariant(account); + state.found += opts ? await this.scanVariant(account, opts) : await this.awaitVariant(account); state.done += 1; } } catch (err) { @@ -263,8 +269,8 @@ export class NbxplorerChainSource implements WalletChainSource { } /** Queue one variant's scan and wait it out. Resolves with how many UTXOs it pulled in. */ - private async scanVariant(account: ScanAccount): Promise { - await this.chain.startUtxoScan(account.accountXpub, account.type); + private async scanVariant(account: ScanAccount, opts: RescanOptions): Promise { + await this.chain.startUtxoScan(account.accountXpub, account.type, { gapLimit: opts.gapLimit }); return this.awaitVariant(account); } diff --git a/src/servers/sidecar/wallet/chain-source.ts b/src/servers/sidecar/wallet/chain-source.ts index f20f10c0..681bb578 100644 --- a/src/servers/sidecar/wallet/chain-source.ts +++ b/src/servers/sidecar/wallet/chain-source.ts @@ -16,7 +16,7 @@ // about which addresses are the wallet's, which is the kind of bug that loses coins rather than failing. import type { SpendableUtxo } from './psbt'; -import type { AddressType, FeeEstimates, OnchainTx, RescanState } from './types'; +import type { AddressType, FeeEstimates, OnchainTx, RescanOptions, RescanState } from './types'; /** BIP44 chain index: 0 is the receive chain, 1 the internal (change) chain. */ export type ChainIndex = 0 | 1; @@ -132,11 +132,14 @@ export interface WalletChainSource { * Search the chain for this wallet's coins from scratch, rather than from whenever the upstream * started watching it. Returns immediately — a rescan takes minutes, and the caller is an HTTP route. * + * `opts.gapLimit` widens the search past the source's own default, for a wallet restored from one that + * issued addresses in bulk. Ignored by a source with no such knob. + * * Optional, and Esplora does not implement it — it has nothing to rescan, because a gap-limit walk * already asks about every address every time. This exists for an upstream that *indexes*, where a * newly registered account starts empty and stays empty until told to go and look. */ - startRescan?(accounts: readonly ScanAccount[]): RescanHandle; + startRescan?(accounts: readonly ScanAccount[], opts?: RescanOptions): RescanHandle; /** * Find a scan already in flight upstream and return a handle to it, without starting one. diff --git a/src/servers/sidecar/wallet/routes.ts b/src/servers/sidecar/wallet/routes.ts index 0f189c78..91749d54 100644 --- a/src/servers/sidecar/wallet/routes.ts +++ b/src/servers/sidecar/wallet/routes.ts @@ -139,6 +139,9 @@ function handleConfig(): Response { const KINDS: readonly WalletKind[] = ['onchain', 'lnd', 'cln-rest', 'lndhub', 'nwc']; +/** Highest gap limit a rescan may be asked for. Four variants at this width is already tens of minutes. */ +const MAX_GAP_LIMIT = 100_000; + async function handleWallets(ctx: OfficerContext, seg: string[]): Promise { const { req, userId } = ctx; @@ -297,7 +300,14 @@ async function handleWallets(ctx: OfficerContext, seg: string[]): Promise(ctx.req); + if (gapLimit !== undefined && (!Number.isInteger(gapLimit) || gapLimit < 1 || gapLimit > MAX_GAP_LIMIT)) { + return badRequest(`gapLimit must be a whole number between 1 and ${MAX_GAP_LIMIT}`); + } + return json({ rescan: await backend.startRescan({ gapLimit }) }); } case 'fees': @@ -491,16 +501,26 @@ async function deleteWalletRoute(ctx: OfficerContext, walletId: number): Promise const wallet = await getWallet(ctx.userId, walletId); if (!wallet) return json({ error: 'wallet not found' }, 404); + // A bodyless DELETE is a caller that skipped the confirmation, not a malformed request — it should be + // told which field is missing rather than that its JSON did not parse. + type DeleteBody = { passphrase?: string; confirmName?: string }; + const { passphrase, confirmName } = await body(ctx.req).catch((): DeleteBody => ({})); + // Deleting a seeded wallet destroys the only copy of the key material Officer holds. Require the // passphrase, even when the wallet is already unlocked — an open session must not be enough. if (wallet.hasSeed) { - const { passphrase } = await body<{ passphrase?: string }>(ctx.req); if (!passphrase) return badRequest('passphrase is required to delete a seeded wallet'); const sealed = await getSealedSeed(ctx.userId, walletId); if (!sealed) throw new BackendError('wallet seed is missing', 500, 'NO_SEED'); if (!(await verifyPassphrase(walletId, JSON.parse(sealed) as SeedEnvelope, passphrase))) { return json({ error: 'incorrect passphrase' }, 401); } + } else if (confirmName?.trim() !== wallet.name) { + // A wallet with no seed of its own still holds the node credential, the labels and the freezes, and + // it deleted on one unconfirmed request — the name check lived only in the dialog, so anything that + // was not the dialog skipped it. Naming the wallet is the same bar the UI already asks for, now + // enforced where it cannot be bypassed. + return badRequest('confirmName must match the wallet name'); } sessionFor(walletId).lock(); diff --git a/src/servers/sidecar/wallet/types.ts b/src/servers/sidecar/wallet/types.ts index 3e9f6f58..cf04561f 100644 --- a/src/servers/sidecar/wallet/types.ts +++ b/src/servers/sidecar/wallet/types.ts @@ -324,7 +324,7 @@ export interface WalletBackend { * Optional because only a wallet-level chain source can do it. Absent means the backend has nothing to * rescan (a node backend already knows its own coins) or the source has no such endpoint. */ - startRescan?(): Promise; + startRescan?(opts?: RescanOptions): Promise; /** * Re-attach to a rescan that is running upstream but was not started by this process. @@ -364,6 +364,17 @@ export type SyncState = { rescan: RescanState | null; }; +/** + * How far a deep rescan should look, when the default is not far enough. + * + * The default gap limit is 1000 consecutive unused addresses, which covers any wallet a person has used + * by hand. It does not cover one restored from a wallet that issued addresses in bulk — and there is no + * way to detect that case, because the whole problem is that the coins beyond the gap are invisible. + * Raising it costs the node CPU on one scan; leaving it too low costs the owner the coins. So the knob is + * exposed rather than tuned, and the caller who knows their history is the one who sets it. + */ +export type RescanOptions = { gapLimit?: number }; + /** * A deep chain rescan — the upstream searching the whole UTXO set for a wallet's coins, rather than * indexing forward from the moment the wallet was registered. diff --git a/src/workspaces/officerdev/src/apps/Wallet/RescanCard.tsx b/src/workspaces/officerdev/src/apps/Wallet/RescanCard.tsx index cb0ee806..0bfe01a0 100644 --- a/src/workspaces/officerdev/src/apps/Wallet/RescanCard.tsx +++ b/src/workspaces/officerdev/src/apps/Wallet/RescanCard.tsx @@ -1,5 +1,7 @@ +import { useState } from 'react'; import { CheckCircle2, Loader2, Search, TriangleAlert } from 'lucide-react'; import { Button } from '@/components/ui/button'; +import { Input } from '@/components/ui/input'; import { ADDRESS_TYPE_LABELS } from './shared'; import { useBalances, useWalletOperations } from './useWalletData'; @@ -22,6 +24,7 @@ type RescanCardProps = { walletId: number }; export const RescanCard = ({ walletId }: RescanCardProps) => { const { sync } = useBalances(walletId); const { rescan } = useWalletOperations(walletId); + const [gapLimit, setGapLimit] = useState(''); // `sync` null is a node backend, which owns its own coins and has nothing to look for. `canRescan` // false is Esplora. Note this cannot key off `sync.rescan` being null — that is equally what NBXplorer @@ -30,6 +33,8 @@ export const RescanCard = ({ walletId }: RescanCardProps) => { const state = sync.rescan; const running = state?.running === true; + const parsed = Number(gapLimit); + const depth = gapLimit.trim() && Number.isInteger(parsed) && parsed > 0 ? parsed : null; return (
@@ -42,14 +47,38 @@ export const RescanCard = ({ walletId }: RescanCardProps) => { zero until it has looked. It scans all four address types, one at a time, and takes a few minutes.

- +
+ + + {/* The one thing about a scan only the owner can know: how many addresses their old wallet handed + out and never had paid. 1000 covers anyone who clicked "new address" by hand; a wallet that + issued them in bulk needs more, and no amount of looking from here can tell which it was. */} + +
{state && }
diff --git a/src/workspaces/officerdev/src/apps/Wallet/dialogs/DeleteWalletDialog.tsx b/src/workspaces/officerdev/src/apps/Wallet/dialogs/DeleteWalletDialog.tsx index c3440800..c8822ee8 100644 --- a/src/workspaces/officerdev/src/apps/Wallet/dialogs/DeleteWalletDialog.tsx +++ b/src/workspaces/officerdev/src/apps/Wallet/dialogs/DeleteWalletDialog.tsx @@ -16,7 +16,9 @@ import { useWalletLifecycle } from '../useWalletData'; // Delete a wallet. For a seeded wallet this destroys the only copy of the key material Officer holds, so // the sidecar demands the passphrase even when the wallet is currently unlocked — an open session must not -// be enough to erase a seed. Typing the wallet's name is the second, local check against the wrong row. +// be enough to erase a seed. Typing the wallet's name is the second check against the wrong row, and it is +// sent rather than merely enforced here: a wallet with no seed has no passphrase gate, so without it the +// only thing standing between a node wallet and deletion was a dialog anything could route around. type DeleteWalletDialogProps = { open: boolean; @@ -44,7 +46,11 @@ export const DeleteWalletDialog = ({ open, onOpenChange, wallet, onDeleted }: De ev.preventDefault(); if (!canSubmit) return; try { - await remove.mutateAsync({ walletId: wallet.id, passphrase: wallet.hasSeed ? passphrase : undefined }); + await remove.mutateAsync({ + walletId: wallet.id, + confirmName: confirmName.trim(), + passphrase: wallet.hasSeed ? passphrase : undefined, + }); close(); onDeleted?.(); } finally { diff --git a/src/workspaces/officerdev/src/apps/Wallet/useWalletData.ts b/src/workspaces/officerdev/src/apps/Wallet/useWalletData.ts index ee18f572..731ad089 100644 --- a/src/workspaces/officerdev/src/apps/Wallet/useWalletData.ts +++ b/src/workspaces/officerdev/src/apps/Wallet/useWalletData.ts @@ -369,8 +369,16 @@ export function useWalletLifecycle() { const remove = useMutation({ // The passphrase is required for a seeded wallet even when it is already unlocked — an open session // must not be enough to destroy the only copy of the key material. - mutationFn: ({ walletId, passphrase }: { walletId: number; passphrase?: string }) => - del<{ ok: true }>(`/wallet/_officer/wallets/${walletId}`, passphrase ? { passphrase } : {}), + mutationFn: ({ + walletId, + passphrase, + confirmName, + }: { + walletId: number; + passphrase?: string; + confirmName: string; + }) => + del<{ ok: true }>(`/wallet/_officer/wallets/${walletId}`, { confirmName, ...(passphrase ? { passphrase } : {}) }), onSuccess: () => { qc.invalidateQueries({ queryKey: ROOT_KEY }); toast.success('Wallet deleted'); @@ -519,7 +527,8 @@ export function useWalletOperations(walletId: number | null) { // of the balances poll, which speeds up on its own while one is running, so the invalidation here is // only to put the first "scanning…" on screen without waiting out the current interval. const rescan = useMutation({ - mutationFn: () => post<{ rescan: RescanState }>(`${base(walletId!)}/rescan`, {}), + mutationFn: (input: { gapLimit?: number } = {}) => + post<{ rescan: RescanState }>(`${base(walletId!)}/rescan`, input), onSuccess: () => { qc.invalidateQueries({ queryKey: [...ROOT_KEY, 'balances', walletId] }); toast.success('Scanning the chain for this wallet’s coins — this takes a few minutes');