Network tests & updates #692
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently, pool::getHost() uses a fixed loop of 0-99 regardless of how many hosts it has in it's lists so it can use a percentage to factor in whether it should use an entry or not. Using network regtest and a single host of 127.0.0.1:14038, hsd burns a lot of CPU when refilling peers and this blocks incoming requests so much it times out. This is less noticeable the more hosts you have. Instead, use shorter loop of 10. This brings CPU utlisation on my NetBSD Xen DOMU on regtest with one host from 100% to 6% and connecting 4 to peers on testnet from 85% to 8%, allowing RPC commands to now work. Fixes handshake-org#220.
This commit removes the discover external check in the pool. Currently we already store all local addresses that are sent to us from peers in the hostlist, so there is no reason to rely on an external service to check for our local/external address. This was removed from bitcoin in commit 845c86d.
This commit adds the getGroup function to net addresses. It allows for network addresses to be grouped into buckets, such that we can limit the number of outgoing connections per group to a certain amount. This will allow for increased connection diversity and reduced attack surface area. Co-authored-by: Nodari Chkuaselidze <nodar.chkuaselidze@gmail.com>
This commit checks the outbound peers to see which group their network addresses lives in before connecting. By doing this, we prevent a node from connecting to all outbound addresses in a very close network group bunch. This helps prevent someone spinning up multiple nodes on a similar network e.g. AWS, GCP and using those to fill other nodes' outbounds. This commit just adds 2 operations in both addOutbound and addLoader. It first checks if the new address pull from hostlist belongs in the same group as an already connected peer. If it does not, it precedes as usual, and if the connection succeeds then it adds that new outbound group to the list of groups currently connected to. Co-authored-by: Nodari Chkuaselidze <nodar.chkuaselidze@gmail.com>
nodech
added
net/p2p
pinheadmz
reviewed
Mar 22, 2022
lib/net/pool.js
Outdated
Comment on lines
3395
to
3397
| // Don't connect to outbound peers in the same group. | ||
| if (this.connectedGroups.has(addr.getGroup())) | ||
| continue; |
There was a problem hiding this comment.
should we allow this with some low probability? or is it always bad
There was a problem hiding this comment.
hm.. Maybe last 2 tries ? pc80 ?
There was a problem hiding this comment.
looks like Bitcoin Core really doesnt allow this at all:
https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L2114-L2117
So I defer to you, just not sure given the limited size of the handshake network if this is actually beneficial.
|
Another thing to consider: bump the version of the hosts.json file? Maybe even add a migration in this PR since we are grouping addrs in buckets in a different way now. |
pinheadmz
reviewed
Mar 22, 2022
nodech
force-pushed
the
net-updates
branch
2 times, most recently
from
5cc8615
to
09f9133
Compare
pinheadmz
reviewed
Apr 1, 2022
This was referenced Apr 7, 2022
pinheadmz
added a commit
to pinheadmz/hsd
that referenced
this pull request
Aug 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds tests and also includes several older PRs.
binetwith bug fixes.