Fix lints (cometbft#625)

This touches a large number of files, but I believe it's necessary as part of our tech debt cleanup. There are so many functions littering the codebase where:

1. We have unused parameters and they should have been refactored away, but they weren't
2. We have unused parameters and they should be named according to proper Go conventions (e.g. in interface implementations where a particular function signature is required, but the variable names should either be left out or `_` when unused)
3. We have redundant code (a whole bunch of redundant `if` statements and error checks, for example)
4. We use bad naming conventions for variables, like `copy` or `len`

I'm also tired of having the linter fail locally. And if we don't do this, it will all just rot even more.

This PR targets `main`, but should be applied across all the backport branches (which will, of course, require some conflict resolution, but I'm fine with doing that). I've left TODOs in some places where changes need to be made in follow-up PRs.

Commits are organized by package so that it's hopefully easier to review.

If you pick up on formatting changes, please see cometbft#604.

---

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments

.github/workflows/lint.yml

@@ -31,7 +31,7 @@ jobs:
             go.sum
       - uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.51
+          version: latest
           args: --timeout 10m
           github-token: ${{ secrets.github_token }}
         if: env.GIT_DIFF

Makefile

@@ -231,7 +231,7 @@ format:
 
 lint:
 	@echo "--> Running linter"
-	@go run github.com/golangci/golangci-lint/cmd/golangci-lint run
+	@go run github.com/golangci/golangci-lint/cmd/golangci-lint@latest run
 .PHONY: lint
 
 vulncheck:

abci/client/grpc_client.go

@@ -49,7 +49,7 @@ func NewGRPCClient(addr string, mustConnect bool) Client {
 	return cli
 }
 
-func dialerFunc(ctx context.Context, addr string) (net.Conn, error) {
+func dialerFunc(_ context.Context, addr string) (net.Conn, error) {
 	return cmtnet.Connect(addr)
 }
 
@@ -202,7 +202,7 @@ func (cli *grpcClient) Query(ctx context.Context, req *types.RequestQuery) (*typ
 	return cli.client.Query(ctx, types.ToRequestQuery(req).GetQuery(), grpc.WaitForReady(true))
 }
 
-func (cli *grpcClient) Commit(ctx context.Context, req *types.RequestCommit) (*types.ResponseCommit, error) {
+func (cli *grpcClient) Commit(ctx context.Context, _ *types.RequestCommit) (*types.ResponseCommit, error) {
 	return cli.client.Commit(ctx, types.ToRequestCommit().GetCommit(), grpc.WaitForReady(true))
 }
 

abci/client/grpc_client_test.go

@@ -75,6 +75,6 @@ func TestGRPC(t *testing.T) {
 	}
 }
 
-func dialerFunc(ctx context.Context, addr string) (net.Conn, error) {
+func dialerFunc(_ context.Context, addr string) (net.Conn, error) {
 	return cmtnet.Connect(addr)
 }

abci/client/socket_client.go

@@ -169,7 +169,7 @@ func (cli *socketClient) recvResponseRoutine(conn io.Reader) {
 			return
 		}
 
-		var res = &types.Response{}
+		res := &types.Response{}
 		err := types.ReadMessage(r, res)
 		if err != nil {
 			cli.stopForError(fmt.Errorf("read message: %w", err))
@@ -291,7 +291,7 @@ func (cli *socketClient) Query(ctx context.Context, req *types.RequestQuery) (*t
 	return reqRes.Response.GetQuery(), cli.Error()
 }
 
-func (cli *socketClient) Commit(ctx context.Context, req *types.RequestCommit) (*types.ResponseCommit, error) {
+func (cli *socketClient) Commit(ctx context.Context, _ *types.RequestCommit) (*types.ResponseCommit, error) {
 	reqRes, err := cli.queueRequest(ctx, types.ToRequestCommit())
 	if err != nil {
 		return nil, err

abci/client/socket_client_test.go

@@ -122,7 +122,8 @@ func TestBulk(t *testing.T) {
 }
 
 func setupClientServer(t *testing.T, app types.Application) (
-	service.Service, abcicli.Client) {
+	service.Service, abcicli.Client,
+) {
 	t.Helper()
 
 	// some port between 20k and 30k
@@ -156,7 +157,7 @@ type slowApp struct {
 	types.BaseApplication
 }
 
-func (slowApp) CheckTx(_ context.Context, req *types.RequestCheckTx) (*types.ResponseCheckTx, error) {
+func (slowApp) CheckTx(context.Context, *types.RequestCheckTx) (*types.ResponseCheckTx, error) {
 	time.Sleep(time.Second)
 	return &types.ResponseCheckTx{}, nil
 }

abci/cmd/abci-cli/abci-cli.go

@@ -51,7 +51,6 @@ var RootCmd = &cobra.Command{
 	Short: "the ABCI CLI tool wraps an ABCI client",
 	Long:  "the ABCI CLI tool wraps an ABCI client and is used for testing ABCI servers",
 	PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-
 		switch cmd.Use {
 		case "kvstore", "version", "help [command]":
 			return nil
@@ -196,6 +195,7 @@ var echoCmd = &cobra.Command{
 	Args:  cobra.ExactArgs(1),
 	RunE:  cmdEcho,
 }
+
 var infoCmd = &cobra.Command{
 	Use:   "info",
 	Short: "get some info about the application",
@@ -281,7 +281,6 @@ var testCmd = &cobra.Command{
 
 // Generates new Args array based off of previous call args to maintain flag persistence
 func persistentArgs(line []byte) []string {
-
 	// generate the arguments to run from original os.Args
 	// to maintain flag arguments
 	args := os.Args
@@ -308,7 +307,7 @@ func compose(fs []func() error) error {
 	return err
 }
 
-func cmdTest(cmd *cobra.Command, args []string) error {
+func cmdTest(cmd *cobra.Command, _ []string) error {
 	ctx := cmd.Context()
 	return compose(
 		[]func() error{
@@ -361,7 +360,7 @@ func cmdTest(cmd *cobra.Command, args []string) error {
 		})
 }
 
-func cmdBatch(cmd *cobra.Command, args []string) error {
+func cmdBatch(cmd *cobra.Command, _ []string) error {
 	bufReader := bufio.NewReader(os.Stdin)
 LOOP:
 	for {
@@ -387,7 +386,7 @@ LOOP:
 	return nil
 }
 
-func cmdConsole(cmd *cobra.Command, args []string) error {
+func cmdConsole(cmd *cobra.Command, _ []string) error {
 	for {
 		fmt.Printf("> ")
 		bufReader := bufio.NewReader(os.Stdin)
@@ -695,7 +694,7 @@ func cmdProcessProposal(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-func cmdKVStore(cmd *cobra.Command, args []string) error {
+func cmdKVStore(*cobra.Command, []string) error {
 	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
 
 	// Create the application - in memory or persisted to disk
@@ -734,7 +733,6 @@ func cmdKVStore(cmd *cobra.Command, args []string) error {
 //--------------------------------------------------------------------------------
 
 func printResponse(cmd *cobra.Command, args []string, rsps ...response) {
-
 	if flagVerbose {
 		fmt.Println(">", cmd.Use, strings.Join(args, " "))
 	}
@@ -745,7 +743,6 @@ func printResponse(cmd *cobra.Command, args []string, rsps ...response) {
 			fmt.Printf("-> code: OK\n")
 		} else {
 			fmt.Printf("-> code: %d\n", rsp.Code)
-
 		}
 
 		if len(rsp.Data) != 0 {

abci/example/kvstore/helpers.go

@@ -14,7 +14,7 @@ import (
 
 // RandVal creates one random validator, with a key derived
 // from the input value
-func RandVal(i int) types.ValidatorUpdate {
+func RandVal() types.ValidatorUpdate {
 	pubkey := cmtrand.Bytes(32)
 	power := cmtrand.Uint16() + 1
 	v := types.UpdateValidator(pubkey, int64(power), "")
@@ -28,7 +28,7 @@ func RandVal(i int) types.ValidatorUpdate {
 func RandVals(cnt int) []types.ValidatorUpdate {
 	res := make([]types.ValidatorUpdate, cnt)
 	for i := 0; i < cnt; i++ {
-		res[i] = RandVal(i)
+		res[i] = RandVal()
 	}
 	return res
 }

abci/example/kvstore/kvstore.go

@@ -84,7 +84,7 @@ func (app *Application) SetGenBlockEvents() {
 // begins and let's the application know what Tendermint versions it's interacting with. Based from this information,
 // Tendermint will ensure it is in sync with the application by potentially replaying the blocks it has. If the
 // Application returns a 0 appBlockHeight, Tendermint will call InitChain to initialize the application with consensus related data
-func (app *Application) Info(_ context.Context, req *types.RequestInfo) (*types.ResponseInfo, error) {
+func (app *Application) Info(context.Context, *types.RequestInfo) (*types.ResponseInfo, error) {
 	// Tendermint expects the application to persist validators, on start-up we need to reload them to memory if they exist
 	if len(app.valAddrToPubKeyMap) == 0 && app.state.Height > 0 {
 		validators := app.getValidators()
@@ -324,8 +324,7 @@ func (app *Application) FinalizeBlock(_ context.Context, req *types.RequestFinal
 // Commit is called after FinalizeBlock and after Tendermint state which includes the updates to
 // AppHash, ConsensusParams and ValidatorSet has occurred.
 // The KVStore persists the validator updates and the new key values
-func (app *Application) Commit(_ context.Context, _ *types.RequestCommit) (*types.ResponseCommit, error) {
-
+func (app *Application) Commit(context.Context, *types.RequestCommit) (*types.ResponseCommit, error) {
 	// apply the validator updates to state (note this is really the validator set at h + 2)
 	for _, valUpdate := range app.valUpdates {
 		app.updateValidator(valUpdate)

abci/example/kvstore/kvstore_test.go

@@ -133,7 +133,6 @@ func TestPersistentKVStoreInfo(t *testing.T) {
 	resInfo, err = kvstore.Info(ctx, &types.RequestInfo{})
 	require.NoError(t, err)
 	require.Equal(t, height, resInfo.LastBlockHeight)
-
 }
 
 // add a validator, remove a validator, update a validator
@@ -200,15 +199,14 @@ func TestValUpdates(t *testing.T) {
 	vals1 = append([]types.ValidatorUpdate{v1}, vals1[1:]...)
 	vals2 = kvstore.getValidators()
 	valsEqual(t, vals1, vals2)
-
 }
 
 func TestCheckTx(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	kvstore := NewInMemoryApplication()
 
-	val := RandVal(1)
+	val := RandVal()
 
 	testCases := []struct {
 		expCode uint32
@@ -255,7 +253,8 @@ func makeApplyBlock(
 	kvstore types.Application,
 	heightInt int,
 	diff []types.ValidatorUpdate,
-	txs ...[]byte) {
+	txs ...[]byte,
+) {
 	// make and apply block
 	height := int64(heightInt)
 	hash := []byte("foo")
@@ -270,7 +269,6 @@ func makeApplyBlock(
 	require.NoError(t, err)
 
 	valsEqual(t, diff, resFinalizeBlock.ValidatorUpdates)
-
 }
 
 // order doesn't matter

abci/server/grpc_server.go

@@ -37,7 +37,6 @@ func NewGRPCServer(protoAddr string, app types.Application) service.Service {
 
 // OnStart starts the gRPC service.
 func (s *GRPCServer) OnStart() error {
-
 	ln, err := net.Listen(s.proto, s.addr)
 	if err != nil {
 		return err
@@ -72,6 +71,6 @@ func (app *gRPCApplication) Echo(_ context.Context, req *types.RequestEcho) (*ty
 	return &types.ResponseEcho{Message: req.Message}, nil
 }
 
-func (app *gRPCApplication) Flush(_ context.Context, req *types.RequestFlush) (*types.ResponseFlush, error) {
+func (app *gRPCApplication) Flush(context.Context, *types.RequestFlush) (*types.ResponseFlush, error) {
 	return &types.ResponseFlush{}, nil
 }

abci/tests/server/client.go

@@ -68,7 +68,7 @@ func FinalizeBlock(ctx context.Context, client abcicli.Client, txBytes [][]byte,
 	return nil
 }
 
-func PrepareProposal(ctx context.Context, client abcicli.Client, txBytes [][]byte, txExpected [][]byte, dataExp []byte) error {
+func PrepareProposal(ctx context.Context, client abcicli.Client, txBytes [][]byte, txExpected [][]byte, _ []byte) error {
 	res, _ := client.PrepareProposal(ctx, &types.RequestPrepareProposal{Txs: txBytes})
 	for i, tx := range res.Txs {
 		if !bytes.Equal(tx, txExpected[i]) {

abci/types/application.go

@@ -45,39 +45,39 @@ func NewBaseApplication() *BaseApplication {
 	return &BaseApplication{}
 }
 
-func (BaseApplication) Info(_ context.Context, req *RequestInfo) (*ResponseInfo, error) {
+func (BaseApplication) Info(context.Context, *RequestInfo) (*ResponseInfo, error) {
 	return &ResponseInfo{}, nil
 }
 
-func (BaseApplication) CheckTx(_ context.Context, req *RequestCheckTx) (*ResponseCheckTx, error) {
+func (BaseApplication) CheckTx(context.Context, *RequestCheckTx) (*ResponseCheckTx, error) {
 	return &ResponseCheckTx{Code: CodeTypeOK}, nil
 }
 
-func (BaseApplication) Commit(_ context.Context, req *RequestCommit) (*ResponseCommit, error) {
+func (BaseApplication) Commit(context.Context, *RequestCommit) (*ResponseCommit, error) {
 	return &ResponseCommit{}, nil
 }
 
-func (BaseApplication) Query(_ context.Context, req *RequestQuery) (*ResponseQuery, error) {
+func (BaseApplication) Query(context.Context, *RequestQuery) (*ResponseQuery, error) {
 	return &ResponseQuery{Code: CodeTypeOK}, nil
 }
 
-func (BaseApplication) InitChain(_ context.Context, req *RequestInitChain) (*ResponseInitChain, error) {
+func (BaseApplication) InitChain(context.Context, *RequestInitChain) (*ResponseInitChain, error) {
 	return &ResponseInitChain{}, nil
 }
 
-func (BaseApplication) ListSnapshots(_ context.Context, req *RequestListSnapshots) (*ResponseListSnapshots, error) {
+func (BaseApplication) ListSnapshots(context.Context, *RequestListSnapshots) (*ResponseListSnapshots, error) {
 	return &ResponseListSnapshots{}, nil
 }
 
-func (BaseApplication) OfferSnapshot(_ context.Context, req *RequestOfferSnapshot) (*ResponseOfferSnapshot, error) {
+func (BaseApplication) OfferSnapshot(context.Context, *RequestOfferSnapshot) (*ResponseOfferSnapshot, error) {
 	return &ResponseOfferSnapshot{}, nil
 }
 
-func (BaseApplication) LoadSnapshotChunk(_ context.Context, _ *RequestLoadSnapshotChunk) (*ResponseLoadSnapshotChunk, error) {
+func (BaseApplication) LoadSnapshotChunk(context.Context, *RequestLoadSnapshotChunk) (*ResponseLoadSnapshotChunk, error) {
 	return &ResponseLoadSnapshotChunk{}, nil
 }
 
-func (BaseApplication) ApplySnapshotChunk(_ context.Context, req *RequestApplySnapshotChunk) (*ResponseApplySnapshotChunk, error) {
+func (BaseApplication) ApplySnapshotChunk(context.Context, *RequestApplySnapshotChunk) (*ResponseApplySnapshotChunk, error) {
 	return &ResponseApplySnapshotChunk{}, nil
 }
 
@@ -94,15 +94,15 @@ func (BaseApplication) PrepareProposal(_ context.Context, req *RequestPreparePro
 	return &ResponsePrepareProposal{Txs: txs}, nil
 }
 
-func (BaseApplication) ProcessProposal(_ context.Context, req *RequestProcessProposal) (*ResponseProcessProposal, error) {
+func (BaseApplication) ProcessProposal(context.Context, *RequestProcessProposal) (*ResponseProcessProposal, error) {
 	return &ResponseProcessProposal{Status: ResponseProcessProposal_ACCEPT}, nil
 }
 
-func (BaseApplication) ExtendVote(_ context.Context, req *RequestExtendVote) (*ResponseExtendVote, error) {
+func (BaseApplication) ExtendVote(context.Context, *RequestExtendVote) (*ResponseExtendVote, error) {
 	return &ResponseExtendVote{}, nil
 }
 
-func (BaseApplication) VerifyVoteExtension(_ context.Context, req *RequestVerifyVoteExtension) (*ResponseVerifyVoteExtension, error) {
+func (BaseApplication) VerifyVoteExtension(context.Context, *RequestVerifyVoteExtension) (*ResponseVerifyVoteExtension, error) {
 	return &ResponseVerifyVoteExtension{
 		Status: ResponseVerifyVoteExtension_ACCEPT,
 	}, nil

blocksync/pool.go

@@ -664,9 +664,8 @@ OUTER_LOOP:
 				if peerID == bpr.peerID {
 					bpr.reset()
 					continue OUTER_LOOP
-				} else {
-					continue WAIT_LOOP
 				}
+				continue WAIT_LOOP
 			case <-bpr.gotBlockCh:
 				// We got a block!
 				// Continue the for-loop and wait til Quit.

blocksync/reactor.go

@@ -66,8 +66,8 @@ type Reactor struct {
 
 // NewReactor returns new reactor instance.
 func NewReactor(state sm.State, blockExec *sm.BlockExecutor, store *store.BlockStore,
-	blockSync bool, metrics *Metrics) *Reactor {
-
+	blockSync bool, metrics *Metrics,
+) *Reactor {
 	if state.LastBlockHeight != store.Height() {
 		panic(fmt.Sprintf("state (%v) and store (%v) height mismatch", state.LastBlockHeight,
 			store.Height()))
@@ -169,15 +169,13 @@ func (bcR *Reactor) AddPeer(peer p2p.Peer) {
 }
 
 // RemovePeer implements Reactor by removing peer from the pool.
-func (bcR *Reactor) RemovePeer(peer p2p.Peer, reason interface{}) {
+func (bcR *Reactor) RemovePeer(peer p2p.Peer, _ interface{}) {
 	bcR.pool.RemovePeer(peer.ID())
 }
 
 // respondToPeer loads a block and sends it to the requesting peer,
 // if we have it. Otherwise, we'll respond saying we don't have it.
-func (bcR *Reactor) respondToPeer(msg *bcproto.BlockRequest,
-	src p2p.Peer) (queued bool) {
-
+func (bcR *Reactor) respondToPeer(msg *bcproto.BlockRequest, src p2p.Peer) (queued bool) {
 	block := bcR.store.LoadBlock(msg.Height)
 	if block == nil {
 		bcR.Logger.Info("Peer asking for a block we don't have", "src", src, "height", msg.Height)

cmd/cometbft/commands/debug/kill.go

@@ -32,7 +32,7 @@ $ cometbft debug 34255 /path/to/cmt-debug.zip`,
 	RunE: killCmdHandler,
 }
 
-func killCmdHandler(cmd *cobra.Command, args []string) error {
+func killCmdHandler(_ *cobra.Command, args []string) error {
 	pid, err := strconv.ParseUint(args[0], 10, 64)
 	if err != nil {
 		return err

cmd/cometbft/commands/gen_node_key.go

@@ -19,7 +19,7 @@ var GenNodeKeyCmd = &cobra.Command{
 	RunE:    genNodeKey,
 }
 
-func genNodeKey(cmd *cobra.Command, args []string) error {
+func genNodeKey(*cobra.Command, []string) error {
 	nodeKeyFile := config.NodeKeyFile()
 	if cmtos.FileExists(nodeKeyFile) {
 		return fmt.Errorf("node key at %s already exists", nodeKeyFile)

cmd/cometbft/commands/gen_validator.go

@@ -19,7 +19,7 @@ var GenValidatorCmd = &cobra.Command{
 	Run:     genValidator,
 }
 
-func genValidator(cmd *cobra.Command, args []string) {
+func genValidator(*cobra.Command, []string) {
 	pv := privval.GenFilePV("", "")
 	jsbz, err := cmtjson.Marshal(pv)
 	if err != nil {