Skip to content

Commit 5c7cc30

Browse files
committed
Merge #1468: feat: use Weight type instead of usize
438cd46 refactor(wallet)!: change WeightedUtxo to use Weight type (Leonardo Lima) Pull request description: fixes #1466 depends on #1448 <!-- You can erase any parts of this template not applicable to your Pull Request. --> ### Description This PR is a follow-up on top of #1448, and should be rebased and merged after it, it uses the rust-bitcoin `Weight` type instead of the current `usize` usage. NOTE: ~~It also adds a new `MiniscriptError` variant, and remove the `.unwrap()` usage.~~ As suggested I'll address this on another issue #1485, trying to reproduce the error first. <!-- Describe the purpose of this PR, what's being adding and/or fixed --> ### Notes to the reviewers It should be ready to review after #1448 gets merged. <!-- 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 --> ### Changelog notice - Change `WeightedUtxo` `satisfaction_weight` has been changed to use `Weight` type. - Change `TxBuilder` methods `add_foreign_utxo` and `add_foreign_utxo_with_sequence` to expect `Weight` as `satisfaction_weight` type. <!-- Notice the release manager should include in the release tag message changelog --> <!-- See https://keepachangelog.com/en/1.0.0/ for examples --> ### 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 ACKs for top commit: storopoli: Anyways, ACK 438cd46 notmandatory: ACK 438cd46 Tree-SHA512: 1998fe659833da890ce07aa746572ae24d035e636732c1a11b7828ffed48e753adb4108f42d00b7cd05e6f45831a7a9840faa26f94058fc13760497837af002f
2 parents 275e069 + 438cd46 commit 5c7cc30

File tree

5 files changed

+43
-68
lines changed

5 files changed

+43
-68
lines changed

crates/wallet/src/types.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use core::convert::AsRef;
1414

1515
use bdk_chain::ConfirmationTime;
1616
use bitcoin::blockdata::transaction::{OutPoint, Sequence, TxOut};
17-
use bitcoin::psbt;
17+
use bitcoin::{psbt, Weight};
1818

1919
use serde::{Deserialize, Serialize};
2020

@@ -72,7 +72,7 @@ pub struct WeightedUtxo {
7272
/// properly maintain the feerate when adding this input to a transaction during coin selection.
7373
///
7474
/// [weight units]: https://en.bitcoin.it/wiki/Weight_units
75-
pub satisfaction_weight: usize,
75+
pub satisfaction_weight: Weight,
7676
/// The UTXO
7777
pub utxo: Utxo,
7878
}

crates/wallet/src/wallet/coin_selection.rs

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,10 @@
5252
//! (&mut selected_amount, &mut additional_weight),
5353
//! |(selected_amount, additional_weight), weighted_utxo| {
5454
//! **selected_amount += weighted_utxo.utxo.txout().value.to_sat();
55-
//! **additional_weight += Weight::from_wu(
56-
//! (TxIn::default().segwit_weight().to_wu()
57-
//! + weighted_utxo.satisfaction_weight as u64)
58-
//! as u64,
59-
//! );
55+
//! **additional_weight += TxIn::default()
56+
//! .segwit_weight()
57+
//! .checked_add(weighted_utxo.satisfaction_weight)
58+
//! .expect("`Weight` addition should not cause an integer overflow");
6059
//! Some(weighted_utxo.utxo)
6160
//! },
6261
//! )
@@ -344,10 +343,10 @@ fn select_sorted_utxos(
344343
|(selected_amount, fee_amount), (must_use, weighted_utxo)| {
345344
if must_use || **selected_amount < target_amount + **fee_amount {
346345
**fee_amount += (fee_rate
347-
* Weight::from_wu(
348-
TxIn::default().segwit_weight().to_wu()
349-
+ weighted_utxo.satisfaction_weight as u64,
350-
))
346+
* (TxIn::default()
347+
.segwit_weight()
348+
.checked_add(weighted_utxo.satisfaction_weight)
349+
.expect("`Weight` addition should not cause an integer overflow")))
351350
.to_sat();
352351
**selected_amount += weighted_utxo.utxo.txout().value.to_sat();
353352
Some(weighted_utxo.utxo)
@@ -390,9 +389,10 @@ struct OutputGroup {
390389
impl OutputGroup {
391390
fn new(weighted_utxo: WeightedUtxo, fee_rate: FeeRate) -> Self {
392391
let fee = (fee_rate
393-
* Weight::from_wu(
394-
TxIn::default().segwit_weight().to_wu() + weighted_utxo.satisfaction_weight as u64,
395-
))
392+
* (TxIn::default()
393+
.segwit_weight()
394+
.checked_add(weighted_utxo.satisfaction_weight)
395+
.expect("`Weight` addition should not cause an integer overflow")))
396396
.to_sat();
397397
let effective_value = weighted_utxo.utxo.txout().value.to_sat() as i64 - fee as i64;
398398
OutputGroup {
@@ -774,7 +774,7 @@ mod test {
774774
))
775775
.unwrap();
776776
WeightedUtxo {
777-
satisfaction_weight: P2WPKH_SATISFACTION_SIZE,
777+
satisfaction_weight: Weight::from_wu_usize(P2WPKH_SATISFACTION_SIZE),
778778
utxo: Utxo::Local(LocalOutput {
779779
outpoint,
780780
txout: TxOut {
@@ -834,7 +834,7 @@ mod test {
834834
let mut res = Vec::new();
835835
for i in 0..utxos_number {
836836
res.push(WeightedUtxo {
837-
satisfaction_weight: P2WPKH_SATISFACTION_SIZE,
837+
satisfaction_weight: Weight::from_wu_usize(P2WPKH_SATISFACTION_SIZE),
838838
utxo: Utxo::Local(LocalOutput {
839839
outpoint: OutPoint::from_str(&format!(
840840
"ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:{}",
@@ -865,7 +865,7 @@ mod test {
865865
fn generate_same_value_utxos(utxos_value: u64, utxos_number: usize) -> Vec<WeightedUtxo> {
866866
(0..utxos_number)
867867
.map(|i| WeightedUtxo {
868-
satisfaction_weight: P2WPKH_SATISFACTION_SIZE,
868+
satisfaction_weight: Weight::from_wu_usize(P2WPKH_SATISFACTION_SIZE),
869869
utxo: Utxo::Local(LocalOutput {
870870
outpoint: OutPoint::from_str(&format!(
871871
"ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:{}",
@@ -1512,7 +1512,7 @@ mod test {
15121512
fn test_filter_duplicates() {
15131513
fn utxo(txid: &str, value: u64) -> WeightedUtxo {
15141514
WeightedUtxo {
1515-
satisfaction_weight: 0,
1515+
satisfaction_weight: Weight::ZERO,
15161516
utxo: Utxo::Local(LocalOutput {
15171517
outpoint: OutPoint::new(bitcoin::hashes::Hash::hash(txid.as_bytes()), 0),
15181518
txout: TxOut {

crates/wallet/src/wallet/mod.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,17 @@ use bdk_chain::{
3131
Append, BlockId, ChainPosition, ConfirmationTime, ConfirmationTimeHeightAnchor, FullTxOut,
3232
Indexed, IndexedTxGraph,
3333
};
34-
use bitcoin::secp256k1::{All, Secp256k1};
3534
use bitcoin::sighash::{EcdsaSighashType, TapSighashType};
3635
use bitcoin::{
3736
absolute, psbt, Address, Block, FeeRate, Network, OutPoint, Script, ScriptBuf, Sequence,
3837
Transaction, TxOut, Txid, Witness,
3938
};
4039
use bitcoin::{consensus::encode::serialize, transaction, BlockHash, Psbt};
4140
use bitcoin::{constants::genesis_block, Amount};
41+
use bitcoin::{
42+
secp256k1::{All, Secp256k1},
43+
Weight,
44+
};
4245
use core::fmt;
4346
use core::mem;
4447
use core::ops::Deref;
@@ -1662,8 +1665,7 @@ impl Wallet {
16621665
let satisfaction_weight = self
16631666
.get_descriptor_for_keychain(keychain)
16641667
.max_weight_to_satisfy()
1665-
.unwrap()
1666-
.to_wu() as usize;
1668+
.unwrap();
16671669
WeightedUtxo {
16681670
utxo: Utxo::Local(LocalOutput {
16691671
outpoint: txin.previous_output,
@@ -1677,8 +1679,9 @@ impl Wallet {
16771679
}
16781680
}
16791681
None => {
1680-
let satisfaction_weight =
1681-
serialize(&txin.script_sig).len() * 4 + serialize(&txin.witness).len();
1682+
let satisfaction_weight = Weight::from_wu_usize(
1683+
serialize(&txin.script_sig).len() * 4 + serialize(&txin.witness).len(),
1684+
);
16821685
WeightedUtxo {
16831686
utxo: Utxo::Foreign {
16841687
outpoint: txin.previous_output,
@@ -1987,15 +1990,14 @@ impl Wallet {
19871990
descriptor.at_derivation_index(child).ok()
19881991
}
19891992

1990-
fn get_available_utxos(&self) -> Vec<(LocalOutput, usize)> {
1993+
fn get_available_utxos(&self) -> Vec<(LocalOutput, Weight)> {
19911994
self.list_unspent()
19921995
.map(|utxo| {
19931996
let keychain = utxo.keychain;
19941997
(utxo, {
19951998
self.get_descriptor_for_keychain(keychain)
19961999
.max_weight_to_satisfy()
19972000
.unwrap()
1998-
.to_wu() as usize
19992001
})
20002002
})
20012003
.collect()

crates/wallet/src/wallet/tx_builder.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ use core::fmt;
4444

4545
use bitcoin::psbt::{self, Psbt};
4646
use bitcoin::script::PushBytes;
47-
use bitcoin::{absolute, Amount, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid};
47+
use bitcoin::{
48+
absolute, Amount, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid, Weight,
49+
};
4850
use rand_core::RngCore;
4951

5052
use super::coin_selection::CoinSelectionAlgorithm;
@@ -295,9 +297,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
295297

296298
for utxo in utxos {
297299
let descriptor = wallet.get_descriptor_for_keychain(utxo.keychain);
298-
299-
let satisfaction_weight =
300-
descriptor.max_weight_to_satisfy().unwrap().to_wu() as usize;
300+
let satisfaction_weight = descriptor.max_weight_to_satisfy().unwrap();
301301
self.params.utxos.push(WeightedUtxo {
302302
satisfaction_weight,
303303
utxo: Utxo::Local(utxo),
@@ -366,7 +366,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
366366
&mut self,
367367
outpoint: OutPoint,
368368
psbt_input: psbt::Input,
369-
satisfaction_weight: usize,
369+
satisfaction_weight: Weight,
370370
) -> Result<&mut Self, AddForeignUtxoError> {
371371
self.add_foreign_utxo_with_sequence(
372372
outpoint,
@@ -381,7 +381,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
381381
&mut self,
382382
outpoint: OutPoint,
383383
psbt_input: psbt::Input,
384-
satisfaction_weight: usize,
384+
satisfaction_weight: Weight,
385385
sequence: Sequence,
386386
) -> Result<&mut Self, AddForeignUtxoError> {
387387
if psbt_input.witness_utxo.is_none() {

crates/wallet/tests/wallet.rs

Lines changed: 10 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,11 +1387,7 @@ fn test_add_foreign_utxo() {
13871387
builder
13881388
.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000))
13891389
.only_witness_utxo()
1390-
.add_foreign_utxo(
1391-
utxo.outpoint,
1392-
psbt_input,
1393-
foreign_utxo_satisfaction.to_wu() as usize,
1394-
)
1390+
.add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction)
13951391
.unwrap();
13961392
let mut psbt = builder.finish().unwrap();
13971393
wallet1.insert_txout(utxo.outpoint, utxo.txout);
@@ -1467,11 +1463,7 @@ fn test_calculate_fee_with_missing_foreign_utxo() {
14671463
builder
14681464
.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000))
14691465
.only_witness_utxo()
1470-
.add_foreign_utxo(
1471-
utxo.outpoint,
1472-
psbt_input,
1473-
foreign_utxo_satisfaction.to_wu() as usize,
1474-
)
1466+
.add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction)
14751467
.unwrap();
14761468
let psbt = builder.finish().unwrap();
14771469
let tx = psbt.extract_tx().expect("failed to extract tx");
@@ -1488,11 +1480,8 @@ fn test_add_foreign_utxo_invalid_psbt_input() {
14881480
.unwrap();
14891481

14901482
let mut builder = wallet.build_tx();
1491-
let result = builder.add_foreign_utxo(
1492-
outpoint,
1493-
psbt::Input::default(),
1494-
foreign_utxo_satisfaction.to_wu() as usize,
1495-
);
1483+
let result =
1484+
builder.add_foreign_utxo(outpoint, psbt::Input::default(), foreign_utxo_satisfaction);
14961485
assert!(matches!(result, Err(AddForeignUtxoError::MissingUtxo)));
14971486
}
14981487

@@ -1520,7 +1509,7 @@ fn test_add_foreign_utxo_where_outpoint_doesnt_match_psbt_input() {
15201509
non_witness_utxo: Some(tx1.as_ref().clone()),
15211510
..Default::default()
15221511
},
1523-
satisfaction_weight.to_wu() as usize
1512+
satisfaction_weight
15241513
)
15251514
.is_err(),
15261515
"should fail when outpoint doesn't match psbt_input"
@@ -1533,7 +1522,7 @@ fn test_add_foreign_utxo_where_outpoint_doesnt_match_psbt_input() {
15331522
non_witness_utxo: Some(tx2.as_ref().clone()),
15341523
..Default::default()
15351524
},
1536-
satisfaction_weight.to_wu() as usize
1525+
satisfaction_weight
15371526
)
15381527
.is_ok(),
15391528
"should be ok when outpoint does match psbt_input"
@@ -1565,11 +1554,7 @@ fn test_add_foreign_utxo_only_witness_utxo() {
15651554
..Default::default()
15661555
};
15671556
builder
1568-
.add_foreign_utxo(
1569-
utxo2.outpoint,
1570-
psbt_input,
1571-
satisfaction_weight.to_wu() as usize,
1572-
)
1557+
.add_foreign_utxo(utxo2.outpoint, psbt_input, satisfaction_weight)
15731558
.unwrap();
15741559
assert!(
15751560
builder.finish().is_err(),
@@ -1585,11 +1570,7 @@ fn test_add_foreign_utxo_only_witness_utxo() {
15851570
};
15861571
builder
15871572
.only_witness_utxo()
1588-
.add_foreign_utxo(
1589-
utxo2.outpoint,
1590-
psbt_input,
1591-
satisfaction_weight.to_wu() as usize,
1592-
)
1573+
.add_foreign_utxo(utxo2.outpoint, psbt_input, satisfaction_weight)
15931574
.unwrap();
15941575
assert!(
15951576
builder.finish().is_ok(),
@@ -1605,11 +1586,7 @@ fn test_add_foreign_utxo_only_witness_utxo() {
16051586
..Default::default()
16061587
};
16071588
builder
1608-
.add_foreign_utxo(
1609-
utxo2.outpoint,
1610-
psbt_input,
1611-
satisfaction_weight.to_wu() as usize,
1612-
)
1589+
.add_foreign_utxo(utxo2.outpoint, psbt_input, satisfaction_weight)
16131590
.unwrap();
16141591
assert!(
16151592
builder.finish().is_ok(),
@@ -3420,11 +3397,7 @@ fn test_taproot_foreign_utxo() {
34203397
let mut builder = wallet1.build_tx();
34213398
builder
34223399
.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000))
3423-
.add_foreign_utxo(
3424-
utxo.outpoint,
3425-
psbt_input,
3426-
foreign_utxo_satisfaction.to_wu() as usize,
3427-
)
3400+
.add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction)
34283401
.unwrap();
34293402
let psbt = builder.finish().unwrap();
34303403
let sent_received =

0 commit comments

Comments
 (0)