Banscore (later misbehavior score) used to be part of getpeerinfo but was removed back in https://github.com/bitcoin/bitcoin/pull/19469. The original idea was to get rid of the whole concept of an incremental score that will end up triggering a disconnection (ref). However, this was never fully removed, but just removed from the public interface.
The way misbehavior was assigned to peers was pretty arbitrary: some behavior would automatically trigger disconnection + discouragement, while other would be permitted up to a certain extend before facing the same outcome. Part of the reason for this to exist was fear in being to harsh and, potentially, trigger a disconnection too eagerly and end up partitioning the network (specially if someone could trigger that can of behavior for other peers). Looks like this is actually not the case, plus the approach makes it harder to test broken implementations (due to arbitrary limits).
This PR gets rid of the score altogether, making misbehaving a binary operation (that would either get you disconnected and discouraged or not have any consequence at all)
Increased misbehavior (immediate disconnection)
BlockValidationResult::BLOCK_MISSING_PREV: The peer has provided us a block we are missing the parent of. This can only have happened as an unrequested offer
reason -> prev-blk-not-foundCheckHeadersAreContinuous(headers) == false: The peer has provided a collection of headers that do not connect to each other (a single gap is enough to make this fail). This applies to a single headers message IINM
reason -> non-continuous headers sequencevAddr.size() > MAX_ADDR_TO_SEND: The peers has sent us an ADDR(or ADDR2) message containing more than 1000 addresses
reason -> addr message size = ...vInv.size() > MAX_INV_SZ (INV) : The peer has sent us an INV message containing more than 50000 items
reason -> inv message size = ...vInv.size() > MAX_INV_SZ (GETDATA): The peer has sent us an GET_DATA message requesting more than 50000 items
reason -> getdata message size = ...nCount > MAX_HEADERS_RESULTS: The peer has sent us a HEADERS message containing more than 2000 headers
reason -> headers message size = ...Removed misbehavior
peer.m_num_unconnecting_headers_msgs % MAX_NUM_UNCONNECTING_HEADERS_MSGS == 0: A peer send us more than MAX_NUM_UNCONNECTING_HEADERS_MSGS non connecting headers.
reason -> X non-connecting headersThis is not an issue anymore due to https://github.com/bitcoin/bitcoin/pull/25454 where the number of in-flight GETHEADERS request per peer is limited to one every 2 minutes. A malicious peer could still send us unconnecting headers and make us request the missing parts every 2 mins, making us waste bandwidth. However, there are easier ways to do this, and we cannot have code to prevent every single one (high complexity with little to no benefit).
🐞 There was a bug in one of the first versions of the PR regarding this functionality that would have bypassed the countermeasures of #25454: the timer for to send a GETHEADERS request is cleared every time a HEADERS message is received, despite it connecting or not, meaning a malicious peer could send us non-connecting headers, received our GETHEADERS request, ignore it, and keep sending us unconnecting headers. This would trigger a never ending loop. This was found by @mzumsande:
!headers_connect_blockindex && nCount <= MAX_BLOCKS_TO_ANNOUNCE: A peer send us a headers message containing more than 8 headers (the arbitrary limit to distinguish between BIP130 headers or not).
reason -> invalid header receivedBIP130 does not specify any limit for how many blocks can be announced under a HEADERS message. Bitcoin Core chooses to limit that to 8 (MAX_BLOCKS_TO_ANNOUNCE), and used to also enforce it when receiving. This was pretty arbitrary, and didn’t make much sense after removing the misbehavior score for BIP130 headers.
The PR mentions: 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).*
Q: How does the system clock affect BIP130 announcements? I may need to check BIP130 in more detail.
A: A block will be rejected if it is more than 2 hours in the future. If our system block is broken, we may reject any potentially connecting block because of the timestamp rule, which will make a potentially valid HEADERS message rejected. This would previously, eventually, trigger a disconnection while now it does not.
First pass: