fix(wallet): include base tx overhead fee in manual coin selection target

[Tyagiquamar]

Problem

select_utxos_with_algorithm passed target_amount to BDK's coin_select unchanged. BDK's coin selection only adds the per-input satisfaction fee on top of that target — it does not account for the fixed transaction overhead (the recipient output plus version/marker/flag/varints/locktime). The transaction builder used by send_to_address, however, pays for the entire transaction.

As a result, a changeless BranchAndBound selection could be accepted by select_utxos_with_algorithm yet rejected by send_to_address as InsufficientFunds — short by exactly the base-overhead fee. The library effectively said "these UTXOs are enough" and then "these same UTXOs are insufficient".

Fixes #104.

Fix

Inflate the selection target by the fee for the fixed base-transaction overhead (BASE_TX_WEIGHT, ~188 WU ≈ 47 vB) before calling coin_select. The recipient-output size uses the conservative P2PKH size so the inflation is never too small for other script types. Selection and the builder now agree for the same target and fee rate.

Tests

• New unit regression test selected_utxos_satisfy_tx_builder_for_all_algorithms builds the spend exactly like the manual-UTXO path of build_transaction_psbt and asserts the builder accepts the selection for the deterministic algorithms (BranchAndBound, LargestFirst, OldestFirst). It failed before the fix with InsufficientFunds { needed: 35722, available: 35700 } and passes after. SingleRandomDraw is omitted because its randomized selection can legitimately land on a dust-change rejection, making the test flaky.
• New end-to-end test test_selected_utxos_are_accepted_by_send_to_address reproduces the issue's wallet layout (18 P2WPKH UTXOs) against real chain infra and asserts send_to_address accepts the selection.
• cargo test -p bdk-wallet-aggregate --lib — 39 passed, 0 failed.
• cargo fmt --check and cargo clippy -p bdk-wallet-aggregate --lib -- -D warnings both clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7eb66a4199

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ovitrif]

The manual-send precheck still rejects a valid low-fee selection returned by the new path. The existing P2TR/P2WSH coverage thread also remains unresolved.

[ovitrif]

The manual-send precheck issue is fixed. The new P2TR/P2WSH test still does not protect the 224-WU estimate: at 250 sat/kwu, both 188 WU and 224 WU select the same eight inputs totaling 35,600 sats, and the 43-byte-output transaction needs about 35,598 sats. Reverting BASE_TX_WEIGHT to 188 WU would leave this test green. Could we use a boundary fixture that fails with 188 WU and passes with 224 WU?

[Tyagiquamar]

Good catch. The previous fixture didn't distinguish 188 WU from 224 WU. I replaced it with a boundary case where the lower allowance selects an insufficient set while 224 WU selects a builder-valid set (at 1,000 sat/kwu, 188 WU selects 10,470 sats which is insufficient for TxBuilder needing 10,486 sats for a 43-vB output, whereas 224 WU selects 10,970 sats which TxBuilder accepts). I also mutation-checked the regression: it fails when BASE_TX_WEIGHT is reverted to 188 WU and passes at 224 WU. \cargo test -p bdk-wallet-aggregate --lib\ passes.

[coreyphillips]

I rechecked b375267 and found a few remaining issues:

• cargo test --test multi_address_types_tests --no-run fails because test_p2tr_recipient is not imported and the test uses private ldk_node::Error instead of NodeError. After those fixes, the UniFFI build also needs fee_rate.clone() because it is an Arc.
• Strict Clippy fails on the new eight-argument inner selector.
• Base-target inflation can turn valid P2WPKH change into CoinSelectionFailed. A 35,655 sat input for 35,000 sats at 250 sat/kwu yields 500 sats of selection change. BDK accepts it because the script-specific dust floor is 294 sats, but the wrapper rejects anything below 546.
• Removing the manual precheck also removes the overflow guard. u64::MAX with selected UTXOs now panics instead of returning InsufficientFunds.
• The 224 WU allowance still undercounts valid future-witness recipients. A v2 40-byte program needs a 246 WU skeleton, so selection can still succeed before the builder rejects it.

[ovitrif]

The replacement boundary fixture fixes the prior 224-WU coverage gap. The compile blockers already reported on the current head still prevent approval: #109 (comment)

[Tyagiquamar]

Thanks — all five findings are addressed in 71da270:

• The integration test source now imports test_p2tr_recipient, uses public NodeError, and clones the shared fee-rate handle.
• The selector uses a request context, so strict Clippy passes without an allowance.
• Recipient output weight is derived from the actual script when available; BDK now owns script-specific dust handling, including sub-546-sat P2WPKH change.
• Checked arithmetic restores the selected-value overflow guard and maps overflow to InsufficientFunds.
• RBF deficit selection remains overhead-free; normal transaction selection uses the 42-WU envelope plus the actual recipient TxOut weight, with the conservative fallback retained when no script is available.

Verified: aggregate tests 45/45, strict aggregate Clippy, aggregate formatting, and cargo check --lib pass. The exact integration --no-run command is blocked on Windows before the test target by electrsd 0.36.1's Unix-only build script. No CI checks are currently reported for this branch.

[ovitrif]

utACK

[ovitrif]

@Tyagiquamar Please merge master in and push or rebase on master's tip and force-push