From ae223576077448bd4ec250c73f5d8fe5e9a9ac7d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 27 Oct 2016 16:30:17 -0400 Subject: [PATCH] Replace CValidationState param in ProcessNewBlock with BlockChecked --- src/main.cpp | 43 ++++++++++++--------------------------- src/main.h | 10 +++++++-- src/rpc/mining.cpp | 16 +++++---------- src/test/miner_tests.cpp | 4 +--- src/test/test_bitcoin.cpp | 3 +-- 5 files changed, 28 insertions(+), 48 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 7b203cf62..14923163f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3787,7 +3787,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha return true; } -bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool *fNewBlock) +bool ProcessNewBlock(const CChainParams& chainparams, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool *fNewBlock) { { LOCK(cs_main); @@ -3795,14 +3795,18 @@ bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, c // Store to disk CBlockIndex *pindex = NULL; if (fNewBlock) *fNewBlock = false; + CValidationState state; bool ret = AcceptBlock(*pblock, state, chainparams, &pindex, fForceProcessing, dbp, fNewBlock); CheckBlockIndex(chainparams.GetConsensus()); - if (!ret) + if (!ret) { + GetMainSignals().BlockChecked(*pblock, state); return error("%s: AcceptBlock FAILED", __func__); + } } NotifyHeaderTip(); + CValidationState state; // Only used to report errors, not invalidity - ignore it if (!ActivateBestChain(state, chainparams, pblock)) return error("%s: ActivateBestChain failed", __func__); @@ -5912,26 +5916,18 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, fBlockRead = true; // mapBlockSource is only used for sending reject messages and DoS scores, // so the race between here and cs_main in ProcessNewBlock is fine. + // BIP 152 permits peers to relay compact blocks after validating + // the header only; we should not punish peers if the block turns + // out to be invalid. mapBlockSource.emplace(resp.blockhash, std::make_pair(pfrom->GetId(), false)); } } // Don't hold cs_main when we call into ProcessNewBlock if (fBlockRead) { - CValidationState state; 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) - // BIP 152 permits peers to relay compact blocks after validating - // the header only; we should not punish peers if the block turns - // out to be invalid. - ProcessNewBlock(state, chainparams, &block, true, NULL, &fNewBlock); - int nDoS; - if (state.IsInvalid(nDoS)) { - assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes - connman.PushMessage(pfrom, NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(), - state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), block.GetHash()); - LOCK(cs_main); - mapBlockSource.erase(resp.blockhash); - } else if (fNewBlock) + ProcessNewBlock(chainparams, &block, true, NULL, &fNewBlock); + if (fNewBlock) pfrom->nLastBlockTime = GetTime(); } } @@ -6090,7 +6086,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, LogPrint("net", "received block %s peer=%d\n", block.GetHash().ToString(), pfrom->id); - CValidationState state; // 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 @@ -6107,21 +6102,9 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, mapBlockSource.emplace(hash, std::make_pair(pfrom->GetId(), true)); } bool fNewBlock = false; - ProcessNewBlock(state, chainparams, &block, forceProcessing, NULL, &fNewBlock); - int nDoS; - if (state.IsInvalid(nDoS)) { - assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes - connman.PushMessage(pfrom, NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(), - state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), block.GetHash()); - if (nDoS > 0) { - LOCK(cs_main); - Misbehaving(pfrom->GetId(), nDoS); - } - LOCK(cs_main); - mapBlockSource.erase(hash); - } else if (fNewBlock) + ProcessNewBlock(chainparams, &block, forceProcessing, NULL, &fNewBlock); + if (fNewBlock) pfrom->nLastBlockTime = GetTime(); - } diff --git a/src/main.h b/src/main.h index 5e5d083ef..4a589a163 100644 --- a/src/main.h +++ b/src/main.h @@ -215,15 +215,21 @@ static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 550 * 1024 * 1024; * Process an incoming block. This only returns after the best known valid * block is made active. Note that it does not, however, guarantee that the * specific block passed to it has been checked for validity! + * + * If you want to *possibly* get feedback on whether pblock is valid, you must + * install a CValidationInterface (see validationinterface.h) - this will have + * its BlockChecked method called whenever *any* block completes validation. + * + * Note that we guarantee that either the proof-of-work is valid on pblock, or + * (and possibly also) BlockChecked will have been called. * - * @param[out] state This may be set to an Error state if any error occurred processing it, including during validation/connection/etc of otherwise unrelated blocks during reorganization; or it may be set to an Invalid state if pblock is itself invalid (but this is not guaranteed even when the block is checked). If you want to *possibly* get feedback on whether pblock is valid, you must also install a CValidationInterface (see validationinterface.h) - this will have its BlockChecked method called whenever *any* block completes validation. * @param[in] pblock The block we want to process. * @param[in] fForceProcessing Process this block even if unrequested; used for non-network block sources and whitelisted peers. * @param[out] dbp The already known disk position of pblock, or NULL if not yet stored. * @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(CValidationState& state, const CChainParams& chainparams, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool* fNewBlock); +bool ProcessNewBlock(const CChainParams& chainparams, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool* fNewBlock); /** Check whether enough disk space is available for an incoming block */ bool CheckDiskSpace(uint64_t nAdditionalBytes = 0); /** Open a block file (blk?????.dat) */ diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d9fe9530a..ad545bdf0 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -131,8 +131,7 @@ UniValue generateBlocks(boost::shared_ptr coinbaseScript, int nG if (pblock->nNonce == nInnerLoopCount) { continue; } - CValidationState state; - if (!ProcessNewBlock(state, Params(), pblock, true, NULL, NULL)) + if (!ProcessNewBlock(Params(), pblock, true, NULL, NULL)) throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted"); ++nHeight; blockHashes.push_back(pblock->GetHash().GetHex()); @@ -754,10 +753,9 @@ UniValue submitblock(const JSONRPCRequest& request) } } - CValidationState state; submitblock_StateCatcher sc(block.GetHash()); RegisterValidationInterface(&sc); - bool fAccepted = ProcessNewBlock(state, Params(), &block, true, NULL, NULL); + bool fAccepted = ProcessNewBlock(Params(), &block, true, NULL, NULL); UnregisterValidationInterface(&sc); if (fBlockPresent) { @@ -765,13 +763,9 @@ UniValue submitblock(const JSONRPCRequest& request) return "duplicate-inconclusive"; return "duplicate"; } - if (fAccepted) - { - if (!sc.found) - return "inconclusive"; - state = sc.state; - } - return BIP22ValidationResult(state); + if (!sc.found) + return "inconclusive"; + return BIP22ValidationResult(sc.state); } UniValue estimatefee(const JSONRPCRequest& request) diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index e71a4b1eb..1ef70c343 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -222,9 +222,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) txFirst.push_back(new CTransaction(pblock->vtx[0])); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); pblock->nNonce = blockinfo[i].nonce; - CValidationState state; - BOOST_CHECK(ProcessNewBlock(state, chainparams, pblock, true, NULL, NULL)); - BOOST_CHECK(state.IsValid()); + BOOST_CHECK(ProcessNewBlock(chainparams, pblock, true, NULL, NULL)); pblock->hashPrevBlock = pblock->GetHash(); } diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 4440585c0..3f7416f23 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -126,8 +126,7 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector& while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce; - CValidationState state; - ProcessNewBlock(state, chainparams, &block, true, NULL, NULL); + ProcessNewBlock(chainparams, &block, true, NULL, NULL); CBlock result = block; return result;