From a61d5c3a91100110b3a5720ccb81500c6ba9b890 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 4 Aug 2026 20:56:37 +0000 Subject: [PATCH] frozen coins are never spendable, and a rescan already running is adopted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit four wallet defects, none urgent, all cheap: selectCoins let an explicit coin-control pick override a freeze. naming an outpoint now overrides only the confirmed-only default; frozen is absolute. balances counted frozen coins in onchainConfirmed, so Send showed a figure a max-value spend could not reach. Balances gains onchainFrozen — a component of confirmed, not a deduction — filled at the route layer, because freezing is Officer policy in Postgres and no backend can see it. The route only reads utxos when something is actually frozen. Send subtracts it under "Spendable"; Overview lists it beside unconfirmed. a rescan in flight upstream was invisible after a sidecar restart, and a second POST would have queued behind it (scantxoutset is single-threaded node-wide). adoptRescan polls an existing NBXplorer scan instead of starting one, and the GET route falls back to it when local state is gone. the per-variant scan deadline counted queue time, so a variant that sat behind another wallet's scan timed out without ever having run. the deadline now refreshes while the status reads Queued. Co-Authored-By: Claude Opus 5 --- .../sidecar/wallet/backends/clnrest.ts | 3 +- src/servers/sidecar/wallet/backends/lnd.ts | 1 + src/servers/sidecar/wallet/backends/lndhub.ts | 1 + src/servers/sidecar/wallet/backends/nwc.ts | 1 + .../sidecar/wallet/backends/onchain.ts | 19 ++++++ .../wallet/chain-source-esplora.test.ts | 1 + .../sidecar/wallet/chain-source-nbxplorer.ts | 66 +++++++++++++++++-- src/servers/sidecar/wallet/chain-source.ts | 9 +++ src/servers/sidecar/wallet/psbt.ts | 11 +++- src/servers/sidecar/wallet/routes.ts | 32 ++++++++- src/servers/sidecar/wallet/types.ts | 24 ++++++- .../src/apps/Wallet/OverviewView.tsx | 22 +++++-- .../officerdev/src/apps/Wallet/SendView.tsx | 13 +++- .../officerdev/src/apps/Wallet/shared.ts | 6 ++ 14 files changed, 191 insertions(+), 18 deletions(-) diff --git a/src/servers/sidecar/wallet/backends/clnrest.ts b/src/servers/sidecar/wallet/backends/clnrest.ts index fb52ea91..8eabd636 100644 --- a/src/servers/sidecar/wallet/backends/clnrest.ts +++ b/src/servers/sidecar/wallet/backends/clnrest.ts @@ -550,7 +550,8 @@ export class ClnRestBackend extends BaseBackend { lightningInbound += Math.floor((total - ours) / 1000); } - return { onchainConfirmed, onchainUnconfirmed, lightningBalance, lightningInbound }; + // Core Lightning reserves coins itself; Officer freezing is the route's overlay, so 0 here. + return { onchainConfirmed, onchainUnconfirmed, onchainFrozen: 0, lightningBalance, lightningInbound }; } // ── on-chain ─────────────────────────────────────────────────────────────────────────────────── diff --git a/src/servers/sidecar/wallet/backends/lnd.ts b/src/servers/sidecar/wallet/backends/lnd.ts index 304b2f0f..5ce0483e 100644 --- a/src/servers/sidecar/wallet/backends/lnd.ts +++ b/src/servers/sidecar/wallet/backends/lnd.ts @@ -460,6 +460,7 @@ export class LndBackend extends BaseBackend { ]); return { onchainConfirmed: toSats(chain.confirmed_balance), + onchainFrozen: 0, onchainUnconfirmed: toSats(chain.unconfirmed_balance), // `balance` is the deprecated flat field; the nested Amount is authoritative on v0.11+. lightningBalance: toSats(channels.local_balance?.sat ?? channels.balance), diff --git a/src/servers/sidecar/wallet/backends/lndhub.ts b/src/servers/sidecar/wallet/backends/lndhub.ts index 5baf8c3b..8e020e76 100644 --- a/src/servers/sidecar/wallet/backends/lndhub.ts +++ b/src/servers/sidecar/wallet/backends/lndhub.ts @@ -341,6 +341,7 @@ export class LndHubBackend extends BaseBackend { return { // A custodial account has no on-chain balance of its own; deposits land in the lightning balance. onchainConfirmed: 0, + onchainFrozen: 0, onchainUnconfirmed: 0, lightningBalance: num(res?.BTC?.AvailableBalance), // Inbound liquidity is the operator's problem and is never reported. diff --git a/src/servers/sidecar/wallet/backends/nwc.ts b/src/servers/sidecar/wallet/backends/nwc.ts index f262d385..478443f3 100644 --- a/src/servers/sidecar/wallet/backends/nwc.ts +++ b/src/servers/sidecar/wallet/backends/nwc.ts @@ -278,6 +278,7 @@ export class NwcBackend extends BaseBackend { return { // NWC is lightning-only; there is no on-chain side to report. onchainConfirmed: 0, + onchainFrozen: 0, onchainUnconfirmed: 0, // `balance` is msats (the WebLN shim Zeus uses divides by 1000 before the UI ever sees it). lightningBalance: Math.floor((res.balance ?? 0) / 1000), diff --git a/src/servers/sidecar/wallet/backends/onchain.ts b/src/servers/sidecar/wallet/backends/onchain.ts index 69a26f89..32902a41 100644 --- a/src/servers/sidecar/wallet/backends/onchain.ts +++ b/src/servers/sidecar/wallet/backends/onchain.ts @@ -339,6 +339,23 @@ export class OnchainBackend extends BaseBackend { return state; } + /** + * Re-attach to a rescan that outlived this process, and put the refresh back on the end of it. + * + * Read-only with respect to the upstream: it never starts a scan, so calling it when nothing is running + * costs one request per account and answers null. + */ + async adoptRescan(): Promise { + const adopt = this.chain.adoptRescan?.bind(this.chain); + if (!adopt) return null; + const handle = await adopt(this.scanContext().accounts); + if (!handle) return null; + // The same chaining startRescan does, and for the same reason: coins recovered by a scan are invisible + // until the snapshot in front of them is rebuilt. + void handle.done.then(() => this.refresh()).catch(() => {}); + return handle.state; + } + /** * The wallet's view of the chain, stale-while-revalidate. * @@ -528,6 +545,8 @@ export class OnchainBackend extends BaseBackend { } return { onchainConfirmed: confirmed, + // Filled in by the route, which is the only layer that knows what the owner froze. + onchainFrozen: 0, onchainUnconfirmed: unconfirmed, // No channels exist, and null is the contract's "this backend has no lightning" value. lightningBalance: null, diff --git a/src/servers/sidecar/wallet/chain-source-esplora.test.ts b/src/servers/sidecar/wallet/chain-source-esplora.test.ts index d060f245..add5d585 100644 --- a/src/servers/sidecar/wallet/chain-source-esplora.test.ts +++ b/src/servers/sidecar/wallet/chain-source-esplora.test.ts @@ -173,6 +173,7 @@ describe('EsploraChainSource through OnchainBackend', () => { expect(await backend.getBalances()).toEqual({ onchainConfirmed: 0, onchainUnconfirmed: 0, + onchainFrozen: 0, lightningBalance: null, lightningInbound: null, }); diff --git a/src/servers/sidecar/wallet/chain-source-nbxplorer.ts b/src/servers/sidecar/wallet/chain-source-nbxplorer.ts index 2c61755b..3f26a4fc 100644 --- a/src/servers/sidecar/wallet/chain-source-nbxplorer.ts +++ b/src/servers/sidecar/wallet/chain-source-nbxplorer.ts @@ -203,11 +203,51 @@ export class NbxplorerChainSource implements WalletChainSource { return { state, done: this.rescanRun }; } - private async runRescan(accounts: readonly ScanAccount[], state: RescanState): Promise { + /** + * Pick up a scan that survived a restart of this sidecar. + * + * `scantxoutset` runs on the node and outlives us; only the handle to it was ever in memory. So the + * upstream is asked directly, and if anything is Queued or Pending the run is rebuilt around the same + * polling loop with starting suppressed — a duplicate POST would sit behind the real scan for its whole + * duration, since the node runs one at a time. + * + * `found` restarts from zero and a variant that finished before the restart contributes nothing to it: + * its result has expired upstream and there is no way to recover the count. The number under-reports + * for that one run, which is the honest answer — and the refresh chained on the end is what actually + * puts the coins on screen. + */ + async adoptRescan(accounts: readonly ScanAccount[]): Promise { + if (this.rescan?.running && this.rescanRun) return { state: this.rescan, done: this.rescanRun }; + + const live = await Promise.all( + accounts.map(async (account) => { + // A probe failing is not an answer either way, and must not be reported as "a scan is running". + const status = await this.chain.getUtxoScanStatus(account.accountXpub, account.type).catch(() => null); + return status?.status === 'Queued' || status?.status === 'Pending'; + }), + ); + if (!live.some(Boolean)) return null; + + const state: RescanState = { + running: true, + done: 0, + total: accounts.length, + current: accounts[0]?.type ?? null, + found: 0, + startedAt: Date.now(), + finishedAt: null, + error: null, + }; + this.rescan = state; + this.rescanRun = this.runRescan(accounts, state, false); + return { state, done: this.rescanRun }; + } + + private async runRescan(accounts: readonly ScanAccount[], state: RescanState, start = true): Promise { try { for (const account of accounts) { state.current = account.type; - state.found += await this.scanVariant(account); + state.found += start ? await this.scanVariant(account) : await this.awaitVariant(account); state.done += 1; } } catch (err) { @@ -224,10 +264,25 @@ 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 { - const { accountXpub, type } = account; - await this.chain.startUtxoScan(accountXpub, type); + await this.chain.startUtxoScan(account.accountXpub, account.type); + return this.awaitVariant(account); + } - const deadline = Date.now() + SCAN_TIMEOUT_MS; + /** + * Wait out a scan that is already queued upstream, without starting one. + * + * Split out of scanVariant for adoption: a scan this process did not start must never be re-queued, + * because scantxoutset is single-threaded node-wide and a duplicate would sit behind the real one for + * its whole duration. An account that already finished answers 404 or Complete and returns at once. + */ + private async awaitVariant(account: ScanAccount): Promise { + const { accountXpub, type } = account; + + // The timeout bounds how long a scan may RUN, not how long it may wait its turn. scantxoutset is + // single-threaded node-wide, so four variants queue behind each other and the last one could spend + // most of its ten minutes not started — timing out while the node was never asked to do anything + // wrong. The deadline is therefore pushed forward for as long as the status reads Queued. + let deadline = Date.now() + SCAN_TIMEOUT_MS; let found = 0; let seen = false; let nulls = 0; @@ -249,6 +304,7 @@ export class NbxplorerChainSource implements WalletChainSource { nulls = 0; found = status.progress?.found ?? found; + if (status.status === 'Queued') deadline = Date.now() + SCAN_TIMEOUT_MS; if (status.status === 'Complete') return found; if (status.status === 'Error') { throw new BackendError(`utxo scan of the ${type} account failed: ${status.error ?? 'unknown'}`, 502, 'SCAN'); diff --git a/src/servers/sidecar/wallet/chain-source.ts b/src/servers/sidecar/wallet/chain-source.ts index 2667ad07..f20f10c0 100644 --- a/src/servers/sidecar/wallet/chain-source.ts +++ b/src/servers/sidecar/wallet/chain-source.ts @@ -138,6 +138,15 @@ export interface WalletChainSource { */ startRescan?(accounts: readonly ScanAccount[]): RescanHandle; + /** + * Find a scan already in flight upstream and return a handle to it, without starting one. + * + * Same handle contract as `startRescan`, so the caller chains its refresh onto `done` identically — + * which is the point: an adopted scan has to end in a snapshot rebuild too, or the coins it recovered + * stay invisible. Null when nothing is running. + */ + adoptRescan?(accounts: readonly ScanAccount[]): Promise; + /** The rescan in flight, or the last one's outcome. Null when this source has never run one. */ rescanState?(): RescanState | null; } diff --git a/src/servers/sidecar/wallet/psbt.ts b/src/servers/sidecar/wallet/psbt.ts index 17304a34..75ea41d0 100644 --- a/src/servers/sidecar/wallet/psbt.ts +++ b/src/servers/sidecar/wallet/psbt.ts @@ -268,10 +268,15 @@ export function selectCoins(params: CoinSelectionParams): CoinSelection { const allow = params.outpoints && params.outpoints.length > 0 ? new Set(params.outpoints) : null; const eligible = params.utxos.filter((u) => { - // An explicit coin-control pick is authoritative: it overrides both the frozen flag and the - // confirmed-only default, because the user named this exact outpoint. - if (allow) return allow.has(outpointOf(u)); + // Frozen is never overridable, not even by naming the outpoint. Freezing is the owner saying "this + // coin does not exist for spending purposes" — a reservation against an open channel, a coin held + // for tax lots, dust being kept out of the way — and a rule that a sufficiently explicit request + // can bypass is not a rule. The route refuses a named frozen outpoint before ever reaching here; + // this is what makes that true for any future caller too. if (u.frozen) return false; + // An explicit coin-control pick is otherwise authoritative: it overrides the confirmed-only + // default, because the user named this exact outpoint and can see what it is. + if (allow) return allow.has(outpointOf(u)); if (!params.spendUnconfirmed && u.confirmations < 1) return false; return true; }); diff --git a/src/servers/sidecar/wallet/routes.ts b/src/servers/sidecar/wallet/routes.ts index f775db1b..0f189c78 100644 --- a/src/servers/sidecar/wallet/routes.ts +++ b/src/servers/sidecar/wallet/routes.ts @@ -33,6 +33,7 @@ import { BIP_ADDRESS_TYPE, type BitcoinNetwork, type Capability, + type Balances, type SyncState, type Utxo, type WalletBackend, @@ -252,7 +253,7 @@ async function handleWallets(ctx: OfficerContext, seg: string[]): Promise { + const balances = await backend.getBalances(); + const frozenList = await getFrozenOutpoints(walletId); + if (frozenList.length === 0) return balances; + + const frozen = new Set(frozenList); + const utxos = await backend.getUtxos(); + // Confirmed only, to match what it is a component of. An unconfirmed frozen coin is not spendable for + // a reason that has nothing to do with freezing. + const sats = utxos + .filter((u) => frozen.has(`${u.txid}:${u.vout}`) && u.confirmations > 0) + .reduce((sum, u) => sum + u.amountSats, 0); + return { ...balances, onchainFrozen: sats }; +} + // ── utxos ──────────────────────────────────────────────────────────────────────────────────────── async function utxosRoute( diff --git a/src/servers/sidecar/wallet/types.ts b/src/servers/sidecar/wallet/types.ts index 5e0e91f3..3e9f6f58 100644 --- a/src/servers/sidecar/wallet/types.ts +++ b/src/servers/sidecar/wallet/types.ts @@ -57,9 +57,18 @@ export type NodeInfo = { }; export type Balances = { - /** Spendable on-chain, in sats. */ + /** Confirmed on-chain, in sats. Includes anything frozen — see `onchainFrozen`. */ onchainConfirmed: number; onchainUnconfirmed: number; + /** + * How much of `onchainConfirmed` sits in UTXOs the owner froze, and so cannot be spent. + * + * A component of the confirmed figure rather than a deduction from it, because the coins are still + * the wallet's — freezing is a spending policy, not a loss. Officer's overlay: freezing lives in + * Postgres and no backend knows about it, so every backend reports 0 and the route fills it in. + * Without this the UI shows a balance a max-value send then refuses to spend, with no visible reason. + */ + onchainFrozen: number; /** Sum of local balance across active channels, in sats. Null when the backend has no channels. */ lightningBalance: number | null; /** Sum of remote balance — i.e. inbound liquidity — in sats. */ @@ -316,6 +325,19 @@ export interface WalletBackend { * rescan (a node backend already knows its own coins) or the source has no such endpoint. */ startRescan?(): Promise; + + /** + * Re-attach to a rescan that is running upstream but was not started by this process. + * + * A deep rescan takes minutes and lives on the indexer, not here — so restarting the sidecar mid-scan + * used to lose the wallet's only handle on it. `getSyncState().rescan` read back null, the UI showed no + * scan at all, and the refresh chained onto its completion went with it, leaving the balance stale + * until the cache TTL happened to expire. This asks the upstream what is actually in flight. + * + * Resolves null when nothing is running. Costs an upstream call, so callers reach for it only when + * there is no local state to report. + */ + adoptRescan?(): Promise; } /** diff --git a/src/workspaces/officerdev/src/apps/Wallet/OverviewView.tsx b/src/workspaces/officerdev/src/apps/Wallet/OverviewView.tsx index 41c854f2..85a2b929 100644 --- a/src/workspaces/officerdev/src/apps/Wallet/OverviewView.tsx +++ b/src/workspaces/officerdev/src/apps/Wallet/OverviewView.tsx @@ -1,4 +1,5 @@ import type { ReactNode } from 'react'; +import type { Balances } from './shared'; import { ArrowDownLeft, ArrowUpRight, Bitcoin, Clock, Loader2, Zap } from 'lucide-react'; import { Link } from 'react-router'; import { KIND_LABELS, walletSectionPath } from './shared'; @@ -14,6 +15,21 @@ import { useBalances, useCapabilities, useTransactions, useWalletInfo } from './ // Every number here is a read, so the whole screen works against a locked wallet — which is its resting // state. Nothing on this page prompts for a passphrase. +/** + * The sub-line under the on-chain figure. Unconfirmed and frozen are both reasons the headline number is + * not what a send can use, and both can be true at once, so they read as a list rather than a choice. + */ +function onchainHint(balances: Balances | null | undefined): string | undefined { + if (!balances) return undefined; + const parts: string[] = []; + if (balances.onchainUnconfirmed !== 0) { + const sign = balances.onchainUnconfirmed > 0 ? '+' : ''; + parts.push(`${sign}${formatSats(balances.onchainUnconfirmed)} sats unconfirmed`); + } + if (balances.onchainFrozen > 0) parts.push(`${formatSats(balances.onchainFrozen)} sats frozen`); + return parts.length ? parts.join(' · ') : undefined; +} + export const OverviewView = () => { const { wallet, walletId, isLoading } = useSelectedWallet(); const { capabilities } = useCapabilities(walletId); @@ -55,11 +71,7 @@ export const OverviewView = () => { icon={} label="On-chain" value={} - hint={ - balances && balances.onchainUnconfirmed !== 0 - ? `${balances.onchainUnconfirmed > 0 ? '+' : ''}${formatSats(balances.onchainUnconfirmed)} sats unconfirmed` - : undefined - } + hint={onchainHint(balances)} /> {hasLightning && ( {

Spendable

- + {/* Frozen coins are confirmed and are the wallet's, but coin selection will not touch them. This + header said "Spendable" over a figure that included them, so a max-value send was rejected as + having no spendable UTXOs while the number above it said otherwise. */} +
+ {!!balances?.onchainFrozen && ( +

+ {formatSats(balances.onchainFrozen)} sats frozen and excluded +

+ )}
diff --git a/src/workspaces/officerdev/src/apps/Wallet/shared.ts b/src/workspaces/officerdev/src/apps/Wallet/shared.ts index 868c2cdc..8fe48948 100644 --- a/src/workspaces/officerdev/src/apps/Wallet/shared.ts +++ b/src/workspaces/officerdev/src/apps/Wallet/shared.ts @@ -106,6 +106,12 @@ export type NodeInfo = { export type Balances = { onchainConfirmed: number; onchainUnconfirmed: number; + /** + * The part of `onchainConfirmed` the owner has frozen. Already counted in it, not deducted — the coins + * are still the wallet's; freezing is a spending policy. Subtract it to get what a send can actually + * use, which is what "Spendable" has to mean or a max-value send fails for no visible reason. + */ + onchainFrozen: number; /** Null when the backend has no channels of its own. */ lightningBalance: number | null; lightningInbound: number | null;