feat(metrics): add ChangelessWaste for privacy-driven coin selection#47
Draft
evanlinjin wants to merge 8 commits into
Draft
feat(metrics): add ChangelessWaste for privacy-driven coin selection#47evanlinjin wants to merge 8 commits into
evanlinjin wants to merge 8 commits into
Conversation
8606b6f to
bbaf01b
Compare
d03543f to
a52499b
Compare
69e4f74 to
48baab7
Compare
Adds a waste metric restricted to changeless selections. Removing the change output from the picture eliminates the non-monotonic discontinuity that complicates the general waste bound, so the LB reduces to a lower bound on `input_weight * (feerate - long_term_feerate)`. For target-not-met, rate_diff >= 0 we reuse LowestFee::bound's resize trick applied to input_weight rather than fee. Includes proptests `can_eventually_find_best_solution` and `ensure_bound_is_not_too_tight`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous bound for `rate_diff < 0` was `all_selected.input_weight * rate_diff`, which ignored that selecting every candidate would typically force a change output (making the selection infeasible under the changeless constraint). The new bound recasts the problem as: minimize the weight of candidates excluded from `D_all` such that `excess_with_drain` drops below `change_policy.min_value`. This is a 0/1 covering knapsack; the LP relaxation (sort positive-ev_feerate candidates by `ev/weight` descending and exclude fractionally) gives a safe upper bound on `D.input_weight` for any feasible changeless descendant. Benchmark improvements (round counts, BnB cap 2M): n=15 rate_diff_neg: 3,590 -> 415 (~9x) n=20 rate_diff_neg: 34,296 -> 2,138 (~16x) n=30 rate_diff_neg: 2M cap -> 74,055 (>27x, was hitting cap) rate_diff >= 0 paths are unchanged. `ensure_bound_is_not_too_tight` proptest verifies the new LB across 256 randomized scenarios. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`LowestFee::bound` and `ChangelessWaste::bound` both used the same "resize trick" to lower-bound a monotone quantity of any target-meeting descendant: walk the value_pwu-sorted unselected list until the target is crossed, then fractionally resize the crossing input to hit the target with zero excess. Extract it into a private `resize_bound` helper in `metrics.rs`, returning a `ResizeBound` enum so each caller keeps its own exact-match computation (`LowestFee` returns `score`, `ChangelessWaste` returns `waste`) while sharing the find + deselect + scale mechanism. No behavior change; all tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nt fees Both `ChangelessWaste::bound` paths mixed rate-based `effective_value` with the `min(rate, absolute, replacement)` excess, producing an invalid (too-tight) bound / wrong prune when a non-rate fee constraint binds. A too-tight BnB bound silently prunes the optimum, yielding a suboptimal changeless selection. - `ub_changeless_input_weight`: the LP knapsack credits each removed candidate its *rate*-based `effective_value`, but when `absolute_excess` or `replacement_excess` is the binding constraint, removing a candidate drops it by more than that (full `value`, or `value - weight*incremental_relay_feerate`). Crediting the smaller rate-ev over-removes weight, so the resulting upper bound on `input_weight` falls below the true max and the bound becomes too tight. Fall back to the trivial (always valid) `d_all.input_weight()` UB whenever an absolute fee or replacement is present; keep the tight knapsack for the rate-dominated case. - `change_unavoidable`: the "least excess" construction adds negative-*rate*-ev candidates, which does not minimise the true excess when a replacement is present (a rate-negative candidate can be replacement-positive and raise `replacement_excess`), so it could wrongly prune a branch that holds a changeless solution. Return `false` (never prune — always safe) when a replacement is present. An absolute fee is safe here since `absolute_excess` only grows as inputs are added. Adds a deterministic regression test with high-weight candidates and a binding absolute fee (the default random pool never generates the weight/value ratio that exposes the gap), and threads an `absolute` fee through the test `StrategyParams` so the ChangelessWaste proptests exercise it. Found by high-effort multi-agent code review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
6fccf10 to
fda1d77
Compare
ChangelessWaste::drain_value documented "the same rule" as LowestFee but was missing the max_weight clause: change whose output would push the tx over the cap must be refused (excess goes to fee), making the selection changeless. Without it, selections the documented policy admits were rejected by score(). Refusal also breaks two assumptions in bound(), so guard both: - change_unavoidable: a descendant whose change is refused by the cap is changeless regardless of its excess, so the least-excess prune is only sound when no descendant can trigger the refusal (i.e. even the heaviest descendant fits the cap with the drain added). - ub_changeless_input_weight: a cap-refused descendant keeps every excess-contributing candidate, invalidating the knapsack's premise that changeless descendants must shed positive-ev weight. Fall back to the D_all upper bound clamped by the cap. Both guards are covered by deterministic regression tests that fail with the guard removed, via a new assert_bound_admissible helper that also catches invalid prunes (ensure_bound_is_not_too_tight skips bound=None branches, so it cannot). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ht corrections The bound/prune logic mixed weight-unit-linear candidate arithmetic (effective_value = value - raw_weight * spwu) with the true excess, which is computed from the vbyte-rounded fee (fee(W) in [W*spwu, W*spwu + sat_vb + 1]) on the corrected weight (input_weight() adds the segwit header, +1 per legacy input in a segwit tx, and varint growth on top of raw weights). Three spots turned that mismatch into an invalid bound or prune, silently making BnB skip the optimal (or only) changeless solution: - change_unavoidable: a candidate with small positive linear ev can still lower a descendant's true excess, so the least-excess construction wasn't minimal. Prune only when the construction clears the changeless edge by a slack of (corrections(D_all) - corrections(cs)) * spwu + sat_vb + 1. - ub_changeless_input_weight: a descendant can become changeless while shedding up to sat_vb + 1 sats less than `delta` in linear terms, so the knapsack over-removed weight. Demand delta - (sat_vb + 1). - the rate_diff >= 0 lower bound reused the resize_bound walk, whose crossing point and scale embed the greedy prefix's actual corrected weight and rounded fee - which an all-legacy descendant avoiding the prefix's segwit corrections can beat. Replace it with a fractional knapsack in raw-linear terms only (cs's corrections are common to all descendants and corrections only grow, so the raw-linear quantity is a true floor). This also subsumes the previous is_funded fast path (funded => gap <= 0). Each fix has a deterministic regression test that fails without it, plus a segwit-heavy proptest (gen_candidates makes segwit candidates only 1% of the time, which is why the existing suite never hit these). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… f32 The rewritten rate_diff >= 0 bound computed its funding gap by subtracting f32 conversions of full-magnitude sat values. f32's 24-bit mantissa is coarser than a sat above ~0.17 BTC (32 sats at 5 BTC), so an exactly-funded selection could produce a phantom positive gap, and with only negative-ev candidates left the phantom gap became an invalid None prune of a branch whose own selection is funded, changeless and scoreable — BnB would silently drop the optimal (or only) solution at BTC-scale values. - Reinstate the exact-integer is_funded fast path: funded-ness at the boundary is never decided by a float sign. - Run the gap walk (and the other two sat-magnitude comparisons from the previous commit: change_unavoidable's slack test and the knapsack delta) in f64, which is exact for every sat amount. - Prune only on a residual gap above half a sat; a sub-half-sat residual is bounded instead of pruned. Regression test uses the review verifier's concrete reproduction: 5 BTC candidate, integer excess exactly 0, f32 gap +32 (both values sit on round-to-even ties that resolve away from each other); it fails with a "bound pruned the branch but a descendant scores" panic before this fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review round 2 polish, no behavior change: - build the all-selected descendant once per bound() call and pass it to change_unavoidable and ub_changeless_input_weight instead of each cloning + select_all-ing its own (bound runs at every BnB node) - rewrite resize_bound's doc: drop the "(fee, input_weight, ...)" generality and the ideal_iw example — that exact usage was removed from ChangelessWaste as an invalid bound — and document the corrections caveat for the remaining fee-space caller - assert (not just print) the load-bearing preconditions of the corrections-prune and vbyte-rounding regression tests, so fee-model drift cannot turn them vacuous Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Summary
Adds
ChangelessWaste, a BnB metric that minimises the waste metric subject to the constraint that the selection produces no change output.The use case is privacy-driven coin selection — CoinJoin / PayJoin / send-to-one-shot addresses where the change output is itself a leak. This matches what Bitcoin Core's BnB-CB algorithm covers. We deliberately don't expose a cross-regime
Wastemetric (see Why no generalWastemetric? below).This branch is rebased onto the current
master, which landed two breaking refactors the metric now conforms to:Targetis passed toBnbMetricmethods per-call instead of stored on the metric.BnbMetric::drain) rather than taking aChangePolicy.Public additions
ChangelessWaste— likeLowestFee, it owns its change decision instead of taking aChangePolicy. Fields:long_term_feerate,dust_relay_feerate,drain_weights. It uses the same rule asLowestFeeto decide whether a selection would have change (change is worthwhile when the recovered excess exceeds the future spend cost and clears the dust threshold), and rejects any such selection — only genuinely changeless selections are scored.drain()returnsDrain::NONEby definition.For a changeless selection the waste collapses to
input_weight * (feerate - long_term_feerate) + max(0, excess), which (unlike the general with-change waste) is monotone, so the bound reduces to boundingD.input_weightfor any target-meeting descendantD ⊇ cs:rate_diff >= 0: lower-boundD.input_weight—cs.input_weight()when the target is already met, otherwise the resize trick (below).rate_diff < 0: upper-boundD.input_weightvia an LP-relaxed fractional knapsack over the positive-effective_valuecandidates that must be excluded to keep the selection changeless (i.e. to keepexcess_with_drain_weight <= max(drain_spend_cost, dust_threshold - 1)).Branches where every reachable descendant is forced to have change are pruned via
change_unavoidable(the same heuristicChangelessuses).Includes proptests
can_eventually_find_best_solution(finds the true optimum) andensure_bound_is_not_too_tight(the bound is a valid lower bound), plus two hand-written regime sanity checks.Internal cleanup
LowestFee::boundandChangelessWaste::boundshared the same "resize trick" — walk thevalue_pwu-sorted unselected list until the target is crossed, then fractionally resize the crossing input so the target is hit with zero excess. Extracted into a single private helper insrc/metrics.rs:resize_boundreturns aResizeBoundenum (Exact|Resize) so each caller keeps its own exact-match computation —LowestFeereturnsscore,ChangelessWastereturnswaste— while sharing the find + deselect + scale mechanism. No behavior change forLowestFee(verified by its existing proptests).Bound tightening for
ChangelessWaste(rate_diff < 0)The consolidation case (
rate_diff < 0) previously used a trivialall_selected.input_weight * rate_diffupper bound, which ignores that selecting every candidate would typically force a change output. The LP-relaxed knapsack UB is dramatically tighter:(round counts, BnB cap 2M).
rate_diff >= 0paths are unchanged.Why no general
Wastemetric?Earlier iterations included a cross-regime
Wastemetric scoring both changeless and with-change selections. It was dropped because:ChangelessWastehas a clean bound.long_term_feerate > feerate(a common low-fee scenario) every input has negative waste contribution, so the unconstrained BnB optimum is "select every UTXO" — rarely what a caller wants.ChangelessWaste.Note
ChangelessWasteis a standalone metric (notChangeless<Waste>) precisely because its value is the custom tightinput_weight-based bound — composing a generalWastemetric would mean building that hard cross-regime bound only to constrain it away.Test plan
cargo test --release— full suite passes (40 tests incl. lib + doctests), notably theensure_bound_is_not_too_tightandcan_eventually_find_best_solutionproptests for bothChangelessWasteandLowestFee.cargo clippy --all-targets --all-features— clean.cargo fmt --all --check— clean.cargo doc --no-depswith-D warnings— clean.🤖 Generated with Claude Code