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

Add Wallet list_unspent method #158

Merged
merged 4 commits into from
Jul 12, 2022
Merged
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- APIs Added
- `TxBuilder.add_data(data: Vec<u8>)`
- `Wallet.list_unspent()` returns `Vec<LocalUtxo>`

## [v0.7.0]

- Update BDK to version 0.19.0
Expand Down
25 changes: 25 additions & 0 deletions src/bdk.udl
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,28 @@ callback interface Progress {
void update(f32 progress, string? message);
};

dictionary OutPoint {
string txid;
u32 vout;
};

dictionary TxOut {
u64 value;
string address;
};

enum KeychainKind {
"External",
"Internal",
};

dictionary LocalUtxo {
OutPoint outpoint;
TxOut txout;
KeychainKind keychain;
boolean is_spent;
};

interface Wallet {
[Throws=BdkError]
constructor(string descriptor, string? change_descriptor, Network network, DatabaseConfig database_config);
Expand All @@ -169,6 +191,9 @@ interface Wallet {

[Throws=BdkError]
void sync([ByRef] Blockchain blockchain, Progress? progress);

[Throws=BdkError]
sequence<LocalUtxo> list_unspent();
};

interface PartiallySignedBitcoinTransaction {
Expand Down
56 changes: 55 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ use bdk::miniscript::BareCtx;
use bdk::wallet::AddressIndex as BdkAddressIndex;
use bdk::wallet::AddressInfo as BdkAddressInfo;
use bdk::{
BlockTime, Error, FeeRate, SignOptions, SyncOptions as BdkSyncOptions, Wallet as BdkWallet,
BlockTime, Error, FeeRate, KeychainKind, SignOptions, SyncOptions as BdkSyncOptions,
Wallet as BdkWallet,
};
use std::convert::{From, TryFrom};
use std::fmt;
Expand Down Expand Up @@ -172,6 +173,51 @@ struct Wallet {
wallet_mutex: Mutex<BdkWallet<AnyDatabase>>,
}

pub struct OutPoint {
txid: String,
vout: u32,
}

pub struct TxOut {
value: u64,
address: String,
Copy link
Member

Choose a reason for hiding this comment

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

At first glance it looks good to me, but it's still something I want to bring to the attention of other reviewers (@notmandatory, @artfuldev). The TxOut in BDK contains a Script instead of an address of type String. I don't think we need the Script at the moment, but it does change the shape of the API, so something to note.

}

pub struct LocalUtxo {
Copy link
Member

@thunderbiscuit thunderbiscuit Jun 17, 2022

Choose a reason for hiding this comment

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

The BDK version of this struct has a boolean is_spent. I assume you didn't include it here because LocalUtxo will only be returned from the list_unspent() method, which means that boolean will only ever been set to false. In this case, however, I think the naming could lead to confusion if some developers are familiar with the Rust version of BDK.

I propose we either:

  1. Rename the struct UnspentLocalUtxo to make explicit the fact that it is unspent, or
  2. Add the is_spent boolean to it, which for now will only ever be false

I suggest we go with (2) because to me that feels more forward-compatible. If ever we implement other features that use the LocalUtxo struct we'll have it in its entirety already. I also feel like it might allow you to simplify some of the mapping between the LocalUtxo struct you have here and the one defined in BDK. Finally, we've been discussing how we want to try and keep the API in the bindings as close as we can to the BDK API. It's not always possible, but in this case I think it is.

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 also feel that (2) is a better choice here so just added the field

outpoint: OutPoint,
txout: TxOut,
keychain: KeychainKind,
is_spent: bool,
}

// This trait is used to convert the bdk TxOut type with field `script_pubkey: Script`
// into the bdk-ffi TxOut type which has a field `address: String` instead
trait NetworkLocalUtxo {
fn from_utxo(x: &bdk::LocalUtxo, network: Network) -> LocalUtxo;
}

impl NetworkLocalUtxo for LocalUtxo {
fn from_utxo(x: &bdk::LocalUtxo, network: Network) -> LocalUtxo {
LocalUtxo {
outpoint: OutPoint {
txid: x.outpoint.txid.to_string(),
vout: x.outpoint.vout,
},
txout: TxOut {
value: x.txout.value,
address: bdk::bitcoin::util::address::Address::from_script(
&x.txout.script_pubkey,
network,
)
.unwrap()
.to_string(),
},
keychain: x.keychain,
is_spent: x.is_spent,
}
}
}

pub trait Progress: Send + Sync + 'static {
fn update(&self, progress: f32, message: Option<String>);
}
Expand Down Expand Up @@ -283,6 +329,14 @@ impl Wallet {
let transactions = self.get_wallet().list_transactions(true)?;
Ok(transactions.iter().map(Transaction::from).collect())
}

fn list_unspent(&self) -> Result<Vec<LocalUtxo>, Error> {
let unspents = self.get_wallet().list_unspent()?;
Ok(unspents
.iter()
.map(|u| LocalUtxo::from_utxo(u, self.get_network()))
.collect())
}
}

pub struct ExtendedKeyInfo {
Expand Down