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.

diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs
index ddc6d7d1715628623e0a21d9536c1c6361e4474f..dccc427db86102683c7032571f66445ebdb9e2f2 100644 (file)
--- 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);
             }
         };