Merge pull request #6555 from sellout/zip-317-legacy-wallet

Support ZIP 317 fees for legacy wallet operations

contrib/debian/examples/zcash.conf

@@ -74,7 +74,7 @@
 #rpcpassword=YourSuperGreatPasswordNumber_DO_NOT_USE_THIS_OR_YOU_WILL_GET_ROBBED_385593
 
 # How many seconds zcash will wait for a complete RPC HTTP request.
-# after the HTTP connection is established. 
+# after the HTTP connection is established.
 #rpcclienttimeout=30
 
 # By default, only RPC connections from localhost are allowed.
@@ -82,7 +82,7 @@
 # either as a single IPv4/IPv6 or with a subnet specification.
 
 # NOTE: opening up the RPC port to hosts outside your local trusted network is NOT RECOMMENDED,
-# because the rpcpassword is transmitted over the network unencrypted and also because anyone 
+# because the rpcpassword is transmitted over the network unencrypted and also because anyone
 # that can authenticate on the RPC port can steal your keys + take over the account running zcashd
 # For more information see https://github.com/zcash/zcash/issues/1497
 
@@ -114,17 +114,6 @@
 # the fee rate is applied as though it were 1000 bytes.
 #paytxfee=<amt>
 
-# If paytxfee is not set, include enough fee that transactions created by
-# legacy APIs begin confirmation on average within n blocks. This is only
-# used if there is sufficient mempool data to estimate the fee; if not,
-# the fallback fee set by mintxfee is used.
-#txconfirmtarget=2
-
-# The fallback fee rate (in ZEC per 1000 bytes) used by legacy APIs when
-# paytxfee has not been set and there is insufficient mempool data to
-# estimate a fee according to the txconfirmtarget option.
-#mintxfee=0.00001
-
 ##
 ## Miscellaneous options
 ##

doc/man/zcashd.1

@@ -309,11 +309,6 @@ Enable the Sprout to Sapling migration
 .IP
 Set the Sapling migration address
 .HP
-\fB\-mintxfee=\fR<amt>
-.IP
-Fees (in ZEC/kB) smaller than this are considered zero fee for
-transaction creation (default: 0.00001)
-.HP
 \fB\-orchardactionlimit=\fR<n>
 .IP
 Set the maximum number of Orchard actions permitted in a transaction
@@ -340,11 +335,6 @@ Send transactions as zero\-fee transactions if possible (default: 0)
 .IP
 Spend unconfirmed change when sending transactions (default: 1)
 .HP
-\fB\-txconfirmtarget=\fR<n>
-.IP
-If paytxfee is not set, include enough fee so transactions begin
-confirmation on average within n blocks (default: 2)
-.HP
 \fB\-txexpirydelta\fR
 .IP
 Set the number of blocks after which a transaction that has not been

doc/release-notes.md

@@ -25,8 +25,8 @@ RPC Changes
   while you should already be checking the async operation status, there are now
   more cases that may trigger failure at that stage.
 - The `AllowRevealedRecipients` privacy policy is now required in order to choose a
-  transparent change address for a transaction. This will only occur when the wallet 
+  transparent change address for a transaction. This will only occur when the wallet
-  is unable to construct the transaction without selecting funds from the transparent 
+  is unable to construct the transaction without selecting funds from the transparent
   pool, so the impact of this change is that for such transactions, the user must specify
   `AllowFullyTransparent`.
 - The `estimatepriority` RPC call has been removed.
@@ -59,6 +59,12 @@ Removal of Priority Estimation
   these priority estimates. It will automatically be converted to the new format
   which is not readable by prior versions of the software.
 
+Removal of obsolete config options
+----------------------------------
+
+- The fee changes for ZIP 317 have made the `-mintxfee` and `-txconfirmtarget` config options
+  obsolete. They have been removed and will now cause a warning if used.
+
 [Deprecations](https://zcash.github.io/zcash/user/deprecation.html)
 --------------
 

qa/rpc-tests/prioritisetransaction.py

@@ -12,7 +12,7 @@ from test_framework.util import (
     sync_blocks,
     sync_mempools,
 )
-from test_framework.mininode import COIN
+from test_framework.zip317 import conventional_fee_zats
 
 import time
 
@@ -43,8 +43,6 @@ class PrioritiseTransactionTest (BitcoinTestFramework):
         print("Mining 11kb blocks...")
         self.nodes[0].generate(501)
 
-        base_fee = self.nodes[0].getnetworkinfo()['relayfee']
-
         # 11 kb blocks will only hold about 50 txs, so this will fill mempool with older txs
         taddr = self.nodes[1].getnewaddress()
         for _ in range(900):
@@ -74,7 +72,7 @@ class PrioritiseTransactionTest (BitcoinTestFramework):
         print("in_block_template =", in_block_template)
         #assert_equal(in_block_template, False)
 
-        priority_success = self.nodes[0].prioritisetransaction(priority_tx_0, 0, int(3 * base_fee * COIN))
+        priority_success = self.nodes[0].prioritisetransaction(priority_tx_0, 0, conventional_fee_zats(2))
         assert(priority_success)
 
         # Check that prioritised transaction is not in getblocktemplate()
@@ -113,7 +111,7 @@ class PrioritiseTransactionTest (BitcoinTestFramework):
 
         # Node 1 doesn't get the next block, so this *shouldn't* be mined despite being prioritised on node 1
         priority_tx_1 = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
-        self.nodes[1].prioritisetransaction(priority_tx_1, 0, int(3 * base_fee * COIN))
+        self.nodes[1].prioritisetransaction(priority_tx_1, 0, conventional_fee_zats(2))
 
         # Mine block on node 0
         blk_hash = self.nodes[0].generate(1)

qa/rpc-tests/show_help.py

@@ -257,12 +257,6 @@ Wallet options:
   -migrationdestaddress=<zaddr>
        Set the Sapling migration address
 
-  -mintxfee=<amt>
-       The fallback fee rate (in ZEC per 1000 bytes) used by legacy APIs
-       (sendtoaddress, sendmany, and fundrawtransaction) when -paytxfee has not
-       been set and there is insufficient mempool data to estimate a fee
-       according to the -txconfirmtarget option (default: 0.00001)
-
   -orchardactionlimit=<n>
        Set the maximum number of Orchard actions permitted in a transaction
        (default 50)
@@ -271,9 +265,8 @@ Wallet options:
        The preferred fee rate (in ZEC per 1000 bytes) used for transactions
        created by legacy APIs (sendtoaddress, sendmany, and
        fundrawtransaction). If the transaction is less than 1000 bytes then the
-       fee rate is applied as though it were 1000 bytes. See the descriptions
+       fee rate is applied as though it were 1000 bytes. When this option is
-       of -txconfirmtarget and -mintxfee options for how the fee is calculated
+       not set, the ZIP 317 fee calculation is used.
-       when this option is not set.
 
   -rescan
        Rescan the block chain for missing wallet transactions on startup
@@ -285,13 +278,6 @@ Wallet options:
   -spendzeroconfchange
        Spend unconfirmed change when sending transactions (default: 1)
 
-  -txconfirmtarget=<n>
-       If -paytxfee is not set, include enough fee that transactions created by
-       legacy APIs (sendtoaddress, sendmany, and fundrawtransaction) begin
-       confirmation on average within n blocks. This is only used if there is
-       sufficient mempool data to estimate the fee; if not, the fallback fee
-       set by -mintxfee is used. (default: 2)
-
   -txexpirydelta
        Set the number of blocks after which a transaction that has not been
        mined will become invalid (min: 4, default: 20 (pre-Blossom) or 40

qa/rpc-tests/wallet_shieldingcoinbase.py

@@ -319,8 +319,10 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework):
         assert_equal("Amount out of range" in errorString, True)
 
         # Send will fail because fee is more than `WEIGHT_RATIO_CAP * conventional_fee`
-        myopid = self.nodes[0].z_sendmany(myzaddr, recipients, 1, (WEIGHT_RATIO_CAP * many_recipient_fee) + Decimal('0.00000001'))
+        num_fewer_recipients = 400
-        wait_and_assert_operationid_status(self.nodes[0], myopid, 'failed', 'Fee 0.40000001 is greater than 4 times the conventional fee for this tx (which is 0.10). There is no prioritisation benefit to a fee this large (see https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction), and it likely indicates a mistake in setting the fee.')
+        fewer_recipients = recipients[0:num_fewer_recipients]
+        myopid = self.nodes[0].z_sendmany(myzaddr, fewer_recipients, 1, (WEIGHT_RATIO_CAP * conventional_fee(2+num_fewer_recipients)) + Decimal('0.00000001'))
+        wait_and_assert_operationid_status(self.nodes[0], myopid, 'failed', 'Fee 0.08040001 is greater than 4 times the conventional fee for this tx (which is 0.0201). There is no prioritisation benefit to a fee this large (see https://zips.z.cash/zip-0317#recommended-algorithm-for-block-template-construction), and it likely indicates a mistake in setting the fee.')
 
         # Send will succeed because the balance of non-coinbase utxos is 10.0
         try:

src/amount.cpp

@@ -5,6 +5,7 @@
 // file COPYING or https://www.opensource.org/licenses/mit-license.php .
 
 #include "amount.h"
+#include "consensus/consensus.h"
 #include "policy/fees.h"
 
 #include "tinyformat.h"
@@ -14,10 +15,11 @@ const std::string MINOR_CURRENCY_UNIT = "zatoshis";
 
 CFeeRate::CFeeRate(const CAmount& nFeePaid, size_t nSize)
 {
-    if (nSize > 0)
+    if (nSize > 0) {
-        nSatoshisPerK = nFeePaid*1000/nSize;
+        nSatoshisPerK = std::min(nFeePaid*1000/nSize, (uint64_t)INT64_MAX / MAX_BLOCK_SIZE);
-    else
+    } else {
         nSatoshisPerK = 0;
+    }
 }
 
 CAmount CFeeRate::GetFeeForRelay(size_t nSize) const

src/main.h

@@ -76,6 +76,8 @@ static const CAmount DEFAULT_TRANSACTION_MAXFEE = 0.1 * COIN;
 static const CAmount HIGH_TX_FEE_PER_KB = 0.01 * COIN;
 /** Warn if -maxtxfee is set to a fee higher than this in zatoshis. */
 static const CAmount HIGH_MAX_TX_FEE = 100 * HIGH_TX_FEE_PER_KB;
+//! -maxtxfee will error if called with a fee that won’t allow tx to have this many actions
+static const unsigned int LOW_LOGICAL_ACTIONS = 10;
 /** Default for -maxorphantx, maximum number of orphan transactions kept in memory */
 static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100;
 /** Default for -limitancestorcount, max number of in-mempool ancestors */

src/util/moneystr.cpp

@@ -33,6 +33,10 @@ std::string FormatMoney(const CAmount& n)
     return str;
 }
 
+std::string DisplayMoney(const CAmount& zat)
+{
+    return FormatMoney(zat) + " " + CURRENCY_UNIT;
+}
 
 bool ParseMoney(const string& str, CAmount& nRet)
 {

src/util/moneystr.h

@@ -16,6 +16,9 @@
 #include "amount.h"
 
 std::string FormatMoney(const CAmount& n);
+/// Like `FormatMoney`, but meant to be human-readable, not parseable. E.g., it includes the
+/// `CURRENCY_UNIT` in the result.
+std::string DisplayMoney(const CAmount& n);
 bool ParseMoney(const std::string& str, CAmount& nRet);
 bool ParseMoney(const char* pszIn, CAmount& nRet);
 

src/wallet/asyncrpcoperation_common.cpp

@@ -179,6 +179,15 @@ void ThrowInputSelectionError(
                     WEIGHT_RATIO_CAP,
                     FormatMoney(err.conventionalFee)));
         },
+        [](const MaxFeeError& err) {
+            throw JSONRPCError(
+                RPC_INVALID_PARAMETER,
+                strprintf(
+                    "Fee (%s) is greater than the maximum fee allowed by this instance (%s). Run "
+                    "zcashd with `-maxtxfee` to adjust this limit.",
+                    DisplayMoney(err.fixedFee),
+                    DisplayMoney(maxTxFee)));
+        },
         [](const ExcessOrchardActionsError& err) {
             std::string side;
             switch (err.side) {

src/wallet/wallet.cpp

@@ -30,6 +30,7 @@
 #include "zcash/Address.hpp"
 #include "zcash/JoinSplit.hpp"
 #include "zcash/Note.hpp"
+#include "zip317.h"
 #include "crypter.h"
 #include "wallet/asyncrpcoperation_saplingmigration.h"
 
@@ -47,7 +48,6 @@ using namespace libzcash;
 CWallet* pwalletMain = NULL;
 /** Transaction fee set by the user */
 CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE);
-unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET;
 bool bSpendZeroConfChange = DEFAULT_SPEND_ZEROCONF_CHANGE;
 bool fPayAtLeastCustomFee = true;
 unsigned int nAnchorConfirmations = DEFAULT_ANCHOR_CONFIRMATIONS;
@@ -55,13 +55,6 @@ unsigned int nOrchardActionLimit = DEFAULT_ORCHARD_ACTION_LIMIT;
 
 const char * DEFAULT_WALLET_DAT = "wallet.dat";
 
-/**
- * -mintxfee: the fallback fee rate (in ZEC per 1000 bytes) used by legacy APIs
- * when -paytxfee has not been set and there is insufficient mempool data to
- * estimate a fee according to the -txconfirmtarget option.
- */
-CFeeRate CWallet::minTxFee = CFeeRate(DEFAULT_TRANSACTION_MINFEE);
-
 std::set<ReceiverType> CWallet::DefaultReceiverTypes(int nHeight) {
     // For now, just ignore the height information because the default
     // is always the same.
@@ -5710,6 +5703,15 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
 
                 unsigned int nBytes = ::GetSerializeSize(txNew, SER_NETWORK, PROTOCOL_VERSION);
 
+                // Limit size
+                if (nBytes >= max_tx_size)
+                {
+                    strFailReason = _("Transaction too large");
+                    return false;
+                }
+
+                CAmount nFeeNeeded = GetMinimumFee(txNew, nBytes);
+
                 // Remove scriptSigs if we used dummy signatures for fee calculation
                 if (!sign) {
                     for (CTxIn& vin : txNew.vin)
@@ -5719,15 +5721,6 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
                 // Embed the constructed transaction data in wtxNew.
                 *static_cast<CTransaction*>(&wtxNew) = CTransaction(txNew);
 
-                // Limit size
-                if (nBytes >= max_tx_size)
-                {
-                    strFailReason = _("Transaction too large");
-                    return false;
-                }
-
-                CAmount nFeeNeeded = GetMinimumFee(nBytes, nTxConfirmTarget, mempool);
-
                 // If we made it here and we aren't even able to meet the relay fee on the next pass, give up
                 // because we must be at the maximum allowed fee.
                 if (nFeeNeeded < ::minRelayTxFee.GetFeeForRelay(nBytes))
@@ -5800,39 +5793,22 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, std::optional<std::reference_
     return true;
 }
 
-CAmount CWallet::GetRequiredFee(unsigned int nTxBytes)
+CAmount CWallet::ConstrainFee(CAmount requestedFee, unsigned int nTxBytes)
 {
-    return std::max(minTxFee.GetFee(nTxBytes), ::minRelayTxFee.GetFeeForRelay(nTxBytes));
+    return std::min(std::max(::minRelayTxFee.GetFeeForRelay(nTxBytes), requestedFee), maxTxFee);
 }
 
-CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool)
+CAmount CWallet::GetMinimumFee(const CTransaction& tx, unsigned int nTxBytes)
 {
     // payTxFee is user-set "I want to pay this much"
-    CAmount nFeeNeeded = payTxFee.GetFee(nTxBytes);
+    CAmount nFeeNeeded = payTxFee.GetFee(std::max((unsigned int)1000, nTxBytes));
-    // TODO: fPayAtLeastCustomFee is always true so we could simplify this by saying
+    // User didn't set: use conventional fee
-    // `payTxFee.GetFee(std::max(1000, nTxBytes))` above, if we do not remove this code
-    // completely.
-    if (fPayAtLeastCustomFee && nFeeNeeded > 0 && nFeeNeeded < payTxFee.GetFeePerK())
-        nFeeNeeded = payTxFee.GetFeePerK();
-    // User didn't set: use -txconfirmtarget to estimate...
     if (nFeeNeeded == 0)
-        nFeeNeeded = pool.estimateFee(nConfirmTarget).GetFee(nTxBytes);
+        nFeeNeeded = tx.GetConventionalFee();
-    // ... unless we don't have enough mempool data, in which case fall
+    return ConstrainFee(nFeeNeeded, nTxBytes);
-    // back to the required fee
-    if (nFeeNeeded == 0)
-        nFeeNeeded = GetRequiredFee(nTxBytes);
-    // prevent user from paying a non-sense fee (like 1 satoshi): 0 < fee < minRelayFee
-    if (nFeeNeeded < ::minRelayTxFee.GetFeeForRelay(nTxBytes))
-        nFeeNeeded = ::minRelayTxFee.GetFeeForRelay(nTxBytes);
-    // But always obey the maximum
-    if (nFeeNeeded > maxTxFee)
-        nFeeNeeded = maxTxFee;
-    return nFeeNeeded;
 }
 
 
-
-
 DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
 {
     if (!fFileBacked)
@@ -6506,21 +6482,14 @@ std::string CWallet::GetWalletHelpString(bool showDebug)
     strUsage += HelpMessageOpt("-keypool=<n>", strprintf(_("Set key pool size to <n> (default: %u)"), DEFAULT_KEYPOOL_SIZE));
     strUsage += HelpMessageOpt("-migration", _("Enable the Sprout to Sapling migration"));
     strUsage += HelpMessageOpt("-migrationdestaddress=<zaddr>", _("Set the Sapling migration address"));
-    strUsage += HelpMessageOpt("-mintxfee=<amt>", strprintf(_("The fallback fee rate (in %s per 1000 bytes) used by legacy APIs (sendtoaddress, sendmany, and fundrawtransaction) when -paytxfee "
-                                                              "has not been set and there is insufficient mempool data to estimate a fee according to the -txconfirmtarget option (default: %s)"),
-                                                            CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MINFEE)));
     strUsage += HelpMessageOpt("-orchardactionlimit=<n>", strprintf(_("Set the maximum number of Orchard actions permitted in a transaction (default %u)"), DEFAULT_ORCHARD_ACTION_LIMIT));
     strUsage += HelpMessageOpt("-paytxfee=<amt>", strprintf(_("The preferred fee rate (in %s per 1000 bytes) used for transactions created by legacy APIs (sendtoaddress, sendmany, and fundrawtransaction). "
-                                                              "If the transaction is less than 1000 bytes then the fee rate is applied as though it were 1000 bytes. See the descriptions of -txconfirmtarget "
+                                                              "If the transaction is less than 1000 bytes then the fee rate is applied as though it were 1000 bytes. When this option is not set, "
-                                                              "and -mintxfee options for how the fee is calculated when this option is not set."),
+                                                              "the ZIP 317 fee calculation is used."),
                                                             CURRENCY_UNIT));
     strUsage += HelpMessageOpt("-rescan", _("Rescan the block chain for missing wallet transactions on startup"));
     strUsage += HelpMessageOpt("-salvagewallet", _("Attempt to recover private keys from a corrupt wallet on startup (implies -rescan)"));
     strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE));
-    strUsage += HelpMessageOpt("-txconfirmtarget=<n>", strprintf(_("If -paytxfee is not set, include enough fee that transactions created by legacy APIs (sendtoaddress, sendmany, and fundrawtransaction) "
-                                                                   "begin confirmation on average within n blocks. This is only used if there is sufficient mempool data to estimate the fee; if not, the "
-                                                                   "fallback fee set by -mintxfee is used. (default: %u)"),
-                                                                 DEFAULT_TX_CONFIRM_TARGET));
     strUsage += HelpMessageOpt("-txexpirydelta", strprintf(_("Set the number of blocks after which a transaction that has not been mined will become invalid (min: %u, default: %u (pre-Blossom) or %u (post-Blossom))"), TX_EXPIRING_SOON_THRESHOLD + 1, DEFAULT_PRE_BLOSSOM_TX_EXPIRY_DELTA, DEFAULT_POST_BLOSSOM_TX_EXPIRY_DELTA));
     strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format on startup"));
     strUsage += HelpMessageOpt("-wallet=<file>", _("Specify wallet file absolute path or a path relative to the data directory") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT));
@@ -6725,11 +6694,7 @@ bool CWallet::ParameterInteraction(const CChainParams& params)
 {
     if (mapArgs.count("-mintxfee"))
     {
-        CAmount n = 0;
+        UIWarning(_("The argument -mintxfee is no longer supported."));
-        if (ParseMoney(mapArgs["-mintxfee"], n) && n > 0)
-            CWallet::minTxFee = CFeeRate(n);
-        else
-            return UIError(AmountErrMsg("mintxfee", mapArgs["-mintxfee"]));
     }
     if (mapArgs.count("-paytxfee"))
     {
@@ -6753,13 +6718,15 @@ bool CWallet::ParameterInteraction(const CChainParams& params)
         if (nMaxFee > HIGH_MAX_TX_FEE)
             UIWarning(_("-maxtxfee is set to a very high fee rate! Fee rates this large could be paid on a single transaction."));
         maxTxFee = nMaxFee;
-        if (CFeeRate(maxTxFee, 1000) < ::minRelayTxFee)
+        if (maxTxFee < CalculateConventionalFee(LOW_LOGICAL_ACTIONS))
         {
-            return UIError(strprintf(_("Invalid amount for -maxtxfee=<amount>: '%s' (must be at least the minimum relay fee rate of %s to prevent stuck transactions)"),
+            return UIError(strprintf(_("Invalid amount for -maxtxfee=<amount>: '%s' (must allow for at least %d logical actions at the conventional fee)"),
-                                       mapArgs["-maxtxfee"], ::minRelayTxFee.ToString()));
+                                       mapArgs["-maxtxfee"], LOW_LOGICAL_ACTIONS));
         }
     }
-    nTxConfirmTarget = GetArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET);
+    if (mapArgs.count("-txconfirmtarget")) {
+        UIWarning(_("The argument -txconfirmtarget is no longer supported."));
+    }
     if (mapArgs.count("-txexpirydelta")) {
         int64_t expiryDelta = atoi64(mapArgs["-txexpirydelta"]);
         uint32_t minExpiryDelta = TX_EXPIRING_SOON_THRESHOLD + 1;

src/wallet/wallet.h

@@ -52,7 +52,6 @@ extern CWallet* pwalletMain;
  * Settings
  */
 extern CFeeRate payTxFee;
-extern unsigned int nTxConfirmTarget;
 extern bool bSpendZeroConfChange;
 extern bool fPayAtLeastCustomFee;
 extern unsigned int nAnchorConfirmations;
@@ -63,14 +62,10 @@ extern unsigned int nOrchardActionLimit;
 static const unsigned int DEFAULT_KEYPOOL_SIZE = 100;
 //! -paytxfee default
 static const CAmount DEFAULT_TRANSACTION_FEE = 0;
-//! -mintxfee default
-static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000;
 //! minimum change amount
 static const CAmount MIN_CHANGE = CENT;
 //! Default for -spendzeroconfchange
 static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true;
-//! -txconfirmtarget default
-static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 2;
 static const bool DEFAULT_WALLETBROADCAST = true;
 //! Size of witness cache
 //  Should be large enough that we can expect not to reorg beyond our cache
@@ -1949,17 +1944,17 @@ public:
 
     bool CommitTransaction(CWalletTx& wtxNew, std::optional<std::reference_wrapper<CReserveKey>> reservekey, CValidationState& state);
 
-    static CFeeRate minTxFee;
+    /** Adjust the requested fee by bounding it below to the minimum relay fee required
+     * for a transaction of the given size and bounding it above to the maximum fee
+     * configured using the `-maxtxfee` configuration option.
+     */
+    static CAmount ConstrainFee(CAmount requestedFee, unsigned int nTxBytes);
+
     /**
      * Estimate the minimum fee considering user set parameters
      * and the required fee.
      */
-    static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool);
+    static CAmount GetMinimumFee(const CTransaction& tx, unsigned int nTxBytes);
-    /**
-     * Return the minimum required fee taking into account the
-     * floating relay fee rate and user set minimum transaction fee rate.
-     */
-    static CAmount GetRequiredFee(unsigned int nTxBytes);
 
     /**
      * The set of default receiver types used when the wallet generates

src/wallet/wallet_tx_builder.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or https://www.opensource.org/licenses/mit-license.php .
 
+#include "util/moneystr.h"
 #include "wallet/wallet_tx_builder.h"
 #include "zip317.h"
 
@@ -149,7 +150,9 @@ WalletTxBuilder::PrepareTransaction(
         std::optional<CAmount> fee,
         uint32_t anchorConfirmations) const
 {
-    assert(fee < MAX_MONEY);
+    if (fee.has_value() && maxTxFee < fee.value()) {
+        return tl::make_unexpected(MaxFeeError(fee.value()));
+    }
 
     int anchorHeight = GetAnchorHeight(chain, anchorConfirmations);
     bool afterNU5 = params.GetConsensus().NetworkUpgradeActive(anchorHeight, Consensus::UPGRADE_NU5);
@@ -274,6 +277,19 @@ CalcZIP317Fee(
     return CalculateConventionalFee(logicalActionCount);
 }
 
+CAmount GetConstrainedFee(
+        const std::optional<SpendableInputs>& inputs,
+        const std::vector<ResolvedPayment>& payments,
+        const std::optional<ChangeAddress>& changeAddr)
+{
+    // We know that minRelayFee <= MINIMUM_FEE <= conventional_fee, so we can use an arbitrary
+    // transaction size when constraining the fee, because we are guaranteed to already satisfy the
+    // lower bound.
+    constexpr unsigned int DUMMY_TX_SIZE = 1;
+
+    return CWallet::ConstrainFee(CalcZIP317Fee(inputs, payments, changeAddr), DUMMY_TX_SIZE);
+}
+
 InvalidFundsError ReportInvalidFunds(
         const SpendableInputs& spendable,
         bool hasPhantomChange,
@@ -350,7 +366,7 @@ WalletTxBuilder::IterateLimit(
     SpendableInputs spendableMut;
 
     auto previousFee = MINIMUM_FEE;
-    auto updatedFee = CalcZIP317Fee(std::nullopt, resolved.GetResolvedPayments(), std::nullopt);
+    auto updatedFee = GetConstrainedFee(std::nullopt, resolved.GetResolvedPayments(), std::nullopt);
     // This is used to increase the target amount just enough (generally by 0 or 1) to force
     // selection of additional notes.
     CAmount bumpTargetAmount{0};
@@ -393,7 +409,7 @@ WalletTxBuilder::IterateLimit(
                 }
             }
             previousFee = updatedFee;
-            updatedFee = CalcZIP317Fee(
+            updatedFee = GetConstrainedFee(
                     spendableMut,
                     resolved.GetResolvedPayments(),
                     changeAmount > 0 ? changeAddr : std::nullopt);
@@ -937,7 +953,23 @@ TransactionBuilderResult TransactionEffects::ApproveAndBuild(
     }
 
     // Build the transaction
-    return builder.Build();
+    auto result = builder.Build();
+
+    if (result.IsTx()) {
+        auto minRelayFee =
+            ::minRelayTxFee.GetFeeForRelay(
+                    ::GetSerializeSize(result.GetTxOrThrow(), SER_NETWORK, PROTOCOL_VERSION));
+        // This should only be possible if a user has provided an explicit fee.
+        if (fee < minRelayFee) {
+            return TransactionBuilderResult(
+                    strprintf(
+                            "Fee (%s) is below the minimum relay fee for this transaction (%s)",
+                            DisplayMoney(fee),
+                            DisplayMoney(minRelayFee)));
+        }
+    }
+
+    return result;
 }
 
 // TODO: Lock Orchard notes (#6226)

src/wallet/wallet_tx_builder.h

@@ -305,6 +305,15 @@ public:
         conventionalFee(conventionalFee), fixedFee(fixedFee) { }
 };
 
+/// Error when a fee is higher than this instance allows.
+class MaxFeeError {
+public:
+    CAmount fixedFee;
+
+    MaxFeeError(CAmount fixedFee):
+        fixedFee(fixedFee) { }
+};
+
 enum ActionSide {
     Input,
     Output,
@@ -326,6 +335,7 @@ typedef std::variant<
     InvalidFundsError,
     ChangeNotAllowedError,
     AbsurdFeeError,
+    MaxFeeError,
     ExcessOrchardActionsError> InputSelectionError;
 
 class InputSelection {