Some linters updates (#935)

Linting: 
- Aligning struct fields
- Wrapping errors in Errorf
- Updating our Golangci config
- Making more linters pass

.github/workflows/lint.yml

@@ -14,5 +14,5 @@ jobs:
     - name: golangci-lint
       uses: golangci/golangci-lint-action@v2
       with:
-        version: v1.43
+        version: v1.45
         args: --timeout 5m

.golangci.yml

@@ -5,17 +5,25 @@ issues:
   exclude-rules:
     - path: _test\.go
       linters:
+        - bodyclose
+        - cyclop
+        - errcheck
+        - forbidigo
+        - gocyclo
         - gomnd
         - gosec
-        - errcheck
+        - nilnil
+        - noctx
     - path: protobuf/gen.go
       linters:
         - lll
       source: "^//go:generate "
-    # https://github.com/go-critic/go-critic/issues/926
-    - linters:
-        - gocritic
-      text: "unnecessaryDefer:"
+    - path: cmd
+      linters:
+        - forbidigo # we use Println in our UX
+    - path: core/broadcast.go
+      linters:
+        - interfacer
 run:
   skip-dirs:
     - demo
@@ -24,42 +32,33 @@ run:
 linters:
   # please, do not use `enable-all`: it's deprecated and will be removed soon.
   # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
-  disable-all: true
-  enable:
-    - gocyclo
-    - bodyclose
-    - deadcode
-    - depguard
-    - dogsled
-    - dupl
-    - errcheck
-    - funlen
-    - gochecknoinits
-    - goconst
-    - gocritic
-    - gocyclo
-    - gofmt
-    - goimports
-    - gomnd
-    - goprintffuncname
-    - gosec
-    - gosimple
-    - govet
-    - ineffassign
-    - lll
-    - misspell
-    - nakedret
-    - nolintlint
-    - rowserrcheck
-    - staticcheck
-    - structcheck
-    - stylecheck
-    - typecheck
-    - unconvert
-    - unparam
-    - unused
-    - varcheck
-    - whitespace
+  enable-all: true
+  disable:
+    - containedctx
+    - contextcheck
+    - cyclop
+    - exhaustivestruct
+    - forcetypeassert
+    - gci
+    - gochecknoglobals
+    - gocognit
+    - godot
+    - godox
+    - goerr113
+    - gofumpt
+    - ifshort
+    - ireturn
+    - nestif
+    - nlreturn
+    - paralleltest
+    - promlinter
+    - tagliatelle
+    - testpackage
+    - thelper
+    - varnamelen
+    - wrapcheck
+    - wsl
+
 
 linters-settings:
   dupl:

chain/beacon/node.go

@@ -54,8 +54,8 @@ type Handler struct {
 	running bool
 	serving bool
 	stopped bool
-	l       log.Logger
 	version commonutils.Version
+	l       log.Logger
 }
 
 // NewHandler returns a fresh handler ready to serve and create randomness

chain/beacon/sync.go

@@ -202,7 +202,7 @@ func (s *syncer) SyncChain(req *proto.SyncRequest, stream proto.Protocol_SyncCha
 }
 
 func peersToString(peers []net.Peer) string {
-	var adds []string
+	adds := make([]string, 0, len(peers))
 	for _, p := range peers {
 		adds = append(adds, p.Address())
 	}

chain/convert.go

@@ -24,7 +24,7 @@ func InfoFromProto(p *drand.ChainInfoPacket) (*Info, error) {
 
 	sch, err := scheme.GetSchemeByIDWithDefault(p.SchemeID)
 	if err != nil {
-		return nil, fmt.Errorf("scheme id received is not valid. Err: %s", err)
+		return nil, fmt.Errorf("scheme id received is not valid. Err: %w", err)
 	}
 
 	return &Info{
@@ -62,12 +62,12 @@ func (c *Info) ToProto(metadata *common.Metadata) *drand.ChainInfoPacket {
 func InfoFromJSON(buff io.Reader) (*Info, error) {
 	chainProto := new(drand.ChainInfoPacket)
 	if err := json.NewDecoder(buff).Decode(chainProto); err != nil {
-		return nil, fmt.Errorf("reading group file (%v)", err)
+		return nil, fmt.Errorf("reading group file (%w)", err)
 	}
 
 	chainInfo, err := InfoFromProto(chainProto)
 	if err != nil {
-		return nil, fmt.Errorf("invalid chain info: %s", err)
+		return nil, fmt.Errorf("invalid chain info: %w", err)
 	}
 
 	return chainInfo, nil

client/aggregator.go

@@ -148,6 +148,7 @@ func (c *watchAggregator) Watch(ctx context.Context) <-chan Result {
 			c.cancelPassive()
 			c.cancelPassive = nil
 		}
+		// TODO: any reason we are not passing the existing context here?
 		ctx, cancel := context.WithCancel(context.Background())
 		go c.distribute(c.Client.Watch(ctx), cancel)
 	}

client/client.go

@@ -173,13 +173,14 @@ type clientConfig struct {
 	fullVerify bool
 	// insecure indicates the root of trust does not need to be present.
 	insecure bool
+	// autoWatch causes the client to start watching immediately in the background so that new randomness
+	// is proactively fetched and added to the cache.
+	autoWatch bool
 	// cache size - how large of a cache to keep locally.
 	cacheSize int
 	// customized client log.
 	log log.Logger
-	// autoWatch causes the client to start watching immediately in the background so that new randomness
-	// is proactively fetched and added to the cache.
-	autoWatch bool
+
 	// autoWatchRetry specifies the time after which the watch channel
 	// created by the autoWatch is re-opened when no context error occurred.
 	autoWatchRetry time.Duration

client/http/http.go

@@ -240,7 +240,7 @@ func (h *httpClient) FetchChainInfo(ctx context.Context, chainHash []byte) (*cha
 	defer cancel()
 
 	go func() {
-		url := ""
+		var url string
 		if len(chainHash) > 0 {
 			url = fmt.Sprintf("%s%x/info", h.root, chainHash)
 		} else {

client/optimizing.go

@@ -221,7 +221,7 @@ func (oc *optimizingClient) fastestClients() []Client {
 	oc.RLock()
 	defer oc.RUnlock()
 	// copy the current ordered client list so we iterate over a stable slice
-	var clients []Client
+	clients := make([]Client, 0, len(oc.stats))
 	for _, s := range oc.stats {
 		clients = append(clients, s.client)
 	}
@@ -231,7 +231,7 @@ func (oc *optimizingClient) fastestClients() []Client {
 // Get returns the randomness at `round` or an error.
 func (oc *optimizingClient) Get(ctx context.Context, round uint64) (res Result, err error) {
 	clients := oc.fastestClients()
-	stats := []*requestStat{}
+	var stats []*requestStat
 	ch := raceGet(ctx, clients, round, oc.requestTimeout, oc.requestConcurrency)
 	err = errors.New("no valid clients")
 
@@ -610,7 +610,7 @@ func (oc *optimizingClient) Info(ctx context.Context) (chainInfo *chain.Info, er
 		chainInfo, err = c.Info(ctx)
 		cancel()
 		if err == nil {
-			return
+			break
 		}
 	}
 	return

cmd/client/lib/cli.go

@@ -83,7 +83,7 @@ var (
 		Usage: "Local (host:)port for constructed libp2p host to listen on",
 	}
 
-	// JsonFlag is the CLI flag for enabling JSON output for logger
+	// JSONFlag is the value of the CLI flag `json` enabling JSON output of the loggers
 	JSONFlag = &cli.BoolFlag{
 		Name:  "json",
 		Usage: "Set the output as json format",

cmd/drand-cli/cli.go

@@ -564,7 +564,7 @@ func resetCmd(c *cli.Context) error {
 
 	answer, err := reader.ReadString('\n')
 	if err != nil {
-		return fmt.Errorf("error reading: %s", err)
+		return fmt.Errorf("error reading: %w", err)
 	}
 
 	answer = strings.ToLower(strings.TrimSpace(answer))
@@ -687,14 +687,14 @@ func keygenCmd(c *cli.Context) error {
 		return nil
 	}
 	if err := fileStore.SaveKeyPair(priv); err != nil {
-		return fmt.Errorf("could not save key: %s", err)
+		return fmt.Errorf("could not save key: %w", err)
 	}
 
 	fullpath := path.Join(config.ConfigFolderMB(), beaconID, key.KeyFolderName)
 	absPath, err := filepath.Abs(fullpath)
 
 	if err != nil {
-		return fmt.Errorf("err getting full path: %s", err)
+		return fmt.Errorf("err getting full path: %w", err)
 	}
 	fmt.Println("Generated keys at ", absPath)
 
@@ -712,14 +712,14 @@ func groupOut(c *cli.Context, group *key.Group) error {
 	if c.IsSet("out") {
 		groupPath := c.String("out")
 		if err := key.Save(groupPath, group, false); err != nil {
-			return fmt.Errorf("drand: can't save group to specified file name: %v", err)
+			return fmt.Errorf("drand: can't save group to specified file name: %w", err)
 		}
 	} else if c.Bool(hashOnly.Name) {
 		fmt.Fprintf(output, "%x\n", group.Hash())
 	} else {
 		var buff bytes.Buffer
 		if err := toml.NewEncoder(&buff).Encode(group.TOML()); err != nil {
-			return fmt.Errorf("drand: can't encode group to TOML: %v", err)
+			return fmt.Errorf("drand: can't encode group to TOML: %w", err)
 		}
 		buff.WriteString("\n")
 		fmt.Fprintf(output, "Copy the following snippet into a new group.toml file\n")
@@ -743,7 +743,7 @@ func getThreshold(c *cli.Context) (int, error) {
 
 func checkConnection(c *cli.Context) error {
 	var names []string
-	beaconID := ""
+	var beaconID string
 
 	if c.IsSet(groupFlag.Name) {
 		if c.IsSet(beaconIDFlag.Name) {
@@ -754,7 +754,7 @@ func checkConnection(c *cli.Context) error {
 		}
 		group := new(key.Group)
 		if err := key.Load(c.String(groupFlag.Name), group); err != nil {
-			return fmt.Errorf("loading group failed: %s", err)
+			return fmt.Errorf("loading group failed: %w", err)
 		}
 
 		for _, id := range group.Nodes {
@@ -765,7 +765,7 @@ func checkConnection(c *cli.Context) error {
 		for _, serverAddr := range c.Args().Slice() {
 			_, _, err := gonet.SplitHostPort(serverAddr)
 			if err != nil {
-				return fmt.Errorf("error for address %s: %s", serverAddr, err)
+				return fmt.Errorf("error for address %s: %w", serverAddr, err)
 			}
 			names = append(names, serverAddr)
 		}
@@ -857,13 +857,13 @@ func deleteBeaconCmd(c *cli.Context) error {
 		er = func() error {
 			store, err := boltdb.NewBoltStore(path.Join(storePath, core.DefaultDBFolder), conf.BoltOptions())
 			if err != nil {
-				return fmt.Errorf("beacon id [%s] - invalid bolt store creation: %s", beaconID, err)
+				return fmt.Errorf("beacon id [%s] - invalid bolt store creation: %w", beaconID, err)
 			}
 			defer store.Close()
 
 			lastBeacon, err := store.Last()
 			if err != nil {
-				return fmt.Errorf("beacon id [%s] - can't fetch last beacon: %s", beaconID, err)
+				return fmt.Errorf("beacon id [%s] - can't fetch last beacon: %w", beaconID, err)
 			}
 			if startRound > lastBeacon.Round {
 				return fmt.Errorf("beacon id [%s] - given round is ahead of the chain: %d", beaconID, lastBeacon.Round)
@@ -875,7 +875,7 @@ func deleteBeaconCmd(c *cli.Context) error {
 			for round := startRound; round <= lastBeacon.Round; round++ {
 				err := store.Del(round)
 				if err != nil {
-					return fmt.Errorf("beacon id [%s] - error deleting round %d: %s", beaconID, round, err)
+					return fmt.Errorf("beacon id [%s] - error deleting round %d: %w", beaconID, round, err)
 				}
 				if c.IsSet(verboseFlag.Name) {
 					fmt.Printf("beacon id [%s] - deleted beacon round %d \n", beaconID, round)
@@ -899,7 +899,7 @@ func getGroup(c *cli.Context) (*key.Group, error) {
 		return nil, err
 	}
 	if err := key.Load(groupPath, g); err != nil {
-		return nil, fmt.Errorf("drand: error loading group file: %s", err)
+		return nil, fmt.Errorf("drand: error loading group file: %w", err)
 	}
 	return g, nil
 }
@@ -999,12 +999,12 @@ func getNodes(c *cli.Context) ([]*key.Node, error) {
 func testEmptyGroup(filePath string) error {
 	file, err := os.Open(filePath)
 	if err != nil {
-		return fmt.Errorf("can't open group path: %v", err)
+		return fmt.Errorf("can't open group path: %w", err)
 	}
 	defer file.Close()
 	fi, err := file.Stat()
 	if err != nil {
-		return fmt.Errorf("can't open file info: %v", err)
+		return fmt.Errorf("can't open file info: %w", err)
 	}
 	if fi.Size() == 0 {
 		return errors.New("group file empty")
@@ -1028,7 +1028,7 @@ func getDBStoresPaths(c *cli.Context) (map[string]string, error) {
 	if c.IsSet(allBeaconsFlag.Name) {
 		fi, err := os.ReadDir(conf.ConfigFolderMB())
 		if err != nil {
-			return nil, fmt.Errorf("error trying to read stores from config folder: %s", err)
+			return nil, fmt.Errorf("error trying to read stores from config folder: %w", err)
 		}
 		for _, f := range fi {
 			if f.IsDir() {
@@ -1040,7 +1040,7 @@ func getDBStoresPaths(c *cli.Context) (map[string]string, error) {
 
 		isPresent, err := fs.Exists(path.Join(conf.ConfigFolderMB(), beaconID))
 		if err != nil || !isPresent {
-			return nil, fmt.Errorf("beacon id [%s] - error trying to read store: %s", beaconID, err)
+			return nil, fmt.Errorf("beacon id [%s] - error trying to read store: %w", beaconID, err)
 		}
 
 		stores[beaconID] = path.Join(conf.ConfigFolderMB(), beaconID)

cmd/drand-cli/cli_test.go

@@ -926,7 +926,7 @@ func launchDrandInstances(t *testing.T, n int) ([]*drandInstance, string) {
 		})
 	}
 
-	os.Setenv("DRAND_SHARE_SECRET", "testtesttestesttesttesttestesttesttesttestesttesttesttestest")
+	t.Setenv("DRAND_SHARE_SECRET", "testtesttestesttesttesttestesttesttesttestesttesttesttestest")
 	for _, instance := range ins {
 		instance.run(t, beaconID)
 	}