-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add flag to choose advertising local IP to Prysm node #33
Add flag to choose advertising local IP to Prysm node #33
Conversation
// this host is listening on. | ||
func (h *Host) LocalListenMaddr() (ma.Multiaddr, error) { | ||
for _, maddr := range h.Addrs() { | ||
if manet.IsIPLoopback(maddr) { |
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.
Does it need to be specifically ip4
or ip6
or it doesn't matter and both addresses are acceptable?
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.
That is a good question. I've been checking at the Prysm code, and it doesn't seem to check anything, it just parses the maddr
info using the go-libp2p
primitives:
func (s *Server) AddTrustedPeer(w http.ResponseWriter, r *http.Request) {
...
info, err := peer.AddrInfoFromString(addrRequest.Addr)
....
Do you think that could be a problem? We don't seem to be checking anything like that for the private
maddr neither:
// PrivateListenMaddr returns the first multiaddress in a private IP range that
// this host is listening on.
func (h *Host) PrivateListenMaddr() (ma.Multiaddr, error) {
for _, maddr := range h.Addrs() {
if manet.IsIPLoopback(maddr) {
continue
}
if !manet.IsPrivateAddr(maddr) {
continue
}
return maddr, nil
}
return nil, fmt.Errorf("no private multi address found in %s", h.Addrs())
}
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.
As long as both Hermes and Prysm have both an ip4
and ip6
and can reach each other on both stacks it is fine (or they are both using only ip4
XOR ip6
). Otherwise, we could also restrict both to use ip4
(or ip6
) only
Coming back to this since it got hanging for a few weeks. |
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.
LGTM if everything runs on the ip4
stack
This PR addresses #31
Changes work locally: