Skip to content

Conversation

@ValuedMammal
Copy link
Collaborator

@ValuedMammal ValuedMammal commented Dec 10, 2025

Description

Fix #29 by removing ChangePolicyType from SelectorParams in favor of bdk_coin_select::ChangePolicy. The idea is that this affords maximum flexibility when defining the change policy.

Builds on #31 which implements absolute fee targeting. h/t aagbotemi. A test test_absolute_fee_vs_feerate_target_value is added that checks the expected Target is constructed based on the chosen FeeStrategy.

Changelog

Added

  • selector: Added FeeStrategy enum which defines the fee target as either a feerate or fee amount.

Changed

BREAKING

  • SelectorParams::target_feerate field is changed to fee_strategy.
  • SelectorParams::change_policy field is changed to have type bdk_coin_select::ChangePolicy.
  • SelectorParams::new is changed to accept the fee_strategy and change_policy as inputs.

Removed

  • SelectorParams::change_weight field is removed now that the change weights are represented in the actual change policy.
  • Removed SelectorParams::to_cs_change_policy as the conversion is no longer necessary.
  • Removed ChangePolicyType enum to allow the user to construct the intended ChangePolicy.

The spend weight now accounts for both
the weight of a default `TxIn` as well as the
expected satisfaction weight.
This is redundant now that SelectorParams holds the
ChangePolicy directly.
The name "strategy" is better suited for the enum, as it implies
different strategies can be used, and helps to disambiguate from
other similarly named types `Target` and `TargetFee`, etc.
@ValuedMammal ValuedMammal force-pushed the refactor/change_policy branch from 71169fd to b04db7e Compare December 15, 2025 18:34
@ValuedMammal ValuedMammal changed the title [selector] Refactor change policy selector!: Swap ChangePolicyType for bdk_coin_select::ChangePolicy Dec 15, 2025
@ValuedMammal
Copy link
Collaborator Author

Rebased on #31.

@ValuedMammal ValuedMammal marked this pull request as ready for review December 15, 2025 18:40
Copy link
Collaborator

@aagbotemi aagbotemi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this PR. This approach is solid.

ACK b04db7e

@ValuedMammal ValuedMammal merged commit b8a7a82 into bitcoindevkit:master Dec 17, 2025
6 checks passed
@ValuedMammal ValuedMammal deleted the refactor/change_policy branch December 17, 2025 20:07
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.

Make ChangePolicyType more flexible

2 participants