Skip to content

Commit

Permalink
Merge #1411: feat: update keychain::Balance to use bitcoin::Amount
Browse files Browse the repository at this point in the history
22aa534 feat: use `Amount` on `TxBuilder::add_recipient` (Leonardo Lima)
d5c0e72 feat: use `Amount` on `spk_txout_index` and related (Leonardo Lima)
8a33d98 feat: update `wallet::Balance` to use `bitcoin::Amount` (Leonardo Lima)

Pull request description:

  fixes #823

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  It's being used on `Balance`, and throughout the code, an `u64` represents the amount, which relies on the user to infer its sats, not millisats, or any other representation.

  It updates the usage of `u64` on `Balance`, and other APIs:
  - `TxParams::add_recipient`
  - `KeyChainTxOutIndex::sent_and_received`, `KeyChainTxOutIndex::net_value`
  -  `SpkTxOutIndex::sent_and_received`, `SpkTxOutIndex::net_value`

  <!-- Describe the purpose of this PR, what's being adding and/or fixed -->

  ### Notes to the reviewers

  <!-- In this section you can include notes directed to the reviewers, like explaining why some parts
  of the PR were done in a specific way -->

  It updates some of the APIs to expect the `bitcoin::Amount`, but it does not update internal usage of u64, such as `TxParams` still expects and uses `u64`, please see the PR comments for related discussion.

  ### Changelog notice

  <!-- Notice the release manager should include in the release tag message changelog -->
  <!-- See https://keepachangelog.com/en/1.0.0/ for examples -->

  - Changed the `keychain::Balance` struct fields to use `Amount` instead of `u64`.
  - Changed the `add_recipient` method on `TxBuilder` implementation to expect `bitcoin::Amount`.
  - Changed the `sent_and_received`, and `net_value` methods on `KeyChainTxOutIndex` to expect `bitcoin::Amount`.
  - Changed the `sent_and_received`, and `net_value` methods on `SpkTxOutIndex` to expect `bitcoin::Amount`.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [x] I've added tests for the new feature
  * [x] I've added docs for the new feature

  #### Bugfixes:

  * [x] This pull request breaks the existing API
  * [ ] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  evanlinjin:
    ACK 22aa534

Tree-SHA512: c4e8198d96c0d66cc3d2e4149e8a56bb7565b9cd49ff42113eaebd24b1d7bfeecd7124db0b06524b78b8891ee1bde1546705b80afad408f48495cf3c02446d02
  • Loading branch information
evanlinjin committed May 6, 2024
2 parents 23538c4 + 22aa534 commit dcd2d47
Show file tree
Hide file tree
Showing 18 changed files with 333 additions and 270 deletions.
2 changes: 1 addition & 1 deletion crates/bdk/src/wallet/coin_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
//! .unwrap();
//! let psbt = {
//! let mut builder = wallet.build_tx().coin_selection(AlwaysSpendEverything);
//! builder.add_recipient(to_address.script_pubkey(), 50_000);
//! builder.add_recipient(to_address.script_pubkey(), Amount::from_sat(50_000));
//! builder.finish()?
//! };
//!
Expand Down
16 changes: 8 additions & 8 deletions crates/bdk/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ use bdk_chain::{
IndexedTxGraph,
};
use bdk_persist::{Persist, PersistBackend};
use bitcoin::constants::genesis_block;
use bitcoin::secp256k1::{All, Secp256k1};
use bitcoin::sighash::{EcdsaSighashType, TapSighashType};
use bitcoin::{
absolute, psbt, Address, Block, FeeRate, Network, OutPoint, Script, ScriptBuf, Sequence,
Transaction, TxOut, Txid, Witness,
};
use bitcoin::{consensus::encode::serialize, transaction, Amount, BlockHash, Psbt};
use bitcoin::{consensus::encode::serialize, transaction, BlockHash, Psbt};
use bitcoin::{constants::genesis_block, Amount};
use core::fmt;
use core::ops::Deref;
use descriptor::error::Error as DescriptorError;
Expand Down Expand Up @@ -950,10 +950,10 @@ impl Wallet {
/// [`insert_txout`]: Self::insert_txout
pub fn calculate_fee_rate(&self, tx: &Transaction) -> Result<FeeRate, CalculateFeeError> {
self.calculate_fee(tx)
.map(|fee| bitcoin::Amount::from_sat(fee) / tx.weight())
.map(|fee| Amount::from_sat(fee) / tx.weight())
}

/// Compute the `tx`'s sent and received amounts (in satoshis).
/// Compute the `tx`'s sent and received [`Amount`]s.
///
/// This method returns a tuple `(sent, received)`. Sent is the sum of the txin amounts
/// that spend from previous txouts tracked by this wallet. Received is the summation
Expand All @@ -978,7 +978,7 @@ impl Wallet {
/// let tx = &psbt.clone().extract_tx().expect("tx");
/// let (sent, received) = wallet.sent_and_received(tx);
/// ```
pub fn sent_and_received(&self, tx: &Transaction) -> (u64, u64) {
pub fn sent_and_received(&self, tx: &Transaction) -> (Amount, Amount) {
self.indexed_graph.index.sent_and_received(tx, ..)
}

Expand Down Expand Up @@ -1197,7 +1197,7 @@ impl Wallet {
/// let psbt = {
/// let mut builder = wallet.build_tx();
/// builder
/// .add_recipient(to_address.script_pubkey(), 50_000);
/// .add_recipient(to_address.script_pubkey(), Amount::from_sat(50_000));
/// builder.finish()?
/// };
///
Expand Down Expand Up @@ -1579,7 +1579,7 @@ impl Wallet {
/// let mut psbt = {
/// let mut builder = wallet.build_tx();
/// builder
/// .add_recipient(to_address.script_pubkey(), 50_000)
/// .add_recipient(to_address.script_pubkey(), Amount::from_sat(50_000))
/// .enable_rbf();
/// builder.finish()?
/// };
Expand Down Expand Up @@ -1752,7 +1752,7 @@ impl Wallet {
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
/// let mut psbt = {
/// let mut builder = wallet.build_tx();
/// builder.add_recipient(to_address.script_pubkey(), 50_000);
/// builder.add_recipient(to_address.script_pubkey(), Amount::from_sat(50_000));
/// builder.finish()?
/// };
/// let finalized = wallet.sign(&mut psbt, SignOptions::default())?;
Expand Down
27 changes: 16 additions & 11 deletions crates/bdk/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
//!
//! tx_builder
//! // Create a transaction with one output to `to_address` of 50_000 satoshi
//! .add_recipient(to_address.script_pubkey(), 50_000)
//! .add_recipient(to_address.script_pubkey(), Amount::from_sat(50_000))
//! // With a custom fee rate of 5.0 satoshi/vbyte
//! .fee_rate(FeeRate::from_sat_per_vb(5).expect("valid feerate"))
//! // Only spend non-change outputs
Expand All @@ -47,7 +47,7 @@ use core::marker::PhantomData;

use bitcoin::psbt::{self, Psbt};
use bitcoin::script::PushBytes;
use bitcoin::{absolute, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid};
use bitcoin::{absolute, Amount, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid};

use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
use super::{CreateTxError, Wallet};
Expand Down Expand Up @@ -94,8 +94,8 @@ impl TxBuilderContext for BumpFee {}
/// let mut builder = wallet.build_tx();
/// builder
/// .ordering(TxOrdering::Untouched)
/// .add_recipient(addr1.script_pubkey(), 50_000)
/// .add_recipient(addr2.script_pubkey(), 50_000);
/// .add_recipient(addr1.script_pubkey(), Amount::from_sat(50_000))
/// .add_recipient(addr2.script_pubkey(), Amount::from_sat(50_000));
/// builder.finish()?
/// };
///
Expand All @@ -104,7 +104,7 @@ impl TxBuilderContext for BumpFee {}
/// let mut builder = wallet.build_tx();
/// builder.ordering(TxOrdering::Untouched);
/// for addr in &[addr1, addr2] {
/// builder.add_recipient(addr.script_pubkey(), 50_000);
/// builder.add_recipient(addr.script_pubkey(), Amount::from_sat(50_000));
/// }
/// builder.finish()?
/// };
Expand Down Expand Up @@ -274,7 +274,7 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
///
/// let builder = wallet
/// .build_tx()
/// .add_recipient(to_address.script_pubkey(), 50_000)
/// .add_recipient(to_address.script_pubkey(), Amount::from_sat(50_000))
/// .policy_path(path, KeychainKind::External);
///
/// # Ok::<(), anyhow::Error>(())
Expand Down Expand Up @@ -713,21 +713,26 @@ impl std::error::Error for AllowShrinkingError {}

impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
/// Replace the recipients already added with a new list
pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, u64)>) -> &mut Self {
self.params.recipients = recipients;
pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, Amount)>) -> &mut Self {
self.params.recipients = recipients
.into_iter()
.map(|(script, amount)| (script, amount.to_sat()))
.collect();
self
}

/// Add a recipient to the internal list
pub fn add_recipient(&mut self, script_pubkey: ScriptBuf, amount: u64) -> &mut Self {
self.params.recipients.push((script_pubkey, amount));
pub fn add_recipient(&mut self, script_pubkey: ScriptBuf, amount: Amount) -> &mut Self {
self.params
.recipients
.push((script_pubkey, amount.to_sat()));
self
}

/// Add data as an output, using OP_RETURN
pub fn add_data<T: AsRef<PushBytes>>(&mut self, data: &T) -> &mut Self {
let script = ScriptBuf::new_op_return(data);
self.add_recipient(script, 0u64);
self.add_recipient(script, Amount::ZERO);
self
}

Expand Down
10 changes: 5 additions & 5 deletions crates/bdk/tests/psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn test_psbt_malformed_psbt_input_legacy() {
let (mut wallet, _) = get_funded_wallet(get_test_wpkh());
let send_to = wallet.peek_address(KeychainKind::External, 0);
let mut builder = wallet.build_tx();
builder.add_recipient(send_to.script_pubkey(), 10_000);
builder.add_recipient(send_to.script_pubkey(), Amount::from_sat(10_000));
let mut psbt = builder.finish().unwrap();
psbt.inputs.push(psbt_bip.inputs[0].clone());
let options = SignOptions {
Expand All @@ -31,7 +31,7 @@ fn test_psbt_malformed_psbt_input_segwit() {
let (mut wallet, _) = get_funded_wallet(get_test_wpkh());
let send_to = wallet.peek_address(KeychainKind::External, 0);
let mut builder = wallet.build_tx();
builder.add_recipient(send_to.script_pubkey(), 10_000);
builder.add_recipient(send_to.script_pubkey(), Amount::from_sat(10_000));
let mut psbt = builder.finish().unwrap();
psbt.inputs.push(psbt_bip.inputs[1].clone());
let options = SignOptions {
Expand All @@ -47,7 +47,7 @@ fn test_psbt_malformed_tx_input() {
let (mut wallet, _) = get_funded_wallet(get_test_wpkh());
let send_to = wallet.peek_address(KeychainKind::External, 0);
let mut builder = wallet.build_tx();
builder.add_recipient(send_to.script_pubkey(), 10_000);
builder.add_recipient(send_to.script_pubkey(), Amount::from_sat(10_000));
let mut psbt = builder.finish().unwrap();
psbt.unsigned_tx.input.push(TxIn::default());
let options = SignOptions {
Expand All @@ -63,7 +63,7 @@ fn test_psbt_sign_with_finalized() {
let (mut wallet, _) = get_funded_wallet(get_test_wpkh());
let send_to = wallet.peek_address(KeychainKind::External, 0);
let mut builder = wallet.build_tx();
builder.add_recipient(send_to.script_pubkey(), 10_000);
builder.add_recipient(send_to.script_pubkey(), Amount::from_sat(10_000));
let mut psbt = builder.finish().unwrap();

// add a finalized input
Expand Down Expand Up @@ -201,7 +201,7 @@ fn test_psbt_multiple_internalkey_signers() {
// the prevout we're spending
let prevouts = &[TxOut {
script_pubkey: send_to.script_pubkey(),
value: Amount::from_sat(to_spend),
value: to_spend,
}];
let prevouts = Prevouts::All(prevouts);
let input_index = 0;
Expand Down
Loading

0 comments on commit dcd2d47

Please sign in to comment.