From b3b3c2a5623d5c942d2b3565cc2d833c65105555 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Tue, 26 Apr 2016 13:55:13 +0200 Subject: [PATCH] Reduce cs_main locks during ConnectTip/SyncWithWallets --- src/main.cpp | 40 +++++++++++++++----------- src/validationinterface.cpp | 4 +-- src/validationinterface.h | 6 ++-- src/wallet/wallet.cpp | 43 +++++++++++----------------- src/wallet/wallet.h | 6 ++-- src/zmq/zmqnotificationinterface.cpp | 2 +- src/zmq/zmqnotificationinterface.h | 2 +- 7 files changed, 49 insertions(+), 54 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 3a07190a6..db457f6f5 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1541,7 +1541,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C } } - SyncWithWallets(tx, NULL, NULL); + SyncWithWallets(tx, NULL); return true; } @@ -2770,7 +2770,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: BOOST_FOREACH(const CTransaction &tx, block.vtx) { - SyncWithWallets(tx, pindexDelete->pprev, NULL); + SyncWithWallets(tx, pindexDelete->pprev); } return true; } @@ -2785,7 +2785,7 @@ static int64_t nTimePostConnect = 0; * Connect a new block to chainActive. pblock is either NULL or a pointer to a CBlock * corresponding to pindexNew, to bypass loading it again from disk. */ -bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const CBlock* pblock) +bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const CBlock* pblock, std::list &txConflicted, std::vector > &txChanged) { assert(pindexNew->pprev == chainActive.Tip()); // Read block from disk. @@ -2821,20 +2821,13 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, return false; int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4; LogPrint("bench", " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); - // Remove conflicting transactions from the mempool. - list txConflicted; + // Remove conflicting transactions from the mempool.; mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, txConflicted, !IsInitialBlockDownload()); // Update chainActive & related variables. UpdateTip(pindexNew, chainparams); - // Tell wallet about transactions that went from mempool - // to conflicted: - BOOST_FOREACH(const CTransaction &tx, txConflicted) { - SyncWithWallets(tx, pindexNew, NULL); - } - // ... and about transactions that got confirmed: - BOOST_FOREACH(const CTransaction &tx, pblock->vtx) { - SyncWithWallets(tx, pindexNew, pblock); - } + + for(unsigned int i=0; i < pblock->vtx.size(); i++) + txChanged.push_back(std::make_tuple(pblock->vtx[i], pindexNew, i)); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; LogPrint("bench", " - Connect postprocess: %.2fms [%.2fs]\n", (nTime6 - nTime5) * 0.001, nTimePostConnect * 0.000001); @@ -2916,7 +2909,7 @@ static void PruneBlockIndexCandidates() { * Try to make some progress towards making pindexMostWork the active block. * pblock is either NULL or a pointer to a CBlock corresponding to pindexMostWork. */ -static bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const CBlock* pblock, bool& fInvalidFound) +static bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const CBlock* pblock, bool& fInvalidFound, std::list& txConflicted, std::vector >& txChanged) { AssertLockHeld(cs_main); const CBlockIndex *pindexOldTip = chainActive.Tip(); @@ -2949,7 +2942,7 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c // Connect new blocks. BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) { - if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : NULL)) { + if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : NULL, txConflicted, txChanged)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (!state.CorruptionPossible()) @@ -3024,6 +3017,8 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, break; const CBlockIndex *pindexFork; + std::list txConflicted; + std::vector > txChanged; bool fInitialDownload; int nNewHeight; { @@ -3038,7 +3033,7 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, return true; bool fInvalidFound = false; - if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : NULL, fInvalidFound)) + if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : NULL, fInvalidFound, txConflicted, txChanged)) return false; if (fInvalidFound) { @@ -3053,6 +3048,17 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // When we reach this point, we switched to a new tip (stored in pindexNewTip). // Notifications/callbacks that can run without cs_main + + // throw all transactions though the signal-interface + // while _not_ holding the cs_main lock + BOOST_FOREACH(const CTransaction &tx, txConflicted) + { + SyncWithWallets(tx, pindexNewTip); + } + // ... and about transactions that got confirmed: + for(unsigned int i = 0; i < txChanged.size(); i++) + SyncWithWallets(std::get<0>(txChanged[i]), std::get<1>(txChanged[i]), std::get<2>(txChanged[i])); + // Always notify the UI if a new block tip was connected if (pindexFork != pindexNewTip) { uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 8da0c7285..cf1d6ca08 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -48,6 +48,6 @@ void UnregisterAllValidationInterfaces() { g_signals.UpdatedBlockTip.disconnect_all_slots(); } -void SyncWithWallets(const CTransaction &tx, const CBlockIndex *pindex, const CBlock *pblock) { - g_signals.SyncTransaction(tx, pindex, pblock); +void SyncWithWallets(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) { + g_signals.SyncTransaction(tx, pindex, posInBlock); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 01b8e4765..094b1cfe2 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -28,12 +28,12 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn); /** Unregister all wallets from core */ void UnregisterAllValidationInterfaces(); /** Push an updated transaction to all registered wallets */ -void SyncWithWallets(const CTransaction& tx, const CBlockIndex *pindex, const CBlock* pblock = NULL); +void SyncWithWallets(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock = -1); class CValidationInterface { protected: virtual void UpdatedBlockTip(const CBlockIndex *pindex) {} - virtual void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, const CBlock *pblock) {} + virtual void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) {} virtual void SetBestChain(const CBlockLocator &locator) {} virtual void UpdatedTransaction(const uint256 &hash) {} virtual void Inventory(const uint256 &hash) {} @@ -50,7 +50,7 @@ struct CMainSignals { /** Notifies listeners of updated block chain tip */ boost::signals2::signal UpdatedBlockTip; /** Notifies listeners of updated transaction data (transaction, and optionally the block it is found in. */ - boost::signals2::signal SyncTransaction; + boost::signals2::signal SyncTransaction; /** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */ boost::signals2::signal UpdatedTransaction; /** Notifies listeners of a new active block chain. */ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ee9254050..888aa029a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -886,18 +886,18 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn) * pblock is optional, but should be provided if the transaction is known to be in a block. * If fUpdate is true, existing transactions will be updated. */ -bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate) { { AssertLockHeld(cs_wallet); - if (pblock) { + if (posInBlock != -1) { BOOST_FOREACH(const CTxIn& txin, tx.vin) { std::pair range = mapTxSpends.equal_range(txin.prevout); while (range.first != range.second) { if (range.first->second != tx.GetHash()) { - LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), pblock->GetHash().ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); - MarkConflicted(pblock->GetHash(), range.first->second); + LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), pIndex->GetBlockHash().ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); + MarkConflicted(pIndex->GetBlockHash(), range.first->second); } range.first++; } @@ -911,8 +911,8 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl CWalletTx wtx(this,tx); // Get merkle branch if transaction was found in a block - if (pblock) - wtx.SetMerkleBranch(*pblock); + if (posInBlock != -1) + wtx.SetMerkleBranch(pIndex, posInBlock); return AddToWallet(wtx, false); } @@ -1037,11 +1037,11 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) } } -void CWallet::SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, const CBlock* pblock) +void CWallet::SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock) { LOCK2(cs_main, cs_wallet); - if (!AddToWalletIfInvolvingMe(tx, pblock, true)) + if (!AddToWalletIfInvolvingMe(tx, pindex, posInBlock, true)) return; // Not one of ours // If a transaction changes 'conflicted' state, that changes the balance @@ -1399,9 +1399,10 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) CBlock block; ReadBlockFromDisk(block, pindex, Params().GetConsensus()); - BOOST_FOREACH(CTransaction& tx, block.vtx) + int posInBlock; + for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++) { - if (AddToWalletIfInvolvingMe(tx, &block, fUpdate)) + if (AddToWalletIfInvolvingMe(block.vtx[posInBlock], pindex, posInBlock, fUpdate)) ret++; } pindex = chainActive.Next(pindex); @@ -3526,31 +3527,19 @@ CWalletKey::CWalletKey(int64_t nExpires) nTimeExpires = nExpires; } -int CMerkleTx::SetMerkleBranch(const CBlock& block) +int CMerkleTx::SetMerkleBranch(const CBlockIndex* pindex, int posInBlock) { AssertLockHeld(cs_main); CBlock blockTmp; // Update the tx's hashBlock - hashBlock = block.GetHash(); + hashBlock = pindex->GetBlockHash(); - // Locate the transaction - for (nIndex = 0; nIndex < (int)block.vtx.size(); nIndex++) - if (block.vtx[nIndex] == *(CTransaction*)this) - break; - if (nIndex == (int)block.vtx.size()) - { - nIndex = -1; - LogPrintf("ERROR: SetMerkleBranch(): couldn't find tx in block\n"); - return 0; - } + // set the position of the transaction in the block + nIndex = posInBlock; // Is the tx in a block that's in the main chain - BlockMap::iterator mi = mapBlockIndex.find(hashBlock); - if (mi == mapBlockIndex.end()) - return 0; - const CBlockIndex* pindex = (*mi).second; - if (!pindex || !chainActive.Contains(pindex)) + if (!chainActive.Contains(pindex)) return 0; return chainActive.Height() - pindex->nHeight + 1; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 50c94ccfb..952acd153 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -200,7 +200,7 @@ public: READWRITE(nIndex); } - int SetMerkleBranch(const CBlock& block); + int SetMerkleBranch(const CBlockIndex* pIndex, int posInBlock); /** * Return depth of transaction in blockchain: @@ -731,8 +731,8 @@ public: void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); bool LoadToWallet(const CWalletTx& wtxIn); - void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, const CBlock* pblock); - bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate); + void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock); + bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate); int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false); void ReacceptWalletTransactions(); void ResendWalletTransactions(int64_t nBestBlockTime); diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 870553242..376e7dec5 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -141,7 +141,7 @@ void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindex) } } -void CZMQNotificationInterface::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, const CBlock* pblock) +void CZMQNotificationInterface::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int posInBlock) { for (std::list::iterator i = notifiers.begin(); i!=notifiers.end(); ) { diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index 7b52e7775..a85344726 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -24,7 +24,7 @@ protected: void Shutdown(); // CValidationInterface - void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, const CBlock* pblock); + void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock); void UpdatedBlockTip(const CBlockIndex *pindex); private: