From a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab Mon Sep 17 00:00:00 2001 From: John Newbery Date: Mon, 30 Sep 2019 16:25:04 -0400 Subject: [PATCH] [validation] Fix peer punishment for bad blocks Because the call to MaybePunishNode() in PeerLogicValidation::BlockChecked() only previously happened if the REJECT code was > 0 and < REJECT_INTERNAL, then there are cases were MaybePunishNode() can get called where it wasn't previously: - when AcceptBlockHeader() fails with CACHED_INVALID. - when AcceptBlockHeader() fails with BLOCK_MISSING_PREV. Note that BlockChecked() cannot fail with an 'internal' reject code. The only internal reject code was REJECT_HIGHFEE, which was only set in ATMP. This change restores the behaviour pre-commit 5d08c9c579ba8cc7b684105c6a08263992b08d52 which did punish nodes that sent us CACHED_INVALID and BLOCK_MISSING_PREV blocks. --- src/net_processing.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b6839dcf215..8d5cccc7ff1 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1234,11 +1234,12 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta const uint256 hash(block.GetHash()); std::map>::iterator it = mapBlockSource.find(hash); - if (state.IsInvalid()) { - // 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) { + // If the block failed validation, we know where it came from and we're still connected + // to that peer, maybe punish. + if (state.IsInvalid() && + it != mapBlockSource.end() && + State(it->second.first)) { MaybePunishNode(/*nodeid=*/ it->second.first, state, /*via_compact_block=*/ !it->second.second); - } } // Check that: // 1. The block is valid