Merge #1323: spend: ensure fee above min relay value

74a53ba spend: add 10 sats to fee for 1 sat/vb txs (Michael Mallan)

Pull request description:

  This is a quick change to address #1322.

  It adds 10 sats to the fee for a non-replacement transaction targeted at 1 sat/vb.

ACKs for top commit:
  darosior:
    utACK 74a53ba. Looking forward to a real fix.

Tree-SHA512: c8ef2a49be67b94c2198a246a82f92f16661308151540197a7831a06e1eca277fa3dafe733b53a97dc3c9fc6639f4b724237863f186e4ffeb0bf61c74a1e9c0b

src/commands/mod.rs

@@ -1494,7 +1494,7 @@ mod tests {
 
         // Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 161 sats fees.
         // At 2sats/vb, it's twice that.
-        assert_eq!(tx.output[1].value.to_sat(), 89_839);
+        assert_eq!(tx.output[1].value.to_sat(), 89_829);
         let psbt = if let CreateSpendResult::Success { psbt, .. } = control
             .create_spend(&destinations, &[dummy_op], 2, None)
             .unwrap()
@@ -1562,18 +1562,18 @@ mod tests {
             dummy_addr.payload().script_pubkey()
         );
         assert_eq!(tx.output[0].value.to_sat(), 95_000);
-        // change = 100_000 - 95_000 - /* fee without change */ 127 - /* extra fee for change output */ 43 = 4830
+        // change = 100_000 - 95_000 - /* fee without change */ 128 - /* extra fee for change output */ 43 = 4829
         assert_eq!(
             warnings,
             vec![
                 "Dust UTXO. The minimal change output allowed by Liana is 5000 sats. \
-                Instead of creating a change of 4839 sats, it was added to the \
+                Instead of creating a change of 4829 sats, it was added to the \
                 transaction fee. Select a larger input to avoid this from happening."
             ]
         );
 
         // Increase the target value by the change amount and the warning will disappear.
-        *destinations.get_mut(&dummy_addr).unwrap() = 95_000 + 4_839;
+        *destinations.get_mut(&dummy_addr).unwrap() = 95_000 + 4_829;
         let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()
@@ -1588,7 +1588,7 @@ mod tests {
 
         // Now increase target also by the extra fee that was paying for change and we can still create the spend.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            95_000 + 4_830 + /* fee for change output */ 43;
+            95_000 + 4_829 + /* fee for change output */ 43;
         let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()
@@ -1603,15 +1603,15 @@ mod tests {
 
         // Now increase the target by 1 more sat and we will have insufficient funds.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            95_000 + 4_839 + /* fee for change output */ 43 + 1;
+            95_000 + 4_829 + /* fee for change output */ 43 + 1;
         assert_eq!(
             control.create_spend(&destinations, &[dummy_op], 1, None),
             Ok(CreateSpendResult::InsufficientFunds { missing: 1 }),
         );
 
         // Now decrease the target so that the lost change is just 1 sat.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            100_000 - /* fee without change */ 118 - /* extra fee for change output */ 43 - 1;
+            100_000 - /* fee without change */ 128 - /* extra fee for change output */ 43 - 1;
         let warnings = if let CreateSpendResult::Success { warnings, .. } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()
@@ -1632,7 +1632,7 @@ mod tests {
 
         // Now decrease the target value so that we have enough for a change output.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            95_000 - /* fee without change */ 118 - /* extra fee for change output */ 43;
+            95_000 - /* fee without change */ 128 - /* extra fee for change output */ 43;
         let (psbt, warnings) = if let CreateSpendResult::Success { psbt, warnings } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()
@@ -1648,7 +1648,7 @@ mod tests {
 
         // Now increase the target by 1 and we'll get a warning again, this time for 1 less than the dust threshold.
         *destinations.get_mut(&dummy_addr).unwrap() =
-            95_000 - /* fee without change */ 118 - /* extra fee for change output */ 43 + 1;
+            95_000 - /* fee without change */ 128 - /* extra fee for change output */ 43 + 1;
         let warnings = if let CreateSpendResult::Success { warnings, .. } = control
             .create_spend(&destinations, &[dummy_op], 1, None)
             .unwrap()

src/spend.rs

@@ -12,6 +12,7 @@ use bdk_coin_select::{
     metrics::LowestFee, Candidate, ChangePolicy, CoinSelector, DrainWeights, FeeRate, Replace,
     Target, TargetFee, TargetOutputs, TXIN_BASE_WEIGHT,
 };
+use log::info;
 use miniscript::bitcoin::{
     self,
     absolute::{Height, LockTime},
@@ -384,9 +385,21 @@ fn select_coins_for_spend(
         long_term_feerate,
     );
 
+    // FIXME: This is a quick change to avoid going below the min relay fee:
+    // If the required feerate is 1 sat/vb and this is not a replacement tx,
+    // use the replaced_fee parameter to ensure we pay at least 10 sats more
+    // than the size of the tx.
+    // E.g. if vsize = 186, the fee will be pay >= 196 sats.
+    let replaced_fee_modified = if replaced_fee.is_none() && feerate_vb_u32 == 1 {
+        info!("setting replaced fee to 10");
+        Some(10)
+    } else {
+        replaced_fee
+    };
+
     // Finally, run the coin selection algorithm. We use an opportunistic BnB and if it couldn't
     // find any solution we fall back to selecting coins by descending value.
-    let replace = replaced_fee.map(Replace::new);
+    let replace = replaced_fee_modified.map(Replace::new);
     let target_fee = TargetFee {
         rate: feerate,
         replace,

tests/test_spend.py

@@ -134,7 +134,7 @@ def sign_and_broadcast(psbt):
     res = lianad.rpc.createspend(destinations, [outpoint], 1)
     psbt = PSBT.from_base64(res["psbt"])
     sign_and_broadcast(psbt)
-    change_amount = 858 if USE_TAPROOT else 839
+    change_amount = 848 if USE_TAPROOT else 829
     assert len(psbt.o) == 1
     assert len(res["warnings"]) == 1
     assert (
@@ -153,7 +153,7 @@ def sign_and_broadcast(psbt):
     res = lianad.rpc.createspend(destinations, [outpoint_3], 1)
     psbt = PSBT.from_base64(res["psbt"])
     sign_and_broadcast(psbt)
-    change_amount = 846 if USE_TAPROOT else 827
+    change_amount = 836 if USE_TAPROOT else 817
     assert len(psbt.o) == 1
     assert len(res["warnings"]) == 1
     assert (