Remove locked_chain from GetDepthInMainChain and its callers

We don't remove yet Chain locks as we need to preserve lock
order with CWallet one until swapping at once to avoid
deadlock failures (spotted by --enable-debug)
This commit is contained in:
Antoine Riard
2019-04-29 09:52:01 -04:00
parent 0ff03871ad
commit b66c429c56
7 changed files with 91 additions and 93 deletions

View File

@@ -452,7 +452,7 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran
* Outpoint is spent if any non-conflicted transaction
* spends it:
*/
bool CWallet::IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash, unsigned int n) const
bool CWallet::IsSpent(const uint256& hash, unsigned int n) const
{
const COutPoint outpoint(hash, n);
std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range;
@@ -463,7 +463,7 @@ bool CWallet::IsSpent(interfaces::Chain::Lock& locked_chain, const uint256& hash
const uint256& wtxid = it->second;
std::map<uint256, CWalletTx>::const_iterator mit = mapWallet.find(wtxid);
if (mit != mapWallet.end()) {
int depth = mit->second.GetDepthInMainChain(locked_chain);
int depth = mit->second.GetDepthInMainChain();
if (depth > 0 || (depth == 0 && !mit->second.isAbandoned()))
return true; // Spent
}
@@ -913,7 +913,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(*locked_chain) == 0 && !wtx->InMempool();
return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() == 0 && !wtx->InMempool();
}
void CWallet::MarkInputsDirty(const CTransactionRef& tx)
@@ -926,9 +926,9 @@ void CWallet::MarkInputsDirty(const CTransactionRef& tx)
}
}
bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const uint256& hashTx)
bool CWallet::AbandonTransaction(const uint256& hashTx)
{
auto locked_chain_recursive = chain().lock(); // Temporary. Removed in upcoming lock cleanup
auto locked_chain = chain().lock(); // Temporary. Removed in upcoming lock cleanup
LOCK(cs_wallet);
WalletBatch batch(*database, "r+");
@@ -940,7 +940,7 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui
auto it = mapWallet.find(hashTx);
assert(it != mapWallet.end());
CWalletTx& origtx = it->second;
if (origtx.GetDepthInMainChain(locked_chain) != 0 || origtx.InMempool()) {
if (origtx.GetDepthInMainChain() != 0 || origtx.InMempool()) {
return false;
}
@@ -953,7 +953,7 @@ bool CWallet::AbandonTransaction(interfaces::Chain::Lock& locked_chain, const ui
auto it = mapWallet.find(now);
assert(it != mapWallet.end());
CWalletTx& wtx = it->second;
int currentconfirm = wtx.GetDepthInMainChain(locked_chain);
int currentconfirm = wtx.GetDepthInMainChain();
// 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}
@@ -1009,7 +1009,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
auto it = mapWallet.find(now);
assert(it != mapWallet.end());
CWalletTx& wtx = it->second;
int currentconfirm = wtx.GetDepthInMainChain(*locked_chain);
int currentconfirm = wtx.GetDepthInMainChain();
if (conflictconfirms < currentconfirm) {
// Block is 'more conflicted' than current confirm; update.
// Mark transaction as conflicted with this block.
@@ -1691,7 +1691,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
return result;
}
void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain)
void CWallet::ReacceptWalletTransactions()
{
// If transactions aren't being broadcasted, don't let them into local mempool either
if (!fBroadcastTransactions)
@@ -1704,7 +1704,7 @@ void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain)
CWalletTx& wtx = item.second;
assert(wtx.GetHash() == wtxid);
int nDepth = wtx.GetDepthInMainChain(locked_chain);
int nDepth = wtx.GetDepthInMainChain();
if (!wtx.IsCoinBase() && (nDepth == 0 && !wtx.isAbandoned())) {
mapSorted.insert(std::make_pair(wtx.nOrderPos, &wtx));
@@ -1715,11 +1715,11 @@ void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain)
for (const std::pair<const int64_t, CWalletTx*>& item : mapSorted) {
CWalletTx& wtx = *(item.second);
std::string unused_err_string;
wtx.SubmitMemoryPoolAndRelay(unused_err_string, false, locked_chain);
wtx.SubmitMemoryPoolAndRelay(unused_err_string, false);
}
}
bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, interfaces::Chain::Lock& locked_chain)
bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay)
{
// Can't relay if wallet is not broadcasting
if (!pwallet->GetBroadcastTransactions()) return false;
@@ -1729,7 +1729,7 @@ bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, in
// cause log spam.
if (IsCoinBase()) return false;
// Don't try to submit conflicted or confirmed transactions.
if (GetDepthInMainChain(locked_chain) != 0) return false;
if (GetDepthInMainChain() != 0) return false;
// Submit transaction to mempool for relay
pwallet->WalletLogPrintf("Submitting wtx %s to mempool for relay\n", GetHash().ToString());
@@ -1783,10 +1783,10 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const
return debit;
}
CAmount CWalletTx::GetCredit(interfaces::Chain::Lock& locked_chain, const isminefilter& filter) const
CAmount CWalletTx::GetCredit(const isminefilter& filter) const
{
// Must wait until coinbase is safely deep enough in the chain before valuing it
if (IsImmatureCoinBase(locked_chain))
if (IsImmatureCoinBase())
return 0;
CAmount credit = 0;
@@ -1800,16 +1800,16 @@ CAmount CWalletTx::GetCredit(interfaces::Chain::Lock& locked_chain, const ismine
return credit;
}
CAmount CWalletTx::GetImmatureCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache) const
CAmount CWalletTx::GetImmatureCredit(bool fUseCache) const
{
if (IsImmatureCoinBase(locked_chain) && IsInMainChain(locked_chain)) {
if (IsImmatureCoinBase() && IsInMainChain()) {
return GetCachableAmount(IMMATURE_CREDIT, ISMINE_SPENDABLE, !fUseCache);
}
return 0;
}
CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, bool fUseCache, const isminefilter& filter) const
CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter) const
{
if (pwallet == nullptr)
return 0;
@@ -1818,7 +1818,7 @@ CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, boo
bool allow_cache = (filter & ISMINE_ALL) && (filter & ISMINE_ALL) != ISMINE_ALL;
// Must wait until coinbase is safely deep enough in the chain before valuing it
if (IsImmatureCoinBase(locked_chain))
if (IsImmatureCoinBase())
return 0;
if (fUseCache && allow_cache && m_amounts[AVAILABLE_CREDIT].m_cached[filter]) {
@@ -1830,7 +1830,7 @@ CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, boo
uint256 hashTx = GetHash();
for (unsigned int i = 0; i < tx->vout.size(); i++)
{
if (!pwallet->IsSpent(locked_chain, hashTx, i) && (allow_used_addresses || !pwallet->IsUsedDestination(hashTx, i))) {
if (!pwallet->IsSpent(hashTx, i) && (allow_used_addresses || !pwallet->IsUsedDestination(hashTx, i))) {
const CTxOut &txout = tx->vout[i];
nCredit += pwallet->GetCredit(txout, filter);
if (!MoneyRange(nCredit))
@@ -1845,9 +1845,9 @@ CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, boo
return nCredit;
}
CAmount CWalletTx::GetImmatureWatchOnlyCredit(interfaces::Chain::Lock& locked_chain, const bool fUseCache) const
CAmount CWalletTx::GetImmatureWatchOnlyCredit(const bool fUseCache) const
{
if (IsImmatureCoinBase(locked_chain) && IsInMainChain(locked_chain)) {
if (IsImmatureCoinBase() && IsInMainChain()) {
return GetCachableAmount(IMMATURE_CREDIT, ISMINE_WATCH_ONLY, !fUseCache);
}
@@ -1878,7 +1878,7 @@ bool CWalletTx::IsTrusted(interfaces::Chain::Lock& locked_chain, std::set<uint25
{
// Quick answer in most cases
if (!locked_chain.checkFinalTx(*tx)) return false;
int nDepth = GetDepthInMainChain(locked_chain);
int nDepth = GetDepthInMainChain();
if (nDepth >= 1) return true;
if (nDepth < 0) return false;
// using wtx's cached debit
@@ -1954,7 +1954,7 @@ void CWallet::ResendWalletTransactions()
// any confirmed or conflicting txs.
if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue;
std::string unused_err_string;
if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true, *locked_chain)) ++submitted_tx_count;
if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true)) ++submitted_tx_count;
}
} // locked_chain and cs_wallet
@@ -1991,9 +1991,9 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons
{
const CWalletTx& wtx = entry.second;
const bool is_trusted{wtx.IsTrusted(*locked_chain, trusted_parents)};
const int tx_depth{wtx.GetDepthInMainChain(*locked_chain)};
const CAmount tx_credit_mine{wtx.GetAvailableCredit(*locked_chain, /* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)};
const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(*locked_chain, /* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)};
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)};
if (is_trusted && tx_depth >= min_depth) {
ret.m_mine_trusted += tx_credit_mine;
ret.m_watchonly_trusted += tx_credit_watchonly;
@@ -2002,8 +2002,8 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons
ret.m_mine_untrusted_pending += tx_credit_mine;
ret.m_watchonly_untrusted_pending += tx_credit_watchonly;
}
ret.m_mine_immature += wtx.GetImmatureCredit(*locked_chain);
ret.m_watchonly_immature += wtx.GetImmatureWatchOnlyCredit(*locked_chain);
ret.m_mine_immature += wtx.GetImmatureCredit();
ret.m_watchonly_immature += wtx.GetImmatureWatchOnlyCredit();
}
}
return ret;
@@ -2047,10 +2047,10 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<
continue;
}
if (wtx.IsImmatureCoinBase(locked_chain))
if (wtx.IsImmatureCoinBase())
continue;
int nDepth = wtx.GetDepthInMainChain(locked_chain);
int nDepth = wtx.GetDepthInMainChain();
if (nDepth < 0)
continue;
@@ -2110,7 +2110,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<
if (IsLockedCoin(entry.first, i))
continue;
if (IsSpent(locked_chain, wtxid, i))
if (IsSpent(wtxid, i))
continue;
isminetype mine = IsMine(wtx.tx->vout[i]);
@@ -2169,7 +2169,7 @@ std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins(interfaces::Ch
for (const COutPoint& output : lockedCoins) {
auto it = mapWallet.find(output.hash);
if (it != mapWallet.end()) {
int depth = it->second.GetDepthInMainChain(locked_chain);
int depth = it->second.GetDepthInMainChain();
if (depth >= 0 && output.n < it->second.tx->vout.size() &&
IsMine(it->second.tx->vout[output.n]) == ISMINE_SPENDABLE) {
CTxDestination address;
@@ -2909,7 +2909,7 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
}
std::string err_string;
if (!wtx.SubmitMemoryPoolAndRelay(err_string, true, *locked_chain)) {
if (!wtx.SubmitMemoryPoolAndRelay(err_string, true)) {
WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string);
// TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
}
@@ -3129,10 +3129,10 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances(interfaces::Chain:
if (!wtx.IsTrusted(locked_chain, trusted_parents))
continue;
if (wtx.IsImmatureCoinBase(locked_chain))
if (wtx.IsImmatureCoinBase())
continue;
int nDepth = wtx.GetDepthInMainChain(locked_chain);
int nDepth = wtx.GetDepthInMainChain();
if (nDepth < (wtx.IsFromMe(ISMINE_ALL) ? 0 : 1))
continue;
@@ -3144,7 +3144,7 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances(interfaces::Chain:
if(!ExtractDestination(wtx.tx->vout[i].scriptPubKey, addr))
continue;
CAmount n = IsSpent(locked_chain, walletEntry.first, i) ? 0 : wtx.tx->vout[i].nValue;
CAmount n = IsSpent(walletEntry.first, i) ? 0 : wtx.tx->vout[i].nValue;
if (!balances.count(addr))
balances[addr] = 0;
@@ -3908,7 +3908,7 @@ void CWallet::postInitProcess()
// Add wallet transactions that aren't already in a block to mempool
// Do this here as mempool requires genesis block to be loaded
ReacceptWalletTransactions(*locked_chain);
ReacceptWalletTransactions();
// Update wallet transactions with current mempool transactions.
chain().requestMempoolTransactions(*this);
@@ -3934,7 +3934,7 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn)
m_pre_split = false;
}
int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const
int CWalletTx::GetDepthInMainChain() const
{
assert(pwallet != nullptr);
AssertLockHeld(pwallet->cs_wallet);
@@ -3943,19 +3943,19 @@ int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const
return (pwallet->GetLastBlockHeight() - m_confirm.block_height + 1) * (isConflicted() ? -1 : 1);
}
int CWalletTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const
int CWalletTx::GetBlocksToMaturity() const
{
if (!IsCoinBase())
return 0;
int chain_depth = GetDepthInMainChain(locked_chain);
int chain_depth = GetDepthInMainChain();
assert(chain_depth >= 0); // coinbase tx should not be conflicted
return std::max(0, (COINBASE_MATURITY+1) - chain_depth);
}
bool CWalletTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const
bool CWalletTx::IsImmatureCoinBase() const
{
// note GetBlocksToMaturity is 0 for non-coinbase tx
return GetBlocksToMaturity(locked_chain) > 0;
return GetBlocksToMaturity() > 0;
}
std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool single_coin) const {