From ac051d7ae9512883e11a89ab002ad2d2c3c55c19 Mon Sep 17 00:00:00 2001 From: Daniela Brozzoni Date: Mon, 11 Jul 2022 21:52:11 +0200 Subject: [PATCH] Calculate fee amount after output addition We would previously calculate the fee amount in two steps: 1. Add the weight of the empty transaction 2. Add the weight of each output That's unnecessary: you can just use the weight of the transaction *after* the output addition. This is clearer, but also avoids a rare bug: if there are many outputs, adding them would cause the "number of outputs" transaction parameter lenght to increase, and we wouldn't notice it. This might still happen when adding the drain output - this commit also adds a comment as a reminder. --- src/wallet/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index ddc6d7d1..dccc427d 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -736,8 +736,6 @@ where let mut outgoing: u64 = 0; let mut received: u64 = 0; - fee_amount += fee_rate.fee_wu(tx.weight()); - let recipients = params.recipients.iter().map(|(r, v)| (r, *v)); for (index, (script_pubkey, value)) in recipients.enumerate() { @@ -753,13 +751,14 @@ where script_pubkey: script_pubkey.clone(), value, }; - fee_amount += fee_rate.fee_vb(serialize(&new_out).len()); tx.output.push(new_out); outgoing += value; } + fee_amount += fee_rate.fee_wu(tx.weight()); + if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeAllowed && self.change_descriptor.is_none() { @@ -851,6 +850,9 @@ where script_pubkey: drain_script, }; + // TODO: We should pay attention when adding a new output: this might increase + // the lenght of the "number of vouts" parameter by 2 bytes, potentially making + // our feerate too low tx.output.push(drain_output); } };