From 365e5c420593ee2d3fc43b933fcb4842ed522c3d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Mon, 1 Nov 2021 21:00:13 +0530 Subject: [PATCH 1/7] merge bitcoin#15039: Avoid leaking nLockTime fingerprint when anti-fee-sniping --- src/wallet/wallet.cpp | 95 ++++++++++++++++++----------- test/functional/test_runner.py | 1 + test/functional/wallet_create_tx.py | 35 +++++++++++ test/functional/wallet_txn_clone.py | 2 +- 4 files changed, 98 insertions(+), 35 deletions(-) create mode 100755 test/functional/wallet_create_tx.py diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 87f187ea2a..688fc10004 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3294,6 +3294,66 @@ bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::ve return nValueTotal > 0; } +static bool IsCurrentForAntiFeeSniping(interfaces::Chain::Lock& locked_chain) +{ + if (::ChainstateActive().IsInitialBlockDownload()) { + return false; + } + constexpr int64_t MAX_ANTI_FEE_SNIPING_TIP_AGE = 4 * 60; // in seconds + if (::ChainActive().Tip()->GetBlockTime() < (GetTime() - MAX_ANTI_FEE_SNIPING_TIP_AGE)) { + return false; + } + return true; +} + +/** + * Return a height-based locktime for new transactions (uses the height of the + * current chain tip unless we are not synced with the current chain + */ +static uint32_t GetLocktimeForNewTransaction(interfaces::Chain::Lock& locked_chain) +{ + uint32_t locktime; + // Discourage fee sniping. + // + // For a large miner the value of the transactions in the best block and + // the mempool can exceed the cost of deliberately attempting to mine two + // blocks to orphan the current best block. By setting nLockTime such that + // only the next block can include the transaction, we discourage this + // practice as the height restricted and limited blocksize gives miners + // considering fee sniping fewer options for pulling off this attack. + // + // A simple way to think about this is from the wallet's point of view we + // always want the blockchain to move forward. By setting nLockTime this + // way we're basically making the statement that we only want this + // transaction to appear in the next block; we don't want to potentially + // encourage reorgs by allowing transactions to appear at lower heights + // than the next block in forks of the best chain. + // + // Of course, the subsidy is high enough, and transaction volume low + // enough, that fee sniping isn't a problem yet, but by implementing a fix + // now we ensure code won't be written that makes assumptions about + // nLockTime that preclude a fix later. + if (IsCurrentForAntiFeeSniping(locked_chain)) { + locktime = locked_chain.getHeight().value_or(-1); + + // Secondly occasionally randomly pick a nLockTime even further back, so + // that transactions that are delayed after signing for whatever reason, + // e.g. high-latency mix networks and some CoinJoin implementations, have + // better privacy. + if (GetRandInt(10) == 0) + locktime = std::max(0, (int)locktime - GetRandInt(100)); + } else { + // If our chain is lagging behind, we can't discourage fee sniping nor help + // the privacy of high-latency transactions. To avoid leaking a potentially + // unique "nLockTime fingerprint", set nLockTime to a constant. + locktime = 0; + } + + assert(locktime <= (unsigned int)::ChainActive().Height()); + assert(locktime < LOCKTIME_THRESHOLD); + return locktime; +} + bool CWallet::SelectCoinsGroupedByAddresses(std::vector& vecTallyRet, bool fSkipDenominated, bool fAnonymizable, bool fSkipUnconfirmed, int nMaxOupointsPerAddress) const { auto locked_chain = chain().lock(); @@ -3487,8 +3547,6 @@ bool CWallet::GetBudgetSystemCollateralTX(interfaces::Chain::Lock& locked_chain, bool CWallet::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, int nExtraPayloadSize) { - uint32_t const height = locked_chain.getHeight().value_or(-1); - CAmount nValue = 0; int nChangePosRequest = nChangePosInOut; unsigned int nSubtractFeeFromAmount = 0; @@ -3511,39 +3569,8 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std } CMutableTransaction txNew; + txNew.nLockTime = GetLocktimeForNewTransaction(locked_chain); - // Discourage fee sniping. - // - // For a large miner the value of the transactions in the best block and - // the mempool can exceed the cost of deliberately attempting to mine two - // blocks to orphan the current best block. By setting nLockTime such that - // only the next block can include the transaction, we discourage this - // practice as the height restricted and limited blocksize gives miners - // considering fee sniping fewer options for pulling off this attack. - // - // A simple way to think about this is from the wallet's point of view we - // always want the blockchain to move forward. By setting nLockTime this - // way we're basically making the statement that we only want this - // transaction to appear in the next block; we don't want to potentially - // encourage reorgs by allowing transactions to appear at lower heights - // than the next block in forks of the best chain. - // - // Of course, the subsidy is high enough, and transaction volume low - // enough, that fee sniping isn't a problem yet, but by implementing a fix - // now we ensure code won't be written that makes assumptions about - // nLockTime that preclude a fix later. - - txNew.nLockTime = height; - - // Secondly occasionally randomly pick a nLockTime even further back, so - // that transactions that are delayed after signing for whatever reason, - // e.g. high-latency mix networks and some CoinJoin implementations, have - // better privacy. - if (GetRandInt(10) == 0) - txNew.nLockTime = std::max(0, (int)txNew.nLockTime - GetRandInt(100)); - - assert(txNew.nLockTime <= height); - assert(txNew.nLockTime < LOCKTIME_THRESHOLD); FeeCalculation feeCalc; CFeeRate discard_rate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(*this, ::feeEstimator); unsigned int nBytes{0}; diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 417a5aef84..8f6929f6db 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -202,6 +202,7 @@ BASE_SCRIPTS = [ 'rpc_mnauth.py', 'rpc_verifyislock.py', 'rpc_verifychainlock.py', + 'wallet_create_tx.py', 'p2p_fingerprint.py', 'rpc_platform_filter.py', 'feature_dip0020_activation.py', diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py new file mode 100755 index 0000000000..27dc0fb279 --- /dev/null +++ b/test/functional/wallet_create_tx.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) + + +class CreateTxWalletTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + self.log.info('Check that we have some (old) blocks and that anti-fee-sniping is disabled') + assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 200) + txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) + tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) + assert_equal(tx['locktime'], 0) + + self.log.info('Check that anti-fee-sniping is enabled when we mine a recent block') + self.nodes[0].generate(1) + txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) + tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) + assert 0 < tx['locktime'] <= 201 + + +if __name__ == '__main__': + CreateTxWalletTest().main() diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index a26fece9de..23a327a377 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -59,7 +59,7 @@ class TxnMallTest(BitcoinTestFramework): # Construct a clone of tx1, to be malleated rawtx1 = self.nodes[0].getrawtransaction(txid1, 1) - clone_inputs = [{"txid": rawtx1["vin"][0]["txid"], "vout": rawtx1["vin"][0]["vout"]}] + clone_inputs = [{"txid": rawtx1["vin"][0]["txid"], "vout": rawtx1["vin"][0]["vout"], "sequence": rawtx1["vin"][0]["sequence"]}] clone_outputs = {rawtx1["vout"][0]["scriptPubKey"]["addresses"][0]: rawtx1["vout"][0]["value"], rawtx1["vout"][1]["scriptPubKey"]["addresses"][0]: rawtx1["vout"][1]["value"]} clone_locktime = rawtx1["locktime"] From a97eebd068d46a1320a139f36a3696fefa771693 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sun, 14 Nov 2021 15:24:24 +0530 Subject: [PATCH 2/7] merge bitcoin#15039: Avoid leaking nLockTime fingerprint when anti-fee-sniping --- src/wallet/wallet.cpp | 2 +- test/functional/wallet_create_tx.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 688fc10004..6a033bdce4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3299,7 +3299,7 @@ static bool IsCurrentForAntiFeeSniping(interfaces::Chain::Lock& locked_chain) if (::ChainstateActive().IsInitialBlockDownload()) { return false; } - constexpr int64_t MAX_ANTI_FEE_SNIPING_TIP_AGE = 4 * 60; // in seconds + constexpr int64_t MAX_ANTI_FEE_SNIPING_TIP_AGE = 8 * 60 * 60; // in seconds if (::ChainActive().Tip()->GetBlockTime() < (GetTime() - MAX_ANTI_FEE_SNIPING_TIP_AGE)) { return false; } diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py index 27dc0fb279..7dfabc96ad 100755 --- a/test/functional/wallet_create_tx.py +++ b/test/functional/wallet_create_tx.py @@ -19,6 +19,7 @@ class CreateTxWalletTest(BitcoinTestFramework): def run_test(self): self.log.info('Check that we have some (old) blocks and that anti-fee-sniping is disabled') + self.bump_mocktime(8 * 60 * 60 + 1) assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 200) txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) From 2c98320c1adeef55075fd23f262f020884c19d34 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sun, 14 Nov 2021 15:32:37 +0530 Subject: [PATCH 3/7] merge bitcoin#15288: Remove wallet -> node global function calls --- contrib/devtools/circular-dependencies.py | 9 ++ src/Makefile.bench.include | 2 +- src/coinjoin/util.cpp | 4 +- src/interfaces/chain.cpp | 72 ++++++++++++++ src/interfaces/chain.h | 69 ++++++++++++- src/interfaces/node.cpp | 2 +- src/interfaces/wallet.cpp | 8 +- src/rpc/governance.cpp | 2 +- src/rpc/mining.cpp | 6 +- src/rpc/util.cpp | 5 +- src/rpc/util.h | 2 +- src/ui_interface.cpp | 2 +- src/ui_interface.h | 7 +- src/wallet/fees.cpp | 17 ++-- src/wallet/fees.h | 8 +- src/wallet/init.cpp | 16 ++-- src/wallet/rpcdump.cpp | 9 +- src/wallet/rpcwallet.cpp | 31 +++--- src/wallet/test/coinjoin_tests.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 6 +- src/wallet/wallet.cpp | 112 ++++++++++------------ src/wallet/wallet.h | 10 +- 22 files changed, 267 insertions(+), 134 deletions(-) diff --git a/contrib/devtools/circular-dependencies.py b/contrib/devtools/circular-dependencies.py index abfa5ed5ae..2e4657f1dd 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"): diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 844f4181bb..ffd97844ee 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -76,7 +76,7 @@ bench_bench_dash_SOURCES += bench/coin_selection.cpp bench_bench_dash_SOURCES += bench/wallet_balance.cpp endif -bench_bench_dash_LDADD += $(BACKTRACE_LIB) $(BOOST_LIBS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(MINIUPNPC_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(BLS_LIBS) $(GMP_LIBS) +bench_bench_dash_LDADD += $(BACKTRACE_LIB) $(BOOST_LIBS) $(BDB_LIBS) $(SSL_LIBS) $(CRYPTO_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(MINIUPNPC_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(BLS_LIBS) $(GMP_LIBS) bench_bench_dash_LDFLAGS = $(LDFLAGS_WRAP_EXCEPTIONS) $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) CLEAN_BITCOIN_BENCH = bench/*.gcda bench/*.gcno $(GENERATED_BENCH_FILES) diff --git a/src/coinjoin/util.cpp b/src/coinjoin/util.cpp index a61d9a381b..507019c84d 100644 --- a/src/coinjoin/util.cpp +++ b/src/coinjoin/util.cpp @@ -111,7 +111,7 @@ CTransactionBuilder::CTransactionBuilder(std::shared_ptr pwalletIn, con tallyItem(tallyItemIn) { // Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not - coinControl.m_discard_feerate = ::GetDiscardRate(*pwallet.get(), ::feeEstimator); + coinControl.m_discard_feerate = ::GetDiscardRate(*pwallet.get()); // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction coinControl.m_feerate = std::max(::feeEstimator.estimateSmartFee((int)pwallet->m_confirm_target, nullptr, true), pwallet->m_pay_tx_fee); // Change always goes back to origin @@ -307,7 +307,7 @@ bool CTransactionBuilder::Commit(std::string& strResult) } CValidationState state; - if (!pwallet->CommitTransaction(tx, {}, {}, dummyReserveKey, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, {}, {}, dummyReserveKey, state)) { strResult = state.GetRejectReason(); return false; } diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index a2bc2a006a..0c3043f336 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -6,8 +6,19 @@ #include #include +#include +#include +#include +#include +#include #include +#include +#include #include +#include +#include +#include +#include #include #include #include @@ -146,6 +157,17 @@ class LockImpl : public Chain::Lock } return nullopt; } + bool checkFinalTx(const CTransaction& tx) override + { + 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 */, + false /* bypass limits */, absurd_fee); + } }; class LockingStateImpl : public LockImpl, public UniqueLock @@ -191,6 +213,56 @@ public: LOCK(cs_main); return GuessVerificationProgress(Params().TxData(), LookupBlockIndex(block_hash)); } + 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; + } + void relayTransaction(const uint256& txid) override + { + CInv inv(CCoinJoin::GetDSTX(txid) ? MSG_DSTX : 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); + } + 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); + } + 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); + } + CAmount maxTxFee() override { return ::maxTxFee; } + bool getPruneMode() override { return ::fPruneMode; } + bool p2pEnabled() override { return g_connman != nullptr; } + bool isInitialBlockDownload() override { return ::ChainstateActive().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); } + 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 3a54b9164e..7745e2b9c0 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -5,20 +5,29 @@ #ifndef BITCOIN_INTERFACES_CHAIN_H #define BITCOIN_INTERFACES_CHAIN_H -#include +#include // For Optional and nullopt +#include // For CTransactionRef #include +#include #include #include #include class CBlock; class CScheduler; +class CValidationState; +class CFeeRate; class uint256; struct CBlockLocator; +struct FeeCalculation; + +typedef std::shared_ptr CTransactionRef; namespace interfaces { +class Wallet; + //! Interface for giving wallet processes access to blockchain state. class Chain { @@ -102,6 +111,13 @@ 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; + + //! 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 @@ -127,6 +143,57 @@ 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 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; + + //! 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; + + //! 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; + + //! Check if p2p enabled. + virtual bool p2pEnabled() = 0; + + // Check if in IBD. + virtual bool isInitialBlockDownload() = 0; + + //! 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; + + //! 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 18d4b4e3fa..40d99e35ba 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -419,7 +419,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/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 437b3f1147..ae7a47bcc1 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -55,7 +55,7 @@ public: auto locked_chain = m_wallet.chain().lock(); LOCK2(mempool.cs, 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; } @@ -110,7 +110,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 = ::BlockIndex().find(wtx.hashBlock); @@ -120,7 +120,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(); @@ -539,7 +539,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/governance.cpp b/src/rpc/governance.cpp index 1f9012acfc..af86bd6b3f 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -247,7 +247,7 @@ static UniValue gobject_prepare(const JSONRPCRequest& request) CReserveKey reservekey(pwallet); // -- send the tx to the network CValidationState state; - if (!pwallet->CommitTransaction(tx, {}, {}, reservekey, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, {}, {}, reservekey, state)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "CommitTransaction failed! Reason given: " + state.GetRejectReason()); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 6a16749e5d..e79d1fcc7c 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -868,7 +868,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; @@ -939,7 +940,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 5381ac7cb8..d478d41319 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -4,12 +4,10 @@ #include #include -#include #include #include #include #include -#include InitInterfaces* g_rpc_interfaces = nullptr; @@ -96,10 +94,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 e5e58e9042..83fdf94513 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -35,7 +35,7 @@ CScript CreateMultisigRedeemscript(const int required, const std::vector 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::NotifyChainLock(const std::string& bestChainLockHash, int bestChainLockHeight) { return g_ui_signals.NotifyChainLock(bestChainLockHash, bestChainLockHeight); } diff --git a/src/ui_interface.h b/src/ui_interface.h index 9102d942f9..964019d162 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -11,7 +11,6 @@ #include #include -class CWallet; class CBlockIndex; class CDeterministicMNList; namespace boost { @@ -20,6 +19,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/fees.cpp b/src/wallet/fees.cpp index eefe7df2bb..837e84d660 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -18,9 +17,9 @@ CAmount GetRequiredFee(const CWallet& wallet, unsigned int nTxBytes) return GetRequiredFeeRate(wallet).GetFee(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; @@ -34,7 +33,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 @@ -63,14 +62,14 @@ 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; if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; } // 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; @@ -86,10 +85,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 b3cd064abc..26d9394999 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/init.cpp b/src/wallet/init.cpp index ed5e2396e7..3013ad6397 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -264,12 +264,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()); } @@ -290,14 +293,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/rpcdump.cpp b/src/wallet/rpcdump.cpp index 68faa38653..491377ce76 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -136,8 +136,9 @@ 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()) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); @@ -282,7 +283,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); @@ -473,7 +474,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); @@ -531,7 +532,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/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 4ac2cb0cb4..48122d7a2d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -302,7 +302,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"); } @@ -328,7 +328,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); } @@ -414,7 +414,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) } if (!request.params[7].isNull()) { - coin_control.m_confirm_target = ParseConfirmTarget(request.params[7]); + coin_control.m_confirm_target = ParseConfirmTarget(request.params[7], pwallet->chain().estimateMaxBlocks()); } if (!request.params[8].isNull()) { @@ -653,7 +653,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); @@ -677,8 +676,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() || !CheckFinalTx(*wtx.tx)) + if (wtx.IsCoinBase() || !locked_chain->checkFinalTx(*wtx.tx)) { continue; + } for (const CTxOut& txout : wtx.tx->vout) if (txout.scriptPubKey == scriptPubKey) @@ -727,7 +727,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); @@ -745,7 +744,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) @@ -919,7 +918,7 @@ static UniValue sendmany(const JSONRPCRequest& request) auto locked_chain = pwallet->chain().lock(); LOCK2(mempool.cs, 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"); } @@ -949,7 +948,7 @@ static UniValue sendmany(const JSONRPCRequest& request) } if (!request.params[8].isNull()) { - coin_control.m_confirm_target = ParseConfirmTarget(request.params[8]); + coin_control.m_confirm_target = ParseConfirmTarget(request.params[8], pwallet->chain().estimateMaxBlocks()); } if (!request.params[9].isNull()) { @@ -1012,7 +1011,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); } @@ -1110,8 +1109,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()) @@ -1145,7 +1142,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); @@ -2776,7 +2773,7 @@ static UniValue upgradetohd(const JSONRPCRequest& request) SecureString secureMnemonic; secureMnemonic.reserve(256); if (!generate_mnemonic) { - if (::ChainstateActive().IsInitialBlockDownload()) { + if (pwallet->chain().isInitialBlockDownload()) { throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set mnemonic while still in Initial Block Download"); } secureMnemonic = request.params[0].get_str().c_str(); @@ -3040,7 +3037,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(); @@ -3050,7 +3047,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) { @@ -3332,7 +3329,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")) { diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index 03f5ff0a83..4a662ba616 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -95,7 +95,7 @@ public: BOOST_CHECK(wallet->CreateTransaction(*locked_chain, {{GetScriptForDestination(tallyItem.txdest), nAmount, false}}, tx, reserveKey, nFeeRet, nChangePosRet, strError, coinControl)); } CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reserveKey, nullptr, state)); + BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reserveKey, state)); AddTxToChain(tx->GetHash()); for (size_t n = 0; n < tx->vout.size(); ++n) { if (nChangePosRet != -1 && n == nChangePosRet) { diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 5579ee30d8..4c39c5f84b 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -373,7 +373,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); @@ -581,7 +581,7 @@ public: CCoinControl coinControl; BOOST_CHECK(wallet->CreateTransaction(*wallet->chain().lock(), GetRecipients(vecEntries), tx, reserveKey, nFeeRet, nChangePosRet, strError, coinControl)); CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reserveKey, nullptr, state)); + BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reserveKey, state)); CMutableTransaction blocktx; { LOCK(wallet->cs_wallet); @@ -927,7 +927,7 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup tx2, reservekey2, fee, changePos, error, dummy)); } CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(tx1, {}, {}, reservekey1, nullptr, state)); + BOOST_CHECK(wallet->CommitTransaction(tx1, {}, {}, reservekey1, state)); reservekey2.KeepKey(); BOOST_CHECK_EQUAL(wallet->GetAvailableBalance(), 0); CreateAndProcessBlock({CMutableTransaction(*tx2)}, GetScriptForRawPubKey({})); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6a033bdce4..1ca4e9341d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -1079,7 +1080,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); @@ -2301,21 +2302,21 @@ 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); } } -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) { 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 (connman) { - connman->RelayTransaction(*tx); + if (pwallet->chain().p2pEnabled()) { + pwallet->chain().relayTransaction(GetHash()); return true; } } @@ -2576,10 +2577,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) @@ -2618,7 +2617,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; @@ -2637,7 +2636,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; @@ -2662,7 +2661,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()); } @@ -2807,7 +2806,6 @@ CAmount CWallet::GetNormalizedAnonymizedBalance() const // trusted. CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth, const bool fAddLocked) const { - LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -2815,7 +2813,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; } @@ -2869,7 +2867,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< for (auto pcoin : GetSpendableTXs()) { const uint256& wtxid = pcoin->GetHash(); - if (!CheckFinalTx(*pcoin->tx)) + if (!locked_chain.checkFinalTx(*pcoin->tx)) continue; if (pcoin->IsImmatureCoinBase(locked_chain)) @@ -3044,10 +3042,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) { @@ -3572,7 +3570,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std txNew.nLockTime = GetLocktimeForNewTransaction(locked_chain); FeeCalculation feeCalc; - CFeeRate discard_rate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(*this, ::feeEstimator); + CFeeRate discard_rate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(*this); unsigned int nBytes{0}; { std::vector vecCoins; @@ -3727,7 +3725,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std txin.scriptSig = CScript(); } - nFee = GetMinimumFee(*this, nBytes, coin_control, ::mempool, ::feeEstimator, &feeCalc); + nFee = GetMinimumFee(*this, nBytes, coin_control, &feeCalc); // If we made it here and we aren't even able to meet the relay fee on the next pass, give up // because we must be at the maximum allowed fee. @@ -3905,16 +3903,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; } @@ -3934,7 +3923,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(); @@ -3976,11 +3965,11 @@ 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 { - wtx.RelayWalletTransaction(*locked_chain, connman); + wtx.RelayWalletTransaction(*locked_chain); } } } @@ -4929,17 +4918,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; @@ -4949,7 +4938,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, AddWallet(walletInstance); auto error = [&](const std::string& strError) { RemoveWallet(walletInstance); - InitError(strError); + chain.initError(strError); return nullptr; }; DBErrors nLoadWalletRet; @@ -4966,8 +4955,8 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } 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) { @@ -5055,7 +5044,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, std::string strBackupError; if(!walletInstance->AutoBackupWallet("", strBackupWarning, strBackupError)) { if (!strBackupWarning.empty()) { - InitWarning(strBackupWarning); + chain.initWarning(strBackupWarning); } if (!strBackupError.empty()) { return error(strBackupError); @@ -5063,12 +5052,12 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } } 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)); } } else if (gArgs.IsArgSet("-usehd")) { @@ -5091,12 +5080,12 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, 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); } @@ -5106,12 +5095,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 @@ -5119,28 +5108,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; } @@ -5180,7 +5169,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) { @@ -5192,7 +5181,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, } } - 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 @@ -5242,7 +5231,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()); @@ -5543,17 +5532,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 */, - false /* bypass_limits */, nAbsurdFee); + bool ret = locked_chain.submitToMemoryPool(tx, pwallet->chain().maxTxFee(), state); fInMempool |= ret; return ret; } @@ -5567,7 +5553,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 diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7d9cfd7fff..0e4fee2c81 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -103,8 +103,6 @@ class COutput; class CReserveKey; class CScript; class CTxDSIn; -class CTxMemPool; -class CBlockPolicyEstimator; class CWalletTx; struct FeeCalculation; enum class FeeEstimateMode; @@ -542,10 +540,10 @@ 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); + 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 @@ -1021,7 +1019,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); struct Balance { CAmount m_mine_trusted{0}; //!< Trusted, at depth=GetBalance.min_depth or more CAmount m_mine_untrusted_pending{0}; //!< Untrusted, but in mempool (pending) @@ -1057,7 +1055,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, int nExtraPayloadSize = 0); - 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 b30bc75a32a68a5b07af0625446bb2d76c31d3b3 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Mon, 17 Apr 2017 10:16:21 -0400 Subject: [PATCH 4/7] merge bitcoin#10221: Stop treating coinbase outputs differently in GUI --- src/wallet/wallet.cpp | 27 --------------------------- src/wallet/wallet.h | 4 ---- 2 files changed, 31 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1ca4e9341d..e7444c8a9a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1451,33 +1451,6 @@ void CWallet::BlockConnected(const std::shared_ptr& pblock, const m_last_block_processed = pindex->GetBlockHash(); - // The GUI expects a NotifyTransactionChanged when a coinbase tx - // which is in our wallet moves from in-the-best-block to - // 2-confirmations (as it only displays them at that time). - // We do that here. - if (hashPrevBestCoinbase.IsNull()) { - // Immediately after restart we have no idea what the coinbase - // transaction from the previous block is. - // For correctness we scan over the entire wallet, looking for - // the previous block's coinbase, just in case it is ours, so - // that we can notify the UI that it should now be displayed. - if (pindex->pprev) { - for (const std::pair& p : mapWallet) { - if (p.second.IsCoinBase() && p.second.hashBlock == pindex->pprev->GetBlockHash()) { - NotifyTransactionChanged(this, p.first, CT_UPDATED); - break; - } - } - } - } else { - std::map::const_iterator mi = mapWallet.find(hashPrevBestCoinbase); - if (mi != mapWallet.end()) { - NotifyTransactionChanged(this, hashPrevBestCoinbase, CT_UPDATED); - } - } - - hashPrevBestCoinbase = pblock->vtx[0]->GetHash(); - // reset cache to make sure no longer immature coins are included fAnonymizableTallyCached = false; fAnonymizableTallyCachedNonDenom = false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0e4fee2c81..1e8e119df5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -753,10 +753,6 @@ private: /** Internal database handle. */ std::unique_ptr database; - // Used to NotifyTransactionChanged of the previous block's coinbase when - // the next block comes in - uint256 hashPrevBestCoinbase; - // A helper function which loops through wallet UTXOs std::unordered_set GetSpendableTXs() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); From 71b90dad9760a2be674b1b5bd097f9209290df18 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Sun, 31 Oct 2021 21:23:36 +0530 Subject: [PATCH 5/7] partial bitcoin#14982: Remove unused PreCommand signal --- src/rpc/server.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 31a2035bf3..ccf6603d33 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -41,7 +41,6 @@ static struct CRPCSignals { boost::signals2::signal Started; boost::signals2::signal Stopped; - boost::signals2::signal PreCommand; } g_rpcSignals; void RPCServer::OnStarted(std::function slot) @@ -630,8 +629,6 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const } } - g_rpcSignals.PreCommand(*pcmd); - try { // Execute, convert arguments to array if necessary From 438c93bd9a285fb07598ea58603abb0531f48429 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Mon, 1 Nov 2021 16:54:34 +0530 Subject: [PATCH 6/7] merge bitcoin#10973: separate wallet from node --- src/Makefile.am | 2 + src/bench/wallet_balance.cpp | 7 +- src/interfaces/chain.cpp | 114 +++++++++++++++++++++++- src/interfaces/chain.h | 57 +++++++++++- src/interfaces/wallet.cpp | 9 +- src/node/coin.cpp | 21 +++++ src/node/coin.h | 22 +++++ src/qt/walletmodeltransaction.cpp | 2 +- src/rpc/rawtransaction.cpp | 42 +++++---- src/rpc/server.cpp | 61 +++++++------ src/rpc/server.h | 31 +++++-- src/test/rpc_tests.cpp | 5 +- src/wallet/fees.cpp | 9 +- src/wallet/init.cpp | 2 +- src/wallet/rpcdump.cpp | 14 +-- src/wallet/rpcwallet.cpp | 8 +- src/wallet/rpcwallet.h | 9 +- src/wallet/test/wallet_test_fixture.cpp | 9 +- src/wallet/test/wallet_test_fixture.h | 2 +- src/wallet/wallet.cpp | 54 ++++++----- src/wallet/wallet.h | 16 ++-- 21 files changed, 366 insertions(+), 130 deletions(-) create mode 100644 src/node/coin.cpp create mode 100644 src/node/coin.h diff --git a/src/Makefile.am b/src/Makefile.am index e14ea60f29..585efa61b3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -226,6 +226,7 @@ BITCOIN_CORE_H = \ netbase.h \ netfulfilledman.h \ netmessagemaker.h \ + node/coin.h \ node/coinstats.h \ node/transaction.h \ noui.h \ @@ -386,6 +387,7 @@ libdash_server_a_SOURCES = \ net.cpp \ netfulfilledman.cpp \ net_processing.cpp \ + node/coin.cpp \ node/coinstats.cpp \ node/transaction.cpp \ noui.cpp \ diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 32959cfa20..08b9a3126c 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -22,12 +22,7 @@ struct WalletTestingSetup { void handleNotifications() { - RegisterValidationInterface(&m_wallet); - } - - ~WalletTestingSetup() - { - UnregisterValidationInterface(&m_wallet); + m_wallet.m_chain_notifications_handler = m_chain->handleNotifications(m_wallet); } }; diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 0c3043f336..ab2adb703f 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -7,21 +7,28 @@ #include #include #include +#include #include #include +#include #include #include #include #include #include +#include +#include +#include #include #include #include #include #include #include +#include #include #include +#include #include #include @@ -175,6 +182,94 @@ class LockingStateImpl : public LockImpl, public UniqueLock using UniqueLock::UniqueLock; }; +class NotificationsHandlerImpl : public Handler, CValidationInterface +{ +public: + explicit NotificationsHandlerImpl(Chain& chain, Chain::Notifications& notifications) + : m_chain(chain), m_notifications(¬ifications) + { + RegisterValidationInterface(this); + } + ~NotificationsHandlerImpl() override { disconnect(); } + void disconnect() override + { + if (m_notifications) { + m_notifications = nullptr; + UnregisterValidationInterface(this); + } + } + void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override + { + m_notifications->TransactionAddedToMempool(tx, nAcceptTime); + } + void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override + { + m_notifications->TransactionRemovedFromMempool(tx, reason); + } + void BlockConnected(const std::shared_ptr& block, + const CBlockIndex* index, + const std::vector& tx_conflicted) override + { + m_notifications->BlockConnected(*block, tx_conflicted); + } + void BlockDisconnected(const std::shared_ptr& block, const CBlockIndex* pindexDisconnected) override + { + m_notifications->BlockDisconnected(*block); + } + void ChainStateFlushed(const CBlockLocator& locator) override { m_notifications->ChainStateFlushed(locator); } + void ResendWalletTransactions(int64_t best_block_time, CConnman*) override + { + // `cs_main` is always held when this method is called, so it is safe to + // call `assumeLocked`. This is awkward, and the `assumeLocked` method + // should be able to be removed entirely if `ResendWalletTransactions` + // is replaced by a wallet timer as suggested in + // https://github.com/bitcoin/bitcoin/issues/15619 + auto locked_chain = m_chain.assumeLocked(); + m_notifications->ResendWalletTransactions(*locked_chain, best_block_time); + } + Chain& m_chain; + Chain::Notifications* m_notifications; +}; + +class RpcHandlerImpl : public Handler +{ +public: + RpcHandlerImpl(const CRPCCommand& command) : m_command(command), m_wrapped_command(&command) + { + m_command.actor = [this](const JSONRPCRequest& request, UniValue& result, bool last_handler) { + if (!m_wrapped_command) return false; + try { + return m_wrapped_command->actor(request, result, last_handler); + } catch (const UniValue& e) { + // If this is not the last handler and a wallet not found + // exception was thrown, return false so the next handler can + // try to handle the request. Otherwise, reraise the exception. + if (!last_handler) { + const UniValue& code = e["code"]; + if (code.isNum() && code.get_int() == RPC_WALLET_NOT_FOUND) { + return false; + } + } + throw; + } + }; + ::tableRPC.appendCommand(m_command.name, &m_command); + } + + void disconnect() override final + { + if (m_wrapped_command) { + m_wrapped_command = nullptr; + ::tableRPC.removeCommand(m_command.name, &m_command); + } + } + + ~RpcHandlerImpl() override { disconnect(); } + + CRPCCommand m_command; + const CRPCCommand* m_wrapped_command; +}; + class ChainImpl : public Chain { public: @@ -208,6 +303,7 @@ public: } return true; } + void findCoins(std::map& coins) override { return FindCoins(coins); } double guessVerificationProgress(const uint256& block_hash) override { LOCK(cs_main); @@ -254,17 +350,33 @@ public: { return ::mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); } + CFeeRate relayMinFee() override { return ::minRelayTxFee; } + CFeeRate relayIncrementalFee() override { return ::incrementalRelayFee; } + CFeeRate relayDustFee() override { return ::dustRelayFee; } CAmount maxTxFee() override { return ::maxTxFee; } bool getPruneMode() override { return ::fPruneMode; } bool p2pEnabled() override { return g_connman != nullptr; } bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); } + bool shutdownRequested() override { return ShutdownRequested(); } 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); } void loadWallet(std::unique_ptr wallet) override { ::uiInterface.LoadWallet(wallet); } + void showProgress(const std::string& title, int progress, bool resume_possible) override + { + ::uiInterface.ShowProgress(title, progress, resume_possible); + } + std::unique_ptr handleNotifications(Notifications& notifications) override + { + return MakeUnique(*this, notifications); + } + void waitForNotifications() override { SyncWithValidationInterfaceQueue(); } + std::unique_ptr handleRpc(const CRPCCommand& command) override + { + return MakeUnique(command); + } }; - } // namespace std::unique_ptr MakeChain() { return MakeUnique(); } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 7745e2b9c0..c80e8ea36b 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -15,18 +15,28 @@ #include class CBlock; +class CRPCCommand; class CScheduler; class CValidationState; class CFeeRate; +class CBlockIndex; +class Coin; class uint256; struct CBlockLocator; struct FeeCalculation; +enum class MemPoolRemovalReason; + +namespace llmq { +class CChainLockSig; +class CInstantSendLock; +} // namespace llmq typedef std::shared_ptr CTransactionRef; namespace interfaces { class Wallet; +class Handler; //! Interface for giving wallet processes access to blockchain state. class Chain @@ -140,6 +150,11 @@ public: int64_t* time = nullptr, int64_t* max_time = nullptr) = 0; + //! Look up unspent output information. Returns coins in the mempool and in + //! the current chain UTXO set. Iterates through all the keys in the map and + //! populates the values. + virtual void findCoins(std::map& coins) = 0; + //! Estimate fraction of total transactions verified if blocks up to //! the specified block hash are verified. virtual double guessVerificationProgress(const uint256& block_hash) = 0; @@ -165,6 +180,15 @@ public: //! Pool min fee. virtual CFeeRate mempoolMinFee() = 0; + //! Relay current minimum fee (from -minrelaytxfee and -incrementalrelayfee settings). + virtual CFeeRate relayMinFee() = 0; + + //! Relay incremental fee setting (-incrementalrelayfee), reflecting cost of relay. + virtual CFeeRate relayIncrementalFee() = 0; + + //! Relay dust fee setting (-dustrelayfee), reflecting lowest rate it's economical to spend. + virtual CFeeRate relayDustFee() = 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 @@ -177,9 +201,12 @@ public: //! Check if p2p enabled. virtual bool p2pEnabled() = 0; - // Check if in IBD. + //! Check if in IBD. virtual bool isInitialBlockDownload() = 0; + //! Check if shutdown requested. + virtual bool shutdownRequested() = 0; + //! Get adjusted time. virtual int64_t getAdjustedTime() = 0; @@ -194,6 +221,34 @@ public: //! Send wallet load notification to the GUI. virtual void loadWallet(std::unique_ptr wallet) = 0; + + //! Send progress indicator. + virtual void showProgress(const std::string& title, int progress, bool resume_possible) = 0; + + //! Chain notifications. + class Notifications + { + public: + virtual ~Notifications() {} + virtual void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) {} + virtual void TransactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason) {} + virtual void BlockConnected(const CBlock& block, const std::vector& tx_conflicted) {} + virtual void BlockDisconnected(const CBlock& block) {} + virtual void ChainStateFlushed(const CBlockLocator& locator) {} + virtual void ResendWalletTransactions(Lock& locked_chain, int64_t best_block_time) {} + virtual void NotifyChainLock(const CBlockIndex* pindexChainLock, const std::shared_ptr& clsig) {} + virtual void NotifyTransactionLock(const CTransactionRef &tx, const std::shared_ptr& islock) {} + }; + + //! Register handler for notifications. + virtual std::unique_ptr handleNotifications(Notifications& notifications) = 0; + + //! Wait for pending notifications to be handled. + virtual void waitForNotifications() = 0; + + //! Register handler for RPC. Command is not copied, so reference + //! needs to remain valid until Handler is disconnected. + virtual std::unique_ptr handleRpc(const CRPCCommand& command) = 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 ae7a47bcc1..a3c8744d7a 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -110,12 +110,8 @@ 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 mapBlockIndex below. Removed in upcoming commit. - WalletTxStatus result; - auto mi = ::BlockIndex().find(wtx.hashBlock); - CBlockIndex* block = mi != ::BlockIndex().end() ? mi->second : nullptr; - result.block_height = (block ? block->nHeight : std::numeric_limits::max()); + result.block_height = locked_chain.getBlockHeight(wtx.hashBlock).get_value_or(std::numeric_limits::max()); result.blocks_to_maturity = wtx.GetBlocksToMaturity(locked_chain); result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain); result.time_received = wtx.nTimeReceived; @@ -604,7 +600,7 @@ public: : m_chain(chain), m_wallet_filenames(std::move(wallet_filenames)) { } - void registerRpcs() override { return RegisterWalletRPCCommands(::tableRPC); } + void registerRpcs() override { return RegisterWalletRPCCommands(m_chain, m_rpc_handlers); } bool verify() override { return VerifyWallets(m_chain, m_wallet_filenames); } bool load() override { return LoadWallets(m_chain, m_wallet_filenames); } void start(CScheduler& scheduler) override { return StartWallets(scheduler); } @@ -614,6 +610,7 @@ public: Chain& m_chain; std::vector m_wallet_filenames; + std::vector> m_rpc_handlers; }; } // namespace diff --git a/src/node/coin.cpp b/src/node/coin.cpp new file mode 100644 index 0000000000..ad8d1d3af4 --- /dev/null +++ b/src/node/coin.cpp @@ -0,0 +1,21 @@ +// Copyright (c) 2019 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 + +#include +#include + +void FindCoins(std::map& coins) +{ + LOCK2(cs_main, ::mempool.cs); + CCoinsViewCache& chain_view = ::ChainstateActive().CoinsTip(); + CCoinsViewMemPool mempool_view(&chain_view, ::mempool); + for (auto& coin : coins) { + if (!mempool_view.GetCoin(coin.first, coin.second)) { + // Either the coin is not in the CCoinsViewCache or is spent. Clear it. + coin.second.Clear(); + } + } +} diff --git a/src/node/coin.h b/src/node/coin.h new file mode 100644 index 0000000000..eb95b75cfb --- /dev/null +++ b/src/node/coin.h @@ -0,0 +1,22 @@ +// Copyright (c) 2019 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_NODE_COIN_H +#define BITCOIN_NODE_COIN_H + +#include + +class COutPoint; +class Coin; + +/** + * Look up unspent output information. Returns coins in the mempool and in the + * current chain UTXO set. Iterates through all the keys in the map and + * populates the values. + * + * @param[in,out] coins map to fill + */ +void FindCoins(std::map& coins); + +#endif // BITCOIN_NODE_COIN_H diff --git a/src/qt/walletmodeltransaction.cpp b/src/qt/walletmodeltransaction.cpp index 1850f0c605..9e31cb4c2a 100644 --- a/src/qt/walletmodeltransaction.cpp +++ b/src/qt/walletmodeltransaction.cpp @@ -29,7 +29,7 @@ std::unique_ptr& WalletModelTransaction::getWtx() unsigned int WalletModelTransaction::getTransactionSize() { - return wtx != nullptr ? ::GetSerializeSize(wtx->get(), SER_NETWORK, PROTOCOL_VERSION) : 0; + return wtx ? ::GetSerializeSize(wtx->get(), SER_NETWORK, PROTOCOL_VERSION) : 0; } CAmount WalletModelTransaction::getTransactionFee() const diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 2291cf0a82..0b8763a5c5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -768,23 +769,20 @@ static UniValue combinerawtransaction(const JSONRPCRequest& request) return EncodeHexTx(CTransaction(mergedTx)); } +// TODO(https://github.com/bitcoin/bitcoin/pull/10973#discussion_r267084237): +// This function is called from both wallet and node rpcs +// (signrawtransactionwithwallet and signrawtransactionwithkey). It should be +// moved to a util file so wallet code doesn't need to link against node code. +// Also the dependency on interfaces::Chain should be removed, so +// signrawtransactionwithkey doesn't need access to a Chain instance. UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, const UniValue& prevTxsUnival, CBasicKeyStore *keystore, bool is_temp_keystore, const UniValue& hashType) { // Fetch previous transactions (inputs): - CCoinsView viewDummy; - CCoinsViewCache view(&viewDummy); - { - LOCK2(cs_main, mempool.cs); - CCoinsViewCache& viewChain = ::ChainstateActive().CoinsTip(); - CCoinsViewMemPool viewMempool(&viewChain, mempool); - view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view - - for (const CTxIn& txin : mtx.vin) { - view.AccessCoin(txin.prevout); // Load entries from viewChain into view; can fail. - } - - view.SetBackend(viewDummy); // switch back to avoid locking mempool for too long + std::map coins; + for (const CTxIn& txin : mtx.vin) { + coins[txin.prevout]; // Create empty map entry keyed by prevout. } + chain.findCoins(coins); // Add previous txouts given in the RPC call: if (!prevTxsUnival.isNull()) { @@ -816,10 +814,10 @@ UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, con CScript scriptPubKey(pkData.begin(), pkData.end()); { - const Coin& coin = view.AccessCoin(out); - if (!coin.IsSpent() && coin.out.scriptPubKey != scriptPubKey) { + auto coin = coins.find(out); + if (coin != coins.end() && !coin->second.IsSpent() && coin->second.out.scriptPubKey != scriptPubKey) { std::string err("Previous output scriptPubKey mismatch:\n"); - err = err + ScriptToAsmStr(coin.out.scriptPubKey) + "\nvs:\n"+ + err = err + ScriptToAsmStr(coin->second.out.scriptPubKey) + "\nvs:\n"+ ScriptToAsmStr(scriptPubKey); throw JSONRPCError(RPC_DESERIALIZATION_ERROR, err); } @@ -830,7 +828,7 @@ UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, con newcoin.out.nValue = AmountFromValue(find_value(prevOut, "amount")); } newcoin.nHeight = 1; - view.AddCoin(out, std::move(newcoin), true); + coins[out] = std::move(newcoin); } // if redeemScript and private keys were given, add redeemScript to the keystore so it can be signed @@ -862,15 +860,15 @@ UniValue SignTransaction(interfaces::Chain& chain, CMutableTransaction& mtx, con // Sign what we can: for (unsigned int i = 0; i < mtx.vin.size(); i++) { CTxIn& txin = mtx.vin[i]; - const Coin& coin = view.AccessCoin(txin.prevout); - if (coin.IsSpent()) { + auto coin = coins.find(txin.prevout); + if (coin == coins.end() || coin->second.IsSpent()) { TxInErrorToJSON(txin, vErrors, "Input not found or already spent"); continue; } - const CScript& prevPubKey = coin.out.scriptPubKey; - const CAmount& amount = coin.out.nValue; + const CScript& prevPubKey = coin->second.out.scriptPubKey; + const CAmount& amount = coin->second.out.nValue; - SignatureData sigdata = DataFromTransaction(mtx, i, coin.out); + SignatureData sigdata = DataFromTransaction(mtx, i, coin->second.out); // Only sign SIGHASH_SINGLE if there's a corresponding output: if (!fHashSingle || (i < mtx.vout.size())) { ProduceSignature(*keystore, MutableTransactionSignatureCreator(&mtx, i, amount, nHashType), prevPubKey, sigdata); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index ccf6603d33..3a6496c060 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -33,6 +33,7 @@ static std::string rpcWarmupStatus GUARDED_BY(cs_rpcWarmup) = "RPC server starte static RPCTimerInterface* timerInterface = nullptr; /* Map of name to timer. */ static std::map > deadlineTimers; +static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler, std::multimap> mapPlatformRestrictions); // Any commands submitted by this user will have their commands filtered based on the mapPlatformRestrictions static const std::string defaultPlatformUser = "platform-user"; @@ -201,11 +202,11 @@ std::string CRPCTable::help(const std::string& strCommand, const std::string& st { std::string strRet; std::string category; - std::set setDone; + std::set setDone; std::vector > vCommands; for (const auto& entry : mapCommands) - vCommands.push_back(make_pair(entry.second->category + entry.first, entry.second)); + vCommands.push_back(make_pair(entry.second.front()->category + entry.first, entry.second.front())); sort(vCommands.begin(), vCommands.end()); JSONRPCRequest jreq(helpreq); @@ -225,9 +226,9 @@ std::string CRPCTable::help(const std::string& strCommand, const std::string& st jreq.params.setArray(); jreq.params.push_back(strSubCommand); } - rpcfn_type pfn = pcmd->actor; - if (setDone.insert(pfn).second) - (*pfn)(jreq); + UniValue unused_result; + if (setDone.insert(pcmd->unique_id).second) + pcmd->actor(jreq, unused_result, true /* last_handler */); } catch (const std::exception& e) { @@ -356,32 +357,32 @@ CRPCTable::CRPCTable() const CRPCCommand *pcmd; pcmd = &vRPCCommands[vcidx]; - mapCommands[pcmd->name] = pcmd; + mapCommands[pcmd->name].push_back(pcmd); } } -const CRPCCommand *CRPCTable::operator[](const std::string &name) const -{ - std::map::const_iterator it = mapCommands.find(name); - if (it == mapCommands.end()) - return nullptr; - return (*it).second; -} - bool CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd) { if (IsRPCRunning()) return false; - // don't allow overwriting for now - std::map::const_iterator it = mapCommands.find(name); - if (it != mapCommands.end()) - return false; - - mapCommands[name] = pcmd; + mapCommands[name].push_back(pcmd); return true; } +bool CRPCTable::removeCommand(const std::string& name, const CRPCCommand* pcmd) +{ + auto it = mapCommands.find(name); + if (it != mapCommands.end()) { + auto new_end = std::remove(it->second.begin(), it->second.end(), pcmd); + if (it->second.end() != new_end) { + it->second.erase(new_end, it->second.end()); + return true; + } + } + return false; +} + void StartRPC() { LogPrint(BCLog::RPC, "Starting RPC\n"); @@ -564,10 +565,20 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const } // Find method - const CRPCCommand *pcmd = tableRPC[request.strMethod]; - if (!pcmd) - throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found"); + auto it = mapCommands.find(request.strMethod); + if (it != mapCommands.end()) { + UniValue result; + for (const auto& command : it->second) { + if (ExecuteCommand(*command, request, result, &command == &it->second.back(), mapPlatformRestrictions)) { + return result; + } + } + } + throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found"); +} +static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler, std::multimap> mapPlatformRestrictions) +{ // Before executing the RPC Command, filter commands from platform rpc user if (fMasternodeMode && request.authUser == gArgs.GetArg("-platform-user", defaultPlatformUser)) { // replace this with structured binding in c++20 @@ -633,9 +644,9 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const { // Execute, convert arguments to array if necessary if (request.params.isObject()) { - return pcmd->actor(transformNamedArguments(request, pcmd->argNames)); + return command.actor(transformNamedArguments(request, command.argNames), result, last_handler); } else { - return pcmd->actor(request); + return command.actor(request, result, last_handler); } } catch (const std::exception& e) diff --git a/src/rpc/server.h b/src/rpc/server.h index ee9cc431fb..6a145ff7fb 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -129,10 +129,31 @@ typedef UniValue(*rpcfn_type)(const JSONRPCRequest& jsonRequest); class CRPCCommand { public: + //! RPC method handler reading request and assigning result. Should return + //! true if request is fully handled, false if it should be passed on to + //! subsequent handlers. + using Actor = std::function; + + //! Constructor taking Actor callback supporting multiple handlers. + CRPCCommand(std::string category, std::string name, Actor actor, std::vector args, intptr_t unique_id) + : category(std::move(category)), name(std::move(name)), actor(std::move(actor)), argNames(std::move(args)), + unique_id(unique_id) + { + } + + //! Simplified constructor taking plain rpcfn_type function pointer. + CRPCCommand(const char* category, const char* name, rpcfn_type fn, std::initializer_list args) + : CRPCCommand(category, name, + [fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn(request); return true; }, + {args.begin(), args.end()}, intptr_t(fn)) + { + } + std::string category; std::string name; - rpcfn_type actor; + Actor actor; std::vector argNames; + intptr_t unique_id; }; /** @@ -141,11 +162,10 @@ public: class CRPCTable { private: - std::map mapCommands; + std::map> mapCommands; std::multimap> mapPlatformRestrictions; public: CRPCTable(); - const CRPCCommand* operator[](const std::string& name) const; std::string help(const std::string& name, const std::string& strSubCommand, const JSONRPCRequest& helpreq) const; void InitPlatformRestrictions(); @@ -169,9 +189,7 @@ public: * * Returns false if RPC server is already running (dump concurrency protection). * - * Commands cannot be overwritten (returns false). - * - * Commands with different method names but the same callback function will + * Commands with different method names but the same unique_id will * be considered aliases, and only the first registered method name will * show up in the help text command listing. Aliased commands do not have * to have the same behavior. Server and client code can distinguish @@ -179,6 +197,7 @@ public: * register different names, types, and numbers of parameters. */ bool appendCommand(const std::string& name, const CRPCCommand* pcmd); + bool removeCommand(const std::string& name, const CRPCCommand* pcmd); }; bool IsDeprecatedRPCEnabled(const std::string& method); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 744fa03aa2..b3971ef2e7 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -31,10 +31,9 @@ UniValue CallRPC(std::string args) request.strMethod = strMethod; request.params = RPCConvertValues(strMethod, vArgs); request.fHelp = false; - BOOST_CHECK(tableRPC[strMethod]); - rpcfn_type method = tableRPC[strMethod]->actor; + if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); try { - UniValue result = (*method)(request); + UniValue result = tableRPC.execute(request); return result; } catch (const UniValue& objError) { diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 837e84d660..a5f692174e 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -21,8 +21,9 @@ CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinC { CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes); // Always obey the maximum - if (fee_needed > maxTxFee) { - fee_needed = maxTxFee; + const CAmount max_tx_fee = wallet.chain().maxTxFee(); + if (fee_needed > max_tx_fee) { + fee_needed = max_tx_fee; if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; } return fee_needed; @@ -30,7 +31,7 @@ CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinC CFeeRate GetRequiredFeeRate(const CWallet& wallet) { - return std::max(wallet.m_min_fee, ::minRelayTxFee); + return std::max(wallet.m_min_fee, wallet.chain().relayMinFee()); } CFeeRate GetMinimumFeeRate(const CWallet& wallet, const CCoinControl& coin_control, FeeCalculation* feeCalc) @@ -92,6 +93,6 @@ CFeeRate GetDiscardRate(const CWallet& wallet) // 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 - discard_rate = std::max(discard_rate, ::dustRelayFee); + discard_rate = std::max(discard_rate, wallet.chain().relayDustFee()); return discard_rate; } diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 3013ad6397..bf237c71ad 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -279,7 +279,7 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& wal LogPrintf("Using wallet directory %s\n", GetWalletDir().string()); - uiInterface.InitMessage(_("Verifying wallet(s)...")); + chain.initMessage(_("Verifying wallet(s)...")); // Parameter interaction code should have thrown an error if -salvagewallet // was enabled with more than wallet file, so the wallet_files size check diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 491377ce76..69463c6f6c 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -561,11 +561,11 @@ UniValue importwallet(const JSONRPCRequest& request) // Use uiInterface.ShowProgress instead of pwallet.ShowProgress because pwallet.ShowProgress has a cancel button tied to AbortRescan which // we don't want for this progress bar showing the import progress. uiInterface.ShowProgress does not have a cancel button. - uiInterface.ShowProgress(strprintf("%s " + _("Importing..."), pwallet->GetDisplayName()), 0, false); // show progress dialog in GUI + pwallet->chain().showProgress(strprintf("%s " + _("Importing..."), pwallet->GetDisplayName()), 0, false); // show progress dialog in GUI std::vector> keys; std::vector> scripts; while (file.good()) { - uiInterface.ShowProgress("", std::max(1, std::min(50, (int)(((double)file.tellg() / (double)nFilesize) * 100))), false); + pwallet->chain().showProgress("", std::max(1, std::min(50, (int)(((double)file.tellg() / (double)nFilesize) * 100))), false); std::string line; std::getline(file, line); if (line.empty() || line[0] == '#') @@ -603,13 +603,13 @@ UniValue importwallet(const JSONRPCRequest& request) file.close(); // We now know whether we are importing private keys, so we can error if private keys are disabled if (keys.size() > 0 && pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { - uiInterface.ShowProgress("", 100, false); // hide progress dialog in GUI + pwallet->chain().showProgress("", 100, false); // hide progress dialog in GUI throw JSONRPCError(RPC_WALLET_ERROR, "Importing wallets is disabled when private keys are disabled"); } double total = (double)(keys.size() + scripts.size()); double progress = 0; for (const auto& key_tuple : keys) { - uiInterface.ShowProgress("", std::max(50, std::min(75, (int)((progress / total) * 100) + 50)), false); + pwallet->chain().showProgress("", std::max(50, std::min(75, (int)((progress / total) * 100) + 50)), false); const CKey& key = std::get<0>(key_tuple); int64_t time = std::get<1>(key_tuple); bool has_label = std::get<2>(key_tuple); @@ -634,7 +634,7 @@ UniValue importwallet(const JSONRPCRequest& request) progress++; } for (const auto& script_pair : scripts) { - uiInterface.ShowProgress("", std::max(50, std::min(75, (int)((progress / total) * 100) + 50)), false); + pwallet->chain().showProgress("", std::max(50, std::min(75, (int)((progress / total) * 100) + 50)), false); const CScript& script = script_pair.first; int64_t time = script_pair.second; CScriptID id(script); @@ -653,10 +653,10 @@ UniValue importwallet(const JSONRPCRequest& request) } progress++; } - uiInterface.ShowProgress("", 100, false); // hide progress dialog in GUI + pwallet->chain().showProgress("", 100, false); // hide progress dialog in GUI pwallet->UpdateTimeFirstKey(nTimeBegin); } - uiInterface.ShowProgress("", 100, false); // hide progress dialog in GUI + pwallet->chain().showProgress("", 100, false); // hide progress dialog in GUI RescanWallet(*pwallet, reserver, nTimeBegin, false /* update */); pwallet->MarkDirty(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 48122d7a2d..6157bffe47 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2447,8 +2447,8 @@ static UniValue settxfee(const JSONRPCRequest& request) CFeeRate tx_fee_rate(nAmount, 1000); if (tx_fee_rate == 0) { // automatic selection - } else if (tx_fee_rate < ::minRelayTxFee) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("txfee cannot be less than min relay tx fee (%s)", ::minRelayTxFee.ToString())); + } else if (tx_fee_rate < pwallet->chain().relayMinFee()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("txfee cannot be less than min relay tx fee (%s)", pwallet->chain().relayMinFee().ToString())); } else if (tx_fee_rate < pwallet->m_min_fee) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("txfee cannot be less than wallet min fee (%s)", pwallet->m_min_fee.ToString())); } @@ -4271,8 +4271,8 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterWalletRPCCommands(CRPCTable &t) +void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector>& handlers) { for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) - t.appendCommand(commands[vcidx].name, &commands[vcidx]); + handlers.emplace_back(chain.handleRpc(commands[vcidx])); } diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index 8da57d0fe8..ddb2f139cc 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -5,7 +5,9 @@ #ifndef BITCOIN_WALLET_RPCWALLET_H #define BITCOIN_WALLET_RPCWALLET_H +#include #include +#include class CRPCTable; class CWallet; @@ -14,7 +16,12 @@ class UniValue; struct PartiallySignedTransaction; class CTransaction; -void RegisterWalletRPCCommands(CRPCTable &t); +namespace interfaces { +class Chain; +class Handler; +} + +void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector>& handlers); /** * Figures out what wallet, if any, to use for a JSONRPCRequest. diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index b78bd9922e..bd3f1e1f05 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -13,12 +13,7 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName): { bool fFirstRun; m_wallet.LoadWallet(fFirstRun); - RegisterValidationInterface(&m_wallet); + m_wallet.m_chain_notifications_handler = m_chain->handleNotifications(m_wallet); - RegisterWalletRPCCommands(tableRPC); -} - -WalletTestingSetup::~WalletTestingSetup() -{ - UnregisterValidationInterface(&m_wallet); + m_chain_client->registerRpcs(); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index d839fd1f13..d95b5054a2 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -17,9 +17,9 @@ */ struct WalletTestingSetup: public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); - ~WalletTestingSetup(); std::unique_ptr m_chain = interfaces::MakeChain(); + std::unique_ptr m_chain_client = interfaces::MakeWalletClient(*m_chain, {}); CWallet m_wallet; }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e7444c8a9a..923328dd09 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -114,7 +114,7 @@ static void ReleaseWallet(CWallet* wallet) wallet->WalletLogPrintf("Releasing wallet\n"); wallet->BlockUntilSyncedToCurrentChain(); wallet->Flush(); - UnregisterValidationInterface(wallet); + wallet->m_chain_notifications_handler.reset(); delete wallet; // Wallet is now released, notify UnloadWallet, if any. { @@ -1427,7 +1427,8 @@ void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolR } } -void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) { +void CWallet::BlockConnected(const CBlock& block, const std::vector& vtxConflicted) { + const uint256& block_hash = block.GetHash(); auto locked_chain = chain().lock(); LOCK(cs_wallet); // TODO: Temporarily ensure that mempool removals are notified before @@ -1443,24 +1444,24 @@ void CWallet::BlockConnected(const std::shared_ptr& pblock, const // UNKNOWN because it's a manual removal, not using mempool logic TransactionRemovedFromMempool(ptx, MemPoolRemovalReason::UNKNOWN); } - for (size_t i = 0; i < pblock->vtx.size(); i++) { - SyncTransaction(pblock->vtx[i], pindex->GetBlockHash(), i); + for (size_t i = 0; i < block.vtx.size(); i++) { + SyncTransaction(block.vtx[i], block_hash, i); // UNKNOWN because it's a manual removal, not using mempool logic - TransactionRemovedFromMempool(pblock->vtx[i], MemPoolRemovalReason::UNKNOWN); + TransactionRemovedFromMempool(block.vtx[i], MemPoolRemovalReason::UNKNOWN); } - m_last_block_processed = pindex->GetBlockHash(); + m_last_block_processed = block_hash; // reset cache to make sure no longer immature coins are included fAnonymizableTallyCached = false; fAnonymizableTallyCachedNonDenom = false; } -void CWallet::BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected) { +void CWallet::BlockDisconnected(const CBlock& block) { auto locked_chain = chain().lock(); LOCK(cs_wallet); - for (const CTransactionRef& ptx : pblock->vtx) { + for (const CTransactionRef& ptx : block.vtx) { // NOTE: do NOT pass pindex here SyncTransaction(ptx, {} /* block hash */, 0 /* position in block */); } @@ -1492,7 +1493,7 @@ void CWallet::BlockUntilSyncedToCurrentChain() { // ...otherwise put a callback in the validation interface queue and wait // for the queue to drain enough to execute it (indicating we are caught up // at least with the time we entered this function). - SyncWithValidationInterfaceQueue(); + chain().waitForNotifications(); } @@ -2177,7 +2178,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc progress_end = chain().guessVerificationProgress(stop_block.IsNull() ? tip_hash : stop_block); } double progress_current = progress_begin; - while (block_height && !fAbortRescan && !ShutdownRequested()) { + while (block_height && !fAbortRescan && !chain().shutdownRequested()) { m_scanning_progress = (progress_current - progress_begin) / (progress_end - progress_begin); if (*block_height % 100 == 0 && progress_end - progress_begin > 0.0) { ShowProgress(strprintf("%s " + _("Rescanning..."), GetDisplayName()), std::max(1, std::min(99, (int)(m_scanning_progress * 100)))); @@ -2240,7 +2241,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc if (block_height && fAbortRescan) { WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", *block_height, progress_current); result.status = ScanResult::USER_ABORT; - } else if (block_height && ShutdownRequested()) { + } else if (block_height && chain().shutdownRequested()) { WalletLogPrintf("Rescan interrupted by shutdown request at block %d. Progress=%f\n", *block_height, progress_current); result.status = ScanResult::USER_ABORT; } @@ -2615,7 +2616,7 @@ std::vector CWallet::ResendWalletTransactionsBefore(interfaces::Chain:: return result; } -void CWallet::ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) +void CWallet::ResendWalletTransactions(interfaces::Chain::Lock& locked_chain, int64_t nBestBlockTime) { // Do this infrequently and randomly to avoid giving away // that these are our transactions. @@ -2633,8 +2634,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); + std::vector relayed = ResendWalletTransactionsBefore(locked_chain, nBestBlockTime-5*60); if (!relayed.empty()) WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed.size()); } @@ -2829,7 +2829,6 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector &vCoins, bool fOnlySafe, const CCoinControl *coinControl, const CAmount &nMinimumAmount, const CAmount &nMaximumAmount, const CAmount &nMinimumSumAmount, const uint64_t nMaximumCount, const int nMinDepth, const int nMaxDepth) const { - AssertLockHeld(cs_main); AssertLockHeld(cs_wallet); vCoins.clear(); @@ -2924,7 +2923,6 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector< std::map> CWallet::ListCoins(interfaces::Chain::Lock& locked_chain) const { - AssertLockHeld(cs_main); AssertLockHeld(cs_wallet); std::map> result; @@ -3265,13 +3263,13 @@ bool CWallet::SelectTxDSInsByDenomination(int nDenom, CAmount nValueMax, std::ve return nValueTotal > 0; } -static bool IsCurrentForAntiFeeSniping(interfaces::Chain::Lock& locked_chain) +static bool IsCurrentForAntiFeeSniping(interfaces::Chain& chain, interfaces::Chain::Lock& locked_chain) { - if (::ChainstateActive().IsInitialBlockDownload()) { + if (chain.isInitialBlockDownload()) { return false; } constexpr int64_t MAX_ANTI_FEE_SNIPING_TIP_AGE = 8 * 60 * 60; // in seconds - if (::ChainActive().Tip()->GetBlockTime() < (GetTime() - MAX_ANTI_FEE_SNIPING_TIP_AGE)) { + if (locked_chain.getBlockTime(*locked_chain.getHeight()) < (GetTime() - MAX_ANTI_FEE_SNIPING_TIP_AGE)) { return false; } return true; @@ -3281,7 +3279,7 @@ static bool IsCurrentForAntiFeeSniping(interfaces::Chain::Lock& locked_chain) * Return a height-based locktime for new transactions (uses the height of the * current chain tip unless we are not synced with the current chain */ -static uint32_t GetLocktimeForNewTransaction(interfaces::Chain::Lock& locked_chain) +static uint32_t GetLocktimeForNewTransaction(interfaces::Chain& chain, interfaces::Chain::Lock& locked_chain) { uint32_t locktime; // Discourage fee sniping. @@ -3304,7 +3302,7 @@ static uint32_t GetLocktimeForNewTransaction(interfaces::Chain::Lock& locked_cha // enough, that fee sniping isn't a problem yet, but by implementing a fix // now we ensure code won't be written that makes assumptions about // nLockTime that preclude a fix later. - if (IsCurrentForAntiFeeSniping(locked_chain)) { + if (IsCurrentForAntiFeeSniping(chain, locked_chain)) { locktime = locked_chain.getHeight().value_or(-1); // Secondly occasionally randomly pick a nLockTime even further back, so @@ -3540,7 +3538,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std } CMutableTransaction txNew; - txNew.nLockTime = GetLocktimeForNewTransaction(locked_chain); + txNew.nLockTime = GetLocktimeForNewTransaction(chain(), locked_chain); FeeCalculation feeCalc; CFeeRate discard_rate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(*this); @@ -3630,7 +3628,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std } } - if (IsDust(txout, ::dustRelayFee)) + if (IsDust(txout, chain().relayDustFee())) { if (recipient.fSubtractFeeFromAmount && nFeeRet > 0) { @@ -5101,9 +5099,9 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, _("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) { + if (walletInstance->m_pay_tx_fee < chain.relayMinFee()) { chain.initError(strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)"), - gArgs.GetArg("-paytxfee", ""), ::minRelayTxFee.ToString())); + gArgs.GetArg("-paytxfee", ""), chain.relayMinFee().ToString())); return nullptr; } } @@ -5206,8 +5204,8 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, 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()); + // Register with the validation interface. It's ok to do this after rescan since we're still holding locked_chain. + walletInstance->m_chain_notifications_handler = chain.handleNotifications(*walletInstance); walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); @@ -5463,8 +5461,6 @@ int CMerkleTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const if (hashUnset()) return 0; - AssertLockHeld(cs_main); - return locked_chain.getBlockDepth(hashBlock) * (nIndex == -1 ? -1 : 1); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1e8e119df5..3c146ae2ae 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -507,7 +508,7 @@ public: CAmount GetCredit(interfaces::Chain::Lock& locked_chain, const isminefilter& filter) const; CAmount GetImmatureCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache=true) const; // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct - // annotation "EXCLUSIVE_LOCKS_REQUIRED(cs_main, pwallet->cs_wallet)". The + // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The // annotation "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid // having to resolve the issue of member access into incomplete type CWallet. CAmount GetAvailableCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache=true, const isminefilter& filter=ISMINE_SPENDABLE) const NO_THREAD_SAFETY_ANALYSIS; @@ -656,7 +657,7 @@ class WalletRescanReserver; //forward declarations for ScanForWalletTransactions * A CWallet is an extension of a keystore, which also maintains a set of transactions and balances, * and provides the ability to create new transactions. */ -class CWallet final : public CCryptoKeyStore, public CValidationInterface +class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notifications { private: std::atomic fAbortRescan{false}; @@ -850,6 +851,9 @@ public: std::map mapHdPubKeys; // m_chain_notifications_handler; + /** Interface for accessing chain state. */ interfaces::Chain& chain() const { return m_chain; } @@ -991,8 +995,8 @@ public: bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); void LoadToWallet(const CWalletTx& wtxIn) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override; - void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) override; - void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected) override; + void BlockConnected(const CBlock& block, const std::vector& vtxConflicted) override; + void BlockDisconnected(const CBlock& block) override; int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update); struct ScanResult { @@ -1013,7 +1017,7 @@ public: ScanResult ScanForWalletTransactions(const uint256& first_block, const uint256& last_block, const WalletRescanReserver& reserver, bool fUpdate); void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override; void ReacceptWalletTransactions(); - void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void ResendWalletTransactions(interfaces::Chain::Lock& locked_chain, int64_t nBestBlockTime) override; // ResendWalletTransactionsBefore may only be called if fBroadcastTransactions! std::vector ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime); struct Balance { @@ -1292,6 +1296,8 @@ public: /** Implement lookup of key origin information through wallet key metadata. */ bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override; + + friend struct WalletTestingSetup; }; /** A key allocated from the key pool. */ From 6cc3648faef88eeeca8464414c7ae9099b7e9bcb Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 17 Apr 2019 13:27:02 -0400 Subject: [PATCH 7/7] merge bitcoin#15779: Add wallet_balance benchmark --- src/bench/wallet_balance.cpp | 21 +++------------------ src/wallet/test/wallet_test_fixture.cpp | 7 ++++--- src/wallet/wallet.cpp | 12 +++++++++--- src/wallet/wallet.h | 12 ++++++++---- 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 08b9a3126c..9822956a17 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -11,31 +11,16 @@ #include -struct WalletTestingSetup { - std::unique_ptr m_chain = interfaces::MakeChain(); - CWallet m_wallet; - - WalletTestingSetup() - : m_wallet{*m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()} - { - } - - void handleNotifications() - { - m_wallet.m_chain_notifications_handler = m_chain->handleNotifications(m_wallet); - } -}; - static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_watchonly, const bool add_mine, const uint32_t epoch_iters) { const auto& ADDRESS_WATCHONLY = ADDRESS_B58T_UNSPENDABLE; - WalletTestingSetup wallet_t{}; - auto& wallet = wallet_t.m_wallet; + std::unique_ptr chain = interfaces::MakeChain(); + CWallet wallet{*chain.get(), WalletLocation(), WalletDatabase::CreateMock()}; { bool first_run; if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false); - wallet_t.handleNotifications(); + wallet.handleNotifications(); } diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index bd3f1e1f05..dc26ec10f3 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -8,12 +8,13 @@ #include #include -WalletTestingSetup::WalletTestingSetup(const std::string& chainName): - TestingSetup(chainName), m_wallet(*m_chain, WalletLocation(), WalletDatabase::CreateMock()) +WalletTestingSetup::WalletTestingSetup(const std::string& chainName) + : TestingSetup(chainName), + m_wallet(*m_chain, WalletLocation(), WalletDatabase::CreateMock()) { bool fFirstRun; m_wallet.LoadWallet(fFirstRun); - m_wallet.m_chain_notifications_handler = m_chain->handleNotifications(m_wallet); + m_wallet.handleNotifications(); m_chain_client->registerRpcs(); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 923328dd09..70261b394d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include #include @@ -18,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -33,6 +31,9 @@ #include #include #include +#include +#include +#include #include #include @@ -5205,7 +5206,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, chain.loadWallet(interfaces::MakeWallet(walletInstance)); // Register with the validation interface. It's ok to do this after rescan since we're still holding locked_chain. - walletInstance->m_chain_notifications_handler = chain.handleNotifications(*walletInstance); + walletInstance->handleNotifications(); walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); @@ -5220,6 +5221,11 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, return walletInstance; } +void CWallet::handleNotifications() +{ + m_chain_notifications_handler = m_chain.handleNotifications(*this); +} + void CWallet::postInitProcess() { // Add wallet transactions that aren't already in a block to mempool diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3c146ae2ae..ce48aad7e7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -13,12 +13,12 @@ #include #include #include +#include