From 015865ee9e5dd450ceb6cece489f924aaa1137e3 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Mon, 31 Oct 2016 10:03:49 -0400 Subject: Fix compact block handling to not ban if block is invalid --- src/main.cpp | 47 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 61d0aaf0b9..850380abba 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -178,8 +178,10 @@ namespace { * Sources of received blocks, saved to be able to send them reject * messages or ban them when processing happens afterwards. Protected by * cs_main. + * Set mapBlockSource[hash].second to false if the node should not be + * punished if the block is invalid. */ - map mapBlockSource; + map> mapBlockSource; /** * Filter for transactions that were recently rejected by @@ -1885,13 +1887,13 @@ void static InvalidChainFound(CBlockIndex* pindexNew) void static InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) { int nDoS = 0; if (state.IsInvalid(nDoS)) { - std::map::iterator it = mapBlockSource.find(pindex->GetBlockHash()); - if (it != mapBlockSource.end() && State(it->second)) { + std::map>::iterator it = mapBlockSource.find(pindex->GetBlockHash()); + if (it != mapBlockSource.end() && State(it->second.first)) { assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), pindex->GetBlockHash()}; - State(it->second)->rejects.push_back(reject); - if (nDoS > 0) - Misbehaving(it->second, nDoS); + State(it->second.first)->rejects.push_back(reject); + if (nDoS > 0 && it->second.second) + Misbehaving(it->second.first, nDoS); } } if (!state.CorruptionPossible()) { @@ -3761,7 +3763,7 @@ static bool IsSuperMajority(int minVersion, const CBlockIndex* pstart, unsigned } -bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp) +bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool fMayBanPeerIfInvalid) { { LOCK(cs_main); @@ -3773,7 +3775,7 @@ bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, C bool fNewBlock = false; bool ret = AcceptBlock(*pblock, state, chainparams, &pindex, fRequested, dbp, &fNewBlock); if (pindex && pfrom) { - mapBlockSource[pindex->GetBlockHash()] = pfrom->GetId(); + mapBlockSource[pindex->GetBlockHash()] = std::make_pair(pfrom->GetId(), fMayBanPeerIfInvalid); if (fNewBlock) pfrom->nLastBlockTime = GetTime(); } CheckBlockIndex(chainparams.GetConsensus()); @@ -4717,7 +4719,6 @@ std::string GetWarnings(const std::string& strFor) ////////////////////////////////////////////////////////////////////////////// -// // Messages // @@ -5791,17 +5792,33 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom, chainActive.Tip(), chainparams.GetConsensus()), resp.blockhash)); pfrom->PushMessage(NetMsgType::GETDATA, invs); } else { + // Block is either okay, or possibly we received + // READ_STATUS_CHECKBLOCK_FAILED. + // Note that CheckBlock can only fail for one of a few reasons: + // 1. bad-proof-of-work (impossible here, because we've already + // accepted the header) + // 2. merkleroot doesn't match the transactions given (already + // caught in FillBlock with READ_STATUS_FAILED, so + // impossible here) + // 3. the block is otherwise invalid (eg invalid coinbase, + // block is too big, too many legacy sigops, etc). + // So if CheckBlock failed, #3 is the only possibility. + // Under BIP 152, we don't DoS-ban unless proof of work is + // invalid (we don't require all the stateless checks to have + // been run). This is handled below, so just treat this as + // though the block was successfully read, and rely on the + // handling in ProcessNewBlock to ensure the block index is + // updated, reject messages go out, etc. CValidationState state; - ProcessNewBlock(state, chainparams, pfrom, &block, false, NULL); + // 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, pfrom, &block, false, NULL, false); int nDoS; if (state.IsInvalid(nDoS)) { assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes pfrom->PushMessage(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); - } } } } @@ -5968,7 +5985,7 @@ 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(); - ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL); + ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL, true); int nDoS; if (state.IsInvalid(nDoS)) { assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes -- cgit v1.2.3