Skip to content
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

Merged
merged 1 commit into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cmd/hermes/cmd_eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var ethConfig = &struct {
Libp2pHost string
Libp2pPort int
Libp2pPeerscoreSnapshotFreq time.Duration
LocalTrustedAddr bool
PrysmHost string
PrysmPortHTTP int
PrysmPortGRPC int
Expand All @@ -42,6 +43,7 @@ var ethConfig = &struct {
Libp2pHost: "127.0.0.1",
Libp2pPort: 0,
Libp2pPeerscoreSnapshotFreq: 60 * time.Second,
LocalTrustedAddr: false, // default -> advertise the private multiaddress to our trusted Prysm node
PrysmHost: "",
PrysmPortHTTP: 3500, // default -> https://docs.prylabs.network/docs/prysm-usage/p2p-host-ip
PrysmPortGRPC: 4000, // default -> https://docs.prylabs.network/docs/prysm-usage/p2p-host-ip
Expand Down Expand Up @@ -139,6 +141,13 @@ var cmdEthFlags = []cli.Flag{
Destination: &ethConfig.Libp2pPeerscoreSnapshotFreq,
DefaultText: "random",
},
&cli.BoolFlag{
Name: "local.trusted.addr",
EnvVars: []string{"HERMES_ETH_LOCAL_TRUSTED_ADDRESS"},
Usage: "To advertise the localhost multiaddress to our trusted control Prysm node",
Value: ethConfig.LocalTrustedAddr,
Destination: &ethConfig.LocalTrustedAddr,
},
&cli.StringFlag{
Name: "prysm.host",
EnvVars: []string{"HERMES_ETH_PRYSM_HOST"},
Expand Down Expand Up @@ -219,6 +228,7 @@ func cmdEthAction(c *cli.Context) error {
Libp2pPeerscoreSnapshotFreq: ethConfig.Libp2pPeerscoreSnapshotFreq,
GossipSubMessageEncoder: encoder.SszNetworkEncoder{},
RPCEncoder: encoder.SszNetworkEncoder{},
LocalTrustedAddr: ethConfig.LocalTrustedAddr,
PrysmHost: ethConfig.PrysmHost,
PrysmPortHTTP: ethConfig.PrysmPortHTTP,
PrysmPortGRPC: ethConfig.PrysmPortGRPC,
Expand Down
23 changes: 17 additions & 6 deletions eth/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/aws/aws-sdk-go-v2/service/kinesis"
gk "github.com/dennis-tra/go-kinesis"
"github.com/libp2p/go-libp2p/core/peer"
ma "github.com/multiformats/go-multiaddr"
eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
"github.com/thejerf/suture/v4"
"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -209,7 +210,7 @@ func NewNode(cfg *NodeConfig) (*Node, error) {
reqResp: reqResp,
pubSub: pubSub,
pryClient: pryClient,
peerer: NewPeerer(h, pryClient),
peerer: NewPeerer(h, pryClient, cfg.LocalTrustedAddr),
disc: disc,
eventCallbacks: []func(ctx context.Context, event *host.TraceEvent){},
}
Expand Down Expand Up @@ -398,15 +399,25 @@ func (n *Node) Start(ctx context.Context) error {
connSignal := n.host.ConnSignal(timeoutCtx, addrInfo.ID)

// register ourselves as a trusted peer by submitting our private ip address
privateMaddr, err := n.host.PrivateListenMaddr()
if err != nil {
return err
var trustedMaddr ma.Multiaddr
if n.cfg.LocalTrustedAddr {
trustedMaddr, err = n.host.LocalListenMaddr()
if err != nil {
return err
}
slog.Info("Adding ourselves as a trusted peer to Prysm", tele.LogAttrPeerID(n.host.ID()), "on local maddr", trustedMaddr)
} else {
trustedMaddr, err = n.host.PrivateListenMaddr()
if err != nil {
return err
}
slog.Info("Adding ourselves as a trusted peer to Prysm", tele.LogAttrPeerID(n.host.ID()), "on priv maddr", trustedMaddr)
}

slog.Info("Adding ourselves as a trusted peer to Prysm", tele.LogAttrPeerID(n.host.ID()), "maddr", privateMaddr)
if err := n.pryClient.AddTrustedPeer(ctx, n.host.ID(), privateMaddr); err != nil {
if err := n.pryClient.AddTrustedPeer(ctx, n.host.ID(), trustedMaddr); err != nil {
return fmt.Errorf("failed adding ourself as trusted peer: %w", err)
}

defer func() {
// unregister ourselves as a trusted peer from prysm. Context timeout
// is not necessary because the pryClient applies a 5s timeout to each API call
Expand Down
7 changes: 4 additions & 3 deletions eth/node_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ type NodeConfig struct {
RPCEncoder encoder.NetworkEncoding

// The address information where the Beacon API or Prysm's custom API is accessible at
PrysmHost string
PrysmPortHTTP int
PrysmPortGRPC int
LocalTrustedAddr bool
PrysmHost string
PrysmPortHTTP int
PrysmPortGRPC int

// The AWS Kinesis Data Stream configuration
AWSConfig *aws.Config // if set, we consider Kinesis to be enabled
Expand Down
31 changes: 20 additions & 11 deletions eth/peerer.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/libp2p/go-libp2p/core/network"
ma "github.com/multiformats/go-multiaddr"
"github.com/probe-lab/hermes/host"
"github.com/probe-lab/hermes/tele"
"github.com/thejerf/suture/v4"
Expand All @@ -17,16 +18,17 @@ import (
// [PeererClient] implementations can be used. In the case of Prysm, use the
// [PrysmClient] implementation as it implements [PeererClient].
type Peerer struct {
host *host.Host
pryClient *PrysmClient
host *host.Host
pryClient *PrysmClient
localTrustedAddr bool
}

var _ suture.Service = (*Peerer)(nil)

// NewPeerer creates a new instance of the Peerer struct.
// It takes a pointer to a *host.Host and a [PeererClient] implementation as parameters.
// It returns a pointer to the newly created Peerer instance.
func NewPeerer(h *host.Host, pryClient *PrysmClient) *Peerer {
func NewPeerer(h *host.Host, pryClient *PrysmClient, localTrustedAddr bool) *Peerer {
return &Peerer{
host: h,
pryClient: pryClient,
Expand Down Expand Up @@ -69,15 +71,22 @@ func (p *Peerer) Serve(ctx context.Context) error {

slog.Warn("Not registered as a trusted peer")

// we're not in the list of trusted peers
// get our private liste multiaddress and register again
privateMaddr, err := p.host.PrivateListenMaddr()
if err != nil {
return err
}

// register ourselves as a trusted peer
if err := p.pryClient.AddTrustedPeer(ctx, p.host.ID(), privateMaddr); err != nil {
// register ourselves as a trusted peer by submitting our private ip address
var trustedMaddr ma.Multiaddr
if p.localTrustedAddr {
trustedMaddr, err = p.host.LocalListenMaddr()
if err != nil {
return err
}
} else {
trustedMaddr, err = p.host.PrivateListenMaddr()
if err != nil {
return err
}
}
slog.Info("ensuring we are trusted by trusted Prysm with peer_id:", tele.LogAttrPeerID(p.host.ID()), "on local maddr", trustedMaddr)
if err := p.pryClient.AddTrustedPeer(ctx, p.host.ID(), trustedMaddr); err != nil {
return fmt.Errorf("failed adding ourself as trusted peer: %w", err)
}

Expand Down
11 changes: 11 additions & 0 deletions host/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,17 @@ func (h *Host) PrivateListenMaddr() (ma.Multiaddr, error) {
return nil, fmt.Errorf("no private multi address found in %s", h.Addrs())
}

// LocalListenMaddr returns the first multiaddress in a localhost IP range that
// this host is listening on.
func (h *Host) LocalListenMaddr() (ma.Multiaddr, error) {
for _, maddr := range h.Addrs() {
if manet.IsIPLoopback(maddr) {
Copy link
Contributor

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?

Copy link
Contributor Author

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())
}

Copy link
Contributor

@guillaumemichel guillaumemichel Jun 20, 2024

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

return maddr, nil
}
}
return nil, fmt.Errorf("no local multi address found in %s", h.Addrs())
}

func (h *Host) TracedTopicHandler(handler TopicHandler) TopicHandler {
return func(ctx context.Context, msg *pubsub.Message) error {
slog.Debug("Handling gossip message", "topic", msg.GetTopic())
Expand Down
Loading