Skip to content

feat: repair a finished selection where ancestry is shared - #79

Draft
evanlinjin wants to merge 33 commits into
bitcoindevkit:masterfrom
evanlinjin:experiment/repair-pass
Draft

feat: repair a finished selection where ancestry is shared#79
evanlinjin wants to merge 33 commits into
bitcoindevkit:masterfrom
evanlinjin:experiment/repair-pass

Conversation

@evanlinjin

Copy link
Copy Markdown
Member

Draft. The last three commits, stacked on experiment/deepen-on-bound
(#78), which is on experiment/cheap-nodes
(#77).

GitHub will not take a base branch that only exists in the fork, so this is opened against master
and the diff shown includes everything below it. The change to review here is the top three commits.


Branch and bound decides candidates one at a time in a fixed order, so no sort key it uses can
express "this coin is cheap only because that other coin is already selected". Shared ancestry is
exactly that: the first coin off an unconfirmed parent pays the whole bump and every later one off
the same parent pays nothing, and a per-candidate key has to pick one of those two prices before it
knows which the coin will be.

This adds CoinSelector::repair, which corrects that after the fact.

The problem, measured

Extending the benchmark's scale tier so every family runs at 20,000 candidates turned up
three fixtures Bitcoin Core still won. tools/whyscale.py decomposes the package-fee gap into the
only three things that can produce it:

shared_ancestry_20000 nested_ancestry_20000 shared_ancestry_200000
inputs, cs − core −1 −1 0
child weight, cs − core −272 −272 +172
parents dragged in, cs − core +1 0 +1
ancestor bump, cs − core +4,912 +3,087 +2,616
package fee, cs − core +5,480 +3,050 +3,597
share of gap that is bumping 90% 101% 73%

We take one fewer input and build a lighter child — worth about 680 sat — then give it all back and
more by dragging in a parent nobody else is paying for.

The control that isolates the mechanism is private_ancestry_20000: same shape, same size, but every
ancestor reachable from exactly one candidate. Both engines report an identical 218,289 bump over
identical 108 parents, and we win on child weight alone. So this is not "ancestry is hard" — it is
specifically shared ancestry, where what a coin costs depends on which other coins are selected.

And the search does not rescue it: on shared_ancestry_20000 we run 370,432 rounds and return the
greedy seed unimproved. At this pool size the ordering is the answer.

The change

Take a selected coin that is the only one paying for some ancestor, try replacing it with an
unselected coin that drags in nothing new, and keep the swap when the metric scores it better. Repeat
until nothing improves or the swap ceiling is reached.

Only improving swaps are kept, so the score this leaves is never worse than the one it was
given
. It is a hill climb and stops at a local optimum, not a proof of anything.

This is the one thing in the crate that deselects, which is the part that needs care rather than
the swapping. Everything else — branch and bound, select_srd — only ever adds, so run_bnb has
always returned a superset of what the caller had selected, and that is how a required ("must spend")
input is expressed. repair therefore takes the protected set explicitly, and run_bnb passes the
selection it was handed. It also restores the caller's ban set first: the selector comes back
mid-traversal carrying every ban the exclusion frames on the path to that node applied, and those are
search state, not caller intent.

Gated on has_shared_ancestors, and then on whether this selection actually holds a coin that is
the sole payer for an ancestor. The first alone is not enough: it is a property of the problem and is
true as soon as one unconfirmed parent has two spendable outputs, which is the everyday shape of a
wallet that made a payment with change.

run_bnb runs it. bnb_solutions cannot — it is an iterator over improvements rather than a
finished answer — so callers driving that directly call repair themselves, which is what the
benchmark runner does.

What it does

Across all 52 benchmark fixtures at a 100 ms budget it improves 10 and worsens none:

fixture before after
wallet_mixed_200000 773,150 242,240 −68.7%
high_feerate_20000 2,717,840 1,246,960 −54.1%
wallet_mixed_20000 707,660 351,200 −50.4%
nested_ancestry_20000 1,386,060 1,224,980 −11.6%
shared_ancestry_200000 1,331,840 1,196,830 −10.1%
shared_ancestry_20000 1,232,220 1,107,760 −10.1%
shared_ancestry_500 89,990 82,120 −8.8%
shared_ancestry_1000 165,350 154,550 −6.5%
subsidizing_ancestry_20000 5,357,280 5,238,610 −2.2%
shared_ancestry_2000 301,090 296,360 −1.6%

All three fixtures Core held become wins with about 10% to spare, and coin-select now takes every
fixture in the scale tier
. On the 42-fixture matrix at Core's own TOTAL_TRIES round budget it is
41 of 42 on fee against Core's 0 — the 42nd is a tie at an optimum the oracle proves by enumerating
all 2^20 subsets.

Every selection in that run is re-derived from the fixture by the harness: each package reaches the
target feerate once its ancestor union is counted, each stays inside max_weight, and each runner's
own bump figures match an independent recomputation.

And the fixtures are not the only evidence. The last review of this stack found a regression on
thousands of randomly generated pools that the fixture set could not see, so the same check was run
here: every ancestry family regenerated at every small size under 100 different seeds, the pass off
and on, under a fixed round budget. 2,800 pools, no regression, and every repaired selection passes
the verifier.

That check needed a control of its own. A first version ran under a wall-clock deadline and reported
five regressions in 700 pools — all five vanished on a round budget, and one of them was on a pool
with no shared ancestry at all, where the pass returns immediately without looking at anything. Under
a deadline the two arms search different numbers of rounds, so it was measuring the scheduler.

What review changed

The branch was reviewed before this was written up, and the review found the same class of problem
as the last one in this stack: a regression the 42 fixtures cannot see, because every fixture starts
from an empty selection.

run_bnb was dropping caller-preselected inputs. On a four-coin pool where the required input is
the sole payer for its parent, run_bnb returned a cheaper selection without it — silently. The
claim above originally read "the selection this leaves is never worse", which was true of the score
and false of the result: a lower score reached by evicting an input the caller required is worse, not
better. Fixed by passing the protected set, and by the regression test in tests/ancestor.rs.

The gate did not confine the damage either. has_shared_ancestors is a problem-level predicate while
the outgoing set is built from drags_in, which includes private ancestors — so one shared parent
anywhere in the pool exposed every privately-parented required coin.

The search's transient bans were truncating the replacement pool, so how much of it the pass
could see depended on where the last improvement happened to be found. Not a wrong answer, but it
made the measured benefit partly an artifact: the fixtures showing the largest gains are the ones
that return the ban-free greedy seed.

Also taken: an early-out for a selection holding no sole payer; the head-sizing comment claimed a
bound that is not true, since a swapped-in coin joins the selection and can be swapped out later;
REPAIR_REPLACEMENTS_PER_PASS made private, since nothing takes it as an argument; and the
round-trip test now says what it actually establishes.

Every fee figure above is unchanged by the fixes.

Cost

Everything that scales with the pool happens once, before the loop. Getting there took three goes:

version 200,000 candidates, 1,000 swaps answers
rebuild replacement set + view per accepted swap 264 ms
pool-sized work hoisted out of the loop byte-identical
partition the replacement list instead of sorting it 49.6 ms byte-identical
skip taken replacements instead of compacting the list 8.7 ms byte-identical

The first was more expensive than the search it was meant to be a cheap addition to. The last was
found by review: free.retain per accepted swap was pool-sized work back inside the loop, which is
exactly what hoisting it out was for. It does not show on the benchmark's fixtures, which take few
swaps — on those the pass is 12 ms against a 113 ms search at 200,000 candidates and 1.1 ms against
100 ms at 20,000
— and it is 5.7× on a pool built so the swaps actually land.

The constant

DEFAULT_REPAIR_SWAPS = 1_000 — a ceiling on the cost, not a target. 1,000, 20,000 and 100,000
return byte-identical selections on every scale fixture, and the most any fixture actually took was
892. Public so a caller driving bnb_solutions can match what run_bnb does.

What this is not

It does not help where the loss is not a one-swap error. At a 1 ms budget it closes
nested_ancestry_200 and shared_ancestry_100, and takes no swap at all on
subsidizing_ancestry_50, where we are 51% behind — that is a search-time problem, not an ordering
one.

Four other approaches to the same three losses were measured and rejected, recorded in the
benchmark's STRATEGIES.md: re-keying candidates dynamically as ancestry gets paid for
and adding Core's effective-value ordering as a seed both produce the identical prefix at 20,000
candidates; cluster-granularity selection has no measured headroom, since 100× more swaps of this
pass find nothing further; and gating PR #76's seed on density would save under a millisecond of a
100 ms budget.

evanlinjin and others added 30 commits August 14, 2026 04:52
…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.
`LowestFeeChangeless` only applied its selected-value bound to pools of at
most 24 candidates. The cap existed because best-first search treats a
bound as a priority: a bound that grows with the selection pushed funded
branches to the back of the heap, so on a big pool the frontier starved
before it reached one.

Depth-first search reads a bound as a cut instead of a ranking — it
finishes a branch's descendants before its siblings — so the bound can be
applied at every pool size, where it prunes inclusion branches that have
already overshot the incumbent.
Yield the greedy selection before expanding the first node, and adopt its
score as the incumbent. The search is otherwise not anytime: a caller whose
round budget runs out before the first complete selection gets
`NoBnbSolution::RoundLimit` and falls through to whatever fallback it has,
which on a large pool is far worse than the selection a single greedy pass
would have handed it for free.

Only the incumbent changes, not the bound, so the optimum stays reachable
and the improving-solutions contract is unaffected. Metrics that reject the
greedy prefix outright — `LowestFeeChangeless`, which will not score a
selection that overshoots — are unchanged, and `RoundLimit` still means what
it did for them.

The two round-count assertions in `tests/bnb.rs` each move by one: the seed
is a round.
Bitcoin Core's `SelectCoinsBnB` computes `is_feerate_high` once and lets it
decide whether a prune that is only sometimes valid may fire; it does not
drop the prune because the general case is unsound. `bound_with_ancestors`
took the other route — "never returns `None`" — on the grounds that a fat
private deficit can un-fund a prefix a subset would have funded, so
infeasibility is not something it may claim.

That argument covers "select everything and it is still unfunded". It does
not cover the case this relaxation can prove outright: a fee constraint
whose deficit the best input still available cannot close at *any* weight.
Descendants only add, the deficit is already computed against the
branch-wide `ancestor_bump_lower_bound`, and the gain already ignores
whatever ancestors those inputs would drag in — so the estimate is
optimistic on every axis, and a deficit it still cannot close belongs to an
empty subtree.

The scan that finds the best value-per-weight candidate already runs, so
the test is free. It also prunes the unfunded leaves that had nothing left
to add, which the old path could only rank.
Port Bitcoin Core's `SelectCoinsBnB` lookahead. Core keeps a running
`curr_available_value` over the coins it has not decided on yet and
backtracks as soon as that total cannot close the gap to the target; the
cut needs no incumbent, so it fires from the very first descent. We had
the same idea only in `LowestFee::bound`'s no-ancestor path, as an O(n)
rescan that ran after the relaxation had already been set up, and not at
all when the problem has ancestors.

`SelectionCache` now carries the value and weight of the undecided
candidates worth selecting, maintained by the same add/sub/ban/unban hooks
that already track reachable ancestor surplus, so the test is O(1).

Two one-sided relaxations keep it from pruning a branch that holds a
solution: only candidates with positive standalone effective value count
toward the total, and the current ancestor bump is swapped for
`ancestor_bump_lower_bound`, which holds for the whole subtree. That
second one is what lets the prune run with ancestors present, where
funding is not monotone and "select everything and it is still unfunded"
would have been an unsound claim.
LowestFee already decides for itself whether a selection should carry a
change output, adding one only when it lowers the long-term fee, clears the
dust threshold and fits max_weight. A separate changeless objective duplicates
that decision and constrains it, and nothing in the crate needs the constraint.

Removes LowestFeeChangeless along with the Changeless wrapper the unreleased
changelog already retired, plus their tests and proptest regressions.

BREAKING CHANGE: LowestFeeChangeless and Changeless are gone. Callers that
required a changeless transaction should use LowestFee and inspect the Drain it
returns.
`bound_with_ancestors` scanned every undecided candidate at each unfunded node
to find the greatest value-per-weight and to notice weightless value. Branch
and bound asks for that bound at every unfunded node, so an O(n) scan there
made per-node cost grow with the pool: measured on shared_ancestry_*, 2389
ns/round at n=500 rising to 9384 at n=2000, against 385-2056 for the
no-ancestry fixtures.

The metric already requires candidates in descending value-per-weight order,
and that order is keyed on f32. The exact f64 maximum can therefore only lie
inside the run sharing the first undecided candidate's f32 key, which is why
the old code scanned in f64 rather than taking the first: two exact ratios can
tie in f32 and be ordered either way. Scanning just that run keeps the exact
answer without touching the tail. Weightless value becomes a counter kept
where the undecided aggregates already are.

5.9x to 8.8x faster per round at n=500 to 2000, and byte-identical results:
across all 42 benchmark fixtures the score, selection, round count and
exhausted flag are unchanged.

A debug assertion checks the tie-run result against a full scan, so the
ordering assumption is verified on every node the test suite searches.
`SelectionView` overrides these with cache-backed versions, so every call site
in the crate and its tests already resolved to the view; the `CoinSelector`
copies recomputed the same answers by iterating and had no callers left.

Removes `effective_value`, `implied_feerate`, `rate_excess_wu`,
`replacement_excess_wu` and `waste`, plus the two private helpers they were the
last users of.

`missing` and `drain` are deliberately kept even though the view also has them:
the crate's own front-page example calls them on a bare `CoinSelector`, which
is the case they exist for. The same argument keeps the rest of the overlap --
`weight`, `excess`, `is_funded` and friends all have live callers holding a
selector rather than a view, and routing those through `compute_view` would
cost an O(n) cache build to replace an O(n) method.

BREAKING CHANGE: obtain a `SelectionView` with `CoinSelector::compute_view` and
call the removed methods there.
`SelectionView` answered every one of these from its cache while the `CoinSelector`
copy recomputed the same figure by iterating the selection. Keeping both meant two
implementations of the weight model, the excess model and the ancestor bump, and the
slower one was the default a caller reached for.

Removes `absolute_excess`, `ancestor_bump`, `ancestor_bump_lower_bound`, `drain`,
`drain_value`, `excess`, `fee`, `implied_fee`, `input_weight`, `is_funded`,
`is_funded_with_drain`, `is_within_max_weight`, `missing`, `rate_excess`,
`replacement_excess`, `selected_value` and `weight` from `CoinSelector`, along with
the two private helpers they were the last users of.

`select_until` now hands its predicate a `&SelectionView` and maintains that view's
cache incrementally, so the greedy pass behind `select_until_target_met` -- which
seeds every branch-and-bound search -- costs one cache build plus O(1) per step
instead of rescanning the selection on every iteration.

The crate's own front-page example now goes through `compute_view` too, which is what
the removed methods were kept for.

BREAKING CHANGE: obtain a `SelectionView` with `CoinSelector::compute_view` and call
the removed methods there. `CoinSelector::select_until` takes a predicate over
`&SelectionView` rather than `&CoinSelector`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLiTkESMktypGJhFag2ZBM
`seed` carried `(CoinSelector, Ordf32)` while `best` separately held the same
score. They are set together in `seed_greedy_incumbent` and nothing runs
between construction and the first `next()`, so the score in the tuple was
always exactly `best`. Store the selection alone and read the score from
`best` when yielding.

No behaviour change: identical score, selection, round count and exhausted
flag on all 42 benchmark fixtures.
`drags_in` and `shared_drags_in` were one dense `Bitset` per candidate over every
ancestor, costing candidates x ancestors bits. On a 200,000-candidate pool with 26,666
ancestors that is 667 MB per array of very nearly nothing: a candidate drags in its
residing transactions and their unconfirmed parents, which measures mean 0.42 entries and
never more than two, so the sets are 0.002% full.

The cost is not only memory. Iterating a dense bitset is O(ancestors) per candidate
however few bits are set, so building the selection cache — which walks every candidate's
shared set — is O(candidates x ancestors) in time too. Setting up a search on 200,000
candidates took 464 ms before expanding a single node, which is enough to lose a
wall-clock budget outright: the benchmark harness reported "no solution" on that fixture
because the deadline expired during construction.

Stored flat instead: one `Vec<u32>` of indices with per-candidate offsets. Every read of
these sets is a full walk of one candidate's entries and they never change after
construction, so a slice is all they need to be. Construction reuses one scratch bitset
rather than allocating per candidate, so the old cost does not reappear while building.

    200,000 candidates, 26,666 ancestors     peak RSS      setup
      dense bitset                            1,332 MB     464 ms
      flat indices + offsets                     58 MB      54 ms

`Bitset` is unchanged where it is used over candidates — the selected and banned sets are
dense and membership-tested constantly.

Breaking: `drags_in` and `shared_drags_in` now return `&[u32]` rather than `&Bitset`.

Byte-identical to the parent commit on all 42 benchmark fixtures — same selections,
scores, round counts and exhausted flags. 80 tests green on `--all-features` and
`--no-default-features`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLiTkESMktypGJhFag2ZBM
The search order is descending `value / weight`, which the `LowestFee` bound depends on and which
cannot see ancestry at all: a candidate whose unconfirmed parents cost more to bump than the next
candidate is worth still sorts ahead of it. On a pool the search can work through, that is
invisible, because the search fixes it. On a pool it cannot — a few hundred thousand candidates,
where branch and bound returns the greedy prefix it started from — the ordering *is* the answer,
and the blind one drags in parents it did not have to.

So take a second greedy prefix, ordered by `(value - own bump) / weight`, and keep whichever of the
two the metric scores better. `local_bump` overcounts a shared parent that some other selected
candidate would have dragged in anyway, which is why this is an incumbent rather than the order the
search runs in: the ordering the bound relies on is untouched, the optimum stays reachable, and the
reordered prefix is adopted only when it actually scores better.

Costs one greedy pass and one sort, and only when the problem has unconfirmed ancestors at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLiTkESMktypGJhFag2ZBM
Depth-first traversal is linear in memory but expands whatever is under its feet, so it
prunes against whatever incumbent its dive order happened to find. On problems whose
candidates share unconfirmed ancestors that is far from the optimum, and the search
cannot recover: `subsidizing_ancestry_50` burns forty million nodes without improving on
an incumbent 2.5x worse than the answer a priority queue proves in 55,737.

This runs the same depth-first traversal in passes under a rising ceiling on the bound.
Pass k visits the nodes whose bound is at or below the threshold, which is the set a
priority queue expands before it first pops a node of that bound, so the passes
reconstruct best-first's expansion order without a frontier.

The incumbent carries across passes, and a pass ending with the incumbent at or below the
threshold proves it optimal: any better selection would have had every node on its path
bounded by its own score, so it could not have been pruned by either rule.

The threshold schedule is a speed knob and never a correctness one — raising the
threshold past the smallest rejected bound only ever adds nodes to a pass, never skips
one — so `eps` is free to trade re-expansion against how closely the queue's order is
followed.

`bnb_solutions` is unchanged and takes the plain dive; the new behaviour is opt-in
through `bnb_solutions_with_deepening`.

Measured on coinselect-benchmark's 42 fixtures at a wall-clock budget, eps=0.1:

    subsidizing_ancestry_50   40,000,000 nodes, not exhausted, child fee 11,332
                          ->      64,544 nodes, exhausted,     child fee  4,508
    shared_ancestry_200       36,242 -> 21,069     nested_ancestry_200  30,203 -> 22,477
    subsidizing_ancestry_100  30,140 -> 18,925     subsidizing_ancestry_200 27,281 -> 22,999

Exhausted rises from 31 to 34 of 42 and peak RSS stays flat at 3.5 MB. Where both
traversals exhaust they agree on all 32 fixtures, and the brute-force oracle confirms the
optimum on all 9 fixtures small enough to enumerate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLiTkESMktypGJhFag2ZBM
Iterative deepening reconstructs a priority queue's node ordering, but it is not anytime:
under a ceiling low enough to be useful the early passes may not reach a complete
selection at all. On a pool too large to exhaust that is strictly worse than diving, which
reaches leaves immediately — measured at +70% on `wallet_mixed_2000`, and no threshold
step fixes both ends, because a step large enough to protect it is large enough to lose
`subsidizing_ancestry_50` outright.

So dive first and deepen after, carrying the incumbent across. The dive hands over once it
has gone as long without an improvement as it took to find the one it holds, which keeps
its budget on a pool that is still creeping downward and gives up quickly on one that is
stuck — the failure this exists to fix.

That rule needs a floor, because the greedy incumbent is set before the first node and so
leaves it nothing to measure against. The floor scales on candidate count rather than on
the budget, which is not visible here: a dive to a leaf costs at most one node per
candidate, so the floor is that depth times a constant. 200 was the best single value over
42 fixtures at three budgets and the metric is not sharply peaked around it.

Wallet track, against the plain dive, eps=0.1:

      10 ms   -0.48%   2 better, 0 worse
     100 ms   -3.81%   5 better, 0 worse   exhausted 28 -> 31 of 42
    1000 ms   -3.89%   5 better, 1 worse   exhausted 31 -> 33 of 42

`subsidizing_ancestry_50` reaches the optimum of 4,508 after a 10,001-node dive and 8
passes. Peak RSS stays at 3.6 MB. The default path is untouched: no flag, no behaviour
change, byte-identical to the parent commit on all 42 fixtures.

The one regression is opportunity cost, not a lost incumbent: handing over ends the dive,
so against a dive that keeps the whole budget the hybrid can come out behind. It cannot
come out behind a dive given the same dive budget.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLiTkESMktypGJhFag2ZBM
Branch and bound decides candidates one at a time in a fixed order, so no
sort key it uses can express "this coin is cheap only because that other
coin is already selected". Shared ancestry is exactly that: the first coin
off an unconfirmed parent pays the whole bump and every later one off the
same parent pays nothing, and a per-candidate key has to pick one of those
two prices before it knows which the coin will be.

The benchmark measures the consequence. At 20,000 candidates every ancestry
family lands within half a percent of Bitcoin Core, and on the three
fixtures Core still takes, the ancestor bump is 73-101% of the whole gap:
coin-select takes one fewer input and builds a lighter child, then gives it
all back dragging in a parent nobody else pays for. The control that
isolates the mechanism is `private_ancestry_20000`, same shape and size but
with every ancestor reachable from one candidate: both engines report an
identical bump over identical parents and coin-select wins on weight alone.

So correct it afterwards. `CoinSelector::repair` takes a selected coin that
is the only one paying for some ancestor, tries replacing it with an
unselected coin that drags in nothing new, and keeps the swap when the
metric scores it better. Only improving swaps are kept, so the selection it
leaves is never worse than the one it was given.

On the three fixtures Core held, package fee against Core's:

  shared_ancestry_20000    1,232,220 -> 1,107,760  (Core 1,226,740)
  nested_ancestry_20000    1,386,060 -> 1,224,980  (Core 1,383,010)
  shared_ancestry_200000   1,331,840 -> 1,196,830  (Core 1,328,243)

Every loss becomes a win with about 10% to spare. It also improves two
fixtures coin-select already won, `wallet_mixed_20000` by 50%.

Gated on `has_shared_ancestors`: with each ancestor reachable from one
candidate there is nothing set-dependent for the order to have got wrong,
and the pass would be a pure cost. `run_bnb` runs it; `bnb_solutions`
cannot, being an iterator over improvements rather than a finished answer,
so callers using it directly call `repair` themselves.

Everything that scales with the pool happens once, before the loop. A first
version rebuilt the replacement set and the view on every accepted swap,
which cost 264 ms on 200,000 candidates for a thousand swaps - more than
the search it was supposed to be a cheap addition to. Sorting the whole
replacement list to read its first few hundred entries was another 3 ms of
that. As it stands the pass costs 12 ms against a 113 ms search at 200,000
candidates, and 1.1 ms against 100 ms at 20,000.

The swap ceiling is 1,000, which is a ceiling and not a target: 1,000,
20,000 and 100,000 return byte-identical selections on every scale fixture,
and the most any of them actually took was 892.
`repair` trials a swap by mutating one cached view and undoing it, up to a
thousand times over. The cache carries f64 accumulators for reachable
ancestor surplus, so if `add` and `sub` are not exact inverses the score
drifts as the pass runs and every later comparison is against a corrupted
incumbent — silently, since nothing recomputes it from scratch.
…banned

Review found a regression the 42-fixture benchmark cannot see, because every
fixture starts from an empty selection.

**`run_bnb` was dropping caller-preselected inputs.** Branch and bound only
ever selects and bans - `BnbIter` starts from a clone and never touches the
root selection - so `run_bnb` has always returned a superset of what the
caller had selected. That is how a wallet pins a required UTXO, and it is
what `select_srd` documents for itself. `repair` deselects, making it the
one thing in the search path that can break it, and it did: on a four-coin
pool where the required input is the sole payer for its parent, `run_bnb`
silently returned a cheaper selection without it.

The gate did not confine the damage either. `has_shared_ancestors` is a
property of the problem, while the outgoing set is built from `drags_in`,
which includes private ancestors - so one shared parent anywhere in the pool
exposed every privately-parented required coin. `repair` now takes the
protected set explicitly and `run_bnb` passes the selection it was handed.

**The search's transient bans were truncating the replacement pool.** The
selector comes back mid-traversal carrying every ban the exclusion frames on
the path to that node applied, and `free` is built from `unselected_indices`,
which excludes banned. So how much of the pool the pass could see depended on
where the last improvement happened to be found. Not a wrong answer, but it
made the measured benefit an artifact: the fixtures showing the large gains
are the ones returning the ban-free greedy seed. `run_bnb` now restores the
caller's ban set before repairing.

Also from the review:

- `free.retain` per accepted swap was pool-sized work back inside the loop,
  which is exactly what hoisting it out was for. A bitset of taken indices
  instead: on a 200,000-candidate pool taking 1,000 swaps, 49.6 ms -> 8.7 ms,
  answers byte-identical. The 12 ms figure in the commit before this was a
  low-swap measurement and did not show it.
- `has_shared_ancestors` is true as soon as one unconfirmed parent has two
  spendable outputs, which is the everyday shape of a wallet that made a
  payment with change - so a selection holding no sole-payer at all still
  paid for a full pool-sized pass. Checking that first costs nothing, since
  the refcounts are already built.
- The head-sizing comment claimed at most one entry consumed per selected
  coin. A swapped-in coin joins the selection and can be swapped out later,
  so the bound is `max_swaps`. Sized by that instead.
- `REPAIR_REPLACEMENTS_PER_PASS` is private: nothing takes it as an argument,
  so a caller could read it and not use it.
- The round-trip test says what it actually establishes - `add` and `sub`
  touch the accumulators in different orders, so exactness there is a
  property of the magnitudes. What makes it safe is that the only reader is
  the bound, which `repair` never calls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant