Merge #20220: wallet, rpc: explicit fee rate follow-ups/fixes for 0.21

0be29000c011dec0722481dbebb159873da6fa54 rpc: update conf_target helps for correctness/consistency (Jon Atack)
778b9be40667876c705e586849ea9c9e44cf451c wallet, rpc: fix send subtract_fee_from_outputs help (Jon Atack)
603c0050837ec65765208dd54dde354145fbe098 wallet: add rpc send explicit fee rate coverage (Jon Atack)
dd341e602d5160fc621c0299179b91403756b61d wallet: add sendtoaddress/sendmany explicit fee rate coverage (Jon Atack)
44e7bfa60313e4ae67da49e5ba4535038b71b453 wallet: add walletcreatefundedpsbt explicit fee rate coverage (Jon Atack)
6e1ea4273e52fdcd86c87628aa595c03a071ca8c test: refactor for walletcreatefundedpsbt fee rate coverage (Jon Atack)
3ac7b0c6f1c68e74a84d868a454f508bada6b09d wallet: fundrawtx fee rate coverage, fixup ParseConfirmTarget() (Jon Atack)
2d8eba8f8425a2515022d51f1f5b4911329fbf55 wallet: combine redundant bumpfee invalid params and args tests (Jon Atack)
1697a40b6f841a54ee0d9744ed7fd09034b0ddad wallet: improve bumpfee error/help, add explicit fee rate coverage (Jon Atack)
fc5721723d34f76f9e1ffd2e31f274ea6b22f894 wallet: fix SetFeeEstimateMode() error message (Jon Atack)
052427eef1c9da84c474c5161b1910d3328ef0da wallet, bugfix: fix bumpfee with explicit fee rate modes (Jon Atack)

Pull request description:

  Follow-up to #11413 providing a base to build on for #19543:

  - bugfix for `bumpfee` raising a JSON error with explicit feerates, fixes issue #20219
  - adds explicit feerate test coverage for `bumpfee`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendtoaddress`, and `sendmany`
  - improves a few related RPC error messages and `ParseConfirmTarget()` / error message
  - fixes/improves the explicit fee rate information in the 6 RPC helps, of which 2 were also missing `conf_target` sat/B units

  This provides a spec and regression coverage for the potential next step of a universal `sat/vB` feerate argument (see #19543), as well as immediate coverage and minimum fixes for 0.21.

ACKs for top commit:
  kallewoof:
    Concept/Tested ACK 0be29000c011dec0722481dbebb159873da6fa54
  meshcollider:
    Code review + functional test run ACK 0be29000c011dec0722481dbebb159873da6fa54

Tree-SHA512: efd965003e991cba51d4504e2940f06ab3d742e34022e96a673606b44fad85596aa03a8c1809f06df7ebcf21a38e18a891e54392fe3d6fb4d120bbe4ea0cf5e0

1 files changed, 58 insertions, 12 deletions

diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py
index 60ab2d457e..921a726d4b 100755
--- a/test/functional/wallet_send.py
+++ b/test/functional/wallet_send.py
@@ -97,6 +97,8 @@ class WalletSendTest(BitcoinTestFramework):
             except AssertionError:
                 # Provide debug info if the test fails
                 self.log.error("Unexpected successful result:")
+                self.log.error(arg_conf_target)
+                self.log.error(arg_estimate_mode)
                 self.log.error(options)
                 res = from_wallet.send(outputs=outputs, conf_target=arg_conf_target, estimate_mode=arg_estimate_mode, options=options)
                 self.log.error(res)
@@ -224,8 +226,10 @@ class WalletSendTest(BitcoinTestFramework):
         assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"],
                      self.nodes[1].decodepsbt(res2["psbt"])["fee"])
         # but not at the same time
-        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical",
-                       conf_target=1, estimate_mode="economical", add_to_wallet=False, expect_error=(-8,"Use either conf_target and estimate_mode or the options dictionary to control fee rate"))
+        for mode in ["unset", "economical", "conservative", "btc/kb", "sat/b"]:
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical",
+                conf_target=1, estimate_mode=mode, add_to_wallet=False,
+                expect_error=(-8, "Use either conf_target and estimate_mode or the options dictionary to control fee rate"))
 
         self.log.info("Create PSBT from watch-only wallet w3, sign with w2...")
         res = self.test_send(from_wallet=w3, to_wallet=w1, amount=1)
@@ -246,19 +250,61 @@ class WalletSendTest(BitcoinTestFramework):
         res = w2.walletprocesspsbt(res["psbt"])
         assert res["complete"]
 
-        self.log.info("Set fee rate...")
+        self.log.info("Test setting explicit fee rate")
+        res1 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=1, arg_estimate_mode="economical", add_to_wallet=False)
+        res2 = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=1, estimate_mode="economical", add_to_wallet=False)
+        assert_equal(self.nodes[1].decodepsbt(res1["psbt"])["fee"], self.nodes[1].decodepsbt(res2["psbt"])["fee"])
+
+        res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.00007, estimate_mode="btc/kb", add_to_wallet=False)
+        fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
+        assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00007"))
+
         res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=2, estimate_mode="sat/b", add_to_wallet=False)
         fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
         assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00002"))
-        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=-1, estimate_mode="sat/b",
-                       expect_error=(-3, "Amount out of range"))
-        # Fee rate of 0.1 satoshi per byte should throw an error
-        # TODO: error should use sat/b
-        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode="sat/b",
-                       expect_error=(-4, "Fee rate (0.00000100 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"))
-
-        self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.000001, estimate_mode="BTC/KB",
-                       expect_error=(-4, "Fee rate (0.00000100 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"))
+
+        res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=0.00004531, arg_estimate_mode="btc/kb", add_to_wallet=False)
+        fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
+        assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00004531"))
+
+        res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=3, arg_estimate_mode="sat/b", add_to_wallet=False)
+        fee = self.nodes[1].decodepsbt(res["psbt"])["fee"]
+        assert_fee_amount(fee, Decimal(len(res["hex"]) / 2), Decimal("0.00003"))
+
+        # TODO: This test should pass with all modes, e.g. with the next line uncommented, for consistency with the other explicit feerate RPCs.
+        # for mode in ["unset", "economical", "conservative", "btc/kb", "sat/b"]:
+        for mode in ["btc/kb", "sat/b"]:
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=-1, estimate_mode=mode,
+                expect_error=(-3, "Amount out of range"))
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0, estimate_mode=mode,
+                expect_error=(-4, "Fee rate (0.00000000 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"))
+
+        for mode in ["foo", Decimal("3.141592")]:
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode=mode,
+                expect_error=(-8, "Invalid estimate_mode parameter"))
+            # TODO: these 2 equivalent sends with an invalid estimate_mode arg should both fail, but they do not...why?
+            # self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_conf_target=0.1, arg_estimate_mode=mode,
+            #     expect_error=(-8, "Invalid estimate_mode parameter"))
+            # assert_raises_rpc_error(-8, "Invalid estimate_mode parameter", lambda: w0.send({w1.getnewaddress(): 1}, 0.1, mode))
+
+        # TODO: These tests should pass for consistency with the other explicit feerate RPCs, but they do not.
+        # for mode in ["unset", "economical", "conservative", "btc/kb", "sat/b"]:
+        #     self.log.debug("{}".format(mode))
+        #     for k, v in {"string": "", "object": {"foo": "bar"}}.items():
+        #         self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=v, estimate_mode=mode,
+        #             expect_error=(-3, "Expected type number for conf_target, got {}".format(k)))
+
+        # TODO: error should use sat/B instead of BTC/kB if sat/B is selected.
+        # Test setting explicit fee rate just below the minimum.
+        for unit, fee_rate in {"sat/B": 0.99999999, "BTC/kB": 0.00000999}.items():
+            self.log.info("Explicit fee rate raises RPC error 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit))
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=fee_rate, estimate_mode=unit,
+                expect_error=(-4, "Fee rate (0.00000999 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"))
+
+        self.log.info("Explicit fee rate raises RPC error if estimate_mode is passed without a conf_target")
+        for unit, fee_rate in {"sat/B": 100, "BTC/kB": 0.001}.items():
+            self.test_send(from_wallet=w0, to_wallet=w1, amount=1, estimate_mode=unit,
+                expect_error=(-8, "Selected estimate_mode {} requires a fee rate to be specified in conf_target".format(unit)))
 
         # TODO: Return hex if fee rate is below -maxmempool
         # res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode="sat/b", add_to_wallet=False)