diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -15,6 +15,25 @@ 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/chain.cpp b/src/interfaces/chain.cpp --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -4,12 +4,39 @@ #include +#include #include +#include + +#include +#include namespace interfaces { namespace { - class ChainImpl : public Chain {}; + 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 = std::make_unique( + ::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 std::make_unique(); + } + }; } // namespace diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -43,7 +43,8 @@ bool commit(WalletValueMap value_map, WalletOrderForm order_form, std::string from_account, 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), @@ -217,19 +218,23 @@ return m_wallet.GetDestValues(prefix); } 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); } 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 @@ -237,7 +242,8 @@ const CCoinControl &coin_control, bool sign, int &change_pos, Amount &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 = std::make_unique(m_wallet); if (!m_wallet.CreateTransaction(recipients, pending->m_tx, pending->m_key, fee, change_pos, @@ -250,11 +256,13 @@ return m_wallet.TransactionCanBeAbandoned(txid); } bool abandonTransaction(const TxId &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); } CTransactionRef getTx(const TxId &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; @@ -262,7 +270,8 @@ return {}; } WalletTx getWalletTx(const TxId &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); @@ -270,7 +279,8 @@ return {}; } 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) { @@ -281,7 +291,7 @@ bool tryGetTxStatus(const TxId &txid, interfaces::WalletTxStatus &tx_status, int &num_blocks, int64_t &block_time) override { - TRY_LOCK(::cs_main, locked_chain); + auto locked_chain = m_wallet.chain().lock(true /* try_lock */); if (!locked_chain) { return false; } @@ -302,7 +312,8 @@ WalletOrderForm &order_form, 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(); @@ -331,7 +342,7 @@ } 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; } @@ -348,23 +359,28 @@ return m_wallet.GetAvailableBalance(&coin_control); } 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); } Amount 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); } Amount 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]; @@ -378,7 +394,8 @@ } 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 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -127,7 +127,7 @@ 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, diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -154,7 +155,8 @@ WalletRescanReserver reserver(pwallet); bool fRescan = true; { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -366,7 +368,8 @@ } { - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); CTxDestination dest = DecodeDestination(request.params[0].get_str(), config.GetChainParams()); @@ -437,7 +440,7 @@ size_t 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)) { @@ -461,7 +464,8 @@ 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); @@ -500,7 +504,8 @@ "eae7628734ea0a5\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); TxId txid; txid.SetHex(request.params[0].get_str()); @@ -592,7 +597,8 @@ } { - 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); @@ -645,7 +651,8 @@ 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); @@ -829,7 +836,8 @@ HelpExampleRpc("dumpprivkey", "\"myaddress\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -883,7 +891,8 @@ HelpExampleCli("dumpwallet", "\"test\"") + HelpExampleRpc("dumpwallet", "\"test\"")); - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -1464,7 +1473,8 @@ 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 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -283,7 +284,8 @@ HelpExampleCli("getaccountaddress", "\"myaccount\"") + HelpExampleRpc("getaccountaddress", "\"myaccount\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); // Parse the account first so we don't generate a key if there's an error std::string account = LabelFromValue(request.params[0]); @@ -478,7 +480,8 @@ "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); CTxDestination dest = DecodeDestination(request.params[0].get_str(), config.GetChainParams()); @@ -537,7 +540,8 @@ HelpExampleRpc("getaddressesbyaccount", "\"tabby\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); std::string strAccount = LabelFromValue(request.params[0]); @@ -668,7 +672,8 @@ // 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(), config.GetChainParams()); @@ -743,7 +748,8 @@ // 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(); @@ -808,7 +814,8 @@ "\"1D1ZrZNe3JUo7ZycKEYQQiQAWd9y54F4XX\", \"my message\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); @@ -887,7 +894,8 @@ // 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 = @@ -986,7 +994,8 @@ // 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; @@ -1111,7 +1120,8 @@ // 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 &account_value = request.params[0]; @@ -1167,7 +1177,8 @@ // 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()); } @@ -1229,7 +1240,8 @@ "\"timotei\", \"akiko\", 0.01, 6, \"happy birthday!\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); std::string strFrom = LabelFromValue(request.params[0]); std::string strTo = LabelFromValue(request.params[1]); @@ -1335,7 +1347,8 @@ // 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 label = LabelFromValue(request.params[0]); CTxDestination dest = @@ -1540,7 +1553,8 @@ // 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( @@ -1706,7 +1720,8 @@ 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()) { @@ -1969,7 +1984,9 @@ // 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(config, pwallet, request.params, false); } @@ -2035,7 +2052,8 @@ // 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(config, pwallet, request.params, true); } @@ -2353,7 +2371,8 @@ // 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 strAccount = "*"; if (!request.params[0].isNull()) { @@ -2493,7 +2512,8 @@ // 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); int nMinDepth = 1; if (!request.params[0].isNull()) { @@ -2677,7 +2697,8 @@ // 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); // Block index of the specified block or the common ancestor, if the block // provided was in a deactivated chain. @@ -2866,7 +2887,8 @@ // 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); TxId txid; txid.SetHex(request.params[0].get_str()); @@ -2943,7 +2965,8 @@ // 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); TxId txid; txid.SetHex(request.params[0].get_str()); @@ -2986,7 +3009,8 @@ // 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)) { @@ -3024,7 +3048,8 @@ "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 @@ -3085,7 +3110,8 @@ HelpExampleRpc("walletpassphrase", "\"my pass phrase\", 60")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (request.fHelp) { return true; @@ -3167,7 +3193,8 @@ "\"old one\", \"new one\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (request.fHelp) { return true; @@ -3236,7 +3263,8 @@ HelpExampleRpc("walletlock", "")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (request.fHelp) { return true; @@ -3293,7 +3321,8 @@ HelpExampleRpc("encryptwallet", "\"my pass phrase\"")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); if (request.fHelp) { return true; @@ -3398,7 +3427,8 @@ // 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); @@ -3529,7 +3559,8 @@ "\nAs a json rpc call\n" + HelpExampleRpc("listlockunspent", "")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); std::vector vOutpts; pwallet->ListLockedCoins(vOutpts); @@ -3573,7 +3604,8 @@ HelpExampleRpc("settxfee", "0.00001")); } - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); Amount nAmount = AmountFromValue(request.params[0]); CFeeRate tx_fee_rate(nAmount, 1000); @@ -3661,7 +3693,8 @@ // 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); @@ -3946,7 +3979,8 @@ "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 " @@ -4137,7 +4171,8 @@ 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, @@ -4505,7 +4540,8 @@ } // Sign the transaction - LOCK2(cs_main, pwallet->cs_wallet); + auto locked_chain = pwallet->chain().lock(); + LOCK(pwallet->cs_wallet); return SignTransaction(pwallet->chain(), mtx, request.params[1], pwallet, false, request.params[2]); } @@ -4602,7 +4638,7 @@ CBlockIndex *pindexStop = nullptr; CBlockIndex *pChainTip = nullptr; { - LOCK(cs_main); + auto locked_chain = pwallet->chain().lock(); pindexStart = chainActive.Genesis(); pChainTip = chainActive.Tip(); @@ -4629,7 +4665,7 @@ // 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.hasData()) { @@ -5081,7 +5117,8 @@ "Cannot set a HD seed to a wallet with private keys disabled"); } - 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->CanSupportFeature(FEATURE_HD)) { diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -42,7 +43,7 @@ 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. @@ -152,7 +153,7 @@ GetScriptForRawPubKey(coinbaseKey.GetPubKey())) .vtx[0]); - LOCK(cs_main); + auto locked_chain = chain->lock(); std::string backup_file = (SetDataDir("importwallet_rescan") / "wallet.backup").string(); @@ -211,7 +212,8 @@ CWallet wallet(Params(), *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; @@ -233,7 +235,7 @@ SetMockTime(mockTime); CBlockIndex *block = nullptr; if (blockTime > 0) { - LOCK(cs_main); + auto locked_chain = wallet.chain().lock(); auto inserted = mapBlockIndex.emplace(BlockHash(GetRandHash()), new CBlockIndex); assert(inserted.second); @@ -341,6 +343,9 @@ } std::unique_ptr m_chain = interfaces::MakeChain(); + // Temporary. Removed in upcoming lock cleanup + std::unique_ptr m_locked_chain = + m_chain->assumeLocked(); std::unique_ptr wallet; }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1167,7 +1168,8 @@ } bool CWallet::TransactionCanBeAbandoned(const TxId &txid) const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); const CWalletTx *wtx = GetWalletTx(txid); return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() == 0 && !wtx->InMempool(); @@ -1183,7 +1185,9 @@ } bool CWallet::AbandonTransaction(const TxId &txid) { - LOCK2(cs_main, cs_wallet); + // Temporary. Removed in upcoming lock cleanup + auto locked_chain_recursive = chain().lock(); + LOCK(cs_wallet); WalletBatch batch(*database, "r+"); @@ -1243,7 +1247,8 @@ } void CWallet::MarkConflicted(const BlockHash &hashBlock, const TxId &txid) { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); int conflictconfirms = 0; CBlockIndex *pindex = LookupBlockIndex(hashBlock); @@ -1314,7 +1319,8 @@ } 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->GetId()); @@ -1334,7 +1340,8 @@ 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 @@ -1357,7 +1364,8 @@ } 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); @@ -1374,9 +1382,8 @@ // 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) { @@ -1832,7 +1839,7 @@ // to be scanned. CBlockIndex *startBlock = nullptr; { - LOCK(cs_main); + auto locked_chain = chain().lock(); startBlock = chainActive.FindEarliestAtLeast(startTime - TIMESTAMP_WINDOW); WalletLogPrintf( @@ -1894,7 +1901,7 @@ double progress_begin; double progress_end; { - LOCK(cs_main); + auto locked_chain = chain().lock(); progress_begin = GuessVerificationProgress(chainParams.TxData(), pindex); if (pindexStop == nullptr) { @@ -1926,7 +1933,8 @@ CBlock block; if (ReadBlockFromDisk(block, pindex, chainParams.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 @@ -1946,7 +1954,7 @@ break; } { - LOCK(cs_main); + auto locked_chain = chain().lock(); pindex = chainActive.Next(pindex); progress_current = GuessVerificationProgress(chainParams.TxData(), pindex); @@ -1982,7 +1990,8 @@ 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 @@ -2295,6 +2304,8 @@ nLastResend = GetTime(); + // Temporary. Removed in upcoming lock cleanup + auto locked_chain = chain().assumeLocked(); // Rebroadcast unconfirmed txes older than 5 minutes before the last block // was found: std::vector relayed = @@ -2314,7 +2325,8 @@ */ Amount CWallet::GetBalance(const isminefilter &filter, const int min_depth) const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); Amount nTotal = Amount::zero(); for (const auto &entry : mapWallet) { @@ -2328,7 +2340,8 @@ } Amount CWallet::GetUnconfirmedBalance() const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); Amount nTotal = Amount::zero(); for (const auto &entry : mapWallet) { @@ -2343,7 +2356,8 @@ } Amount CWallet::GetImmatureBalance() const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); Amount nTotal = Amount::zero(); for (const auto &entry : mapWallet) { @@ -2355,7 +2369,8 @@ } Amount CWallet::GetUnconfirmedWatchOnlyBalance() const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); Amount nTotal = Amount::zero(); for (const auto &entry : mapWallet) { @@ -2370,7 +2385,8 @@ } Amount CWallet::GetImmatureWatchOnlyBalance() const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); Amount nTotal = Amount::zero(); for (const auto &entry : mapWallet) { @@ -2389,7 +2405,8 @@ // trusted. Amount CWallet::GetLegacyBalance(const isminefilter &filter, int minDepth, const std::string *account) const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); Amount balance = Amount::zero(); for (const auto &entry : mapWallet) { @@ -2427,7 +2444,8 @@ } Amount CWallet::GetAvailableBalance(const CCoinControl *coinControl) const { - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); Amount balance = Amount::zero(); std::vector vCoins; @@ -2888,7 +2906,8 @@ // 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; @@ -3007,7 +3026,8 @@ { std::set setCoins; - LOCK2(cs_main, cs_wallet); + auto locked_chain = chain().lock(); + LOCK(cs_wallet); std::vector vAvailableCoins; AvailableCoins(vAvailableCoins, true, &coinControl); @@ -3380,7 +3400,8 @@ std::vector> orderForm, std::string fromAccount, 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); @@ -3452,7 +3473,8 @@ } 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); @@ -4593,6 +4615,8 @@ return nullptr; } + // Temporary. Removed in upcoming lock cleanup + auto locked_chain = chain.assumeLocked(); walletInstance->ChainStateFlushed(chainActive.GetLocator()); } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { // Make it impossible to disable private keys after creation @@ -4692,7 +4716,8 @@ // 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)) {