Merge #1061: spend: reword warning messages

ed86696daf556533592b3285c70a2386a84ece71 tests: remove print statements (jp1ac4)
5d6ca97e4cf2e1778552d4980291eca1d2cc57c7 spend: reword warning messages (jp1ac4)
8078a2791a81eecd7f662e4fd43965e185c1737e tests: use function to calculate additional fee (jp1ac4)
deb75884bfe8a47c9143fd7006960c81d956d6d4 tests: run black (jp1ac4)
f14c3a7f75f4483e83da5a6dfd2dc0cb3624133a spend: fix typo in enum variant (jp1ac4)

Pull request description:

  As discussed with @kloaec, this PR rewords the warning messages when creating a spend.

  I didn't include the selected feerate value in the updated messages as that would have required more code changes, albeit fairly simple ones, beyond the message wording itself. I can add those here if required or otherwise in a follow-up.

  Here's a screenshot showing the new warnings:

  ![image](https://github.com/wizardsardine/liana/assets/121959000/7bd4ec6b-a878-4b25-950a-a4ea87175a6f)

ACKs for top commit:
  darosior:
    ACK ed86696daf556533592b3285c70a2386a84ece71

Tree-SHA512: e8f72206ccba74609e1a8b4a9dccd5975534de7a392d24e54ab8bb5ca6d4d38a75864e148e463e66ffda48fef1a68ab0e953c6fe24a4e6bf28eca9535135f7e4

src/commands/mod.rs

@@ -1571,7 +1571,14 @@ mod tests {
         );
         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
-        assert_eq!(warnings, vec!["Change amount of 4830 sats added to fee as it was too small to create a transaction output."]);
+        assert_eq!(
+            warnings,
+            vec![
+                "Dust UTXO. The minimal change output allowed by Liana is 5000 sats. \
+                Instead of creating a change of 4830 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_830;
@@ -1622,7 +1629,14 @@ mod tests {
             panic!("expect successful spend creation")
         };
         // Message uses "sat" instead of "sats" when value is 1.
-        assert_eq!(warnings, vec!["Change amount of 1 sat added to fee as it was too small to create a transaction output."]);
+        assert_eq!(
+            warnings,
+            vec![
+                "Dust UTXO. The minimal change output allowed by Liana is 5000 sats. \
+                Instead of creating a change of 1 sat, it was added to the \
+                transaction fee. Select a larger input to avoid this from happening."
+            ]
+        );
 
         // Now decrease the target value so that we have enough for a change output.
         *destinations.get_mut(&dummy_addr).unwrap() =
@@ -1651,7 +1665,14 @@ mod tests {
         } else {
             panic!("expect successful spend creation")
         };
-        assert_eq!(warnings, vec!["Change amount of 4999 sats added to fee as it was too small to create a transaction output."]);
+        assert_eq!(
+            warnings,
+            vec![
+                "Dust UTXO. The minimal change output allowed by Liana is 5000 sats. \
+                Instead of creating a change of 4999 sats, it was added to the \
+                transaction fee. Select a larger input to avoid this from happening."
+            ]
+        );
 
         // Now if we mark the coin as spent, we won't create another Spend transaction containing
         // it.

src/spend.rs

@@ -543,7 +543,7 @@ pub enum SpendTxFees {
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
 pub enum CreateSpendWarning {
     ChangeAddedToFee(u64),
-    AddtionalFeeForAncestors(u64),
+    AdditionalFeeForAncestors(u64),
 }
 
 impl fmt::Display for CreateSpendWarning {
@@ -551,15 +551,20 @@ impl fmt::Display for CreateSpendWarning {
         match self {
             CreateSpendWarning::ChangeAddedToFee(amt) => write!(
                 f,
-                "Change amount of {} sat{} added to fee as it was too small to create a transaction output.",
+                "Dust UTXO. The minimal change output allowed by Liana is {} sats. \
+                Instead of creating a change of {} sat{}, it was added to the \
+                transaction fee. Select a larger input to avoid this from happening.",
+                DUST_OUTPUT_SATS,
                 amt,
-                if *amt > 1 {"s"} else {""},
+                if *amt > 1 { "s" } else { "" },
             ),
-            CreateSpendWarning::AddtionalFeeForAncestors(amt) => write!(
+            CreateSpendWarning::AdditionalFeeForAncestors(amt) => write!(
                 f,
-                "An additional fee of {} sat{} has been added to pay for ancestors at the target feerate.",
+                "CPFP: an unconfirmed input was selected. The current transaction fee \
+                was increased by {} sat{} to make the average feerate of both the input \
+                and current transaction equal to the selected feerate.",
                 amt,
-                if *amt > 1 {"s"} else {""},
+                if *amt > 1 { "s" } else { "" },
             ),
         }
     }
@@ -737,7 +742,7 @@ pub fn create_spend(
     }
 
     if fee_for_ancestors.to_sat() > 0 {
-        warnings.push(CreateSpendWarning::AddtionalFeeForAncestors(
+        warnings.push(CreateSpendWarning::AdditionalFeeForAncestors(
             fee_for_ancestors.to_sat(),
         ));
     }

tests/test_framework/signer.py

@@ -93,7 +93,9 @@ def sign_psbt_taproot(psbt, hds):
     psbt_str = psbt.to_base64()
     for hd in hds:
         xprv = hd.get_xpriv()
-        proc = subprocess.run([bin_path, psbt_str, xprv], capture_output=True, check=True)
+        proc = subprocess.run(
+            [bin_path, psbt_str, xprv], capture_output=True, check=True
+        )
         psbt_str = proc.stdout.decode("utf-8")
 
     return PSBT.from_base64(psbt_str)

tests/test_framework/utils.py

@@ -24,7 +24,9 @@ DEFAULT_BITCOIND_PATH = "bitcoind"
 BITCOIND_PATH = os.getenv("BITCOIND_PATH", DEFAULT_BITCOIND_PATH)
 OLD_LIANAD_PATH = os.getenv("OLD_LIANAD_PATH", None)
 IS_NOT_BITCOIND_24 = bool(int(os.getenv("IS_NOT_BITCOIND_24", True)))
-USE_TAPROOT = bool(int(os.getenv("USE_TAPROOT", False)))  # TODO: switch to True in a couple releases.
+USE_TAPROOT = bool(
+    int(os.getenv("USE_TAPROOT", False))
+)  # TODO: switch to True in a couple releases.
 
 
 COIN = 10**8

tests/test_misc.py

@@ -250,12 +250,15 @@ def test_coinbase_deposit(lianad, bitcoind):
     assert coin["is_change"]
     bitcoind.generate_block(100)
     wait_for_sync()
-    coin = next(c for c in lianad.rpc.listcoins()["coins"] if c["outpoint"] == coin["outpoint"])
+    coin = next(
+        c for c in lianad.rpc.listcoins()["coins"] if c["outpoint"] == coin["outpoint"]
+    )
     assert not coin["is_immature"] and coin["block_height"] is not None
 
 
 @pytest.mark.skipif(
-    OLD_LIANAD_PATH is None or USE_TAPROOT, reason="Need the old lianad binary to create the datadir."
+    OLD_LIANAD_PATH is None or USE_TAPROOT,
+    reason="Need the old lianad binary to create the datadir.",
 )
 def test_migration(lianad_multisig, bitcoind):
     """Test we can start a newer lianad on a datadir created by an older lianad."""

tests/test_spend.py

@@ -1,6 +1,19 @@
 from fixtures import *
 from test_framework.serializations import PSBT, uint256_from_str
-from test_framework.utils import sign_and_broadcast_psbt, wait_for, COIN, RpcError, USE_TAPROOT
+from test_framework.utils import (
+    sign_and_broadcast_psbt,
+    wait_for,
+    COIN,
+    RpcError,
+    USE_TAPROOT,
+)
+
+
+def additional_fees(anc_vsize, anc_fee, target_feerate):
+    """The additional fee which must have been computed by lianad."""
+    computed_anc_vsize = int(anc_fee / target_feerate)
+    extra_vsize = anc_vsize - computed_anc_vsize
+    return extra_vsize * target_feerate
 
 
 def test_spend_change(lianad, bitcoind):
@@ -126,7 +139,9 @@ def test_coin_marked_spent(lianad, bitcoind):
     assert len(res["warnings"]) == 1
     assert (
         res["warnings"][0]
-        == f"Change amount of {change_amount} sats added to fee as it was too small to create a transaction output."
+        == "Dust UTXO. The minimal change output allowed by Liana is 5000 sats. "
+        f"Instead of creating a change of {change_amount} sats, it was added to the "
+        "transaction fee. Select a larger input to avoid this from happening."
     )
 
     # Spend the third coin to an address of ours, no change
@@ -143,7 +158,9 @@ def test_coin_marked_spent(lianad, bitcoind):
     assert len(res["warnings"]) == 1
     assert (
         res["warnings"][0]
-        == f"Change amount of {change_amount} sats added to fee as it was too small to create a transaction output."
+        == "Dust UTXO. The minimal change output allowed by Liana is 5000 sats. "
+        f"Instead of creating a change of {change_amount} sats, it was added to the "
+        "transaction fee. Select a larger input to avoid this from happening."
     )
 
     # Spend the fourth coin to an address of ours, with change
@@ -173,15 +190,15 @@ def test_coin_marked_spent(lianad, bitcoind):
     psbt = PSBT.from_base64(res["psbt"])
     sign_and_broadcast(psbt)
     assert len(psbt.o) == 4
-    assert bitcoind.rpc.getmempoolentry(deposit_d)["ancestorsize"] == 165
+    anc_vsize = bitcoind.rpc.getmempoolentry(deposit_d)["ancestorsize"]
-    assert bitcoind.rpc.getmempoolentry(deposit_d)["fees"]["ancestor"] * COIN == 165
+    anc_fees = int(bitcoind.rpc.getmempoolentry(deposit_d)["fees"]["ancestor"] * COIN)
-    # ancestor vsize at feerate 2 sat/vb = ancestor_fee / 2 = 165 / 2 = 82
+    additional_fee = additional_fees(anc_vsize, anc_fees, 2)
-    # extra_weight <= (extra vsize * witness factor) = (165 - 82) * 4 = 332
-    # additional fee at 2 sat/vb (0.5 sat/wu) = 332 * 0.5 = 166
     assert len(res["warnings"]) == 1
     assert (
         res["warnings"][0]
-        == "An additional fee of 166 sats has been added to pay for ancestors at the target feerate."
+        == "CPFP: an unconfirmed input was selected. The current transaction fee "
+        f"was increased by {additional_fee} sats to make the average feerate of "
+        "both the input and current transaction equal to the selected feerate."
     )
 
     # All the spent coins must have been detected as such
@@ -316,14 +333,6 @@ def test_coin_selection(lianad, bitcoind):
     dest_addr_2 = bitcoind.rpc.getnewaddress()
     # If feerate is higher than ancestor, we'll need to pay extra.
 
-    def additional_fees(anc_vsize, anc_fee, target_feerate):
-        """The additional fee which must have been computed by lianad."""
-        computed_anc_vsize = int(anc_fee / target_feerate)
-        print(f"c  ", computed_anc_vsize)
-        extra_vsize = anc_vsize - computed_anc_vsize
-        print("e  ", extra_vsize)
-        return extra_vsize * target_feerate
-
     # Try 10 sat/vb:
     feerate = 10
     spend_res_2 = lianad.rpc.createspend({dest_addr_2: 10_000}, [], feerate)
@@ -334,12 +343,16 @@ def test_coin_selection(lianad, bitcoind):
         bytes.fromhex(spend_txid_1)[::-1]
     )
     anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"]
-    anc_fees = int(bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN)
+    anc_fees = int(
+        bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN
+    )
     additional_fee = additional_fees(anc_vsize, anc_fees, feerate)
     assert len(spend_res_2["warnings"]) == 1
     assert (
         spend_res_2["warnings"][0]
-        == f"An additional fee of {additional_fee} sats has been added to pay for ancestors at the target feerate."
+        == "CPFP: an unconfirmed input was selected. The current transaction fee "
+        f"was increased by {additional_fee} sats to make the average feerate of "
+        "both the input and current transaction equal to the selected feerate."
     )
 
     # Try 3 sat/vb:
@@ -352,12 +365,16 @@ def test_coin_selection(lianad, bitcoind):
         bytes.fromhex(spend_txid_1)[::-1]
     )
     anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"]
-    anc_fees = int(bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN)
+    anc_fees = int(
+        bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN
+    )
     additional_fee = additional_fees(anc_vsize, anc_fees, feerate)
     assert len(spend_res_2["warnings"]) == 1
     assert (
         spend_res_2["warnings"][0]
-        == f"An additional fee of {additional_fee} sats has been added to pay for ancestors at the target feerate."
+        == "CPFP: an unconfirmed input was selected. The current transaction fee "
+        f"was increased by {additional_fee} sats to make the average feerate of "
+        "both the input and current transaction equal to the selected feerate."
     )
 
     # 2 sat/vb is same feerate as ancestor and we have no warnings:
@@ -398,12 +415,18 @@ def test_coin_selection(lianad, bitcoind):
     anc_vsize = bitcoind.rpc.getmempoolentry(deposit_2)["ancestorsize"]
     anc_fees = int(bitcoind.rpc.getmempoolentry(deposit_2)["fees"]["ancestor"] * COIN)
     prev_anc_vsize = bitcoind.rpc.getmempoolentry(spend_txid_1)["ancestorsize"]
-    prev_anc_fees = int(bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN)
+    prev_anc_fees = int(
-    additional_fee = additional_fees(anc_vsize, anc_fees, feerate) + additional_fees(prev_anc_vsize, prev_anc_fees, feerate)
+        bitcoind.rpc.getmempoolentry(spend_txid_1)["fees"]["ancestor"] * COIN
+    )
+    additional_fee = additional_fees(anc_vsize, anc_fees, feerate) + additional_fees(
+        prev_anc_vsize, prev_anc_fees, feerate
+    )
     assert len(spend_res_3["warnings"]) == 1
     assert (
         spend_res_3["warnings"][0]
-        == f"An additional fee of {additional_fee} sats has been added to pay for ancestors at the target feerate."
+        == "CPFP: an unconfirmed input was selected. The current transaction fee "
+        f"was increased by {additional_fee} sats to make the average feerate of "
+        "both the input and current transaction equal to the selected feerate."
     )
     spend_psbt_3 = PSBT.from_base64(spend_res_3["psbt"])
     spend_txid_3 = sign_and_broadcast_psbt(lianad, spend_psbt_3)