From 79d579f4e11b57f90fed314bccd25230f918729f Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 26 Jul 2017 10:23:01 -0400 Subject: Remove uses of cs_main in wallet code This commit does not change behavior. It is easiest to review this change with: git log -p -n1 -U0 --- src/interfaces/chain.cpp | 24 ++++++++++ src/interfaces/chain.h | 20 +++++++++ src/interfaces/wallet.cpp | 55 +++++++++++++++-------- src/qt/test/wallettests.cpp | 2 +- src/wallet/feebumper.cpp | 13 ++++-- src/wallet/rpcdump.cpp | 30 ++++++++----- src/wallet/rpcwallet.cpp | 95 ++++++++++++++++++++++++++-------------- src/wallet/test/wallet_tests.cpp | 11 +++-- src/wallet/wallet.cpp | 71 ++++++++++++++++++++---------- 9 files changed, 227 insertions(+), 94 deletions(-) (limited to 'src') diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 28b36717d5..2571a91031 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -4,13 +4,37 @@ #include +#include #include +#include + +#include +#include namespace interfaces { namespace { +class LockImpl : public Chain::Lock +{ +}; + +class LockingStateImpl : public LockImpl, public UniqueLock +{ + using UniqueLock::UniqueLock; +}; + class ChainImpl : public Chain { +public: + std::unique_ptr lock(bool try_lock) override + { + auto result = MakeUnique(::cs_main, "cs_main", __FILE__, __LINE__, try_lock); + if (try_lock && result && !*result) return {}; + // std::move necessary on some compilers due to conversion from + // LockingStateImpl to Lock pointer + return std::move(result); + } + std::unique_ptr assumeLocked() override { return MakeUnique(); } }; } // namespace diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 30bc9f5f73..fe5658de4b 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -18,6 +18,26 @@ class Chain { public: virtual ~Chain() {} + + //! Interface for querying locked chain state, used by legacy code that + //! assumes state won't change between calls. New code should avoid using + //! the Lock interface and instead call higher-level Chain methods + //! that return more information so the chain doesn't need to stay locked + //! between calls. + class Lock + { + public: + virtual ~Lock() {} + }; + + //! Return Lock interface. Chain is locked when this is called, and + //! unlocked when the returned interface is freed. + virtual std::unique_ptr lock(bool try_lock = false) = 0; + + //! Return Lock interface assuming chain is already locked. This + //! method is temporary and is only used in a few places to avoid changing + //! behavior while code is transitioned to use the Chain::Lock interface. + virtual std::unique_ptr assumeLocked() = 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 a29440ee4a..d03daf88dd 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -53,7 +53,8 @@ public: WalletOrderForm order_form, std::string& reject_reason) override { - LOCK2(cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); CValidationState state; if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), std::move(order_form), m_key, g_connman.get(), state)) { reject_reason = state.GetRejectReason(); @@ -209,22 +210,26 @@ public: } void lockCoin(const COutPoint& output) override { - LOCK2(cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.LockCoin(output); } void unlockCoin(const COutPoint& output) override { - LOCK2(cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.UnlockCoin(output); } bool isLockedCoin(const COutPoint& output) override { - LOCK2(cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.IsLockedCoin(output.hash, output.n); } void listLockedCoins(std::vector& outputs) override { - LOCK2(cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.ListLockedCoins(outputs); } std::unique_ptr createTransaction(const std::vector& recipients, @@ -234,7 +239,8 @@ public: CAmount& fee, std::string& fail_reason) override { - LOCK2(cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); auto pending = MakeUnique(m_wallet); if (!m_wallet.CreateTransaction(recipients, pending->m_tx, pending->m_key, fee, change_pos, fail_reason, coin_control, sign)) { @@ -245,7 +251,8 @@ public: bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet.TransactionCanBeAbandoned(txid); } bool abandonTransaction(const uint256& txid) override { - LOCK2(cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.AbandonTransaction(txid); } bool transactionCanBeBumped(const uint256& txid) override @@ -274,7 +281,8 @@ public: } CTransactionRef getTx(const uint256& txid) override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); auto mi = m_wallet.mapWallet.find(txid); if (mi != m_wallet.mapWallet.end()) { return mi->second.tx; @@ -283,7 +291,8 @@ public: } WalletTx getWalletTx(const uint256& txid) override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); auto mi = m_wallet.mapWallet.find(txid); if (mi != m_wallet.mapWallet.end()) { return MakeWalletTx(m_wallet, mi->second); @@ -292,7 +301,8 @@ public: } std::vector getWalletTxs() override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); std::vector result; result.reserve(m_wallet.mapWallet.size()); for (const auto& entry : m_wallet.mapWallet) { @@ -304,7 +314,7 @@ public: interfaces::WalletTxStatus& tx_status, int& num_blocks) override { - TRY_LOCK(::cs_main, locked_chain); + auto locked_chain = m_wallet.chain().lock(true /* try_lock */); if (!locked_chain) { return false; } @@ -326,7 +336,8 @@ public: bool& in_mempool, int& num_blocks) override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); auto mi = m_wallet.mapWallet.find(txid); if (mi != m_wallet.mapWallet.end()) { num_blocks = ::chainActive.Height(); @@ -353,7 +364,7 @@ public: } bool tryGetBalances(WalletBalances& balances, int& num_blocks) override { - TRY_LOCK(cs_main, locked_chain); + auto locked_chain = m_wallet.chain().lock(true /* try_lock */); if (!locked_chain) return false; TRY_LOCK(m_wallet.cs_wallet, locked_wallet); if (!locked_wallet) { @@ -370,27 +381,32 @@ public: } isminetype txinIsMine(const CTxIn& txin) override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.IsMine(txin); } isminetype txoutIsMine(const CTxOut& txout) override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.IsMine(txout); } CAmount getDebit(const CTxIn& txin, isminefilter filter) override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.GetDebit(txin, filter); } CAmount getCredit(const CTxOut& txout, isminefilter filter) override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); return m_wallet.GetCredit(txout, filter); } CoinsList listCoins() override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); CoinsList result; for (const auto& entry : m_wallet.ListCoins()) { auto& group = result[entry.first]; @@ -403,7 +419,8 @@ public: } std::vector getCoins(const std::vector& outputs) override { - LOCK2(::cs_main, m_wallet.cs_wallet); + auto locked_chain = m_wallet.chain().lock(); + LOCK(m_wallet.cs_wallet); std::vector result; result.reserve(outputs.size()); for (const auto& output : outputs) { diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 57ae2150c8..f02fd8aea7 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -143,7 +143,7 @@ void TestGUI() wallet->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); } { - LOCK(cs_main); + auto locked_chain = wallet->chain().lock(); WalletRescanReserver reserver(wallet.get()); reserver.reserve(); wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, true); diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 5eb70cd7a5..1fb7f13313 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -64,7 +65,8 @@ namespace feebumper { bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid) { - LOCK2(cs_main, wallet->cs_wallet); + auto locked_chain = wallet->chain().lock(); + LOCK(wallet->cs_wallet); const CWalletTx* wtx = wallet->GetWalletTx(txid); if (wtx == nullptr) return false; @@ -76,7 +78,8 @@ bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid) Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoinControl& coin_control, CAmount total_fee, std::vector& errors, CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx) { - LOCK2(cs_main, wallet->cs_wallet); + auto locked_chain = wallet->chain().lock(); + LOCK(wallet->cs_wallet); errors.clear(); auto it = wallet->mapWallet.find(txid); if (it == wallet->mapWallet.end()) { @@ -212,13 +215,15 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin } bool SignTransaction(CWallet* wallet, CMutableTransaction& mtx) { - LOCK2(cs_main, wallet->cs_wallet); + auto locked_chain = wallet->chain().lock(); + LOCK(wallet->cs_wallet); return wallet->SignTransaction(mtx); } Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector& errors, uint256& bumped_txid) { - LOCK2(cs_main, wallet->cs_wallet); + auto locked_chain = wallet->chain().lock(); + LOCK(wallet->cs_wallet); if (!errors.empty()) { return Result::MISC_ERROR; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 2a4cf0147e..05e14fd64a 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -133,7 +134,8 @@ UniValue importprivkey(const JSONRPCRequest& request) WalletRescanReserver reserver(pwallet); bool fRescan = true; { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -305,7 +307,8 @@ UniValue importaddress(const JSONRPCRequest& request) fP2SH = request.params[3].get_bool(); { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); CTxDestination dest = DecodeDestination(request.params[0].get_str()); if (IsValidDestination(dest)) { @@ -362,7 +365,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request) unsigned int txnIndex = 0; if (merkleBlock.txn.ExtractMatches(vMatch, vIndex) == merkleBlock.header.hashMerkleRoot) { - LOCK(cs_main); + auto locked_chain = pwallet->chain().lock(); const CBlockIndex* pindex = LookupBlockIndex(merkleBlock.header.GetHash()); if (!pindex || !chainActive.Contains(pindex)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain"); @@ -382,7 +385,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request) wtx.nIndex = txnIndex; wtx.hashBlock = merkleBlock.header.GetHash(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (pwallet->IsMine(*wtx.tx)) { pwallet->AddToWallet(wtx, false); @@ -412,7 +416,8 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) + HelpExampleRpc("removeprunedfunds", "\"a8d0c0184dde994a09ec054286f1ce581bebf46446a512166eae7628734ea0a5\"") ); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); uint256 hash(ParseHashV(request.params[0], "txid")); std::vector vHash; @@ -483,7 +488,8 @@ UniValue importpubkey(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); for (const auto& dest : GetAllDestinationsForKey(pubKey)) { ImportAddress(pwallet, dest, strLabel); @@ -535,7 +541,8 @@ UniValue importwallet(const JSONRPCRequest& request) int64_t nTimeBegin = 0; bool fGood = true; { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -653,7 +660,8 @@ UniValue dumpprivkey(const JSONRPCRequest& request) + HelpExampleRpc("dumpprivkey", "\"myaddress\"") ); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -700,7 +708,8 @@ UniValue dumpwallet(const JSONRPCRequest& request) + HelpExampleRpc("dumpwallet", "\"test\"") ); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -1134,7 +1143,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) int64_t nLowestTimestamp = 0; UniValue response(UniValue::VARR); { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); // Verify all timestamps are present before importing any keys. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b8e874fec6..8db7a82476 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -374,7 +375,8 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); CTxDestination dest = DecodeDestination(request.params[0].get_str()); if (!IsValidDestination(dest)) { @@ -456,7 +458,8 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); UniValue jsonGroupings(UniValue::VARR); std::map balances = pwallet->GetAddressBalances(); @@ -509,7 +512,8 @@ static UniValue signmessage(const JSONRPCRequest& request) + HelpExampleRpc("signmessage", "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\", \"my message\"") ); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -575,7 +579,8 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); // Bitcoin address CTxDestination dest = DecodeDestination(request.params[0].get_str()); @@ -642,7 +647,8 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); // Minimum confirmations int nMinDepth = 1; @@ -708,7 +714,8 @@ static UniValue getbalance(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); const UniValue& dummy_value = request.params[0]; if (!dummy_value.isNull() && dummy_value.get_str() != "*") { @@ -746,7 +753,8 @@ static UniValue getunconfirmedbalance(const JSONRPCRequest &request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); return ValueFromAmount(pwallet->GetUnconfirmedBalance()); } @@ -807,7 +815,8 @@ static UniValue sendmany(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (pwallet->GetBroadcastTransactions() && !g_connman) { throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); @@ -946,7 +955,8 @@ static UniValue addmultisigaddress(const JSONRPCRequest& request) throw std::runtime_error(msg); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); std::string label; if (!request.params[2].isNull()) @@ -1186,7 +1196,8 @@ static UniValue listreceivedbyaddress(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); return ListReceived(pwallet, request.params, false); } @@ -1230,7 +1241,8 @@ static UniValue listreceivedbylabel(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); return ListReceived(pwallet, request.params, true); } @@ -1406,7 +1418,8 @@ UniValue listtransactions(const JSONRPCRequest& request) UniValue ret(UniValue::VARR); { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); const CWallet::TxItems & txOrdered = pwallet->wtxOrdered; @@ -1506,7 +1519,8 @@ static UniValue listsinceblock(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); const CBlockIndex* pindex = nullptr; // Block index of the specified block or the common ancestor, if the block provided was in a deactivated chain. const CBlockIndex* paltindex = nullptr; // Block index of the specified block, even if it's in a deactivated chain. @@ -1641,7 +1655,8 @@ static UniValue gettransaction(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); uint256 hash(ParseHashV(request.params[0], "txid")); @@ -1708,7 +1723,8 @@ static UniValue abandontransaction(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); uint256 hash(ParseHashV(request.params[0], "txid")); @@ -1747,7 +1763,8 @@ static UniValue backupwallet(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); std::string strDest = request.params[0].get_str(); if (!pwallet->BackupWallet(strDest)) { @@ -1783,7 +1800,8 @@ static UniValue keypoolrefill(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); // 0 is interpreted by TopUpKeyPool() as the default keypool size given by -keypool unsigned int kpSize = 0; @@ -1834,7 +1852,8 @@ static UniValue walletpassphrase(const JSONRPCRequest& request) ); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); @@ -1912,7 +1931,8 @@ static UniValue walletpassphrasechange(const JSONRPCRequest& request) ); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called."); @@ -1968,7 +1988,8 @@ static UniValue walletlock(const JSONRPCRequest& request) ); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletlock was called."); @@ -2014,7 +2035,8 @@ static UniValue encryptwallet(const JSONRPCRequest& request) ); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an encrypted wallet, but encryptwallet was called."); @@ -2088,7 +2110,8 @@ static UniValue lockunspent(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); RPCTypeCheckArgument(request.params[0], UniValue::VBOOL); @@ -2198,7 +2221,8 @@ static UniValue listlockunspent(const JSONRPCRequest& request) + HelpExampleRpc("listlockunspent", "") ); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); std::vector vOutpts; pwallet->ListLockedCoins(vOutpts); @@ -2239,7 +2263,8 @@ static UniValue settxfee(const JSONRPCRequest& request) ); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); CAmount nAmount = AmountFromValue(request.params[0]); CFeeRate tx_fee_rate(nAmount, 1000); @@ -2294,7 +2319,8 @@ static UniValue getwalletinfo(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); UniValue obj(UniValue::VOBJ); @@ -2563,7 +2589,8 @@ static UniValue resendwallettransactions(const JSONRPCRequest& request) if (!g_connman) throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled"); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (!pwallet->GetBroadcastTransactions()) { throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet transaction broadcasting is disabled with -walletbroadcast"); @@ -2700,7 +2727,8 @@ static UniValue listunspent(const JSONRPCRequest& request) UniValue results(UniValue::VARR); std::vector vecOutputs; { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); pwallet->AvailableCoins(vecOutputs, !include_unsafe, nullptr, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount, nMinDepth, nMaxDepth); } @@ -3019,7 +3047,8 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) } // Sign the transaction - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); return SignTransaction(pwallet->chain(), mtx, request.params[1], pwallet, false, request.params[2]); @@ -3124,7 +3153,8 @@ static UniValue bumpfee(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -3264,7 +3294,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) CBlockIndex *pindexStop = nullptr; CBlockIndex *pChainTip = nullptr; { - LOCK(cs_main); + auto locked_chain = pwallet->chain().lock(); pindexStart = chainActive.Genesis(); pChainTip = chainActive.Tip(); @@ -3288,7 +3318,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) // We can't rescan beyond non-pruned blocks, stop and throw an error if (fPruneMode) { - LOCK(cs_main); + auto locked_chain = pwallet->chain().lock(); CBlockIndex *block = pindexStop ? pindexStop : pChainTip; while (block && block->nHeight >= pindexStart->nHeight) { if (!(block->nStatus & BLOCK_HAVE_DATA)) { @@ -3688,7 +3718,8 @@ UniValue sethdseed(const JSONRPCRequest& request) throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Cannot set a new HD seed while still in Initial Block Download"); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); // Do not do anything to non-HD wallets if (!pwallet->IsHDEnabled()) { diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 6d943d63a4..df3eaff186 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -43,7 +44,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); CBlockIndex* newTip = chainActive.Tip(); - LOCK(cs_main); + auto locked_chain = chain->lock(); // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. @@ -132,7 +133,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) SetMockTime(KEY_TIME); m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); - LOCK(cs_main); + auto locked_chain = chain->lock(); std::string backup_file = (SetDataDir("importwallet_rescan") / "wallet.backup").string(); @@ -187,7 +188,8 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) auto chain = interfaces::MakeChain(); CWallet wallet(*chain, WalletLocation(), WalletDatabase::CreateDummy()); CWalletTx wtx(&wallet, m_coinbase_txns.back()); - LOCK2(cs_main, wallet.cs_wallet); + auto locked_chain = chain->lock(); + LOCK(wallet.cs_wallet); wtx.hashBlock = chainActive.Tip()->GetBlockHash(); wtx.nIndex = 0; @@ -209,7 +211,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 SetMockTime(mockTime); CBlockIndex* block = nullptr; if (blockTime > 0) { - LOCK(cs_main); + auto locked_chain = wallet.chain().lock(); auto inserted = mapBlockIndex.emplace(GetRandHash(), new CBlockIndex); assert(inserted.second); const uint256& hash = inserted.first->first; @@ -317,6 +319,7 @@ public: } std::unique_ptr m_chain = interfaces::MakeChain(); + std::unique_ptr m_locked_chain = m_chain->assumeLocked(); // Temporary. Removed in upcoming lock cleanup std::unique_ptr wallet; }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7c9708cf5e..6e657bcdcd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1005,7 +1006,8 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const CBlockI bool CWallet::TransactionCanBeAbandoned(const uint256& hashTx) const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); const CWalletTx* wtx = GetWalletTx(hashTx); return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() == 0 && !wtx->InMempool(); } @@ -1022,7 +1024,8 @@ void CWallet::MarkInputsDirty(const CTransactionRef& tx) bool CWallet::AbandonTransaction(const uint256& hashTx) { - LOCK2(cs_main, cs_wallet); + auto locked_chain_recursive = chain().lock(); // Temporary. Removed in upcoming lock cleanup + LOCK(cs_wallet); WalletBatch batch(*database, "r+"); @@ -1077,7 +1080,8 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); int conflictconfirms = 0; CBlockIndex* pindex = LookupBlockIndex(hashBlock); @@ -1140,7 +1144,8 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pin } void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); SyncTransaction(ptx); auto it = mapWallet.find(ptx->GetHash()); @@ -1158,7 +1163,8 @@ void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) { } void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); // TODO: Temporarily ensure that mempool removals are notified before // connected transactions. This shouldn't matter, but the abandoned // state of transactions in our wallet is currently cleared when we @@ -1180,7 +1186,8 @@ void CWallet::BlockConnected(const std::shared_ptr& pblock, const } void CWallet::BlockDisconnected(const std::shared_ptr& pblock) { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); for (const CTransactionRef& ptx : pblock->vtx) { SyncTransaction(ptx); @@ -1199,7 +1206,7 @@ void CWallet::BlockUntilSyncedToCurrentChain() { // We could also take cs_wallet here, and call m_last_block_processed // protected by cs_wallet instead of cs_main, but as long as we need // cs_main here anyway, it's easier to just call it cs_main-protected. - LOCK(cs_main); + auto locked_chain = chain().lock(); const CBlockIndex* initialChainTip = chainActive.Tip(); if (m_last_block_processed && m_last_block_processed->GetAncestor(initialChainTip->nHeight) == initialChainTip) { @@ -1600,7 +1607,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r // to be scanned. CBlockIndex* startBlock = nullptr; { - LOCK(cs_main); + auto locked_chain = chain().lock(); startBlock = chainActive.FindEarliestAtLeast(startTime - TIMESTAMP_WINDOW); WalletLogPrintf("%s: Rescanning last %i blocks\n", __func__, startBlock ? chainActive.Height() - startBlock->nHeight + 1 : 0); } @@ -1652,7 +1659,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock double progress_begin; double progress_end; { - LOCK(cs_main); + auto locked_chain = chain().lock(); progress_begin = GuessVerificationProgress(chainParams.TxData(), pindex); if (pindexStop == nullptr) { tip = chainActive.Tip(); @@ -1674,7 +1681,8 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock CBlock block; if (ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); if (pindex && !chainActive.Contains(pindex)) { // Abort scan if current block is no longer active, to prevent // marking transactions as coming from the wrong block. @@ -1691,7 +1699,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, CBlock break; } { - LOCK(cs_main); + auto locked_chain = chain().lock(); pindex = chainActive.Next(pindex); progress_current = GuessVerificationProgress(chainParams.TxData(), pindex); if (pindexStop == nullptr && tip != chainActive.Tip()) { @@ -1716,7 +1724,8 @@ void CWallet::ReacceptWalletTransactions() // If transactions aren't being broadcasted, don't let them into local mempool either if (!fBroadcastTransactions) return; - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); std::map mapSorted; // Sort pending wallet transactions based on their initial wallet insertion order @@ -2007,6 +2016,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(nBestBlockTime-5*60, connman); if (!relayed.empty()) WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed.size()); @@ -2027,7 +2037,8 @@ CAmount CWallet::GetBalance(const isminefilter& filter, const int min_depth) con { CAmount nTotal = 0; { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; @@ -2044,7 +2055,8 @@ CAmount CWallet::GetUnconfirmedBalance() const { CAmount nTotal = 0; { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; @@ -2059,7 +2071,8 @@ CAmount CWallet::GetImmatureBalance() const { CAmount nTotal = 0; { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; @@ -2073,7 +2086,8 @@ CAmount CWallet::GetUnconfirmedWatchOnlyBalance() const { CAmount nTotal = 0; { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; @@ -2088,7 +2102,8 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const { CAmount nTotal = 0; { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; @@ -2106,7 +2121,8 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const // trusted. CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth) const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); CAmount balance = 0; for (const auto& entry : mapWallet) { @@ -2139,7 +2155,8 @@ CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth) cons CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); CAmount balance = 0; std::vector vCoins; @@ -2499,7 +2516,8 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC // Acquire the locks to prevent races to the new locked unspents between the // CreateTransaction call and LockCoin calls (when lockUnspents is true). - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); CReserveKey reservekey(this); CTransactionRef tx_new; @@ -2624,7 +2642,8 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac int nBytes; { std::set setCoins; - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); { std::vector vAvailableCoins; AvailableCoins(vAvailableCoins, true, &coin_control); @@ -2962,7 +2981,8 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, CReserveKey& reservekey, CConnman* connman, CValidationState& state) { { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); CWalletTx wtxNew(this, std::move(tx)); wtxNew.mapValue = std::move(mapValue); @@ -3008,7 +3028,8 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve DBErrors CWallet::LoadWallet(bool& fFirstRunRet) { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); fFirstRunRet = false; DBErrors nLoadWalletRet = WalletBatch(*database,"cr+").LoadWallet(this); @@ -4010,6 +4031,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, return nullptr; } + auto locked_chain = chain.assumeLocked(); // Temporary. Removed in upcoming lock cleanup walletInstance->ChainStateFlushed(chainActive.GetLocator()); } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { // Make it impossible to disable private keys after creation @@ -4097,7 +4119,8 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // Try to top up keypool. No-op if the wallet is locked. walletInstance->TopUpKeyPool(); - LOCK2(cs_main, walletInstance->cs_wallet); + auto locked_chain = chain.lock(); + LOCK(walletInstance->cs_wallet); CBlockIndex *pindexRescan = chainActive.Genesis(); if (!gArgs.GetBoolArg("-rescan", false)) -- cgit v1.2.3