Skip to content

Commit dabd2ca

Browse files
committed
fix(wallet): reassign TxBuilder result for addUtxos/setExactSequence
bdk_dart's TxBuilder is immutable — every method returns a new builder instance instead of mutating in place. Two call sites in buildPsbt discarded the return value, silently no-opping both manual UTXO selection and the RBF-off toggle: BDK always picked inputs on its own regardless of the user's coin selection, and disabling RBF never actually changed the sequence field. Pre-existing on main (not trezor-specific); affects hot wallets and every remote signer (Ledger/BitBox/Trezor) identically. Adds an offline regression test that funds a real BDK wallet via a hand-crafted unconfirmed transaction (no network) and exercises buildPsbt end-to-end, proving manual selection now overrides otherwise-unspendable coins and that the RBF sequence is set correctly in both directions.
1 parent af48a87 commit dabd2ca

2 files changed

Lines changed: 334 additions & 2 deletions

File tree

lib/core/wallet/data/datasources/bdk_wallet_datasource.dart

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,19 @@ class BdkWalletDatasource {
198198
),
199199
)
200200
.toList();
201-
txBuilder.addUtxos(outpoints: selectableOutPoints);
201+
// bdk_dart's TxBuilder is immutable — every method returns a NEW
202+
// builder instance rather than mutating in place. Discarding the
203+
// return value (as this call did before) silently drops the manual
204+
// UTXO selection and leaves BDK to pick inputs automatically.
205+
txBuilder = txBuilder.addUtxos(outpoints: selectableOutPoints);
202206
}
203207

204208
// bdk_dart always has RBF (nSequence = 0xFFFFFFFD) enabled by default,
205209
// so we set the sequence to 0xFFFFFFFE if replaceByFee is explicitly set to false to disable RBF.
206-
if (!replaceByFee) txBuilder.setExactSequence(nsequence: 0xFFFFFFFE);
210+
// Same immutable-builder pitfall as addUtxos above — must reassign.
211+
if (!replaceByFee) {
212+
txBuilder = txBuilder.setExactSequence(nsequence: 0xFFFFFFFE);
213+
}
207214

208215
switch (networkFee) {
209216
case AbsoluteFee(:final sats):
Lines changed: 325 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,325 @@
1+
// Regression test for a bug where `BdkWalletDatasource.buildPsbt` silently
2+
// dropped manual UTXO selection and the RBF-off sequence flag.
3+
//
4+
// bdk_dart's `TxBuilder` is an IMMUTABLE builder: every method (`addUtxos`,
5+
// `setExactSequence`, `feeAbsolute`, `unspendable`, ...) returns a brand new
6+
// `TxBuilder` instance rather than mutating the receiver in place. Two call
7+
// sites in `buildPsbt` called `txBuilder.addUtxos(...)` and
8+
// `txBuilder.setExactSequence(...)` without reassigning the result, so both
9+
// calls were silent no-ops: BDK picked inputs on its own regardless of the
10+
// user's manual coin selection, and the RBF-off toggle never took effect.
11+
//
12+
// This test exercises the real production method end-to-end against a real
13+
// (offline, in-process) BDK wallet — no network, no mocked repository — so
14+
// it proves the actual `TxBuilder` wiring, not just that a mock was called
15+
// with the right arguments.
16+
//
17+
// Setup, fully deterministic and offline:
18+
// 1. A fixed BIP84 testnet wallet (the canonical
19+
// "abandon ... abandon about" test mnemonic) is used to derive a
20+
// public (watch-only) descriptor pair — exactly what
21+
// `BitcoinWalletRepository.buildPsbt` passes down in production.
22+
// 2. The wallet is "funded" by hand-crafting a raw funding transaction
23+
// with two outputs (to two of the wallet's own addresses) and applying
24+
// it as an unconfirmed transaction via `Wallet.applyUnconfirmedTxs`.
25+
// This is the standard offline way to seed known, deterministic UTXOs
26+
// into a BDK wallet without hitting a real Electrum server.
27+
// 3. `path_provider`'s method channel is mocked to a temp directory so
28+
// `BdkFacade`'s sqlite-file persister (production code, untouched)
29+
// works under `flutter test`.
30+
import 'dart:io';
31+
import 'dart:typed_data';
32+
33+
import 'package:bb_mobile/core/fees/domain/fees_entity.dart';
34+
import 'package:bb_mobile/core/wallet/data/datasources/bdk_facade.dart';
35+
import 'package:bb_mobile/core/wallet/data/datasources/bdk_wallet_datasource.dart';
36+
import 'package:bb_mobile/core/wallet/data/models/wallet_model.dart';
37+
import 'package:bb_mobile/core/wallet/data/models/wallet_utxo_model.dart';
38+
import 'package:bull_sdk/bdk.dart' as bdk;
39+
import 'package:flutter/services.dart';
40+
import 'package:flutter_test/flutter_test.dart';
41+
42+
// The canonical BIP39 test mnemonic ("abandon" x11 + "about"). Public
43+
// knowledge, no funds, safe to hardcode.
44+
const _testMnemonic =
45+
'abandon abandon abandon abandon abandon abandon abandon abandon '
46+
'abandon abandon abandon about';
47+
48+
// A well-known BIP173 test-vector P2WPKH testnet address, used only as an
49+
// external send destination (not owned by the test wallet).
50+
const _externalTestnetAddress = 'tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx';
51+
52+
Uint8List _leUint32(int value) {
53+
final bytes = ByteData(4)..setUint32(0, value, Endian.little);
54+
return bytes.buffer.asUint8List();
55+
}
56+
57+
Uint8List _leUint64(int value) {
58+
final bytes = ByteData(8)..setUint64(0, value, Endian.little);
59+
return bytes.buffer.asUint8List();
60+
}
61+
62+
Uint8List _varInt(int value) {
63+
if (value < 0xfd) return Uint8List.fromList([value]);
64+
if (value <= 0xffff) {
65+
final bytes = ByteData(3)
66+
..setUint8(0, 0xfd)
67+
..setUint16(1, value, Endian.little);
68+
return bytes.buffer.asUint8List();
69+
}
70+
throw UnimplementedError('varint > 0xffff not needed for this test');
71+
}
72+
73+
/// Hand-crafts a minimal, legacy-serialized (non-segwit) raw transaction
74+
/// with one throwaway input (never meant to be valid/spendable — BDK's
75+
/// `apply_unconfirmed_txs` doesn't validate it) and one output per entry in
76+
/// [outputs]. Used to seed deterministic, known UTXOs into an otherwise
77+
/// empty offline wallet.
78+
Uint8List _buildFundingTx(List<({bdk.Script script, int amountSat})> outputs) {
79+
final bytes = BytesBuilder();
80+
bytes.add(_leUint32(2)); // version
81+
bytes.add(_varInt(1)); // 1 (fake) input
82+
bytes.add(Uint8List(32)); // prev txid (all-zero, never spent for real)
83+
bytes.add(_leUint32(0)); // prev vout
84+
bytes.add(_varInt(0)); // empty scriptSig
85+
bytes.add(_leUint32(0xFFFFFFFF)); // sequence
86+
bytes.add(_varInt(outputs.length));
87+
for (final output in outputs) {
88+
bytes.add(_leUint64(output.amountSat));
89+
final script = output.script.toBytes();
90+
bytes.add(_varInt(script.length));
91+
bytes.add(script);
92+
}
93+
bytes.add(_leUint32(0)); // locktime
94+
return bytes.toBytes();
95+
}
96+
97+
void main() {
98+
TestWidgetsFlutterBinding.ensureInitialized();
99+
100+
late Directory tempDir;
101+
late PublicBdkWalletModel walletModel;
102+
late String utxoLargeTxId;
103+
late int utxoLargeVout;
104+
105+
const utxoLargeAmountSat = 200000;
106+
const utxoSmallAmountSat = 30000;
107+
108+
setUp(() async {
109+
tempDir = await Directory.systemTemp.createTemp('bdk_wallet_datasource_');
110+
const channel = MethodChannel('plugins.flutter.io/path_provider');
111+
TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger
112+
.setMockMethodCallHandler(channel, (call) async {
113+
if (call.method == 'getApplicationDocumentsDirectory') {
114+
return tempDir.path;
115+
}
116+
return null;
117+
});
118+
119+
final mnemonic = bdk.Mnemonic.fromString(mnemonic: _testMnemonic);
120+
final secretKey = bdk.DescriptorSecretKey(
121+
networkKind: bdk.NetworkKind.test,
122+
mnemonic: mnemonic,
123+
password: null,
124+
);
125+
final external = bdk.Descriptor.newBip84(
126+
secretKey: secretKey,
127+
keychainKind: bdk.KeychainKind.external_,
128+
networkKind: bdk.NetworkKind.test,
129+
);
130+
final internal = bdk.Descriptor.newBip84(
131+
secretKey: secretKey,
132+
keychainKind: bdk.KeychainKind.internal,
133+
networkKind: bdk.NetworkKind.test,
134+
);
135+
136+
walletModel =
137+
WalletModel.publicBdk(
138+
id: 'bdk-wallet-datasource-test',
139+
externalDescriptor: external.toString(),
140+
internalDescriptor: internal.toString(),
141+
isTestnet: true,
142+
)
143+
as PublicBdkWalletModel;
144+
145+
// Build the wallet once here to fund it, then persist so the
146+
// datasource's own (separate) `BdkFacade.createWallet` call sees the
147+
// exact same UTXOs when the test invokes `buildPsbt`.
148+
final wallet = await BdkFacade.createWallet(walletModel);
149+
150+
final addr0 = wallet.revealNextAddress(
151+
keychain: bdk.KeychainKind.external_,
152+
);
153+
final addr1 = wallet.revealNextAddress(
154+
keychain: bdk.KeychainKind.external_,
155+
);
156+
157+
final fundingTxBytes = _buildFundingTx([
158+
(script: addr0.address.scriptPubkey(), amountSat: utxoLargeAmountSat),
159+
(script: addr1.address.scriptPubkey(), amountSat: utxoSmallAmountSat),
160+
]);
161+
final fundingTx = bdk.Transaction(transactionBytes: fundingTxBytes);
162+
final fundingTxid = fundingTx.computeTxid().toString();
163+
164+
wallet.applyUnconfirmedTxs(
165+
unconfirmedTxs: [
166+
bdk.UnconfirmedTx(
167+
tx: fundingTx,
168+
lastSeen: DateTime.now().millisecondsSinceEpoch ~/ 1000,
169+
),
170+
],
171+
);
172+
173+
// Sanity-check the funding actually landed before persisting, so a
174+
// failure here points clearly at the test's own setup rather than at
175+
// `buildPsbt`.
176+
final utxos = wallet.listUnspent();
177+
expect(
178+
utxos.length,
179+
2,
180+
reason: 'test setup: expected exactly 2 synthetic UTXOs after funding',
181+
);
182+
183+
utxoLargeTxId = fundingTxid;
184+
utxoLargeVout = 0;
185+
186+
await BdkFacade.saveWallet(wallet, walletModel.hexId);
187+
});
188+
189+
tearDown(() async {
190+
if (await tempDir.exists()) {
191+
await tempDir.delete(recursive: true);
192+
}
193+
});
194+
195+
test(
196+
'buildPsbt honors manual UTXO selection: a manually selected UTXO must '
197+
'be spendable even when every other UTXO is frozen/unspendable',
198+
() async {
199+
final datasource = BdkWalletDatasource();
200+
201+
// The LARGE utxo (200k) is marked unspendable — i.e. excluded from
202+
// BDK's own automatic coin-selection pool, exactly like a
203+
// user-frozen coin (see the D7 guarantee in
204+
// PrepareBitcoinSendUsecase). It is ALSO the one manually `selected`
205+
// here, which per BDK's coin-control semantics is supposed to force
206+
// it into the transaction as a mandatory input regardless of the
207+
// unspendable flag (mirroring real coin control: a coin the user
208+
// explicitly picks must be spendable even if it's otherwise
209+
// frozen-by-default for automatic selection).
210+
//
211+
// The only utxo left in the automatic pool is the SMALL one (30k),
212+
// which alone cannot cover the 150k send. This makes the two code
213+
// paths unambiguous, with no dependency on BDK's coin-selection
214+
// tie-breaking heuristics:
215+
// * Fixed code: `addUtxos` really runs, forcing the large utxo in
216+
// -> the build succeeds and spends it.
217+
// * Buggy code (return value of `addUtxos` discarded): no utxo is
218+
// forced in; BDK's automatic selection is left with only the
219+
// 30k utxo, which can't cover 150k -> the build throws.
220+
const sendAmountSat = 150000; // only the large (200k) utxo covers this
221+
final psbt = await datasource.buildPsbt(
222+
wallet: walletModel,
223+
address: _externalTestnetAddress,
224+
amountSat: sendAmountSat,
225+
networkFee: const NetworkFee.relativeSatPerKwu(1000), // 4 sat/vB
226+
unspendable: [(txId: utxoLargeTxId, vout: utxoLargeVout)],
227+
selected: [
228+
WalletUtxoModel.bitcoin(
229+
txId: utxoLargeTxId,
230+
vout: utxoLargeVout,
231+
amountSat: BigInt.from(utxoLargeAmountSat),
232+
scriptPubkey: Uint8List(0),
233+
address: '',
234+
isExternalKeyChain: true,
235+
),
236+
],
237+
replaceByFee: true,
238+
);
239+
240+
final tx = bdk.Psbt(psbtBase64: psbt).extractTx();
241+
final spendsLargeUtxo = tx.input().any(
242+
(input) =>
243+
input.previousOutput.txid.toString() == utxoLargeTxId &&
244+
input.previousOutput.vout == utxoLargeVout,
245+
);
246+
247+
expect(
248+
spendsLargeUtxo,
249+
isTrue,
250+
reason:
251+
'the manually selected UTXO must be forced into the built '
252+
'transaction even though it is also marked unspendable for '
253+
"BDK's own automatic selection",
254+
);
255+
},
256+
);
257+
258+
test(
259+
'buildPsbt sets a non-RBF sequence when replaceByFee is false',
260+
() async {
261+
final datasource = BdkWalletDatasource();
262+
263+
final psbt = await datasource.buildPsbt(
264+
wallet: walletModel,
265+
address: _externalTestnetAddress,
266+
amountSat: 25000,
267+
networkFee: const NetworkFee.relativeSatPerKwu(1000),
268+
selected: [
269+
WalletUtxoModel.bitcoin(
270+
txId: utxoLargeTxId,
271+
vout: utxoLargeVout,
272+
amountSat: BigInt.from(utxoLargeAmountSat),
273+
scriptPubkey: Uint8List(0),
274+
address: '',
275+
isExternalKeyChain: true,
276+
),
277+
],
278+
replaceByFee: false,
279+
);
280+
281+
final tx = bdk.Psbt(psbtBase64: psbt).extractTx();
282+
final selectedInput = tx.input().firstWhere(
283+
(input) =>
284+
input.previousOutput.txid.toString() == utxoLargeTxId &&
285+
input.previousOutput.vout == utxoLargeVout,
286+
);
287+
288+
expect(selectedInput.sequence, 0xFFFFFFFE);
289+
},
290+
);
291+
292+
test(
293+
'buildPsbt keeps the default RBF sequence when replaceByFee is true',
294+
() async {
295+
final datasource = BdkWalletDatasource();
296+
297+
final psbt = await datasource.buildPsbt(
298+
wallet: walletModel,
299+
address: _externalTestnetAddress,
300+
amountSat: 25000,
301+
networkFee: const NetworkFee.relativeSatPerKwu(1000),
302+
selected: [
303+
WalletUtxoModel.bitcoin(
304+
txId: utxoLargeTxId,
305+
vout: utxoLargeVout,
306+
amountSat: BigInt.from(utxoLargeAmountSat),
307+
scriptPubkey: Uint8List(0),
308+
address: '',
309+
isExternalKeyChain: true,
310+
),
311+
],
312+
replaceByFee: true,
313+
);
314+
315+
final tx = bdk.Psbt(psbtBase64: psbt).extractTx();
316+
final selectedInput = tx.input().firstWhere(
317+
(input) =>
318+
input.previousOutput.txid.toString() == utxoLargeTxId &&
319+
input.previousOutput.vout == utxoLargeVout,
320+
);
321+
322+
expect(selectedInput.sequence, 0xFFFFFFFD);
323+
},
324+
);
325+
}

0 commit comments

Comments
 (0)