diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b1283c236c3b2..c37faf49f0cc2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1894,7 +1894,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, BlockTransactions resp; vRecv >> resp; - CBlock block; + std::shared_ptr pblock = std::make_shared(); bool fBlockRead = false; { LOCK(cs_main); @@ -1907,7 +1907,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } PartiallyDownloadedBlock& partialBlock = *it->second.second->partialBlock; - ReadStatus status = partialBlock.FillBlock(block, resp.txn); + ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case of whitelist Misbehaving(pfrom->GetId(), 100); @@ -1950,7 +1950,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, bool fNewBlock = false; // 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(chainparams, &block, true, NULL, &fNewBlock); + ProcessNewBlock(chainparams, pblock, true, NULL, &fNewBlock); if (fNewBlock) pfrom->nLastBlockTime = GetTime(); } @@ -2111,17 +2111,17 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, else if (strCommand == NetMsgType::BLOCK && !fImporting && !fReindex) // Ignore blocks received while importing { - CBlock block; - vRecv >> block; + std::shared_ptr pblock = std::make_shared(); + vRecv >> *pblock; - LogPrint("net", "received block %s peer=%d\n", block.GetHash().ToString(), pfrom->id); + LogPrint("net", "received block %s peer=%d\n", pblock->GetHash().ToString(), pfrom->id); // Process all blocks from whitelisted peers, even if not requested, // unless we're still syncing with the network. // Such an unrequested block may still be processed, subject to the // conditions in AcceptBlock(). bool forceProcessing = pfrom->fWhitelisted && !IsInitialBlockDownload(); - const uint256 hash(block.GetHash()); + const uint256 hash(pblock->GetHash()); { LOCK(cs_main); // Also always process if we requested the block explicitly, as we may @@ -2132,7 +2132,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, mapBlockSource.emplace(hash, std::make_pair(pfrom->GetId(), true)); } bool fNewBlock = false; - ProcessNewBlock(chainparams, &block, forceProcessing, NULL, &fNewBlock); + ProcessNewBlock(chainparams, pblock, forceProcessing, NULL, &fNewBlock); if (fNewBlock) pfrom->nLastBlockTime = GetTime(); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index b9b68fc10f06f..e6d80f06a3c1d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1319,7 +1319,7 @@ UniValue invalidateblock(const JSONRPCRequest& request) } if (state.IsValid()) { - ActivateBestChain(state, Params(), NULL); + ActivateBestChain(state, Params()); } if (!state.IsValid()) { @@ -1357,7 +1357,7 @@ UniValue reconsiderblock(const JSONRPCRequest& request) } CValidationState state; - ActivateBestChain(state, Params(), NULL); + ActivateBestChain(state, Params()); if (!state.IsValid()) { throw JSONRPCError(RPC_DATABASE_ERROR, state.GetRejectReason()); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 61408b3b60222..cb22dec342e9b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -131,7 +131,8 @@ UniValue generateBlocks(boost::shared_ptr coinbaseScript, int nG if (pblock->nNonce == nInnerLoopCount) { continue; } - if (!ProcessNewBlock(Params(), pblock, true, NULL, NULL)) + std::shared_ptr shared_pblock = std::make_shared(*pblock); + if (!ProcessNewBlock(Params(), shared_pblock, true, NULL, NULL)) throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted"); ++nHeight; blockHashes.push_back(pblock->GetHash().GetHex()); @@ -728,7 +729,8 @@ UniValue submitblock(const JSONRPCRequest& request) + HelpExampleRpc("submitblock", "\"mydata\"") ); - CBlock block; + std::shared_ptr blockptr = std::make_shared(); + CBlock& block = *blockptr; if (!DecodeHexBlk(block, request.params[0].get_str())) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Block decode failed"); @@ -758,7 +760,7 @@ UniValue submitblock(const JSONRPCRequest& request) submitblock_StateCatcher sc(block.GetHash()); RegisterValidationInterface(&sc); - bool fAccepted = ProcessNewBlock(Params(), &block, true, NULL, NULL); + bool fAccepted = ProcessNewBlock(Params(), blockptr, true, NULL, NULL); UnregisterValidationInterface(&sc); if (fBlockPresent) { diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index ca703841db63c..bc1bdd8874d9f 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -223,7 +223,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) txFirst.push_back(pblock->vtx[0]); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); pblock->nNonce = blockinfo[i].nonce; - BOOST_CHECK(ProcessNewBlock(chainparams, pblock, true, NULL, NULL)); + std::shared_ptr shared_pblock = std::make_shared(*pblock); + BOOST_CHECK(ProcessNewBlock(chainparams, shared_pblock, true, NULL, NULL)); pblock->hashPrevBlock = pblock->GetHash(); } diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index f979d01a37eab..139389117a593 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -127,7 +127,8 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector& while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce; - ProcessNewBlock(chainparams, &block, true, NULL, NULL); + std::shared_ptr shared_pblock = std::make_shared(block); + ProcessNewBlock(chainparams, shared_pblock, true, NULL, NULL); CBlock result = block; return result; diff --git a/src/validation.cpp b/src/validation.cpp index 7eef14af2b748..a541978c854de 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2152,29 +2152,45 @@ static int64_t nTimeFlush = 0; static int64_t nTimeChainState = 0; static int64_t nTimePostConnect = 0; +/** + * Used to track conflicted transactions removed from mempool and transactions + * applied to the UTXO state as a part of a single ActivateBestChainStep call. + */ +struct ConnectTrace { + std::vector txConflicted; + std::vector > > blocksConnected; +}; + /** * 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. + * + * The block is always added to connectTrace (either after loading from disk or by copying + * pblock) - if that is not intended, care must be taken to remove the last entry in + * blocksConnected in case of failure. */ -bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const CBlock* pblock, std::vector &txConflicted, std::vector> &txChanged) +bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace) { assert(pindexNew->pprev == chainActive.Tip()); // Read block from disk. int64_t nTime1 = GetTimeMicros(); - CBlock block; if (!pblock) { - if (!ReadBlockFromDisk(block, pindexNew, chainparams.GetConsensus())) + std::shared_ptr pblockNew = std::make_shared(); + connectTrace.blocksConnected.emplace_back(pindexNew, pblockNew); + if (!ReadBlockFromDisk(*pblockNew, pindexNew, chainparams.GetConsensus())) return AbortNode(state, "Failed to read block"); - pblock = █ + } else { + connectTrace.blocksConnected.emplace_back(pindexNew, pblock); } + const CBlock& blockConnecting = *connectTrace.blocksConnected.back().second; // Apply the block atomically to the chain state. int64_t nTime2 = GetTimeMicros(); nTimeReadFromDisk += nTime2 - nTime1; int64_t nTime3; LogPrint("bench", " - Load block from disk: %.2fms [%.2fs]\n", (nTime2 - nTime1) * 0.001, nTimeReadFromDisk * 0.000001); { CCoinsViewCache view(pcoinsTip); - bool rv = ConnectBlock(*pblock, state, pindexNew, view, chainparams); - GetMainSignals().BlockChecked(*pblock, state); + bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, chainparams); + GetMainSignals().BlockChecked(blockConnecting, state); if (!rv) { if (state.IsInvalid()) InvalidBlockFound(pindexNew, state); @@ -2192,13 +2208,10 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, 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.; - mempool.removeForBlock(pblock->vtx, pindexNew->nHeight, &txConflicted, !IsInitialBlockDownload()); + mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, &connectTrace.txConflicted, !IsInitialBlockDownload()); // Update chainActive & related variables. UpdateTip(pindexNew, chainparams); - for (unsigned int i=0; i < pblock->vtx.size(); i++) - txChanged.emplace_back(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); LogPrint("bench", "- Connect block: %.2fms [%.2fs]\n", (nTime6 - nTime1) * 0.001, nTimeTotal * 0.000001); @@ -2279,7 +2292,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, std::vector& txConflicted, std::vector>& txChanged) +static bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) { AssertLockHeld(cs_main); const CBlockIndex *pindexOldTip = chainActive.Tip(); @@ -2312,7 +2325,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, txConflicted, txChanged)) { + if (!ConnectTip(state, chainparams, pindexConnect, pindexConnect == pindexMostWork ? pblock : std::shared_ptr(), connectTrace)) { if (state.IsInvalid()) { // The block violates a consensus rule. if (!state.CorruptionPossible()) @@ -2320,6 +2333,8 @@ static bool ActivateBestChainStep(CValidationState& state, const CChainParams& c state = CValidationState(); fInvalidFound = true; fContinue = false; + // If we didn't actually connect the block, don't notify listeners about it + connectTrace.blocksConnected.pop_back(); break; } else { // A system error occurred (disk space, database error, ...). @@ -2377,20 +2392,16 @@ static void NotifyHeaderTip() { * or an activated best chain. pblock is either NULL or a pointer to a block * that is already loaded (to avoid loading it again from disk). */ -bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, const CBlock *pblock) { +bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { CBlockIndex *pindexMostWork = NULL; CBlockIndex *pindexNewTip = NULL; - std::vector> txChanged; - if (pblock) - txChanged.reserve(pblock->vtx.size()); do { - txChanged.clear(); boost::this_thread::interruption_point(); if (ShutdownRequested()) break; const CBlockIndex *pindexFork; - std::vector txConflicted; + ConnectTrace connectTrace; bool fInitialDownload; { LOCK(cs_main); @@ -2404,7 +2415,8 @@ 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, txConflicted, txChanged)) + std::shared_ptr nullBlockPtr; + if (!ActivateBestChainStep(state, chainparams, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) return false; if (fInvalidFound) { @@ -2421,13 +2433,17 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // throw all transactions though the signal-interface // while _not_ holding the cs_main lock - for (const auto& tx : txConflicted) + for (const auto& tx : connectTrace.txConflicted) { GetMainSignals().SyncTransaction(*tx, pindexNewTip, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); } // ... and about transactions that got confirmed: - for (unsigned int i = 0; i < txChanged.size(); i++) - GetMainSignals().SyncTransaction(*std::get<0>(txChanged[i]), std::get<1>(txChanged[i]), std::get<2>(txChanged[i])); + for (const auto& pair : connectTrace.blocksConnected) { + assert(pair.second); + const CBlock& block = *(pair.second); + for (unsigned int i = 0; i < block.vtx.size(); i++) + GetMainSignals().SyncTransaction(*block.vtx[i], pair.first, i); + } // Notify external listeners about the new tip. GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); @@ -3112,7 +3128,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha return true; } -bool ProcessNewBlock(const CChainParams& chainparams, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool *fNewBlock) +bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool *fNewBlock) { { LOCK(cs_main); diff --git a/src/validation.h b/src/validation.h index f242c20ab9fcb..28e81d21c6ce0 100644 --- a/src/validation.h +++ b/src/validation.h @@ -233,7 +233,7 @@ static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 550 * 1024 * 1024; * @param[out] fNewBlock A boolean which is set to indicate if the block was first received via this call * @return True if state.IsValid() */ -bool ProcessNewBlock(const CChainParams& chainparams, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool* fNewBlock); +bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool* fNewBlock); /** * Process incoming block headers. @@ -278,7 +278,7 @@ std::string GetWarnings(const std::string& strFor); /** Retrieve a transaction (from memory pool, or from disk, if possible) */ bool GetTransaction(const uint256 &hash, CTransactionRef &tx, const Consensus::Params& params, uint256 &hashBlock, bool fAllowSlow = false); /** Find the best known block, and make it the tip of the block chain */ -bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, const CBlock* pblock = NULL); +bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock = std::shared_ptr()); CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); /**