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

bdk/src/wallet/tx_builder.rs

Lines 742 to 751 in dd51380

	/// Policy regarding the use of change outputs when creating a transaction
	#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
	pub enum ChangeSpendPolicy {
	/// Use both change and non-change outputs (default)
	ChangeAllowed,
	/// Only use change outputs (see [`TxBuilder::only_spend_change`])
	OnlyChange,
	/// Only use non-change outputs (see [`TxBuilder::do_not_spend_change`])
	ChangeForbidden,
	}

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.

bdk/src/wallet/tx_builder.rs

Line 145 in dd51380

pub(crate) change_policy: ChangeSpendPolicy,

Similarly, this field should be called spend_internal_policy.

LocalUtxo::is_spent -> LocalUtxo::is_used

bdk/src/types.rs

Lines 134 to 135 in dd51380

	/// Whether this UTXO is spent or not
	pub is_spent: bool,

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.