perf: search branch and bound with in-place DFS by evanlinjin · Pull Request #68 · bitcoindevkit/coin-select

evanlinjin · 2026-08-14T09:24:56Z

Summary

Replace best-first branch-and-bound (a BinaryHeap of cloned branches) with in-place DFS that visits the better-bound child first and backtracks.

One selector/cache pair, mutated and undone, instead of cloning every node onto a heap.
Equal bounds still prefer inclusion, same as the old tie-break.
Exclusion still batches interchangeable candidates (same value, weight, and drags_in).
Under a round cap this completes selections on large pools where the old frontier often spent the budget without a solution.

Public API is unchanged (BnbIter stays pub(crate)).

Based on #64

This branch is stacked on #64 (feature/ancestor-aware-selection-no-clustor). Merge #64 first; the DFS change is the tip commit perf: search branch and bound with in-place DFS. Review that commit (or retarget this PR onto #64's branch) for a DFS-only diff.

Benchmarks (`cargo bench -- run_bnb --quick`)

Versus best-first on the same #64 tree:

bench	best-first	DFS
`run_bnb_lowest_fee` 20 / 50 / 100	85 µs / 2.4 ms / 6.0 ms	43 µs / 1.2 ms / 4.4 ms
`run_bnb_lowest_fee_exhaust_cap` 200 / 500 / 1000	94 / 97 / 120 ms, no solution	34 / 65 / 113 ms, finds solutions
ancestors private 100	130 ms	54 ms
ancestors shared 100	156 ms	54 ms

DFS also finds a LowestFee solution on a 10k-coin pool within the 100k-round cap.

Some instances visit more nodes (exact-match regression: 3,194 → 62,452 rounds) but wall-clock still improves from skipping heap/clones.

Test plan

cargo fmt -- --check
cargo clippy --all-targets -- -D warnings
cargo test --release (bnb, lowest_fee, lowest_fee_changeless, ancestor, including exhaustive/proptest oracles)
Criterion run_bnb* benches vs the previous best-first implementation

…y_count Fixes CoinSelector::input_weight undercounting candidates that group multiple legacy inputs in a segwit transaction (where each legacy input serializes a 1 WU empty witness). Tracking segwit and legacy input counts separately also allows a single Candidate to mix legacy and segwit inputs.

…legacy Replaces the boolean is_segwit parameter in Candidate::new with explicit new_segwit and new_legacy constructors. Clarifies in doc comments that satisfaction_weight is the additional weight required beyond TXIN_BASE_WEIGHT (which already accounts for a 1-byte scriptSigLen).

…call A selector was built for one target and evaluated against it throughout, but every method took the target as a parameter, so nothing stopped `cs.excess(target_a, drain)` being followed by `cs.is_funded(target_b)`. The correctness arguments in the metrics are all stated at a fixed target -- `LowestFee::bound`'s proof that a changeless superset always costs more, `Changeless::change_unavoidable`'s assumption that the drain decision is monotone in the excess -- and were held together by convention rather than by types. `CoinSelector::new` now takes the target and owns it. Twenty signatures *lose* a parameter rather than gaining one: fifteen public methods (`excess`, `implied_fee`, `is_funded`, `drain`, `select_until_target_met`, the four `*_excess`, ...), plus `bnb_solutions` and `run_bnb`, plus all three `BnbMetric` methods. The crate had already reached this conclusion one layer down: `BnbIter` stored the target as a field, took it once in `BnbIter::new`, and then re-passed it into `metric.score` and `metric.bound` at every node. That field and the re-threading are both gone. This is a breaking change, and it reaches `BnbMetric`, so metrics implemented outside this crate need their signatures updated: fn score(&mut self, cs: &CoinSelector<'_>) -> Option<Ordf32>; fn bound(&mut self, cs: &CoinSelector<'_>) -> Option<Ordf32>; fn drain(&mut self, cs: &CoinSelector<'_>) -> Drain; `CoinSelector::target()` exposes the target for metrics that need to read it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Move the fixed target, candidates, and optional ancestor graph into one immutable problem object. CoinSelector now borrows that object, keeping all calculations tied to the same inputs and allowing ancestry metadata to remain separate from Candidate. Provide new_no_ancestors for prebuilt candidates and new for constructing candidates from input groups and their unconfirmed transaction graph.

Selecting an unconfirmed coin means paying to bump its ancestors. The feerate obligation includes the shortfall of the union of ancestors the selected candidates drag in (each charged once; weight and fee netted; saturates at 0). Score is still the child fee — the bump is already inside it. With ancestors, LowestFee falls back to a loose but admissible fee floor; tightening is a follow-up. BnB only batch-bans look-alikes with the same drags_in; Changeless disables its prune when ancestors are present.

Precompute ancestors reachable through exactly one candidate as summed private packages. Keep bitset de-duplication only for ancestors shared by multiple candidates, preserving exact union accounting while reducing the common-path work in every fee calculation. Add Criterion coverage for private and shared ancestry at 20, 50, and 100 candidates, plus exhaustive regressions for the optimized representation.

For funded nodes, subtract the ancestor surplus still reachable by a descendant. For unfunded nodes, derive a minimum added child weight from independent fractional relaxations of the target-rate, absolute-fee, and RBF constraints, then evaluate the fee floor at that weight. Candidate ancestry is deliberately represented only by the global bump lower bound: package surplus can absorb a later private deficit, so a per-candidate ancestor cost is not admissible. Keep infeasibility prunes off because ancestor funding is non-monotone. Add regressions for package subsidy, absolute/RBF double counting, and large-float cancellation, plus the existing exhaustive proptests.

Maintain aggregate selection state per branch and expose it through SelectionView so metric evaluation avoids repeatedly walking selected candidates. Track each branch's candidate cursor to skip repeated scans, and extend benchmarks across wallet- and exchange-scale pools.

Keep SelectionView's hypothetical updates set-like and synchronize ancestor reachability when branches exclude candidates. Remove unsound funding and changeless assumptions exposed by non-monotone ancestor debt, and preserve conservative fee rounding in the bound. Add regressions for public view updates, exclusion transitions, weight caps, mixed serialization overhead, and floating-point edge cases.

Separate deterministic solution-finding cases from larger pools expected to exhaust the fixed round cap. Assert each fixture's expected search outcome before measuring it so benchmark comparisons cannot silently time different paths.

Store private ancestor totals directly and allocate shared reference tracking only when the problem actually has shared ancestry. Preserve an explicit precision allowance for large floating-point ancestor fees so the smaller cache does not tighten the admissible bound.

Replace generic metric composition with a changeless metric that reuses LowestFee's funding, weight-cap, dust, and change decisions. Add a monotone selected-value bound for pools up to 24 candidates while retaining LowestFee's ordering for larger pools to avoid finite-round starvation. Cover the constrained objective with exhaustive and serialization-edge regressions, and document the migration from Changeless and tuple metrics.

Replace the best-first BinaryHeap frontier with depth-first search that visits the better-bound child first and backtracks in place. This drops per-branch selector/cache clones and, under a round cap, finds complete solutions on large pools where the old frontier often exhausted the budget without a selection.

evanlinjin · 2026-08-14T11:07:42Z

This is AI-generated slop that actually produces worse results.

evanlinjin and others added 14 commits August 14, 2026 04:52

bench: restore capped BnB frontier coverage

28c5455

Separate deterministic solution-finding cases from larger pools expected to exhaust the fixed round cap. Assert each fixture's expected search outcome before measuring it so benchmark comparisons cannot silently time different paths.

docs: polish ancestor-aware API guidance

7e965cf

evanlinjin closed this Aug 14, 2026

Navigation Menu

Uh oh!

perf: search branch and bound with in-place DFS - #68

perf: search branch and bound with in-place DFS#68
evanlinjin wants to merge 14 commits into
bitcoindevkit:masterfrom
evanlinjin:experiment/bnb-dfs

evanlinjin commented Aug 14, 2026

Uh oh!

evanlinjin commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

evanlinjin commented Aug 14, 2026

Summary

Based on #64

Benchmarks (cargo bench -- run_bnb --quick)

Test plan

Uh oh!

evanlinjin commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Benchmarks (`cargo bench -- run_bnb --quick`)