From 23530a88a8802b438dc0a40e253aeae79bb42e4d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 10 Apr 2017 14:44:16 +0200 Subject: [PATCH] Merge #10135: [p2p] Send the correct error code in reject messages 5d08c9c Send the correct error code in reject messages (John Newbery) Tree-SHA512: 0cd3ef3ae202584b138cc0bbfba4125635822e0c5a755fb9276a604b39286959ab22dabc3104aa5d7e71358cd69d965de2a333ff04bf3e8ed43cf0296ac01264 --- src/net_processing.cpp | 6 +++--- src/validation.cpp | 10 ++++++---- test/functional/p2p-fullblocktest.py | 4 ++-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1df626b70f..6a32b8455e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -917,8 +917,8 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta int nDoS = 0; if (state.IsInvalid(nDoS)) { - if (it != mapBlockSource.end() && State(it->second.first)) { - assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes + // Don't send reject message with code 0 or an internal reject code. + if (it != mapBlockSource.end() && State(it->second.first) && state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) { CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), hash}; State(it->second.first)->rejects.push_back(reject); if (nDoS > 0 && it->second.second) @@ -2348,7 +2348,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrint("mempoolrej", "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(), pfrom->id, FormatStateMessage(state)); - if (state.GetRejectCode() < REJECT_INTERNAL) // Never send AcceptToMemoryPool's internal codes over P2P + if (state.GetRejectCode() > 0 && state.GetRejectCode() < REJECT_INTERNAL) // Never send AcceptToMemoryPool's internal codes over P2P connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), inv.hash)); if (nDoS > 0) { diff --git a/src/validation.cpp b/src/validation.cpp index f4095e2ec4..de9fe3bb57 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2185,7 +2185,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd LogPrint("bench", " - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs]\n", (unsigned)block.vtx.size(), 0.001 * (nTime3 - nTime2), 0.001 * (nTime3 - nTime2) / block.vtx.size(), nInputs <= 1 ? 0 : 0.001 * (nTime3 - nTime2) / (nInputs-1), nTimeConnect * 0.000001); if (!control.Wait()) - return state.DoS(100, false); + return state.DoS(100, error("%s: CheckQueue failed", __func__), REJECT_INVALID, "block-validation-failed"); int64_t nTime4 = GetTimeMicros(); nTimeVerify += nTime4 - nTime2; LogPrint("bench", " - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs]\n", nInputs - 1, 0.001 * (nTime4 - nTime2), nInputs <= 1 ? 0 : 0.001 * (nTime4 - nTime2) / (nInputs-1), nTimeVerify * 0.000001); @@ -3338,10 +3338,12 @@ static bool CheckIndexAgainstCheckpoint(const CBlockIndex* pindexPrev, CValidati return true; int nHeight = pindexPrev->nHeight+1; - // Don't accept any forks from the main chain prior to last checkpoint + // Don't accept any forks from the main chain prior to last checkpoint. + // GetLastCheckpoint finds the last checkpoint in MapCheckpoints that's in our + // MapBlockIndex. CBlockIndex* pcheckpoint = Checkpoints::GetLastCheckpoint(chainparams.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) - return state.DoS(100, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight)); + return state.DoS(100, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), REJECT_CHECKPOINT, "bad-fork-prior-to-checkpoint"); return true; } @@ -3471,7 +3473,7 @@ static bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state CBlockIndex* pindexPrev = NULL; BlockMap::iterator mi = mapBlockIndex.find(block.hashPrevBlock); if (mi == mapBlockIndex.end()) - return state.DoS(10, error("%s: prev block not found", __func__), 0, "bad-prevblk"); + return state.DoS(10, error("%s: prev block not found", __func__), 0, "prev-blk-not-found"); pindexPrev = (*mi).second; assert(pindexPrev); diff --git a/test/functional/p2p-fullblocktest.py b/test/functional/p2p-fullblocktest.py index 90ec10758e..f060216860 100755 --- a/test/functional/p2p-fullblocktest.py +++ b/test/functional/p2p-fullblocktest.py @@ -404,7 +404,7 @@ class FullBlockTest(ComparisonTestFramework): # Extend the b26 chain to make sure bitcoind isn't accepting b26 b27 = block(27, spend=out[7]) - yield rejected(RejectResult(0, b'bad-prevblk')) + yield rejected(False) # Now try a too-large-coinbase script tip(15) @@ -416,7 +416,7 @@ class FullBlockTest(ComparisonTestFramework): # Extend the b28 chain to make sure bitcoind isn't accepting b28 b29 = block(29, spend=out[7]) - yield rejected(RejectResult(0, b'bad-prevblk')) + yield rejected(False) # b30 has a max-sized coinbase scriptSig. tip(23)