fix(dot/network): fix dht connection on discovery on devnet

chain/gssmr/defaults.go

@@ -74,6 +74,8 @@ var (
 	DefaultNoMDNS = false
 	// DefaultMinPeers is the default minimum desired peer count
 	DefaultMinPeers = 1
+	// DefaultMaxPeers is the default maximum desired peer count
+	DefaultMaxPeers = 50
 
 	// DefaultDiscoveryInterval is the default interval for searching for DHT peers
 	DefaultDiscoveryInterval = time.Second * 10

cmd/gossamer/config.go

@@ -121,7 +121,9 @@ func createDotConfig(ctx *cli.Context) (*dot.Config, error) {
 		return nil, err
 	}
 
-	logger.Infof("loaded package log configuration: %v", cfg.Log)
+	// TODO: log this better.
+	// See https://github.com/ChainSafe/gossamer/issues/1945
+	logger.Infof("loaded package log configuration: %#v", cfg.Log)
 
 	// set global configuration values
 	if err := setDotGlobalConfig(ctx, tomlCfg, &cfg.Global); err != nil {

dot/config.go

@@ -207,6 +207,7 @@ func GssmrConfig() *Config {
 			NoMDNS:            gssmr.DefaultNoMDNS,
 			DiscoveryInterval: gssmr.DefaultDiscoveryInterval,
 			MinPeers:          gssmr.DefaultMinPeers,
+			MaxPeers:          gssmr.DefaultMaxPeers,
 		},
 		RPC: RPCConfig{
 			Port:    gssmr.DefaultRPCHTTPPort,

dot/network/discovery.go

@@ -152,7 +152,8 @@ func (d *discovery) advertise() {
 
 			ttl, err = d.rd.Advertise(d.ctx, string(d.pid))
 			if err != nil {
-				logger.Debugf("failed to advertise in the DHT: %s", err)
+				// TODO: This fails consistently.
+				logger.Warnf("failed to advertise in the DHT: %s", err)
 				ttl = tryAdvertiseTimeout
 			}
 		case <-d.ctx.Done():
@@ -199,21 +200,9 @@ func (d *discovery) findPeers(ctx context.Context) {
 
 			logger.Tracef("found new peer %s via DHT", peer.ID)
 
-			// TODO: this isn't working on the devnet (#2026)
-			// can remove the code block below which directly connects
-			// once that's fixed
 			d.h.Peerstore().AddAddrs(peer.ID, peer.Addrs, peerstore.PermanentAddrTTL)
 			d.handler.AddPeer(0, peer.ID)
 
-			// found a peer, try to connect if we need more peers
-			if len(d.h.Network().Peers()) >= d.maxPeers {
-				d.h.Peerstore().AddAddrs(peer.ID, peer.Addrs, peerstore.PermanentAddrTTL)
-				return
-			}
-
-			if err = d.h.Connect(d.ctx, peer); err != nil {
-				logger.Tracef("failed to connect to discovered peer %s: %s", peer.ID, err)
-			}
 		}
 	}
 }

dot/network/host.go

@@ -98,12 +98,15 @@ func newHost(ctx context.Context, cfg *Config) (*host, error) {
 		return nil, err
 	}
 
+	// TODO: What should be the right value of max in and max out?
 	const reservedOnly = false
 	peerCfgSet := peerset.NewConfigSet(
 		uint32(cfg.MaxPeers-cfg.MinPeers),
-		uint32(cfg.MinPeers),
+		uint32(cfg.MaxPeers/2),
 		reservedOnly,
-		peerSetSlotAllocTime)
+		peerSetSlotAllocTime,
+	)
+
 	// create connection manager
 	cm, err := newConnManager(cfg.MinPeers, cfg.MaxPeers, peerCfgSet)
 	if err != nil {

dot/network/service.go

@@ -676,21 +676,21 @@ func (s *Service) processMessage(msg peerset.Message) {
 			var err error
 			addrInfo, err = s.host.discovery.findPeer(peerID)
 			if err != nil {
-				logger.Debugf("failed to find peer id %s: %s", peerID, err)
+				logger.Warnf("failed to find peer id %s: %s", peerID, err)
 				return
 			}
 		}
 
 		err := s.host.connect(addrInfo)
 		if err != nil {
-			logger.Debugf("failed to open connection for peer %s: %s", peerID, err)
+			logger.Warnf("failed to open connection for peer %s: %s", peerID, err)
 			return
 		}
 		logger.Debugf("connection successful with peer %s", peerID)
 	case peerset.Drop, peerset.Reject:
 		err := s.host.closePeer(peerID)
 		if err != nil {
-			logger.Debugf("failed to close connection with peer %s: %s", peerID, err)
+			logger.Warnf("failed to close connection with peer %s: %s", peerID, err)
 			return
 		}
 		logger.Debugf("connection dropped successfully for peer %s", peerID)

dot/peerset/handler.go

@@ -3,7 +3,9 @@
 
 package peerset
 
-import "github.com/libp2p/go-libp2p-core/peer"
+import (
+	"github.com/libp2p/go-libp2p-core/peer"
+)
 
 // Handler manages peerSet.
 type Handler struct {

dot/peerset/peerset.go

@@ -156,9 +156,9 @@ type PeerSet struct {
 // config is configuration of a single set.
 type config struct {
 	// maximum number of slot occupying nodes for incoming connections.
-	inPeers uint32
+	maxInPeers uint32
 	// maximum number of slot occupying nodes for outgoing connections.
-	outPeers uint32
+	maxOutPeers uint32
 
 	// TODO Use in future for reserved only peers
 	// if true, we only accept reservedNodes (#1888).
@@ -174,10 +174,10 @@ type ConfigSet struct {
 }
 
 // NewConfigSet creates a new config set for the peerSet
-func NewConfigSet(in, out uint32, reservedOnly bool, allocTime time.Duration) *ConfigSet {
+func NewConfigSet(maxInPeers, maxOutPeers uint32, reservedOnly bool, allocTime time.Duration) *ConfigSet {
 	set := &config{
-		inPeers:           in,
-		outPeers:          out,
+		maxInPeers:        maxInPeers,
+		maxOutPeers:       maxOutPeers,
 		reservedOnly:      reservedOnly,
 		periodicAllocTime: allocTime,
 	}
@@ -349,6 +349,8 @@ func (ps *PeerSet) allocSlots(setIdx int) error {
 		}
 
 		if n.getReputation() < BannedThresholdValue {
+			logger.Warnf("reputation is lower than banned threshold value, reputation: %d, banned threshold value: %d",
+				n.getReputation(), BannedThresholdValue)
 			break
 		}
 
@@ -362,12 +364,14 @@ func (ps *PeerSet) allocSlots(setIdx int) error {
 			PeerID: reservePeer,
 		}
 	}
+
 	// nothing more to do if we're in reserved mode.
 	if ps.isReservedOnly {
 		return nil
 	}
 
 	for peerState.hasFreeOutgoingSlot(setIdx) {
+
 		peerID := peerState.highestNotConnectedPeer(setIdx)
 		if peerID == "" {
 			break
@@ -380,6 +384,7 @@ func (ps *PeerSet) allocSlots(setIdx int) error {
 		}
 
 		if err = peerState.tryOutgoing(setIdx, peerID); err != nil {
+			logger.Errorf("could not set peer as outgoing connection, peer: %s, error: %s", peerID.Pretty(), err)
 			break
 		}
 
@@ -402,7 +407,9 @@ func (ps *PeerSet) addReservedPeers(setID int, peers ...peer.ID) error {
 		}
 
 		ps.reservedNode[peerID] = struct{}{}
-		ps.peerState.addNoSlotNode(setID, peerID)
+		if err := ps.peerState.addNoSlotNode(setID, peerID); err != nil {
+			return err
+		}
 		if err := ps.allocSlots(setID); err != nil {
 			return err
 		}
@@ -418,7 +425,9 @@ func (ps *PeerSet) removeReservedPeers(setID int, peers ...peer.ID) error {
 		}
 
 		delete(ps.reservedNode, peerID)
-		ps.peerState.removeNoSlotNode(setID, peerID)
+		if err := ps.peerState.removeNoSlotNode(setID, peerID); err != nil {
+			return err
+		}
 
 		// nothing more to do if not in reservedOnly mode.
 		if !ps.isReservedOnly {
@@ -624,7 +633,7 @@ func (ps *PeerSet) doWork() {
 			l := ps.peerState.getSetLength()
 			for i := 0; i < l; i++ {
 				if err := ps.allocSlots(i); err != nil {
-					logger.Debugf("failed to do action on peerSet: %s", err)
+					logger.Warnf("failed to do action on peerSet: %s", err)
 				}
 			}
 		case act, ok := <-ps.actionQueue:

dot/peerset/peerstate.go

@@ -4,6 +4,7 @@
 package peerset
 
 import (
+	"fmt"
 	"math"
 	"sort"
 	"time"
@@ -39,7 +40,7 @@ type Info struct {
 	// number of slot occupying nodes for which the MembershipState is ingoing.
 	numIn uint32
 
-	// number of slot occupying nodes for which the MembershipState is ingoing.
+	// number of slot occupying nodes for which the MembershipState is outgoing.
 	numOut uint32
 
 	// maximum allowed number of slot occupying nodes for which the MembershipState is ingoing.
@@ -57,8 +58,8 @@ type Info struct {
 
 // node represents state of a single node that we know about
 type node struct {
-	// list of Set the node belongs to.
-	// always has a fixed size equal to the one of PeersState Set. The various possible Set
+	// list of Set, the node belongs to.
+	// always has a fixed size, equal to the one of PeersState Set. The various possible Set
 	// are indices into this Set.
 	state []MembershipState
 
@@ -127,8 +128,8 @@ func NewPeerState(cfgs []*config) (*PeersState, error) {
 		info := Info{
 			numIn:       0,
 			numOut:      0,
-			maxIn:       cfg.inPeers,
-			maxOut:      cfg.outPeers,
+			maxIn:       cfg.maxInPeers,
+			maxOut:      cfg.maxOutPeers,
 			noSlotNodes: make(map[peer.ID]struct{}),
 		}
 
@@ -237,17 +238,17 @@ func (ps *PeersState) hasFreeIncomingSlot(set int) bool {
 
 // addNoSlotNode adds a node to the list of nodes that don't occupy slots.
 // has no effect if the node was already in the group.
-func (ps *PeersState) addNoSlotNode(idx int, peerID peer.ID) {
+func (ps *PeersState) addNoSlotNode(idx int, peerID peer.ID) error {
 	if _, ok := ps.sets[idx].noSlotNodes[peerID]; ok {
 		logger.Debugf("peer %s already exists in no slot node", peerID)
-		return
+		return nil
 	}
 
 	// Insert peerStatus
 	ps.sets[idx].noSlotNodes[peerID] = struct{}{}
 	n, err := ps.getNode(peerID)
 	if err != nil {
-		return
+		return fmt.Errorf("could not get node: %w", err)
 	}
 
 	switch n.state[idx] {
@@ -258,17 +259,19 @@ func (ps *PeersState) addNoSlotNode(idx int, peerID peer.ID) {
 	}
 
 	ps.nodes[peerID] = n
+	return nil
 }
 
-func (ps *PeersState) removeNoSlotNode(idx int, peerID peer.ID) {
+func (ps *PeersState) removeNoSlotNode(idx int, peerID peer.ID) error {
 	if _, ok := ps.sets[idx].noSlotNodes[peerID]; !ok {
-		return
+		logger.Debugf("peer %s is not in \"no slot node\" map", peerID)
+		return nil
 	}
 
 	delete(ps.sets[idx].noSlotNodes, peerID)
 	n, err := ps.getNode(peerID)
 	if err != nil {
-		return
+		return fmt.Errorf("could not get node: %w", err)
 	}
 
 	switch n.state[idx] {
@@ -277,6 +280,7 @@ func (ps *PeersState) removeNoSlotNode(idx int, peerID peer.ID) {
 	case outgoing:
 		ps.sets[idx].numOut++
 	}
+	return nil
 }
 
 // disconnect updates the node status to the notConnected state.
@@ -356,16 +360,13 @@ func (ps *PeersState) forgetPeer(set int, peerID peer.ID) error {
 }
 
 // tryOutgoing tries to set the peer as connected as an outgoing connection.
-// If there are enough slots available, switches the node to Connected and returns nil error. If
-// the slots are full, the node stays "not connected" and we return error.
+// If there are enough slots available, switches the node to Connected and returns
+// nil error. If the slots are full, the node stays "not connected" and we return error.
 // non slot occupying nodes don't count towards the number of slots.
 func (ps *PeersState) tryOutgoing(setID int, peerID peer.ID) error {
-	var isNoSlotOccupied bool
-	if _, ok := ps.sets[setID].noSlotNodes[peerID]; ok {
-		isNoSlotOccupied = true
-	}
+	_, isNoSlotNode := ps.sets[setID].noSlotNodes[peerID]
 
-	if !ps.hasFreeOutgoingSlot(setID) && !isNoSlotOccupied {
+	if !ps.hasFreeOutgoingSlot(setID) && !isNoSlotNode {
 		return ErrOutgoingSlotsUnavailable
 	}
 
@@ -375,7 +376,7 @@ func (ps *PeersState) tryOutgoing(setID int, peerID peer.ID) error {
 	}
 
 	n.state[setID] = outgoing
-	if !isNoSlotOccupied {
+	if !isNoSlotNode {
 		ps.sets[setID].numOut++
 	}
 

dot/peerset/peerstate_test.go

@@ -44,13 +44,15 @@ func TestNoSlotNodeDoesntOccupySlot(t *testing.T) {
 	state := newTestPeerState(t, 1, 1)
 
 	// peer1 will not occupy any slot.
-	state.addNoSlotNode(0, peer1)
+	err := state.addNoSlotNode(0, peer1)
+	require.NoError(t, err)
+
 	// initially peer1 state will be unknownPeer.
 	require.Equal(t, unknownPeer, state.peerStatus(0, peer1))
 	// discover peer1
 	state.discover(0, peer1)
 	// peer1 will become an incoming connection.
-	err := state.tryAcceptIncoming(0, peer1)
+	err = state.tryAcceptIncoming(0, peer1)
 	require.NoError(t, err)
 	// peer1 is connected
 	require.Equal(t, connectedPeer, state.peerStatus(0, peer1))
@@ -116,12 +118,13 @@ func TestDisconnectNoSlotDoesntPanic(t *testing.T) {
 
 	state := newTestPeerState(t, 1, 1)
 
-	state.addNoSlotNode(0, peer1)
+	err := state.addNoSlotNode(0, peer1)
+	require.NoError(t, err)
 
 	require.Equal(t, unknownPeer, state.peerStatus(0, peer1))
 
 	state.discover(0, peer1)
-	err := state.tryOutgoing(0, peer1)
+	err = state.tryOutgoing(0, peer1)
 	require.NoError(t, err)
 
 	require.Equal(t, connectedPeer, state.peerStatus(0, peer1))
@@ -198,10 +201,11 @@ func TestSortedPeers(t *testing.T) {
 	const msgChanSize = 1
 	state := newTestPeerState(t, 2, 1)
 
-	state.addNoSlotNode(0, peer1)
+	err := state.addNoSlotNode(0, peer1)
+	require.NoError(t, err)
 
 	state.discover(0, peer1)
-	err := state.tryAcceptIncoming(0, peer1)
+	err = state.tryAcceptIncoming(0, peer1)
 	require.NoError(t, err)
 
 	require.Equal(t, connectedPeer, state.peerStatus(0, peer1))

dot/peerset/test_helpers.go

@@ -29,8 +29,8 @@ func newTestPeerSet(t *testing.T, in, out uint32, bootNodes, reservedPeers []pee
 	con := &ConfigSet{
 		Set: []*config{
 			{
-				inPeers:           in,
-				outPeers:          out,
+				maxInPeers:        in,
+				maxOutPeers:       out,
 				reservedOnly:      reservedOnly,
 				periodicAllocTime: time.Second * 2,
 			},
@@ -53,8 +53,8 @@ func newTestPeerState(t *testing.T, maxIn, maxOut uint32) *PeersState {
 	t.Helper()
 	state, err := NewPeerState([]*config{
 		{
-			inPeers:  maxIn,
-			outPeers: maxOut,
+			maxInPeers:  maxIn,
+			maxOutPeers: maxOut,
 		},
 	})
 	require.NoError(t, err)