From 081accb875412f38718f2e40ed7bbdf15e6f4ef8 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Mon, 31 Jul 2017 15:30:21 -0400 Subject: [PATCH] Pass chain locked variables where needed This commit does not change behavior. All it does is pass new function parameters. It is easiest to review this change with: git log -p -n1 -U0 --word-diff-regex=. --- src/interfaces/wallet.cpp | 48 +++++++----- src/threadsafety.h | 11 +++ src/wallet/feebumper.cpp | 10 +-- src/wallet/rpcdump.cpp | 2 +- src/wallet/rpcwallet.cpp | 66 ++++++++-------- src/wallet/test/wallet_tests.cpp | 16 ++-- src/wallet/wallet.cpp | 130 ++++++++++++++++--------------- src/wallet/wallet.h | 44 +++++------ 8 files changed, 175 insertions(+), 152 deletions(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index d03daf88dda..672a557d416 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -69,7 +69,7 @@ public: }; //! Construct wallet tx struct. -static WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, const CWalletTx& wtx) { WalletTx result; result.tx = wtx.tx; @@ -87,7 +87,7 @@ static WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) EXCLUSIVE_LO IsMine(wallet, result.txout_address.back()) : ISMINE_NO); } - result.credit = wtx.GetCredit(ISMINE_ALL); + result.credit = wtx.GetCredit(locked_chain, ISMINE_ALL); result.debit = wtx.GetDebit(ISMINE_ALL); result.change = wtx.GetChange(); result.time = wtx.GetTxTime(); @@ -97,32 +97,38 @@ static WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) EXCLUSIVE_LO } //! Construct wallet tx status struct. -static WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx) { + LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. + WalletTxStatus result; auto mi = ::mapBlockIndex.find(wtx.hashBlock); CBlockIndex* block = mi != ::mapBlockIndex.end() ? mi->second : nullptr; result.block_height = (block ? block->nHeight : std::numeric_limits::max()); - result.blocks_to_maturity = wtx.GetBlocksToMaturity(); - result.depth_in_main_chain = wtx.GetDepthInMainChain(); + result.blocks_to_maturity = wtx.GetBlocksToMaturity(locked_chain); + 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_trusted = wtx.IsTrusted(); + result.is_trusted = wtx.IsTrusted(locked_chain); result.is_abandoned = wtx.isAbandoned(); result.is_coinbase = wtx.IsCoinBase(); - result.is_in_main_chain = wtx.IsInMainChain(); + result.is_in_main_chain = wtx.IsInMainChain(locked_chain); return result; } //! Construct wallet TxOut struct. -static WalletTxOut MakeWalletTxOut(CWallet& wallet, const CWalletTx& wtx, int n, int depth) EXCLUSIVE_LOCKS_REQUIRED(cs_main, wallet.cs_wallet) +WalletTxOut MakeWalletTxOut(interfaces::Chain::Lock& locked_chain, + CWallet& wallet, + const CWalletTx& wtx, + int n, + int depth) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { WalletTxOut result; result.txout = wtx.tx->vout[n]; result.time = wtx.GetTxTime(); result.depth_in_main_chain = depth; - result.is_spent = wallet.IsSpent(wtx.GetHash(), n); + result.is_spent = wallet.IsSpent(locked_chain, wtx.GetHash(), n); return result; } @@ -242,7 +248,7 @@ public: 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, + if (!m_wallet.CreateTransaction(*locked_chain, recipients, pending->m_tx, pending->m_key, fee, change_pos, fail_reason, coin_control, sign)) { return {}; } @@ -253,7 +259,7 @@ public: { auto locked_chain = m_wallet.chain().lock(); LOCK(m_wallet.cs_wallet); - return m_wallet.AbandonTransaction(txid); + return m_wallet.AbandonTransaction(*locked_chain, txid); } bool transactionCanBeBumped(const uint256& txid) override { @@ -295,7 +301,7 @@ public: LOCK(m_wallet.cs_wallet); auto mi = m_wallet.mapWallet.find(txid); if (mi != m_wallet.mapWallet.end()) { - return MakeWalletTx(m_wallet, mi->second); + return MakeWalletTx(*locked_chain, m_wallet, mi->second); } return {}; } @@ -306,7 +312,7 @@ public: std::vector result; result.reserve(m_wallet.mapWallet.size()); for (const auto& entry : m_wallet.mapWallet) { - result.emplace_back(MakeWalletTx(m_wallet, entry.second)); + result.emplace_back(MakeWalletTx(*locked_chain, m_wallet, entry.second)); } return result; } @@ -327,7 +333,7 @@ public: return false; } num_blocks = ::chainActive.Height(); - tx_status = MakeWalletTxStatus(mi->second); + tx_status = MakeWalletTxStatus(*locked_chain, mi->second); return true; } WalletTx getWalletTxDetails(const uint256& txid, @@ -343,8 +349,8 @@ public: num_blocks = ::chainActive.Height(); in_mempool = mi->second.InMempool(); order_form = mi->second.vOrderForm; - tx_status = MakeWalletTxStatus(mi->second); - return MakeWalletTx(m_wallet, mi->second); + tx_status = MakeWalletTxStatus(*locked_chain, mi->second); + return MakeWalletTx(*locked_chain, m_wallet, mi->second); } return {}; } @@ -408,11 +414,11 @@ public: auto locked_chain = m_wallet.chain().lock(); LOCK(m_wallet.cs_wallet); CoinsList result; - for (const auto& entry : m_wallet.ListCoins()) { + for (const auto& entry : m_wallet.ListCoins(*locked_chain)) { auto& group = result[entry.first]; for (const auto& coin : entry.second) { - group.emplace_back( - COutPoint(coin.tx->GetHash(), coin.i), MakeWalletTxOut(m_wallet, *coin.tx, coin.i, coin.nDepth)); + group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i), + MakeWalletTxOut(*locked_chain, m_wallet, *coin.tx, coin.i, coin.nDepth)); } } return result; @@ -427,9 +433,9 @@ public: result.emplace_back(); auto it = m_wallet.mapWallet.find(output.hash); if (it != m_wallet.mapWallet.end()) { - int depth = it->second.GetDepthInMainChain(); + int depth = it->second.GetDepthInMainChain(*locked_chain); if (depth >= 0) { - result.back() = MakeWalletTxOut(m_wallet, it->second, output.n, depth); + result.back() = MakeWalletTxOut(*locked_chain, m_wallet, it->second, output.n, depth); } } } diff --git a/src/threadsafety.h b/src/threadsafety.h index 47e6b2ea381..33acddc65cf 100644 --- a/src/threadsafety.h +++ b/src/threadsafety.h @@ -54,4 +54,15 @@ #define ASSERT_EXCLUSIVE_LOCK(...) #endif // __GNUC__ +// Utility class for indicating to compiler thread analysis that a mutex is +// locked (when it couldn't be determined otherwise). +struct SCOPED_LOCKABLE LockAnnotation +{ + template + explicit LockAnnotation(Mutex& mutex) EXCLUSIVE_LOCK_FUNCTION(mutex) + { + } + ~LockAnnotation() UNLOCK_FUNCTION() {} +}; + #endif // BITCOIN_THREADSAFETY_H diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 1fb7f133136..7a71aea7155 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -19,7 +19,7 @@ //! Check whether transaction has descendant in wallet or mempool, or has been //! mined, or conflicts with a mined transaction. Return a feebumper::Result. -static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWalletTx& wtx, std::vector& errors) EXCLUSIVE_LOCKS_REQUIRED(cs_main, wallet->cs_wallet) +static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chain, const CWallet* wallet, const CWalletTx& wtx, std::vector& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) { if (wallet->HasWalletSpend(wtx.GetHash())) { errors.push_back("Transaction has descendants in the wallet"); @@ -35,7 +35,7 @@ static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWallet } } - if (wtx.GetDepthInMainChain() != 0) { + if (wtx.GetDepthInMainChain(locked_chain) != 0) { errors.push_back("Transaction has been mined, or is conflicted with a mined transaction"); return feebumper::Result::WALLET_ERROR; } @@ -71,7 +71,7 @@ bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid) if (wtx == nullptr) return false; std::vector errors_dummy; - feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy); + feebumper::Result res = PreconditionChecks(*locked_chain, wallet, *wtx, errors_dummy); return res == feebumper::Result::OK; } @@ -88,7 +88,7 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin } const CWalletTx& wtx = it->second; - Result result = PreconditionChecks(wallet, wtx, errors); + Result result = PreconditionChecks(*locked_chain, wallet, wtx, errors); if (result != Result::OK) { return result; } @@ -235,7 +235,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti CWalletTx& oldWtx = it->second; // make sure the transaction still has no descendants and hasn't been mined in the meantime - Result result = PreconditionChecks(wallet, oldWtx, errors); + Result result = PreconditionChecks(*locked_chain, wallet, oldWtx, errors); if (result != Result::OK) { return result; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 05e14fd64af..e1e4fc51fe2 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -732,7 +732,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) std::map mapKeyBirth; const std::map& mapKeyPool = pwallet->GetAllReserveKeys(); - pwallet->GetKeyBirthTimes(mapKeyBirth); + pwallet->GetKeyBirthTimes(*locked_chain, mapKeyBirth); std::set scripts = pwallet->GetCScripts(); // TODO: include scripts in GetKeyBirthTimes() output instead of separate diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8db7a824762..5a89448e023 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -91,9 +91,9 @@ void EnsureWalletIsUnlocked(CWallet * const pwallet) } } -static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniValue& entry) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx, UniValue& entry) { - int confirms = wtx.GetDepthInMainChain(); + int confirms = wtx.GetDepthInMainChain(locked_chain); entry.pushKV("confirmations", confirms); if (wtx.IsCoinBase()) entry.pushKV("generated", true); @@ -103,7 +103,7 @@ static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniVa entry.pushKV("blockindex", wtx.nIndex); entry.pushKV("blocktime", LookupBlockIndex(wtx.hashBlock)->GetBlockTime()); } else { - entry.pushKV("trusted", wtx.IsTrusted()); + entry.pushKV("trusted", wtx.IsTrusted(locked_chain)); } uint256 hash = wtx.GetHash(); entry.pushKV("txid", hash.GetHex()); @@ -292,7 +292,7 @@ static UniValue setlabel(const JSONRPCRequest& request) } -static CTransactionRef SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue) +static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue) { CAmount curBalance = pwallet->GetBalance(); @@ -319,7 +319,7 @@ static CTransactionRef SendMoney(CWallet * const pwallet, const CTxDestination & CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; vecSend.push_back(recipient); CTransactionRef tx; - if (!pwallet->CreateTransaction(vecSend, tx, reservekey, nFeeRequired, nChangePosRet, strError, coin_control)) { + if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, reservekey, nFeeRequired, nChangePosRet, strError, coin_control)) { if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired)); throw JSONRPCError(RPC_WALLET_ERROR, strError); @@ -418,7 +418,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); - CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue)); + CTransactionRef tx = SendMoney(*locked_chain, pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue)); return tx->GetHash().GetHex(); } @@ -462,7 +462,7 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request) LOCK(pwallet->cs_wallet); UniValue jsonGroupings(UniValue::VARR); - std::map balances = pwallet->GetAddressBalances(); + std::map balances = pwallet->GetAddressBalances(*locked_chain); for (const std::set& grouping : pwallet->GetAddressGroupings()) { UniValue jsonGrouping(UniValue::VARR); for (const CTxDestination& address : grouping) @@ -579,6 +579,7 @@ 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); @@ -606,7 +607,7 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request) for (const CTxOut& txout : wtx.tx->vout) if (txout.scriptPubKey == scriptPubKey) - if (wtx.GetDepthInMainChain() >= nMinDepth) + if (wtx.GetDepthInMainChain(*locked_chain) >= nMinDepth) nAmount += txout.nValue; } @@ -647,6 +648,7 @@ 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); @@ -670,7 +672,7 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request) { CTxDestination address; if (ExtractDestination(txout.scriptPubKey, address) && IsMine(*pwallet, address) && setAddress.count(address)) { - if (wtx.GetDepthInMainChain() >= nMinDepth) + if (wtx.GetDepthInMainChain(*locked_chain) >= nMinDepth) nAmount += txout.nValue; } } @@ -902,7 +904,7 @@ static UniValue sendmany(const JSONRPCRequest& request) int nChangePosRet = -1; std::string strFailReason; CTransactionRef tx; - bool fCreated = pwallet->CreateTransaction(vecSend, tx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control); + bool fCreated = pwallet->CreateTransaction(*locked_chain, vecSend, tx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control); if (!fCreated) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; @@ -1007,8 +1009,10 @@ struct tallyitem } }; -static UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pwallet->cs_wallet) +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()) @@ -1042,7 +1046,7 @@ static UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bo if (wtx.IsCoinBase() || !CheckFinalTx(*wtx.tx)) continue; - int nDepth = wtx.GetDepthInMainChain(); + int nDepth = wtx.GetDepthInMainChain(locked_chain); if (nDepth < nMinDepth) continue; @@ -1199,7 +1203,7 @@ static UniValue listreceivedbyaddress(const JSONRPCRequest& request) auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); - return ListReceived(pwallet, request.params, false); + return ListReceived(*locked_chain, pwallet, request.params, false); } static UniValue listreceivedbylabel(const JSONRPCRequest& request) @@ -1244,7 +1248,7 @@ static UniValue listreceivedbylabel(const JSONRPCRequest& request) auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); - return ListReceived(pwallet, request.params, true); + return ListReceived(*locked_chain, pwallet, request.params, true); } static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) @@ -1264,7 +1268,7 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) * @param ret The UniValue into which the result is stored. * @param filter The "is mine" filter bool. */ -static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) { CAmount nFee; std::list listReceived; @@ -1292,14 +1296,14 @@ static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, int n entry.pushKV("vout", s.vout); entry.pushKV("fee", ValueFromAmount(-nFee)); if (fLong) - WalletTxToJSON(pwallet->chain(), wtx, entry); + WalletTxToJSON(pwallet->chain(), locked_chain, wtx, entry); entry.pushKV("abandoned", wtx.isAbandoned()); ret.push_back(entry); } } // Received - if (listReceived.size() > 0 && wtx.GetDepthInMainChain() >= nMinDepth) + if (listReceived.size() > 0 && wtx.GetDepthInMainChain(locked_chain) >= nMinDepth) { for (const COutputEntry& r : listReceived) { @@ -1314,9 +1318,9 @@ static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, int n MaybePushAddress(entry, r.destination); if (wtx.IsCoinBase()) { - if (wtx.GetDepthInMainChain() < 1) + if (wtx.GetDepthInMainChain(locked_chain) < 1) entry.pushKV("category", "orphan"); - else if (wtx.IsImmatureCoinBase()) + else if (wtx.IsImmatureCoinBase(locked_chain)) entry.pushKV("category", "immature"); else entry.pushKV("category", "generate"); @@ -1331,7 +1335,7 @@ static void ListTransactions(CWallet* const pwallet, const CWalletTx& wtx, int n } entry.pushKV("vout", r.vout); if (fLong) - WalletTxToJSON(pwallet->chain(), wtx, entry); + WalletTxToJSON(pwallet->chain(), locked_chain, wtx, entry); ret.push_back(entry); } } @@ -1427,7 +1431,7 @@ UniValue listtransactions(const JSONRPCRequest& request) for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { CWalletTx *const pwtx = (*it).second; - ListTransactions(pwallet, *pwtx, 0, true, ret, filter); + ListTransactions(*locked_chain, pwallet, *pwtx, 0, true, ret, filter); if ((int)ret.size() >= (nCount+nFrom)) break; } } @@ -1563,8 +1567,8 @@ static UniValue listsinceblock(const JSONRPCRequest& request) for (const std::pair& pairWtx : pwallet->mapWallet) { CWalletTx tx = pairWtx.second; - if (depth == -1 || tx.GetDepthInMainChain() < depth) { - ListTransactions(pwallet, tx, 0, true, transactions, filter); + if (depth == -1 || tx.GetDepthInMainChain(*locked_chain) < depth) { + ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter); } } @@ -1581,7 +1585,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) if (it != pwallet->mapWallet.end()) { // We want all transactions regardless of confirmation count to appear here, // even negative confirmation ones, hence the big negative. - ListTransactions(pwallet, it->second, -100000000, true, removed, filter); + ListTransactions(*locked_chain, pwallet, it->second, -100000000, true, removed, filter); } } paltindex = paltindex->pprev; @@ -1672,7 +1676,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) } const CWalletTx& wtx = it->second; - CAmount nCredit = wtx.GetCredit(filter); + CAmount nCredit = wtx.GetCredit(*locked_chain, filter); CAmount nDebit = wtx.GetDebit(filter); CAmount nNet = nCredit - nDebit; CAmount nFee = (wtx.IsFromMe(filter) ? wtx.tx->GetValueOut() - nDebit : 0); @@ -1681,10 +1685,10 @@ static UniValue gettransaction(const JSONRPCRequest& request) if (wtx.IsFromMe(filter)) entry.pushKV("fee", ValueFromAmount(nFee)); - WalletTxToJSON(pwallet->chain(), wtx, entry); + WalletTxToJSON(pwallet->chain(), *locked_chain, wtx, entry); UniValue details(UniValue::VARR); - ListTransactions(pwallet, wtx, 0, false, details, filter); + ListTransactions(*locked_chain, pwallet, wtx, 0, false, details, filter); entry.pushKV("details", details); std::string strHex = EncodeHexTx(*wtx.tx, RPCSerializationFlags()); @@ -1731,7 +1735,7 @@ static UniValue abandontransaction(const JSONRPCRequest& request) if (!pwallet->mapWallet.count(hash)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id"); } - if (!pwallet->AbandonTransaction(hash)) { + if (!pwallet->AbandonTransaction(*locked_chain, hash)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not eligible for abandonment"); } @@ -2160,7 +2164,7 @@ static UniValue lockunspent(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout index out of bounds"); } - if (pwallet->IsSpent(outpt.hash, outpt.n)) { + if (pwallet->IsSpent(*locked_chain, outpt.hash, outpt.n)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected unspent output"); } @@ -2596,7 +2600,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(GetTime(), g_connman.get()); + std::vector txids = pwallet->ResendWalletTransactionsBefore(*locked_chain, GetTime(), g_connman.get()); UniValue result(UniValue::VARR); for (const uint256& txid : txids) { @@ -2729,7 +2733,7 @@ static UniValue listunspent(const JSONRPCRequest& request) { auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); - pwallet->AvailableCoins(vecOutputs, !include_unsafe, nullptr, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount, nMinDepth, nMaxDepth); + pwallet->AvailableCoins(*locked_chain, vecOutputs, !include_unsafe, nullptr, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount, nMinDepth, nMaxDepth); } LOCK(pwallet->cs_wallet); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index df3eaff1863..c6aac8aad56 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -195,13 +195,13 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) // Call GetImmatureCredit() once before adding the key to the wallet to // cache the current immature credit amount, which is 0. - BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 0); + BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(*locked_chain), 0); // Invalidate the cached value, add the key, and make sure a new immature // credit amount is calculated. wtx.MarkDirty(); wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); - BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 50*COIN); + BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(*locked_chain), 50*COIN); } static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime) @@ -302,7 +302,7 @@ public: int changePos = -1; std::string error; CCoinControl dummy; - BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, reservekey, fee, changePos, error, 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)); CMutableTransaction blocktx; @@ -332,7 +332,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) std::map> list; { LOCK2(cs_main, wallet->cs_wallet); - list = wallet->ListCoins(); + list = wallet->ListCoins(*m_locked_chain); } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), coinbaseAddress); @@ -348,7 +348,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */}); { LOCK2(cs_main, wallet->cs_wallet); - list = wallet->ListCoins(); + list = wallet->ListCoins(*m_locked_chain); } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), coinbaseAddress); @@ -358,7 +358,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) { LOCK2(cs_main, wallet->cs_wallet); std::vector available; - wallet->AvailableCoins(available); + wallet->AvailableCoins(*m_locked_chain, available); BOOST_CHECK_EQUAL(available.size(), 2U); } for (const auto& group : list) { @@ -370,14 +370,14 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) { LOCK2(cs_main, wallet->cs_wallet); std::vector available; - wallet->AvailableCoins(available); + wallet->AvailableCoins(*m_locked_chain, available); BOOST_CHECK_EQUAL(available.size(), 0U); } // Confirm ListCoins still returns same result as before, despite coins // being locked. { LOCK2(cs_main, wallet->cs_wallet); - list = wallet->ListCoins(); + list = wallet->ListCoins(*m_locked_chain); } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), coinbaseAddress); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6e657bcdcd0..8ea4c5c4957 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -594,7 +594,7 @@ void CWallet::SyncMetaData(std::pair ran * Outpoint is spent if any non-conflicted transaction * spends it: */ -bool CWallet::IsSpent(const uint256& hash, unsigned int n) const +bool CWallet::IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash, unsigned int n) const { const COutPoint outpoint(hash, n); std::pair range; @@ -605,7 +605,7 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const const uint256& wtxid = it->second; std::map::const_iterator mit = mapWallet.find(wtxid); if (mit != mapWallet.end()) { - int depth = mit->second.GetDepthInMainChain(); + int depth = mit->second.GetDepthInMainChain(locked_chain); if (depth > 0 || (depth == 0 && !mit->second.isAbandoned())) return true; // Spent } @@ -1009,7 +1009,7 @@ bool CWallet::TransactionCanBeAbandoned(const uint256& hashTx) const auto locked_chain = chain().lock(); LOCK(cs_wallet); const CWalletTx* wtx = GetWalletTx(hashTx); - return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() == 0 && !wtx->InMempool(); + return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain(*locked_chain) == 0 && !wtx->InMempool(); } void CWallet::MarkInputsDirty(const CTransactionRef& tx) @@ -1022,7 +1022,7 @@ void CWallet::MarkInputsDirty(const CTransactionRef& tx) } } -bool CWallet::AbandonTransaction(const uint256& hashTx) +bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const uint256& hashTx) { auto locked_chain_recursive = chain().lock(); // Temporary. Removed in upcoming lock cleanup LOCK(cs_wallet); @@ -1036,7 +1036,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) auto it = mapWallet.find(hashTx); assert(it != mapWallet.end()); CWalletTx& origtx = it->second; - if (origtx.GetDepthInMainChain() != 0 || origtx.InMempool()) { + if (origtx.GetDepthInMainChain(locked_chain) != 0 || origtx.InMempool()) { return false; } @@ -1049,7 +1049,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) auto it = mapWallet.find(now); assert(it != mapWallet.end()); CWalletTx& wtx = it->second; - int currentconfirm = wtx.GetDepthInMainChain(); + int currentconfirm = wtx.GetDepthInMainChain(locked_chain); // If the orig tx was not in block, none of its spends can be assert(currentconfirm <= 0); // if (currentconfirm < 0) {Tx and spends are already conflicted, no need to abandon} @@ -1110,7 +1110,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) auto it = mapWallet.find(now); assert(it != mapWallet.end()); CWalletTx& wtx = it->second; - int currentconfirm = wtx.GetDepthInMainChain(); + int currentconfirm = wtx.GetDepthInMainChain(*locked_chain); if (conflictconfirms < currentconfirm) { // Block is 'more conflicted' than current confirm; update. // Mark transaction as conflicted with this block. @@ -1735,7 +1735,7 @@ void CWallet::ReacceptWalletTransactions() CWalletTx& wtx = item.second; assert(wtx.GetHash() == wtxid); - int nDepth = wtx.GetDepthInMainChain(); + int nDepth = wtx.GetDepthInMainChain(*locked_chain); if (!wtx.IsCoinBase() && (nDepth == 0 && !wtx.isAbandoned())) { mapSorted.insert(std::make_pair(wtx.nOrderPos, &wtx)); @@ -1746,18 +1746,18 @@ void CWallet::ReacceptWalletTransactions() for (const std::pair& item : mapSorted) { CWalletTx& wtx = *(item.second); CValidationState state; - wtx.AcceptToMemoryPool(maxTxFee, state); + wtx.AcceptToMemoryPool(*locked_chain, maxTxFee, state); } } -bool CWalletTx::RelayWalletTransaction(CConnman* connman) +bool CWalletTx::RelayWalletTransaction(interfaces::Chain::Lock& locked_chain, CConnman* connman) { assert(pwallet->GetBroadcastTransactions()); - if (!IsCoinBase() && !isAbandoned() && GetDepthInMainChain() == 0) + if (!IsCoinBase() && !isAbandoned() && GetDepthInMainChain(locked_chain) == 0) { CValidationState state; /* GetDepthInMainChain already catches known conflicts. */ - if (InMempool() || AcceptToMemoryPool(maxTxFee, state)) { + if (InMempool() || AcceptToMemoryPool(locked_chain, maxTxFee, state)) { pwallet->WalletLogPrintf("Relaying wtx %s\n", GetHash().ToString()); if (connman) { CInv inv(MSG_TX, GetHash()); @@ -1815,10 +1815,10 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const return debit; } -CAmount CWalletTx::GetCredit(const isminefilter& filter) const +CAmount CWalletTx::GetCredit(interfaces::Chain::Lock& locked_chain, const isminefilter& filter) const { // Must wait until coinbase is safely deep enough in the chain before valuing it - if (IsImmatureCoinBase()) + if (IsImmatureCoinBase(locked_chain)) return 0; CAmount credit = 0; @@ -1848,9 +1848,9 @@ CAmount CWalletTx::GetCredit(const isminefilter& filter) const return credit; } -CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const +CAmount CWalletTx::GetImmatureCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache) const { - if (IsImmatureCoinBase() && IsInMainChain()) { + if (IsImmatureCoinBase(locked_chain) && IsInMainChain(locked_chain)) { if (fUseCache && fImmatureCreditCached) return nImmatureCreditCached; nImmatureCreditCached = pwallet->GetCredit(*tx, ISMINE_SPENDABLE); @@ -1861,13 +1861,13 @@ CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const return 0; } -CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter) const +CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache, const isminefilter& filter) const { if (pwallet == nullptr) return 0; // Must wait until coinbase is safely deep enough in the chain before valuing it - if (IsImmatureCoinBase()) + if (IsImmatureCoinBase(locked_chain)) return 0; CAmount* cache = nullptr; @@ -1889,7 +1889,7 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter uint256 hashTx = GetHash(); for (unsigned int i = 0; i < tx->vout.size(); i++) { - if (!pwallet->IsSpent(hashTx, i)) + if (!pwallet->IsSpent(locked_chain, hashTx, i)) { const CTxOut &txout = tx->vout[i]; nCredit += pwallet->GetCredit(txout, filter); @@ -1906,9 +1906,9 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter return nCredit; } -CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool fUseCache) const +CAmount CWalletTx::GetImmatureWatchOnlyCredit(interfaces::Chain::Lock& locked_chain, const bool fUseCache) const { - if (IsImmatureCoinBase() && IsInMainChain()) { + if (IsImmatureCoinBase(locked_chain) && IsInMainChain(locked_chain)) { if (fUseCache && fImmatureWatchCreditCached) return nImmatureWatchCreditCached; nImmatureWatchCreditCached = pwallet->GetCredit(*tx, ISMINE_WATCH_ONLY); @@ -1933,12 +1933,14 @@ bool CWalletTx::InMempool() const return fInMempool; } -bool CWalletTx::IsTrusted() 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)) return false; - int nDepth = GetDepthInMainChain(); + int nDepth = GetDepthInMainChain(locked_chain); if (nDepth >= 1) return true; if (nDepth < 0) @@ -1973,7 +1975,7 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const return CTransaction(tx1) == CTransaction(tx2); } -std::vector CWallet::ResendWalletTransactionsBefore(int64_t nTime, CConnman* connman) +std::vector CWallet::ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime, CConnman* connman) { std::vector result; @@ -1992,7 +1994,7 @@ std::vector CWallet::ResendWalletTransactionsBefore(int64_t nTime, CCon for (const std::pair& item : mapSorted) { CWalletTx& wtx = *item.second; - if (wtx.RelayWalletTransaction(connman)) + if (wtx.RelayWalletTransaction(locked_chain, connman)) result.push_back(wtx.GetHash()); } return result; @@ -2017,7 +2019,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); + std::vector relayed = ResendWalletTransactionsBefore(*locked_chain, nBestBlockTime-5*60, connman); if (!relayed.empty()) WalletLogPrintf("%s: rebroadcast %u unconfirmed transactions\n", __func__, relayed.size()); } @@ -2042,8 +2044,8 @@ CAmount CWallet::GetBalance(const isminefilter& filter, const int min_depth) con for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; - if (pcoin->IsTrusted() && pcoin->GetDepthInMainChain() >= min_depth) { - nTotal += pcoin->GetAvailableCredit(true, filter); + if (pcoin->IsTrusted(*locked_chain) && pcoin->GetDepthInMainChain(*locked_chain) >= min_depth) { + nTotal += pcoin->GetAvailableCredit(*locked_chain, true, filter); } } } @@ -2060,8 +2062,8 @@ CAmount CWallet::GetUnconfirmedBalance() const for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; - if (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0 && pcoin->InMempool()) - nTotal += pcoin->GetAvailableCredit(); + if (!pcoin->IsTrusted(*locked_chain) && pcoin->GetDepthInMainChain(*locked_chain) == 0 && pcoin->InMempool()) + nTotal += pcoin->GetAvailableCredit(*locked_chain); } } return nTotal; @@ -2076,7 +2078,7 @@ CAmount CWallet::GetImmatureBalance() const for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; - nTotal += pcoin->GetImmatureCredit(); + nTotal += pcoin->GetImmatureCredit(*locked_chain); } } return nTotal; @@ -2091,8 +2093,8 @@ CAmount CWallet::GetUnconfirmedWatchOnlyBalance() const for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; - if (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0 && pcoin->InMempool()) - nTotal += pcoin->GetAvailableCredit(true, ISMINE_WATCH_ONLY); + if (!pcoin->IsTrusted(*locked_chain) && pcoin->GetDepthInMainChain(*locked_chain) == 0 && pcoin->InMempool()) + nTotal += pcoin->GetAvailableCredit(*locked_chain, true, ISMINE_WATCH_ONLY); } } return nTotal; @@ -2107,7 +2109,7 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const for (const auto& entry : mapWallet) { const CWalletTx* pcoin = &entry.second; - nTotal += pcoin->GetImmatureWatchOnlyCredit(); + nTotal += pcoin->GetImmatureWatchOnlyCredit(*locked_chain); } } return nTotal; @@ -2121,14 +2123,15 @@ CAmount CWallet::GetImmatureWatchOnlyBalance() const // trusted. CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth) const { + LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit. auto locked_chain = chain().lock(); LOCK(cs_wallet); CAmount balance = 0; for (const auto& entry : mapWallet) { const CWalletTx& wtx = entry.second; - const int depth = wtx.GetDepthInMainChain(); - if (depth < 0 || !CheckFinalTx(*wtx.tx) || wtx.IsImmatureCoinBase()) { + const int depth = wtx.GetDepthInMainChain(*locked_chain); + if (depth < 0 || !CheckFinalTx(*wtx.tx) || wtx.IsImmatureCoinBase(*locked_chain)) { continue; } @@ -2160,7 +2163,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const CAmount balance = 0; std::vector vCoins; - AvailableCoins(vCoins, true, coinControl); + AvailableCoins(*locked_chain, vCoins, true, coinControl); for (const COutput& out : vCoins) { if (out.fSpendable) { balance += out.tx->tx->vout[out.i].nValue; @@ -2169,7 +2172,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const return balance; } -void CWallet::AvailableCoins(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 +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); @@ -2185,10 +2188,10 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const if (!CheckFinalTx(*pcoin->tx)) continue; - if (pcoin->IsImmatureCoinBase()) + if (pcoin->IsImmatureCoinBase(locked_chain)) continue; - int nDepth = pcoin->GetDepthInMainChain(); + int nDepth = pcoin->GetDepthInMainChain(locked_chain); if (nDepth < 0) continue; @@ -2197,7 +2200,7 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const if (nDepth == 0 && !pcoin->InMempool()) continue; - bool safeTx = pcoin->IsTrusted(); + bool safeTx = pcoin->IsTrusted(locked_chain); // We should not consider coins from transactions that are replacing // other transactions. @@ -2247,7 +2250,7 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const if (IsLockedCoin(entry.first, i)) continue; - if (IsSpent(wtxid, i)) + if (IsSpent(locked_chain, wtxid, i)) continue; isminetype mine = IsMine(pcoin->tx->vout[i]); @@ -2278,7 +2281,7 @@ void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const } } -std::map> CWallet::ListCoins() const +std::map> CWallet::ListCoins(interfaces::Chain::Lock& locked_chain) const { AssertLockHeld(cs_main); AssertLockHeld(cs_wallet); @@ -2286,7 +2289,7 @@ std::map> CWallet::ListCoins() const std::map> result; std::vector availableCoins; - AvailableCoins(availableCoins); + AvailableCoins(locked_chain, availableCoins); for (const COutput& coin : availableCoins) { CTxDestination address; @@ -2301,7 +2304,7 @@ std::map> CWallet::ListCoins() const for (const COutPoint& output : lockedCoins) { auto it = mapWallet.find(output.hash); if (it != mapWallet.end()) { - int depth = it->second.GetDepthInMainChain(); + int depth = it->second.GetDepthInMainChain(locked_chain); if (depth >= 0 && output.n < it->second.tx->vout.size() && IsMine(it->second.tx->vout[output.n]) == ISMINE_SPENDABLE) { CTxDestination address; @@ -2521,7 +2524,7 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC CReserveKey reservekey(this); CTransactionRef tx_new; - if (!CreateTransaction(vecSend, tx_new, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { + if (!CreateTransaction(*locked_chain, vecSend, tx_new, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { return false; } @@ -2580,7 +2583,7 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec return m_default_address_type; } -bool CWallet::CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, +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) { CAmount nValue = 0; @@ -2646,7 +2649,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac LOCK(cs_wallet); { std::vector vAvailableCoins; - AvailableCoins(vAvailableCoins, true, &coin_control); + AvailableCoins(*locked_chain, vAvailableCoins, true, &coin_control); CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy // Create change script that will be used if we need change @@ -3015,11 +3018,11 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve if (fBroadcastTransactions) { // Broadcast - if (!wtx.AcceptToMemoryPool(maxTxFee, state)) { + if (!wtx.AcceptToMemoryPool(*locked_chain, maxTxFee, 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(connman); + wtx.RelayWalletTransaction(*locked_chain, connman); } } } @@ -3413,7 +3416,7 @@ int64_t CWallet::GetOldestKeyPoolTime() return oldestKey; } -std::map CWallet::GetAddressBalances() +std::map CWallet::GetAddressBalances(interfaces::Chain::Lock& locked_chain) { std::map balances; @@ -3423,13 +3426,13 @@ std::map CWallet::GetAddressBalances() { const CWalletTx *pcoin = &walletEntry.second; - if (!pcoin->IsTrusted()) + if (!pcoin->IsTrusted(locked_chain)) continue; - if (pcoin->IsImmatureCoinBase()) + if (pcoin->IsImmatureCoinBase(locked_chain)) continue; - int nDepth = pcoin->GetDepthInMainChain(); + int nDepth = pcoin->GetDepthInMainChain(locked_chain); if (nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? 0 : 1)) continue; @@ -3441,7 +3444,7 @@ std::map CWallet::GetAddressBalances() if(!ExtractDestination(pcoin->tx->vout[i].scriptPubKey, addr)) continue; - CAmount n = IsSpent(walletEntry.first, i) ? 0 : pcoin->tx->vout[i].nValue; + CAmount n = IsSpent(locked_chain, walletEntry.first, i) ? 0 : pcoin->tx->vout[i].nValue; if (!balances.count(addr)) balances[addr] = 0; @@ -3666,7 +3669,7 @@ void CWallet::ListLockedCoins(std::vector& vOutpts) const /** @} */ // end of Actions -void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) const { +void CWallet::GetKeyBirthTimes(interfaces::Chain::Lock& locked_chain, std::map &mapKeyBirth) const { AssertLockHeld(cs_wallet); // mapKeyMetadata mapKeyBirth.clear(); @@ -4119,6 +4122,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // Try to top up keypool. No-op if the wallet is locked. walletInstance->TopUpKeyPool(); + LockAnnotation lock(::cs_main); // Temporary, for FindForkInGlobalIndex below. Removed in upcoming commit. auto locked_chain = chain.lock(); LOCK(walletInstance->cs_wallet); @@ -4255,7 +4259,7 @@ void CMerkleTx::SetMerkleBranch(const CBlockIndex* pindex, int posInBlock) nIndex = posInBlock; } -int CMerkleTx::GetDepthInMainChain() const +int CMerkleTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const { if (hashUnset()) return 0; @@ -4270,23 +4274,25 @@ int CMerkleTx::GetDepthInMainChain() const return ((nIndex == -1) ? (-1) : 1) * (chainActive.Height() - pindex->nHeight + 1); } -int CMerkleTx::GetBlocksToMaturity() const +int CMerkleTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const { if (!IsCoinBase()) return 0; - int chain_depth = GetDepthInMainChain(); + int chain_depth = GetDepthInMainChain(locked_chain); assert(chain_depth >= 0); // coinbase tx should not be conflicted return std::max(0, (COINBASE_MATURITY+1) - chain_depth); } -bool CMerkleTx::IsImmatureCoinBase() const +bool CMerkleTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const { // note GetBlocksToMaturity is 0 for non-coinbase tx - return GetBlocksToMaturity() > 0; + return GetBlocksToMaturity(locked_chain) > 0; } -bool CWalletTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state) +bool CWalletTx::AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, const CAmount& nAbsurdFee, 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 diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2fb2140ac7f..f96798201f1 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -7,6 +7,7 @@ #define BITCOIN_WALLET_WALLET_H #include +#include #include #include #include @@ -33,11 +34,6 @@ #include #include -namespace interfaces { -class Chain; -} // namespace interfaces - - //! Responsible for reading and validating the -wallet arguments and verifying the wallet database. // This function will perform salvage on the wallet if requested, as long as only one wallet is // being loaded (WalletParameterInteraction forbids -salvagewallet, -zapwallettxes or -upgradewallet with multiwallet). @@ -289,22 +285,22 @@ public: * 0 : in memory pool, waiting to be included in a block * >=1 : this many blocks deep in the main chain */ - int GetDepthInMainChain() const EXCLUSIVE_LOCKS_REQUIRED(cs_main); - bool IsInMainChain() const EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return GetDepthInMainChain() > 0; } + int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const; + bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } /** * @return number of blocks to maturity for this transaction: * 0 : is not a coinbase transaction, or is a mature coinbase transaction * >0 : is a coinbase transaction which matures in this many blocks */ - int GetBlocksToMaturity() const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } void setAbandoned() { hashBlock = ABANDON_HASH; } const uint256& GetHash() const { return tx->GetHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } - bool IsImmatureCoinBase() const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; }; //Get the marginal bytes of spending the specified output @@ -483,14 +479,14 @@ public: //! filter decides which addresses will count towards the debit CAmount GetDebit(const isminefilter& filter) const; - CAmount GetCredit(const isminefilter& filter) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); - CAmount GetImmatureCredit(bool fUseCache=true) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + 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 "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid // having to resolve the issue of member access into incomplete type CWallet. - CAmount GetAvailableCredit(bool fUseCache=true, const isminefilter& filter=ISMINE_SPENDABLE) const NO_THREAD_SAFETY_ANALYSIS; - CAmount GetImmatureWatchOnlyCredit(const bool fUseCache=true) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CAmount GetAvailableCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache=true, const isminefilter& filter=ISMINE_SPENDABLE) const NO_THREAD_SAFETY_ANALYSIS; + CAmount GetImmatureWatchOnlyCredit(interfaces::Chain::Lock& locked_chain, const bool fUseCache=true) const; CAmount GetChange() const; // Get the marginal bytes if spending the specified output from this transaction @@ -511,15 +507,15 @@ public: bool IsEquivalentTo(const CWalletTx& tx) const; bool InMempool() const; - bool IsTrusted() const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool IsTrusted(interfaces::Chain::Lock& locked_chain) const; int64_t GetTxTime() const; // RelayWalletTransaction may only be called if fBroadcastTransactions! - bool RelayWalletTransaction(CConnman* connman) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool RelayWalletTransaction(interfaces::Chain::Lock& locked_chain, CConnman* connman); /** Pass this transaction to the mempool. Fails if absolute fee exceeds absurd fee. */ - bool AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + bool AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, const CAmount& nAbsurdFee, CValidationState& state); // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation @@ -798,12 +794,12 @@ public: /** * populate vCoins with vector of available COutputs. */ - void AvailableCoins(std::vector& vCoins, bool fOnlySafe=true, const CCoinControl *coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0, const int nMinDepth = 0, const int nMaxDepth = 9999999) const EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet); + void AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector& vCoins, bool fOnlySafe=true, const CCoinControl *coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0, const int nMinDepth = 0, const int nMaxDepth = 9999999) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Return list of available coins and locked coins grouped by non-change output address. */ - std::map> ListCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet); + std::map> ListCoins(interfaces::Chain::Lock& locked_chain) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Find non-change parent output. @@ -819,7 +815,7 @@ public: bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector groups, std::set& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; - bool IsSpent(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet); + bool IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::vector GroupOutputs(const std::vector& outputs, bool single_coin) const; bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -883,7 +879,7 @@ public: bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); bool EncryptWallet(const SecureString& strWalletPassphrase); - void GetKeyBirthTimes(std::map &mapKeyBirth) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void GetKeyBirthTimes(interfaces::Chain::Lock& locked_chain, std::map &mapKeyBirth) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); unsigned int ComputeTimeSmart(const CWalletTx& wtx) const; /** @@ -905,7 +901,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(int64_t nTime, CConnman* connman) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + std::vector ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime, CConnman* connman); CAmount GetBalance(const isminefilter& filter=ISMINE_SPENDABLE, const int min_depth=0) const; CAmount GetUnconfirmedBalance() const; CAmount GetImmatureBalance() const; @@ -928,7 +924,7 @@ public: * selected by SelectCoins(); Also create the change output, when needed * @note passing nChangePosInOut as -1 will result in setting a random position */ - bool CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, + bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign = true); bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, CReserveKey& reservekey, CConnman* connman, CValidationState& state); @@ -987,7 +983,7 @@ public: const std::map& GetAllReserveKeys() const { return m_pool_key_to_index; } std::set> GetAddressGroupings() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - std::map GetAddressBalances() EXCLUSIVE_LOCKS_REQUIRED(cs_main); + std::map GetAddressBalances(interfaces::Chain::Lock& locked_chain); std::set GetLabelAddresses(const std::string& label) const; @@ -1082,7 +1078,7 @@ public: bool TransactionCanBeAbandoned(const uint256& hashTx) const; /* Mark a transaction (and it in-wallet descendants) as abandoned so its inputs may be respent. */ - bool AbandonTransaction(const uint256& hashTx); + bool AbandonTransaction(interfaces::Chain::Lock& locked_chain, const uint256& hashTx); /** Mark a transaction as replaced by another transaction (e.g., BIP 125). */ bool MarkReplaced(const uint256& originalHash, const uint256& newHash);