Merge #881: descriptors: add unsigned tx max size helper

927c252d2e1f4755241a5f83666c034f7f8c43c3 spend: use helper for unsigned tx max size (jp1ac4)
af416f6502924946da8d2e9a070fde2f5d731776 commands: use helper for unsigned tx max size (jp1ac4)
e9264bdf0d8ed2b1532a9cde231a0e86ff657690 descriptors: add helper for unsigned tx max size (jp1ac4)
0f8571b90127fc68ac1304892ca95fee8e16d024 descriptors: use witness scale factor from lib (jp1ac4)

Pull request description:

  This is a first step towards resolving #880.

  It adds a helper function to `LianaDescriptor` that calculates the max size of an unsigned transaction after satisfaction,  assuming all transaction inputs are from that descriptor.

  Separately, I use this new helper function in spend and commands modules (I can drop/squash these commits as required).

ACKs for top commit:
  darosior:
    ACK 927c252d2e1f4755241a5f83666c034f7f8c43c3

Tree-SHA512: 5be12484de9c74bb493066390b71544ac28b481256d7b5214234bfe7558422dffc29ddc666b69e3260d4cca765ddf2ae7e21b0dd8c5f73425f0e7a52cf533db0

src/commands/mod.rs

@@ -29,10 +29,7 @@ use std::{
 };
 
 use miniscript::{
-    bitcoin::{
-        self, address, bip32, constants::WITNESS_SCALE_FACTOR,
-        psbt::PartiallySignedTransaction as Psbt,
-    },
+    bitcoin::{self, address, bip32, psbt::PartiallySignedTransaction as Psbt},
     psbt::PsbtExt,
 };
 use serde::{Deserialize, Serialize};
@@ -179,29 +176,6 @@ impl<'a> TxGetter for BitcoindTxGetter<'a> {
     }
 }
 
-/// An unsigned transaction's maximum possible size in vbytes after satisfaction.
-///
-/// This assumes all inputs are internal (or have the same `max_sat_weight` value).
-///
-/// `tx` is the unsigned transaction.
-///
-/// `max_sat_weight` is the maximum weight difference of an input in the
-/// transaction before and after satisfaction. Must be in weight units.
-fn unsigned_tx_max_vbytes(tx: &bitcoin::Transaction, max_sat_weight: u64) -> u64 {
-    let witness_factor: u64 = WITNESS_SCALE_FACTOR.try_into().unwrap();
-    let num_inputs: u64 = tx.input.len().try_into().unwrap();
-    let tx_wu: u64 = tx
-        .weight()
-        .to_wu()
-        .checked_add(max_sat_weight.checked_mul(num_inputs).unwrap())
-        .unwrap();
-    tx_wu
-        .checked_add(witness_factor.checked_sub(1).unwrap())
-        .unwrap()
-        .checked_div(witness_factor)
-        .unwrap()
-}
-
 fn coin_to_candidate(
     coin: &Coin,
     must_select: bool,
@@ -826,12 +800,6 @@ impl DaemonControl {
         if !is_cancel {
             candidate_coins.extend(&confirmed_cands);
         }
-        let max_sat_weight: u64 = self
-            .config
-            .main_descriptor
-            .max_sat_weight()
-            .try_into()
-            .expect("it must fit");
         // Try with increasing fee until fee paid by replacement transaction is high enough.
         // Replacement fee must be at least:
         // sum of fees paid by original transactions + incremental feerate * replacement size.
@@ -868,7 +836,10 @@ impl DaemonControl {
                     return Err(e.into());
                 }
             };
-            replacement_vsize = unsigned_tx_max_vbytes(&rbf_psbt.unsigned_tx, max_sat_weight);
+            replacement_vsize = self
+                .config
+                .main_descriptor
+                .unsigned_tx_max_vbytes(&rbf_psbt.unsigned_tx);
 
             // Make sure it satisfies RBF rule 4.
             if rbf_psbt.fee().expect("has already been sanity checked")

src/descriptors/mod.rs

@@ -1,13 +1,14 @@
 use miniscript::{
     bitcoin::{
         self, bip32,
+        constants::WITNESS_SCALE_FACTOR,
         psbt::{Input as PsbtIn, Psbt},
         secp256k1,
     },
     descriptor, translate_hash_clone, ForEachKey, TranslatePk, Translator,
 };
 
-use std::{collections::BTreeMap, error, fmt, str};
+use std::{collections::BTreeMap, convert::TryInto, error, fmt, str};
 
 use serde::{Deserialize, Serialize};
 
@@ -17,8 +18,6 @@ pub use keys::*;
 pub mod analysis;
 pub use analysis::*;
 
-pub const WITNESS_FACTOR: usize = 4;
-
 #[derive(Debug)]
 pub enum LianaDescError {
     Miniscript(miniscript::Error),
@@ -204,9 +203,9 @@ impl LianaDescriptor {
     /// size of the witness stack length varint.
     pub fn max_sat_vbytes(&self) -> usize {
         self.max_sat_weight()
-            .checked_add(WITNESS_FACTOR - 1)
+            .checked_add(WITNESS_SCALE_FACTOR - 1)
             .unwrap()
-            .checked_div(WITNESS_FACTOR)
+            .checked_div(WITNESS_SCALE_FACTOR)
             .unwrap()
     }
 
@@ -355,6 +354,26 @@ impl LianaDescriptor {
             .unwrap_or(&policy.primary_path);
         Ok(self.prune_bip32_derivs(psbt, path_info))
     }
+
+    /// Maximum possible size in vbytes of an unsigned transaction, `tx`,
+    /// after satisfaction, assuming all inputs of `tx` are from this
+    /// descriptor.
+    pub fn unsigned_tx_max_vbytes(&self, tx: &bitcoin::Transaction) -> u64 {
+        let witness_factor: u64 = WITNESS_SCALE_FACTOR.try_into().unwrap();
+        let num_inputs: u64 = tx.input.len().try_into().unwrap();
+        let max_sat_weight: u64 = self.max_sat_weight().try_into().unwrap();
+        // Add weights together before converting to vbytes to avoid rounding up multiple times.
+        let tx_wu = tx
+            .weight()
+            .to_wu()
+            .checked_add(max_sat_weight.checked_mul(num_inputs).unwrap())
+            .unwrap();
+        tx_wu
+            .checked_add(witness_factor.checked_sub(1).unwrap())
+            .unwrap()
+            .checked_div(witness_factor)
+            .unwrap()
+    }
 }
 
 impl SinglePathLianaDesc {
@@ -476,8 +495,8 @@ mod tests {
 
     // Convert a size in weight units to a size in virtual bytes, rounding up.
     fn wu_to_vb(vb: usize) -> usize {
-        (vb + WITNESS_FACTOR - 1)
-            .checked_div(WITNESS_FACTOR)
+        (vb + WITNESS_SCALE_FACTOR - 1)
+            .checked_div(WITNESS_SCALE_FACTOR)
             .expect("Non 0")
     }
 

src/spend.rs

@@ -11,7 +11,6 @@ use miniscript::bitcoin::{
     self,
     absolute::{Height, LockTime},
     bip32,
-    constants::WITNESS_SCALE_FACTOR,
     psbt::{Input as PsbtIn, Output as PsbtOut, Psbt},
     secp256k1,
 };
@@ -134,14 +133,7 @@ fn sanity_check_psbt(
     }
 
     // Check the feerate isn't insane.
-    // Add weights together before converting to vbytes to avoid rounding up multiple times
-    // and increasing the result, which could lead to the feerate in sats/vb falling below 1.
-    let tx_wu = tx.weight().to_wu() + (spent_desc.max_sat_weight() * tx.input.len()) as u64;
-    let tx_vb = tx_wu
-        .checked_add(WITNESS_SCALE_FACTOR as u64 - 1)
-        .unwrap()
-        .checked_div(WITNESS_SCALE_FACTOR as u64)
-        .unwrap();
+    let tx_vb = spent_desc.unsigned_tx_max_vbytes(tx);
     let feerate_sats_vb = abs_fee
         .checked_div(tx_vb)
         .ok_or(SpendCreationError::InsaneFees(