From 74d1565e0f2b4bd5e67592e7966d7959028ba353 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Wed, 17 Nov 2021 17:47:08 +0530 Subject: [PATCH 01/13] testing --- cmd/gossamer/config.go | 3 +++ dot/config.go | 1 + dot/network/discovery.go | 16 +++++++++++++++- dot/network/host.go | 15 ++++++++++++++- dot/network/mdns.go | 4 ++-- dot/network/service.go | 13 ++++++++++--- dot/peerset/handler.go | 8 +++++++- dot/peerset/peerset.go | 30 ++++++++++++++++++++++++++---- dot/peerset/peerstate.go | 30 +++++++++++++++++------------- dot/peerset/test_helpers.go | 8 ++++---- 10 files changed, 99 insertions(+), 29 deletions(-) diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index c381499059..e5b57c8933 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -121,6 +121,9 @@ func createDotConfig(ctx *cli.Context) (*dot.Config, error) { return nil, err } + // TODO: log this better. Show package wise log levels. Currently + // it prints something like `{INFO INFO INFO INFO INFO INFO INFO INFO}` + // which is not informative. logger.Infof("loaded package log configuration: %v", cfg.Log) // set global configuration values diff --git a/dot/config.go b/dot/config.go index cb2a600397..2a9a8ab67e 100644 --- a/dot/config.go +++ b/dot/config.go @@ -190,6 +190,7 @@ func GssmrConfig() *Config { NoMDNS: gssmr.DefaultNoMDNS, DiscoveryInterval: gssmr.DefaultDiscoveryInterval, MinPeers: gssmr.DefaultMinPeers, + MaxPeers: 100, }, RPC: RPCConfig{ Port: gssmr.DefaultRPCHTTPPort, diff --git a/dot/network/discovery.go b/dot/network/discovery.go index 911f88f5b4..401830b9ca 100644 --- a/dot/network/discovery.go +++ b/dot/network/discovery.go @@ -150,7 +150,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(): @@ -197,6 +198,19 @@ 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 + // } + + fmt.Println("network/discovery findPeer 213") d.h.Peerstore().AddAddrs(peer.ID, peer.Addrs, peerstore.PermanentAddrTTL) d.handler.AddPeer(0, peer.ID) } diff --git a/dot/network/host.go b/dot/network/host.go index 6adba8e72b..e7af61e230 100644 --- a/dot/network/host.go +++ b/dot/network/host.go @@ -88,7 +88,9 @@ func newHost(ctx context.Context, cfg *Config) (*host, error) { return nil, err } - peerCfgSet := peerset.NewConfigSet(uint32(cfg.MaxPeers-cfg.MinPeers), uint32(cfg.MinPeers), false, peerSetSlotAllocTime) + peerCfgSet := peerset.NewConfigSet(2, 5, false, peerSetSlotAllocTime) + + // peerCfgSet := peerset.NewConfigSet(uint32(cfg.MaxPeers-cfg.MinPeers), uint32(cfg.MinPeers), false, peerSetSlotAllocTime) // create connection manager cm, err := newConnManager(cfg.MinPeers, cfg.MaxPeers, peerCfgSet) if err != nil { @@ -124,6 +126,7 @@ func newHost(ctx context.Context, cfg *Config) (*host, error) { // set libp2p host options opts := []libp2p.Option{ + libp2p.DefaultTransports, libp2p.ListenAddrs(addr), libp2p.DisableRelay(), libp2p.Identity(cfg.privateKey), @@ -236,12 +239,22 @@ func (h *host) bootstrap() { for _, info := range h.persistentPeers { h.h.Peerstore().AddAddrs(info.ID, info.Addrs, peerstore.PermanentAddrTTL) h.cm.peerSetHandler.AddReservedPeer(0, info.ID) + + err := h.connect(info) + if err != nil { + logger.Debugf("failed to bootstrap to peer, error: %s", err) + } } for _, addrInfo := range h.bootnodes { logger.Debugf("bootstrapping to peer %s", addrInfo.ID) h.h.Peerstore().AddAddrs(addrInfo.ID, addrInfo.Addrs, peerstore.PermanentAddrTTL) h.cm.peerSetHandler.AddPeer(0, addrInfo.ID) + + err := h.connect(addrInfo) + if err != nil { + logger.Debugf("failed to bootstrap to peer, error: %s", err) + } } } diff --git a/dot/network/mdns.go b/dot/network/mdns.go index fc329218f1..51349e9113 100644 --- a/dot/network/mdns.go +++ b/dot/network/mdns.go @@ -86,8 +86,8 @@ func (m *mdns) close() error { // HandlePeerFound is event handler called when a peer is found func (n Notifee) HandlePeerFound(p peer.AddrInfo) { n.logger.Debugf( - "Peer %s found using mDNS discovery, with host %s", - p.ID, n.host.id()) + "Peer %s found using mDNS discovery, with host %s,\n address info %s", + p.ID, n.host.id(), p.String()) n.host.h.Peerstore().AddAddrs(p.ID, p.Addrs, peerstore.PermanentAddrTTL) // connect to found peer diff --git a/dot/network/service.go b/dot/network/service.go index 0f51a2f049..f3ff31a919 100644 --- a/dot/network/service.go +++ b/dot/network/service.go @@ -237,6 +237,10 @@ func (s *Service) Start() error { logger.Infof("Started listening on %s", addr) } + for _, addr := range s.host.h.Addrs() { + logger.Infof("Started listening on %s", fmt.Sprintf(" %s/p2p/%s\n", addr, s.host.h.ID().Pretty())) + } + s.startPeerSetHandler() if !s.noMDNS { @@ -729,6 +733,7 @@ func (s *Service) ReportPeer(change peerset.ReputationChange, p peer.ID) { func (s *Service) startPeerSetHandler() { s.host.cm.peerSetHandler.Start() + fmt.Println("startPeerSetHandler 736") // wait for peerSetHandler to start. if !s.noBootstrap { s.host.bootstrap() @@ -746,21 +751,23 @@ 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.Errorf("failed to find peer id %s: %s", peerID, err) return } } + fmt.Println("network/service processMessage 758") + err := s.host.connect(addrInfo) if err != nil { - logger.Debugf("failed to open connection for peer %s: %s", peerID, err) + logger.Errorf("failed to open connection for peer %s,\n address info %s\n: %s", peerID, addrInfo.String(), 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.Errorf("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/handler.go b/dot/peerset/handler.go index ac4b733ebb..f7fa42d16e 100644 --- a/dot/peerset/handler.go +++ b/dot/peerset/handler.go @@ -3,7 +3,11 @@ package peerset -import "github.com/libp2p/go-libp2p-core/peer" +import ( + "fmt" + + "github.com/libp2p/go-libp2p-core/peer" +) // Handler manages peerSet. type Handler struct { @@ -53,6 +57,8 @@ func (h *Handler) SetReservedPeer(setID int, peers ...peer.ID) { // AddPeer adds peer to peerSet. func (h *Handler) AddPeer(setID int, peers ...peer.ID) { + fmt.Println("peerset/handler AddPeer 56") + h.actionQueue <- action{ actionCall: addToPeerSet, setID: setID, diff --git a/dot/peerset/peerset.go b/dot/peerset/peerset.go index 36a44c0407..d0d4cf3a6e 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -155,9 +155,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). @@ -175,8 +175,8 @@ type ConfigSet struct { // NewConfigSet creates a new config set for the peerSet func NewConfigSet(in, out uint32, reservedOnly bool, allocTime time.Duration) *ConfigSet { set := &config{ - inPeers: in, - outPeers: out, + maxInPeers: in, + maxOutPeers: out, reservedOnly: reservedOnly, periodicAllocTime: allocTime, } @@ -326,13 +326,17 @@ func (ps *PeerSet) reportPeer(change ReputationChange, peers ...peer.ID) error { // allocSlots tries to fill available outgoing slots of nodes for the given set. func (ps *PeerSet) allocSlots(setIdx int) error { + fmt.Println("peerset/peerset allocSlots 329") err := ps.updateTime() if err != nil { + logger.Errorf(err.Error()) return err } peerState := ps.peerState for reservePeer := range ps.reservedNode { + fmt.Println("peerset/peerset looping in reserve peer") + status := peerState.peerStatus(setIdx, reservePeer) switch status { case connectedPeer: @@ -344,14 +348,17 @@ func (ps *PeerSet) allocSlots(setIdx int) error { var n *node n, err = ps.peerState.getNode(reservePeer) if err != nil { + logger.Errorf(err.Error()) return err } if n.getReputation() < BannedThresholdValue { + logger.Warnf("reputation is lower than banned threshold value") break } if err = peerState.tryOutgoing(setIdx, reservePeer); err != nil { + logger.Errorf(err.Error()) return err } @@ -360,15 +367,22 @@ func (ps *PeerSet) allocSlots(setIdx int) error { setID: uint64(setIdx), PeerID: reservePeer, } + fmt.Println("peerset/peerset allocSlots 363") + } + // nothing more to do if we're in reserved mode. if ps.isReservedOnly { + fmt.Println("peerset/peerset allocSlots 373") return nil } for peerState.hasFreeOutgoingSlot(setIdx) { + fmt.Println("peerset/peerset allocSlots 378") + peerID := peerState.highestNotConnectedPeer(setIdx) if peerID == "" { + fmt.Println("peerset/peerset allocSlots 82") break } @@ -379,6 +393,7 @@ func (ps *PeerSet) allocSlots(setIdx int) error { } if err = peerState.tryOutgoing(setIdx, peerID); err != nil { + logger.Errorf(err.Error()) break } @@ -387,6 +402,7 @@ func (ps *PeerSet) allocSlots(setIdx int) error { setID: uint64(setIdx), PeerID: peerID, } + fmt.Println("peerset/peerset allocSlots 403") logger.Debugf("Sent connect message to peer %s", peerID) } @@ -475,6 +491,8 @@ func (ps *PeerSet) addPeer(setID int, peers peer.IDSlice) error { return nil } + fmt.Println("peerset/peerset addPeer 478") + ps.peerState.discover(setID, pid) if err := ps.allocSlots(setID); err != nil { return err @@ -621,7 +639,9 @@ func (ps *PeerSet) doWork() { select { case <-ticker.C: l := ps.peerState.getSetLength() + fmt.Println("ps.peerState.getSetLength()", ps.peerState.getSetLength()) for i := 0; i < l; i++ { + fmt.Println("are we looping because of this?") if err := ps.allocSlots(i); err != nil { logger.Debugf("failed to do action on peerSet: %s", err) } @@ -646,6 +666,8 @@ func (ps *PeerSet) doWork() { case reportPeer: err = ps.reportPeer(act.reputation, act.peers...) case addToPeerSet: + fmt.Println("peerset/peerset doWork 649") + err = ps.addPeer(act.setID, act.peers) case removeFromPeerSet: err = ps.removePeer(act.setID, act.peers...) diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index 3d7ad5dc31..97f1236c75 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 + // 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{}), } @@ -211,8 +212,10 @@ func (ps *PeersState) highestNotConnectedPeer(set int) peer.ID { var peerID peer.ID for id, n := range ps.nodes { if n.state[set] != notConnected { + fmt.Printf("connected, peer id: %s\n", id.Pretty()) continue } + fmt.Printf("not connected, peer id: %s\n", id.Pretty()) val := int(n.rep) if val >= maxRep { @@ -225,6 +228,7 @@ func (ps *PeersState) highestNotConnectedPeer(set int) peer.ID { } func (ps *PeersState) hasFreeOutgoingSlot(set int) bool { + fmt.Printf("ps.sets[set].numOut: %d < ps.sets[set].maxOut: %d\n", ps.sets[set].numOut, ps.sets[set].maxOut) return ps.sets[set].numOut < ps.sets[set].maxOut } @@ -246,6 +250,7 @@ func (ps *PeersState) addNoSlotNode(idx int, peerID peer.ID) { ps.sets[idx].noSlotNodes[peerID] = struct{}{} n, err := ps.getNode(peerID) if err != nil { + logger.Warnf("could not get node, error: %s", err) return } @@ -261,12 +266,14 @@ func (ps *PeersState) addNoSlotNode(idx int, peerID peer.ID) { func (ps *PeersState) removeNoSlotNode(idx int, peerID peer.ID) { if _, ok := ps.sets[idx].noSlotNodes[peerID]; !ok { + logger.Debugf("peer %s already does not exist in no slot node", peerID) return } delete(ps.sets[idx].noSlotNodes, peerID) n, err := ps.getNode(peerID) if err != nil { + logger.Warnf("could not get node, error: %s", err) return } @@ -354,16 +361,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 } @@ -373,7 +377,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/test_helpers.go b/dot/peerset/test_helpers.go index ae8b116e86..c126bd6ad3 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 { //nolint t.Helper() state, err := NewPeerState([]*config{ { - inPeers: maxIn, - outPeers: maxOut, + maxInPeers: maxIn, + maxOutPeers: maxOut, }, }) require.NoError(t, err) From 74e3fa5f03e53d5a688a1ec5d8b203f8c4dab7a5 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 22 Nov 2021 20:50:42 +0530 Subject: [PATCH 02/13] some cleanup --- chain/gssmr/defaults.go | 2 ++ dot/config.go | 2 +- dot/network/discovery.go | 25 ------------------------- dot/network/host.go | 16 ++-------------- dot/network/mdns.go | 4 ++-- dot/network/service.go | 9 +-------- dot/peerset/handler.go | 4 ---- dot/peerset/peerset.go | 28 +++++----------------------- dot/peerset/peerstate.go | 6 ++---- 9 files changed, 15 insertions(+), 81 deletions(-) diff --git a/chain/gssmr/defaults.go b/chain/gssmr/defaults.go index 7c3717ffb1..eb9c290384 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 = 100 // DefaultDiscoveryInterval is the default interval for searching for DHT peers DefaultDiscoveryInterval = time.Second * 10 diff --git a/dot/config.go b/dot/config.go index 3aee9f8302..e2b3c973f1 100644 --- a/dot/config.go +++ b/dot/config.go @@ -206,7 +206,7 @@ func GssmrConfig() *Config { NoMDNS: gssmr.DefaultNoMDNS, DiscoveryInterval: gssmr.DefaultDiscoveryInterval, MinPeers: gssmr.DefaultMinPeers, - MaxPeers: 100, + MaxPeers: gssmr.DefaultMaxPeers, }, RPC: RPCConfig{ Port: gssmr.DefaultRPCHTTPPort, diff --git a/dot/network/discovery.go b/dot/network/discovery.go index f4259e5c71..8ef951918b 100644 --- a/dot/network/discovery.go +++ b/dot/network/discovery.go @@ -198,34 +198,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 - // } - - fmt.Println("network/discovery findPeer 213") - // 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 e7af61e230..2ee4b040ac 100644 --- a/dot/network/host.go +++ b/dot/network/host.go @@ -88,9 +88,8 @@ func newHost(ctx context.Context, cfg *Config) (*host, error) { return nil, err } - peerCfgSet := peerset.NewConfigSet(2, 5, false, peerSetSlotAllocTime) - - // peerCfgSet := peerset.NewConfigSet(uint32(cfg.MaxPeers-cfg.MinPeers), uint32(cfg.MinPeers), false, peerSetSlotAllocTime) + // TODO: What should be the right value of max in and max out? + peerCfgSet := peerset.NewConfigSet(uint32(cfg.MaxPeers-cfg.MinPeers), uint32(cfg.MaxPeers), false, peerSetSlotAllocTime) // create connection manager cm, err := newConnManager(cfg.MinPeers, cfg.MaxPeers, peerCfgSet) if err != nil { @@ -126,7 +125,6 @@ func newHost(ctx context.Context, cfg *Config) (*host, error) { // set libp2p host options opts := []libp2p.Option{ - libp2p.DefaultTransports, libp2p.ListenAddrs(addr), libp2p.DisableRelay(), libp2p.Identity(cfg.privateKey), @@ -239,22 +237,12 @@ func (h *host) bootstrap() { for _, info := range h.persistentPeers { h.h.Peerstore().AddAddrs(info.ID, info.Addrs, peerstore.PermanentAddrTTL) h.cm.peerSetHandler.AddReservedPeer(0, info.ID) - - err := h.connect(info) - if err != nil { - logger.Debugf("failed to bootstrap to peer, error: %s", err) - } } for _, addrInfo := range h.bootnodes { logger.Debugf("bootstrapping to peer %s", addrInfo.ID) h.h.Peerstore().AddAddrs(addrInfo.ID, addrInfo.Addrs, peerstore.PermanentAddrTTL) h.cm.peerSetHandler.AddPeer(0, addrInfo.ID) - - err := h.connect(addrInfo) - if err != nil { - logger.Debugf("failed to bootstrap to peer, error: %s", err) - } } } diff --git a/dot/network/mdns.go b/dot/network/mdns.go index 51349e9113..fc329218f1 100644 --- a/dot/network/mdns.go +++ b/dot/network/mdns.go @@ -86,8 +86,8 @@ func (m *mdns) close() error { // HandlePeerFound is event handler called when a peer is found func (n Notifee) HandlePeerFound(p peer.AddrInfo) { n.logger.Debugf( - "Peer %s found using mDNS discovery, with host %s,\n address info %s", - p.ID, n.host.id(), p.String()) + "Peer %s found using mDNS discovery, with host %s", + p.ID, n.host.id()) n.host.h.Peerstore().AddAddrs(p.ID, p.Addrs, peerstore.PermanentAddrTTL) // connect to found peer diff --git a/dot/network/service.go b/dot/network/service.go index 5a9450eb79..94914e7f3c 100644 --- a/dot/network/service.go +++ b/dot/network/service.go @@ -254,10 +254,6 @@ func (s *Service) Start() error { logger.Infof("Started listening on %s", addr) } - for _, addr := range s.host.h.Addrs() { - logger.Infof("Started listening on %s", fmt.Sprintf(" %s/p2p/%s\n", addr, s.host.h.ID().Pretty())) - } - s.startPeerSetHandler() if !s.noMDNS { @@ -656,7 +652,6 @@ func (s *Service) ReportPeer(change peerset.ReputationChange, p peer.ID) { func (s *Service) startPeerSetHandler() { s.host.cm.peerSetHandler.Start() - fmt.Println("startPeerSetHandler 736") // wait for peerSetHandler to start. if !s.noBootstrap { s.host.bootstrap() @@ -679,11 +674,9 @@ func (s *Service) processMessage(msg peerset.Message) { } } - fmt.Println("network/service processMessage 758") - err := s.host.connect(addrInfo) if err != nil { - logger.Errorf("failed to open connection for peer %s,\n address info %s\n: %s", peerID, addrInfo.String(), err) + logger.Errorf("failed to open connection for peer %s, error: %s", peerID, err) return } logger.Debugf("connection successful with peer %s", peerID) diff --git a/dot/peerset/handler.go b/dot/peerset/handler.go index f7fa42d16e..1be00572ae 100644 --- a/dot/peerset/handler.go +++ b/dot/peerset/handler.go @@ -4,8 +4,6 @@ package peerset import ( - "fmt" - "github.com/libp2p/go-libp2p-core/peer" ) @@ -57,8 +55,6 @@ func (h *Handler) SetReservedPeer(setID int, peers ...peer.ID) { // AddPeer adds peer to peerSet. func (h *Handler) AddPeer(setID int, peers ...peer.ID) { - fmt.Println("peerset/handler AddPeer 56") - h.actionQueue <- action{ actionCall: addToPeerSet, setID: setID, diff --git a/dot/peerset/peerset.go b/dot/peerset/peerset.go index d0d4cf3a6e..39c993f3e1 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -173,10 +173,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{ - maxInPeers: in, - maxOutPeers: out, + maxInPeers: maxInPeers, + maxOutPeers: maxOutPeers, reservedOnly: reservedOnly, periodicAllocTime: allocTime, } @@ -326,17 +326,13 @@ func (ps *PeerSet) reportPeer(change ReputationChange, peers ...peer.ID) error { // allocSlots tries to fill available outgoing slots of nodes for the given set. func (ps *PeerSet) allocSlots(setIdx int) error { - fmt.Println("peerset/peerset allocSlots 329") err := ps.updateTime() if err != nil { - logger.Errorf(err.Error()) return err } peerState := ps.peerState for reservePeer := range ps.reservedNode { - fmt.Println("peerset/peerset looping in reserve peer") - status := peerState.peerStatus(setIdx, reservePeer) switch status { case connectedPeer: @@ -348,7 +344,6 @@ func (ps *PeerSet) allocSlots(setIdx int) error { var n *node n, err = ps.peerState.getNode(reservePeer) if err != nil { - logger.Errorf(err.Error()) return err } @@ -358,7 +353,6 @@ func (ps *PeerSet) allocSlots(setIdx int) error { } if err = peerState.tryOutgoing(setIdx, reservePeer); err != nil { - logger.Errorf(err.Error()) return err } @@ -367,22 +361,17 @@ func (ps *PeerSet) allocSlots(setIdx int) error { setID: uint64(setIdx), PeerID: reservePeer, } - fmt.Println("peerset/peerset allocSlots 363") - } // nothing more to do if we're in reserved mode. if ps.isReservedOnly { - fmt.Println("peerset/peerset allocSlots 373") return nil } for peerState.hasFreeOutgoingSlot(setIdx) { - fmt.Println("peerset/peerset allocSlots 378") peerID := peerState.highestNotConnectedPeer(setIdx) if peerID == "" { - fmt.Println("peerset/peerset allocSlots 82") break } @@ -393,7 +382,7 @@ func (ps *PeerSet) allocSlots(setIdx int) error { } if err = peerState.tryOutgoing(setIdx, peerID); err != nil { - logger.Errorf(err.Error()) + logger.Errorf("could not set peer as outgoing connection, peer: %s, error: %d", peerID.Pretty(), err) break } @@ -402,7 +391,6 @@ func (ps *PeerSet) allocSlots(setIdx int) error { setID: uint64(setIdx), PeerID: peerID, } - fmt.Println("peerset/peerset allocSlots 403") logger.Debugf("Sent connect message to peer %s", peerID) } @@ -491,8 +479,6 @@ func (ps *PeerSet) addPeer(setID int, peers peer.IDSlice) error { return nil } - fmt.Println("peerset/peerset addPeer 478") - ps.peerState.discover(setID, pid) if err := ps.allocSlots(setID); err != nil { return err @@ -639,11 +625,9 @@ func (ps *PeerSet) doWork() { select { case <-ticker.C: l := ps.peerState.getSetLength() - fmt.Println("ps.peerState.getSetLength()", ps.peerState.getSetLength()) for i := 0; i < l; i++ { - fmt.Println("are we looping because of this?") 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: @@ -666,8 +650,6 @@ func (ps *PeerSet) doWork() { case reportPeer: err = ps.reportPeer(act.reputation, act.peers...) case addToPeerSet: - fmt.Println("peerset/peerset doWork 649") - err = ps.addPeer(act.setID, act.peers) case removeFromPeerSet: err = ps.removePeer(act.setID, act.peers...) diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index 97f1236c75..0c71cbf95d 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -4,7 +4,6 @@ package peerset import ( - "fmt" "math" "sort" "time" @@ -212,10 +211,8 @@ func (ps *PeersState) highestNotConnectedPeer(set int) peer.ID { var peerID peer.ID for id, n := range ps.nodes { if n.state[set] != notConnected { - fmt.Printf("connected, peer id: %s\n", id.Pretty()) continue } - fmt.Printf("not connected, peer id: %s\n", id.Pretty()) val := int(n.rep) if val >= maxRep { @@ -228,7 +225,6 @@ func (ps *PeersState) highestNotConnectedPeer(set int) peer.ID { } func (ps *PeersState) hasFreeOutgoingSlot(set int) bool { - fmt.Printf("ps.sets[set].numOut: %d < ps.sets[set].maxOut: %d\n", ps.sets[set].numOut, ps.sets[set].maxOut) return ps.sets[set].numOut < ps.sets[set].maxOut } @@ -250,6 +246,7 @@ func (ps *PeersState) addNoSlotNode(idx int, peerID peer.ID) { ps.sets[idx].noSlotNodes[peerID] = struct{}{} n, err := ps.getNode(peerID) if err != nil { + // TODO: Return the error logger.Warnf("could not get node, error: %s", err) return } @@ -273,6 +270,7 @@ func (ps *PeersState) removeNoSlotNode(idx int, peerID peer.ID) { delete(ps.sets[idx].noSlotNodes, peerID) n, err := ps.getNode(peerID) if err != nil { + // TODO: Return the error logger.Warnf("could not get node, error: %s", err) return } From 270e0baf69f0ce89cbb27e1c62094916bfead2d0 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Fri, 26 Nov 2021 17:39:33 +0530 Subject: [PATCH 03/13] Addressed some reviews --- chain/gssmr/defaults.go | 2 +- cmd/gossamer/config.go | 7 +++---- dot/network/service.go | 6 +++--- dot/peerset/peerset.go | 13 +++++++++---- dot/peerset/peerstate.go | 21 ++++++++++----------- dot/peerset/peerstate_test.go | 16 ++++++++++------ 6 files changed, 36 insertions(+), 29 deletions(-) diff --git a/chain/gssmr/defaults.go b/chain/gssmr/defaults.go index 2e27ca66b5..806a86f5b9 100644 --- a/chain/gssmr/defaults.go +++ b/chain/gssmr/defaults.go @@ -75,7 +75,7 @@ var ( // DefaultMinPeers is the default minimum desired peer count DefaultMinPeers = 1 // DefaultMaxPeers is the default maximum desired peer count - DefaultMaxPeers = 100 + 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 4c1a4c25a1..929402f1fc 100644 --- a/cmd/gossamer/config.go +++ b/cmd/gossamer/config.go @@ -121,10 +121,9 @@ func createDotConfig(ctx *cli.Context) (*dot.Config, error) { return nil, err } - // TODO: log this better. Show package wise log levels. Currently - // it prints something like `{INFO INFO INFO INFO INFO INFO INFO INFO}` - // which is not informative. - 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/dot/network/service.go b/dot/network/service.go index 2b6a5ce2f8..f24e16fdb8 100644 --- a/dot/network/service.go +++ b/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.Errorf("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.Errorf("failed to open connection for peer %s, error: %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.Errorf("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 8d8ce91c6a..1813364e2d 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -349,7 +349,8 @@ func (ps *PeerSet) allocSlots(setIdx int) error { } if n.getReputation() < BannedThresholdValue { - logger.Warnf("reputation is lower than banned threshold value") + logger.Warnf("reputation is lower than banned threshold value, reputation: %d, banned threshold value: %d", + n.getReputation(), BannedThresholdValue) break } @@ -383,7 +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: %d", peerID.Pretty(), err) + logger.Errorf("could not set peer as outgoing connection, peer: %s, error: %s", peerID.Pretty(), err) break } @@ -406,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 } @@ -422,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 { diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index 1d7e0ba48a..447fad4f51 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -4,6 +4,7 @@ package peerset import ( + "fmt" "math" "sort" "time" @@ -237,19 +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 { - // TODO: Return the error - logger.Warnf("could not get node, error: %s", err) - return + return fmt.Errorf("could not get node: %w", err) } switch n.state[idx] { @@ -260,20 +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 { - logger.Debugf("peer %s already does not exist in no slot node", peerID) - 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 { - // TODO: Return the error - logger.Warnf("could not get node, error: %s", err) - return + return fmt.Errorf("could not get node: %w", err) } switch n.state[idx] { @@ -282,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. diff --git a/dot/peerset/peerstate_test.go b/dot/peerset/peerstate_test.go index 29afc25755..bd1541c3e6 100644 --- a/dot/peerset/peerstate_test.go +++ b/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)) From 8e32e93d244978b603fbd0906f4e06f8e59dfeec Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Fri, 26 Nov 2021 17:43:47 +0530 Subject: [PATCH 04/13] Set max out going connection to half of max peers --- dot/network/host.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/network/host.go b/dot/network/host.go index 724eb2338b..c9a9355e19 100644 --- a/dot/network/host.go +++ b/dot/network/host.go @@ -102,7 +102,7 @@ func newHost(ctx context.Context, cfg *Config) (*host, error) { const reservedOnly = false peerCfgSet := peerset.NewConfigSet( uint32(cfg.MaxPeers-cfg.MinPeers), - uint32(cfg.MaxPeers), + uint32(cfg.MaxPeers/2), reservedOnly, peerSetSlotAllocTime, ) From b93df809bfe7d91b260e2062978a5200d89e9f41 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 29 Nov 2021 18:06:14 +0530 Subject: [PATCH 05/13] temp --- chain/gssmr/config.toml | 1 + cmd/gossamer/config_test.go | 7 +++++++ cmd/gossamer/export_test.go | 3 +++ dot/peerset/peerset.go | 31 +++++++++++++++++++++++++++++-- 4 files changed, 40 insertions(+), 2 deletions(-) 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/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/peerset/peerset.go b/dot/peerset/peerset.go index 1813364e2d..8afd4265e0 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 From 00d78016e5e01269c0947576c2c0abc6e390c9c1 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 29 Nov 2021 21:22:54 +0530 Subject: [PATCH 06/13] temp --- dot/peerset/peerset_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/peerset/peerset_test.go b/dot/peerset/peerset_test.go index 5c4e37d3ab..9fcebe8f3f 100644 --- a/dot/peerset/peerset_test.go +++ b/dot/peerset/peerset_test.go @@ -53,7 +53,7 @@ func TestAddReservedPeers(t *testing.T) { handler.AddReservedPeer(0, reservedPeer) handler.AddReservedPeer(0, reservedPeer2) - time.Sleep(time.Millisecond * 200) + time.Sleep(time.Millisecond * 2000) expectedMsgs := []Message{ {Status: Connect, setID: 0, PeerID: bootNode}, From a9b1737f0ff0422cdc37d934771f48a254fe7144 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Mon, 29 Nov 2021 21:57:09 +0530 Subject: [PATCH 07/13] temp --- dot/peerset/peerstate.go | 1 + dot/peerset/peerstate_test.go | 3 +++ dot/peerset/test_helpers.go | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index 447fad4f51..e947471621 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -72,6 +72,7 @@ type node struct { } // newNode method to create a node with 0 Reputation at starting. +// Here, n is number of sets. func newNode(n int) *node { now := time.Now() sets := make([]MembershipState, n) diff --git a/dot/peerset/peerstate_test.go b/dot/peerset/peerstate_test.go index bd1541c3e6..3b58544f38 100644 --- a/dot/peerset/peerstate_test.go +++ b/dot/peerset/peerstate_test.go @@ -42,6 +42,7 @@ func TestNoSlotNodeDoesntOccupySlot(t *testing.T) { t.Parallel() state := newTestPeerState(t, 1, 1) + state.nodes[peer1] = newNode(1) // peer1 will not occupy any slot. err := state.addNoSlotNode(0, peer1) @@ -118,6 +119,7 @@ func TestDisconnectNoSlotDoesntPanic(t *testing.T) { state := newTestPeerState(t, 1, 1) + state.nodes[peer1] = newNode(1) err := state.addNoSlotNode(0, peer1) require.NoError(t, err) @@ -200,6 +202,7 @@ func TestSortedPeers(t *testing.T) { const msgChanSize = 1 state := newTestPeerState(t, 2, 1) + state.nodes[peer1] = newNode(1) err := state.addNoSlotNode(0, peer1) require.NoError(t, err) diff --git a/dot/peerset/test_helpers.go b/dot/peerset/test_helpers.go index f8f68191d5..ed228a8b84 100644 --- a/dot/peerset/test_helpers.go +++ b/dot/peerset/test_helpers.go @@ -44,7 +44,7 @@ func newTestPeerSet(t *testing.T, in, out uint32, bootNodes, reservedPeers []pee handler.AddPeer(0, bootNodes...) handler.AddReservedPeer(0, reservedPeers...) - time.Sleep(time.Millisecond * 100) + time.Sleep(time.Millisecond * 4000) return handler } From a7d09c519775e859561f3448f19a33a99cd01916 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 30 Nov 2021 16:54:26 +0530 Subject: [PATCH 08/13] fixed some tests and addressed reviews --- dot/network/connmgr_test.go | 17 +++++++++++------ dot/network/host_test.go | 4 ++-- dot/peerset/peerset.go | 9 ++++----- dot/peerset/peerstate.go | 2 +- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/dot/network/connmgr_test.go b/dot/network/connmgr_test.go index eedfdc7209..4feb8c2b80 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) { @@ -137,7 +142,7 @@ func TestPersistentPeers(t *testing.T) { } nodeB := createTestService(t, configB) - time.Sleep(time.Millisecond * 600) + time.Sleep(time.Millisecond * 2000) // B should have connected to A during bootstrap conns := nodeB.host.h.Network().ConnsToPeer(nodeA.host.id()) require.NotEqual(t, 0, len(conns)) @@ -221,13 +226,13 @@ func TestSetReservedPeer(t *testing.T) { node3 := createTestService(t, config) node3.noGossip = true - time.Sleep(time.Millisecond * 600) + time.Sleep(time.Millisecond * 3000) require.Equal(t, 2, node3.host.peerCount()) node3.host.h.Peerstore().AddAddrs(addrC.ID, addrC.Addrs, peerstore.PermanentAddrTTL) node3.host.cm.peerSetHandler.SetReservedPeer(0, addrC.ID) - time.Sleep(200 * time.Millisecond) + time.Sleep(2000 * time.Millisecond) // reservedOnly mode is not yet implemented, so nodeA and nodeB won't be disconnected (#1888). // TODO: once reservedOnly mode is implemented and reservedOnly is set to true, change expected value to 1 (nodeC) diff --git a/dot/network/host_test.go b/dot/network/host_test.go index 484346a926..30b3fbba70 100644 --- a/dot/network/host_test.go +++ b/dot/network/host_test.go @@ -453,7 +453,7 @@ func Test_AddReservedPeers(t *testing.T) { err := nodeA.host.addReservedPeers(nodeBPeerAddr) require.NoError(t, err) - time.Sleep(100 * time.Millisecond) + time.Sleep(1000 * time.Millisecond) require.Equal(t, 1, nodeA.host.peerCount()) } @@ -485,7 +485,7 @@ func Test_RemoveReservedPeers(t *testing.T) { err := nodeA.host.addReservedPeers(nodeBPeerAddr) require.NoError(t, err) - time.Sleep(100 * time.Millisecond) + time.Sleep(1000 * time.Millisecond) require.Equal(t, 1, nodeA.host.peerCount()) pID := nodeB.host.addrInfo().ID.String() diff --git a/dot/peerset/peerset.go b/dot/peerset/peerset.go index 8afd4265e0..41e77b15b8 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -398,7 +398,6 @@ func (ps *PeerSet) allocSlots(setIdx int) error { } for peerState.hasFreeOutgoingSlot(setIdx) { - peerID := peerState.highestNotConnectedPeer(setIdx) if peerID == "" { break @@ -411,7 +410,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) + logger.Errorf("could not set peer %s as outgoing connection: %s", peerID.Pretty(), err) break } @@ -435,10 +434,10 @@ func (ps *PeerSet) addReservedPeers(setID int, peers ...peer.ID) error { ps.reservedNode[peerID] = struct{}{} if err := ps.peerState.addNoSlotNode(setID, peerID); err != nil { - return err + 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 @@ -453,7 +452,7 @@ func (ps *PeerSet) removeReservedPeers(setID int, peers ...peer.ID) error { delete(ps.reservedNode, peerID) if err := ps.peerState.removeNoSlotNode(setID, peerID); err != nil { - return err + return fmt.Errorf("could not remove from the list of no-slot nodes: %w", err) } // nothing more to do if not in reservedOnly mode. diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index e947471621..aed7e00edc 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -265,7 +265,7 @@ func (ps *PeersState) addNoSlotNode(idx int, peerID peer.ID) error { func (ps *PeersState) removeNoSlotNode(idx int, peerID peer.ID) error { if _, ok := ps.sets[idx].noSlotNodes[peerID]; !ok { - logger.Debugf("peer %s is not in \"no slot node\" map", peerID) + logger.Debugf("peer %s is not in no-slot node map", peerID) return nil } From 35513080d85decc0e2dfe417b73c037498bfc447 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 30 Nov 2021 21:10:43 +0530 Subject: [PATCH 09/13] fix tests --- dot/network/connmgr_test.go | 6 +++--- dot/network/host_test.go | 4 ++-- dot/peerset/peerset.go | 2 ++ dot/peerset/peerset_test.go | 2 +- dot/peerset/peerstate.go | 2 +- dot/peerset/test_helpers.go | 2 +- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/dot/network/connmgr_test.go b/dot/network/connmgr_test.go index 4feb8c2b80..f67a99f77a 100644 --- a/dot/network/connmgr_test.go +++ b/dot/network/connmgr_test.go @@ -142,7 +142,7 @@ func TestPersistentPeers(t *testing.T) { } nodeB := createTestService(t, configB) - time.Sleep(time.Millisecond * 2000) + time.Sleep(time.Millisecond * 600) // B should have connected to A during bootstrap conns := nodeB.host.h.Network().ConnsToPeer(nodeA.host.id()) require.NotEqual(t, 0, len(conns)) @@ -226,13 +226,13 @@ func TestSetReservedPeer(t *testing.T) { node3 := createTestService(t, config) node3.noGossip = true - time.Sleep(time.Millisecond * 3000) + time.Sleep(time.Millisecond * 600) require.Equal(t, 2, node3.host.peerCount()) node3.host.h.Peerstore().AddAddrs(addrC.ID, addrC.Addrs, peerstore.PermanentAddrTTL) node3.host.cm.peerSetHandler.SetReservedPeer(0, addrC.ID) - time.Sleep(2000 * time.Millisecond) + time.Sleep(200 * time.Millisecond) // reservedOnly mode is not yet implemented, so nodeA and nodeB won't be disconnected (#1888). // TODO: once reservedOnly mode is implemented and reservedOnly is set to true, change expected value to 1 (nodeC) diff --git a/dot/network/host_test.go b/dot/network/host_test.go index 30b3fbba70..484346a926 100644 --- a/dot/network/host_test.go +++ b/dot/network/host_test.go @@ -453,7 +453,7 @@ func Test_AddReservedPeers(t *testing.T) { err := nodeA.host.addReservedPeers(nodeBPeerAddr) require.NoError(t, err) - time.Sleep(1000 * time.Millisecond) + time.Sleep(100 * time.Millisecond) require.Equal(t, 1, nodeA.host.peerCount()) } @@ -485,7 +485,7 @@ func Test_RemoveReservedPeers(t *testing.T) { err := nodeA.host.addReservedPeers(nodeBPeerAddr) require.NoError(t, err) - time.Sleep(1000 * time.Millisecond) + time.Sleep(100 * time.Millisecond) require.Equal(t, 1, nodeA.host.peerCount()) pID := nodeB.host.addrInfo().ID.String() diff --git a/dot/peerset/peerset.go b/dot/peerset/peerset.go index 41e77b15b8..59ac6dfaaf 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -432,6 +432,8 @@ func (ps *PeerSet) addReservedPeers(setID int, peers ...peer.ID) error { return nil } + ps.peerState.discover(setID, peerID) + ps.reservedNode[peerID] = struct{}{} if err := ps.peerState.addNoSlotNode(setID, peerID); err != nil { return fmt.Errorf("could not add to list of no-slot nodes: %w", err) diff --git a/dot/peerset/peerset_test.go b/dot/peerset/peerset_test.go index 9fcebe8f3f..5c4e37d3ab 100644 --- a/dot/peerset/peerset_test.go +++ b/dot/peerset/peerset_test.go @@ -53,7 +53,7 @@ func TestAddReservedPeers(t *testing.T) { handler.AddReservedPeer(0, reservedPeer) handler.AddReservedPeer(0, reservedPeer2) - time.Sleep(time.Millisecond * 2000) + time.Sleep(time.Millisecond * 200) expectedMsgs := []Message{ {Status: Connect, setID: 0, PeerID: bootNode}, diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index aed7e00edc..2b0dd1e7b7 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -249,7 +249,7 @@ func (ps *PeersState) addNoSlotNode(idx int, peerID peer.ID) error { ps.sets[idx].noSlotNodes[peerID] = struct{}{} n, err := ps.getNode(peerID) if err != nil { - return fmt.Errorf("could not get node: %w", err) + return fmt.Errorf("could not get node for peer id %s: %w", peerID, err) } switch n.state[idx] { diff --git a/dot/peerset/test_helpers.go b/dot/peerset/test_helpers.go index ed228a8b84..f8f68191d5 100644 --- a/dot/peerset/test_helpers.go +++ b/dot/peerset/test_helpers.go @@ -44,7 +44,7 @@ func newTestPeerSet(t *testing.T, in, out uint32, bootNodes, reservedPeers []pee handler.AddPeer(0, bootNodes...) handler.AddReservedPeer(0, reservedPeers...) - time.Sleep(time.Millisecond * 4000) + time.Sleep(time.Millisecond * 100) return handler } From 507f138b527742e8630af62adfe79530f46c2016 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 30 Nov 2021 23:54:49 +0530 Subject: [PATCH 10/13] addressed more reviews --- dot/network/discovery.go | 1 - dot/network/host.go | 3 ++- dot/peerset/peerstate.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dot/network/discovery.go b/dot/network/discovery.go index c4182c01b4..a1e54ddbb4 100644 --- a/dot/network/discovery.go +++ b/dot/network/discovery.go @@ -152,7 +152,6 @@ func (d *discovery) advertise() { ttl, err = d.rd.Advertise(d.ctx, string(d.pid)) if err != nil { - // TODO: This fails consistently. logger.Warnf("failed to advertise in the DHT: %s", err) ttl = tryAdvertiseTimeout } diff --git a/dot/network/host.go b/dot/network/host.go index d8cc40308d..6662c48833 100644 --- a/dot/network/host.go +++ b/dot/network/host.go @@ -98,7 +98,8 @@ 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? + // 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), diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index 2b0dd1e7b7..cc02d35de3 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -58,7 +58,7 @@ type Info struct { // node represents state of a single node that we know about type node struct { - // list of Set, the node belongs to. + // 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 @@ -361,8 +361,8 @@ 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. // non slot occupying nodes don't count towards the number of slots. func (ps *PeersState) tryOutgoing(setID int, peerID peer.ID) error { _, isNoSlotNode := ps.sets[setID].noSlotNodes[peerID] From 33d3c6445ad2897bdb09c18b110f85ecfc9d7979 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Wed, 1 Dec 2021 11:58:53 +0530 Subject: [PATCH 11/13] Update dot/peerset/peerstate.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Eclésio Junior --- dot/peerset/peerstate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index cc02d35de3..e93a63938b 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -272,7 +272,7 @@ func (ps *PeersState) removeNoSlotNode(idx int, peerID peer.ID) error { delete(ps.sets[idx].noSlotNodes, peerID) n, err := ps.getNode(peerID) if err != nil { - return fmt.Errorf("could not get node: %w", err) + return fmt.Errorf("could not get node for peer id %s: %w", peerID, err) } switch n.state[idx] { From 50c66c22fd60c7e753194872160336c0e5ce4c3d Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Wed, 1 Dec 2021 21:04:27 +0530 Subject: [PATCH 12/13] Update dot/peerset/peerstate.go Co-authored-by: Quentin McGaw --- dot/peerset/peerstate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index e93a63938b..dd0acf5d4c 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -362,7 +362,7 @@ 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. -// If the slots are full, the node stays "not connected" and we return the error. +// 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 { _, isNoSlotNode := ps.sets[setID].noSlotNodes[peerID] From dba4a57c9aa22df78fe1716c10db76805ad29cba Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Wed, 1 Dec 2021 21:06:04 +0530 Subject: [PATCH 13/13] Update dot/peerset/peerstate.go Co-authored-by: Quentin McGaw --- dot/peerset/peerstate.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dot/peerset/peerstate.go b/dot/peerset/peerstate.go index dd0acf5d4c..16acfab4a3 100644 --- a/dot/peerset/peerstate.go +++ b/dot/peerset/peerstate.go @@ -71,8 +71,7 @@ type node struct { rep Reputation } -// newNode method to create a node with 0 Reputation at starting. -// Here, n is number of sets. +// newNode creates a node with n number of sets and 0 reputation. func newNode(n int) *node { now := time.Now() sets := make([]MembershipState, n)