net-address: Add get group function. #217
Conversation
kilpatty
changed the title
kilpatty
force-pushed
the
add-group-netaddress
branch
from
03e6ed9
to
4feb91a
Compare
Codecov Report
@@ Coverage Diff @@
## master #217 +/- ##
==========================================
+ Coverage 52.99% 53.06% +0.07%
==========================================
Files 129 129
Lines 35773 35829 +56
Branches 6032 6046 +14
==========================================
+ Hits 18957 19013 +56
Misses 16816 16816
Continue to review full report at Codecov.
|
kilpatty
force-pushed
the
add-group-netaddress
branch
2 times, most recently
from
46b9b01
to
ddf5844
Compare
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.
kilpatty
force-pushed
the
add-group-netaddress
branch
from
b9e0d49
to
8c62603
Compare
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.
|
Note to self: It'd be great to get a test case ensuring that we aren't adding 2 peers from the same group to outbound. I think it would be a good fit for when someone ports over the bcoin net tests. See: #197 |
kilpatty
changed the title
#197 has been closed, it would be ideal to have an issue to track this. Is there any additional information you would like to be in the issue? |
|
@tynes I can open one here |
|
Having this merged would be very helpful for the experimental testnet as we are noticing some strange connectivity problems between nodes on the network. It depends on The plan is to review and merge it here: |
| startByte = 6; | ||
| bits = 4; | ||
| // for he.net, use /36 groups | ||
| } else if ( |
There was a problem hiding this comment.
I think this would read a lot better if it was a function
| let startByte = 0; | ||
| let bits = 16; | ||
|
|
||
| // all local addresses belong to the same group |
There was a problem hiding this comment.
Style nit: generally start comments with a capital letter and end with a period
|
@chjj Want me to kill this now with the most recent changes? |
nodech
added
breaking-minor
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.
TODO:
Add to hostlist-Not backwards compatible, should go into a different PR, with other hostlist changes.