From 96f2a5dd002127d8d24fa2336fb93f69329af298 Mon Sep 17 00:00:00 2001
From: Antoine Poinsot <darosior@protonmail.com>
Date: Wed, 6 Dec 2023 16:47:39 +0100
Subject: [PATCH] Feerate must be set, except for cancel it mustn't

---
 src/commands/mod.rs | 18 +++++++++++++++++-
 tests/test_rpc.py   | 10 ++++++++--
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/src/commands/mod.rs b/src/commands/mod.rs
index 19ae85c70..fa7d65cc1 100644
--- a/src/commands/mod.rs
+++ b/src/commands/mod.rs
@@ -175,6 +175,8 @@ pub enum InsaneFeeInfo {
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum RbfErrorInfo {
+    MissingFeerate,
+    SuperfluousFeerate,
     TooLowFeerate(u64),
     NotSignaling,
 }
@@ -182,6 +184,12 @@ pub enum RbfErrorInfo {
 impl fmt::Display for RbfErrorInfo {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
+            Self::MissingFeerate => {
+                write!(f, "A feerate must be provided if not creating a cancel.")
+            }
+            Self::SuperfluousFeerate => {
+                write!(f, "A feerate must not be provided if creating a cancel. We'll always use the smallest one which satisfies the RBF rules.")
+            }
             Self::TooLowFeerate(r) => write!(f, "Feerate too low: {}.", r),
             Self::NotSignaling => write!(f, "Replacement candidate does not signal for RBF."),
         }
@@ -862,6 +870,10 @@ impl DaemonControl {
     ) -> Result<CreateSpendResult, CommandError> {
         let mut db_conn = self.db.connection();
 
+        if is_cancel && feerate_vb.is_some() {
+            return Err(CommandError::RbfError(RbfErrorInfo::SuperfluousFeerate));
+        }
+
         let prev_psbt = db_conn
             .spend_tx(txid)
             .ok_or(CommandError::UnknownSpend(*txid))?;
@@ -919,7 +931,11 @@ impl DaemonControl {
             );
         // Check replacement transaction's target feerate, if set, is high enough,
         // and otherwise set it to the min feerate found above.
-        let feerate_vb = feerate_vb.unwrap_or(min_feerate_vb);
+        let feerate_vb = if is_cancel {
+            min_feerate_vb
+        } else {
+            feerate_vb.ok_or(CommandError::RbfError(RbfErrorInfo::MissingFeerate))?
+        };
         if feerate_vb < min_feerate_vb {
             return Err(CommandError::RbfError(RbfErrorInfo::TooLowFeerate(
                 feerate_vb,
diff --git a/tests/test_rpc.py b/tests/test_rpc.py
index 91f2d7b3b..b3bee3dc4 100644
--- a/tests/test_rpc.py
+++ b/tests/test_rpc.py
@@ -1183,8 +1183,14 @@ def test_rbfpsbt_cancel(lianad, bitcoind):
     )
     # We can use RBF and let the command choose the min possible feerate (1 larger than previous).
     rbf_1_res = lianad.rpc.rbfpsbt(first_txid, True)
-    # We can also pass the feerate explicitly.
-    rbf_1_res = lianad.rpc.rbfpsbt(first_txid, True, 2)
+    # But we can't set the feerate explicitly.
+    with pytest.raises(
+        RpcError,
+        match=re.escape(
+            "A feerate must not be provided if creating a cancel."
+        ),
+    ):
+        rbf_1_res = lianad.rpc.rbfpsbt(first_txid, True, 2)
     rbf_1_psbt = PSBT.from_base64(rbf_1_res["psbt"])
     # Replacement only has a single input.
     assert len(rbf_1_psbt.i) == 1