Update coin selection terminology

[murchandamus]

This renames the following:

• must_use_utxos ⇒ required_utxos
• may_use_utxos ⇒ optional_utxos
• get_must_may_use_utxos ⇒ preselect_utxos

"must_use" and "may_use" are visually similar and this change improves the readability as well as better describing the two separate sets.

[murchandamus]

Strong opinions, weakly held. ;)

[afilini]

Ahahaha, as I said on Discord I don't have any particular preference between the two, but I also don't think my opinion should matter too much in this context because I'm generally terrible at naming things..

I wanted to get some feedback from native english speakers just to see what they think about this, then we'll decide what to do :)

[notmandatory]

I prefer this new terminology, it's shorter and clearer. It will impact @danielabrozzoni for PR #148, and @LLFourn originally introduced the current names. If they have no objections I think we should accept this PR.

[danielabrozzoni]

Cool for me, ACK :)
I'll update #148 accordingly.

[LLFourn]

ACK. One comment needs to be fixed up.

Fort those interested in the way I think about naming:

In general I try and tune the level of abstraction in the names of things to my level of confidence about them. For example get_must_may_use_utxo is a very concrete name which indicates I don't have much confidence in it. This is a hint to the next person that they shouldn't be afraid to totally remove it if they think of a better way of arranging things.

preselect_utxos on the other hand is a more abstract and therefore more confident name. The hint for the reader is that the author thought that "utxo preselection" is a necessary step when solving the problem.

I don't have an issue with preselect_utxos since perhaps by having a single function that takes all the builder arguments and returns the two utxo lists I am really closely modelling the abstract problem of figuring out which inputs a transaction can use.

WRT to may/must vs optional/required I don't have much of a preference.

[LLFourn]

change the comment above as well.

[murchandamus]

I've updated the comment and amended the change into the optional_utxos commit.

[murchandamus]

Thanks, your comment on naming is great food for thought.
Regarding preselect_utxos, you've found a better explanation than I could have explicitly formulated. The coin selection code I was dealing with previously had to scale to wallets holding hundreds of thousands of UXTOs. The Branch and Bound algorithm cannot search that amount of data. Reducing the search space in advance of more computationally intensive steps worked out to be pretty important. Filtering uneconomic UTXOs is just the first most obvious shortcut.

[danielabrozzoni]

As @xekyo was saying in #148, there are some other variables that might benefit from a name change :)
https://github.com/bitcoindevkit/bdk/pull/148/files/04b88295c1aec9172076ada6ce4199c2e905da40#r513023603
https://github.com/bitcoindevkit/bdk/pull/148/files/04b88295c1aec9172076ada6ce4199c2e905da40#r513028036 (here I prefer output_and_header_fees, but payload_fees is fine anyway)

(TL;DR: Mark suggests to change

• amount_needed -> sum_receiver_outputs
• fee_amount -> output_and_header_fees or payload_fees)

I actuall like both proposals, but I would feel better if we changed them in the whole coin selection code and not only in the bnb implementation :) Do you think you can add those changes in this MR too?

Thoughts? @LLFourn @notmandatory

[notmandatory]

As a coin selection novice I like the current names amount_needed and fee_amount because based on the docs I think I understand what they mean. I also prefer shorter names if possible.

Could there be any other amount needed by coin selection other than to pay for the sum of the receiver's outputs?

For fee amount it's a little less clear, it's really the current accumulated fee amount, but fee amount for short still makes sense. Could future algorithms need to keep track of some other fee amount?

That all said, these are my weakly held opinions. :-)

[LLFourn]

amount_needed -> sum_receiver_outputs

I think amount_needed is better since it implies that this is the goal of the coin selection

fee_amount -> output_and_header_fees or payload_fees

I agree that fee_amount is not great since it is really "what we've computed as the fee so far, please add to this and return it". I don't have a strong opinion about what to replace it with.

[murchandamus]

I think amount_needed is better since it implies that this is the goal of the coin selection

My point was that amount_needed was a bit ambiguous in that it is the amount needed to pay the recipients, but the actual selection target at that point is amount_needed + fee_amount, where fee_amount is the cost of the outputs and the transaction header. The inputs' costs are dynamically gathered by accounting for the inputs per their effective_value.

That said, I wouldn't want to suggest that it's a priority or a big win to change either.

[afilini]

I guess I'll just merge this one now since more or less everybody seems to agree that the current terminology is at least not terrible.

If you have more ideas for other things that would benefit from a renaming, feel free to open more issues/PRs!