[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.

1 files changed, 5 insertions, 4 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index b6839dcf21..8d5cccc7ff 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<uint256, std::pair<NodeId, bool>>::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