diff --git a/chain/gssmr/config.toml b/chain/gssmr/config.toml index d75a4b8acc..80301de7d2 100644 --- a/chain/gssmr/config.toml +++ b/chain/gssmr/config.toml @@ -32,6 +32,7 @@ nobootstrap = false nomdns = false discovery-interval = 10 min-peers = 1 +max-peers = 50 [rpc] enabled = false diff --git a/chain/gssmr/defaults.go b/chain/gssmr/defaults.go index ac93168cd8..806a86f5b9 100644 --- a/chain/gssmr/defaults.go +++ b/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 diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index 4a5f290ef7..929402f1fc 100644 --- a/cmd/gossamer/config.go +++ b/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 { diff --git a/cmd/gossamer/config_test.go b/cmd/gossamer/config_test.go index 5161cbdeab..d468641a32 100644 --- a/cmd/gossamer/config_test.go +++ b/cmd/gossamer/config_test.go @@ -458,6 +458,7 @@ func TestNetworkConfigFromFlags(t *testing.T) { NoMDNS: testCfg.Network.NoMDNS, DiscoveryInterval: time.Second * 10, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, }, { @@ -472,6 +473,7 @@ func TestNetworkConfigFromFlags(t *testing.T) { NoMDNS: testCfg.Network.NoMDNS, DiscoveryInterval: time.Second * 10, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, }, { @@ -486,6 +488,7 @@ func TestNetworkConfigFromFlags(t *testing.T) { NoMDNS: testCfg.Network.NoMDNS, DiscoveryInterval: time.Second * 10, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, }, { @@ -500,6 +503,7 @@ func TestNetworkConfigFromFlags(t *testing.T) { NoMDNS: testCfg.Network.NoMDNS, DiscoveryInterval: time.Second * 10, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, }, { @@ -514,6 +518,7 @@ func TestNetworkConfigFromFlags(t *testing.T) { NoMDNS: true, DiscoveryInterval: time.Second * 10, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, }, { @@ -528,6 +533,7 @@ func TestNetworkConfigFromFlags(t *testing.T) { NoMDNS: false, DiscoveryInterval: time.Second * 10, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, PublicIP: "10.0.5.2", }, }, @@ -909,6 +915,7 @@ func TestUpdateConfigFromGenesisData(t *testing.T) { NoMDNS: testCfg.Network.NoMDNS, DiscoveryInterval: testCfg.Network.DiscoveryInterval, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, RPC: testCfg.RPC, System: testCfg.System, diff --git a/cmd/gossamer/export_test.go b/cmd/gossamer/export_test.go index 190ec72900..a1e8f0955b 100644 --- a/cmd/gossamer/export_test.go +++ b/cmd/gossamer/export_test.go @@ -75,6 +75,7 @@ func TestExportCommand(t *testing.T) { NoMDNS: testCfg.Network.NoMDNS, DiscoveryInterval: testCfg.Network.DiscoveryInterval, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, RPC: testCfg.RPC, Pprof: testCfg.Pprof, @@ -112,6 +113,7 @@ func TestExportCommand(t *testing.T) { NoMDNS: testCfg.Network.NoMDNS, DiscoveryInterval: testCfg.Network.DiscoveryInterval, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, RPC: testCfg.RPC, Pprof: testCfg.Pprof, @@ -149,6 +151,7 @@ func TestExportCommand(t *testing.T) { NoMDNS: testCfg.Network.NoMDNS, DiscoveryInterval: testCfg.Network.DiscoveryInterval, MinPeers: testCfg.Network.MinPeers, + MaxPeers: testCfg.Network.MaxPeers, }, RPC: testCfg.RPC, Pprof: testCfg.Pprof, diff --git a/dot/config.go b/dot/config.go index 3e937d0d3a..43072dac56 100644 --- a/dot/config.go +++ b/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, diff --git a/dot/network/connmgr_test.go b/dot/network/connmgr_test.go index eedfdc7209..f67a99f77a 100644 --- a/dot/network/connmgr_test.go +++ b/dot/network/connmgr_test.go @@ -43,10 +43,15 @@ func TestMinPeers(t *testing.T) { } nodeB := createTestService(t, configB) - require.Equal(t, min, nodeB.host.peerCount()) + require.GreaterOrEqual(t, nodeB.host.peerCount(), len(nodes)) - nodeB.host.cm.peerSetHandler.DisconnectPeer(0, nodes[0].host.id()) - require.GreaterOrEqual(t, min, nodeB.host.peerCount()) + // check that peer count is at least greater than minimum number of peers, + // even after trying to disconnect from all peers + for _, node := range nodes { + nodeB.host.cm.peerSetHandler.DisconnectPeer(0, node.host.id()) + } + + require.GreaterOrEqual(t, nodeB.host.peerCount(), min) } func TestMaxPeers(t *testing.T) { diff --git a/dot/network/discovery.go b/dot/network/discovery.go index 0a8e42c045..a1e54ddbb4 100644 --- a/dot/network/discovery.go +++ b/dot/network/discovery.go @@ -152,7 +152,7 @@ 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) + logger.Warnf("failed to advertise in the DHT: %s", err) ttl = tryAdvertiseTimeout } case <-d.ctx.Done(): @@ -199,21 +199,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) - } } } } diff --git a/dot/network/host.go b/dot/network/host.go index bd659e3e63..6662c48833 100644 --- a/dot/network/host.go +++ b/dot/network/host.go @@ -98,12 +98,16 @@ func newHost(ctx context.Context, cfg *Config) (*host, error) { return nil, err } + // We have tried to set maxInPeers and maxOutPeers such that number of peer + // connections remain between min peers and max peers 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 { diff --git a/dot/network/service.go b/dot/network/service.go index d66e00594c..f6ad9d0727 100644 --- a/dot/network/service.go +++ b/dot/network/service.go @@ -675,21 +675,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) diff --git a/dot/peerset/peerset.go b/dot/peerset/peerset.go index 8f1675897e..ac27b52c7b 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -53,6 +53,33 @@ const ( disconnect ) +func (a ActionReceiver) String() string { + switch a { + case addReservedPeer: + return "addReservedPeer" + case removeReservedPeer: + return "removeReservedPeer" + case setReservedPeers: + return "setReservedPeers" + case setReservedOnly: + return "setReservedOnly" + case reportPeer: + return "reportPeer" + case addToPeerSet: + return "addToPeerSet" + case removeFromPeerSet: + return "removeFromPeerSet" + case incoming: + return "incoming" + case sortedPeers: + return "sortedPeers" + case disconnect: + return "disconnect" + default: + return "invalid action" + } +} + // action struct stores the action type and required parameters to perform action type action struct { actionCall ActionReceiver @@ -67,8 +94,8 @@ func (a action) String() string { for i := range a.peers { peersStrings[i] = a.peers[i].String() } - return fmt.Sprintf("{call=%d, set-id=%d, reputation change %v, peers=[%s]", - a.actionCall, a.setID, a.reputation, strings.Join(peersStrings, ", ")) + return fmt.Sprintf("{call=%s, set-id=%d, reputation change %v, peers=[%s]", + a.actionCall.String(), a.setID, a.reputation, strings.Join(peersStrings, ", ")) } // Status represents the enum value for Message @@ -156,9 +183,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 +201,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, } @@ -351,6 +378,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 } @@ -364,6 +393,7 @@ func (ps *PeerSet) allocSlots(setIdx int) error { PeerID: reservePeer, } } + // nothing more to do if we're in reserved mode. if ps.isReservedOnly { return nil @@ -382,6 +412,7 @@ func (ps *PeerSet) allocSlots(setIdx int) error { } if err = peerState.tryOutgoing(setIdx, peerID); err != nil { + logger.Errorf("could not set peer %s as outgoing connection: %s", peerID.Pretty(), err) break } @@ -403,10 +434,14 @@ func (ps *PeerSet) addReservedPeers(setID int, peers ...peer.ID) error { return nil } + ps.peerState.discover(setID, peerID) + ps.reservedNode[peerID] = struct{}{} - ps.peerState.addNoSlotNode(setID, peerID) + if err := ps.peerState.addNoSlotNode(setID, peerID); err != nil { + return fmt.Errorf("could not add to list of no-slot nodes: %w", err) + } if err := ps.allocSlots(setID); err != nil { - return err + return fmt.Errorf("could not allocate slots: %w", err) } } return nil @@ -420,7 +455,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 fmt.Errorf("could not remove from the list of no-slot nodes: %w", err) + } // nothing more to do if not in reservedOnly mode. if !ps.isReservedOnly { @@ -645,7 +682,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: diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index 6929462bad..16acfab4a3 100644 --- a/dot/peerset/peerstate.go +++ b/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 + // state is a list of sets containing the node. + // always has a fixed size, equal to the one of PeersState Set. The various possible Set // are indices into this Set. state []MembershipState @@ -70,7 +71,7 @@ type node struct { rep Reputation } -// newNode method to create a node with 0 Reputation at starting. +// newNode creates a node with n number of sets and 0 reputation. func newNode(n int) *node { now := time.Now() sets := make([]MembershipState, n) @@ -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 for peer id %s: %w", peerID, 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 for peer id %s: %w", peerID, 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. +// If the slots are full, the node stays "not connected" and we return the error ErrOutgoingSlotsUnavailable. // 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++ } diff --git a/dot/peerset/peerstate_test.go b/dot/peerset/peerstate_test.go index 29afc25755..3b58544f38 100644 --- a/dot/peerset/peerstate_test.go +++ b/dot/peerset/peerstate_test.go @@ -42,15 +42,18 @@ func TestNoSlotNodeDoesntOccupySlot(t *testing.T) { t.Parallel() state := newTestPeerState(t, 1, 1) + state.nodes[peer1] = newNode(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 +119,14 @@ func TestDisconnectNoSlotDoesntPanic(t *testing.T) { state := newTestPeerState(t, 1, 1) - state.addNoSlotNode(0, peer1) + state.nodes[peer1] = newNode(1) + 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)) @@ -197,11 +202,13 @@ func TestSortedPeers(t *testing.T) { const msgChanSize = 1 state := newTestPeerState(t, 2, 1) + state.nodes[peer1] = newNode(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)) diff --git a/dot/peerset/test_helpers.go b/dot/peerset/test_helpers.go index 1604f7623c..f8f68191d5 100644 --- a/dot/peerset/test_helpers.go +++ b/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)