From 80f52a2267f44a9cae4440615df3ff989be1579c Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 18:13:23 -0400 Subject: [PATCH 01/16] Remove uses of CheckFinalTx in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 6 ++++++ src/interfaces/chain.h | 4 ++++ src/interfaces/wallet.cpp | 4 ++-- src/wallet/rpcwallet.cpp | 10 +++------- src/wallet/wallet.cpp | 9 +++------ 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index da810bc5e63..c7c2d4e91c3 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -132,6 +133,11 @@ class LockImpl : public Chain::Lock } return nullopt; } + bool checkFinalTx(const CTransaction& tx) override + { + LockAnnotation lock(::cs_main); + return CheckFinalTx(tx); + } }; class LockingStateImpl : public LockImpl, public UniqueLock diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 3a54b9164e0..453938751dd 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -14,6 +14,7 @@ class CBlock; class CScheduler; +class CTransaction; class uint256; struct CBlockLocator; @@ -102,6 +103,9 @@ public: //! is guaranteed to be an ancestor of the block used to create the //! locator. virtual Optional findLocatorFork(const CBlockLocator& locator) = 0; + + //! Check if transaction will be final given chain height current time. + virtual bool checkFinalTx(const CTransaction& tx) = 0; }; //! Return Lock interface. Chain is locked when this is called, and diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 0dac75834e0..e870d3d5377 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -99,7 +99,7 @@ WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, co //! Construct wallet tx status struct. WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx) { - LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. + LockAnnotation lock(::cs_main); // Temporary, for mapBlockIndex below. Removed in upcoming commit. WalletTxStatus result; auto mi = ::mapBlockIndex.find(wtx.hashBlock); @@ -109,7 +109,7 @@ WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const C result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain); result.time_received = wtx.nTimeReceived; result.lock_time = wtx.tx->nLockTime; - result.is_final = CheckFinalTx(*wtx.tx); + result.is_final = locked_chain.checkFinalTx(*wtx.tx); result.is_trusted = wtx.IsTrusted(locked_chain); result.is_abandoned = wtx.isAbandoned(); result.is_coinbase = wtx.IsCoinBase(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 97c6c38be11..eae0be86d32 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -607,7 +607,6 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); @@ -630,7 +629,7 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request) CAmount nAmount = 0; for (const std::pair& pairWtx : pwallet->mapWallet) { const CWalletTx& wtx = pairWtx.second; - if (wtx.IsCoinBase() || !CheckFinalTx(*wtx.tx)) + if (wtx.IsCoinBase() || !locked_chain->checkFinalTx(*wtx.tx)) continue; for (const CTxOut& txout : wtx.tx->vout) @@ -679,7 +678,6 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); @@ -696,7 +694,7 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request) CAmount nAmount = 0; for (const std::pair& pairWtx : pwallet->mapWallet) { const CWalletTx& wtx = pairWtx.second; - if (wtx.IsCoinBase() || !CheckFinalTx(*wtx.tx)) + if (wtx.IsCoinBase() || !locked_chain->checkFinalTx(*wtx.tx)) continue; for (const CTxOut& txout : wtx.tx->vout) @@ -1051,8 +1049,6 @@ struct tallyitem static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, CWallet * const pwallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { - LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. - // Minimum confirmations int nMinDepth = 1; if (!params[0].isNull()) @@ -1083,7 +1079,7 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, CWallet * co for (const std::pair& pairWtx : pwallet->mapWallet) { const CWalletTx& wtx = pairWtx.second; - if (wtx.IsCoinBase() || !CheckFinalTx(*wtx.tx)) + if (wtx.IsCoinBase() || !locked_chain.checkFinalTx(*wtx.tx)) continue; int nDepth = wtx.GetDepthInMainChain(locked_chain); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 388422bec84..eb99c6cd77b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2075,10 +2075,8 @@ bool CWalletTx::InMempool() const bool CWalletTx::IsTrusted(interfaces::Chain::Lock& locked_chain) const { - LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. - // Quick answer in most cases - if (!CheckFinalTx(*tx)) + if (!locked_chain.checkFinalTx(*tx)) return false; int nDepth = GetDepthInMainChain(locked_chain); if (nDepth >= 1) @@ -2263,7 +2261,6 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const // trusted. CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth) const { - LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -2271,7 +2268,7 @@ CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth) cons for (const auto& entry : mapWallet) { const CWalletTx& wtx = entry.second; const int depth = wtx.GetDepthInMainChain(*locked_chain); - if (depth < 0 || !CheckFinalTx(*wtx.tx) || wtx.IsImmatureCoinBase(*locked_chain)) { + if (depth < 0 || !locked_chain->checkFinalTx(*wtx.tx) || wtx.IsImmatureCoinBase(*locked_chain)) { continue; } @@ -2325,7 +2322,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< const uint256& wtxid = entry.first; const CWalletTx* pcoin = &entry.second; - if (!CheckFinalTx(*pcoin->tx)) + if (!locked_chain.checkFinalTx(*pcoin->tx)) continue; if (pcoin->IsImmatureCoinBase(locked_chain)) From bdc6628683197945326cbdfea3f53ec0b7d1949f Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 19:25:26 -0400 Subject: [PATCH 02/16] Remove use of IsRBFOptIn in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 7 +++++++ src/interfaces/chain.h | 6 +++++- src/wallet/rpcwallet.cpp | 3 +-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index c7c2d4e91c3..7cab303aadf 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -6,9 +6,11 @@ #include #include +#include #include #include #include +#include #include #include #include @@ -183,6 +185,11 @@ public: LOCK(cs_main); return GuessVerificationProgress(Params().TxData(), LookupBlockIndex(block_hash)); } + RBFTransactionState isRBFOptIn(const CTransaction& tx) override + { + LOCK(::mempool.cs); + return IsRBFOptIn(tx, ::mempool); + } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 453938751dd..486f1ea1697 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -5,7 +5,8 @@ #ifndef BITCOIN_INTERFACES_CHAIN_H #define BITCOIN_INTERFACES_CHAIN_H -#include +#include // For Optional and nullopt +#include // For RBFTransactionState #include #include @@ -131,6 +132,9 @@ public: //! Estimate fraction of total transactions verified if blocks up to //! the specified block hash are verified. virtual double guessVerificationProgress(const uint256& block_hash) = 0; + + //! Check if transaction is RBF opt in. + virtual RBFTransactionState isRBFOptIn(const CTransaction& tx) = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index eae0be86d32..72177995350 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -124,8 +124,7 @@ static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& lo // Add opt-in RBF status std::string rbfStatus = "no"; if (confirms <= 0) { - LOCK(mempool.cs); - RBFTransactionState rbfState = IsRBFOptIn(*wtx.tx, mempool); + RBFTransactionState rbfState = chain.isRBFOptIn(*wtx.tx); if (rbfState == RBFTransactionState::UNKNOWN) rbfStatus = "unknown"; else if (rbfState == RBFTransactionState::REPLACEABLE_BIP125) From 291276f7f40df9fcd62e54c016953705bf0ed04a Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 19:29:50 -0400 Subject: [PATCH 03/16] Remove use of GetCountWithDescendants in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 6 ++++++ src/interfaces/chain.h | 3 +++ src/wallet/feebumper.cpp | 4 +--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 7cab303aadf..8c5921970b0 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -190,6 +190,12 @@ public: LOCK(::mempool.cs); return IsRBFOptIn(tx, ::mempool); } + bool hasDescendantsInMempool(const uint256& txid) override + { + LOCK(::mempool.cs); + auto it_mp = ::mempool.mapTx.find(txid); + return it_mp != ::mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1; + } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 486f1ea1697..aa4f17a8ec0 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -135,6 +135,9 @@ public: //! Check if transaction is RBF opt in. virtual RBFTransactionState isRBFOptIn(const CTransaction& tx) = 0; + + //! Check if transaction has descendants in mempool. + virtual bool hasDescendantsInMempool(const uint256& txid) = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 7a71aea7155..f4b419ca2a1 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -27,9 +27,7 @@ static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chai } { - LOCK(mempool.cs); - auto it_mp = mempool.mapTx.find(wtx.GetHash()); - if (it_mp != mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1) { + if (wallet->chain().hasDescendantsInMempool(wtx.GetHash())) { errors.push_back("Transaction has descendants in the mempool"); return feebumper::Result::INVALID_PARAMETER; } From cd32160af0528cc746968ee0eadf4f63c98665f2 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 19:42:27 -0400 Subject: [PATCH 04/16] Remove use of GetTransactionAncestry in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 4 ++++ src/interfaces/chain.h | 4 ++++ src/wallet/wallet.cpp | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 8c5921970b0..d7bad0543ce 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -196,6 +196,10 @@ public: auto it_mp = ::mempool.mapTx.find(txid); return it_mp != ::mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1; } + void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) override + { + ::mempool.GetTransactionAncestry(txid, ancestors, descendants); + } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index aa4f17a8ec0..33bbfefd9f3 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -9,6 +9,7 @@ #include // For RBFTransactionState #include +#include #include #include #include @@ -138,6 +139,9 @@ public: //! Check if transaction has descendants in mempool. virtual bool hasDescendantsInMempool(const uint256& txid) = 0; + + //! Calculate mempool ancestor and descendant counts for the given transaction. + virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index eb99c6cd77b..a77b1c60f2b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4528,7 +4528,7 @@ std::vector CWallet::GroupOutputs(const std::vector& outpu CInputCoin input_coin = output.GetInputCoin(); size_t ancestors, descendants; - mempool.GetTransactionAncestry(output.tx->GetHash(), ancestors, descendants); + chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants); if (!single_coin && ExtractDestination(output.tx->tx->vout[output.i].scriptPubKey, dst)) { // Limit output groups to no more than 10 entries, to protect // against inadvertently creating a too-large transaction From 1fb0a4a04e9cda19ed5ad04694a39c83c91b6072 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 19:45:45 -0400 Subject: [PATCH 05/16] Remove use of CalculateMemPoolAncestors in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 14 ++++++++++++++ src/interfaces/chain.h | 3 +++ src/wallet/wallet.cpp | 11 +---------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index d7bad0543ce..c12215cd606 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -200,6 +200,20 @@ public: { ::mempool.GetTransactionAncestry(txid, ancestors, descendants); } + bool checkChainLimits(CTransactionRef tx) override + { + LockPoints lp; + CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); + CTxMemPool::setEntries ancestors; + auto limit_ancestor_count = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); + auto limit_ancestor_size = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT) * 1000; + auto limit_descendant_count = gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT); + auto limit_descendant_size = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT) * 1000; + std::string unused_error_string; + LOCK(::mempool.cs); + return ::mempool.CalculateMemPoolAncestors(entry, ancestors, limit_ancestor_count, limit_ancestor_size, + limit_descendant_count, limit_descendant_size, unused_error_string); + } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 33bbfefd9f3..2e0328eb25b 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -142,6 +142,9 @@ public: //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; + + //! Check chain limits. + virtual bool checkChainLimits(CTransactionRef tx) = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a77b1c60f2b..5c95c7ec614 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3127,16 +3127,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits - LockPoints lp; - CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); - CTxMemPool::setEntries setAncestors; - size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); - size_t nLimitAncestorSize = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; - size_t nLimitDescendants = gArgs.GetArg("-limitdescendantcount", DEFAULT_DESCENDANT_LIMIT); - size_t nLimitDescendantSize = gArgs.GetArg("-limitdescendantsize", DEFAULT_DESCENDANT_SIZE_LIMIT)*1000; - std::string errString; - LOCK(::mempool.cs); - if (!::mempool.CalculateMemPoolAncestors(entry, setAncestors, nLimitAncestors, nLimitAncestorSize, nLimitDescendants, nLimitDescendantSize, errString)) { + if (!chain().checkChainLimits(tx)) { strFailReason = _("Transaction has too long of a mempool chain"); return false; } From cc02c796d3517931acc861b0f9bc50e36e1c95f9 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 21:40:29 -0400 Subject: [PATCH 06/16] Remove uses of fee globals in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 14 ++++++++++++++ src/interfaces/chain.h | 10 ++++++++++ src/interfaces/wallet.cpp | 2 +- src/rpc/mining.cpp | 6 ++++-- src/rpc/util.cpp | 5 +---- src/rpc/util.h | 2 +- src/wallet/feebumper.cpp | 4 ++-- src/wallet/fees.cpp | 17 ++++++++--------- src/wallet/fees.h | 8 +++----- src/wallet/rpcwallet.cpp | 8 ++++---- src/wallet/wallet.cpp | 12 ++++++------ src/wallet/wallet.h | 2 -- 12 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index c12215cd606..f8886068792 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -6,6 +6,8 @@ #include #include +#include +#include #include #include #include @@ -214,6 +216,18 @@ public: return ::mempool.CalculateMemPoolAncestors(entry, ancestors, limit_ancestor_count, limit_ancestor_size, limit_descendant_count, limit_descendant_size, unused_error_string); } + CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override + { + return ::feeEstimator.estimateSmartFee(num_blocks, calc, conservative); + } + unsigned int estimateMaxBlocks() override + { + return ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); + } + CFeeRate mempoolMinFee() override + { + return ::mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); + } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 2e0328eb25b..d0f74cb100c 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -19,6 +19,7 @@ class CScheduler; class CTransaction; class uint256; struct CBlockLocator; +struct FeeCalculation; namespace interfaces { @@ -145,6 +146,15 @@ public: //! Check chain limits. virtual bool checkChainLimits(CTransactionRef tx) = 0; + + //! Estimate smart fee. + virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0; + + //! Fee estimator max target. + virtual unsigned int estimateMaxBlocks() = 0; + + //! Pool min fee. + virtual CFeeRate mempoolMinFee() = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index e870d3d5377..f3a0b416dbe 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -457,7 +457,7 @@ public: { FeeCalculation fee_calc; CAmount result; - result = GetMinimumFee(*m_wallet, tx_bytes, coin_control, ::mempool, ::feeEstimator, &fee_calc); + result = GetMinimumFee(*m_wallet, tx_bytes, coin_control, &fee_calc); if (returned_target) *returned_target = fee_calc.returnedTarget; if (reason) *reason = fee_calc.reason; return result; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 6625a03bbd9..f2acb8fbf59 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -843,7 +843,8 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request) RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VSTR}); RPCTypeCheckArgument(request.params[0], UniValue::VNUM); - unsigned int conf_target = ParseConfirmTarget(request.params[0]); + unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); + unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target); bool conservative = true; if (!request.params[1].isNull()) { FeeEstimateMode fee_mode; @@ -915,7 +916,8 @@ static UniValue estimaterawfee(const JSONRPCRequest& request) RPCTypeCheck(request.params, {UniValue::VNUM, UniValue::VNUM}, true); RPCTypeCheckArgument(request.params[0], UniValue::VNUM); - unsigned int conf_target = ParseConfirmTarget(request.params[0]); + unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); + unsigned int conf_target = ParseConfirmTarget(request.params[0], max_target); double threshold = 0.95; if (!request.params[1].isNull()) { threshold = request.params[1].get_real(); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 1eec916abfd..fb048311de0 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -4,11 +4,9 @@ #include #include -#include #include #include #include -#include InitInterfaces* g_rpc_interfaces = nullptr; @@ -130,10 +128,9 @@ UniValue DescribeAddress(const CTxDestination& dest) return boost::apply_visitor(DescribeAddressVisitor(), dest); } -unsigned int ParseConfirmTarget(const UniValue& value) +unsigned int ParseConfirmTarget(const UniValue& value, unsigned int max_target) { int target = value.get_int(); - unsigned int max_target = ::feeEstimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); if (target < 1 || (unsigned int)target > max_target) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid conf_target, must be between %u - %u", 1, max_target)); } diff --git a/src/rpc/util.h b/src/rpc/util.h index a83ae98b7e7..e1a5491def2 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -33,7 +33,7 @@ CScript CreateMultisigRedeemscript(const int required, const std::vectornValue -= nDelta; - if (poutput->nValue <= GetDustThreshold(*poutput, GetDiscardRate(*wallet, ::feeEstimator))) { + if (poutput->nValue <= GetDustThreshold(*poutput, GetDiscardRate(*wallet))) { wallet->WalletLogPrintf("Bumping fee and discarding dust output\n"); new_fee += poutput->nValue; mtx.vout.erase(mtx.vout.begin() + nOutput); diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 9e2984ff053..545adaebc18 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -19,9 +18,9 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes) } -CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc) +CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, FeeCalculation* feeCalc) { - CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, pool, estimator, feeCalc).GetFee(nTxBytes); + CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes); // Always obey the maximum if (fee_needed > maxTxFee) { fee_needed = maxTxFee; @@ -35,7 +34,7 @@ CFeeRate GetRequiredFeeRate(const CWallet& wallet) return std::max(wallet.m_min_fee, ::minRelayTxFee); } -CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc) +CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, FeeCalculation* feeCalc) { /* User control of how to calculate fee uses the following parameter precedence: 1. coin_control.m_feerate @@ -64,7 +63,7 @@ CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_contr if (coin_control.m_fee_mode == FeeEstimateMode::CONSERVATIVE) conservative_estimate = true; else if (coin_control.m_fee_mode == FeeEstimateMode::ECONOMICAL) conservative_estimate = false; - feerate_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate); + feerate_needed = wallet.chain().estimateSmartFee(target, conservative_estimate, feeCalc); if (feerate_needed == CFeeRate(0)) { // if we don't have enough data for estimateSmartFee, then use fallback fee feerate_needed = wallet.m_fallback_fee; @@ -74,7 +73,7 @@ CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_contr if (wallet.m_fallback_fee == CFeeRate(0)) return feerate_needed; } // Obey mempool min fee when using smart fee estimation - CFeeRate min_mempool_feerate = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); + CFeeRate min_mempool_feerate = wallet.chain().mempoolMinFee(); if (feerate_needed < min_mempool_feerate) { feerate_needed = min_mempool_feerate; if (feeCalc) feeCalc->reason = FeeReason::MEMPOOL_MIN; @@ -90,10 +89,10 @@ CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_contr return feerate_needed; } -CFeeRate GetDiscardRate(const CWallet& wallet, const CBlockPolicyEstimator& estimator) +CFeeRate GetDiscardRate(const CWallet& wallet) { - unsigned int highest_target = estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); - CFeeRate discard_rate = estimator.estimateSmartFee(highest_target, nullptr /* FeeCalculation */, false /* conservative */); + unsigned int highest_target = wallet.chain().estimateMaxBlocks(); + CFeeRate discard_rate = wallet.chain().estimateSmartFee(highest_target, false /* conservative */); // Don't let discard_rate be greater than longest possible fee estimate if we get a valid fee estimate discard_rate = (discard_rate == CFeeRate(0)) ? wallet.m_discard_rate : std::min(discard_rate, wallet.m_discard_rate); // Discard rate must be at least dustRelayFee diff --git a/src/wallet/fees.h b/src/wallet/fees.h index 6bfee456c05..434f211dc24 100644 --- a/src/wallet/fees.h +++ b/src/wallet/fees.h @@ -8,10 +8,8 @@ #include -class CBlockPolicyEstimator; class CCoinControl; class CFeeRate; -class CTxMemPool; class CWallet; struct FeeCalculation; @@ -25,7 +23,7 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes); * Estimate the minimum fee considering user set parameters * and the required fee */ -CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc); +CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinControl& coin_control, FeeCalculation* feeCalc); /** * Return the minimum required feerate taking into account the @@ -37,11 +35,11 @@ CFeeRate GetRequiredFeeRate(const CWallet& wallet); * Estimate the minimum fee rate considering user set parameters * and the required fee */ -CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation* feeCalc); +CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, FeeCalculation* feeCalc); /** * Return the maximum feerate for discarding change. */ -CFeeRate GetDiscardRate(const CWallet& wallet, const CBlockPolicyEstimator& estimator); +CFeeRate GetDiscardRate(const CWallet& wallet); #endif // BITCOIN_WALLET_FEES_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 72177995350..09e91dc5880 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -424,7 +424,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) } if (!request.params[6].isNull()) { - coin_control.m_confirm_target = ParseConfirmTarget(request.params[6]); + coin_control.m_confirm_target = ParseConfirmTarget(request.params[6], pwallet->chain().estimateMaxBlocks()); } if (!request.params[7].isNull()) { @@ -884,7 +884,7 @@ static UniValue sendmany(const JSONRPCRequest& request) } if (!request.params[6].isNull()) { - coin_control.m_confirm_target = ParseConfirmTarget(request.params[6]); + coin_control.m_confirm_target = ParseConfirmTarget(request.params[6], pwallet->chain().estimateMaxBlocks()); } if (!request.params[7].isNull()) { @@ -2989,7 +2989,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f if (options.exists("feeRate")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and feeRate"); } - coinControl.m_confirm_target = ParseConfirmTarget(options["conf_target"]); + coinControl.m_confirm_target = ParseConfirmTarget(options["conf_target"], pwallet->chain().estimateMaxBlocks()); } if (options.exists("estimate_mode")) { if (options.exists("feeRate")) { @@ -3279,7 +3279,7 @@ static UniValue bumpfee(const JSONRPCRequest& request) if (options.exists("confTarget") && options.exists("totalFee")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget and totalFee options should not both be set. Please provide either a confirmation target for fee estimation or an explicit total fee for the transaction."); } else if (options.exists("confTarget")) { // TODO: alias this to conf_target - coin_control.m_confirm_target = ParseConfirmTarget(options["confTarget"]); + coin_control.m_confirm_target = ParseConfirmTarget(options["confTarget"], pwallet->chain().estimateMaxBlocks()); } else if (options.exists("totalFee")) { totalFee = options["totalFee"].get_int64(); if (totalFee <= 0) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5c95c7ec614..75681e2007e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2485,10 +2485,10 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil FeeCalculation feeCalc; CCoinControl temp; temp.m_confirm_target = 1008; - CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, ::mempool, ::feeEstimator, &feeCalc); + CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc); // Calculate cost of change - CAmount cost_of_change = GetDiscardRate(*this, ::feeEstimator).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); + CAmount cost_of_change = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); // Filter by the min conf specs and add to utxo_pool and calculate effective value for (OutputGroup& group : groups) { @@ -2858,10 +2858,10 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std CTxOut change_prototype_txout(0, scriptChange); coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout); - CFeeRate discard_rate = GetDiscardRate(*this, ::feeEstimator); + CFeeRate discard_rate = GetDiscardRate(*this); // Get the fee rate to use effective values in coin selection - CFeeRate nFeeRateNeeded = GetMinimumFeeRate(*this, coin_control, ::mempool, ::feeEstimator, &feeCalc); + CFeeRate nFeeRateNeeded = GetMinimumFeeRate(*this, coin_control, &feeCalc); nFeeRet = 0; bool pick_new_inputs = true; @@ -2994,7 +2994,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std return false; } - nFeeNeeded = GetMinimumFee(*this, nBytes, coin_control, ::mempool, ::feeEstimator, &feeCalc); + nFeeNeeded = GetMinimumFee(*this, nBytes, coin_control, &feeCalc); if (feeCalc.reason == FeeReason::FALLBACK && !m_allow_fallback_fee) { // eventually allow a fallback fee strFailReason = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); @@ -3022,7 +3022,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std // change output. Only try this once. if (nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) { unsigned int tx_size_with_change = nBytes + coin_selection_params.change_output_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size - CAmount fee_needed_with_change = GetMinimumFee(*this, tx_size_with_change, coin_control, ::mempool, ::feeEstimator, nullptr); + CAmount fee_needed_with_change = GetMinimumFee(*this, tx_size_with_change, coin_control, nullptr); CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) { pick_new_inputs = false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2a5d6caaf8c..f215765dab5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -100,8 +100,6 @@ class CCoinControl; class COutput; class CReserveKey; class CScript; -class CTxMemPool; -class CBlockPolicyEstimator; class CWalletTx; struct FeeCalculation; enum class FeeEstimateMode; From cc3836e8f90894432db06d9de6b20eac53d93cbe Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 22:45:01 -0400 Subject: [PATCH 07/16] Remove uses of fPruneMode in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 1 + src/interfaces/chain.h | 3 +++ src/wallet/rpcdump.cpp | 8 ++++---- src/wallet/wallet.cpp | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index f8886068792..bb2af5b8ba5 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -228,6 +228,7 @@ public: { return ::mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); } + bool getPruneMode() override { return ::fPruneMode; } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index d0f74cb100c..96e4f8cf0bb 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -155,6 +155,9 @@ public: //! Pool min fee. virtual CFeeRate mempoolMinFee() = 0; + + //! Check if pruning is enabled. + virtual bool getPruneMode() = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index f38202a2b81..046a6567a5d 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -157,7 +157,7 @@ UniValue importprivkey(const JSONRPCRequest& request) if (!request.params[2].isNull()) fRescan = request.params[2].get_bool(); - if (fRescan && fPruneMode) + if (fRescan && pwallet->chain().getPruneMode()) throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); if (fRescan && !reserver.reserve()) { @@ -313,7 +313,7 @@ UniValue importaddress(const JSONRPCRequest& request) if (!request.params[2].isNull()) fRescan = request.params[2].get_bool(); - if (fRescan && fPruneMode) + if (fRescan && pwallet->chain().getPruneMode()) throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); WalletRescanReserver reserver(pwallet); @@ -501,7 +501,7 @@ UniValue importpubkey(const JSONRPCRequest& request) if (!request.params[2].isNull()) fRescan = request.params[2].get_bool(); - if (fRescan && fPruneMode) + if (fRescan && pwallet->chain().getPruneMode()) throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); WalletRescanReserver reserver(pwallet); @@ -562,7 +562,7 @@ UniValue importwallet(const JSONRPCRequest& request) }, }.ToString()); - if (fPruneMode) + if (pwallet->chain().getPruneMode()) throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled in pruned mode"); WalletRescanReserver reserver(pwallet); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 75681e2007e..a0718309b92 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4334,7 +4334,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, //We can't rescan beyond non-pruned blocks, stop and throw an error //this might happen if a user uses an old wallet within a pruned node // or if he ran -disablewallet for a longer time, then decided to re-enable - if (fPruneMode) + if (chain.getPruneMode()) { int block_height = *tip_height; while (block_height > 0 && locked_chain->haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { From 00dfb2a440b94a24b61cafb519fb122f6a0ae176 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 22:50:07 -0400 Subject: [PATCH 08/16] Remove uses of g_connman in wallet code This commit does not change behavior. --- src/Makefile.am | 8 ++++++-- src/Makefile.bench.include | 4 +++- src/interfaces/chain.cpp | 2 ++ src/interfaces/chain.h | 3 +++ src/wallet/rpcwallet.cpp | 6 +++--- 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index d491530ca10..650e347da38 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -562,16 +562,20 @@ bitcoin_wallet_LDADD = \ $(LIBBITCOIN_WALLET_TOOL) \ $(LIBBITCOIN_WALLET) \ $(LIBBITCOIN_SERVER) \ + $(LIBBITCOIN_WALLET) \ + $(LIBBITCOIN_SERVER) \ $(LIBBITCOIN_COMMON) \ $(LIBBITCOIN_CONSENSUS) \ $(LIBBITCOIN_UTIL) \ $(LIBBITCOIN_CRYPTO) \ + $(LIBBITCOIN_ZMQ) \ $(LIBLEVELDB) \ $(LIBLEVELDB_SSE42) \ $(LIBMEMENV) \ - $(LIBSECP256K1) + $(LIBSECP256K1) \ + $(LIBUNIVALUE) -bitcoin_wallet_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) +bitcoin_wallet_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(CRYPTO_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(MINIUPNPC_LIBS) $(ZMQ_LIBS) # # bitcoinconsensus library # diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 5e787ca222a..829e6446436 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -37,6 +37,8 @@ nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES) bench_bench_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CLFAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/ bench_bench_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) bench_bench_bitcoin_LDADD = \ + $(LIBBITCOIN_WALLET) \ + $(LIBBITCOIN_SERVER) \ $(LIBBITCOIN_WALLET) \ $(LIBBITCOIN_SERVER) \ $(LIBBITCOIN_COMMON) \ @@ -57,7 +59,7 @@ if ENABLE_WALLET bench_bench_bitcoin_SOURCES += bench/coin_selection.cpp endif -bench_bench_bitcoin_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) +bench_bench_bitcoin_LDADD += $(BOOST_LIBS) $(BDB_LIBS) $(CRYPTO_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(MINIUPNPC_LIBS) bench_bench_bitcoin_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_BENCH_FILES) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index bb2af5b8ba5..8493c5de70b 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -229,6 +230,7 @@ public: return ::mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); } bool getPruneMode() override { return ::fPruneMode; } + bool p2pEnabled() override { return g_connman != nullptr; } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 96e4f8cf0bb..44a0b1743ed 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -158,6 +158,9 @@ public: //! Check if pruning is enabled. virtual bool getPruneMode() = 0; + + //! Check if p2p enabled. + virtual bool p2pEnabled() = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 09e91dc5880..6e11fe936c0 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -318,7 +318,7 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet if (nValue > curBalance) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds"); - if (pwallet->GetBroadcastTransactions() && !g_connman) { + if (pwallet->GetBroadcastTransactions() && !pwallet->chain().p2pEnabled()) { throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); } @@ -858,7 +858,7 @@ static UniValue sendmany(const JSONRPCRequest& request) auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); - if (pwallet->GetBroadcastTransactions() && !g_connman) { + if (pwallet->GetBroadcastTransactions() && !pwallet->chain().p2pEnabled()) { throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); } @@ -2690,7 +2690,7 @@ static UniValue resendwallettransactions(const JSONRPCRequest& request) }.ToString() ); - if (!g_connman) + if (!pwallet->chain().p2pEnabled()) throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); auto locked_chain = pwallet->chain().lock(); From 6d6bcc77c058fbcfd39c7e3050bbe82fc89024cf Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 19:34:54 -0400 Subject: [PATCH 09/16] Remove use of g_connman / PushInventory in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 6 ++++++ src/interfaces/chain.h | 3 +++ src/interfaces/wallet.cpp | 2 +- src/wallet/feebumper.cpp | 2 +- src/wallet/rpcwallet.cpp | 6 +++--- src/wallet/test/wallet_tests.cpp | 2 +- src/wallet/wallet.cpp | 20 ++++++++------------ src/wallet/wallet.h | 6 +++--- 8 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 8493c5de70b..4abb1e638fa 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -199,6 +200,11 @@ public: auto it_mp = ::mempool.mapTx.find(txid); return it_mp != ::mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1; } + void relayTransaction(const uint256& txid) override + { + CInv inv(MSG_TX, txid); + g_connman->ForEachNode([&inv](CNode* node) { node->PushInventory(inv); }); + } void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) override { ::mempool.GetTransactionAncestry(txid, ancestors, descendants); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 44a0b1743ed..b4a458cba6b 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -141,6 +141,9 @@ public: //! Check if transaction has descendants in mempool. virtual bool hasDescendantsInMempool(const uint256& txid) = 0; + //! Relay transaction. + virtual void relayTransaction(const uint256& txid) = 0; + //! Calculate mempool ancestor and descendant counts for the given transaction. virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index f3a0b416dbe..7abbee0912f 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -56,7 +56,7 @@ public: auto locked_chain = m_wallet.chain().lock(); LOCK(m_wallet.cs_wallet); CValidationState state; - if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, g_connman.get(), state)) { + if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, state)) { reject_reason = state.GetRejectReason(); return false; } diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index dd4b2fad4be..a1c3a21d4b1 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -245,7 +245,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti CReserveKey reservekey(wallet); CValidationState state; - if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, reservekey, g_connman.get(), state)) { + if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, reservekey, state)) { // NOTE: CommitTransaction never returns false, so this should never happen. errors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state))); return Result::WALLET_ERROR; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 6e11fe936c0..8d1bd3a1696 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -340,7 +340,7 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet throw JSONRPCError(RPC_WALLET_ERROR, strError); } CValidationState state; - if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, reservekey, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, reservekey, state)) { strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strError); } @@ -946,7 +946,7 @@ static UniValue sendmany(const JSONRPCRequest& request) if (!fCreated) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; - if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, keyChange, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, keyChange, state)) { strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); } @@ -2700,7 +2700,7 @@ static UniValue resendwallettransactions(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet transaction broadcasting is disabled with -walletbroadcast"); } - std::vector txids = pwallet->ResendWalletTransactionsBefore(*locked_chain, GetTime(), g_connman.get()); + std::vector txids = pwallet->ResendWalletTransactionsBefore(*locked_chain, GetTime()); UniValue result(UniValue::VARR); for (const uint256& txid : txids) { diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index e674b2faeaa..af57dbf5f6d 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -368,7 +368,7 @@ public: CCoinControl dummy; BOOST_CHECK(wallet->CreateTransaction(*m_locked_chain, {recipient}, tx, reservekey, fee, changePos, error, dummy)); CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reservekey, nullptr, state)); + BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reservekey, state)); CMutableTransaction blocktx; { LOCK(wallet->cs_wallet); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a0718309b92..8746fb556b6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1890,7 +1890,7 @@ void CWallet::ReacceptWalletTransactions() } } -bool CWalletTx::RelayWalletTransaction(interfaces::Chain::Lock& locked_chain, CConnman* connman) +bool CWalletTx::RelayWalletTransaction(interfaces::Chain::Lock& locked_chain) { assert(pwallet->GetBroadcastTransactions()); if (!IsCoinBase() && !isAbandoned() && GetDepthInMainChain(locked_chain) == 0) @@ -1899,12 +1899,8 @@ bool CWalletTx::RelayWalletTransaction(interfaces::Chain::Lock& locked_chain, CC /* GetDepthInMainChain already catches known conflicts. */ if (InMempool() || AcceptToMemoryPool(locked_chain, maxTxFee, state)) { pwallet->WalletLogPrintf("Relaying wtx %s\n", GetHash().ToString()); - if (connman) { - CInv inv(MSG_TX, GetHash()); - connman->ForEachNode([&inv](CNode* pnode) - { - pnode->PushInventory(inv); - }); + if (pwallet->chain().p2pEnabled()) { + pwallet->chain().relayTransaction(GetHash()); return true; } } @@ -2113,7 +2109,7 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const return CTransaction(tx1) == CTransaction(tx2); } -std::vector CWallet::ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime, CConnman* connman) +std::vector CWallet::ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime) { std::vector result; @@ -2132,7 +2128,7 @@ std::vector CWallet::ResendWalletTransactionsBefore(interfaces::Chain:: for (const std::pair& item : mapSorted) { CWalletTx& wtx = *item.second; - if (wtx.RelayWalletTransaction(locked_chain, connman)) + if (wtx.RelayWalletTransaction(locked_chain)) result.push_back(wtx.GetHash()); } return result; @@ -2157,7 +2153,7 @@ void CWallet::ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman // Rebroadcast unconfirmed txes older than 5 minutes before the last // block was found: auto locked_chain = chain().assumeLocked(); // Temporary. Removed in upcoming lock cleanup - std::vector relayed = ResendWalletTransactionsBefore(*locked_chain, nBestBlockTime-5*60, connman); + std::vector relayed = ResendWalletTransactionsBefore(*locked_chain, nBestBlockTime-5*60); if (!relayed.empty()) WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed.size()); } @@ -3147,7 +3143,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std /** * Call after CreateTransaction unless you want to abort */ -bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, CReserveKey& reservekey, CConnman* connman, CValidationState& state) +bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, CReserveKey& reservekey, CValidationState& state) { { auto locked_chain = chain().lock(); @@ -3188,7 +3184,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", FormatStateMessage(state)); // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure. } else { - wtx.RelayWalletTransaction(*locked_chain, connman); + wtx.RelayWalletTransaction(*locked_chain); } } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f215765dab5..41bb4bd8c72 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -535,7 +535,7 @@ public: int64_t GetTxTime() const; // RelayWalletTransaction may only be called if fBroadcastTransactions! - bool RelayWalletTransaction(interfaces::Chain::Lock& locked_chain, CConnman* connman); + bool RelayWalletTransaction(interfaces::Chain::Lock& locked_chain); /** Pass this transaction to the mempool. Fails if absolute fee exceeds absurd fee. */ bool AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, const CAmount& nAbsurdFee, CValidationState& state); @@ -944,7 +944,7 @@ public: void ReacceptWalletTransactions(); void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override EXCLUSIVE_LOCKS_REQUIRED(cs_main); // ResendWalletTransactionsBefore may only be called if fBroadcastTransactions! - std::vector ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime, CConnman* connman); + std::vector ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime); CAmount GetBalance(const isminefilter& filter=ISMINE_SPENDABLE, const int min_depth=0) const; CAmount GetUnconfirmedBalance() const; CAmount GetImmatureBalance() const; @@ -969,7 +969,7 @@ public: */ bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign = true); - bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, CReserveKey& reservekey, CConnman* connman, CValidationState& state); + bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, CReserveKey& reservekey, CValidationState& state); bool DummySignTx(CMutableTransaction &txNew, const std::set &txouts, bool use_max_sig = false) const { From c5e59a96a8561b6a0bcaba0ede2d53dbaac113b0 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 22:54:31 -0400 Subject: [PATCH 10/16] Remove uses of GetAdjustedTime in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 2 ++ src/interfaces/chain.h | 3 +++ src/wallet/wallet.cpp | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 4abb1e638fa..a654f55f554 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -237,6 +238,7 @@ public: } bool getPruneMode() override { return ::fPruneMode; } bool p2pEnabled() override { return g_connman != nullptr; } + int64_t getAdjustedTime() override { return GetAdjustedTime(); } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index b4a458cba6b..c5aa67d0d76 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -164,6 +164,9 @@ public: //! Check if p2p enabled. virtual bool p2pEnabled() = 0; + + //! Get adjusted time. + virtual int64_t getAdjustedTime() = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8746fb556b6..84521047458 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -940,7 +940,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) wtx.BindWallet(this); bool fInsertedNew = ret.second; if (fInsertedNew) { - wtx.nTimeReceived = GetAdjustedTime(); + wtx.nTimeReceived = chain().getAdjustedTime(); wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx); From e2c8ba9f6e782e2545b71e9e34b967c69e18c7f0 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 28 Jul 2017 22:59:47 -0400 Subject: [PATCH 11/16] Remove uses of InitMessage/Warning/Error in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 4 +++ src/interfaces/chain.h | 9 ++++++ src/wallet/init.cpp | 16 ++++++---- src/wallet/wallet.cpp | 66 ++++++++++++++++++++-------------------- 4 files changed, 56 insertions(+), 39 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index a654f55f554..4f60448958f 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -239,6 +240,9 @@ public: bool getPruneMode() override { return ::fPruneMode; } bool p2pEnabled() override { return g_connman != nullptr; } int64_t getAdjustedTime() override { return GetAdjustedTime(); } + void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); } + void initWarning(const std::string& message) override { InitWarning(message); } + void initError(const std::string& message) override { InitError(message); } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index c5aa67d0d76..b1e3f594526 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -167,6 +167,15 @@ public: //! Get adjusted time. virtual int64_t getAdjustedTime() = 0; + + //! Send init message. + virtual void initMessage(const std::string& message) = 0; + + //! Send init warning. + virtual void initWarning(const std::string& message) = 0; + + //! Send init error. + virtual void initError(const std::string& message) = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 20d540c8db8..7ad343c15fc 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -138,12 +138,15 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal // The canonical path cleans the path, preventing >1 Berkeley environment instances for the same directory fs::path canonical_wallet_dir = fs::canonical(wallet_dir, error); if (error || !fs::exists(wallet_dir)) { - return InitError(strprintf(_("Specified -walletdir \"%s\" does not exist"), wallet_dir.string())); + chain.initError(strprintf(_("Specified -walletdir \"%s\" does not exist"), wallet_dir.string())); + return false; } else if (!fs::is_directory(wallet_dir)) { - return InitError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), wallet_dir.string())); + chain.initError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), wallet_dir.string())); + return false; // The canonical path transforms relative paths into absolute ones, so we check the non-canonical version } else if (!wallet_dir.is_absolute()) { - return InitError(strprintf(_("Specified -walletdir \"%s\" is a relative path"), wallet_dir.string())); + chain.initError(strprintf(_("Specified -walletdir \"%s\" is a relative path"), wallet_dir.string())); + return false; } gArgs.ForceSetArg("-walletdir", canonical_wallet_dir.string()); } @@ -164,14 +167,15 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal WalletLocation location(wallet_file); if (!wallet_paths.insert(location.GetPath()).second) { - return InitError(strprintf(_("Error loading wallet %s. Duplicate -wallet filename specified."), wallet_file)); + chain.initError(strprintf(_("Error loading wallet %s. Duplicate -wallet filename specified."), wallet_file)); + return false; } std::string error_string; std::string warning_string; bool verify_success = CWallet::Verify(chain, location, salvage_wallet, error_string, warning_string); - if (!error_string.empty()) InitError(error_string); - if (!warning_string.empty()) InitWarning(warning_string); + if (!error_string.empty()) chain.initError(error_string); + if (!warning_string.empty()) chain.initWarning(warning_string); if (!verify_success) return false; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 84521047458..2a1744fa683 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4087,17 +4087,17 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, std::vector vWtx; if (gArgs.GetBoolArg("-zapwallettxes", false)) { - uiInterface.InitMessage(_("Zapping all transactions from wallet...")); + chain.initMessage(_("Zapping all transactions from wallet...")); std::unique_ptr tempWallet = MakeUnique(chain, location, WalletDatabase::Create(location.GetPath())); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DBErrors::LOAD_OK) { - InitError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); + chain.initError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); return nullptr; } } - uiInterface.InitMessage(_("Loading wallet...")); + chain.initMessage(_("Loading wallet...")); int64_t nStart = GetTimeMillis(); bool fFirstRun = true; @@ -4108,26 +4108,26 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { - InitError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); + chain.initError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); return nullptr; } else if (nLoadWalletRet == DBErrors::NONCRITICAL_ERROR) { - InitWarning(strprintf(_("Error reading %s! All keys read correctly, but transaction data" - " or address book entries might be missing or incorrect."), + chain.initWarning(strprintf(_("Error reading %s! All keys read correctly, but transaction data" + " or address book entries might be missing or incorrect."), walletFile)); } else if (nLoadWalletRet == DBErrors::TOO_NEW) { - InitError(strprintf(_("Error loading %s: Wallet requires newer version of %s"), walletFile, _(PACKAGE_NAME))); + chain.initError(strprintf(_("Error loading %s: Wallet requires newer version of %s"), walletFile, _(PACKAGE_NAME))); return nullptr; } else if (nLoadWalletRet == DBErrors::NEED_REWRITE) { - InitError(strprintf(_("Wallet needed to be rewritten: restart %s to complete"), _(PACKAGE_NAME))); + chain.initError(strprintf(_("Wallet needed to be rewritten: restart %s to complete"), _(PACKAGE_NAME))); return nullptr; } else { - InitError(strprintf(_("Error loading %s"), walletFile)); + chain.initError(strprintf(_("Error loading %s"), walletFile)); return nullptr; } } @@ -4146,7 +4146,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->WalletLogPrintf("Allowing wallet upgrade up to %i\n", nMaxVersion); if (nMaxVersion < walletInstance->GetVersion()) { - InitError(_("Cannot downgrade wallet")); + chain.initError(_("Cannot downgrade wallet")); return nullptr; } walletInstance->SetMaxVersion(nMaxVersion); @@ -4159,7 +4159,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // Do not upgrade versions to any version between HD_SPLIT and FEATURE_PRE_SPLIT_KEYPOOL unless already supporting HD_SPLIT int max_version = walletInstance->nWalletVersion; if (!walletInstance->CanSupportFeature(FEATURE_HD_SPLIT) && max_version >=FEATURE_HD_SPLIT && max_version < FEATURE_PRE_SPLIT_KEYPOOL) { - InitError(_("Cannot upgrade a non HD split wallet without upgrading to support pre split keypool. Please use -upgradewallet=169900 or -upgradewallet with no version specified.")); + chain.initError(_("Cannot upgrade a non HD split wallet without upgrading to support pre split keypool. Please use -upgradewallet=169900 or -upgradewallet with no version specified.")); return nullptr; } @@ -4187,7 +4187,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // Regenerate the keypool if upgraded to HD if (hd_upgrade) { if (!walletInstance->TopUpKeyPool()) { - InitError(_("Unable to generate keys")); + chain.initError(_("Unable to generate keys")); return nullptr; } } @@ -4211,7 +4211,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // Top up the keypool if (walletInstance->CanGenerateKeys() && !walletInstance->TopUpKeyPool()) { - InitError(_("Unable to generate initial keys")); + chain.initError(_("Unable to generate initial keys")); return nullptr; } @@ -4219,34 +4219,34 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->ChainStateFlushed(locked_chain->getTipLocator()); } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { // Make it impossible to disable private keys after creation - InitError(strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile)); + chain.initError(strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile)); return NULL; } else if (walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { LOCK(walletInstance->cs_KeyStore); if (!walletInstance->mapKeys.empty() || !walletInstance->mapCryptedKeys.empty()) { - InitWarning(strprintf(_("Warning: Private keys detected in wallet {%s} with disabled private keys"), walletFile)); + chain.initWarning(strprintf(_("Warning: Private keys detected in wallet {%s} with disabled private keys"), walletFile)); } } if (!gArgs.GetArg("-addresstype", "").empty() && !ParseOutputType(gArgs.GetArg("-addresstype", ""), walletInstance->m_default_address_type)) { - InitError(strprintf("Unknown address type '%s'", gArgs.GetArg("-addresstype", ""))); + chain.initError(strprintf("Unknown address type '%s'", gArgs.GetArg("-addresstype", ""))); return nullptr; } if (!gArgs.GetArg("-changetype", "").empty() && !ParseOutputType(gArgs.GetArg("-changetype", ""), walletInstance->m_default_change_type)) { - InitError(strprintf("Unknown change type '%s'", gArgs.GetArg("-changetype", ""))); + chain.initError(strprintf("Unknown change type '%s'", gArgs.GetArg("-changetype", ""))); return nullptr; } if (gArgs.IsArgSet("-mintxfee")) { CAmount n = 0; if (!ParseMoney(gArgs.GetArg("-mintxfee", ""), n) || 0 == n) { - InitError(AmountErrMsg("mintxfee", gArgs.GetArg("-mintxfee", ""))); + chain.initError(AmountErrMsg("mintxfee", gArgs.GetArg("-mintxfee", ""))); return nullptr; } if (n > HIGH_TX_FEE_PER_KB) { - InitWarning(AmountHighWarn("-mintxfee") + " " + - _("This is the minimum transaction fee you pay on every transaction.")); + chain.initWarning(AmountHighWarn("-mintxfee") + " " + + _("This is the minimum transaction fee you pay on every transaction.")); } walletInstance->m_min_fee = CFeeRate(n); } @@ -4255,12 +4255,12 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, if (gArgs.IsArgSet("-fallbackfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) { - InitError(strprintf(_("Invalid amount for -fallbackfee=: '%s'"), gArgs.GetArg("-fallbackfee", ""))); + chain.initError(strprintf(_("Invalid amount for -fallbackfee=: '%s'"), gArgs.GetArg("-fallbackfee", ""))); return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - InitWarning(AmountHighWarn("-fallbackfee") + " " + - _("This is the transaction fee you may pay when fee estimates are not available.")); + chain.initWarning(AmountHighWarn("-fallbackfee") + " " + + _("This is the transaction fee you may pay when fee estimates are not available.")); } walletInstance->m_fallback_fee = CFeeRate(nFeePerK); walletInstance->m_allow_fallback_fee = nFeePerK != 0; //disable fallback fee in case value was set to 0, enable if non-null value @@ -4268,28 +4268,28 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, if (gArgs.IsArgSet("-discardfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-discardfee", ""), nFeePerK)) { - InitError(strprintf(_("Invalid amount for -discardfee=: '%s'"), gArgs.GetArg("-discardfee", ""))); + chain.initError(strprintf(_("Invalid amount for -discardfee=: '%s'"), gArgs.GetArg("-discardfee", ""))); return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - InitWarning(AmountHighWarn("-discardfee") + " " + - _("This is the transaction fee you may discard if change is smaller than dust at this level")); + chain.initWarning(AmountHighWarn("-discardfee") + " " + + _("This is the transaction fee you may discard if change is smaller than dust at this level")); } walletInstance->m_discard_rate = CFeeRate(nFeePerK); } if (gArgs.IsArgSet("-paytxfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-paytxfee", ""), nFeePerK)) { - InitError(AmountErrMsg("paytxfee", gArgs.GetArg("-paytxfee", ""))); + chain.initError(AmountErrMsg("paytxfee", gArgs.GetArg("-paytxfee", ""))); return nullptr; } if (nFeePerK > HIGH_TX_FEE_PER_KB) { - InitWarning(AmountHighWarn("-paytxfee") + " " + - _("This is the transaction fee you will pay if you send a transaction.")); + chain.initWarning(AmountHighWarn("-paytxfee") + " " + + _("This is the transaction fee you will pay if you send a transaction.")); } walletInstance->m_pay_tx_fee = CFeeRate(nFeePerK, 1000); if (walletInstance->m_pay_tx_fee < ::minRelayTxFee) { - InitError(strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)"), + chain.initError(strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)"), gArgs.GetArg("-paytxfee", ""), ::minRelayTxFee.ToString())); return nullptr; } @@ -4338,12 +4338,12 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } if (rescan_height != block_height) { - InitError(_("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)")); + chain.initError(_("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)")); return nullptr; } } - uiInterface.InitMessage(_("Rescanning...")); + chain.initMessage(_("Rescanning...")); walletInstance->WalletLogPrintf("Rescanning last %i blocks (from block %i)...\n", *tip_height - rescan_height, rescan_height); // No need to read and scan block if block was created before @@ -4358,7 +4358,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, { WalletRescanReserver reserver(walletInstance.get()); if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(locked_chain->getBlockHash(rescan_height), {} /* stop block */, reserver, true /* update */).status)) { - InitError(_("Failed to rescan the wallet during initialization")); + chain.initError(_("Failed to rescan the wallet during initialization")); return nullptr; } } From d02b34c8a8bd446c9620fe626b4379617f9a9639 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Mon, 31 Jul 2017 16:31:29 -0400 Subject: [PATCH 12/16] Remove use of AcceptToMemoryPool in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 8 ++++++++ src/interfaces/chain.h | 13 ++++++++++++- src/wallet/wallet.cpp | 13 +++++-------- src/wallet/wallet.h | 2 +- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 4f60448958f..9f02514df3e 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -146,6 +147,12 @@ class LockImpl : public Chain::Lock LockAnnotation lock(::cs_main); return CheckFinalTx(tx); } + bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) override + { + LockAnnotation lock(::cs_main); + return AcceptToMemoryPool(::mempool, state, tx, nullptr /* missing inputs */, nullptr /* txn replaced */, + false /* bypass limits */, absurd_fee); + } }; class LockingStateImpl : public LockImpl, public UniqueLock @@ -237,6 +244,7 @@ public: { return ::mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); } + CAmount maxTxFee() override { return ::maxTxFee; } bool getPruneMode() override { return ::fPruneMode; } bool p2pEnabled() override { return g_connman != nullptr; } int64_t getAdjustedTime() override { return GetAdjustedTime(); } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index b1e3f594526..be486bd4fce 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -7,6 +7,7 @@ #include // For Optional and nullopt #include // For RBFTransactionState +#include // For CTransactionRef #include #include @@ -16,7 +17,7 @@ class CBlock; class CScheduler; -class CTransaction; +class CValidationState; class uint256; struct CBlockLocator; struct FeeCalculation; @@ -109,6 +110,10 @@ public: //! Check if transaction will be final given chain height current time. virtual bool checkFinalTx(const CTransaction& tx) = 0; + + //! Add transaction to memory pool if the transaction fee is below the + //! amount specified by absurd_fee (as a safeguard). */ + virtual bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) = 0; }; //! Return Lock interface. Chain is locked when this is called, and @@ -159,6 +164,12 @@ public: //! Pool min fee. virtual CFeeRate mempoolMinFee() = 0; + //! Get 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/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2a1744fa683..fb4bd8811fb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1886,7 +1886,7 @@ void CWallet::ReacceptWalletTransactions() for (const std::pair& item : mapSorted) { CWalletTx& wtx = *(item.second); CValidationState state; - wtx.AcceptToMemoryPool(*locked_chain, maxTxFee, state); + wtx.AcceptToMemoryPool(*locked_chain, state); } } @@ -1897,7 +1897,7 @@ bool CWalletTx::RelayWalletTransaction(interfaces::Chain::Lock& locked_chain) { CValidationState state; /* GetDepthInMainChain already catches known conflicts. */ - if (InMempool() || AcceptToMemoryPool(locked_chain, maxTxFee, state)) { + if (InMempool() || AcceptToMemoryPool(locked_chain, state)) { pwallet->WalletLogPrintf("Relaying wtx %s\n", GetHash().ToString()); if (pwallet->chain().p2pEnabled()) { pwallet->chain().relayTransaction(GetHash()); @@ -3180,7 +3180,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve if (fBroadcastTransactions) { // Broadcast - if (!wtx.AcceptToMemoryPool(*locked_chain, maxTxFee, state)) { + if (!wtx.AcceptToMemoryPool(*locked_chain, state)) { WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", FormatStateMessage(state)); // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure. } else { @@ -4474,17 +4474,14 @@ bool CMerkleTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const return GetBlocksToMaturity(locked_chain) > 0; } -bool CWalletTx::AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, const CAmount& nAbsurdFee, CValidationState& state) +bool CWalletTx::AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, CValidationState& state) { - LockAnnotation lock(::cs_main); // Temporary, for AcceptToMemoryPool below. Removed in upcoming commit. - // We must set fInMempool here - while it will be re-set to true by the // entered-mempool callback, if we did not there would be a race where a // 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 = ::AcceptToMemoryPool(mempool, state, tx, nullptr /* pfMissingInputs */, - nullptr /* plTxnReplaced */, false /* bypass_limits */, nAbsurdFee); + bool ret = locked_chain.submitToMemoryPool(tx, pwallet->chain().maxTxFee(), state); fInMempool |= ret; return ret; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 41bb4bd8c72..3cfcf7a27d2 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -538,7 +538,7 @@ public: bool RelayWalletTransaction(interfaces::Chain::Lock& locked_chain); /** Pass this transaction to the mempool. Fails if absolute fee exceeds absurd fee. */ - bool AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, const CAmount& nAbsurdFee, CValidationState& state); + bool AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, CValidationState& state); // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation From 318f41fb2cae0a46b4e4be49156562b8ed640f0c Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Sun, 8 Apr 2018 14:37:50 -0400 Subject: [PATCH 13/16] circular-dependencies: Avoid treating some .h/.cpp files as a unit This avoids a bogus circular dependency error in the next commit: interfaces/chain -> interfaces/wallet -> wallet/wallet -> interfaces/chain Which is incorrect, because interfaces/chain.cpp depends only on the interfaces/wallet.h file, not the interfaces/wallet.cpp file, and it is wrong to treat these as a unit. Inside the interfaces directory, .h files contain abstract class definitions and .cpp files contain implementations of those classes, so you don't need to link against .cpp files if you're only using the abstract class definition in the .h file. An alternative fix might be to rename all the cpp files in the interfaces directory like: chain.cpp->chain_impl.cpp, node.cpp->node_impl.cpp. But just getting the linter to treat these files as independent dependencies seemed like it would allow keeping code organization straightforward and avoiding the need to rename things. --- contrib/devtools/circular-dependencies.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/contrib/devtools/circular-dependencies.py b/contrib/devtools/circular-dependencies.py index abfa5ed5ae7..2e4657f1dd5 100755 --- a/contrib/devtools/circular-dependencies.py +++ b/contrib/devtools/circular-dependencies.py @@ -8,9 +8,18 @@ MAPPING = { 'core_write.cpp': 'core_io.cpp', } +# Directories with header-based modules, where the assumption that .cpp files +# define functions and variables declared in corresponding .h files is +# incorrect. +HEADER_MODULE_PATHS = [ + 'interfaces/' +] + def module_name(path): if path in MAPPING: path = MAPPING[path] + if any(path.startswith(dirpath) for dirpath in HEADER_MODULE_PATHS): + return path if path.endswith(".h"): return path[:-2] if path.endswith(".c"): From 1106a6fde4bfde31a16de45e4cc84ed5da05c5a4 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Sun, 8 Apr 2018 14:37:50 -0400 Subject: [PATCH 14/16] Remove use of uiInterface.LoadWallet in wallet code This also changes the uiInterface.LoadWallet signal argument type from shared_ptr to unique_ptr because CWallet is an internal wallet class that shouldn't be used in non-wallet code (and also can't be passed across process boundaries). This commit does not change behavior. --- src/Makefile.am | 2 -- src/Makefile.bench.include | 2 -- src/interfaces/chain.cpp | 2 ++ src/interfaces/chain.h | 5 +++++ src/interfaces/node.cpp | 2 +- src/ui_interface.cpp | 2 +- src/ui_interface.h | 7 +++++-- src/wallet/wallet.cpp | 3 ++- 8 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 650e347da38..a2418e47935 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -562,8 +562,6 @@ bitcoin_wallet_LDADD = \ $(LIBBITCOIN_WALLET_TOOL) \ $(LIBBITCOIN_WALLET) \ $(LIBBITCOIN_SERVER) \ - $(LIBBITCOIN_WALLET) \ - $(LIBBITCOIN_SERVER) \ $(LIBBITCOIN_COMMON) \ $(LIBBITCOIN_CONSENSUS) \ $(LIBBITCOIN_UTIL) \ diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 829e6446436..77fb416b9c6 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -37,8 +37,6 @@ nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES) bench_bench_bitcoin_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(EVENT_CLFAGS) $(EVENT_PTHREADS_CFLAGS) -I$(builddir)/bench/ bench_bench_bitcoin_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) bench_bench_bitcoin_LDADD = \ - $(LIBBITCOIN_WALLET) \ - $(LIBBITCOIN_SERVER) \ $(LIBBITCOIN_WALLET) \ $(LIBBITCOIN_SERVER) \ $(LIBBITCOIN_COMMON) \ diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 9f02514df3e..725b485e51e 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -251,6 +252,7 @@ public: void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); } void initWarning(const std::string& message) override { InitWarning(message); } void initError(const std::string& message) override { InitError(message); } + void loadWallet(std::unique_ptr wallet) override { ::uiInterface.LoadWallet(wallet); } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index be486bd4fce..116656fef45 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -24,6 +24,8 @@ struct FeeCalculation; namespace interfaces { +class Wallet; + //! Interface for giving wallet processes access to blockchain state. class Chain { @@ -187,6 +189,9 @@ public: //! Send init error. virtual void initError(const std::string& message) = 0; + + //! Send wallet load notification to the GUI. + virtual void loadWallet(std::unique_ptr wallet) = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 96bde7e9f25..6f7dce0c246 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -275,7 +275,7 @@ public: } std::unique_ptr handleLoadWallet(LoadWalletFn fn) override { - return MakeHandler(::uiInterface.LoadWallet_connect([fn](std::shared_ptr wallet) { fn(MakeWallet(wallet)); })); + return MakeHandler(::uiInterface.LoadWallet_connect([fn](std::unique_ptr& wallet) { fn(std::move(wallet)); })); } std::unique_ptr handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override { diff --git a/src/ui_interface.cpp b/src/ui_interface.cpp index 947d7e2308d..16ab24686b1 100644 --- a/src/ui_interface.cpp +++ b/src/ui_interface.cpp @@ -52,7 +52,7 @@ void CClientUIInterface::InitMessage(const std::string& message) { return g_ui_s void CClientUIInterface::NotifyNumConnectionsChanged(int newNumConnections) { return g_ui_signals.NotifyNumConnectionsChanged(newNumConnections); } void CClientUIInterface::NotifyNetworkActiveChanged(bool networkActive) { return g_ui_signals.NotifyNetworkActiveChanged(networkActive); } void CClientUIInterface::NotifyAlertChanged() { return g_ui_signals.NotifyAlertChanged(); } -void CClientUIInterface::LoadWallet(std::shared_ptr wallet) { return g_ui_signals.LoadWallet(wallet); } +void CClientUIInterface::LoadWallet(std::unique_ptr& wallet) { return g_ui_signals.LoadWallet(wallet); } void CClientUIInterface::ShowProgress(const std::string& title, int nProgress, bool resume_possible) { return g_ui_signals.ShowProgress(title, nProgress, resume_possible); } void CClientUIInterface::NotifyBlockTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyBlockTip(b, i); } void CClientUIInterface::NotifyHeaderTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyHeaderTip(b, i); } diff --git a/src/ui_interface.h b/src/ui_interface.h index fe466b3ca40..f1aebce3bb4 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -11,7 +11,6 @@ #include #include -class CWallet; class CBlockIndex; namespace boost { namespace signals2 { @@ -19,6 +18,10 @@ class connection; } } // namespace boost +namespace interfaces { +class Wallet; +} // namespace interfaces + /** General change type (added, updated, removed). */ enum ChangeType { @@ -102,7 +105,7 @@ public: ADD_SIGNALS_DECL_WRAPPER(NotifyAlertChanged, void, ); /** A wallet has been loaded. */ - ADD_SIGNALS_DECL_WRAPPER(LoadWallet, void, std::shared_ptr wallet); + ADD_SIGNALS_DECL_WRAPPER(LoadWallet, void, std::unique_ptr& wallet); /** * Show progress e.g. for verifychain. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fb4bd8811fb..3d64adec53e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -4391,7 +4392,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } } - uiInterface.LoadWallet(walletInstance); + chain.loadWallet(interfaces::MakeWallet(walletInstance)); // Register with the validation interface. It's ok to do this after rescan since we're still holding cs_main. RegisterValidationInterface(walletInstance.get()); From a1df1b48a80bf122efa73677ff72577ec0103a3b Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 10 Jul 2018 15:48:03 -0400 Subject: [PATCH 15/16] Remove use of IsInitialBlockDownload in wallet code This commit does not change behavior. --- src/interfaces/chain.cpp | 1 + src/interfaces/chain.h | 3 +++ src/wallet/rpcwallet.cpp | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 725b485e51e..fb634e73da9 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -248,6 +248,7 @@ public: CAmount maxTxFee() override { return ::maxTxFee; } bool getPruneMode() override { return ::fPruneMode; } bool p2pEnabled() override { return g_connman != nullptr; } + bool isInitialBlockDownload() override { return IsInitialBlockDownload(); } int64_t getAdjustedTime() override { return GetAdjustedTime(); } void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); } void initWarning(const std::string& message) override { InitWarning(message); } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 116656fef45..60f8570e366 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -178,6 +178,9 @@ public: //! Check if p2p enabled. virtual bool p2pEnabled() = 0; + // Check if in IBD. + virtual bool isInitialBlockDownload() = 0; + //! Get adjusted time. virtual int64_t getAdjustedTime() = 0; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8d1bd3a1696..a30df9f037d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3889,7 +3889,7 @@ UniValue sethdseed(const JSONRPCRequest& request) }.ToString()); } - if (IsInitialBlockDownload()) { + if (pwallet->chain().isInitialBlockDownload()) { throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set a new HD seed while still in Initial Block Download"); } From f7efd87c8fb49f82e268a95e989909d453500e2b Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Fri, 1 Feb 2019 17:35:51 -0500 Subject: [PATCH 16/16] Change brace formatting Suggested https://github.com/bitcoin/bitcoin/pull/15288#pullrequestreview-197915100 --- src/wallet/rpcdump.cpp | 12 ++++++++---- src/wallet/rpcwallet.cpp | 12 ++++++++---- src/wallet/wallet.cpp | 12 +++++++----- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 046a6567a5d..b8d5c9d0e11 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -157,8 +157,9 @@ UniValue importprivkey(const JSONRPCRequest& request) if (!request.params[2].isNull()) fRescan = request.params[2].get_bool(); - if (fRescan && pwallet->chain().getPruneMode()) + if (fRescan && pwallet->chain().getPruneMode()) { throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + } if (fRescan && !reserver.reserve()) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); @@ -313,8 +314,9 @@ UniValue importaddress(const JSONRPCRequest& request) if (!request.params[2].isNull()) fRescan = request.params[2].get_bool(); - if (fRescan && pwallet->chain().getPruneMode()) + if (fRescan && pwallet->chain().getPruneMode()) { throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + } WalletRescanReserver reserver(pwallet); if (fRescan && !reserver.reserve()) { @@ -501,8 +503,9 @@ UniValue importpubkey(const JSONRPCRequest& request) if (!request.params[2].isNull()) fRescan = request.params[2].get_bool(); - if (fRescan && pwallet->chain().getPruneMode()) + if (fRescan && pwallet->chain().getPruneMode()) { throw JSONRPCError(RPC_WALLET_ERROR, "Rescan is disabled in pruned mode"); + } WalletRescanReserver reserver(pwallet); if (fRescan && !reserver.reserve()) { @@ -562,8 +565,9 @@ UniValue importwallet(const JSONRPCRequest& request) }, }.ToString()); - if (pwallet->chain().getPruneMode()) + if (pwallet->chain().getPruneMode()) { throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled in pruned mode"); + } WalletRescanReserver reserver(pwallet); if (!reserver.reserve()) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a30df9f037d..2e1fb50e70b 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -628,8 +628,9 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request) CAmount nAmount = 0; for (const std::pair& pairWtx : pwallet->mapWallet) { const CWalletTx& wtx = pairWtx.second; - if (wtx.IsCoinBase() || !locked_chain->checkFinalTx(*wtx.tx)) + if (wtx.IsCoinBase() || !locked_chain->checkFinalTx(*wtx.tx)) { continue; + } for (const CTxOut& txout : wtx.tx->vout) if (txout.scriptPubKey == scriptPubKey) @@ -693,8 +694,9 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request) CAmount nAmount = 0; for (const std::pair& pairWtx : pwallet->mapWallet) { const CWalletTx& wtx = pairWtx.second; - if (wtx.IsCoinBase() || !locked_chain->checkFinalTx(*wtx.tx)) + if (wtx.IsCoinBase() || !locked_chain->checkFinalTx(*wtx.tx)) { continue; + } for (const CTxOut& txout : wtx.tx->vout) { @@ -1078,8 +1080,9 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, CWallet * co for (const std::pair& pairWtx : pwallet->mapWallet) { const CWalletTx& wtx = pairWtx.second; - if (wtx.IsCoinBase() || !locked_chain.checkFinalTx(*wtx.tx)) + if (wtx.IsCoinBase() || !locked_chain.checkFinalTx(*wtx.tx)) { continue; + } int nDepth = wtx.GetDepthInMainChain(locked_chain); if (nDepth < nMinDepth) @@ -2690,8 +2693,9 @@ static UniValue resendwallettransactions(const JSONRPCRequest& request) }.ToString() ); - if (!pwallet->chain().p2pEnabled()) + if (!pwallet->chain().p2pEnabled()) { throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); + } auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3d64adec53e..daf99ad341e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2073,8 +2073,9 @@ bool CWalletTx::InMempool() const bool CWalletTx::IsTrusted(interfaces::Chain::Lock& locked_chain) const { // Quick answer in most cases - if (!locked_chain.checkFinalTx(*tx)) + if (!locked_chain.checkFinalTx(*tx)) { return false; + } int nDepth = GetDepthInMainChain(locked_chain); if (nDepth >= 1) return true; @@ -2129,8 +2130,9 @@ std::vector CWallet::ResendWalletTransactionsBefore(interfaces::Chain:: for (const std::pair& item : mapSorted) { CWalletTx& wtx = *item.second; - if (wtx.RelayWalletTransaction(locked_chain)) + if (wtx.RelayWalletTransaction(locked_chain)) { result.push_back(wtx.GetHash()); + } } return result; } @@ -2319,8 +2321,9 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< const uint256& wtxid = entry.first; const CWalletTx* pcoin = &entry.second; - if (!locked_chain.checkFinalTx(*pcoin->tx)) + if (!locked_chain.checkFinalTx(*pcoin->tx)) { continue; + } if (pcoin->IsImmatureCoinBase(locked_chain)) continue; @@ -4331,8 +4334,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, //We can't rescan beyond non-pruned blocks, stop and throw an error //this might happen if a user uses an old wallet within a pruned node // or if he ran -disablewallet for a longer time, then decided to re-enable - if (chain.getPruneMode()) - { + if (chain.getPruneMode()) { int block_height = *tip_height; while (block_height > 0 && locked_chain->haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { --block_height;