fix(wallet): manual coin selection and RBF toggle - #2473
Merged
Conversation
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.
This comment was marked as resolved.
This comment was marked as resolved.
Two hardenings surfaced by review of the TxBuilder reassignment fix: 1. D7 defense in depth: BDK's documented semantics let a manually added utxo (TxBuilder.addUtxos) override the unspendable filter — the inverse of the app-level D7 invariant that a frozen coin must never be spendable. Production was protected only by PrepareBitcoinSendUsecase stripping frozen coins from the selection; any caller bypassing that usecase (or working from a stale utxo fetch) would not be. BitcoinWalletRepository now takes FrozenWalletUtxoDatasource and reads the frozen table LIVE at build time: frozen outpoints are merged into the unspendable list handed to BDK (automatic selection can never pick a frozen coin, whatever the caller passed) and stripped from the manual selection (a frozen coin can't be forced in via the addUtxos override either). Payjoin-derived exclusions stay at the usecase — they come from another repository, which this repository must not depend on. The datasource keeps raw BDK semantics (thin wrapper); its regression test comment documents that inversion explicitly. 2. replaceByFee null default flipped from false to true: harmless while the datasource's setExactSequence call discarded its result, but wrong once fixed — a caller omitting the flag would have started disabling RBF by default, diverging from the datasource default and BDK's default sequence (0xFFFFFFFD). Repository unit tests (mocktail, frozen store mocked) cover: live-read strip when the caller passes no unspendable list, frozen ∪ caller merge (deduped), caller-list stripping by exact outpoint (txId alone not enough), and RBF flag defaulting/forwarding.
ethicnology
force-pushed
the
fix/bdk-txbuilder-immutable-selection
branch
from
July 18, 2026 14:18
f7e5114 to
04eeb3e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
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.