Add Target::max_weight, built on the metric-decides-change refactor (#49)#51
Open
evanlinjin wants to merge 4 commits into
Open
Add Target::max_weight, built on the metric-decides-change refactor (#49)#51evanlinjin wants to merge 4 commits into
evanlinjin wants to merge 4 commits into
Conversation
30550ca to
4858294
Compare
781e3c3 to
36d7eee
Compare
Add an optional `Target::max_weight` cap (in WU) on the resulting transaction — e.g. for TRUC/BIP-431 packages capped at 10,000/1,000 vB. It's a feasibility constraint mirroring the value target: value is a lower bound, `max_weight` an upper bound. Enforced only where a selection is committed: - `is_within_max_weight`: the anti-monotone weight predicate, kept separate from the monotone value-only `is_target_met` (whose docs disclaim the cap). - `select_until_target_met` now errors with `SelectError::MaxWeightExceeded` instead of silently returning an over-cap selection. - `LowestFee`: refuse a change output that would bust the cap, reject any over-cap selection, and hard-prune subtrees whose lightest (no-drain) form already busts it. A cap-agnostic `fee_score` feeds the bound's estimates so they stay admissible. Because `LowestFee` decides change economically, the existing bound proof survives the cap, so the hard-prune alone suffices. `is_selection_possible` stays a value-only check. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
78fda32 to
8106a2a
Compare
Thread `max_weight` through `Target`, `StrategyParams` and the proptests so BnB is checked against exhaustive search with the cap active. Add `exact_selection_possible`, an exhaustive value-and-weight feasibility oracle, and a `bnb_respects_max_weight` proptest asserting BnB matches it — this validates the metric's weight hard-prune. Impossibility assertions switch from the value-only `is_selection_possible` to this oracle where the cap is in play. Tighten `compare_against_benchmarks`: filter benchmarks by `score().is_some()` (actually-valid solutions, reusing the computed score) rather than the value-only `is_target_met`, add a value-per-weight greedy benchmark so the comparison isn't vacuous under a tight cap, and assert BnB itself found a valid solution whenever a benchmark did (a `None` score used to pass silently). Cap its `n` — the no-solution branch's oracle is O(2^n). Also mark `bnb_always_finds_solution_if_possible` release-only, matching its sibling proptest; it's too slow under debug assertions. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
`Target` now carries two constraints of opposite monotonicity — the value target (a lower bound, monotone) and `max_weight` (an upper bound, anti-monotone). Rename the value-only predicates so their names say which one they check: - `is_target_met` -> `is_funded` - `is_target_met_with_drain` -> `is_funded_with_drain` - `is_selection_possible` -> `is_fundable` "funded"/"fundable" is money-centric and so naturally excludes weight; the `-ed`/`-able` pair distinguishes "this selection covers the value" from "these candidates *can* cover it". Pure rename, no behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
8106a2a to
86c514f
Compare
The bound was cap-blind: admissible but loose, crediting improvements the
weight budget can't actually afford. Tighten it in the cap-binding regime:
- Not-funded ("slurp") branch: reaching the feerate needs a perfect input
weighing `scale * to_resize.weight`. `to_resize` is the best value-per-weight
input available, so if even that can't fit the remaining budget, nothing down
this branch reaches the target within the cap -> prune. It's a fractional
relaxation, so it never prunes a branch with an integer solution.
- Changeless "recover value by adding change" branch: only credit the
improvement if a change output fits the cap. Clearing dust needs heavier
inputs and the change output adds weight, so if change doesn't fit now it
never will -> keep `current_score`.
Both stay admissible (`ensure_bound_is_not_too_tight`) and BnB still finds the
optimum (`can_eventually_find_best_solution`). No effect unless `max_weight`
is set.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
86c514f to
a4fddfe
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
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.
Alternative to #48 for capping transaction weight during coin selection — e.g. TRUC / BIP-431 packages limited to 10,000 / 1,000 vB.
The key design choice: this is built on the metric-decides-change refactor (#49), because that's what keeps the weight cap from breaking
LowestFee's branch-and-bound bound (see Why build on #49).Four commits: (1) logic, (2) tests, (3) a pure rename of the value-feasibility predicates, (4) an optional bound speedup.
Approach
Target::max_weight: Option<u64>— a feasibility constraint that mirrors the value target: value is a lower bound,max_weightan upper bound.is_funded), the cap is a distinct anti-monotone check (is_within_max_weight). That separation is what preserves the monotonicity the BnB bounds rely on.LowestFee(which owns the change decision post-feat!: BnB metrics decide the change output themselves #49): refuses change that would bust the cap, rejects any over-cap selection, and prunes subtrees whose lightest (no-drain) form already busts it.select_until_target_metnow returnsSelectError::MaxWeightExceededinstead of silently returning an over-cap selection.Why build on #49
LowestFee's branch-and-bound rests on one claim about its bound: a selection with a worthwhile change output can't be beaten by a descendant that adds inputs to become changeless. Whether that holds depends on who decides the change policy.Before #49, the change policy is decoupled from the metric, so a selection can carry a change output that doesn't actually minimize the fee. Pile non-effective inputs onto it — each costs more in fee than it adds in value, shrinking the excess — until the change vanishes, and the resulting changeless selection can score better. The bound trusted the with-change score as a floor for the subtree; now it's too high, BnB prunes the branch holding the real optimum, and you get a suboptimal selection. (This is the inadmissibility #48 runs into.)
After #49, the metric owns the decision: change is added only when it strictly lowers the long-term fee (
change_value > spend_fee). Now dropping a worthwhile change can never help. WithAa worthwhile-change selection andB = A + inputs(added valuev ≥ 0) made changeless:Balways costs more, so the bound stays admissible.And that's exactly what
max_weightneeds. The cap enforces itself by refusing a change output that would bust it — i.e. it can force a descendant to be changeless. The proof never asks whyBis changeless, only thatA's change was worthwhile. So on the #49 base the cap needs nothing more than a weight hard-prune — no bound rewrite, no lost optimality. Fold it into the value predicate on the old base instead, and the suboptimality above comes right back.Commit 3: renamed predicates
Targetnow carries two constraints of opposite monotonicity, so the value-only predicates are renamed to say so — "funded/fundable" is money-centric and naturally excludes weight, and-ed/-abledistinguishes "this selection covers the value" from "these candidates can cover it":is_target_met→is_funded,is_target_met_with_drain→is_funded_with_drain,is_selection_possible→is_fundable.Commit 4: cap-aware bound (optional speedup)
Correctness only needs the hard-prune above. Commit 4 additionally teaches the bound about the cap — pruning subtrees that are still under-cap but provably can't reach the value target without busting it. It never changes the result (and stays admissible — the
ensure_bound_is_not_too_tightproptest passes), but it's a large win in the tight-cap regime a safety rail actually lives in.Measured BnB rounds to completion on a variable-weight pool, cap-aware bound off → on:
max_weightThe headline is the first row: for a too-tight cap the bound proves infeasibility instantly, instead of enumerating the entire subtree before giving up.
Testing
max_weightis threaded through theLowestFeeproptests, so BnB is checked against exhaustive search with the cap active.bnb_respects_max_weightproptest cross-checks BnB feasibility against an independent exhaustive oracle — this is what validates the weight hard-prune.Breaking changes
Target { .. }literals now require amax_weightfield.is_target_met→is_funded, etc.).🤖 Generated with Claude Code