Skip to content

Commit

Permalink
fix(tx_builder)!: make TxBuilder Send safe, remove Clone trait
Browse files Browse the repository at this point in the history
  • Loading branch information
notmandatory committed Nov 22, 2024
1 parent 03c7368 commit 663fb13
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 26 deletions.
4 changes: 2 additions & 2 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1195,7 +1195,7 @@ impl Wallet {
/// [`TxBuilder`]: crate::TxBuilder
pub fn build_tx(&mut self) -> TxBuilder<'_, DefaultCoinSelectionAlgorithm> {
TxBuilder {
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
wallet: self,
params: TxParams::default(),
coin_selection: DefaultCoinSelectionAlgorithm::default(),
}
Expand Down Expand Up @@ -1701,7 +1701,7 @@ impl Wallet {
};

Ok(TxBuilder {
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
wallet: self,
params,
coin_selection: DefaultCoinSelectionAlgorithm::default(),
})
Expand Down
23 changes: 5 additions & 18 deletions crates/wallet/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
//! # Ok::<(), anyhow::Error>(())
//! ```

use alloc::{boxed::Box, rc::Rc, string::String, vec::Vec};
use core::cell::RefCell;
use alloc::{boxed::Box, string::String, vec::Vec};
use core::fmt;

use alloc::sync::Arc;
Expand Down Expand Up @@ -111,7 +110,7 @@ use crate::{KeychainKind, LocalOutput, Utxo, WeightedUtxo};
/// [`coin_selection`]: Self::coin_selection
#[derive(Debug)]
pub struct TxBuilder<'a, Cs> {
pub(crate) wallet: Rc<RefCell<&'a mut Wallet>>,
pub(crate) wallet: &'a mut Wallet,
pub(crate) params: TxParams,
pub(crate) coin_selection: Cs,
}
Expand Down Expand Up @@ -161,16 +160,6 @@ impl Default for FeePolicy {
}
}

impl<'a, Cs: Clone> Clone for TxBuilder<'a, Cs> {
fn clone(&self) -> Self {
TxBuilder {
wallet: self.wallet.clone(),
params: self.params.clone(),
coin_selection: self.coin_selection.clone(),
}
}
}

// Methods supported for any CoinSelectionAlgorithm.
impl<'a, Cs> TxBuilder<'a, Cs> {
/// Set a custom fee rate.
Expand Down Expand Up @@ -286,7 +275,7 @@ impl<'a, Cs> TxBuilder<'a, Cs> {
/// the "utxos" and the "unspendable" list, it will be spent.
pub fn add_utxos(&mut self, outpoints: &[OutPoint]) -> Result<&mut Self, AddUtxoError> {
{
let wallet = self.wallet.borrow();
let wallet = &mut self.wallet;
let utxos = outpoints
.iter()
.map(|outpoint| {
Expand Down Expand Up @@ -682,9 +671,7 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs> {
/// **WARNING**: To avoid change address reuse you must persist the changes resulting from one
/// or more calls to this method before closing the wallet. See [`Wallet::reveal_next_address`].
pub fn finish_with_aux_rand(self, rng: &mut impl RngCore) -> Result<Psbt, CreateTxError> {
self.wallet
.borrow_mut()
.create_tx(self.coin_selection, self.params, rng)
self.wallet.create_tx(self.coin_selection, self.params, rng)
}
}

Expand Down Expand Up @@ -750,7 +737,7 @@ impl fmt::Display for AddForeignUtxoError {
#[cfg(feature = "std")]
impl std::error::Error for AddForeignUtxoError {}

type TxSort<T> = dyn Fn(&T, &T) -> core::cmp::Ordering;
type TxSort<T> = dyn (Fn(&T, &T) -> core::cmp::Ordering) + Send + Sync;

/// Ordering of the transaction's inputs and outputs
#[derive(Clone, Default)]
Expand Down
21 changes: 15 additions & 6 deletions crates/wallet/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1645,11 +1645,10 @@ fn test_add_foreign_utxo_only_witness_utxo() {
.max_weight_to_satisfy()
.unwrap();

let mut builder = wallet1.build_tx();
builder.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000));

{
let mut builder = builder.clone();
let mut builder = wallet1.build_tx();
builder.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000));

let psbt_input = psbt::Input {
witness_utxo: Some(utxo2.txout.clone()),
..Default::default()
Expand All @@ -1664,7 +1663,9 @@ fn test_add_foreign_utxo_only_witness_utxo() {
}

{
let mut builder = builder.clone();
let mut builder = wallet1.build_tx();
builder.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000));

let psbt_input = psbt::Input {
witness_utxo: Some(utxo2.txout.clone()),
..Default::default()
Expand All @@ -1680,7 +1681,9 @@ fn test_add_foreign_utxo_only_witness_utxo() {
}

{
let mut builder = builder.clone();
let mut builder = wallet1.build_tx();
builder.add_recipient(addr.script_pubkey(), Amount::from_sat(60_000));

let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx;
let psbt_input = psbt::Input {
non_witness_utxo: Some(tx2.as_ref().clone()),
Expand Down Expand Up @@ -4169,3 +4172,9 @@ fn test_transactions_sort_by() {
.collect();
assert_eq!([None, Some(2000), Some(1000)], conf_heights.as_slice());
}

#[test]
fn test_tx_builder_is_send_safe() {
let (mut wallet, _txid) = get_funded_wallet_wpkh();
let _box: Box<dyn Send + Sync> = Box::new(wallet.build_tx());
}

0 comments on commit 663fb13

Please sign in to comment.