Skip to content

Commit 93331b4

Browse files
richard-ramosrichard-ramos
authored
fix: do not write tcp port 0 and remove 100 chars length limit for multiaddresses (#1129)
1 parent 795322a commit 93331b4

File tree

8 files changed

+108
-48
lines changed

8 files changed

+108
-48
lines changed

‎Makefile

+3-3
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,15 @@ vendor:
6666

6767
lint-install:
6868
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | \
69-
bash -s -- -b $(shell ${GOCMD} env GOPATH)/bin v1.52.2
69+
bash -s -- -b $(shell ${GOCMD} env GOPATH)/bin v1.59.1
7070

7171
lint:
7272
@echo "lint"
73-
@golangci-lint run ./... --deadline=5m
73+
@golangci-lint run ./...
7474

7575
lint-full:
7676
@echo "lint"
77-
@golangci-lint run ./... --config=./.golangci.full.yaml --deadline=5m
77+
@golangci-lint run ./... --config=./.golangci.full.yaml
7878

7979
test-with-race:
8080
${GOCMD} test -race -timeout 300s ./waku/... ./cmd/waku/server/...

‎waku/v2/discv5/discover_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ package discv5
22

33
import (
44
"context"
5+
"testing"
6+
"time"
7+
58
dto "github.com/prometheus/client_model/go"
69
wps "github.com/waku-org/go-waku/waku/v2/peerstore"
710
wakuproto "github.com/waku-org/go-waku/waku/v2/protocol"
811
"github.com/waku-org/go-waku/waku/v2/service"
912
"go.uber.org/zap"
10-
"testing"
11-
"time"
1213

1314
"github.com/ethereum/go-ethereum/p2p/enode"
1415
"github.com/prometheus/client_golang/prometheus"
@@ -255,7 +256,7 @@ func TestDiscV5WithShardFilter(t *testing.T) {
255256
require.NoError(t, err)
256257

257258
// Update node with shard info
258-
err = wenr.Update(l1, wenr.WithWakuRelaySharding(rs1[0]))
259+
err = wenr.Update(utils.Logger(), l1, wenr.WithWakuRelaySharding(rs1[0]))
259260
require.NoError(t, err)
260261

261262
// H2
@@ -271,7 +272,7 @@ func TestDiscV5WithShardFilter(t *testing.T) {
271272
d2.SetHost(host2)
272273

273274
// Update second node with shard info used for first node as well
274-
err = wenr.Update(l2, wenr.WithWakuRelaySharding(rs1[0]))
275+
err = wenr.Update(utils.Logger(), l2, wenr.WithWakuRelaySharding(rs1[0]))
275276
require.NoError(t, err)
276277

277278
// H3

‎waku/v2/node/localnode.go

+56-25
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/ethereum/go-ethereum/p2p/enode"
1010
"github.com/ethereum/go-ethereum/p2p/enr"
1111
"github.com/libp2p/go-libp2p/core/event"
12+
"github.com/multiformats/go-multiaddr"
1213
ma "github.com/multiformats/go-multiaddr"
1314
"github.com/waku-org/go-waku/waku/v2/protocol"
1415
wenr "github.com/waku-org/go-waku/waku/v2/protocol/enr"
@@ -20,7 +21,6 @@ func (w *WakuNode) updateLocalNode(localnode *enode.LocalNode, multiaddrs []ma.M
2021
var options []wenr.ENROption
2122
options = append(options, wenr.WithUDPPort(udpPort))
2223
options = append(options, wenr.WithWakuBitfield(wakuFlags))
23-
options = append(options, wenr.WithMultiaddress(multiaddrs...))
2424

2525
if advertiseAddr != nil {
2626
// An advertised address disables libp2p address updates
@@ -36,32 +36,38 @@ func (w *WakuNode) updateLocalNode(localnode *enode.LocalNode, multiaddrs []ma.M
3636
// Using a static ip will disable endpoint prediction.
3737
options = append(options, wenr.WithIP(ipAddr))
3838
} else {
39-
// We received a libp2p address update, but we should still
40-
// allow discv5 to update the enr record. We set the localnode
41-
// keys manually. It's possible that the ENR record might get
42-
// updated automatically
43-
ip4 := ipAddr.IP.To4()
44-
ip6 := ipAddr.IP.To16()
45-
if ip4 != nil && !ip4.IsUnspecified() {
46-
localnode.SetFallbackIP(ip4)
47-
localnode.Set(enr.IPv4(ip4))
48-
localnode.Set(enr.TCP(uint16(ipAddr.Port)))
49-
} else {
50-
localnode.Delete(enr.IPv4{})
51-
localnode.Delete(enr.TCP(0))
52-
localnode.SetFallbackIP(net.IP{127, 0, 0, 1})
53-
}
39+
if ipAddr.Port != 0 {
40+
// We received a libp2p address update, but we should still
41+
// allow discv5 to update the enr record. We set the localnode
42+
// keys manually. It's possible that the ENR record might get
43+
// updated automatically
44+
ip4 := ipAddr.IP.To4()
45+
ip6 := ipAddr.IP.To16()
46+
if ip4 != nil && !ip4.IsUnspecified() {
47+
localnode.SetFallbackIP(ip4)
48+
localnode.Set(enr.IPv4(ip4))
49+
localnode.Set(enr.TCP(uint16(ipAddr.Port)))
50+
} else {
51+
localnode.Delete(enr.IPv4{})
52+
localnode.Delete(enr.TCP(0))
53+
localnode.SetFallbackIP(net.IP{127, 0, 0, 1})
54+
}
5455

55-
if ip4 == nil && ip6 != nil && !ip6.IsUnspecified() {
56-
localnode.Set(enr.IPv6(ip6))
57-
localnode.Set(enr.TCP6(ipAddr.Port))
58-
} else {
59-
localnode.Delete(enr.IPv6{})
60-
localnode.Delete(enr.TCP6(0))
56+
if ip4 == nil && ip6 != nil && !ip6.IsUnspecified() {
57+
localnode.Set(enr.IPv6(ip6))
58+
localnode.Set(enr.TCP6(ipAddr.Port))
59+
} else {
60+
localnode.Delete(enr.IPv6{})
61+
localnode.Delete(enr.TCP6(0))
62+
}
6163
}
6264
}
6365

64-
return wenr.Update(localnode, options...)
66+
// Writing the IP + Port has priority over writting the multiaddress which might fail or not
67+
// depending on the enr having space
68+
options = append(options, wenr.WithMultiaddress(multiaddrs...))
69+
70+
return wenr.Update(w.log, localnode, options...)
6571
}
6672

6773
func isPrivate(addr *net.TCPAddr) bool {
@@ -228,8 +234,28 @@ func selectCircuitRelayListenAddresses(addresses []ma.Multiaddr) ([]ma.Multiaddr
228234
return result, nil
229235
}
230236

231-
func (w *WakuNode) getENRAddresses(addrs []ma.Multiaddr) (extAddr *net.TCPAddr, multiaddr []ma.Multiaddr, err error) {
237+
func filter0Port(addresses []ma.Multiaddr) ([]ma.Multiaddr, error) {
238+
var result []ma.Multiaddr
239+
for _, addr := range addresses {
240+
portStr, err := addr.ValueForProtocol(ma.P_TCP)
241+
if err != nil && !errors.Is(err, multiaddr.ErrProtocolNotFound) {
242+
return nil, err
243+
}
232244

245+
port, err := strconv.Atoi(portStr)
246+
if err != nil {
247+
return nil, err
248+
}
249+
250+
if port != 0 {
251+
result = append(result, addr)
252+
}
253+
}
254+
255+
return result, nil
256+
}
257+
258+
func (w *WakuNode) getENRAddresses(addrs []ma.Multiaddr) (extAddr *net.TCPAddr, multiaddr []ma.Multiaddr, err error) {
233259
extAddr, err = selectMostExternalAddress(addrs)
234260
if err != nil {
235261
return nil, nil, err
@@ -253,6 +279,11 @@ func (w *WakuNode) getENRAddresses(addrs []ma.Multiaddr) (extAddr *net.TCPAddr,
253279
multiaddr = append(multiaddr, wssAddrs...)
254280
}
255281

282+
multiaddr, err = filter0Port(multiaddr)
283+
if err != nil {
284+
return nil, nil, err
285+
}
286+
256287
return
257288
}
258289

@@ -323,7 +354,7 @@ func (w *WakuNode) watchTopicShards(ctx context.Context) error {
323354
w.log.Warn("A mix of named and static shards found. ENR shard will contain only the following shards", zap.Any("shards", rs[0]))
324355
}
325356

326-
err = wenr.Update(w.localNode, wenr.WithWakuRelaySharding(rs[0]))
357+
err = wenr.Update(w.log, w.localNode, wenr.WithWakuRelaySharding(rs[0]))
327358
if err != nil {
328359
w.log.Warn("could not set ENR shard info", zap.Error(err))
329360
continue

‎waku/v2/peermanager/peer_manager_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ func createHostWithDiscv5AndPM(t *testing.T, hostName string, topic string, enrF
250250
rs, err := wakuproto.TopicsToRelayShards(topic)
251251
require.NoError(t, err)
252252

253-
err = wenr.Update(localNode, wenr.WithWakuRelaySharding(rs[0]))
253+
err = wenr.Update(utils.Logger(), localNode, wenr.WithWakuRelaySharding(rs[0]))
254254
require.NoError(t, err)
255255
pm := NewPeerManager(10, 20, nil, logger)
256256
pm.SetHost(host)

‎waku/v2/protocol/enr/enr.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"github.com/waku-org/go-waku/waku/v2/utils"
1313
)
1414

15+
var ErrNoPortAvailable = errors.New("port not available")
16+
1517
// WakuENRField is the name of the ENR field that contains information about which protocols are supported by the node
1618
const WakuENRField = "waku2"
1719

@@ -59,6 +61,11 @@ func enodeToMultiAddr(node *enode.Node) (multiaddr.Multiaddr, error) {
5961

6062
ipType := "ip4"
6163
portNumber := node.TCP()
64+
65+
if portNumber == 0 {
66+
return nil, ErrNoPortAvailable
67+
}
68+
6269
if utils.IsIPv6(node.IP().String()) {
6370
ipType = "ip6"
6471
var port enr.TCP6
@@ -83,9 +90,12 @@ func Multiaddress(node *enode.Node) (peer.ID, []multiaddr.Multiaddr, error) {
8390

8491
addr, err := enodeToMultiAddr(node)
8592
if err != nil {
86-
return "", nil, err
93+
if !errors.Is(err, ErrNoPortAvailable) {
94+
return "", nil, err
95+
}
96+
} else {
97+
result = append(result, addr)
8798
}
88-
result = append(result, addr)
8999

90100
var multiaddrRaw []byte
91101
if err := node.Record().Load(enr.WithEntry(MultiaddrENRField, &multiaddrRaw)); err != nil {

‎waku/v2/protocol/enr/enr_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/ethereum/go-ethereum/p2p/enr"
1010
ma "github.com/multiformats/go-multiaddr"
1111
"github.com/stretchr/testify/require"
12+
"github.com/waku-org/go-waku/waku/v2/utils"
1213
)
1314

1415
func TestEnodeToMultiAddr(t *testing.T) {
@@ -65,7 +66,7 @@ func updateLocalNode(localnode *enode.LocalNode, multiaddrs []ma.Multiaddr, ipAd
6566
}
6667
}
6768

68-
return Update(localnode, options...)
69+
return Update(utils.Logger(), localnode, options...)
6970
}
7071

7172
func TestMultiaddr(t *testing.T) {

‎waku/v2/protocol/enr/localnode.go

+12-5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/ethereum/go-ethereum/p2p/enode"
1212
"github.com/ethereum/go-ethereum/p2p/enr"
1313
"github.com/multiformats/go-multiaddr"
14+
"go.uber.org/zap"
1415
)
1516

1617
func NewLocalnode(priv *ecdsa.PrivateKey) (*enode.LocalNode, error) {
@@ -71,6 +72,10 @@ func WithWakuBitfield(flags WakuEnrBitfield) ENROption {
7172

7273
func WithIP(ipAddr *net.TCPAddr) ENROption {
7374
return func(localnode *enode.LocalNode) (err error) {
75+
if ipAddr.Port == 0 {
76+
return ErrNoPortAvailable
77+
}
78+
7479
localnode.SetStaticIP(ipAddr.IP)
7580
localnode.Set(enr.TCP(uint16(ipAddr.Port))) // TODO: ipv6?
7681
return nil
@@ -91,11 +96,15 @@ func WithUDPPort(udpPort uint) ENROption {
9196
}
9297
}
9398

94-
func Update(localnode *enode.LocalNode, enrOptions ...ENROption) error {
99+
func Update(logger *zap.Logger, localnode *enode.LocalNode, enrOptions ...ENROption) error {
95100
for _, opt := range enrOptions {
96101
err := opt(localnode)
97102
if err != nil {
98-
return err
103+
if errors.Is(err, ErrNoPortAvailable) {
104+
logger.Warn("no tcp port available. ENR will not contain tcp key")
105+
} else {
106+
return err
107+
}
99108
}
100109
}
101110
return nil
@@ -120,9 +129,7 @@ func writeMultiaddressField(localnode *enode.LocalNode, addrAggr []multiaddr.Mul
120129
fieldRaw = append(fieldRaw, maRaw...)
121130
}
122131

123-
if len(fieldRaw) != 0 && len(fieldRaw) <= 100 { // Max length for multiaddr field before triggering the 300 bytes limit
124-
localnode.Set(enr.WithEntry(MultiaddrENRField, fieldRaw))
125-
}
132+
localnode.Set(enr.WithEntry(MultiaddrENRField, fieldRaw))
126133

127134
// This is to trigger the signing record err due to exceeding 300bytes limit
128135
_ = localnode.Node()

‎waku/v2/service/common_discovery_service.go

+17-7
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@ type PeerData struct {
1818
}
1919

2020
type CommonDiscoveryService struct {
21-
commonService *CommonService
22-
channel chan PeerData
23-
canWriteToChannel sync.Mutex
21+
commonService *CommonService
22+
23+
channel chan PeerData
24+
channelIsClosed bool
25+
channelMutex sync.Mutex
2426
}
2527

2628
func NewCommonDiscoveryService() *CommonDiscoveryService {
@@ -33,7 +35,10 @@ func (sp *CommonDiscoveryService) Start(ctx context.Context, fn func() error) er
3335
return sp.commonService.Start(ctx, func() error {
3436
// currently is used in discv5,peerConnector,rendevzous for returning new discovered Peers to peerConnector for connecting with them
3537
// mutex protection for this operation
38+
sp.channelMutex.Lock()
3639
sp.channel = make(chan PeerData)
40+
sp.channelIsClosed = false
41+
sp.channelMutex.Unlock()
3742
return fn()
3843
})
3944
}
@@ -43,9 +48,10 @@ func (sp *CommonDiscoveryService) Stop(stopFn func()) {
4348
stopFn()
4449
sp.WaitGroup().Wait() // waitgroup is waited here so that channel can be closed after all the go rountines have stopped in service.
4550
// there is a wait in the CommonService too
46-
sp.canWriteToChannel.Lock()
51+
sp.channelMutex.Lock()
4752
close(sp.channel)
48-
sp.canWriteToChannel.Unlock()
53+
sp.channelIsClosed = true
54+
sp.channelMutex.Unlock()
4955
})
5056
}
5157
func (sp *CommonDiscoveryService) GetListeningChan() <-chan PeerData {
@@ -56,8 +62,12 @@ func (sp *CommonDiscoveryService) PushToChan(data PeerData) bool {
5662
return false
5763
}
5864

59-
sp.canWriteToChannel.Lock()
60-
defer sp.canWriteToChannel.Unlock()
65+
sp.channelMutex.Lock()
66+
defer sp.channelMutex.Unlock()
67+
68+
if sp.channelIsClosed {
69+
return false
70+
}
6171

6272
select {
6373
case sp.channel <- data:

0 commit comments

Comments
 (0)