Merge bitcoin/bitcoin#29575: net_processing: make any misbehavior trigger immediate discouragement

6eecba475efd025eb011400af58621ad5823994e net_processing: make MaybePunishNodeFor{Block,Tx} return void (Pieter Wuille)
ae60d485da33f238ed2186799da4e109d4edd3a1 net_processing: remove Misbehavior score and increments (Pieter Wuille)
6457c311977bba3585648e32e3bd5754028aa292 net_processing: make all Misbehaving increments = 100 (Pieter Wuille)
5120ab1478c200b18ee621a6ffa0362f4e991959 net_processing: drop 8 headers threshold for incoming BIP130 (Pieter Wuille)
944c54290d5c081dc433dae7e7941074a3a8b5a7 net_processing: drop Misbehavior for unconnecting headers (Pieter Wuille)
9f66ac7cf1931c4d7c36abbb000b7de306d83a4c net_processing: do not treat non-connecting headers as response (Pieter Wuille)

Pull request description:

  So far, discouragement of peers triggers when their misbehavior score exceeds 100 points. Most types of misbehavior increment the score by 100, triggering immediate discouragement, but some types do not. This PR makes all increments equal to either 100 (meaning any misbehavior will immediately cause disconnection and discouragement) or 0 (making the behavior effectively unconditionally allowed), and then removes the logic for score accumulation.

  This simplifies the code a bit, but also makes protocol expectations clearer: if a peer misbehaves, they get disconnected. There is no good reason why certain types of protocol violations should be permitted 4 times (howmuch=20) or 9 times (howmuch=10), while many others are never allowed. Furthermore, the distinction between these looks arbitrary.

  The specific types of misbehavior that are changed to 100 are:
  * Sending us a `block` which does not connect to our header tree (which necessarily must have been unsollicited). [used to be score 10]
  * Sending us a `headers` with a non-continuous headers sequence. [used to be score 20]
  * Sending us more than 1000 addresses in a single `addr` or `addrv2` message [used to be score 20]
  * Sending us more than 50000 invs in a single `inv` message [used to be score 20]
  * Sending us more than 2000 headers in a single `headers` message [used to be score 20]

  The specific types of misbehavior that are changed to 0 are:
  * Sending us 10 (*) separate BIP130 headers announcements that do not connect to our block tree [used to be score 20]
  * Sending us more than 8 headers in a single `headers` message (which thus does not get treated as a BIP130 announcement) that does not connect to our block tree. [used to be score 10]

  I believe that none of these behaviors are unavoidable, except for the one marked (*) which can in theory happen still due to interaction between BIP130 and variations in system clocks (the max 2 hour in the future rule). This one has been removed entirely. In order to remove the impact of the bug it was designed to deal with, without relying on misbehavior, a separate improvement is included that makes `getheaders`-tracking more accurate.

  In another unrelated improvement, this also gets rid of the 8 header limit heuristic to determine whether an incoming non-connecting `headers` is a potential BIP130 announcement, as this rule is no longer needed to prevent spurious Misbehavior. Instead, any non-connecting `headers` is now treated as a potential announcement.

ACKs for top commit:
  sr-gi:
    ACK [6eecba4](https://github.com/bitcoin/bitcoin/pull/29575/commits/6eecba475efd025eb011400af58621ad5823994e)
  achow101:
    ACK 6eecba475efd025eb011400af58621ad5823994e
  mzumsande:
    Code Review ACK 6eecba475efd025eb011400af58621ad5823994e
  glozow:
    light code review / concept ACK 6eecba475efd025eb011400af58621ad5823994e

Tree-SHA512: e11e8a652c4ec048d8961086110a3594feefbb821e13f45c14ef81016377be0db44b5311751ef635d6e026def1960aff33f644e78ece11cfb54f2b7daa96f946

1 files changed, 4 insertions, 5 deletions

diff --git a/test/functional/p2p_mutated_blocks.py b/test/functional/p2p_mutated_blocks.py
index 64d2fc96a8..708b19b1e5 100755
--- a/test/functional/p2p_mutated_blocks.py
+++ b/test/functional/p2p_mutated_blocks.py
@@ -104,11 +104,10 @@ class MutatedBlocksTest(BitcoinTestFramework):
         block_missing_prev.hashPrevBlock = 123
         block_missing_prev.solve()
 
-        # Attacker gets a DoS score of 10, not immediately disconnected, so we do it 10 times to get to 100
-        for _ in range(10):
-            assert_equal(len(self.nodes[0].getpeerinfo()), 2)
-            with self.nodes[0].assert_debug_log(expected_msgs=["AcceptBlock FAILED (prev-blk-not-found)"]):
-                attacker.send_message(msg_block(block_missing_prev))
+        # Check that non-connecting block causes disconnect
+        assert_equal(len(self.nodes[0].getpeerinfo()), 2)
+        with self.nodes[0].assert_debug_log(expected_msgs=["AcceptBlock FAILED (prev-blk-not-found)"]):
+            attacker.send_message(msg_block(block_missing_prev))
         attacker.wait_for_disconnect(timeout=5)