Better names for types

[evanlinjin]

Description

After going through the codebase of the wallet module, I felt like we could do with some improvements in naming. I will list some suggestions below.

ChangeSpendPolicy -> InternalSpendPolicy

https://github.com/bitcoindevkit/bdk/blob/dd5138052006737b9d8450d9f2d743c5400342d4/src/wallet/tx_builder.rs#L742-L751

Right now we do not keep track of which outputs are change/not-change in the database. We only differentiate based on whether the scriptPubKey is derived from the external descriptor or internal descriptor.

https://github.com/bitcoindevkit/bdk/blob/dd5138052006737b9d8450d9f2d743c5400342d4/src/wallet/tx_builder.rs#L145

Similarly, this field should be called spend_internal_policy.

LocalUtxo::is_spent -> LocalUtxo::is_used

https://github.com/bitcoindevkit/bdk/blob/dd5138052006737b9d8450d9f2d743c5400342d4/src/types.rs#L134-L135

Considering that UTXO is an acronym for "Unspent Transaction Output", this naming is very confusing. "Spent" for me means that the transaction is already included in a mined block. By calling it is_used, we will know whether it has already been used as input during tx creation.

[notmandatory]

Thanks for writing this up! I re-tagged it for discussion since there's nothing broken you just want to discuss some naming improvements.

My two cents on the naming, your suggestions sound fine to me, but we need to be careful not to change names that break a public API unless there is compelling reason to do it. Don't forget some users maybe forced to change their code for only a small readability gain.

[danielabrozzoni]

LocalUtxo::is_spent -> LocalUtxo::is_used Considering that UTXO is an acronym for "Unspent Transaction Output", this naming is very confusing. "Spent" for me means that the transaction is already included in a mined block. By calling it is_used, we will know whether it has already been used as input during tx creation.

Mh, I don't really like is_used: saying that an "unspent tx output" is spent is for sure confusing, but some LocalUtxo in our database are effectively spent, not just "used" (we never delete LocalUtxos, even if they've been spent thousands of blocks ago -> see #573). Instead, I think we should go for LocalUtxo -> LocalTxout/LocalTxOut/LocalTxo as @LLFourn / @afilini were discussing here

[notmandatory]

This sounds like something to tackle as part of the 1.0.0 API changes.

[notmandatory]

These look like small changes but I think we should move to the 2.0 milestone.

[notmandatory]

Already fixed.