-
Notifications
You must be signed in to change notification settings - Fork 682
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
Prefer gossiping bootstrapper IPs #2400
Conversation
@@ -27,7 +27,7 @@ const ( | |||
|
|||
MaxContainersLen = int(4 * DefaultMaxMessageSize / 5) | |||
|
|||
DefaultNetworkPeerListNumValidatorIPs = 15 |
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.
Set this so that we will (even in the first message) send some non-bootstrapper IPs so that the first round of connections don't just result in everyone sending the same peerlists (and therefore the nodes not quickly connecting to the wider network)
Co-authored-by: Dhruba Basu <[email protected]> Signed-off-by: Stephen Buttolph <[email protected]>
s.Initialize(uint64(len(unknownValidators))) | ||
var bootstrapperIDs set.Set[ids.NodeID] | ||
for _, bootstrapper := range genesis.GetBootstrappers(n.config.NetworkID) { | ||
bootstrapperIDs.Add(bootstrapper.ID) |
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.
This will initialize the Set if it hasn't been yet, right?
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.
correct
This PR has become stale because it has been open for 30 days with no activity. Adding the |
This is no longer needed because |
Why this should be merged
How this works
How this was tested