From c77bd11d7f212b108f2b45402ebb23e7c3b38c40 Mon Sep 17 00:00:00 2001 From: Richard Ramos Date: Fri, 19 Nov 2021 09:11:17 -0400 Subject: [PATCH] fix: code review --- waku/v2/dnsdisc/enr_test.go | 12 ------------ waku/v2/node/wakunode2.go | 34 ++++++++++++++++----------------- waku/v2/utils/peer.go | 5 ++++- waku/v2/utils/peer_test.go | 38 +++++++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 31 deletions(-) diff --git a/waku/v2/dnsdisc/enr_test.go b/waku/v2/dnsdisc/enr_test.go index 28c69274..98551452 100644 --- a/waku/v2/dnsdisc/enr_test.go +++ b/waku/v2/dnsdisc/enr_test.go @@ -4,21 +4,9 @@ import ( "context" "testing" - "github.com/ethereum/go-ethereum/p2p/enode" - "github.com/status-im/go-waku/waku/v2/utils" "github.com/stretchr/testify/require" ) -func TestEnodeToMultiAddr(t *testing.T) { - enr := "enr:-IS4QAmC_o1PMi5DbR4Bh4oHVyQunZblg4bTaottPtBodAhJZvxVlWW-4rXITPNg4mwJ8cW__D9FBDc9N4mdhyMqB-EBgmlkgnY0gmlwhIbRi9KJc2VjcDI1NmsxoQOevTdO6jvv3fRruxguKR-3Ge4bcFsLeAIWEDjrfaigNoN0Y3CCdl8" - - parsedNode := enode.MustParse(enr) - expectedMultiAddr := "/ip4/134.209.139.210/tcp/30303/p2p/16Uiu2HAmPLe7Mzm8TsYUubgCAW1aJoeFScxrLj8ppHFivPo97bUZ" - actualMultiAddr, err := utils.EnodeToMultiAddr(parsedNode) - require.NoError(t, err) - require.Equal(t, expectedMultiAddr, actualMultiAddr.String()) -} - // TestRetrieveNodes uses a live connection, so it could be // flaky, it should though pay for itself and should be fairly stable func TestRetrieveNodes(t *testing.T) { diff --git a/waku/v2/node/wakunode2.go b/waku/v2/node/wakunode2.go index 87d794a3..182af24d 100644 --- a/waku/v2/node/wakunode2.go +++ b/waku/v2/node/wakunode2.go @@ -175,6 +175,20 @@ func (w *WakuNode) onAddrChange() { } } +func (w *WakuNode) logAddress(addr ma.Multiaddr) { + log.Info("Listening on ", addr) + + // TODO: make this optional depending on DNS Disc being enabled + if w.opts.privKey != nil { + enr, ip, err := utils.GetENRandIP(addr, w.opts.privKey) + if err != nil { + log.Error("could not obtain ENR record from multiaddress", err) + } else { + log.Info(fmt.Sprintf("ENR for IP %s: %s", ip, enr)) + } + } +} + func (w *WakuNode) checkForAddressChanges() { addrs := w.ListenAddresses() first := make(chan struct{}, 1) @@ -185,15 +199,7 @@ func (w *WakuNode) checkForAddressChanges() { return case <-first: for _, addr := range addrs { - log.Info("Listening on ", addr) - - // TODO: make this optional depending on DNS Disc being enabled - enr, ip, err := utils.GetENRandIP(addr, w.opts.privKey) - if err != nil { - log.Error("could not obtain ENR record from multiaddress", err) - } else { - log.Info(fmt.Sprintf("ENR for IP %s: %s", ip, enr)) - } + w.logAddress(addr) } case <-w.addressChangesSub.Out(): newAddrs := w.ListenAddresses() @@ -213,15 +219,7 @@ func (w *WakuNode) checkForAddressChanges() { log.Warn("Change in host multiaddresses") for _, addr := range newAddrs { w.addrChan <- addr - log.Warn("Listening on ", addr) - - // TODO: make this optional depending on DNS Disc being enabled - enr, ip, err := utils.GetENRandIP(addr, w.opts.privKey) - if err != nil { - log.Error("could not obtain ENR record from multiaddress", err) - } else { - log.Warn(fmt.Sprintf("ENR for IP %s: %s", ip, enr)) - } + w.logAddress(addr) } } } diff --git a/waku/v2/utils/peer.go b/waku/v2/utils/peer.go index 143ac751..0e777561 100644 --- a/waku/v2/utils/peer.go +++ b/waku/v2/utils/peer.go @@ -164,7 +164,10 @@ func GetENRandIP(addr ma.Multiaddr, privK *ecdsa.PrivateKey) (*enode.Node, *net. r.Set(enr.TCP(port)) r.Set(enr.IP(net.ParseIP(ip))) - enode.SignV4(r, privK) + err = enode.SignV4(r, privK) + if err != nil { + return nil, nil, err + } node, err := enode.New(enode.ValidSchemes, r) diff --git a/waku/v2/utils/peer_test.go b/waku/v2/utils/peer_test.go index f32ae92b..db0f5835 100644 --- a/waku/v2/utils/peer_test.go +++ b/waku/v2/utils/peer_test.go @@ -3,10 +3,18 @@ package utils import ( "context" "crypto/rand" + "fmt" + "net" "testing" "time" + gcrypto "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/p2p/enode" + crypto "github.com/libp2p/go-libp2p-core/crypto" + "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/peerstore" + ma "github.com/multiformats/go-multiaddr" + manet "github.com/multiformats/go-multiaddr/net" "github.com/status-im/go-waku/tests" "github.com/stretchr/testify/require" ) @@ -79,3 +87,33 @@ func TestSelectPeerWithLowestRTT(t *testing.T) { _, err = SelectPeerWithLowestRTT(ctx, h1, proto) require.NoError(t, err) } + +func TestEnodeToMultiAddr(t *testing.T) { + enr := "enr:-IS4QAmC_o1PMi5DbR4Bh4oHVyQunZblg4bTaottPtBodAhJZvxVlWW-4rXITPNg4mwJ8cW__D9FBDc9N4mdhyMqB-EBgmlkgnY0gmlwhIbRi9KJc2VjcDI1NmsxoQOevTdO6jvv3fRruxguKR-3Ge4bcFsLeAIWEDjrfaigNoN0Y3CCdl8" + + parsedNode := enode.MustParse(enr) + expectedMultiAddr := "/ip4/134.209.139.210/tcp/30303/p2p/16Uiu2HAmPLe7Mzm8TsYUubgCAW1aJoeFScxrLj8ppHFivPo97bUZ" + actualMultiAddr, err := EnodeToMultiAddr(parsedNode) + require.NoError(t, err) + require.Equal(t, expectedMultiAddr, actualMultiAddr.String()) +} + +func TestGetENRandIP(t *testing.T) { + key, _ := gcrypto.GenerateKey() + privKey := crypto.PrivKey((*crypto.Secp256k1PrivateKey)(key)) + id, _ := peer.IDFromPublicKey(privKey.GetPublic()) + + hostAddr := &net.TCPAddr{IP: net.ParseIP("192.168.0.1"), Port: 9999} + hostMultiAddr, _ := manet.FromNetAddr(hostAddr) + hostInfo, _ := ma.NewMultiaddr(fmt.Sprintf("/p2p/%s", id.Pretty())) + ogMultiaddress := hostMultiAddr.Encapsulate(hostInfo) + + node, resTCPAddr, err := GetENRandIP(ogMultiaddress, key) + require.NoError(t, err) + require.Equal(t, hostAddr, resTCPAddr) + + parsedNode := enode.MustParse(node.String()) + resMultiaddress, err := EnodeToMultiAddr(parsedNode) + require.NoError(t, err) + require.Equal(t, ogMultiaddress.String(), resMultiaddress.String()) +}