From d97fe2016cc7739929aac5c44de5037163b17ad0 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 2 Aug 2017 07:19:28 -0400 Subject: [PATCH] Move some static functions out of wallet.h/cpp This commit just moves a few function declarations and updates callers. Function bodies are moved in two followup MOVEONLY commits. This change is desirable because wallet.h/cpp are monolithic and hard to navigate, so pulling things out and grouping together pieces of related functionality should improve the organization. Another proximate motivation is the wallet process separation work in https://github.com/bitcoin/bitcoin/pull/10973, where (at least initially) parameter parsing and fee estimation are still done in the main process rather than the wallet process, and having functions that run in different processes scrambled up throughout wallet.cpp is unnecessarily confusing. --- src/Makefile.am | 4 ++++ src/init.cpp | 11 ++++++----- src/qt/coincontroldialog.cpp | 3 ++- src/qt/optionsdialog.cpp | 4 ---- src/qt/sendcoinsdialog.cpp | 10 +++++----- src/wallet/feebumper.cpp | 5 +++-- src/wallet/fees.cpp | 13 +++++++++++++ src/wallet/fees.h | 34 ++++++++++++++++++++++++++++++++++ src/wallet/init.cpp | 12 ++++++++++++ src/wallet/init.h | 25 +++++++++++++++++++++++++ src/wallet/rpcwallet.cpp | 3 ++- src/wallet/wallet.cpp | 21 +++++++++++---------- src/wallet/wallet.h | 22 ---------------------- 13 files changed, 117 insertions(+), 50 deletions(-) create mode 100644 src/wallet/fees.cpp create mode 100644 src/wallet/fees.h create mode 100644 src/wallet/init.cpp create mode 100644 src/wallet/init.h diff --git a/src/Makefile.am b/src/Makefile.am index f7abab482..dea656869 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -162,6 +162,8 @@ BITCOIN_CORE_H = \ wallet/crypter.h \ wallet/db.h \ wallet/feebumper.h \ + wallet/fees.h \ + wallet/init.h \ wallet/rpcwallet.h \ wallet/wallet.h \ wallet/walletdb.h \ @@ -239,6 +241,8 @@ libbitcoin_wallet_a_SOURCES = \ wallet/crypter.cpp \ wallet/db.cpp \ wallet/feebumper.cpp \ + wallet/fees.cpp \ + wallet/init.cpp \ wallet/rpcdump.cpp \ wallet/rpcwallet.cpp \ wallet/wallet.cpp \ diff --git a/src/init.cpp b/src/init.cpp index d79c2967b..c6ddefbfc 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -43,6 +43,7 @@ #include "utilmoneystr.h" #include "validationinterface.h" #ifdef ENABLE_WALLET +#include "wallet/init.h" #include "wallet/wallet.h" #endif #include "warnings.h" @@ -420,7 +421,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-maxuploadtarget=", strprintf(_("Tries to keep outbound traffic under the given target (in MiB per 24h), 0 = no limit (default: %d)"), DEFAULT_MAX_UPLOAD_TARGET)); #ifdef ENABLE_WALLET - strUsage += CWallet::GetWalletHelpString(showDebug); + strUsage += GetWalletHelpString(showDebug); #endif #if ENABLE_ZMQ @@ -1035,7 +1036,7 @@ bool AppInitParameterInteraction() if (!ParseMoney(gArgs.GetArg("-minrelaytxfee", ""), n)) { return InitError(AmountErrMsg("minrelaytxfee", gArgs.GetArg("-minrelaytxfee", ""))); } - // High fee check is done afterward in CWallet::ParameterInteraction() + // High fee check is done afterward in WalletParameterInteraction() ::minRelayTxFee = CFeeRate(n); } else if (incrementalRelayFee > ::minRelayTxFee) { // Allow only setting incrementalRelayFee to control both @@ -1068,7 +1069,7 @@ bool AppInitParameterInteraction() nBytesPerSigOp = gArgs.GetArg("-bytespersigop", nBytesPerSigOp); #ifdef ENABLE_WALLET - if (!CWallet::ParameterInteraction()) + if (!WalletParameterInteraction()) return false; #endif @@ -1245,7 +1246,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // ********************************************************* Step 5: verify wallet database integrity #ifdef ENABLE_WALLET - if (!CWallet::Verify()) + if (!WalletVerify()) return false; #endif // ********************************************************* Step 6: network initialization @@ -1566,7 +1567,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // ********************************************************* Step 8: load wallet #ifdef ENABLE_WALLET - if (!CWallet::InitLoadWallet()) + if (!InitLoadWallet()) return false; #else LogPrintf("No wallet support compiled in!\n"); diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index f3ee0fbe3..562c36179 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -18,6 +18,7 @@ #include "policy/fees.h" #include "policy/policy.h" #include "validation.h" // For mempool +#include "wallet/fees.h" #include "wallet/wallet.h" #include @@ -510,7 +511,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) nBytes -= 34; // Fee - nPayFee = CWallet::GetMinimumFee(nBytes, *coinControl, ::mempool, ::feeEstimator, nullptr /* FeeCalculation */); + nPayFee = GetMinimumFee(nBytes, *coinControl, ::mempool, ::feeEstimator, nullptr /* FeeCalculation */); if (nPayAmount > 0) { diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index b80b6541d..6f2f2f37c 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -17,10 +17,6 @@ #include "netbase.h" #include "txdb.h" // for -dbcache defaults -#ifdef ENABLE_WALLET -#include "wallet/wallet.h" // for CWallet::GetRequiredFee() -#endif - #include #include #include diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index a01886c3e..625e43574 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -22,7 +22,7 @@ #include "ui_interface.h" #include "txmempool.h" #include "policy/fees.h" -#include "wallet/wallet.h" +#include "wallet/fees.h" #include #include @@ -185,7 +185,7 @@ void SendCoinsDialog::setModel(WalletModel *_model) connect(ui->checkBoxMinimumFee, SIGNAL(stateChanged(int)), this, SLOT(coinControlUpdateLabels())); connect(ui->optInRBF, SIGNAL(stateChanged(int)), this, SLOT(updateSmartFeeLabel())); connect(ui->optInRBF, SIGNAL(stateChanged(int)), this, SLOT(coinControlUpdateLabels())); - ui->customFee->setSingleStep(CWallet::GetRequiredFee(1000)); + ui->customFee->setSingleStep(GetRequiredFee(1000)); updateFeeSectionControls(); updateMinFeeLabel(); updateSmartFeeLabel(); @@ -610,7 +610,7 @@ void SendCoinsDialog::on_buttonMinimizeFee_clicked() void SendCoinsDialog::setMinimumFee() { ui->radioCustomPerKilobyte->setChecked(true); - ui->customFee->setValue(CWallet::GetRequiredFee(1000)); + ui->customFee->setValue(GetRequiredFee(1000)); } void SendCoinsDialog::updateFeeSectionControls() @@ -643,7 +643,7 @@ void SendCoinsDialog::updateMinFeeLabel() { if (model && model->getOptionsModel()) ui->checkBoxMinimumFee->setText(tr("Pay only the required fee of %1").arg( - BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), CWallet::GetRequiredFee(1000)) + "/kB") + BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), GetRequiredFee(1000)) + "/kB") ); } @@ -668,7 +668,7 @@ void SendCoinsDialog::updateSmartFeeLabel() updateCoinControlState(coin_control); coin_control.m_feerate.reset(); // Explicitly use only fee estimation rate for smart fee labels FeeCalculation feeCalc; - CFeeRate feeRate = CFeeRate(CWallet::GetMinimumFee(1000, coin_control, ::mempool, ::feeEstimator, &feeCalc)); + CFeeRate feeRate = CFeeRate(GetMinimumFee(1000, coin_control, ::mempool, ::feeEstimator, &feeCalc)); ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), feeRate.GetFeePerK()) + "/kB"); diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index c1ea2b629..285b0099c 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -5,6 +5,7 @@ #include "consensus/validation.h" #include "wallet/coincontrol.h" #include "wallet/feebumper.h" +#include "wallet/fees.h" #include "wallet/wallet.h" #include "policy/fees.h" #include "policy/policy.h" @@ -156,7 +157,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin currentResult = BumpFeeResult::INVALID_PARAMETER; return; } - CAmount requiredFee = CWallet::GetRequiredFee(maxNewTxSize); + CAmount requiredFee = GetRequiredFee(maxNewTxSize); if (totalFee < requiredFee) { vErrors.push_back(strprintf("Insufficient totalFee (cannot be less than required fee %s)", FormatMoney(requiredFee))); @@ -166,7 +167,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin nNewFee = totalFee; nNewFeeRate = CFeeRate(totalFee, maxNewTxSize); } else { - nNewFee = CWallet::GetMinimumFee(maxNewTxSize, coin_control, mempool, ::feeEstimator, nullptr /* FeeCalculation */); + nNewFee = GetMinimumFee(maxNewTxSize, coin_control, mempool, ::feeEstimator, nullptr /* FeeCalculation */); nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize); // New fee rate must be at least old rate + minimum incremental relay rate diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp new file mode 100644 index 000000000..3a9e68354 --- /dev/null +++ b/src/wallet/fees.cpp @@ -0,0 +1,13 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "wallet/fees.h" + +#include "policy/policy.h" +#include "txmempool.h" +#include "util.h" +#include "validation.h" +#include "wallet/coincontrol.h" +#include "wallet/wallet.h" diff --git a/src/wallet/fees.h b/src/wallet/fees.h new file mode 100644 index 000000000..7b8a7dc86 --- /dev/null +++ b/src/wallet/fees.h @@ -0,0 +1,34 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_WALLET_FEES_H +#define BITCOIN_WALLET_FEES_H + +#include "amount.h" + +class CBlockPolicyEstimator; +class CCoinControl; +class CFeeRate; +class CTxMemPool; +struct FeeCalculation; + +/** + * Return the minimum required fee taking into account the + * floating relay fee and user set minimum transaction fee + */ +CAmount GetRequiredFee(unsigned int nTxBytes); + +/** + * Estimate the minimum fee considering user set parameters + * and the required fee + */ +CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); + +/** + * Return the maximum feerate for discarding change. + */ +CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator); + +#endif // BITCOIN_WALLET_FEES_H diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp new file mode 100644 index 000000000..094667892 --- /dev/null +++ b/src/wallet/init.cpp @@ -0,0 +1,12 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "wallet/init.h" + +#include "net.h" +#include "util.h" +#include "utilmoneystr.h" +#include "validation.h" +#include "wallet/wallet.h" diff --git a/src/wallet/init.h b/src/wallet/init.h new file mode 100644 index 000000000..fa2251506 --- /dev/null +++ b/src/wallet/init.h @@ -0,0 +1,25 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_WALLET_INIT_H +#define BITCOIN_WALLET_INIT_H + +#include + +//! Return the wallets help message. +std::string GetWalletHelpString(bool showDebug); + +//! Wallets parameter interaction +bool WalletParameterInteraction(); + +//! Responsible for reading and validating the -wallet arguments and verifying the wallet database. +// This function will perform salvage on the wallet if requested, as long as only one wallet is +// being loaded (CWallet::ParameterInteraction forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet). +bool WalletVerify(); + +//! Load wallet databases. +bool InitLoadWallet(); + +#endif // BITCOIN_WALLET_INIT_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a6176c348..30b8c8260 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -8,7 +8,6 @@ #include "chain.h" #include "consensus/validation.h" #include "core_io.h" -#include "init.h" #include "httpserver.h" #include "validation.h" #include "net.h" @@ -27,6 +26,8 @@ #include "wallet/wallet.h" #include "wallet/walletdb.h" +#include // For StartShutdown + #include #include diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 599e74149..a9b615275 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -30,6 +30,7 @@ #include "util.h" #include "ui_interface.h" #include "utilmoneystr.h" +#include "wallet/fees.h" #include @@ -494,7 +495,7 @@ void CWallet::Flush(bool shutdown) dbw->Flush(shutdown); } -bool CWallet::Verify() +bool WalletVerify() { if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) return true; @@ -2599,7 +2600,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC return true; } -static CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator) +CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator) { unsigned int highest_target = estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); CFeeRate discard_rate = estimator.estimateSmartFee(highest_target, nullptr /* FeeCalculation */, false /* conservative */); @@ -3031,12 +3032,12 @@ bool CWallet::AddAccountingEntry(const CAccountingEntry& acentry, CWalletDB *pwa return true; } -CAmount CWallet::GetRequiredFee(unsigned int nTxBytes) +CAmount GetRequiredFee(unsigned int nTxBytes) { - return std::max(minTxFee.GetFee(nTxBytes), ::minRelayTxFee.GetFee(nTxBytes)); + return std::max(CWallet::minTxFee.GetFee(nTxBytes), ::minRelayTxFee.GetFee(nTxBytes)); } -CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) +CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) { /* User control of how to calculate fee uses the following parameter precedence: 1. coin_control.m_feerate @@ -3068,7 +3069,7 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_c fee_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate).GetFee(nTxBytes); if (fee_needed == 0) { // if we don't have enough data for estimateSmartFee, then use fallbackFee - fee_needed = fallbackFee.GetFee(nTxBytes); + fee_needed = CWallet::fallbackFee.GetFee(nTxBytes); if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; } // Obey mempool min fee when using smart fee estimation @@ -3888,7 +3889,7 @@ std::vector CWallet::GetDestValues(const std::string& prefix) const return values; } -std::string CWallet::GetWalletHelpString(bool showDebug) +std::string GetWalletHelpString(bool showDebug) { std::string strUsage = HelpMessageGroup(_("Wallet options:")); strUsage += HelpMessageOpt("-disablewallet", _("Do not load the wallet and disable wallet RPC calls")); @@ -4121,7 +4122,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) return walletInstance; } -bool CWallet::InitLoadWallet() +bool InitLoadWallet() { if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { LogPrintf("Wallet disabled!\n"); @@ -4129,7 +4130,7 @@ bool CWallet::InitLoadWallet() } for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - CWallet * const pwallet = CreateWalletFromFile(walletFile); + CWallet * const pwallet = CWallet::CreateWalletFromFile(walletFile); if (!pwallet) { return false; } @@ -4153,7 +4154,7 @@ void CWallet::postInitProcess(CScheduler& scheduler) } } -bool CWallet::ParameterInteraction() +bool WalletParameterInteraction() { gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f97a99d82..bceeb12fb 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -960,16 +960,6 @@ public: static CFeeRate minTxFee; static CFeeRate fallbackFee; static CFeeRate m_discard_rate; - /** - * Estimate the minimum fee considering user set parameters - * and the required fee - */ - static CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); - /** - * Return the minimum required fee taking into account the - * floating relay fee and user set minimum transaction fee - */ - static CAmount GetRequiredFee(unsigned int nTxBytes); bool NewKeyPool(); size_t KeypoolCountExternalKeys(); @@ -1060,11 +1050,6 @@ public: //! Flush wallet (bitdb flush) void Flush(bool shutdown=false); - //! Responsible for reading and validating the -wallet arguments and verifying the wallet database. - // This function will perform salvage on the wallet if requested, as long as only one wallet is - // being loaded (CWallet::ParameterInteraction forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet). - static bool Verify(); - /** * Address book entry changed. * @note called with lock cs_wallet held. @@ -1101,12 +1086,8 @@ public: /** Mark a transaction as replaced by another transaction (e.g., BIP 125). */ bool MarkReplaced(const uint256& originalHash, const uint256& newHash); - /* Returns the wallets help message */ - static std::string GetWalletHelpString(bool showDebug); - /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ static CWallet* CreateWalletFromFile(const std::string walletFile); - static bool InitLoadWallet(); /** * Wallet post-init setup @@ -1114,9 +1095,6 @@ public: */ void postInitProcess(CScheduler& scheduler); - /* Wallets parameter interaction */ - static bool ParameterInteraction(); - bool BackupWallet(const std::string& strDest); /* Set the HD chain model (chain child index counters) */