Page MenuHomePhabricator

Bump misbehaving factor for unexpected version message behavior
ClosedPublic

Authored by jasonbcox on May 11 2020, 20:33.

Details

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

Diff Detail

Repository
rABC Bitcoin ABC
Branch
misbehaving
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 10702
Build 19200: Default Diff Build & Tests
Build 19199: arc lint + arc unit

Event Timeline

jasonbcox planned changes to this revision.
jasonbcox edited the summary of this revision. (Show Details)
jasonbcox edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.May 13 2020, 23:14