From dd4896e35b12425f41a5755c7c446641818c8da2 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 f7abab482e9e6..dea656869d323 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 0ee828ce92be2..51b08fe1904eb 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(GetArg("-minrelaytxfee", ""), n)) { return InitError(AmountErrMsg("minrelaytxfee", 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 = 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 @@ -1562,7 +1563,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 f3ee0fbe393ae..562c361799db1 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 b80b6541dddd2..6f2f2f37c66d5 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 a01886c3ea8ae..625e435742511 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 4bfd8726a54f7..b0c08cb609362 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 0000000000000..3a9e68354d5f4 --- /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 0000000000000..7b8a7dc868e6d --- /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 0000000000000..0946678926d30 --- /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 0000000000000..a96e76ceb357a --- /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 a7c229fa74d1c..414229c6c242f 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 cf75cdd744961..28b5daaeb086d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -29,6 +29,7 @@ #include "util.h" #include "ui_interface.h" #include "utilmoneystr.h" +#include "wallet/fees.h" #include @@ -461,7 +462,7 @@ void CWallet::Flush(bool shutdown) dbw->Flush(shutdown); } -bool CWallet::Verify() +bool WalletVerify() { if (GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) return true; @@ -2541,7 +2542,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 */); @@ -2969,12 +2970,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 @@ -3006,7 +3007,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 @@ -3832,7 +3833,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")); @@ -4062,7 +4063,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) return walletInstance; } -bool CWallet::InitLoadWallet() +bool InitLoadWallet() { if (GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { LogPrintf("Wallet disabled!\n"); @@ -4070,7 +4071,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; } @@ -4094,7 +4095,7 @@ void CWallet::postInitProcess(CScheduler& scheduler) } } -bool CWallet::ParameterInteraction() +bool WalletParameterInteraction() { 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 7ef2e6f1d8ead..b4403720c70cf 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -972,16 +972,6 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface 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(); @@ -1066,11 +1056,6 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface //! 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. @@ -1107,12 +1092,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface /** 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 @@ -1120,9 +1101,6 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface */ 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) */