honour frozen coins in automatic selection, not just coin control
freezing a coin promised it would not be spent, and the promise only held when the caller named outpoints explicitly. an ordinary send picked its own inputs from a snapshot in which every utxo said frozen: false — the chain has no idea what officer froze — so selectCoins, which has always filtered on that flag, never saw one set. sendAll was the worst case: "send everything" swept the frozen coin too. the frozen list now travels with the request, set by the route and overwritten if a caller supplies one. it can only ever restrict what is spendable, so smuggling a value in gains nothing. the backend still reads no officer table. three tests pin it, including a control that sends successfully once the coin is unfrozen — without it the other two would pass on a wallet that could not spend at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -585,8 +585,16 @@ export class OnchainBackend extends BaseBackend {
|
||||
// that may already be gone, and the failure would arrive as a broadcast rejection after signing.
|
||||
const snap = await this.snapshot({ fresh: true });
|
||||
|
||||
// The snapshot comes off the chain, which knows nothing about freezing, so every UTXO in it claims
|
||||
// `frozen: false`. selectCoins has always honoured the flag — nothing ever set it, which is what made
|
||||
// a plain send happily spend a coin the owner had frozen, and made "send everything" sweep it.
|
||||
const frozen = new Set(req.frozenOutpoints ?? []);
|
||||
const utxos = frozen.size
|
||||
? snap.utxos.map((u) => (frozen.has(`${u.txid}:${u.vout}`) ? { ...u, frozen: true } : u))
|
||||
: snap.utxos;
|
||||
|
||||
const selection = selectCoins({
|
||||
utxos: snap.utxos,
|
||||
utxos,
|
||||
targetSats: req.amountSats ?? 0,
|
||||
sendAll: req.sendAll === true,
|
||||
satPerVbyte: req.satPerVbyte,
|
||||
|
||||
@@ -114,8 +114,10 @@ class StubEsplora {
|
||||
throw new Error('not needed');
|
||||
}
|
||||
|
||||
/** Accepts anything and reports a fixed txid — the spend tests care what got selected, not what got sent. */
|
||||
async broadcast(): Promise<string> {
|
||||
throw new Error('not needed');
|
||||
this.calls.push('broadcast');
|
||||
return 'd'.repeat(64);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -134,7 +136,16 @@ const LOCKED_SIGNER: WalletSigner = {
|
||||
},
|
||||
};
|
||||
|
||||
function backendFor(world: Map<string, Funding>) {
|
||||
/**
|
||||
* A signer holding the fixture's own root key. Only the spend tests need one — everything else runs
|
||||
* locked, which is the state the wallet is in almost all of the time.
|
||||
*/
|
||||
const UNLOCKED_SIGNER: WalletSigner = {
|
||||
isUnlocked: () => true,
|
||||
withRoot: <T>(fn: (root: HDKey) => T): T => fn(HDKey.fromMasterSeed(mnemonicToSeedSync(PUBLIC_TEST_MNEMONIC))),
|
||||
};
|
||||
|
||||
function backendFor(world: Map<string, Funding>, signer: WalletSigner = LOCKED_SIGNER) {
|
||||
const stub = new StubEsplora(world);
|
||||
const source = new EsploraChainSource({
|
||||
chain: stub as unknown as EsploraChain,
|
||||
@@ -144,7 +155,7 @@ function backendFor(world: Map<string, Funding>) {
|
||||
chain: source,
|
||||
network: 'bitcoin',
|
||||
accountXpub: { p2wpkh: ACCOUNT_XPUB },
|
||||
signer: LOCKED_SIGNER,
|
||||
signer,
|
||||
});
|
||||
return { backend, stub };
|
||||
}
|
||||
@@ -251,6 +262,46 @@ describe('EsploraChainSource through OnchainBackend', () => {
|
||||
expect(tx.feeSats).toBeNull();
|
||||
});
|
||||
|
||||
// Freezing a coin is a promise that it will not be spent. It was not being kept: the route checked the
|
||||
// frozen list only against outpoints the caller named explicitly, so coin control was safe while a
|
||||
// plain send — the ordinary case — picked inputs from a snapshot on which every UTXO claimed
|
||||
// `frozen: false`, because the chain has no idea what Officer froze. Sweeping was the worst of it: a
|
||||
// "send everything" emptied the frozen coin too.
|
||||
//
|
||||
// These run unlocked and reach the signer, which is why the fixture key had to be the published test
|
||||
// vector and nothing else.
|
||||
describe('frozen coins', () => {
|
||||
const fundedWorld = () => new Map<string, Funding>([[addressAt(0, 0), { confirmed: 50_000 }]]);
|
||||
/** The single outpoint the stub serves for a funded address. */
|
||||
const OUTPOINT = `${'a'.repeat(64)}:0`;
|
||||
const TO = 'bc1qcr8te4kr609gcawutmrza0j4xv80jy8z306fyu';
|
||||
|
||||
test('automatic selection will not spend one', async () => {
|
||||
const { backend, stub } = backendFor(fundedWorld(), UNLOCKED_SIGNER);
|
||||
await expect(
|
||||
backend.sendCoins({ address: TO, amountSats: 10_000, satPerVbyte: 5, frozenOutpoints: [OUTPOINT] }),
|
||||
).rejects.toThrow(/no spendable UTXOs/);
|
||||
// Nothing was signed and nothing left the machine.
|
||||
expect(stub.calls).not.toContain('broadcast');
|
||||
});
|
||||
|
||||
test('sendAll will not sweep one', async () => {
|
||||
const { backend, stub } = backendFor(fundedWorld(), UNLOCKED_SIGNER);
|
||||
await expect(
|
||||
backend.sendCoins({ address: TO, sendAll: true, satPerVbyte: 5, frozenOutpoints: [OUTPOINT] }),
|
||||
).rejects.toThrow(/no spendable UTXOs/);
|
||||
expect(stub.calls).not.toContain('broadcast');
|
||||
});
|
||||
|
||||
test('the same send succeeds once the coin is not frozen', async () => {
|
||||
// The control. Without it the two tests above would still pass if sending were broken outright.
|
||||
const { backend, stub } = backendFor(fundedWorld(), UNLOCKED_SIGNER);
|
||||
const result = await backend.sendCoins({ address: TO, amountSats: 10_000, satPerVbyte: 5 });
|
||||
expect(result.txid).toBe('d'.repeat(64));
|
||||
expect(stub.calls).toContain('broadcast');
|
||||
});
|
||||
});
|
||||
|
||||
test('fee estimates pass through untouched', async () => {
|
||||
const { backend } = backendFor(new Map());
|
||||
expect(await backend.estimateFees()).toEqual({
|
||||
|
||||
@@ -308,9 +308,12 @@ async function handleWallets(ctx: OfficerContext, seg: string[]): Promise<Respon
|
||||
if (!req2.sendAll && !req2.amountSats) return badRequest('amountSats or sendAll is required');
|
||||
if (!req2.satPerVbyte || req2.satPerVbyte < 1) return badRequest('satPerVbyte must be at least 1');
|
||||
// Never spend a frozen coin, even if the caller passed no explicit outpoint list.
|
||||
const frozen = new Set(await getFrozenOutpoints(walletId));
|
||||
const frozenList = await getFrozenOutpoints(walletId);
|
||||
const frozen = new Set(frozenList);
|
||||
if (req2.outpoints?.some((o) => frozen.has(o))) return badRequest('refusing to spend a frozen UTXO');
|
||||
const result = await backend.sendCoins(req2);
|
||||
// Spread last so a caller cannot supply its own frozen list: the check above only ever covered
|
||||
// outpoints the caller named explicitly, which left automatic selection free to pick a frozen coin.
|
||||
const result = await backend.sendCoins({ ...req2, frozenOutpoints: frozenList });
|
||||
if (req2.label) await setWalletLabel(walletId, 'tx', result.txid, req2.label);
|
||||
return json(result);
|
||||
}
|
||||
|
||||
@@ -141,6 +141,15 @@ export type SendCoinsRequest = {
|
||||
/** Opt out of RBF signalling. Default is replaceable. */
|
||||
rbf?: boolean;
|
||||
label?: string;
|
||||
/**
|
||||
* Outpoints automatic selection must not touch. SET BY THE ROUTE FROM OFFICER'S FROZEN LIST, never by
|
||||
* the caller — whatever arrives in the request body is overwritten.
|
||||
*
|
||||
* Freezing is Officer policy stored in Postgres, and a backend has no business reading that table; so
|
||||
* the frozen set has to travel with the request. Note it only ever restricts what may be spent, so
|
||||
* even a caller who did smuggle a value in could not widen the selection.
|
||||
*/
|
||||
frozenOutpoints?: readonly string[];
|
||||
};
|
||||
|
||||
export type SendCoinsResult = {
|
||||
|
||||
Reference in New Issue
Block a user