diff --git a/waku/v2/node/localnode.go b/waku/v2/node/localnode.go index 7549d8ae..b602897b 100644 --- a/waku/v2/node/localnode.go +++ b/waku/v2/node/localnode.go @@ -6,8 +6,10 @@ import ( "encoding/binary" "errors" "math" + "math/rand" "net" "strconv" + "time" "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/p2p/enr" @@ -24,6 +26,33 @@ func (w *WakuNode) newLocalnode(priv *ecdsa.PrivateKey) (*enode.LocalNode, error return enode.NewLocalNode(db, priv), nil } +func writeMultiaddressField(localnode *enode.LocalNode, addrAggr []ma.Multiaddr) (err error) { + defer func() { + if e := recover(); e != nil { + err = errors.New("could not write enr record") + } + }() + + var fieldRaw []byte + for _, addr := range addrAggr { + maRaw := addr.Bytes() + maSize := make([]byte, 2) + binary.BigEndian.PutUint16(maSize, uint16(len(maRaw))) + + fieldRaw = append(fieldRaw, maSize...) + fieldRaw = append(fieldRaw, maRaw...) + } + + if len(fieldRaw) != 0 { + localnode.Set(enr.WithEntry(utils.MultiaddrENRField, fieldRaw)) + } + + // This is to trigger the signing record err due to exceeding 300bytes limit + _ = localnode.Node() + + return nil +} + func (w *WakuNode) updateLocalNode(localnode *enode.LocalNode, multiaddrs []ma.Multiaddr, ipAddr *net.TCPAddr, udpPort uint, wakuFlags utils.WakuEnrBitfield, advertiseAddr *net.IP, shouldAutoUpdate bool, log *zap.Logger) error { localnode.SetFallbackUDP(int(udpPort)) localnode.Set(enr.WithEntry(utils.WakuENRField, wakuFlags)) @@ -67,47 +96,37 @@ func (w *WakuNode) updateLocalNode(localnode *enode.LocalNode, multiaddrs []ma.M } } - // Adding extra multiaddresses. It will to add as many multiaddresses as possible - // without exceeding the enr max size of 300bytes - var addrAggr []ma.Multiaddr + // Randomly shuffle multiaddresses + rand.Seed(time.Now().UnixNano()) + rand.Shuffle(len(multiaddrs), func(i, j int) { multiaddrs[i], multiaddrs[j] = multiaddrs[j], multiaddrs[i] }) + + // Adding extra multiaddresses. Should probably not exceed the enr max size of 300bytes var err error + failedOnceWritingENR := false + couldWriteENRatLeastOnce := false + successIdx := -1 for i := len(multiaddrs) - 1; i >= 0; i-- { - addrAggr = append(addrAggr, multiaddrs[0:i]...) - err = func() (err error) { - defer func() { - if e := recover(); e != nil { - err = errors.New("could not write enr record") - } - }() - - var fieldRaw []byte - for _, addr := range addrAggr { - maRaw := addr.Bytes() - maSize := make([]byte, 2) - binary.BigEndian.PutUint16(maSize, uint16(len(maRaw))) - - fieldRaw = append(fieldRaw, maSize...) - fieldRaw = append(fieldRaw, maRaw...) - } - - if len(fieldRaw) != 0 { - localnode.Set(enr.WithEntry(utils.MultiaddrENRField, fieldRaw)) - } - - // This is to trigger the signing record err due to exceeding 300bytes limit - _ = localnode.Node() - - return nil - }() - + err = writeMultiaddressField(localnode, multiaddrs[0:i]) if err == nil { + couldWriteENRatLeastOnce = true + successIdx = i break + } else { + failedOnceWritingENR = true } } - // In case multiaddr could not be populated at all - if err != nil { - localnode.Delete(enr.WithEntry(utils.MultiaddrENRField, struct{}{})) + if failedOnceWritingENR { + if !couldWriteENRatLeastOnce { + // In case multiaddr could not be populated at all + localnode.Delete(enr.WithEntry(utils.MultiaddrENRField, struct{}{})) + } else { + // Could write a subset of multiaddresses but not all + err = writeMultiaddressField(localnode, multiaddrs[0:successIdx]) + if err != nil { + return errors.New("could not write new ENR") + } + } } return nil diff --git a/waku/v2/node/wakunode2.go b/waku/v2/node/wakunode2.go index 833fa01f..e961fd72 100644 --- a/waku/v2/node/wakunode2.go +++ b/waku/v2/node/wakunode2.go @@ -91,6 +91,7 @@ type WakuNode struct { protocolEventSub event.Subscription identificationEventSub event.Subscription addressChangesSub event.Subscription + enrChangeCh chan struct{} keepAliveMutex sync.Mutex keepAliveFails map[peer.ID]int @@ -234,6 +235,8 @@ func New(opts ...WakuNodeOption) (*WakuNode, error) { return nil, err } + w.enrChangeCh = make(chan struct{}, 10) + if params.connStatusC != nil { w.connStatusChan = params.connStatusC } @@ -253,6 +256,7 @@ func (w *WakuNode) watchMultiaddressChanges(ctx context.Context) { return case <-first: w.log.Info("listening", logging.MultiAddrs("multiaddr", addrs...)) + w.enrChangeCh <- struct{}{} case <-w.addressChangesSub.Out(): newAddrs := w.ListenAddresses() diff := false @@ -270,6 +274,7 @@ func (w *WakuNode) watchMultiaddressChanges(ctx context.Context) { addrs = newAddrs w.log.Info("listening addresses update received", logging.MultiAddrs("multiaddr", addrs...)) _ = w.setupENR(ctx, addrs) + w.enrChangeCh <- struct{}{} } } } @@ -403,6 +408,8 @@ func (w *WakuNode) Stop() { w.host.Close() + close(w.enrChangeCh) + w.wg.Wait() } @@ -419,13 +426,12 @@ func (w *WakuNode) ID() string { func (w *WakuNode) watchENRChanges(ctx context.Context) { defer w.wg.Done() - timer := time.NewTicker(1 * time.Second) var prevNodeVal string for { select { case <-ctx.Done(): return - case <-timer.C: + case <-w.enrChangeCh: if w.localNode != nil { currNodeVal := w.localNode.Node().String() if prevNodeVal != currNodeVal { diff --git a/waku/v2/utils/enr_test.go b/waku/v2/utils/enr_test.go index e00a921b..cda2efb1 100644 --- a/waku/v2/utils/enr_test.go +++ b/waku/v2/utils/enr_test.go @@ -4,8 +4,10 @@ import ( "encoding/binary" "errors" "math" + "math/rand" "net" "testing" + "time" gcrypto "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/p2p/enode" @@ -25,6 +27,33 @@ func TestEnodeToMultiAddr(t *testing.T) { require.Equal(t, expectedMultiAddr, actualMultiAddr.String()) } +func writeMultiaddressField(localnode *enode.LocalNode, addrAggr []ma.Multiaddr) (err error) { + defer func() { + if e := recover(); e != nil { + err = errors.New("could not write enr record") + } + }() + + var fieldRaw []byte + for _, addr := range addrAggr { + maRaw := addr.Bytes() + maSize := make([]byte, 2) + binary.BigEndian.PutUint16(maSize, uint16(len(maRaw))) + + fieldRaw = append(fieldRaw, maSize...) + fieldRaw = append(fieldRaw, maRaw...) + } + + if len(fieldRaw) != 0 { + localnode.Set(enr.WithEntry(MultiaddrENRField, fieldRaw)) + } + + // This is to trigger the signing record err due to exceeding 300bytes limit + _ = localnode.Node() + + return nil +} + // TODO: this function is duplicated in localnode.go. Remove duplication func updateLocalNode(localnode *enode.LocalNode, multiaddrs []ma.Multiaddr, ipAddr *net.TCPAddr, udpPort uint, wakuFlags WakuEnrBitfield, advertiseAddr *net.IP, shouldAutoUpdate bool, log *zap.Logger) error { localnode.SetFallbackUDP(int(udpPort)) @@ -69,45 +98,37 @@ func updateLocalNode(localnode *enode.LocalNode, multiaddrs []ma.Multiaddr, ipAd } } - var addrAggr []ma.Multiaddr + // Randomly shuffle multiaddresses + rand.Seed(time.Now().UnixNano()) + rand.Shuffle(len(multiaddrs), func(i, j int) { multiaddrs[i], multiaddrs[j] = multiaddrs[j], multiaddrs[i] }) + + // Adding a single extra multiaddress. Should probably not exceed the enr max size of 300bytes var err error + failedOnceWritingENR := false + couldWriteENRatLeastOnce := false + successIdx := -1 for i := len(multiaddrs) - 1; i >= 0; i-- { - addrAggr = append(addrAggr, multiaddrs[0:i]...) - err = func() (err error) { - defer func() { - if e := recover(); e != nil { - err = errors.New("could not write enr record") - } - }() - - var fieldRaw []byte - for _, addr := range addrAggr { - maRaw := addr.Bytes() - maSize := make([]byte, 2) - binary.BigEndian.PutUint16(maSize, uint16(len(maRaw))) - - fieldRaw = append(fieldRaw, maSize...) - fieldRaw = append(fieldRaw, maRaw...) - } - - if len(fieldRaw) != 0 { - localnode.Set(enr.WithEntry(MultiaddrENRField, fieldRaw)) - } - - // This is to trigger the signing record err due to exceeding 300bytes limit - _ = localnode.Node() - - return nil - }() - + err = writeMultiaddressField(localnode, multiaddrs[0:i]) if err == nil { + couldWriteENRatLeastOnce = true + successIdx = i break + } else { + failedOnceWritingENR = true } } - // In case multiaddr could not be populated at all - if err != nil { - localnode.Delete(enr.WithEntry(MultiaddrENRField, struct{}{})) + if failedOnceWritingENR { + if !couldWriteENRatLeastOnce { + // In case multiaddr could not be populated at all + localnode.Delete(enr.WithEntry(MultiaddrENRField, struct{}{})) + } else { + // Could write a subset of multiaddresses but not all + err = writeMultiaddressField(localnode, multiaddrs[0:successIdx]) + if err != nil { + return errors.New("could not write new ENR") + } + } } return nil