-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
backport: Merge bitcoin#22423, 22096 #5884
Conversation
f71025e
to
e17634c
Compare
Hello @UdjinM6 @knst @PastaPastaPasta requesting review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 651fad5
@@ -0,0 +1,62 @@ | |||
#!/usr/bin/env python3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider to backport bitcoin#22568 in a follow-up PR. There are no any important/critical fix, just tests improvements, so, that's an out-of-scope of current PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 651fad5
…cements 5730a43 test: Add functional test for AddrFetch connections (Martin Zumsande) c34ad33 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande) 533500d p2p: Add timeout for AddrFetch peers (Martin Zumsande) b6c5d1e p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande) Pull request description: AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them. This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement. So we'll disconnect before the peer gets a chance to answer our `getaddr`. I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us. The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers. Fix this by only disconnecting after receiving an `addr` message of size > 1. [Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test. ACKs for top commit: amitiuttarwar: reACK 5730a43 naumenkogs: ACK 5730a43 jnewbery: ACK 5730a43 Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
22423 was already merged in #6021 |
bitcoin backports