refactor!: eliminate redundant satisfaction weight in SelectorParams

[evanlinjin]

Fixes #40
Fixes #42

Summary

This PR fixes two issues with SelectorParams:

1. Redundant satisfaction weight (invalid representation)

SelectorParams previously had two independent fields that could both specify the change output's satisfaction weight:

• change_script: ScriptSource — when this is the Descriptor variant, the satisfaction weight is derivable from the descriptor via max_weight_to_satisfy.
• change_policy: ChangePolicy — also encodes the satisfaction weight (the spend cost of the change output).

Nothing enforced that these agreed with each other, so callers could silently provide inconsistent values.

Fixed by introducing ChangeScript, which bundles a raw script with an optional satisfaction_weight, or holds a DefiniteDescriptor from which it is derived automatically. DrainWeights is now computed internally from ChangeScript — there is a single source of truth.

ChangeScript::Descriptor also accepts optional satisfaction_assets — when provided, the satisfaction weight is computed via Plan for a tighter estimate (useful for multisig/complex descriptors). Otherwise it falls back to max_weight_to_satisfy.

The raw bdk_coin_select::ChangePolicy is also replaced with a ChangePolicy enum (NoDust, NoDustLeastWaste) that is converted to the bdk_coin_select type internally. This is Debug + Clone + PartialEq + Eq.

Both ChangeScript and ChangePolicy have constructor methods to reduce boilerplate:

• ChangeScript::from_descriptor(desc), from_descriptor_with_assets(desc, assets), from_script(script, weight)
• ChangePolicy::no_dust(), no_dust_least_waste(rate), with a builder-style .min_value(amt)

2. Hacky AbsoluteFee implementation

FeeStrategy::AbsoluteFee was implemented by smuggling the fee into value_sum and setting the feerate to zero. This meant the coin selector had no awareness of the fee, which interacted poorly with RBF logic (the zero feerate bypasses the original tx's feerate floor). Removed in favor of a direct target_feerate: FeeRate field.

Design rationale

The satisfaction weight is a physical property of the script — it describes how much it costs to spend a particular output. It is not a policy choice. When change_script is a descriptor, the satisfaction weight is literally derived from it. The fact that it affects coin selection doesn't make it part of the change policy, any more than the dust threshold is (and dust is already derived from the script).

For the raw script case (e.g. silent payments), the satisfaction weight can't be derived and must be provided externally — but it's still a property of the script, not a policy decision. Bundling it into ChangeScript::Script makes this explicit.

Context

• feat(selector): allow multiple change sources with ChangeDescriptor #18 (comment)
• selector!: Swap ChangePolicyType for bdk_coin_select::ChangePolicy #32 (comment)

[aagbotemi]

Since the previous implementation for AbsoluteFee was broken for RBF, removing it is correct. Targeting absolute fee is a legitimate use case and the open PR on bdk_coin_select #38 adding TargetFee::absolute as a field addresses this. Once that merges, SelectorParams and to_cs_target() should be updated by exposing and mapping onto it.

[nymius]

cACK ffd1211

This is similar to #18 original's idea.

I have to review closer the RBF considerations, but I understand the removal of FeeStrategy as it is mainly needed for the absolute fee case, and it can be covered in other better ways.

I agree that ChangePolicy is a decision based on constraints, and the weight derived from ChangeScript is a property, so I don't see any concern raising from clearly reflecting that in the API.

[ValuedMammal]

Appreciate your efforts, I'd just ask that you stick to the contribution guidelines in particular with regard to the contribution workflow.

• Can you link to the issue being fixed, or open one if you haven't already
• If you do a PR can you try to fix one issue at a time
• Can you include a test if adding a feature or fixing a bug

[evanlinjin]

@ValuedMammal

Can you link to the issue being fixed, or open one if you haven't already

Added: #40

If you do a PR can you try to fix one issue at a time

These changes can't be split — they're one refactor of SelectorParams:

• ChangeScript fixes the redundant satisfaction weight.
• ChangePolicy enum is required by the fix — the old bdk_coin_select::ChangePolicy bundled DrainWeights which contained the satisfaction weight. Can't remove the redundancy without replacing this type.
• Removing FeeStrategy::AbsoluteFee is entangled with the same to_cs_target() being refactored.
• Constructor methods are just helpers for the new types.

Splitting would create intermediate states where the API is half-migrated.

Can you include a test if adding a feature or fixing a bug

You can't really "test" that two fields can't disagree anymore - the fix is that the problematic API shape no longer exists. The compiler enforces it.

[ValuedMammal]

From what I gather your two primary concerns are to

1. Revert FeeStrategy
2. Change the shape of the API

[ValuedMammal]

🔧 It'd be better to not assume the spending transaction is segwit, unless the change script specifically encodes a witness program. You could make it a configurable is_segwit flag as part of the ChangePolicy.

🔧 Change outputs don't typically have a spend weight of 0. Recall that the cost of change is property of the output script, which has implications for coin selection, and less so with the ChangePolicy enum. So I'd say remove the considers_waste function.

[evanlinjin]

Change outputs don't typically have a spend weight of 0.

Yes, but we aren't using that value at this point anyway.

[evanlinjin]

The spend_weight: 0 for NoDust is intentional — bdk_coin_select::ChangePolicy::min_value doesn't use spend weight in its calculations. The reason we gate on considers_waste() is that ChangeScript::from_script allows satisfaction_weight: None, so callers using NoDust with a raw script don't need to provide a satisfaction weight they'll never use. Always computing the real spend weight would force every caller to provide one, making the API more burdensome for no practical benefit.

Regarding the segwit assumption — agreed it's not ideal, but whether the future spending tx is segwit depends on ALL its inputs, not just the change script. We can't know that at coin selection time. That said, assuming segwit is a conservative estimate (slightly overestimates spend cost by ~1 WU), so it seems like a reasonable default.

[ValuedMammal]

We're getting the satisfaction weight from the user, so just make it non-optional rather than introduce a new error path.

[evanlinjin]

The satisfaction weight may not always be easy to compute for raw scripts (e.g. silent payments or exotic script types). Making it non-optional forces callers to compute it even when their ChangePolicy doesn't need it. The error path exists precisely to catch the mismatch at runtime rather than burden all callers upfront.

[evanlinjin]

@nymius can you explain why change output's spend weight cannot be determined for a silent payment tx?

Aren't silent payment outputs always P2TR keyspend path? If this is the case, we can remove the Option<spend weight> for ChangeScript as @ValuedMammal suggested and have a cleaner API.

Edit: I could also be misinterpreting this comment here: #18 (comment)

[nymius]

Aren't silent payment outputs always P2TR keyspend path?

Yes, that's true and naturally, it also applies to silent payment change outputs.

Edit: I could also be misinterpreting this comment here: #18 (comment)

I must have eluded the almost fixed size of p2tr keypath spend witnesses there, and if not, I was giving more importance to the +-1 WU difference than I should have.