Skip to content

Commit

Permalink
Bump misbehaving factor for unexpected version message behavior
Browse files Browse the repository at this point in the history
Summary:
The net protocol expects that the version-verack handshake
is complete before other messages are sent.  There's no reason to
allow 100 out-of-order messages to come from a single peer before
deciding to ban them. This behavior should be treated more suspiciously
and ban the peer after one order of magnitude fewer messages.

To assess the impact of this patch, I examined mainnet logs spanning the last 4 months.
There were only 13 instances of `missing-verack` and none of `missing-version`.
All instances were received from unique peers.  Given this data, I think it's safe to assume
we will not see 10+ `missing-(version|verack)` instances from any currently deployed nodes
that would otherwise not be banned prior to this patch.

Test Plan:
```
ninja check check-functional
```

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6036
  • Loading branch information
jasonbcox authored and ftrader committed Jul 3, 2020
1 parent 6342f98 commit 73ea836
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/net_processing.cpp
Expand Up @@ -2296,7 +2296,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,
if (pfrom->nVersion == 0) {
// Must have a version message before anything else
LOCK(cs_main);
Misbehaving(pfrom, 1, "missing-version");
Misbehaving(pfrom, 10, "missing-version");
return false;
}

Expand Down Expand Up @@ -2345,7 +2345,7 @@ static bool ProcessMessage(const Config &config, CNode *pfrom,
if (!pfrom->fSuccessfullyConnected) {
// Must have a verack message before anything else
LOCK(cs_main);
Misbehaving(pfrom, 1, "missing-verack");
Misbehaving(pfrom, 10, "missing-verack");
return false;
}

Expand Down

0 comments on commit 73ea836

Please sign in to comment.