-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat: bootstrap in dht when the routing table is empty #7340
Conversation
return out, fmt.Errorf("failed to parse bootstrap peer address %q: %s", addrStr, err) | ||
} | ||
bootstrappers = append(bootstrappers, addr) | ||
} |
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.
Changing dht.BootstrapPeers
to take peer.AddrInfo
would make this code simpler...
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.
@Stebalien Have made the change here:
libp2p/go-libp2p-kad-dht#651
Also, we should now use go-libp2p-kad-dht.GetDefaultBootstrapPeerAddrInfos
to set the default for cfg. Bootstrap
.
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.
I'm not sure I really understand what makes this easier, but also no complaints from me :)
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.
It's easier in this case because cfg
has a special function to get AddrInfos.
core/node/libp2p/host.go
Outdated
return out, err | ||
} | ||
var bootstrappers []ma.Multiaddr | ||
for _, addrStr := range cfg.Bootstrap { |
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.
@Stebalien Is the default for cfg.Bootstrap
the DefaultBootstrapPeers
value in go-libp2p-kad-dht ?
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.
Basically, yes. Although I think it's duplicated.
return out, fmt.Errorf("failed to parse bootstrap peer address %q: %s", addrStr, err) | ||
} | ||
bootstrappers = append(bootstrappers, addr) | ||
} |
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.
@Stebalien Have made the change here:
libp2p/go-libp2p-kad-dht#651
Also, we should now use go-libp2p-kad-dht.GetDefaultBootstrapPeerAddrInfos
to set the default for cfg. Bootstrap
.
25ff976
to
1f21f19
Compare
Otherwise, we could end up with only DHT clients and never re-bootstrap. I've left the default go-ipfs bootstrapping code in for now as it's technically possible to disable the DHT entirely.
1f21f19
to
ff17485
Compare
Otherwise, we could end up with only DHT clients and never re-bootstrap.
I've left the default go-ipfs bootstrapping code in for now as it's technically possible to disable the DHT entirely.