lint: fix collection of stale errors (#7090)

Few things that had been annoying.

Author: tychoish

.github/workflows/lint.yml

@@ -23,7 +23,7 @@ jobs:
       - uses: golangci/golangci-lint-action@v2.5.2
         with:
           # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-          version: v1.38
+          version: v1.42.1
           args: --timeout 10m
           github-token: ${{ secrets.github_token }}
         if: env.GIT_DIFF

.golangci.yml

@@ -13,12 +13,12 @@ linters:
     # - gochecknoinits
     # - gocognit
     - goconst
-    - gocritic
+    # - gocritic
     # - gocyclo
     # - godox
     - gofmt
     - goimports
-    - golint
+    - revive
     - gosec
     - gosimple
     - govet

internal/consensus/msgs.go

@@ -77,7 +77,7 @@ func (m *NewRoundStepMessage) ValidateHeight(initialHeight int64) error {
 			m.LastCommitRound, initialHeight)
 	}
 	if m.Height > initialHeight && m.LastCommitRound < 0 {
-		return fmt.Errorf("LastCommitRound can only be negative for initial height %v", // nolint
+		return fmt.Errorf("LastCommitRound can only be negative for initial height %v",
 			initialHeight)
 	}
 	return nil

internal/libs/protoio/io_test.go

@@ -71,7 +71,7 @@ func iotest(writer protoio.WriteCloser, reader protoio.ReadCloser) error {
 			return err
 		}
 		if n != len(bz)+visize {
-			return fmt.Errorf("WriteMsg() wrote %v bytes, expected %v", n, len(bz)+visize) // nolint
+			return fmt.Errorf("WriteMsg() wrote %v bytes, expected %v", n, len(bz)+visize)
 		}
 		lens[i] = n
 	}

internal/mempool/ids.go

@@ -7,17 +7,15 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
-// nolint: golint
-// TODO: Rename type.
-type MempoolIDs struct {
+type IDs struct {
 	mtx       tmsync.RWMutex
 	peerMap   map[types.NodeID]uint16
 	nextID    uint16              // assumes that a node will never have over 65536 active peers
 	activeIDs map[uint16]struct{} // used to check if a given peerID key is used
 }
 
-func NewMempoolIDs() *MempoolIDs {
-	return &MempoolIDs{
+func NewMempoolIDs() *IDs {
+	return &IDs{
 		peerMap: make(map[types.NodeID]uint16),
 
 		// reserve UnknownPeerID for mempoolReactor.BroadcastTx
@@ -28,7 +26,7 @@ func NewMempoolIDs() *MempoolIDs {
 
 // ReserveForPeer searches for the next unused ID and assigns it to the provided
 // peer.
-func (ids *MempoolIDs) ReserveForPeer(peerID types.NodeID) {
+func (ids *IDs) ReserveForPeer(peerID types.NodeID) {
 	ids.mtx.Lock()
 	defer ids.mtx.Unlock()
 
@@ -38,7 +36,7 @@ func (ids *MempoolIDs) ReserveForPeer(peerID types.NodeID) {
 }
 
 // Reclaim returns the ID reserved for the peer back to unused pool.
-func (ids *MempoolIDs) Reclaim(peerID types.NodeID) {
+func (ids *IDs) Reclaim(peerID types.NodeID) {
 	ids.mtx.Lock()
 	defer ids.mtx.Unlock()
 
@@ -50,7 +48,7 @@ func (ids *MempoolIDs) Reclaim(peerID types.NodeID) {
 }
 
 // GetForPeer returns an ID reserved for the peer.
-func (ids *MempoolIDs) GetForPeer(peerID types.NodeID) uint16 {
+func (ids *IDs) GetForPeer(peerID types.NodeID) uint16 {
 	ids.mtx.RLock()
 	defer ids.mtx.RUnlock()
 
@@ -59,7 +57,7 @@ func (ids *MempoolIDs) GetForPeer(peerID types.NodeID) uint16 {
 
 // nextPeerID returns the next unused peer ID to use. We assume that the mutex
 // is already held.
-func (ids *MempoolIDs) nextPeerID() uint16 {
+func (ids *IDs) nextPeerID() uint16 {
 	if len(ids.activeIDs) == MaxActiveIDs {
 		panic(fmt.Sprintf("node has maximum %d active IDs and wanted to get one more", MaxActiveIDs))
 	}

internal/mempool/v0/reactor.go

@@ -39,7 +39,7 @@ type Reactor struct {
 
 	cfg     *config.MempoolConfig
 	mempool *CListMempool
-	ids     *mempool.MempoolIDs
+	ids     *mempool.IDs
 
 	// XXX: Currently, this is the only way to get information about a peer. Ideally,
 	// we rely on message-oriented communication to get necessary peer data.

internal/mempool/v1/reactor.go

@@ -39,7 +39,7 @@ type Reactor struct {
 
 	cfg     *config.MempoolConfig
 	mempool *TxMempool
-	ids     *mempool.MempoolIDs
+	ids     *mempool.IDs
 
 	// XXX: Currently, this is the only way to get information about a peer. Ideally,
 	// we rely on message-oriented communication to get necessary peer data.

internal/p2p/peermanager.go

@@ -180,7 +180,7 @@ func (o *PeerManagerOptions) Validate() error {
 
 	if o.MaxPeers > 0 {
 		if o.MaxConnected == 0 || o.MaxConnected+o.MaxConnectedUpgrade > o.MaxPeers {
-			return fmt.Errorf("MaxConnected %v and MaxConnectedUpgrade %v can't exceed MaxPeers %v", // nolint
+			return fmt.Errorf("MaxConnected %v and MaxConnectedUpgrade %v can't exceed MaxPeers %v",
 				o.MaxConnected, o.MaxConnectedUpgrade, o.MaxPeers)
 		}
 	}
@@ -190,7 +190,7 @@ func (o *PeerManagerOptions) Validate() error {
 			return errors.New("can't set MaxRetryTime without MinRetryTime")
 		}
 		if o.MinRetryTime > o.MaxRetryTime {
-			return fmt.Errorf("MinRetryTime %v is greater than MaxRetryTime %v", // nolint
+			return fmt.Errorf("MinRetryTime %v is greater than MaxRetryTime %v",
 				o.MinRetryTime, o.MaxRetryTime)
 		}
 	}
@@ -200,7 +200,7 @@ func (o *PeerManagerOptions) Validate() error {
 			return errors.New("can't set MaxRetryTimePersistent without MinRetryTime")
 		}
 		if o.MinRetryTime > o.MaxRetryTimePersistent {
-			return fmt.Errorf("MinRetryTime %v is greater than MaxRetryTimePersistent %v", // nolint
+			return fmt.Errorf("MinRetryTime %v is greater than MaxRetryTimePersistent %v",
 				o.MinRetryTime, o.MaxRetryTimePersistent)
 		}
 	}

internal/state/state.go

@@ -195,8 +195,8 @@ func (state *State) ToProto() (*tmstate.State, error) {
 	return sm, nil
 }
 
-// StateFromProto takes a state proto message & returns the local state type
-func StateFromProto(pb *tmstate.State) (*State, error) { //nolint:golint
+// FromProto takes a state proto message & returns the local state type
+func FromProto(pb *tmstate.State) (*State, error) {
 	if pb == nil {
 		return nil, errors.New("nil State")
 	}

internal/state/state_test.go

@@ -1079,7 +1079,7 @@ func TestStateProto(t *testing.T) {
 			assert.NoError(t, err, tt.testName)
 		}
 
-		smt, err := sm.StateFromProto(pbs)
+		smt, err := sm.FromProto(pbs)
 		if tt.expPass2 {
 			require.NoError(t, err, tt.testName)
 			require.Equal(t, tt.state, smt, tt.testName)

internal/state/store.go

@@ -130,7 +130,7 @@ func (store dbStore) loadState(key []byte) (state State, err error) {
 		%v\n`, err))
 	}
 
-	sm, err := StateFromProto(sp)
+	sm, err := FromProto(sp)
 	if err != nil {
 		return state, err
 	}

internal/state/validation_test.go

@@ -270,7 +270,7 @@ func TestValidateBlockEvidence(t *testing.T) {
 				A block with too much evidence fails
 			*/
 			evidence := make([]types.Evidence, 0)
-			var currentBytes int64 = 0
+			var currentBytes int64
 			// more bytes than the maximum allowed for evidence
 			for currentBytes <= maxBytesEvidence {
 				newEv := types.NewMockDuplicateVoteEvidenceWithValidator(height, time.Now(),
@@ -290,7 +290,7 @@ func TestValidateBlockEvidence(t *testing.T) {
 			A good block with several pieces of good evidence passes
 		*/
 		evidence := make([]types.Evidence, 0)
-		var currentBytes int64 = 0
+		var currentBytes int64
 		// precisely the amount of allowed evidence
 		for {
 			newEv := types.NewMockDuplicateVoteEvidenceWithValidator(height, defaultEvidenceTime,

internal/statesync/reactor.go

@@ -734,7 +734,7 @@ func (r *Reactor) handleLightBlockMessage(envelope p2p.Envelope) error {
 		}
 
 	case *ssproto.LightBlockResponse:
-		var height int64 = 0
+		var height int64
 		if msg.LightBlock != nil {
 			height = msg.LightBlock.SignedHeader.Header.Height
 		}

internal/store/store.go

@@ -345,7 +345,7 @@ func (bs *BlockStore) pruneRange(
 	var (
 		err         error
 		pruned      uint64
-		totalPruned uint64 = 0
+		totalPruned uint64
 	)
 
 	batch := bs.db.NewBatch()
@@ -392,7 +392,7 @@ func (bs *BlockStore) batchDelete(
 	start, end []byte,
 	preDeletionHook func(key, value []byte, batch dbm.Batch) error,
 ) (uint64, []byte, error) {
-	var pruned uint64 = 0
+	var pruned uint64
 	iter, err := bs.db.Iterator(start, end)
 	if err != nil {
 		return pruned, start, err

libs/json/helpers_test.go

@@ -61,7 +61,6 @@ func (c CustomValue) MarshalJSON() ([]byte, error) {
 }
 
 func (c CustomValue) UnmarshalJSON(bz []byte) error {
-	c.Value = "custom"
 	return nil
 }
 

node/node_test.go

@@ -249,7 +249,7 @@ func TestCreateProposalBlock(t *testing.T) {
 
 	// fill the evidence pool with more evidence
 	// than can fit in a block
-	var currentBytes int64 = 0
+	var currentBytes int64
 	for currentBytes <= maxEvidenceBytes {
 		ev := types.NewMockDuplicateVoteEvidenceWithValidator(height, time.Now(), privVals[0], "test-chain")
 		currentBytes += int64(len(ev.Bytes()))

types/validation.go

@@ -162,9 +162,9 @@ func verifyCommitBatch(
 	var (
 		val                *Validator
 		valIdx             int32
-		seenVals                 = make(map[int32]int, len(commit.Signatures))
-		batchSigIdxs             = make([]int, 0, len(commit.Signatures))
-		talliedVotingPower int64 = 0
+		talliedVotingPower int64
+		seenVals           = make(map[int32]int, len(commit.Signatures))
+		batchSigIdxs       = make([]int, 0, len(commit.Signatures))
 	)
 	// attempt to create a batch verifier
 	bv, ok := batch.CreateBatchVerifier(vals.GetProposer().PubKey)
@@ -275,9 +275,9 @@ func verifyCommitSingle(
 	var (
 		val                *Validator
 		valIdx             int32
-		seenVals                 = make(map[int32]int, len(commit.Signatures))
-		talliedVotingPower int64 = 0
+		talliedVotingPower int64
 		voteSignBytes      []byte
+		seenVals           = make(map[int32]int, len(commit.Signatures))
 	)
 	for idx, commitSig := range commit.Signatures {
 		if ignoreSig(commitSig) {

types/validator_set_test.go

@@ -508,7 +508,7 @@ func TestAveragingInIncrementProposerPriority(t *testing.T) {
 				{Address: []byte("c"), ProposerPriority: 1}}},
 			// this should average twice but the average should be 0 after the first iteration
 			// (voting power is 0 -> no changes)
-			11, 1 / 3},
+			11, 0},
 		2: {ValidatorSet{
 			Validators: []*Validator{
 				{Address: []byte("a"), ProposerPriority: 100},