lint: fix linter errors and update CI to require passing

.github/workflows/reviewdog.yml

@@ -20,10 +20,11 @@ jobs:
         with:
           golangci_lint_version: "v1.41.1"
           golangci_lint_flags: "-c .golangci.yml --allow-parallel-runners"
-          reporter: "github-pr-review"
+          reporter: "github-pr-check"
           tool_name: "Lint Errors"
           level: "error"
           fail_on_error: true
+          filter_mode: "nofilter"
   # Non-Blocking Warnings Section
   reviewdog-warnings:
     runs-on: ubuntu-latest

.golangci.yml

@@ -7,7 +7,7 @@ linters:
   enable:
     - errcheck
     - gofmt
-    - golint
+    - revive
     - govet
     - ineffassign
     - misspell
@@ -41,6 +41,8 @@ issues:
     - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
     # "EXC0005 staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore"
     - ineffective break statement. Did you mean to break out of the outer loop
+    # revive: irrelevant error about naming
+    - "var-naming: don't use leading k in Go names"
 
   exclude-rules:
     # Add all linters here -- Comment this block out for testing linters

cmd/catchpointdump/net.go

@@ -303,15 +303,24 @@ func deleteLedgerFiles(deleteTracker bool) error {
 
 func loadAndDump(addr string, tarFile string, genesisInitState ledgercore.InitState) error {
 	// delete current ledger files.
-	deleteLedgerFiles(true)
+	err := deleteLedgerFiles(true)
+	if err != nil {
+		reportErrorf("Unable to delete ledger files : %v", err)
+		return err
+	}
 	cfg := config.GetDefaultLocal()
 	l, err := ledger.OpenLedger(logging.Base(), "./ledger", false, genesisInitState, cfg)
 	if err != nil {
 		reportErrorf("Unable to open ledger : %v", err)
 		return err
 	}
 
-	defer deleteLedgerFiles(!loadOnly)
+	defer func() {
+		err := deleteLedgerFiles(!loadOnly)
+		if err != nil {
+			reportErrorf("Unable to delete ledger files : %v", err)
+		}
+	}()
 	defer l.Close()
 
 	catchupAccessor := ledger.MakeCatchpointCatchupAccessor(l, logging.Base())

crypto/merklesignature/const.go

@@ -18,6 +18,7 @@ package merklesignature
 
 import (
 	"fmt"
+
 	"github.com/algorand/go-algorand/crypto"
 )
 
@@ -40,7 +41,7 @@ const (
 var NoKeysCommitment = Commitment{}
 
 func init() {
-	// no keys generated, inner tree of merkle siganture scheme is empty.
+	// no keys generated, inner tree of merkle signature scheme is empty.
 	o, err := New(KeyLifetimeDefault+1, KeyLifetimeDefault+2, KeyLifetimeDefault)
 	if err != nil {
 		panic(fmt.Errorf("initializing empty merkle signature scheme failed, err: %w", err))

crypto/stateproof/coinGenerator.go

@@ -18,9 +18,10 @@ package stateproof
 
 import (
 	"encoding/binary"
-	"golang.org/x/crypto/sha3"
 	"math/big"
 
+	"golang.org/x/crypto/sha3"
+
 	"github.com/algorand/go-algorand/crypto"
 	"github.com/algorand/go-algorand/protocol"
 )
@@ -75,7 +76,7 @@ func makeCoinGenerator(choice *coinChoiceSeed) coinGenerator {
 	choice.version = VersionForCoinGenerator
 	rep := crypto.HashRep(choice)
 	shk := sha3.NewShake256()
-	shk.Write(rep)
+	shk.Write(rep) //nolint:errcheck
 
 	threshold := prepareRejectionSamplingThreshold(choice.signedWeight)
 	return coinGenerator{shkContext: shk, signedWeight: choice.signedWeight, threshold: threshold}
@@ -111,7 +112,7 @@ func (cg *coinGenerator) getNextCoin() uint64 {
 	var randNumFromXof uint64
 	for {
 		var shakeDigest [8]byte
-		cg.shkContext.Read(shakeDigest[:])
+		cg.shkContext.Read(shakeDigest[:]) //nolint:errcheck
 		randNumFromXof = binary.LittleEndian.Uint64(shakeDigest[:])
 
 		z := &big.Int{}

data/transactions/logic/assembler.go

@@ -1230,7 +1230,7 @@ func pseudoImmediatesError(ops *OpStream, name string, specs map[int]OpSpec) {
 		immediateCounts[i] = numImms
 		i++
 	}
-	ops.error(name + " expects " + joinIntsOnOr("immediate argument", immediateCounts...))
+	ops.error(name + " expects " + joinIntsOnOr("immediate argument", immediateCounts...)) //nolint:errcheck
 }
 
 // getSpec finds the OpSpec we need during assembly based on its name, our current version, and the immediates passed in
@@ -1255,7 +1255,7 @@ func getSpec(ops *OpStream, name string, args []string) (OpSpec, string, bool) {
 		}
 		pseudo.Name = name
 		if pseudo.Version > ops.Version {
-			ops.errorf("%s opcode with %s was introduced in v%d", pseudo.Name, joinIntsOnOr("immediate", len(args)), pseudo.Version)
+			ops.errorf("%s opcode with %s was introduced in v%d", pseudo.Name, joinIntsOnOr("immediate", len(args)), pseudo.Version) //nolint:errcheck
 		}
 		if len(args) == 0 {
 			return pseudo, pseudo.Name + " without immediates", true
@@ -1266,9 +1266,9 @@ func getSpec(ops *OpStream, name string, args []string) (OpSpec, string, bool) {
 	if !ok {
 		spec, ok = OpsByName[AssemblerMaxVersion][name]
 		if ok {
-			ops.errorf("%s opcode was introduced in v%d", name, spec.Version)
+			ops.errorf("%s opcode was introduced in v%d", name, spec.Version) //nolint:errcheck
 		} else {
-			ops.errorf("unknown opcode: %s", name)
+			ops.errorf("unknown opcode: %s", name) //nolint:errcheck
 		}
 	}
 	return spec, spec.Name, ok
@@ -1496,7 +1496,7 @@ func (ops *OpStream) trace(format string, args ...interface{}) {
 
 func (ops *OpStream) typeError(err error) {
 	if ops.typeTracking {
-		ops.error(err)
+		ops.error(err) //nolint:errcheck
 	}
 }
 
@@ -1571,10 +1571,10 @@ func (ops *OpStream) assemble(text string) error {
 				directive := first[1:]
 				switch directive {
 				case "pragma":
-					ops.pragma(tokens)
+					ops.pragma(tokens) //nolint:errcheck
 					ops.trace("%3d: #pragma line\n", ops.sourceLine)
 				default:
-					ops.errorf("Unknown directive: %s", directive)
+					ops.errorf("Unknown directive: %s", directive) //nolint:errcheck
 				}
 				continue
 			}
@@ -1618,8 +1618,8 @@ func (ops *OpStream) assemble(text string) error {
 					}
 				}
 				ops.trackStack(args, returns, append([]string{expandedName}, current[1:]...))
-				spec.asm(ops, &spec, current[1:])
-				if spec.deadens() { // An unconditional branch deadens the following code
+				spec.asm(ops, &spec, current[1:]) //nolint:errcheck
+				if spec.deadens() {               // An unconditional branch deadens the following code
 					ops.known.deaden()
 				}
 				if spec.Name == "callsub" {
@@ -1636,7 +1636,7 @@ func (ops *OpStream) assemble(text string) error {
 		if errors.Is(err, bufio.ErrTooLong) {
 			err = errors.New("line too long")
 		}
-		ops.error(err)
+		ops.error(err) //nolint:errcheck
 	}
 
 	// backward compatibility: do not allow jumps behind last instruction in v1
@@ -1741,12 +1741,12 @@ func (ops *OpStream) resolveLabels() {
 		// all branch instructions (currently) are opcode byte and 2 offset bytes, and the destination is relative to the next pc as if the branch was a no-op
 		naturalPc := lr.position + 3
 		if ops.Version < backBranchEnabledVersion && dest < naturalPc {
-			ops.errorf("label %#v is a back reference, back jump support was introduced in v4", lr.label)
+			ops.errorf("label %#v is a back reference, back jump support was introduced in v4", lr.label) //nolint:errcheck
 			continue
 		}
 		jump := dest - naturalPc
 		if jump > 0x7fff {
-			ops.errorf("label %#v is too far away", lr.label)
+			ops.errorf("label %#v is too far away", lr.label) //nolint:errcheck
 			continue
 		}
 		raw[lr.position+1] = uint8(jump >> 8)

data/transactions/logic/eval.go

@@ -1341,17 +1341,17 @@ func opLt(cx *EvalContext) error {
 // opSwap, opLt, and opNot always succeed (return nil). So error checking elided in Gt,Le,Ge
 
 func opGt(cx *EvalContext) error {
-	opSwap(cx)
+	opSwap(cx) //nolint:errcheck
 	return opLt(cx)
 }
 
 func opLe(cx *EvalContext) error {
-	opGt(cx)
+	opGt(cx) //nolint:errcheck
 	return opNot(cx)
 }
 
 func opGe(cx *EvalContext) error {
-	opLt(cx)
+	opLt(cx) //nolint:errcheck
 	return opNot(cx)
 }
 

ledger/accountdb.go

@@ -302,7 +302,7 @@ type compactAccountDeltas struct {
 }
 
 // onlineAccountDelta track all changes of account state within a range,
-// used in conjunction wih compactOnlineAccountDeltas to group and represent per-account changes.
+// used in conjunction with compactOnlineAccountDeltas to group and represent per-account changes.
 // oldAcct represents the "old" state of the account in the DB, and compared against newAcct[0]
 // to determine if the acct became online or went offline.
 type onlineAccountDelta struct {

ledger/acctonline.go

@@ -634,7 +634,6 @@ func (ao *onlineAccounts) lookupOnlineAccountData(rnd basics.Round, addr basics.
 			}
 			// the round number cannot be found in deltas, it is in history
 			inHistory = true
-			err = nil
 		}
 		paramsOffset, err = ao.roundParamsOffset(rnd)
 		if err != nil {
@@ -764,7 +763,6 @@ func (ao *onlineAccounts) TopOnlineAccounts(rnd basics.Round, voteRnd basics.Rou
 			}
 			// the round number cannot be found in deltas, it is in history
 			inMemory = false
-			err = nil
 		}
 
 		modifiedAccounts := make(map[basics.Address]*ledgercore.OnlineAccount)

network/wsNetwork.go

@@ -201,7 +201,7 @@ type GossipNode interface {
 	// this node to send corresponding MsgOfInterest notifications to any
 	// newly connecting peers.  This should be called before the network
 	// is started.
-	RegisterMessageInterest(protocol.Tag) error
+	RegisterMessageInterest(protocol.Tag)
 
 	// SubstituteGenesisID substitutes the "{genesisID}" with their network-specific genesisID.
 	SubstituteGenesisID(rawURL string) string
@@ -2308,7 +2308,7 @@ func SetUserAgentHeader(header http.Header) {
 // this node to send corresponding MsgOfInterest notifications to any
 // newly connecting peers.  This should be called before the network
 // is started.
-func (wn *WebsocketNetwork) RegisterMessageInterest(t protocol.Tag) error {
+func (wn *WebsocketNetwork) RegisterMessageInterest(t protocol.Tag) {
 	wn.messagesOfInterestMu.Lock()
 	defer wn.messagesOfInterestMu.Unlock()
 
@@ -2321,11 +2321,10 @@ func (wn *WebsocketNetwork) RegisterMessageInterest(t protocol.Tag) error {
 
 	wn.messagesOfInterest[t] = true
 	wn.updateMessagesOfInterestEnc()
-	return nil
 }
 
 // DeregisterMessageInterest will tell peers to no longer send us traffic with a protocol Tag
-func (wn *WebsocketNetwork) DeregisterMessageInterest(t protocol.Tag) error {
+func (wn *WebsocketNetwork) DeregisterMessageInterest(t protocol.Tag) {
 	wn.messagesOfInterestMu.Lock()
 	defer wn.messagesOfInterestMu.Unlock()
 
@@ -2338,7 +2337,6 @@ func (wn *WebsocketNetwork) DeregisterMessageInterest(t protocol.Tag) error {
 
 	delete(wn.messagesOfInterest, t)
 	wn.updateMessagesOfInterestEnc()
-	return nil
 }
 
 func (wn *WebsocketNetwork) updateMessagesOfInterestEnc() {

network/wsNetwork_test.go

@@ -1838,7 +1838,7 @@ func TestWebsocketNetworkMessageOfInterest(t *testing.T) {
 	waitReady(t, netB, readyTimeout.C)
 
 	// have netB asking netA to send it only AgreementVoteTag and ProposalPayloadTag
-	require.NoError(t, netB.RegisterMessageInterest(ft2))
+	netB.RegisterMessageInterest(ft2)
 	// send another message which we can track, so that we'll know that the first message was delivered.
 	netB.Broadcast(context.Background(), protocol.VoteBundleTag, []byte{0, 1, 2, 3, 4}, true, nil)
 	messageFilterArriveWg.Wait()

protocol/codec_tester.go

@@ -433,7 +433,7 @@ func RunEncodingTest(t *testing.T, template msgpMarshalUnmarshal) {
 
 		// some objects might appen to the original error additional info.
 		// we ensure that invalidObject error is not failing the test.
-		if errors.As(err, &ErrInvalidObject) {
+		if errors.Is(err, ErrInvalidObject) {
 			continue
 		}
 		require.NoError(t, err)

test/e2e-go/cli/goal/expect/catchpointCatchupWebProxy/webproxy.go

@@ -58,7 +58,7 @@ func main() {
 		// prevent requests for block #2 to go through.
 		if strings.HasSuffix(request.URL.String(), "/block/2") {
 			response.WriteHeader(http.StatusBadRequest)
-			response.Write([]byte("webProxy prevents block 2 from serving"))
+			response.Write([]byte("webProxy prevents block 2 from serving")) //nolint:errcheck
 			return
 		}
 		if *webProxyLogFile != "" {