Skip to content

Update coin selection terminology #146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
afilini merged 3 commits into from
Oct 29, 2020
Merged

Update coin selection terminology #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43a51a1
Rename must_use_utxos to required_utxos
murchandamus Oct 26, 2020
84aee3b
Rename may_use_utxos to optional_uxtos
murchandamus Oct 26, 2020
457e70e
Rename get_must_may_use_utxos to preselect_utxos
murchandamus Oct 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions src/wallet/coin_selection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,16 @@
//! fn coin_select(
//! &self,
//! database: &D,
//! must_use_utxos: Vec<(UTXO, usize)>,
//! may_use_utxos: Vec<(UTXO, usize)>,
//! required_utxos: Vec<(UTXO, usize)>,
//! optional_utxos: Vec<(UTXO, usize)>,
//! fee_rate: FeeRate,
//! amount_needed: u64,
//! fee_amount: f32,
//! ) -> Result<CoinSelectionResult, bdk::Error> {
//! let mut selected_amount = 0;
//! let mut additional_weight = 0;
//! let all_utxos_selected = must_use_utxos
//! .into_iter().chain(may_use_utxos)
//! let all_utxos_selected = required_utxos
//! .into_iter().chain(optional_utxos)
//! .scan((&mut selected_amount, &mut additional_weight), |(selected_amount, additional_weight), (utxo, weight)| {
//! let txin = TxIn {
//! previous_output: utxo.outpoint,
Expand Down Expand Up @@ -139,19 +139,19 @@ pub trait CoinSelectionAlgorithm<D: Database>: std::fmt::Debug {
///
/// - `database`: a reference to the wallet's database that can be used to lookup additional
/// details for a specific UTXO
/// - `must_use_utxos`: the utxos that must be spent regardless of `amount_needed` with their
/// - `required_utxos`: the utxos that must be spent regardless of `amount_needed` with their
/// weight cost
/// - `optional_utxos`: the remaining available utxos to satisfy `amount_needed` with their
/// weight cost
/// - `may_be_spent`: the utxos that may be spent to satisfy `amount_needed` with their weight
/// cost
/// - `fee_rate`: fee rate to use
/// - `amount_needed`: the amount in satoshi to select
/// - `fee_amount`: the amount of fees in satoshi already accumulated from adding outputs and
/// the transaction's header
fn coin_select(
&self,
database: &D,
must_use_utxos: Vec<(UTXO, usize)>,
may_use_utxos: Vec<(UTXO, usize)>,
required_utxos: Vec<(UTXO, usize)>,
optional_utxos: Vec<(UTXO, usize)>,
fee_rate: FeeRate,
amount_needed: u64,
fee_amount: f32,
Expand All @@ -169,8 +169,8 @@ impl<D: Database> CoinSelectionAlgorithm<D> for DumbCoinSelection {
fn coin_select(
&self,
_database: &D,
must_use_utxos: Vec<(UTXO, usize)>,
mut may_use_utxos: Vec<(UTXO, usize)>,
required_utxos: Vec<(UTXO, usize)>,
mut optional_utxos: Vec<(UTXO, usize)>,
fee_rate: FeeRate,
amount_needed: u64,
mut fee_amount: f32,
Expand All @@ -184,14 +184,14 @@ impl<D: Database> CoinSelectionAlgorithm<D> for DumbCoinSelection {
fee_rate
);

// We put the "must_use" UTXOs first and make sure the "may_use" are sorted, initially
// smallest to largest, before being reversed with `.rev()`.
// We put the "required UTXOs" first and make sure the optional UTXOs are sorted,
// initially smallest to largest, before being reversed with `.rev()`.
let utxos = {
may_use_utxos.sort_unstable_by_key(|(utxo, _)| utxo.txout.value);
Copy link
Copy Markdown
Collaborator

@LLFourn LLFourn Oct 27, 2020

Choose a reason for hiding this comment

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

change the comment above as well.

Copy link
Copy Markdown
Contributor Author

@murchandamus murchandamus Oct 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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

must_use_utxos
optional_utxos.sort_unstable_by_key(|(utxo, _)| utxo.txout.value);
required_utxos
.into_iter()
.map(|utxo| (true, utxo))
.chain(may_use_utxos.into_iter().rev().map(|utxo| (false, utxo)))
.chain(optional_utxos.into_iter().rev().map(|utxo| (false, utxo)))
};

// Keep including inputs until we've got enough.
Expand Down
16 changes: 8 additions & 8 deletions src/wallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ where
));
}

let (must_use_utxos, may_use_utxos) = self.get_must_may_use_utxos(
let (required_utxos, optional_utxos) = self.preselect_utxos(
builder.change_policy,
&builder.unspendable,
&builder.utxos,
Expand All @@ -363,8 +363,8 @@ where
mut fee_amount,
} = builder.coin_selection.coin_select(
self.database.borrow().deref(),
must_use_utxos,
may_use_utxos,
required_utxos,
optional_utxos,
fee_rate,
outgoing,
fee_amount,
Expand Down Expand Up @@ -604,7 +604,7 @@ where
.cloned()
.collect::<Vec<_>>();

let (mut must_use_utxos, may_use_utxos) = self.get_must_may_use_utxos(
let (mut required_utxos, optional_utxos) = self.preselect_utxos(
builder.change_policy,
&builder.unspendable,
&builder_extra_utxos[..],
Expand All @@ -613,7 +613,7 @@ where
true, // we only want confirmed transactions for RBF
)?;

must_use_utxos.append(&mut original_utxos);
required_utxos.append(&mut original_utxos);

let amount_needed = tx.output.iter().fold(0, |acc, out| acc + out.value);
let (new_feerate, initial_fee) = match builder
Expand Down Expand Up @@ -645,8 +645,8 @@ where
fee_amount,
} = builder.coin_selection.coin_select(
self.database.borrow().deref(),
must_use_utxos,
may_use_utxos,
required_utxos,
optional_utxos,
new_feerate,
amount_needed,
initial_fee,
Expand Down Expand Up @@ -985,7 +985,7 @@ where
/// Given the options returns the list of utxos that must be used to form the
/// transaction and any further that may be used if needed.
#[allow(clippy::type_complexity)]
fn get_must_may_use_utxos(
fn preselect_utxos(
&self,
change_policy: tx_builder::ChangeSpendPolicy,
unspendable: &HashSet<OutPoint>,
Expand Down