Skip to content
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.

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

Implement utxo reservation #913

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 26 additions & 12 deletions crates/bdk/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,9 @@ impl<D> Wallet<D> {
}

/// Return the list of unspent outputs of this wallet
pub fn list_unspent(&self) -> Vec<LocalUtxo> {
pub fn list_unspent(&self, include_reserved_utxos: bool) -> Vec<LocalUtxo> {
self.keychain_tracker
.full_utxos()
.full_utxos(include_reserved_utxos)
.map(|(&(keychain, derivation_index), utxo)| LocalUtxo {
outpoint: utxo.outpoint,
txout: utxo.txout,
Expand Down Expand Up @@ -388,9 +388,9 @@ impl<D> Wallet<D> {

/// Returns the utxo owned by this wallet corresponding to `outpoint` if it exists in the
/// wallet's database.
pub fn get_utxo(&self, op: OutPoint) -> Option<LocalUtxo> {
pub fn get_utxo(&self, op: OutPoint, include_reserved_utxos: bool) -> Option<LocalUtxo> {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should have a include_reserved_utxos parameter here... If I want bdk to retrieve a specific outpoint, I usually don't care if it's reserved or not.

I would just call full_utxos(true) a couple of lines below and that's it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense.

self.keychain_tracker
.full_utxos()
.full_utxos(include_reserved_utxos)
.find_map(|(&(keychain, derivation_index), txo)| {
if op == txo.outpoint {
Some(LocalUtxo {
Expand Down Expand Up @@ -527,11 +527,12 @@ impl<D> Wallet<D> {

/// Return the balance, separated into available, trusted-pending, untrusted-pending and immature
/// values.
pub fn get_balance(&self) -> Balance {
self.keychain_tracker.balance(|keychain| match keychain {
KeychainKind::External => false,
KeychainKind::Internal => true,
})
pub fn get_balance(&self, include_reserved_utxos: bool) -> Balance {
Copy link
Member

Choose a reason for hiding this comment

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

Similarly to above, when users ask for a balance I suppose they always want the reserved utxos anyways

self.keychain_tracker
.balance(include_reserved_utxos, |keychain| match keychain {
KeychainKind::External => false,
KeychainKind::Internal => true,
})
}

/// Add an external signer
Expand Down Expand Up @@ -866,6 +867,7 @@ impl<D> Wallet<D> {
params.drain_wallet,
params.manually_selected_only,
params.bumping_fee.is_some(), // we mandate confirmed transactions if we're bumping the fee
params.include_reserved_utxos,
current_height.map(LockTime::to_consensus_u32),
);

Expand Down Expand Up @@ -909,6 +911,10 @@ impl<D> Wallet<D> {
})
.collect();

coin_selection.selected.iter().for_each(|u| {
let _ = self.keychain_tracker.mark_reserved(u.outpoint());
});

if tx.output.is_empty() {
// Uh oh, our transaction has no outputs.
// We allow this when:
Expand Down Expand Up @@ -1370,6 +1376,13 @@ impl<D> Wallet<D> {
txout_index.unmark_used(&keychain, index);
}
}

// release all the UTXOs that were reserved by this transaction
tx.input.iter().for_each(|input| {
let _ = self
.keychain_tracker
.unmark_reserved(&input.previous_output);
});
}

fn map_keychain(&self, keychain: KeychainKind) -> KeychainKind {
Expand All @@ -1391,8 +1404,8 @@ impl<D> Wallet<D> {
Some(descriptor.at_derivation_index(child))
}

fn get_available_utxos(&self) -> Vec<(LocalUtxo, usize)> {
self.list_unspent()
fn get_available_utxos(&self, include_reserved_utxos: bool) -> Vec<(LocalUtxo, usize)> {
self.list_unspent(include_reserved_utxos)
.into_iter()
.map(|utxo| {
let keychain = utxo.keychain;
Expand All @@ -1417,11 +1430,12 @@ impl<D> Wallet<D> {
must_use_all_available: bool,
manual_only: bool,
must_only_use_confirmed_tx: bool,
include_reserved_utxos: bool,
current_height: Option<u32>,
) -> (Vec<WeightedUtxo>, Vec<WeightedUtxo>) {
// must_spend <- manually selected utxos
// may_spend <- all other available utxos
let mut may_spend = self.get_available_utxos();
let mut may_spend = self.get_available_utxos(include_reserved_utxos);

may_spend.retain(|may_spend| {
!manually_selected
Expand Down
29 changes: 25 additions & 4 deletions crates/bdk/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ impl TxBuilderContext for BumpFee {}
/// // non-chaining
/// let (psbt2, details) = {
/// let mut builder = wallet.build_tx();
/// builder.include_reserved_utxos(true);
/// builder.ordering(TxOrdering::Untouched);
/// for addr in &[addr1, addr2] {
/// builder.add_recipient(addr.script_pubkey(), 50_000);
Expand Down Expand Up @@ -150,6 +151,7 @@ pub(crate) struct TxParams {
pub(crate) bumping_fee: Option<PreviousFee>,
pub(crate) current_height: Option<LockTime>,
pub(crate) allow_dust: bool,
pub(crate) include_reserved_utxos: bool,
}

#[derive(Clone, Copy, Debug)]
Expand Down Expand Up @@ -274,12 +276,20 @@ impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D,
///
/// These have priority over the "unspendable" utxos, meaning that if a utxo is present both in
/// the "utxos" and the "unspendable" list, it will be spent.
pub fn add_utxos(&mut self, outpoints: &[OutPoint]) -> Result<&mut Self, Error> {
pub fn add_utxos(
Copy link
Member

Choose a reason for hiding this comment

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

Same here, if you want to manually add utxos you probably don't care whether they're unlocked or not.

Unless... we might want to return a specific error (not UnknownUtxo, a more detailed one, like LockedUtxo) if you try to add a locked utxo to a txbuilder that doesn't have include_reserved_utxos. I'm not sure exactly what's the way forward here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will prefer creating an error like LockedUtxo. Make more sense to me.

&mut self,
outpoints: &[OutPoint],
include_reserved_utxos: bool,
) -> Result<&mut Self, Error> {
{
let wallet = self.wallet.borrow();
let utxos = outpoints
.iter()
.map(|outpoint| wallet.get_utxo(*outpoint).ok_or(Error::UnknownUtxo))
.map(|outpoint| {
wallet
.get_utxo(*outpoint, include_reserved_utxos)
.ok_or(Error::UnknownUtxo)
})
.collect::<Result<Vec<_>, _>>()?;

for utxo in utxos {
Expand All @@ -299,8 +309,12 @@ impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D,
///
/// These have priority over the "unspendable" utxos, meaning that if a utxo is present both in
/// the "utxos" and the "unspendable" list, it will be spent.
pub fn add_utxo(&mut self, outpoint: OutPoint) -> Result<&mut Self, Error> {
self.add_utxos(&[outpoint])
pub fn add_utxo(
&mut self,
outpoint: OutPoint,
include_reserved_utxos: bool,
) -> Result<&mut Self, Error> {
self.add_utxos(&[outpoint], include_reserved_utxos)
}

/// Add a foreign UTXO i.e. a UTXO not owned by this wallet.
Expand Down Expand Up @@ -576,6 +590,13 @@ impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D,
self.params.allow_dust = allow_dust;
self
}

/// Specifies whether UTXOs used by previously built but not yet broadcasted
/// transactions can be used in the current transaction building process
pub fn include_reserved_utxos(&mut self, include_reserved_utxos: bool) -> &mut Self {
self.params.include_reserved_utxos = include_reserved_utxos;
self
}
}

impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
Expand Down
Loading