refactor!: Replace Selector with a single into_selection pass#82
Open
evanlinjin wants to merge 3 commits into
Open
refactor!: Replace Selector with a single into_selection pass#82evanlinjin wants to merge 3 commits into
Selector with a single into_selection pass#82evanlinjin wants to merge 3 commits into
Conversation
Fold the whole selection lifecycle into `InputCandidates::into_selection`
and remove the public `Selector` struct. Algorithms now operate directly
on a `bdk_coin_select::CoinSelector` plus a pure-data `SelectionContext`,
instead of a wrapper that conflated lifecycle plumbing with the algorithm
handle.
- `SelectorParams`/`SelectorError` -> `SelectionParams`/`SelectionError`.
- Unify the long-term feerate into one `SelectionParams::longterm_feerate`
(`None` = target feerate), feeding both the change policy and the bnb
metric; the change policy is now always waste-aware.
- `IntoSelectionError` carries distinct `CannotMeetTarget { missing }`
(impossible) vs `AlgorithmFellShort` (algorithm under-selected) variants.
- Fix the reachability pre-check to select effective inputs at the resolved
target feerate (`target.fee.rate`), not the raw one, avoiding a false
`CannotMeetTarget` under RBF.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…match` `SelectionError` lumped change-policy construction failures together with candidate timelock validation, so `to_cs_change_policy` advertised a `LockTypeMismatch` variant it could never return. - `SelectionError` -> `ChangePolicyError` (just `Miniscript` / `InsufficientAssets`), now the honest return type of `to_cs_change_policy`. - `IntoSelectionError::Setup(_)` -> `ChangePolicy(ChangePolicyError)`, and `LockTypeMismatch` is promoted to a top-level `IntoSelectionError` variant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The module no longer holds a `Selector`; it now holds `SelectionParams` and its supporting config/context/error types. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ad96c68 to
e162434
Compare
2 tasks
evanlinjin
added a commit
to evanlinjin/bdk-tx
that referenced
this pull request
Jun 27, 2026
API: - Move longterm_feerate from SelectionStrategy::LowestFee to SelectParams: it feeds the waste-aware change policy for every strategy, and post-bitcoindevkit#82 the bnb algorithm reads it from SelectionParams rather than as an argument. LowestFee now carries only max_rounds. - Rename SelectParams::fee_rate -> feerate. Cleanup: - Inline single-use, Wallet-bound helpers (change_keychain, try_plan, wallet_assets, list_indexed_txouts) and drop the bdk_coin_select_insufficient wrapper. Pure, individually-testable helpers (status_from_position, extended_keys) are kept. Co-Authored-By: Claude Opus 4.8 (1M context) <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.
Description
Reworks the coin-selection API around a single
InputCandidates::into_selectionpass and cleans up its error surface.The public
Selectorstruct is removed as nothing used it. Its methods were only ever called in sequence byinto_selection, so it was public surface with no consumer. That logic now lives insideinto_selection, and algorithms take abdk_coin_select::CoinSelectorplus a pure-dataSelectionContextinstead of a&mut Selector.This also:
SelectionParams::longterm_feerate;SelectorErrorso each error type only carries variants it can actually produce;CannotMeetTargetunder RBF.Notes to the reviewers
min_value_and_waste). Previously aNonelong-term feerate selected a plain dust-only policy. Consequence: raw-script change must now supply a realsatisfaction_weight(descriptor change is unaffected — it's derived). Rationale:min_value_and_wastewithlongterm == targetdoes not collapse to dust-only; it still charges the change output's lifetime cost, which is the correct thing to weigh.select_all_effective(raw target_feerate)but scored withexcess(target.fee.rate)(the RBF-resolved, higher rate). This could result in an under-selection. Fixed by selecting attarget.fee.rate.to_cs_change_policypreviously returned aSelectorErrorthat included aLockTypeMismatchit could never produce. It now returnsChangePolicyError(onlyMiniscript/InsufficientAssets), andLockTypeMismatchis a top-levelIntoSelectionErrorvariant.Changelog notice
Before submitting