From 77339e5aec4da99f727b80829f9697357b4cec45 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 6 May 2014 00:54:10 +0200 Subject: [PATCH 1/4] Get rid of the static chainMostWork (optimization) --- src/main.cpp | 56 +++++++++++++++++++++++++--------------------------- src/main.h | 6 +++--- 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 429473d8f..806f1c20d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -40,7 +40,6 @@ CTxMemPool mempool; map mapBlockIndex; CChain chainActive; -CChain chainMostWork; int64_t nTimeBestReceived = 0; int nScriptCheckThreads = 0; bool fImporting = false; @@ -398,6 +397,12 @@ CBlockIndex *CChain::FindFork(const CBlockLocator &locator) const { return Genesis(); } +CBlockIndex *CChain::FindFork(CBlockIndex *pindex) const { + while (pindex && !Contains(pindex)) + pindex = pindex->pprev; + return pindex; +} + CCoinsViewCache *pcoinsTip = NULL; CBlockTreeDB *pblocktree = NULL; @@ -2035,23 +2040,17 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew) { return true; } -// Make chainMostWork correspond to the chain with the most work in it, that isn't +// Return the tip of the chain with the most work in it, that isn't // known to be invalid (it's however far from certain to be valid). -void static FindMostWorkChain() { - CBlockIndex *pindexNew = NULL; - - // In case the current best is invalid, do not consider it. - while (chainMostWork.Tip() && (chainMostWork.Tip()->nStatus & BLOCK_FAILED_MASK)) { - setBlockIndexValid.erase(chainMostWork.Tip()); - chainMostWork.SetTip(chainMostWork.Tip()->pprev); - } - +static CBlockIndex* FindMostWorkChain() { do { + CBlockIndex *pindexNew = NULL; + // Find the best candidate header. { std::set::reverse_iterator it = setBlockIndexValid.rbegin(); if (it == setBlockIndexValid.rend()) - return; + return NULL; pindexNew = *it; } @@ -2075,18 +2074,9 @@ void static FindMostWorkChain() { } pindexTest = pindexTest->pprev; } - if (fInvalidAncestor) - continue; - - break; + if (!fInvalidAncestor) + return pindexNew; } while(true); - - // Check whether it's actually an improvement. - if (chainMostWork.Tip() && !CBlockIndexWorkComparator()(chainMostWork.Tip(), pindexNew)) - return; - - // We have a new best. - chainMostWork.SetTip(pindexNew); } // Try to activate to the most-work chain (thereby connecting it). @@ -2095,26 +2085,34 @@ bool ActivateBestChain(CValidationState &state) { CBlockIndex *pindexOldTip = chainActive.Tip(); bool fComplete = false; while (!fComplete) { - FindMostWorkChain(); + CBlockIndex *pindexMostWork = FindMostWorkChain(); + CBlockIndex *pindexFork = chainActive.FindFork(pindexMostWork); fComplete = true; // Check whether we have something to do. - if (chainMostWork.Tip() == NULL) break; + if (pindexMostWork == NULL) break; // Disconnect active blocks which are no longer in the best chain. - while (chainActive.Tip() && !chainMostWork.Contains(chainActive.Tip())) { + while (chainActive.Tip() && chainActive.Tip() != pindexFork) { if (!DisconnectTip(state)) return false; } + // Build list of new blocks to connect. + std::vector vpindexToConnect; + vpindexToConnect.reserve(pindexMostWork->nHeight - (pindexFork ? pindexFork->nHeight : -1)); + while (pindexMostWork && pindexMostWork != pindexFork) { + vpindexToConnect.push_back(pindexMostWork); + pindexMostWork = pindexMostWork->pprev; + } + // Connect new blocks. - while (!chainActive.Contains(chainMostWork.Tip())) { - CBlockIndex *pindexConnect = chainMostWork[chainActive.Height() + 1]; + BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) { if (!ConnectTip(state, pindexConnect)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (!state.CorruptionPossible()) - InvalidChainFound(chainMostWork.Tip()); + InvalidChainFound(vpindexToConnect.back()); fComplete = false; state = CValidationState(); break; diff --git a/src/main.h b/src/main.h index 23c866037..98155989a 100644 --- a/src/main.h +++ b/src/main.h @@ -1079,14 +1079,14 @@ public: /** Find the last common block between this chain and a locator. */ CBlockIndex *FindFork(const CBlockLocator &locator) const; + + /** Find the last common block between this chain and a block index entry. */ + CBlockIndex *FindFork(CBlockIndex *pindex) const; }; /** The currently-connected chain of blocks. */ extern CChain chainActive; -/** The currently best known chain of headers (some of which may be invalid). */ -extern CChain chainMostWork; - /** Global variable that points to the active CCoinsView (protected by cs_main) */ extern CCoinsViewCache *pcoinsTip; From 4e0eed88acdd41826868c151373068bfad18b84d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 6 May 2014 01:23:13 +0200 Subject: [PATCH 2/4] Allow ActivateBestChain to release its lock on cs_main --- src/main.cpp | 92 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 37 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 806f1c20d..860487dec 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2079,47 +2079,43 @@ static CBlockIndex* FindMostWorkChain() { } while(true); } -// Try to activate to the most-work chain (thereby connecting it). -bool ActivateBestChain(CValidationState &state) { - LOCK(cs_main); +// Try to make some progress towards making pindexMostWork the active block. +static bool ActivateBestChainStep(CValidationState &state, CBlockIndex *pindexMostWork) { + AssertLockHeld(cs_main); CBlockIndex *pindexOldTip = chainActive.Tip(); - bool fComplete = false; - while (!fComplete) { - CBlockIndex *pindexMostWork = FindMostWorkChain(); - CBlockIndex *pindexFork = chainActive.FindFork(pindexMostWork); - fComplete = true; - - // Check whether we have something to do. - if (pindexMostWork == NULL) break; + CBlockIndex *pindexFork = chainActive.FindFork(pindexMostWork); - // Disconnect active blocks which are no longer in the best chain. - while (chainActive.Tip() && chainActive.Tip() != pindexFork) { - if (!DisconnectTip(state)) - return false; - } + // Disconnect active blocks which are no longer in the best chain. + while (chainActive.Tip() && chainActive.Tip() != pindexFork) { + if (!DisconnectTip(state)) + return false; + } - // Build list of new blocks to connect. - std::vector vpindexToConnect; - vpindexToConnect.reserve(pindexMostWork->nHeight - (pindexFork ? pindexFork->nHeight : -1)); - while (pindexMostWork && pindexMostWork != pindexFork) { - vpindexToConnect.push_back(pindexMostWork); - pindexMostWork = pindexMostWork->pprev; - } + // Build list of new blocks to connect. + std::vector vpindexToConnect; + vpindexToConnect.reserve(pindexMostWork->nHeight - (pindexFork ? pindexFork->nHeight : -1)); + while (pindexMostWork && pindexMostWork != pindexFork) { + vpindexToConnect.push_back(pindexMostWork); + pindexMostWork = pindexMostWork->pprev; + } - // Connect new blocks. - BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) { - if (!ConnectTip(state, pindexConnect)) { - if (state.IsInvalid()) { - // The block violates a consensus rule. - if (!state.CorruptionPossible()) - InvalidChainFound(vpindexToConnect.back()); - fComplete = false; - state = CValidationState(); - break; - } else { - // A system error occurred (disk space, database error, ...). - return false; - } + // Connect new blocks. + BOOST_REVERSE_FOREACH(CBlockIndex *pindexConnect, vpindexToConnect) { + if (!ConnectTip(state, pindexConnect)) { + if (state.IsInvalid()) { + // The block violates a consensus rule. + if (!state.CorruptionPossible()) + InvalidChainFound(vpindexToConnect.back()); + state = CValidationState(); + break; + } else { + // A system error occurred (disk space, database error, ...). + return false; + } + } else { + if (!pindexOldTip || chainActive.Tip()->nChainWork > pindexOldTip->nChainWork) { + // We're in a better position than we were. Return temporarily to release the lock. + break; } } } @@ -2136,6 +2132,28 @@ bool ActivateBestChain(CValidationState &state) { return true; } +bool ActivateBestChain(CValidationState &state) { + do { + boost::this_thread::interruption_point(); + + LOCK(cs_main); + + // Check whether we're done (this could be avoided after the first run, + // but that's not worth optimizing. + CBlockIndex *pindexMostWork = FindMostWorkChain(); + if (pindexMostWork == NULL || pindexMostWork == chainActive.Tip()) + return true; + + if (!ActivateBestChainStep(state, pindexMostWork)) + return false; + + // Check whether we're done now. + if (pindexMostWork == chainActive.Tip()) + return true; + } while(true); + + return true; +} CBlockIndex* AddToBlockIndex(CBlockHeader& block) { From 202e01941c087e0b06a7c18ce344a53ce94e1350 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 7 May 2014 16:45:33 +0200 Subject: [PATCH 3/4] Move all post-chaintip-change notifications to ActivateBestChain --- src/main.cpp | 100 +++++++++++++++++++++++++-------------------------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 860487dec..0d79246a1 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1895,6 +1895,11 @@ bool ConnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex, C for (unsigned int i = 0; i < block.vtx.size(); i++) g_signals.SyncTransaction(block.GetTxHash(i), block.vtx[i], &block); + // Watch for changes to the previous coinbase transaction. + static uint256 hashPrevBestCoinBase; + g_signals.UpdatedTransaction(hashPrevBestCoinBase); + hashPrevBestCoinBase = block.GetTxHash(0); + return true; } @@ -2082,6 +2087,7 @@ static CBlockIndex* FindMostWorkChain() { // Try to make some progress towards making pindexMostWork the active block. static bool ActivateBestChainStep(CValidationState &state, CBlockIndex *pindexMostWork) { AssertLockHeld(cs_main); + bool fInvalidFound = false; CBlockIndex *pindexOldTip = chainActive.Tip(); CBlockIndex *pindexFork = chainActive.FindFork(pindexMostWork); @@ -2107,6 +2113,7 @@ static bool ActivateBestChainStep(CValidationState &state, CBlockIndex *pindexMo if (!state.CorruptionPossible()) InvalidChainFound(vpindexToConnect.back()); state = CValidationState(); + fInvalidFound = true; break; } else { // A system error occurred (disk space, database error, ...). @@ -2120,37 +2127,59 @@ static bool ActivateBestChainStep(CValidationState &state, CBlockIndex *pindexMo } } - if (chainActive.Tip() != pindexOldTip) { - std::string strCmd = GetArg("-blocknotify", ""); - if (!IsInitialBlockDownload() && !strCmd.empty()) - { - boost::replace_all(strCmd, "%s", chainActive.Tip()->GetBlockHash().GetHex()); - boost::thread t(runCommand, strCmd); // thread runs free - } - } + // Callbacks/notifications for a new best chain. + if (fInvalidFound) + CheckForkWarningConditionsOnNewFork(vpindexToConnect.back()); + else + CheckForkWarningConditions(); + + if (!pblocktree->Flush()) + return state.Abort(_("Failed to sync block index")); return true; } bool ActivateBestChain(CValidationState &state) { + CBlockIndex *pindexNewTip = NULL; + CBlockIndex *pindexMostWork = NULL; do { boost::this_thread::interruption_point(); - LOCK(cs_main); + bool fInitialDownload; + { + LOCK(cs_main); + pindexMostWork = FindMostWorkChain(); - // Check whether we're done (this could be avoided after the first run, - // but that's not worth optimizing. - CBlockIndex *pindexMostWork = FindMostWorkChain(); - if (pindexMostWork == NULL || pindexMostWork == chainActive.Tip()) - return true; + // Whether we have anything to do at all. + if (pindexMostWork == NULL || pindexMostWork == chainActive.Tip()) + return true; - if (!ActivateBestChainStep(state, pindexMostWork)) - return false; + if (!ActivateBestChainStep(state, pindexMostWork)) + return false; - // Check whether we're done now. - if (pindexMostWork == chainActive.Tip()) - return true; - } while(true); + pindexNewTip = chainActive.Tip(); + fInitialDownload = IsInitialBlockDownload(); + } + // When we reach this point, we switched to a new tip (stored in pindexNewTip). + + // Notifications/callbacks that can run without cs_main + if (!fInitialDownload) { + uint256 hashNewTip = pindexNewTip->GetBlockHash(); + // Relay inventory, but don't relay old inventory during initial block download. + int nBlockEstimate = Checkpoints::GetTotalBlocksEstimate(); + LOCK(cs_vNodes); + BOOST_FOREACH(CNode* pnode, vNodes) + if (chainActive.Height() > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : nBlockEstimate)) + pnode->PushInventory(CInv(MSG_BLOCK, hashNewTip)); + + std::string strCmd = GetArg("-blocknotify", ""); + if (!strCmd.empty()) { + boost::replace_all(strCmd, "%s", hashNewTip.GetHex()); + boost::thread t(runCommand, strCmd); // thread runs free + } + } + uiInterface.NotifyBlocksChanged(); + } while(pindexMostWork != chainActive.Tip()); return true; } @@ -2215,26 +2244,7 @@ bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBl return state.Abort(_("Failed to write block index")); // New best? - if (!ActivateBestChain(state)) - return false; - - LOCK(cs_main); - if (pindexNew == chainActive.Tip()) - { - // Clear fork warning if its no longer applicable - CheckForkWarningConditions(); - // Notify UI to display prev block's coinbase if it was ours - static uint256 hashPrevBestCoinBase; - g_signals.UpdatedTransaction(hashPrevBestCoinBase); - hashPrevBestCoinBase = block.GetTxHash(0); - } else - CheckForkWarningConditionsOnNewFork(pindexNew); - - if (!pblocktree->Flush()) - return state.Abort(_("Failed to sync block index")); - - uiInterface.NotifyBlocksChanged(); - return true; + return ActivateBestChain(state); } @@ -2554,16 +2564,6 @@ bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex, return state.Abort(_("System error: ") + e.what()); } - // Relay inventory, but don't relay old inventory during initial block download - int nBlockEstimate = Checkpoints::GetTotalBlocksEstimate(); - if (chainActive.Tip()->GetBlockHash() == hash) - { - LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodes) - if (chainActive.Height() > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : nBlockEstimate)) - pnode->PushInventory(CInv(MSG_BLOCK, hash)); - } - return true; } From 18e72167ddfeaea95253b62994c6d64b55b35005 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 7 May 2014 17:10:35 +0200 Subject: [PATCH 4/4] Push cs_mains down in ProcessBlock --- src/main.cpp | 28 +++++++++++++++++----------- src/miner.cpp | 24 ++++++++++++------------ src/rpcserver.cpp | 2 +- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 0d79246a1..16d1cba6e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2243,8 +2243,7 @@ bool ReceivedBlockTransactions(const CBlock &block, CValidationState& state, CBl if (!pblocktree->WriteBlockIndex(CDiskBlockIndex(pindexNew))) return state.Abort(_("Failed to write block index")); - // New best? - return ActivateBestChain(state); + return true; } @@ -2520,7 +2519,6 @@ bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex, } int nHeight = pindex->nHeight; - uint256 hash = pindex->GetBlockHash(); // Check that all transactions are finalized BOOST_FOREACH(const CTransaction& tx, block.vtx) @@ -2593,10 +2591,11 @@ void PushGetBlocks(CNode* pnode, CBlockIndex* pindexBegin, uint256 hashEnd) bool ProcessBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBlockPos *dbp) { - AssertLockHeld(cs_main); - // Check for duplicate uint256 hash = pblock->GetHash(); + + { + LOCK(cs_main); if (mapBlockIndex.count(hash)) return state.Invalid(error("ProcessBlock() : already have block %d %s", mapBlockIndex[hash]->nHeight, hash.ToString()), 0, "duplicate"); if (mapOrphanBlocks.count(hash)) @@ -2665,7 +2664,11 @@ bool ProcessBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBl mapOrphanBlocksByPrev.erase(hashPrev); } - LogPrintf("ProcessBlock: ACCEPTED\n"); + } + + if (!ActivateBestChain(state)) + return error("ProcessBlock() : ActivateBestChain failed"); + return true; } @@ -3101,6 +3104,8 @@ bool InitBlockIndex() { CBlockIndex *pindex = AddToBlockIndex(block); if (!ReceivedBlockTransactions(block, state, pindex, blockPos)) return error("LoadBlockIndex() : genesis block not accepted"); + if (!ActivateBestChain(state)) + return error("LoadBlockIndex() : genesis block cannot be activated"); } catch(std::runtime_error &e) { return error("LoadBlockIndex() : failed to initialize block database: %s", e.what()); } @@ -3230,7 +3235,6 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp) // process block if (nBlockPos >= nStartByte) { - LOCK(cs_main); if (dbp) dbp->nPos = nBlockPos; CValidationState state; @@ -3919,10 +3923,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) CInv inv(MSG_BLOCK, block.GetHash()); pfrom->AddInventoryKnown(inv); - LOCK(cs_main); - // Remember who we got this block from. - mapBlockSource[inv.hash] = pfrom->GetId(); - MarkBlockAsReceived(inv.hash, pfrom->GetId()); + { + LOCK(cs_main); + // Remember who we got this block from. + mapBlockSource[inv.hash] = pfrom->GetId(); + MarkBlockAsReceived(inv.hash, pfrom->GetId()); + } CValidationState state; ProcessBlock(state, pfrom, &block); diff --git a/src/miner.cpp b/src/miner.cpp index ddd277a9b..baaa22c8f 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -484,22 +484,22 @@ bool CheckWork(CBlock* pblock, CWallet& wallet, CReserveKey& reservekey) LOCK(cs_main); if (pblock->hashPrevBlock != chainActive.Tip()->GetBlockHash()) return error("BitcoinMiner : generated block is stale"); + } - // Remove key from key pool - reservekey.KeepKey(); - - // Track how many getdata requests this block gets - { - LOCK(wallet.cs_wallet); - wallet.mapRequestCount[pblock->GetHash()] = 0; - } + // Remove key from key pool + reservekey.KeepKey(); - // Process this block the same as if we had received it from another node - CValidationState state; - if (!ProcessBlock(state, NULL, pblock)) - return error("BitcoinMiner : ProcessBlock, block not accepted"); + // Track how many getdata requests this block gets + { + LOCK(wallet.cs_wallet); + wallet.mapRequestCount[pblock->GetHash()] = 0; } + // Process this block the same as if we had received it from another node + CValidationState state; + if (!ProcessBlock(state, NULL, pblock)) + return error("BitcoinMiner : ProcessBlock, block not accepted"); + return true; } diff --git a/src/rpcserver.cpp b/src/rpcserver.cpp index 72a7fe83e..d4ceb7f99 100644 --- a/src/rpcserver.cpp +++ b/src/rpcserver.cpp @@ -254,7 +254,7 @@ static const CRPCCommand vRPCCommands[] = { "getblocktemplate", &getblocktemplate, true, false, false }, { "getmininginfo", &getmininginfo, true, false, false }, { "getnetworkhashps", &getnetworkhashps, true, false, false }, - { "submitblock", &submitblock, false, false, false }, + { "submitblock", &submitblock, false, true, false }, /* Raw transactions */ { "createrawtransaction", &createrawtransaction, false, false, false },