From 77d4c515071cae0f804b786d405773912518ff94 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sun, 12 Dec 2021 19:08:12 +0530 Subject: [PATCH] merge bitcoin#15778: Move maxtxfee from node to wallet --- src/coinjoin/coinjoin.cpp | 4 +++- src/coinjoin/server.cpp | 3 ++- src/coinjoin/util.cpp | 4 ++-- src/dummywallet.cpp | 1 + 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/fees.cpp | 2 +- src/wallet/init.cpp | 6 ++---- src/wallet/wallet.cpp | 27 +++++++++++++++++++++++++-- src/wallet/wallet.h | 8 ++++++++ 19 files changed, 51 insertions(+), 52 deletions(-) diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index de7bac827b..4660fe1884 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -22,6 +22,8 @@ #include +constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10}; + bool CCoinJoinEntry::AddScriptSig(const CTxIn& txin) { for (auto& txdsin : vecTxDSIn) { @@ -370,7 +372,7 @@ bool CCoinJoin::IsCollateralValid(const CTransaction& txCollateral) { LOCK(cs_main); CValidationState validationState; - if (!AcceptToMemoryPool(mempool, validationState, MakeTransactionRef(txCollateral), nullptr /* pfMissingInputs */, false /* bypass_limits */, maxTxFee /* nAbsurdFee */, true /* fDryRun */)) { + if (!AcceptToMemoryPool(mempool, validationState, MakeTransactionRef(txCollateral), nullptr /* pfMissingInputs */, false /* bypass_limits */, DEFAULT_MAX_RAW_TX_FEE /* nAbsurdFee */, true /* fDryRun */)) { LogPrint(BCLog::COINJOIN, "CCoinJoin::IsCollateralValid -- didn't pass AcceptToMemoryPool()\n"); return false; } diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 419a048764..40497a2c13 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -23,6 +23,7 @@ #include CCoinJoinServer coinJoinServer; +constexpr static CAmount DEFAULT_MAX_RAW_TX_FEE{COIN / 10}; void CCoinJoinServer::ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, CConnman& connman, bool enable_bip61) { @@ -341,7 +342,7 @@ void CCoinJoinServer::CommitFinalTransaction(CConnman& connman) TRY_LOCK(cs_main, lockMain); CValidationState validationState; mempool.PrioritiseTransaction(hashTx, 0.1 * COIN); - if (!lockMain || !AcceptToMemoryPool(mempool, validationState, finalTransaction, nullptr /* pfMissingInputs */, false /* bypass_limits */, maxTxFee /* nAbsurdFee */)) { + if (!lockMain || !AcceptToMemoryPool(mempool, validationState, finalTransaction, nullptr /* pfMissingInputs */, false /* bypass_limits */, DEFAULT_MAX_RAW_TX_FEE /* nAbsurdFee */)) { LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- AcceptToMemoryPool() error: Transaction not valid\n"); SetNull(); // not much we can do in this case, just notify clients diff --git a/src/coinjoin/util.cpp b/src/coinjoin/util.cpp index 507019c84d..7ab33f5768 100644 --- a/src/coinjoin/util.cpp +++ b/src/coinjoin/util.cpp @@ -236,8 +236,8 @@ CAmount CTransactionBuilder::GetFee(unsigned int nBytes) const if (nRequiredFee > nFeeCalc) { nFeeCalc = nRequiredFee; } - if (nFeeCalc > ::maxTxFee) { - nFeeCalc = ::maxTxFee; + if (nFeeCalc > pwallet->m_default_max_tx_fee) { + nFeeCalc = pwallet->m_default_max_tx_fee; } return nFeeCalc; } diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp index 3e52120005..0aaea5d596 100644 --- a/src/dummywallet.cpp +++ b/src/dummywallet.cpp @@ -37,6 +37,7 @@ void DummyWalletInit::AddWalletOptions() const "-disablewallet", "-instantsendnotify=", "-keypool=", + "-maxtxfee=", "-rescan=", "-salvagewallet", "-spendzeroconfchange", diff --git a/src/init.cpp b/src/init.cpp index 951bf81c36..dac1254d3e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -685,8 +685,6 @@ void SetupServerArgs() gArgs.AddArg("-maxsigcachesize=", strprintf("Limit sum of signature cache and script execution cache sizes to MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_SIZE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, 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), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-mocktime=", "Replace actual time with seconds since epoch (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, 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)", - CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); // TODO move setting to wallet gArgs.AddArg("-minsporkkeys=", "Overrides minimum spork signers to change spork value. Only useful for regtest and devnet. Using this on mainnet or testnet will ban you.", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -daemon. To disable logging to file, set -nodebuglogfile)", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); @@ -1496,22 +1494,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 53e7346cb0..0354a5c1ce 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -356,7 +356,6 @@ public: 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 && !::ChainstateActive().IsInitialBlockDownload(); } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 72b431be68..2c125081d9 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -211,12 +211,6 @@ public: //! 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 5ae996d778..787cdd35f7 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -344,7 +344,6 @@ public: } } 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 ec6a41f068..9c892708d2 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -229,9 +229,6 @@ public: //! 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 bb49f9bf73..4419cca9e8 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -544,6 +544,7 @@ public: bool hdEnabled() override { return m_wallet->IsHDEnabled(); } bool IsWalletFlagSet(uint64_t flag) override { return m_wallet->IsWalletFlagSet(flag); } CoinJoin::Client& coinJoin() override { return m_coinjoin; } + 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 6eff7991c1..4a538bf59a 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -266,6 +266,9 @@ public: virtual CoinJoin::Client& coinJoin() = 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 7dc2658b9c..2179d2f199 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -666,7 +666,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 20094194e4..556bdcb8c2 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -270,9 +270,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; return SendCoinsReturn(OK); diff --git a/src/validation.cpp b/src/validation.cpp index 021323b17d..d506fd7016 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -156,7 +156,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 949b07c953..f0c079a123 100644 --- a/src/validation.h +++ b/src/validation.h @@ -53,12 +53,6 @@ struct LockPoints; /** 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 duffs) 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 duffs) -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 */ @@ -146,8 +140,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 duffs) 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; diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 8ef9ee28f5..566bbe85d1 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 8e7fbe7287..b057220416 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -71,6 +71,8 @@ void WalletInit::AddWalletOptions() const CURRENCY_UNIT, FormatMoney(DEFAULT_DISCARD_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_FEE); 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)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_FEE); + 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)), ArgsManager::ALLOW_ANY, 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)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET_FEE); gArgs.AddArg("-paytxfee=", strprintf("Fee (in %s/kB) to add to transactions you send (default: %s)", @@ -165,10 +167,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.")); - if (gArgs.IsArgSet("-walletbackupsdir")) { if (!fs::is_directory(gArgs.GetArg("-walletbackupsdir", ""))) { InitWarning(strprintf(_("Warning: incorrect parameter %s, path must exist! Using default path."), "-walletbackupsdir")); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a219acf0a4..4ef81c7336 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3066,7 +3066,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil utxo_pool.push_back(group); } bnb_used = false; - return KnapsackSolver(nTargetValue, utxo_pool, setCoinsRet, nValueRet, nCoinType == CoinType::ONLY_FULLY_MIXED, maxTxFee); + return KnapsackSolver(nTargetValue, utxo_pool, setCoinsRet, nValueRet, nCoinType == CoinType::ONLY_FULLY_MIXED, m_default_max_tx_fee); } } @@ -5120,6 +5120,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); @@ -5528,7 +5551,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 e17274c504..aaf25edb75 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -74,6 +74,12 @@ static const bool DEFAULT_AVOIDPARTIALSPENDS = false; static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6; static const bool DEFAULT_WALLETBROADCAST = true; static const bool DEFAULT_DISABLE_WALLET = false; +//! -maxtxfee default +static const CAmount DEFAULT_TRANSACTION_MAXFEE = COIN / 10; +//! Discourage users to set fees higher than this amount (in duffs) 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 duffs) +static const CAmount HIGH_MAX_TX_FEE = 100 * HIGH_TX_FEE_PER_KB; //! if set, all keys will be derived by using BIP39/BIP44 static const bool DEFAULT_USE_HD_WALLET = false; @@ -1065,6 +1071,8 @@ public: */ CFeeRate m_fallback_fee{DEFAULT_FALLBACK_FEE}; CFeeRate m_discard_rate{DEFAULT_DISCARD_FEE}; + /** 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);