Merge #16414: 0.18: wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction

d3b3bb8c9f 0.18: test: Add test for maxtxfee option (MarcoFalke)
a11dbaa547 0.18: wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa)
8f354ced6e 0.18: [wallet] abort when attempting to fund a transaction above maxtxfee (Sjors Provoost)

Pull request description:

  Backports #16322 and #16257.

ACKs for top commit:
  Sjors:
    re-ACK d3b3bb8c9f
  MarcoFalke:
    ACK d3b3bb8c9f (did the backport myself, arrived at the same result)

Tree-SHA512: 8b0ca15fc7e893af80239afecf8ff1018d6f249f2fa530babe61ec34ede6103b9b60909259abb730ebf1d54789aceed94b136600158dc3d6c5505b07f28189e5

doc/release-notes-0.18.1-16257.md

@@ -0,0 +1,6 @@
+Wallet changes
+--------------
+When creating a transaction with a fee above `-maxtxfee` (default 0.1 BTC),
+the RPC commands `walletcreatefundedpsbt` and  `fundrawtransaction` will now fail
+instead of rounding down the fee. Beware that the `feeRate` argument is specified
+in BTC per kilobyte, not satoshi per byte.

src/policy/fees.cpp

@@ -38,7 +38,6 @@ std::string StringForFeeReason(FeeReason reason) {
         {FeeReason::PAYTXFEE, "PayTxFee set"},
         {FeeReason::FALLBACK, "Fallback fee"},
         {FeeReason::REQUIRED, "Minimum Required Fee"},
-        {FeeReason::MAXTXFEE, "MaxTxFee limit"}
     };
     auto reason_string = fee_reason_strings.find(reason);
 

src/policy/fees.h

@@ -43,7 +43,6 @@ enum class FeeReason {
     PAYTXFEE,
     FALLBACK,
     REQUIRED,
-    MAXTXFEE,
 };
 
 std::string StringForFeeReason(FeeReason reason);

src/wallet/fees.cpp

@@ -21,13 +21,7 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes)
 
 CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc)
 {
-    CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, pool, estimator, feeCalc).GetFee(nTxBytes);
-    // Always obey the maximum
-    if (fee_needed > maxTxFee) {
-        fee_needed = maxTxFee;
-        if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE;
-    }
-    return fee_needed;
+    return GetMinimumFeeRate(wallet, coin_control, pool, estimator, feeCalc).GetFee(nTxBytes);
 }
 
 CFeeRate GetRequiredFeeRate(const CWallet& wallet)

src/wallet/wallet.cpp

@@ -3126,6 +3126,11 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
         }
     }
 
+    if (nFeeRet > maxTxFee) {
+        strFailReason = _("Fee exceeds maximum configured by -maxtxfee");
+        return false;
+    }
+
     if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) {
         // Lastly, ensure this tx will pass the mempool's chain limits
         LockPoints lp;

test/functional/rpc_fundrawtransaction.py

@@ -662,6 +662,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         result = self.nodes[3].fundrawtransaction(rawtx) # uses min_relay_tx_fee (set by settxfee)
         result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2*min_relay_tx_fee})
         result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10*min_relay_tx_fee})
+        assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[3].fundrawtransaction, rawtx, {"feeRate": 1})
         result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex'])
         assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate)
         assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate)

test/functional/rpc_psbt.py

@@ -128,6 +128,15 @@ class PSBTTest(BitcoinTestFramework):
         assert_equal(walletprocesspsbt_out['complete'], True)
         self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex'])
 
+        # feeRate of 0.1 BTC / KB produces a total fee slightly below -maxtxfee (~0.05280000):
+        res = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 0.1})
+        assert_greater_than(res["fee"], 0.05)
+        assert_greater_than(0.06, res["fee"])
+
+        # feeRate of 10 BTC / KB produces a total fee well above -maxtxfee
+        # previously this was silently capped at -maxtxfee
+        assert_raises_rpc_error(-4, "Fee exceeds maximum configured by -maxtxfee", self.nodes[1].walletcreatefundedpsbt, [{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99}, 0, {"feeRate": 10})
+
         # partially sign multisig things with node 1
         psbtx = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wsh_pos},{"txid":txid,"vout":p2sh_pos},{"txid":txid,"vout":p2sh_p2wsh_pos}], {self.nodes[1].getnewaddress():29.99})['psbt']
         walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(psbtx)

test/functional/wallet_bumpfee.py

@@ -73,6 +73,7 @@ class BumpFeeTest(BitcoinTestFramework):
         test_unconfirmed_not_spendable(rbf_node, rbf_node_address)
         test_bumpfee_metadata(rbf_node, dest_address)
         test_locked_wallet_fails(rbf_node, dest_address)
+        test_maxtxfee_fails(self, rbf_node, dest_address)
         self.log.info("Success")
 
 
@@ -204,6 +205,15 @@ def test_settxfee(rbf_node, dest_address):
     rbf_node.settxfee(Decimal("0.00000000"))  # unset paytxfee
 
 
+def test_maxtxfee_fails(test, rbf_node, dest_address):
+    test.restart_node(1, ['-maxtxfee=0.00003'] + test.extra_args[1])
+    rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
+    rbfid = spend_one_input(rbf_node, dest_address)
+    assert_raises_rpc_error(-4, "Specified or calculated fee 0.0000332 is too high (cannot be higher than maxTxFee 0.00003)", rbf_node.bumpfee, rbfid)
+    test.restart_node(1, test.extra_args[1])
+    rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
+
+
 def test_rebumping(rbf_node, dest_address):
     # check that re-bumping the original tx fails, but bumping the bumper succeeds
     rbfid = spend_one_input(rbf_node, dest_address)

test/functional/wallet_create_tx.py

@@ -6,6 +6,7 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
+    assert_raises_rpc_error,
 )
 from test_framework.blocktools import (
     TIME_GENESIS_BLOCK,
@@ -26,6 +27,10 @@ class CreateTxWalletTest(BitcoinTestFramework):
         self.nodes[0].generate(200)
         self.nodes[0].setmocktime(0)
 
+        self.test_anti_fee_sniping()
+        self.test_tx_size_too_large()
+
+    def test_anti_fee_sniping(self):
         self.log.info('Check that we have some (old) blocks and that anti-fee-sniping is disabled')
         assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 200)
         txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
@@ -38,6 +43,40 @@ class CreateTxWalletTest(BitcoinTestFramework):
         tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex'])
         assert 0 < tx['locktime'] <= 201
 
+    def test_tx_size_too_large(self):
+        # More than 10kB of outputs, so that we hit -maxtxfee with a high feerate
+        outputs = {self.nodes[0].getnewaddress(address_type='bech32'): 0.000025 for i in range(400)}
+        raw_tx = self.nodes[0].createrawtransaction(inputs=[], outputs=outputs)
+
+        for fee_setting in ['-minrelaytxfee=0.01', '-mintxfee=0.01', '-paytxfee=0.01']:
+            self.log.info('Check maxtxfee in combination with {}'.format(fee_setting))
+            self.restart_node(0, extra_args=[fee_setting])
+            assert_raises_rpc_error(
+                -6,
+                "Fee exceeds maximum configured by -maxtxfee",
+                lambda: self.nodes[0].sendmany(dummy="", amounts=outputs),
+            )
+            assert_raises_rpc_error(
+                -4,
+                "Fee exceeds maximum configured by -maxtxfee",
+                lambda: self.nodes[0].fundrawtransaction(hexstring=raw_tx),
+            )
+
+        self.log.info('Check maxtxfee in combination with settxfee')
+        self.restart_node(0)
+        self.nodes[0].settxfee(0.01)
+        assert_raises_rpc_error(
+            -6,
+            "Fee exceeds maximum configured by -maxtxfee",
+            lambda: self.nodes[0].sendmany(dummy="", amounts=outputs),
+        )
+        assert_raises_rpc_error(
+            -4,
+            "Fee exceeds maximum configured by -maxtxfee",
+            lambda: self.nodes[0].fundrawtransaction(hexstring=raw_tx),
+        )
+        self.nodes[0].settxfee(0)
+
 
 if __name__ == '__main__':
     CreateTxWalletTest().main()