From 65f35eb91b4ad11c96703150b202c1e9b9d12266 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 3 Oct 2016 10:06:10 -0400 Subject: [PATCH 1/5] Move FlushStateToDisk call out of ProcessMessages::TX into ATMP --- src/main.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 5e17ec625..83e85f3d2 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -691,6 +691,16 @@ CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& loc CCoinsViewCache *pcoinsTip = NULL; CBlockTreeDB *pblocktree = NULL; +enum FlushStateMode { + FLUSH_STATE_NONE, + FLUSH_STATE_IF_NEEDED, + FLUSH_STATE_PERIODIC, + FLUSH_STATE_ALWAYS +}; + +// See definition for documentation +bool static FlushStateToDisk(CValidationState &state, FlushStateMode mode); + ////////////////////////////////////////////////////////////////////////////// // // mapOrphanTransactions @@ -1581,6 +1591,9 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa BOOST_FOREACH(const uint256& hashTx, vHashTxToUncache) pcoinsTip->Uncache(hashTx); } + // After we've (potentially) uncached entries, ensure our coins cache is still within its size limits + CValidationState stateDummy; + FlushStateToDisk(stateDummy, FLUSH_STATE_PERIODIC); return res; } @@ -2558,13 +2571,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin return true; } -enum FlushStateMode { - FLUSH_STATE_NONE, - FLUSH_STATE_IF_NEEDED, - FLUSH_STATE_PERIODIC, - FLUSH_STATE_ALWAYS -}; - /** * Update the on-disk chain state. * The caches and indexes are flushed depending on the mode we're called with @@ -5684,7 +5690,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, Misbehaving(pfrom->GetId(), nDoS); } } - FlushStateToDisk(state, FLUSH_STATE_PERIODIC); } From fc0c24f67b4323f215f908767b644bf022c7fe9a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 1 Oct 2016 23:44:40 -0400 Subject: [PATCH 2/5] Move MarkBlockAsReceived out of ProcessNewMessage --- src/main.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 83e85f3d2..c6b82bf82 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3702,6 +3702,11 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha // not process unrequested blocks. bool fTooFarAhead = (pindex->nHeight > int(chainActive.Height() + MIN_BLOCKS_TO_KEEP)); + // TODO: Decouple this function from the block download logic by removing fRequested + // This requires some new chain datastructure to efficiently look up if a + // block is in a chain leading to a candidate for best tip, despite not + // being such a candidate itself. + // TODO: deal better with return value and error conditions for duplicate // and unrequested blocks. if (fAlreadyHave) return true; @@ -3750,13 +3755,11 @@ bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, C { { LOCK(cs_main); - bool fRequested = MarkBlockAsReceived(pblock->GetHash()); - fRequested |= fForceProcessing; // Store to disk CBlockIndex *pindex = NULL; bool fNewBlock = false; - bool ret = AcceptBlock(*pblock, state, chainparams, &pindex, fRequested, dbp, &fNewBlock); + bool ret = AcceptBlock(*pblock, state, chainparams, &pindex, fForceProcessing, dbp, &fNewBlock); if (pindex && pfrom) { mapBlockSource[pindex->GetBlockHash()] = pfrom->GetId(); if (fNewBlock) pfrom->nLastBlockTime = GetTime(); @@ -5858,12 +5861,16 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, std::vector invs; invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom, chainActive.Tip(), chainparams.GetConsensus()), resp.blockhash)); pfrom->PushMessage(NetMsgType::GETDATA, invs); - } else + } else { + MarkBlockAsReceived(resp.blockhash); // it is now an empty pointer fBlockRead = true; + } } // Don't hold cs_main when we call into ProcessNewBlock if (fBlockRead) { CValidationState state; - ProcessNewBlock(state, chainparams, pfrom, &block, false, NULL); + // Since we requested this block (it was in mapBlocksInFlight), force it to be processed, + // even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc) + ProcessNewBlock(state, chainparams, pfrom, &block, true, NULL); int nDoS; if (state.IsInvalid(nDoS)) { assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes @@ -6039,6 +6046,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // Such an unrequested block may still be processed, subject to the // conditions in AcceptBlock(). bool forceProcessing = pfrom->fWhitelisted && !IsInitialBlockDownload(); + { + LOCK(cs_main); + // Also always process if we requested the block explicitly, as we may + // need it even though it is not a candidate for a new best tip. + forceProcessing |= MarkBlockAsReceived(block.GetHash()); + } ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL); int nDoS; if (state.IsInvalid(nDoS)) { From d6ea737be19a0001e69e4e854eb1cef21523ea7a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 2 Oct 2016 11:29:35 -0400 Subject: [PATCH 3/5] Remove network state wipe from UnloadBlockIndex. UnloadBlockIndex is only used during init if we end up reindexing to clear our block state so that we can start over. However, at that time no connections have been brought up as CConnman hasn't been started yet, so all of the network processing state logic is empty when its called. Additionally, the initialization of the recentRejects set is moved to InitPeerLogic. --- src/init.cpp | 4 ++++ src/main.cpp | 17 ++++++++--------- src/main.h | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 84b0108ea..0f42d907b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1100,6 +1100,10 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) return false; #endif // ********************************************************* Step 6: network initialization + // Note that we absolutely cannot open any actual connections + // until the very end ("start node") as the UTXO/block state + // is not yet setup and may end up being set up twice if we + // need to reindex later. assert(!g_connman); g_connman = std::unique_ptr(new CConnman(GetRand(std::numeric_limits::max()), GetRand(std::numeric_limits::max()))); diff --git a/src/main.cpp b/src/main.cpp index c6b82bf82..5be97d586 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4272,6 +4272,9 @@ bool RewindBlockIndex(const CChainParams& params) return true; } +// May NOT be used after any connections are up as much +// of the peer-processing logic assumes a consistent +// block index state void UnloadBlockIndex() { LOCK(cs_main); @@ -4282,18 +4285,12 @@ void UnloadBlockIndex() mempool.clear(); mapOrphanTransactions.clear(); mapOrphanTransactionsByPrev.clear(); - nSyncStarted = 0; mapBlocksUnlinked.clear(); vinfoBlockFile.clear(); nLastBlockFile = 0; nBlockSequenceId = 1; - mapBlockSource.clear(); - mapBlocksInFlight.clear(); - nPreferredDownload = 0; setDirtyBlockIndex.clear(); setDirtyFileInfo.clear(); - mapNodeState.clear(); - recentRejects.reset(NULL); versionbitscache.Clear(); for (int b = 0; b < VERSIONBITS_NUM_BITS; b++) { warningcache[b].clear(); @@ -4318,9 +4315,6 @@ bool InitBlockIndex(const CChainParams& chainparams) { LOCK(cs_main); - // Initialize global variables that cannot be constructed at startup. - recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); - // Check whether we're already initialized if (chainActive.Genesis() != NULL) return true; @@ -4709,6 +4703,11 @@ std::string GetWarnings(const std::string& strFor) // blockchain -> download logic notification // +PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn) : connman(connmanIn) { + // Initialize global variables that cannot be constructed at startup. + recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); +} + void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) { const int nNewHeight = pindexNew->nHeight; connman->SetBestHeight(nNewHeight); diff --git a/src/main.h b/src/main.h index 3eab9b89d..3cab1e6af 100644 --- a/src/main.h +++ b/src/main.h @@ -542,7 +542,7 @@ private: CConnman* connman; public: - PeerLogicValidation(CConnman* connmanIn) : connman(connmanIn) {} + PeerLogicValidation(CConnman* connmanIn); virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload); virtual void BlockChecked(const CBlock& block, const CValidationState& state); From d8670fb103f95e6dc6f469d71c9ee4f6ff2407e0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 2 Oct 2016 12:45:24 -0400 Subject: [PATCH 4/5] Move all calls to CheckBlockIndex out of net-processing logic This will result in many more calls to CheckBlockIndex when connecting a list of headers (eg in ::HEADERS messages processing) but its only enabled in debug mode, and that should mostly just be during IBD, so it should be OK. --- src/main.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 5be97d586..b99515a65 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3675,6 +3675,8 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state if (ppindex) *ppindex = pindex; + CheckBlockIndex(chainparams.GetConsensus()); + return true; } @@ -5827,8 +5829,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, return ProcessMessage(pfrom, NetMsgType::HEADERS, vHeadersMsg, nTimeReceived, chainparams, connman); } } - - CheckBlockIndex(chainparams.GetConsensus()); } else if (strCommand == NetMsgType::BLOCKTXN && !fImporting && !fReindex) // Ignore blocks received while importing @@ -6025,8 +6025,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } } } - - CheckBlockIndex(chainparams.GetConsensus()); } NotifyHeaderTip(); From f5b960be4e9a9ab669e1436194fa904ccba58900 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 6 Oct 2016 14:11:45 -0400 Subject: [PATCH 5/5] Move nTimeBestReceived updating into net processing code --- src/main.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index b99515a65..8232b0f74 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -63,7 +63,7 @@ CCriticalSection cs_main; BlockMap mapBlockIndex; CChain chainActive; CBlockIndex *pindexBestHeader = NULL; -int64_t nTimeBestReceived = 0; +int64_t nTimeBestReceived = 0; // Used only to inform the wallet of when we last received a block CWaitableCriticalSection csBestBlock; CConditionVariable cvBlockChange; int nScriptCheckThreads = 0; @@ -2690,7 +2690,6 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { chainActive.SetTip(pindexNew); // New best block - nTimeBestReceived = GetTime(); mempool.AddTransactionsUpdated(1); cvBlockChange.notify_all(); @@ -4736,6 +4735,8 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB } }); } + + nTimeBestReceived = GetTime(); } void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationState& state) {