*: fix multiple lints (#1693)

Fixes lints raised by upgraded golangci-lint, also improves lint config.

category: fixbuild
ticket: none

.golangci.yml

@@ -13,6 +13,7 @@ linters-settings:
   forbidigo:
     forbid:
       - 'fmt\.Print.*(# Avoid debug logging)?'
+      - 'fmt\.Errorf.*(# Prefer app/errors.Wrap)?'
   gci:
     local-prefixes: github.com/obolnetwork/charon
   gocritic:
@@ -50,8 +51,11 @@ linters-settings:
         disabled: true
       - name: cognitive-complexity
         disabled: true
-
       # Some configured revive rules
+      - name: unhandled-error
+        arguments:
+         - 'fmt.Printf'
+         - 'fmt.Println'
       - name: imports-blacklist
         arguments:
          - "errors" # Prefer ./app/errors
@@ -94,6 +98,7 @@ linters:
   disable:
     # Keep disabled
     - containedctx
+    - contextcheck
     - cyclop
     - exhaustivestruct
     - exhaustruct
@@ -108,6 +113,7 @@ linters:
     - gomnd
     - gomoddirectives
     - ifshort
+    - interfacebloat
     - interfacer
     - ireturn
     - lll # Think about adding this (max line length)

app/app.go

@@ -294,7 +294,7 @@ func wireP2P(ctx context.Context, life *lifecycle.Manager, conf Config,
 		conf.TestConfig.TCPNodeCallback(tcpNode)
 	}
 
-	p2p.RegisterConnectionLogger(tcpNode, peerIDs)
+	p2p.RegisterConnectionLogger(ctx, tcpNode, peerIDs)
 
 	life.RegisterStop(lifecycle.StopP2PTCPNode, lifecycle.HookFuncErr(tcpNode.Close))
 

app/errors/errors.go

@@ -86,7 +86,7 @@ func SkipWrap(err error, msg string, skip int, fields ...z.Field) error {
 	}
 
 	return structured{
-		err:    fmt.Errorf("%s: %w", msg, err), // Wrap error message.
+		err:    fmt.Errorf("%s: %w", msg, err), //nolint: forbidigo // Wrap error message using stdlib.
 		fields: fields,
 		stack:  stack,
 	}

app/eth2wrap/httpwrap.go

@@ -50,7 +50,7 @@ type NodePeerCountProvider interface {
 }
 
 // NewHTTPAdapterForT returns a http adapter for testing non-eth2service methods as it is nil.
-func NewHTTPAdapterForT(_ *testing.T, address string, timeout time.Duration) *httpAdapter {
+func NewHTTPAdapterForT(_ *testing.T, address string, timeout time.Duration) Client {
 	return newHTTPAdapter(nil, address, timeout)
 }
 

app/eth2wrap/synthproposer.go

@@ -405,7 +405,7 @@ func getStandardHashFn() shuffle.HashFn {
 	hash := sha256.New()
 	hashFn := func(in []byte) []byte {
 		hash.Reset()
-		hash.Write(in)
+		_, _ = hash.Write(in)
 
 		return hash.Sum(nil)
 	}

cluster/definition.go

@@ -344,21 +344,21 @@ func (d Definition) SetDefinitionHashes() (Definition, error) {
 }
 
 func (d Definition) MarshalJSON() ([]byte, error) {
-	d, err := d.SetDefinitionHashes()
+	d2, err := d.SetDefinitionHashes()
 	if err != nil {
 		return nil, err
 	}
 
 	switch {
-	case isV1x0(d.Version) || isV1x1(d.Version):
-		return marshalDefinitionV1x0or1(d)
-	case isV1x2(d.Version) || isV1x3(d.Version):
+	case isV1x0(d2.Version) || isV1x1(d2.Version):
+		return marshalDefinitionV1x0or1(d2)
+	case isV1x2(d2.Version) || isV1x3(d2.Version):
 		// v1.2 and v1.3 has the same json format.
-		return marshalDefinitionV1x2or3(d)
-	case isV1x4(d.Version):
-		return marshalDefinitionV1x4(d)
-	case isV1x5(d.Version):
-		return marshalDefinitionV1x5(d)
+		return marshalDefinitionV1x2or3(d2)
+	case isV1x4(d2.Version):
+		return marshalDefinitionV1x4(d2)
+	case isV1x5(d2.Version):
+		return marshalDefinitionV1x5(d2)
 	default:
 		return nil, errors.New("unsupported version")
 	}

cmd/relay/p2p.go

@@ -67,7 +67,7 @@ func startP2P(ctx context.Context, config Config, key *k1.PrivateKey, reporter m
 		return nil, errors.Wrap(err, "new tcp node")
 	}
 
-	p2p.RegisterConnectionLogger(tcpNode, nil)
+	p2p.RegisterConnectionLogger(ctx, tcpNode, nil)
 
 	// Reservations are valid for 30min (github.com/libp2p/go-libp2p/p2p/protocol/circuitv2/relay/constraints.go:14)
 	relayResources := relay.DefaultResources()

core/deadline.go

@@ -32,7 +32,7 @@ import (
 const lateFactor = 5
 
 // lateMin defines the minimum absolute value of the lateFactor.
-const lateMin = time.Second * 30
+const lateMin = time.Second * 30 //nolint:revive // Min suffix is minimum not minute.
 
 // Deadliner provides duty Deadline functionality. The C method isn’t thread safe and
 // may only be used by a single goroutine. So, multiple instances are required

core/infosync/infosync.go

@@ -40,7 +40,7 @@ const (
 
 // New returns a new infosync component.
 func New(prioritiser *priority.Component, versions []string, protocols []protocol.ID) *Component {
-	// Add a mock alpha protocol if alpha features enabled in order to to test infosync in prod.
+	// Add a mock alpha protocol if alpha features enabled in order to test infosync in prod.
 	// TODO(corver): Remove this once we have an actual use case.
 	if featureset.Enabled(featureset.MockAlpha) {
 		protocols = append(protocols, "/charon/mock_alpha/1.0.0")

core/qbft/qbft.go

@@ -181,7 +181,7 @@ func Run[I any, V comparable](ctx context.Context, d Definition[I, V], t Transpo
 			if !strings.Contains(fmt.Sprint(r), "bug") {
 				panic(r) // Only catch internal sanity checks.
 			}
-			err = fmt.Errorf("qbft sanity check: %v", r)
+			err = fmt.Errorf("qbft sanity check: %v", r) //nolint: forbidigo // Wrapping a panic, not error.
 		}
 	}()
 

core/tracker/tracker.go

@@ -252,7 +252,7 @@ func (t *Tracker) Run(ctx context.Context) error {
 		case duty := <-t.analyser.C():
 			ctx := log.WithCtx(ctx, z.Any("duty", duty))
 
-			parsigs := analyseParSigs(t.events[duty])
+			parsigs := analyseParSigs(ctx, t.events[duty])
 			t.parSigReporter(ctx, duty, parsigs)
 
 			// Analyse failed duties
@@ -339,7 +339,7 @@ func analyseDutyFailed(duty core.Duty, allEvents map[core.Duty][]event, msgRootC
 }
 
 // analyseParSigs returns a mapping of partial signed data messages by peers (share index) by validator (pubkey).
-func analyseParSigs(events []event) parsigsByMsg {
+func analyseParSigs(ctx context.Context, events []event) parsigsByMsg {
 	type dedupKey struct {
 		Pubkey   core.PubKey
 		ShareIdx int
@@ -361,7 +361,7 @@ func analyseParSigs(events []event) parsigsByMsg {
 
 		root, err := e.parSig.MessageRoot()
 		if err != nil {
-			log.Warn(context.Background(), "Parsig message root", err)
+			log.Warn(ctx, "Parsig message root", err)
 			continue // Just log and ignore as this is highly unlikely and non-critical code.
 		}
 

core/tracker/tracker_internal_test.go

@@ -943,7 +943,7 @@ func TestIsParSigEventExpected(t *testing.T) {
 }
 
 func TestAnalyseParSigs(t *testing.T) {
-	require.Empty(t, analyseParSigs(nil))
+	require.Empty(t, analyseParSigs(context.Background(), nil))
 
 	var events []event
 
@@ -972,7 +972,7 @@ func TestAnalyseParSigs(t *testing.T) {
 		makeEvents(n, pubkey)
 	}
 
-	allParSigMsgs := analyseParSigs(events)
+	allParSigMsgs := analyseParSigs(context.Background(), events)
 
 	lengths := make(map[int]string)
 	for pubkey, parSigMsgs := range allParSigMsgs {

dkg/dkg.go

@@ -261,7 +261,7 @@ func setupP2P(ctx context.Context, key *k1.PrivateKey, p2pConf p2p.Config, peers
 		return nil, nil, err
 	}
 
-	p2p.RegisterConnectionLogger(tcpNode, peerIDs)
+	p2p.RegisterConnectionLogger(ctx, tcpNode, peerIDs)
 
 	for _, relay := range relays {
 		go func(relay *p2p.MutablePeer) {

eth2util/keymanager/keymanager.go

@@ -84,7 +84,7 @@ func (c Client) VerifyConnection(ctx context.Context) error {
 	if err != nil {
 		return errors.Wrap(err, "cannot ping address", z.Str("addr", c.baseURL))
 	}
-	conn.Close()
+	_ = conn.Close()
 
 	return nil
 }
@@ -121,7 +121,7 @@ func postKeys(ctx context.Context, addr string, reqBody keymanagerReq) error {
 	if err != nil {
 		return errors.Wrap(err, "read response")
 	}
-	resp.Body.Close()
+	_ = resp.Body.Close()
 
 	if resp.StatusCode/100 != 2 {
 		return errors.New("failed posting keys", z.Int("status", resp.StatusCode), z.Str("body", string(data)))

p2p/p2p.go

@@ -191,7 +191,11 @@ func (f peerRoutingFunc) FindPeer(ctx context.Context, p peer.ID) (peer.AddrInfo
 // RegisterConnectionLogger registers a connection logger with the host.
 // This is pretty weird and hacky, but that is because libp2p uses the network.Notifiee interface as a map key,
 // so the implementation can only contain fields that are hashable. So we use a channel and do the logic externally. :(.
-func RegisterConnectionLogger(tcpNode host.Host, peerIDs []peer.ID) {
+func RegisterConnectionLogger(ctx context.Context, tcpNode host.Host, peerIDs []peer.ID) {
+	ctx = log.WithTopic(ctx, "p2p")
+	quit := make(chan struct{})
+	defer close(quit)
+
 	var (
 		peers  = make(map[peer.ID]bool)
 		events = make(chan logEvent)
@@ -203,42 +207,47 @@ func RegisterConnectionLogger(tcpNode host.Host, peerIDs []peer.ID) {
 
 	tcpNode.Network().Notify(connLogger{
 		events: events,
+		quit:   quit,
 	})
 
 	go func() {
-		ctx := log.WithTopic(context.Background(), "p2p")
-		for e := range events {
-			addr := NamedAddr(e.Addr)
-			name := PeerName(e.Peer)
-			typ := addrType(e.Addr)
-
-			if e.Listen {
-				log.Debug(ctx, "Libp2p listening on address", z.Str("address", addr))
-				continue
-			} else if e.Connected {
-				log.Debug(ctx, "Libp2p new connection",
-					z.Str("peer", name),
-					z.Any("peer_address", addr),
-					z.Any("direction", e.Direction),
-					z.Str("type", typ),
-				)
-			}
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case e := <-events:
+				addr := NamedAddr(e.Addr)
+				name := PeerName(e.Peer)
+				typ := addrType(e.Addr)
+
+				if e.Listen {
+					log.Debug(ctx, "Libp2p listening on address", z.Str("address", addr))
+					continue
+				} else if e.Connected {
+					log.Debug(ctx, "Libp2p new connection",
+						z.Str("peer", name),
+						z.Any("peer_address", addr),
+						z.Any("direction", e.Direction),
+						z.Str("type", typ),
+					)
+				}
 
-			if !peers[e.Peer] {
-				// Do not instrument relays.
-				continue
-			}
+				if !peers[e.Peer] {
+					// Do not instrument relays.
+					continue
+				}
 
-			if e.Connected {
-				peerConnGauge.WithLabelValues(name, addrType(e.Addr)).Inc()
-				peerConnCounter.WithLabelValues(name).Inc()
-			} else if e.Disconnect {
-				peerConnGauge.WithLabelValues(name, addrType(e.Addr)).Dec()
-			}
+				if e.Connected {
+					peerConnGauge.WithLabelValues(name, addrType(e.Addr)).Inc()
+					peerConnCounter.WithLabelValues(name).Inc()
+				} else if e.Disconnect {
+					peerConnGauge.WithLabelValues(name, addrType(e.Addr)).Dec()
+				}
 
-			// Ensure both connection type metrics are initiated
-			peerConnGauge.WithLabelValues(name, addrTypeDirect).Add(0)
-			peerConnGauge.WithLabelValues(name, addrTypeRelay).Add(0)
+				// Ensure both connection type metrics are initiated
+				peerConnGauge.WithLabelValues(name, addrTypeDirect).Add(0)
+				peerConnGauge.WithLabelValues(name, addrTypeRelay).Add(0)
+			}
 		}
 	}()
 }
@@ -256,34 +265,44 @@ type logEvent struct {
 // connLogger implements network.Notifee and only sends logEvents on a channel since
 // it is used as a map key internally in libp2p, it cannot contain complex types.
 type connLogger struct {
+	quit   chan struct{}
 	events chan logEvent
 }
 
 func (l connLogger) Listen(_ network.Network, addr ma.Multiaddr) {
-	l.events <- logEvent{
+	select {
+	case <-l.quit:
+	case l.events <- logEvent{
 		Addr:   addr,
 		Listen: true,
+	}:
 	}
 }
 
 func (connLogger) ListenClose(network.Network, ma.Multiaddr) {}
 
 func (l connLogger) Connected(_ network.Network, conn network.Conn) {
-	l.events <- logEvent{
+	select {
+	case <-l.quit:
+	case l.events <- logEvent{
 		Peer:      conn.RemotePeer(),
 		Addr:      conn.RemoteMultiaddr(),
 		Direction: conn.Stat().Direction,
 		Connected: true,
 		ConnID:    conn.ID(),
+	}:
 	}
 }
 
 func (l connLogger) Disconnected(_ network.Network, conn network.Conn) {
-	l.events <- logEvent{
+	select {
+	case <-l.quit:
+	case l.events <- logEvent{
 		Peer:       conn.RemotePeer(),
 		Addr:       conn.RemoteMultiaddr(),
 		Disconnect: true,
 		ConnID:     conn.ID(),
+	}:
 	}
 }
 

testutil/genchangelog/main_internal_test.go

@@ -76,7 +76,7 @@ func TestPRFromLog(t *testing.T) {
 		},
 		{
 			in: log{
-				Subject: "build(deps): blah blah",
+				Subject: "build(deps): foo bar",
 			},
 		},
 	}

testutil/validatormock/validatormock.go

@@ -503,7 +503,7 @@ func httpGet(ctx context.Context, base string, endpoint string) ([]byte, error)
 	}
 	defer res.Body.Close()
 
-	if res.StatusCode == 404 {
+	if res.StatusCode == http.StatusNotFound {
 		// Nothing found.  This is not an error, so we return nil on both counts.
 		return nil, nil
 	}