diff options
author | Samuel Dobson <dobsonsa68@gmail.com> | 2020-08-31 22:13:19 +1200 |
---|---|---|
committer | Samuel Dobson <dobsonsa68@gmail.com> | 2020-08-31 22:45:27 +1200 |
commit | 7721b318099e0847ead8d4e864ad3daf4078d547 (patch) | |
tree | 1972eee09ccf41923a021e4b7f02e3ba1cccd7be /src/wallet/wallet.cpp | |
parent | 61b8c04d78fb116e1722659ac455ad27856c6604 (diff) | |
parent | 772ea4844c34ad70d02fd0bd6c0945baa8fff85c (diff) | |
download | bitcoin-7721b318099e0847ead8d4e864ad3daf4078d547.tar.xz |
Merge #19773: wallet: Avoid recursive lock in IsTrusted
772ea4844c34ad70d02fd0bd6c0945baa8fff85c wallet: Avoid recursive lock in IsTrusted (João Barbosa)
819f10f6718659eeeec13af2ce831df3a0984090 wallet, refactor: Immutable CWalletTx::pwallet (João Barbosa)
Pull request description:
This change moves `CWalletTx::IsTrusted` to `CWallet` in order to have TSAN. So now `CWallet::IsTrusted` requires `cs_wallet` and the recursive lock no longer happens.
Motivated by https://github.com/bitcoin/bitcoin/pull/19289/files#r473308226.
ACKs for top commit:
meshcollider:
utACK 772ea4844c34ad70d02fd0bd6c0945baa8fff85c
hebasto:
ACK 772ea4844c34ad70d02fd0bd6c0945baa8fff85c, reviewed and tested on Linux Mint 20 (x86_64).
Tree-SHA512: 702ffd928b2f42a8b90de398790649a5fd04e1ac3877558da928e94cdeb19134883f06c3a73a6826c11c912facf199173375a70200737e164ccaea1bec515b2a
Diffstat (limited to 'src/wallet/wallet.cpp')
-rw-r--r-- | src/wallet/wallet.cpp | 31 |
1 files changed, 16 insertions, 15 deletions
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 48e85ee551..199ae80095 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1965,37 +1965,38 @@ bool CWalletTx::InMempool() const bool CWalletTx::IsTrusted() const { - std::set<uint256> s; - return IsTrusted(s); + std::set<uint256> trusted_parents; + LOCK(pwallet->cs_wallet); + return pwallet->IsTrusted(*this, trusted_parents); } -bool CWalletTx::IsTrusted(std::set<uint256>& trusted_parents) const +bool CWallet::IsTrusted(const CWalletTx& wtx, std::set<uint256>& trusted_parents) const { + AssertLockHeld(cs_wallet); // Quick answer in most cases - if (!pwallet->chain().checkFinalTx(*tx)) return false; - int nDepth = GetDepthInMainChain(); + if (!chain().checkFinalTx(*wtx.tx)) return false; + int nDepth = wtx.GetDepthInMainChain(); if (nDepth >= 1) return true; if (nDepth < 0) return false; // using wtx's cached debit - if (!pwallet->m_spend_zero_conf_change || !IsFromMe(ISMINE_ALL)) return false; + if (!m_spend_zero_conf_change || !wtx.IsFromMe(ISMINE_ALL)) return false; // Don't trust unconfirmed transactions from us unless they are in the mempool. - if (!InMempool()) return false; + if (!wtx.InMempool()) return false; // Trusted if all inputs are from us and are in the mempool: - LOCK(pwallet->cs_wallet); - for (const CTxIn& txin : tx->vin) + for (const CTxIn& txin : wtx.tx->vin) { // Transactions not sent by us: not trusted - const CWalletTx* parent = pwallet->GetWalletTx(txin.prevout.hash); + const CWalletTx* parent = GetWalletTx(txin.prevout.hash); if (parent == nullptr) return false; const CTxOut& parentOut = parent->tx->vout[txin.prevout.n]; // Check that this specific input being spent is trusted - if (pwallet->IsMine(parentOut) != ISMINE_SPENDABLE) return false; + if (IsMine(parentOut) != ISMINE_SPENDABLE) return false; // If we've already trusted this parent, continue if (trusted_parents.count(parent->GetHash())) continue; // Recurse to check that the parent is also trusted - if (!parent->IsTrusted(trusted_parents)) return false; + if (!IsTrusted(*parent, trusted_parents)) return false; trusted_parents.insert(parent->GetHash()); } return true; @@ -2081,7 +2082,7 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons for (const auto& entry : mapWallet) { const CWalletTx& wtx = entry.second; - const bool is_trusted{wtx.IsTrusted(trusted_parents)}; + const bool is_trusted{IsTrusted(wtx, trusted_parents)}; const int tx_depth{wtx.GetDepthInMainChain()}; const CAmount tx_credit_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)}; const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)}; @@ -2149,7 +2150,7 @@ void CWallet::AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe, const if (nDepth == 0 && !wtx.InMempool()) continue; - bool safeTx = wtx.IsTrusted(trusted_parents); + bool safeTx = IsTrusted(wtx, trusted_parents); // We should not consider coins from transactions that are replacing // other transactions. @@ -3348,7 +3349,7 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances() const { const CWalletTx& wtx = walletEntry.second; - if (!wtx.IsTrusted(trusted_parents)) + if (!IsTrusted(wtx, trusted_parents)) continue; if (wtx.IsImmatureCoinBase()) |