[P2P] Rate limiting rumored address processing #2824
Conversation
Liquid369
force-pushed
the
2023_rate_limit_getaddr
branch
2 times, most recently
from
98bf8ce
to
fb8f511
Compare
Fuzzbawls
changed the title
Liquid369
force-pushed
the
2023_rate_limit_getaddr
branch
2 times, most recently
from
fea7667
to
a6d69d8
Compare
Liquid369
force-pushed
the
2023_rate_limit_getaddr
branch
from
a6d69d8
to
3c6280b
Compare
Liquid369
force-pushed
the
2023_rate_limit_getaddr
branch
from
3c6280b
to
0c2deb2
Compare
Liquid369
force-pushed
the
2023_rate_limit_getaddr
branch
from
0c2deb2
to
01e2e26
Compare
Liquid369
force-pushed
the
2023_rate_limit_getaddr
branch
4 times, most recently
from
611cd77
to
3717978
Compare
test/functional/p2p_addrv2_relay.py
Outdated
| 'received: addrv2 (131 bytes) peer=0', | ||
| 'sending addrv2 (131 bytes) peer=1', | ||
| 'sending addrv2 (14 bytes) peer=1', |
There was a problem hiding this comment.
Wrote yesterday on discord, but rewriting here so we don't forget:
Imo the assert here should not be changed because we are functional testing a specific expected behaviour. If this change is due to the fact that m_addr_token_bucket is empty then it is better if you change the node time to refill it, if it's not that then the PR introduced a bug
| @@ -58,13 +74,76 @@ def run_test(self): | |||
| addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) | |||
| msg.addrs = ADDRS | |||
| with self.nodes[0].assert_debug_log([ | |||
| 'Added 10 addresses from 127.0.0.1: 0 tried', | |||
| 'received: addr (301 bytes) peer=0', | |||
There was a problem hiding this comment.
same comment here, an assert that tested an expected result has been changed. If this was due to the fact that the bucket is empty then ok change the node's time to refill it and the unchanged assert should pass. If it does not a bug was introduced
There was a problem hiding this comment.
In this regard, I have restored the old logs but have changed the addr_token_bucket from 1.0 to 10.0 to reflect the logging more accurately rather than accounting for a stricter rate limit.
Take a look let me know, ima try to get this last test working...again
Set for every instance and fix test peer, remove var Refactor debug logs Adjust p2p invalid msgs
Liquid369
force-pushed
the
2023_rate_limit_getaddr
branch
from
0e7eb41
to
c5fd91e
Compare
As of recent there was a release on https://www.halborn.com/blog/post/halborn-discovers-zero-day-impacting-dogecoin-and-280-networks
We want to address potential situation that can cause network instability and node attacks.
This PR back ports
bitcoin#22387
bitcoin#15617
This implements a token bucket for rate limiting addresses, allowing up to 1000 in one request. Getaddr requests are exempt from this rate limiting. Then we are also no longer going to relay to other nodes peers we have banned. Functional tests included.