From 8d40addbd259176ddddab8eaf21aceca435234c8 Mon Sep 17 00:00:00 2001
From: Torkel Rogstad <torkel@rogstad.io>
Date: Thu, 4 Jan 2024 11:24:07 +0100
Subject: [PATCH 1/2] rpc: validate fee estimation mode case insensitive

---
 src/wallet/rpc/spend.cpp       |  5 +++--
 test/functional/wallet_send.py | 24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 6060f017ce..736413d482 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -11,6 +11,7 @@
 #include <script/script.h>
 #include <util/fees.h>
 #include <util/rbf.h>
+#include <util/strencodings.h>
 #include <util/translation.h>
 #include <util/vector.h>
 #include <wallet/coincontrol.h>
@@ -52,7 +53,7 @@ static void InterpretFeeEstimationInstructions(const UniValue& conf_target, cons
     } else {
         options.pushKV("fee_rate", fee_rate);
     }
-    if (!options["conf_target"].isNull() && (options["estimate_mode"].isNull() || (options["estimate_mode"].get_str() == "unset"))) {
+    if (!options["conf_target"].isNull() && (options["estimate_mode"].isNull() || (ToLower(options["estimate_mode"].get_str()) == "unset"))) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Specify estimate_mode");
     }
 }
@@ -202,7 +203,7 @@ static void SetFeeEstimateMode(const CWallet& wallet, CCoinControl& cc, const Un
         if (!conf_target.isNull()) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.");
         }
-        if (!estimate_mode.isNull() && estimate_mode.get_str() != "unset") {
+        if (!estimate_mode.isNull() && ToLower(estimate_mode.get_str()) != "unset") {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both estimate_mode and fee_rate");
         }
         // Fee rates in sat/vB cannot represent more than 3 significant digits.
diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py
index 6ce2a56bfc..6cb500350d 100755
--- a/test/functional/wallet_send.py
+++ b/test/functional/wallet_send.py
@@ -305,6 +305,30 @@ class WalletSendTest(BitcoinTestFramework):
         res = w2.walletprocesspsbt(res["psbt"])
         assert res["complete"]
 
+        # verify that fee estimation modes parse case insensitively
+        self.log.info("Testing case insensitive fee estimation mode parse")
+        for mode in ["ecoNOMICAL", "economical", "ECONOMICAL"]:
+            res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
+                estimate_mode=mode, add_to_wallet=False
+            )
+            assert_equal(res["complete"], True)
+
+            res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
+                arg_estimate_mode=mode, add_to_wallet=False
+            )
+            assert_equal(res["complete"], True)
+
+        # Verify that different variations of 'unset' still counts as
+        # not setting the estimation mode
+        for mode in ["unSET", "unset", "UNSET"]:
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
+                arg_estimate_mode=mode, arg_conf_target=1, add_to_wallet=False,  expect_error = (-8, 'Specify estimate_mode')
+            )
+
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
+                estimate_mode=mode, conf_target=1, add_to_wallet=False,  expect_error = (-8, 'Specify estimate_mode')
+            )
+
         if not self.options.descriptors:
             # Descriptor wallets do not allow mixed watch-only and non-watch-only things in the same wallet.
             # This is specifically testing that w4 ignores its own private keys and creates a psbt with send

From be8ae64b82e2c5b003a8703668ce1751442288e4 Mon Sep 17 00:00:00 2001
From: Torkel Rogstad <torkel@rogstad.io>
Date: Mon, 15 Jan 2024 08:35:45 +0100
Subject: [PATCH 2/2] rpc: validate conf_target is set alongside estimate_mode

---
 src/wallet/rpc/spend.cpp       |  6 +++++-
 test/functional/wallet_send.py | 14 ++++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 736413d482..a4f90b37ba 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -53,9 +53,13 @@ static void InterpretFeeEstimationInstructions(const UniValue& conf_target, cons
     } else {
         options.pushKV("fee_rate", fee_rate);
     }
-    if (!options["conf_target"].isNull() && (options["estimate_mode"].isNull() || (ToLower(options["estimate_mode"].get_str()) == "unset"))) {
+    auto estimate_mode_set = !options["estimate_mode"].isNull() && (ToLower(options["estimate_mode"].get_str()) != "unset");
+    if (!options["conf_target"].isNull() && !estimate_mode_set) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Specify estimate_mode");
     }
+    if (options["conf_target"].isNull() && estimate_mode_set) {
+        throw JSONRPCError(RPC_INVALID_PARAMETER, "estimate_mode should be passed with conf_target");
+    }
 }
 
 std::set<int> InterpretSubtractFeeFromOutputInstructions(const UniValue& sffo_instructions, const std::vector<std::string>& destinations)
diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py
index 6cb500350d..4064178c31 100755
--- a/test/functional/wallet_send.py
+++ b/test/functional/wallet_send.py
@@ -309,12 +309,12 @@ class WalletSendTest(BitcoinTestFramework):
         self.log.info("Testing case insensitive fee estimation mode parse")
         for mode in ["ecoNOMICAL", "economical", "ECONOMICAL"]:
             res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
-                estimate_mode=mode, add_to_wallet=False
+                estimate_mode=mode, conf_target=1, add_to_wallet=False
             )
             assert_equal(res["complete"], True)
 
             res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
-                arg_estimate_mode=mode, add_to_wallet=False
+                arg_estimate_mode=mode, arg_conf_target=1, add_to_wallet=False
             )
             assert_equal(res["complete"], True)
 
@@ -329,6 +329,16 @@ class WalletSendTest(BitcoinTestFramework):
                 estimate_mode=mode, conf_target=1, add_to_wallet=False,  expect_error = (-8, 'Specify estimate_mode')
             )
 
+        # Verify that 'estimate_mode' requires a confirmation target
+        for mode in ["ecoNOMICAL", "economical", "ECONOMICAL"]:
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
+                estimate_mode=mode, conf_target=None, add_to_wallet=False,  expect_error = (-8, 'estimate_mode should be passed with conf_target')
+            )
+
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1,
+                arg_estimate_mode=mode, arg_conf_target=None, add_to_wallet=False,  expect_error = (-8, 'estimate_mode should be passed with conf_target')
+            )
+
         if not self.options.descriptors:
             # Descriptor wallets do not allow mixed watch-only and non-watch-only things in the same wallet.
             # This is specifically testing that w4 ignores its own private keys and creates a psbt with send