Skip to content
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

Request more p2p addrs after peer checks #2324

Merged
merged 1 commit into from
Jan 23, 2024
Merged

Conversation

jrick
Copy link
Member

@jrick jrick commented Jan 23, 2024

If the height or required services is deemed insufficient after the connection handshake, do not request more addresses right away, waiting for its response before killing the TCP connection. This stops ConnectOutbound from returning early with error while the TCP connection is still active, and causing the SPV syncer to begin connecting to more remote peers. While the SPV syncer remains limited to 8 total outbound managed peers, the total count of TCP connections can easily exceed this, and has been observed to max out the circuit limit on Tor proxies.

If the height or required services is deemed insufficient after the connection
handshake, do not request more addresses right away, waiting for its response
before killing the TCP connection.  This stops ConnectOutbound from returning
early with error while the TCP connection is still active, and causing the SPV
syncer to begin connecting to more remote peers.  While the SPV syncer remains
limited to 8 total outbound managed peers, the total count of TCP connections
can easily exceed this, and has been observed to max out the circuit limit on
Tor proxies.

Although this appears to move the address requesting to the foreground of
ConnectOutbound, it only writes the getaddr message.  addr message replies are
handled internally by the RemotePeer.
@jrick jrick merged commit ab6da24 into decred:master Jan 23, 2024
2 checks passed
@jrick jrick deleted the getaddrs branch January 23, 2024 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants