From 5c759c73b2602c7fde1c50dbafe5525904c1b64c Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 20 Feb 2019 13:45:16 -0500 Subject: [PATCH] [wallet] Move maxTxFee to wallet This commit moves the maxtxfee setting to the wallet. There is only one minor behavior change: - an error message in feebumper now refers to -maxtxfee instead of maxTxFee. --- src/dummywallet.cpp | 2 +- src/init.cpp | 18 ------------------ src/interfaces/chain.cpp | 1 - src/interfaces/chain.h | 6 ------ src/interfaces/node.cpp | 1 - src/interfaces/node.h | 3 --- src/interfaces/wallet.cpp | 1 + src/interfaces/wallet.h | 3 +++ src/qt/sendcoinsdialog.cpp | 2 +- src/qt/walletmodel.cpp | 4 ++-- src/validation.cpp | 1 - src/validation.h | 8 -------- src/wallet/feebumper.cpp | 4 ++-- src/wallet/fees.cpp | 2 +- src/wallet/init.cpp | 6 ++---- src/wallet/wallet.cpp | 25 ++++++++++++++++++++++++- src/wallet/wallet.h | 8 ++++++++ 17 files changed, 45 insertions(+), 50 deletions(-) diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 8a76021a5bff3..f5141e4962ad1 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -24,7 +24,7 @@ class DummyWalletInit : public WalletInitInterface { void DummyWalletInit::AddWalletOptions() const { std::vector opts = {"-addresstype", "-changetype", "-disablewallet", "-discardfee=", "-fallbackfee=", - "-keypool=", "-mintxfee=", "-paytxfee=", "-rescan", "-salvagewallet", "-spendzeroconfchange", "-txconfirmtarget=", + "-keypool=", "-maxtxfee=", "-mintxfee=", "-paytxfee=", "-rescan", "-salvagewallet", "-spendzeroconfchange", "-txconfirmtarget=", "-upgradewallet", "-wallet=", "-walletbroadcast", "-walletdir=", "-walletnotify=", "-walletrbf", "-zapwallettxes=", "-dblogsize=", "-flushwallet", "-privdb", "-walletrejectlongchains"}; gArgs.AddHiddenArgs(opts); diff --git a/src/init.cpp b/src/init.cpp index 2b538fb83630a..5b194add124d1 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -502,8 +502,6 @@ void SetupServerArgs() gArgs.AddArg("-mocktime=", "Replace actual time with seconds since epoch (default: 0)", true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-maxsigcachesize=", strprintf("Limit sum of signature cache and script execution cache sizes to MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_SIZE), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-maxtipage=", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", DEFAULT_MAX_TIP_AGE), true, OptionsCategory::DEBUG_TEST); - gArgs.AddArg("-maxtxfee=", strprintf("Maximum total fees (in %s) to use in a single wallet transaction; setting this too low may abort large transactions (default: %s)", // TODO move setting to wallet - CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -daemon. To disable logging to file, set -nodebuglogfile)", false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-shrinkdebugfile", "Shrink debug.log file on client startup (default: 1 when no -debug)", false, OptionsCategory::DEBUG_TEST); @@ -1123,22 +1121,6 @@ bool AppInitParameterInteraction() dustRelayFee = CFeeRate(n); } - // This is required by both the wallet and node - if (gArgs.IsArgSet("-maxtxfee")) - { - CAmount nMaxFee = 0; - if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee)) - return InitError(AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", ""))); - if (nMaxFee > HIGH_MAX_TX_FEE) - InitWarning(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.")); - maxTxFee = nMaxFee; - if (CFeeRate(maxTxFee, 1000) < ::minRelayTxFee) - { - return InitError(strprintf(_("Invalid amount for -maxtxfee=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), - gArgs.GetArg("-maxtxfee", ""), ::minRelayTxFee.ToString())); - } - } - fRequireStandard = !gArgs.GetBoolArg("-acceptnonstdtxn", !chainparams.RequireStandard()); if (chainparams.RequireStandard() && !fRequireStandard) return InitError(strprintf("acceptnonstdtxn is not currently supported for %s chain", chainparams.NetworkIDString())); diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index b61a51b2351ef..16e0d1d6c0d03 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -339,7 +339,6 @@ class ChainImpl : public Chain CFeeRate relayMinFee() override { return ::minRelayTxFee; } CFeeRate relayIncrementalFee() override { return ::incrementalRelayFee; } CFeeRate relayDustFee() override { return ::dustRelayFee; } - CAmount maxTxFee() override { return ::maxTxFee; } bool getPruneMode() override { return ::fPruneMode; } bool p2pEnabled() override { return g_connman != nullptr; } bool isReadyToBroadcast() override { return !::fImporting && !::fReindex && !IsInitialBlockDownload(); } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 17d7b6d8f1544..d62ee683fa713 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -216,12 +216,6 @@ class Chain //! Relay dust fee setting (-dustrelayfee), reflecting lowest rate it's economical to spend. virtual CFeeRate relayDustFee() = 0; - //! Node max tx fee setting (-maxtxfee). - //! This could be replaced by a per-wallet max fee, as proposed at - //! https://github.com/bitcoin/bitcoin/issues/15355 - //! But for the time being, wallets call this to access the node setting. - virtual CAmount maxTxFee() = 0; - //! Check if pruning is enabled. virtual bool getPruneMode() = 0; diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 73a507413324e..f3ee8fe364239 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -207,7 +207,6 @@ class NodeImpl : public Node } } bool getNetworkActive() override { return g_connman && g_connman->GetNetworkActive(); } - CAmount getMaxTxFee() override { return ::maxTxFee; } CFeeRate estimateSmartFee(int num_blocks, bool conservative, int* returned_target = nullptr) override { FeeCalculation fee_calc; diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 76b93af234b8c..1ccd2a31b72ae 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -159,9 +159,6 @@ class Node //! Get network active. virtual bool getNetworkActive() = 0; - //! Get max tx fee. - virtual CAmount getMaxTxFee() = 0; - //! Estimate smart fee. virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, int* returned_target = nullptr) = 0; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index f98a49f9b11f5..e5aee2d806f6d 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -464,6 +464,7 @@ class WalletImpl : public Wallet bool IsWalletFlagSet(uint64_t flag) override { return m_wallet->IsWalletFlagSet(flag); } OutputType getDefaultAddressType() override { return m_wallet->m_default_address_type; } OutputType getDefaultChangeType() override { return m_wallet->m_default_change_type; } + CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; } void remove() override { RemoveWallet(m_wallet); diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index cabc455b1f461..7096f5404724a 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -247,6 +247,9 @@ class Wallet // Get default change type. virtual OutputType getDefaultChangeType() = 0; + //! Get max tx fee. + virtual CAmount getDefaultMaxTxFee() = 0; + // Remove wallet. virtual void remove() = 0; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 6e00ab755c4e9..8a0b265834858 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -578,7 +578,7 @@ void SendCoinsDialog::processSendCoinsReturn(const WalletModel::SendCoinsReturn msgParams.second = CClientUIInterface::MSG_ERROR; break; case WalletModel::AbsurdFee: - msgParams.first = tr("A fee higher than %1 is considered an absurdly high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), model->node().getMaxTxFee())); + msgParams.first = tr("A fee higher than %1 is considered an absurdly high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), model->wallet().getDefaultMaxTxFee())); break; case WalletModel::PaymentRequestExpired: msgParams.first = tr("Payment request expired."); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index f4f3be8f435c5..fd392b7cf7f50 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -222,9 +222,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact } // reject absurdly high fee. (This can never happen because the - // wallet caps the fee at maxTxFee. This merely serves as a + // wallet caps the fee at m_default_max_tx_fee. This merely serves as a // belt-and-suspenders check) - if (nFeeRequired > m_node.getMaxTxFee()) + if (nFeeRequired > m_wallet->getDefaultMaxTxFee()) return AbsurdFee; } diff --git a/src/validation.cpp b/src/validation.cpp index be6257ea2820a..b8f3ee464ee99 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -252,7 +252,6 @@ uint256 hashAssumeValid; arith_uint256 nMinimumChainWork; CFeeRate minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE); -CAmount maxTxFee = DEFAULT_TRANSACTION_MAXFEE; CBlockPolicyEstimator feeEstimator; CTxMemPool mempool(&feeEstimator); diff --git a/src/validation.h b/src/validation.h index 9f00cab49ec15..fecff5bb80a4c 100644 --- a/src/validation.h +++ b/src/validation.h @@ -53,12 +53,6 @@ static const bool DEFAULT_WHITELISTRELAY = true; static const bool DEFAULT_WHITELISTFORCERELAY = false; /** Default for -minrelaytxfee, minimum relay fee for transactions */ static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 1000; -//! -maxtxfee default -static const CAmount DEFAULT_TRANSACTION_MAXFEE = COIN / 10; -//! Discourage users to set fees higher than this amount (in satoshis) per kB -static const CAmount HIGH_TX_FEE_PER_KB = COIN / 100; -//! -maxtxfee will warn if called with a higher fee than this amount (in satoshis) -static const CAmount HIGH_MAX_TX_FEE = 100 * HIGH_TX_FEE_PER_KB; /** Default for -limitancestorcount, max number of in-mempool ancestors */ static const unsigned int DEFAULT_ANCESTOR_LIMIT = 25; /** Default for -limitancestorsize, maximum kilobytes of tx + all in-mempool ancestors */ @@ -162,8 +156,6 @@ extern bool fCheckpointsEnabled; extern size_t nCoinCacheUsage; /** A fee rate smaller than this is considered zero fee (for relaying, mining and transaction creation) */ extern CFeeRate minRelayTxFee; -/** Absolute maximum transaction fee (in satoshis) used by wallet and mempool (rejects high fee in sendrawtransaction) */ -extern CAmount maxTxFee; /** If the tip is older than this (in seconds), the node is considered to be in initial block download. */ extern int64_t nMaxTipAge; extern bool fEnableReplacement; diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index b547ea21d94a0..7ffea3867d2fb 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -162,9 +162,9 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin } // Check that in all cases the new fee doesn't violate maxTxFee - const CAmount max_tx_fee = wallet->chain().maxTxFee(); + const CAmount max_tx_fee = wallet->m_default_max_tx_fee; if (new_fee > max_tx_fee) { - errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)", + errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)", FormatMoney(new_fee), FormatMoney(max_tx_fee))); return Result::WALLET_ERROR; } diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 560c86a70afb3..d9ae18ed60259 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -22,7 +22,7 @@ CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinC { CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes); // Always obey the maximum - const CAmount max_tx_fee = wallet.chain().maxTxFee(); + const CAmount max_tx_fee = wallet.m_default_max_tx_fee; if (fee_needed > max_tx_fee) { fee_needed = max_tx_fee; if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index e7eea94e06c6a..47ef01bfd180b 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -48,6 +48,8 @@ void WalletInit::AddWalletOptions() const gArgs.AddArg("-fallbackfee=", strprintf("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_FALLBACK_FEE)), false, OptionsCategory::WALLET); gArgs.AddArg("-keypool=", strprintf("Set key pool size to (default: %u)", DEFAULT_KEYPOOL_SIZE), false, OptionsCategory::WALLET); + gArgs.AddArg("-maxtxfee=", strprintf("Maximum total fees (in %s) to use in a single wallet transaction; setting this too low may abort large transactions (default: %s)", + CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), false, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-mintxfee=", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for transaction creation (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MINFEE)), false, OptionsCategory::WALLET); gArgs.AddArg("-paytxfee=", strprintf("Fee (in %s/kB) to add to transactions you send (default: %s)", @@ -124,10 +126,6 @@ bool WalletInit::ParameterInteraction() const if (gArgs.GetArg("-prune", 0) && gArgs.GetBoolArg("-rescan", false)) return InitError(_("Rescans are not possible in pruned mode. You will need to use -reindex which will download the whole blockchain again.")); - if (::minRelayTxFee.GetFeePerK() > HIGH_TX_FEE_PER_KB) - InitWarning(AmountHighWarn("-minrelaytxfee") + " " + - _("The wallet will avoid paying less than the minimum relay fee.")); - return true; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9ee6329f8cc40..8a1cb83e74f75 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4204,6 +4204,29 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, return nullptr; } } + + if (gArgs.IsArgSet("-maxtxfee")) + { + CAmount nMaxFee = 0; + if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee)) { + chain.initError(AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", ""))); + return nullptr; + } + if (nMaxFee > HIGH_MAX_TX_FEE) { + chain.initWarning(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.")); + } + if (CFeeRate(nMaxFee, 1000) < chain.relayMinFee()) { + chain.initError(strprintf(_("Invalid amount for -maxtxfee=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), + gArgs.GetArg("-maxtxfee", ""), chain.relayMinFee().ToString())); + return nullptr; + } + walletInstance->m_default_max_tx_fee = nMaxFee; + } + + if (chain.relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) + chain.initWarning(AmountHighWarn("-minrelaytxfee") + " " + + _("The wallet will avoid paying less than the minimum relay fee.")); + walletInstance->m_confirm_target = gArgs.GetArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET); walletInstance->m_spend_zero_conf_change = gArgs.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); walletInstance->m_signal_rbf = gArgs.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); @@ -4394,7 +4417,7 @@ bool CWalletTx::AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, CValid // user could call sendmoney in a loop and hit spurious out of funds errors // because we think that this newly generated transaction's change is // unavailable as we're not yet aware that it is in the mempool. - bool ret = locked_chain.submitToMemoryPool(tx, pwallet->chain().maxTxFee(), state); + bool ret = locked_chain.submitToMemoryPool(tx, pwallet->m_default_max_tx_fee, state); fInMempool |= ret; return ret; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4cc6973802c9d..008765bccc02c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -73,6 +73,12 @@ static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6; static const bool DEFAULT_WALLET_RBF = false; static const bool DEFAULT_WALLETBROADCAST = true; static const bool DEFAULT_DISABLE_WALLET = false; +//! -maxtxfee default +constexpr CAmount DEFAULT_TRANSACTION_MAXFEE{COIN / 10}; +//! Discourage users to set fees higher than this amount (in satoshis) per kB +constexpr CAmount HIGH_TX_FEE_PER_KB{COIN / 100}; +//! -maxtxfee will warn if called with a higher fee than this amount (in satoshis) +constexpr CAmount HIGH_MAX_TX_FEE{100 * HIGH_TX_FEE_PER_KB}; //! Pre-calculated constants for input size estimation in *virtual size* static constexpr size_t DUMMY_NESTED_P2WPKH_INPUT_SIZE = 91; @@ -983,6 +989,8 @@ class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notific CFeeRate m_discard_rate{DEFAULT_DISCARD_FEE}; OutputType m_default_address_type{DEFAULT_ADDRESS_TYPE}; OutputType m_default_change_type{DEFAULT_CHANGE_TYPE}; + /** Absolute maximum transaction fee (in satoshis) used by default for the wallet */ + CAmount m_default_max_tx_fee{DEFAULT_TRANSACTION_MAXFEE}; bool NewKeyPool(); size_t KeypoolCountExternalKeys() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);