Skip to content

Commit

Permalink
Merge bitcoindevkit#1768: refactor(wallet): cleanup and remove unused…
Browse files Browse the repository at this point in the history
… code in create_tx

a2f7a8b refactor(wallet): cleanup and remove unused code in create_tx (Steve Myers)

Pull request description:

  ### Description

  Cleanup and remove unused code in `Wallet::create_tx`, this was noticed during review of bitcoindevkit#1763. See: bitcoindevkit#1763 (comment)

  fixes bitcoindevkit#1710

  ### Notes to the reviewers

  In addition to removing the unneeded assignments to `fee_amount` and `received` I also refactored creation of the change output to be an `if let` instead of `match` statement since it only needs to do something if there is `Excess::Change`.

  I should have done this cleanup as part of bitcoindevkit#1048.

  ### Changelog notice

  None, only internal cleanup.

  ### 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:
  ValuedMammal:
    reACK a2f7a8b

Tree-SHA512: 64e5895ff3dc11f71c48b6c436d5c812504d0a24e92f1fdf451936f372d95ccdd8d89e5ac634a041bdee0a4836182f05127864ed744d560c9f8ec560e092c559
  • Loading branch information
notmandatory committed Dec 23, 2024
2 parents a208144 + a2f7a8b commit 4a3675f
Showing 1 changed file with 13 additions and 29 deletions.
42 changes: 13 additions & 29 deletions crates/wallet/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1399,19 +1399,13 @@ impl Wallet {
}

let mut outgoing = Amount::ZERO;
let mut received = Amount::ZERO;

let recipients = params.recipients.iter().map(|(r, v)| (r, *v));

for (index, (script_pubkey, value)) in recipients.enumerate() {
if !params.allow_dust && value.is_dust(script_pubkey) && !script_pubkey.is_op_return() {
return Err(CreateTxError::OutputBelowDustLimit(index));
}

if self.is_mine(script_pubkey.clone()) {
received += value;
}

let new_out = TxOut {
script_pubkey: script_pubkey.clone(),
value,
Expand Down Expand Up @@ -1467,9 +1461,8 @@ impl Wallet {
rng,
)
.map_err(CreateTxError::CoinSelection)?;
fee_amount += coin_selection.fee_amount;
let excess = &coin_selection.excess;

let excess = &coin_selection.excess;
tx.input = coin_selection
.selected
.iter()
Expand Down Expand Up @@ -1508,28 +1501,19 @@ impl Wallet {
}
}

match excess {
Excess::NoChange {
remaining_amount, ..
} => fee_amount += *remaining_amount,
Excess::Change { amount, fee } => {
if self.is_mine(drain_script.clone()) {
received += *amount;
}
fee_amount += *fee;

// create drain output
let drain_output = TxOut {
value: *amount,
script_pubkey: drain_script,
};
// if there's change, create and add a change output
if let Excess::Change { amount, .. } = excess {
// create drain output
let drain_output = TxOut {
value: *amount,
script_pubkey: drain_script,
};

// TODO: We should pay attention when adding a new output: this might increase
// the length of the "number of vouts" parameter by 2 bytes, potentially making
// our feerate too low
tx.output.push(drain_output);
}
};
// TODO: We should pay attention when adding a new output: this might increase
// the length of the "number of vouts" parameter by 2 bytes, potentially making
// our feerate too low
tx.output.push(drain_output);
}

// sort input/outputs according to the chosen algorithm
params.ordering.sort_tx_with_aux_rand(&mut tx, rng);
Expand Down

0 comments on commit 4a3675f

Please sign in to comment.