fix: Linters warning fixes (#74)

* feat: use the latest golangci-lint version and fix config warnings

* fix: linter warnings

* fix: linter warnings (part 2)

* fix: propagate the error from aggregator.Start

* fix: format golangci config file

* fix: suppress gosec overflow issues

* fix: exclude G115 gosec linter rule

* fix: use crypto/rand number generator

.github/workflows/lint.yml

@@ -20,4 +20,4 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v6
         with:
-          version: v1.60
+          version: v1.61

.golangci.yml

@@ -3,71 +3,65 @@
 run:
   timeout: 3m
   tests: true
-  # default is true. Enables skipping of directories:
-  #   vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
   skip-dirs-use-default: true
-  skip-dirs:
-    - tests
-    - aggregator/db/migrations
 
 service:
-  golangci-lint-version: 1.59.1
+  golangci-lint-version: 1.61.0
 
 linters:
   disable-all: true
   enable:
-  - whitespace # Tool for detection of leading and trailing whitespace
-  # - wsl # Forces you to use empty lines
-  - wastedassign # Finds wasted assignment statements
-  - unconvert  # Unnecessary type conversions
-  - tparallel # Detects inappropriate usage of t.Parallel() method in your Go test codes
-  - thelper # Detects golang test helpers without t.Helper() call and checks the consistency of test helpers
-  - stylecheck # Stylecheck is a replacement for golint
-  - prealloc # Finds slice declarations that could potentially be pre-allocated
-  - predeclared # Finds code that shadows one of Go's predeclared identifiers
-  - nolintlint # Ill-formed or insufficient nolint directives
-  # - nlreturn # Checks for a new line before return and branch statements to increase code clarity
-  - misspell # Misspelled English words in comments
-  - makezero # Finds slice declarations with non-zero initial length
-  - lll # Long lines
-  - importas  # Enforces consistent import aliases
-  - gosec # Security problems
-  - gofmt # Whether the code was gofmt-ed
-  - goimports # Unused imports
-  - goconst # Repeated strings that could be replaced by a constant
-  - forcetypeassert # Finds forced type assertions
-  - dogsled # Checks assignments with too many blank identifiers (e.g. x, , , _, := f())
-  - dupl # Code clone detection
-  - errname # Checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
-  - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13
-  - gocritic # gocritic is a Go source code linter that maintains checks that are not in other linters
-  - errcheck # Errcheck is a go lint rule for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases
-  # - godox # Godox is a linter for TODOs and FIXMEs left in the code
-  - gci # Gci is a linter for checking the consistency of the code with the go code style guide
-  - gomnd # Gomnd is a linter for magic numbers
-  # - revive 
-  - unparam # Unparam is a linter for unused function parameters
+    - whitespace # Tool for detection of leading and trailing whitespace
+    # - wsl # Forces you to use empty lines
+    - wastedassign # Finds wasted assignment statements
+    - unconvert  # Unnecessary type conversions
+    - tparallel # Detects inappropriate usage of t.Parallel() method in your Go test codes
+    - thelper # Detects golang test helpers without t.Helper() call and checks the consistency of test helpers
+    - stylecheck # Stylecheck is a replacement for golint
+    - prealloc # Finds slice declarations that could potentially be pre-allocated
+    - predeclared # Finds code that shadows one of Go's predeclared identifiers
+    - nolintlint # Ill-formed or insufficient nolint directives
+    # - nlreturn # Checks for a new line before return and branch statements to increase code clarity
+    - misspell # Misspelled English words in comments
+    - makezero # Finds slice declarations with non-zero initial length
+    - lll # Long lines
+    - importas  # Enforces consistent import aliases
+    - gosec # Security problems
+    - gofmt # Whether the code was gofmt-ed
+    - goimports # Unused imports
+    - goconst # Repeated strings that could be replaced by a constant
+    - forcetypeassert # Finds forced type assertions
+    - dogsled # Checks assignments with too many blank identifiers (e.g. x, , , _, := f())
+    - dupl # Code clone detection
+    - errname # Checks that sentinel errors are prefixed with Err and error types are suffixed with Error
+    - errorlint # Error wrapping introduced in Go 1.13
+    - gocritic # gocritic is a Go source code linter that maintains checks that are not in other linters
+    - errcheck # Errcheck is a go lint rule for checking for unchecked errors
+    # - godox # Linter for TODOs and FIXMEs left in the code
+    - gci # Gci checks the consistency of the code with the Go code style guide
+    - mnd # mnd is a linter for magic numbers
+    # - revive 
+    - unparam # Unused function parameters
 
 linters-settings:
   gofmt:
     simplify: true
   gocritic:
     enabled-checks:
       - ruleguard
-    # settings:
-    #   ruleguard:
-    #     rules: "./gorules/rules.go"
   revive:
     rules:
-    - name: exported
-      arguments:
-      - disableStutteringCheck
+      - name: exported
+        arguments:
+          - disableStutteringCheck
   goconst:
     min-len: 3
     min-occurrences: 3
+  gosec:
+     excludes:
+      - G115  # Potential integer overflow when converting between integer types
 
 issues:
-  # new-from-rev: origin/develop # report only new issues with reference to develop branch
   whole-files: true
   exclude-rules:
     - path: '(_test\.go|^test/.*)'
@@ -78,9 +72,11 @@ issues:
     - path: 'etherman/contracts/contracts_(banana|elderberry)\.go'
       linters:
         - dupl
+  exclude-dirs:
+    - tests
+    - aggregator/db/migrations
   include:
     - EXC0012 # Exported (.+) should have comment( \(or a comment on this block\))? or be unexported
     - EXC0013 # Package comment should be of the form "(.+)...
     - EXC0014 # Comment on exported (.+) should be of the form "(.+)..."
     - EXC0015 # Should have a package comment
-

aggoracle/oracle.go

@@ -63,12 +63,15 @@ func (a *AggOracle) Start(ctx context.Context) {
 		case <-a.ticker.C:
 			blockNumToFetch, gerToInject, err = a.getLastFinalisedGER(ctx, blockNumToFetch)
 			if err != nil {
-				if errors.Is(err, l1infotreesync.ErrBlockNotProcessed) {
+				switch {
+				case errors.Is(err, l1infotreesync.ErrBlockNotProcessed):
 					log.Debugf("syncer is not ready for the block %d", blockNumToFetch)
-				} else if errors.Is(err, l1infotreesync.ErrNotFound) {
+
+				case errors.Is(err, l1infotreesync.ErrNotFound):
 					blockNumToFetch = 0
 					log.Debugf("syncer has not found any GER until block %d", blockNumToFetch)
-				} else {
+
+				default:
 					log.Error("error calling getLastFinalisedGER: ", err)
 				}
 

aggregator/aggregator.go

@@ -277,7 +277,7 @@ func (a *Aggregator) handleReorg(reorgData synchronizer.ReorgExecutionResult) {
 			"Halting the aggregator due to a L1 reorg. " +
 				"Reorged data has been deleted, so it is safe to manually restart the aggregator.",
 		)
-		time.Sleep(10 * time.Second) //nolint:gomnd
+		time.Sleep(10 * time.Second) //nolint:mnd
 	}
 }
 
@@ -375,6 +375,7 @@ func (a *Aggregator) handleRollbackBatches(rollbackData synchronizer.RollbackBat
 		}
 
 		marshalledBookMark, err = proto.Marshal(bookMark)
+		//nolint:gocritic
 		if err != nil {
 			log.Error("failed to marshal bookmark: %v", err)
 		} else {
@@ -403,7 +404,7 @@ func (a *Aggregator) handleRollbackBatches(rollbackData synchronizer.RollbackBat
 		a.halted.Store(true)
 		for {
 			log.Errorf("Halting the aggregator due to an error handling rollback batches event: %v", err)
-			time.Sleep(10 * time.Second) //nolint:gomnd
+			time.Sleep(10 * time.Second) //nolint:mnd
 		}
 	}
 }
@@ -738,7 +739,7 @@ func (a *Aggregator) Start() error {
 
 		err = a.streamClient.Start()
 		if err != nil {
-			log.Fatalf("failed to start stream client, error: %v", err)
+			return fmt.Errorf("failed to start stream client, error: %w", err)
 		}
 
 		bookMark := &datastream.BookMark{
@@ -748,12 +749,12 @@ func (a *Aggregator) Start() error {
 
 		marshalledBookMark, err := proto.Marshal(bookMark)
 		if err != nil {
-			log.Fatalf("failed to marshal bookmark: %v", err)
+			return fmt.Errorf("failed to marshal bookmark: %w", err)
 		}
 
 		err = a.streamClient.ExecCommandStartBookmark(marshalledBookMark)
 		if err != nil {
-			log.Fatalf("failed to connect to data stream: %v", err)
+			return fmt.Errorf("failed to connect to data stream: %w", err)
 		}
 
 		// A this point everything is ready, so start serving
@@ -1151,6 +1152,7 @@ func (a *Aggregator) validateEligibleFinalProof(
 	batchNumberToVerify := lastVerifiedBatchNum + 1
 
 	if proof.BatchNumber != batchNumberToVerify {
+		//nolint:gocritic
 		if proof.BatchNumber < batchNumberToVerify && proof.BatchNumberFinal >= batchNumberToVerify {
 			// We have a proof that contains some batches below the last batch verified, anyway can be eligible as final proof
 			log.Warnf(
@@ -1764,8 +1766,9 @@ func (a *Aggregator) buildInputProver(
 	l1InfoTreeData := map[uint32]*prover.L1Data{}
 	forcedBlockhashL1 := common.Hash{}
 	l1InfoRoot := batchToVerify.L1InfoRoot.Bytes()
+	//nolint:gocritic
 	if !isForcedBatch {
-		tree, err := l1infotree.NewL1InfoTree(32, [][32]byte{}) //nolint:gomnd
+		tree, err := l1infotree.NewL1InfoTree(32, [][32]byte{}) //nolint:mnd
 		if err != nil {
 			return nil, err
 		}
@@ -1777,7 +1780,10 @@ func (a *Aggregator) buildInputProver(
 
 		aLeaves := make([][32]byte, len(leaves))
 		for i, leaf := range leaves {
-			aLeaves[i] = l1infotree.HashLeafData(leaf.GlobalExitRoot, leaf.PreviousBlockHash, uint64(leaf.Timestamp.Unix()))
+			aLeaves[i] = l1infotree.HashLeafData(
+				leaf.GlobalExitRoot,
+				leaf.PreviousBlockHash,
+				uint64(leaf.Timestamp.Unix()))
 		}
 
 		for _, l2blockRaw := range batchRawData.Blocks {
@@ -1877,10 +1883,12 @@ func (a *Aggregator) buildInputProver(
 	return inputProver, nil
 }
 
-func getWitness(batchNumber uint64, URL string, fullWitness bool) ([]byte, error) {
-	var witness string
-	var response rpc.Response
-	var err error
+func getWitness(batchNumber uint64, url string, fullWitness bool) ([]byte, error) {
+	var (
+		witness  string
+		response rpc.Response
+		err      error
+	)
 
 	witnessType := "trimmed"
 	if fullWitness {
@@ -1889,7 +1897,7 @@ func getWitness(batchNumber uint64, URL string, fullWitness bool) ([]byte, error
 
 	log.Infof("Requesting witness for batch %d of type %s", batchNumber, witnessType)
 
-	response, err = rpc.JSONRPCCall(URL, "zkevm_getBatchWitness", batchNumber, witnessType)
+	response, err = rpc.JSONRPCCall(url, "zkevm_getBatchWitness", batchNumber, witnessType)
 	if err != nil {
 		return nil, err
 	}

aggregator/profitabilitychecker.go

@@ -36,15 +36,15 @@ func NewTxProfitabilityCheckerBase(
 
 // IsProfitable checks pol collateral with min reward
 func (pc *TxProfitabilityCheckerBase) IsProfitable(ctx context.Context, polCollateral *big.Int) (bool, error) {
-	//if pc.IntervalAfterWhichBatchSentAnyway != 0 {
+	// if pc.IntervalAfterWhichBatchSentAnyway != 0 {
 	//	ok, err := isConsolidatedBatchAppeared(ctx, pc.State, pc.IntervalAfterWhichBatchSentAnyway)
 	//	if err != nil {
 	//		return false, err
 	//	}
 	//	if ok {
 	//		return true, nil
 	//	}
-	//}
+	// }
 	return polCollateral.Cmp(pc.MinReward) >= 0, nil
 }
 
@@ -64,20 +64,20 @@ func NewTxProfitabilityCheckerAcceptAll(state stateInterface, interval time.Dura
 
 // IsProfitable validate batch anyway and don't check anything
 func (pc *TxProfitabilityCheckerAcceptAll) IsProfitable(ctx context.Context, polCollateral *big.Int) (bool, error) {
-	//if pc.IntervalAfterWhichBatchSentAnyway != 0 {
+	// if pc.IntervalAfterWhichBatchSentAnyway != 0 {
 	//	ok, err := isConsolidatedBatchAppeared(ctx, pc.State, pc.IntervalAfterWhichBatchSentAnyway)
 	//	if err != nil {
 	//		return false, err
 	//	}
 	//	if ok {
 	//		return true, nil
 	//	}
-	//}
+	// }
 	return true, nil
 }
 
 // TODO: now it's impossible to check, when batch got consolidated, bcs it's not saved
-//func isConsolidatedBatchAppeared(ctx context.Context, state stateInterface,
+// func isConsolidatedBatchAppeared(ctx context.Context, state stateInterface,
 //  intervalAfterWhichBatchConsolidatedAnyway time.Duration) (bool, error) {
 //	batch, err := state.GetLastVerifiedBatch(ctx, nil)
 //	if err != nil {
@@ -89,4 +89,4 @@ func (pc *TxProfitabilityCheckerAcceptAll) IsProfitable(ctx context.Context, pol
 //	}
 //
 //	return false, err
-//}
+// }

aggregator/prover/prover.go

@@ -442,13 +442,13 @@ func fea2scalar(v []uint64) *big.Int {
 		return big.NewInt(0)
 	}
 	res := new(big.Int).SetUint64(v[0])
-	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[1]), 32))  //nolint:gomnd
-	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[2]), 64))  //nolint:gomnd
-	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[3]), 96))  //nolint:gomnd
-	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[4]), 128)) //nolint:gomnd
-	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[5]), 160)) //nolint:gomnd
-	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[6]), 192)) //nolint:gomnd
-	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[7]), 224)) //nolint:gomnd
+	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[1]), 32))  //nolint:mnd
+	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[2]), 64))  //nolint:mnd
+	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[3]), 96))  //nolint:mnd
+	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[4]), 128)) //nolint:mnd
+	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[5]), 160)) //nolint:mnd
+	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[6]), 192)) //nolint:mnd
+	res.Add(res, new(big.Int).Lsh(new(big.Int).SetUint64(v[7]), 224)) //nolint:mnd
 
 	return res
 }

claimsponsor/claimsponsor.go

@@ -271,15 +271,19 @@ func (c *ClaimSponsor) AddClaimToQueue(ctx context.Context, claim *Claim) error
 
 	var queuePosition uint64
 	lastQueuePosition, _, err := getLastQueueIndex(tx)
-	if errors.Is(err, ErrNotFound) {
+	switch {
+	case errors.Is(err, ErrNotFound):
 		queuePosition = 0
-	} else if err != nil {
+
+	case err != nil:
 		tx.Rollback()
 
 		return err
-	} else {
+
+	default:
 		queuePosition = lastQueuePosition + 1
 	}
+
 	err = tx.Put(queueTable, dbCommon.Uint64ToBytes(queuePosition), claim.Key())
 	if err != nil {
 		tx.Rollback()

cmd/run.go

@@ -101,6 +101,7 @@ func start(cliCtx *cli.Context) error {
 			// start aggregator in a goroutine, checking for errors
 			go func() {
 				if err := aggregator.Start(); err != nil {
+					aggregator.Stop()
 					log.Fatal(err)
 				}
 			}()

config/types/duration_test.go

@@ -43,7 +43,7 @@ func TestDurationUnmarshal(t *testing.T) {
 			err = json.Unmarshal(input, &d)
 
 			if testCase.expectedResult != nil {
-				require.Equal(t, (*testCase.expectedResult).Nanoseconds(), d.Nanoseconds())
+				require.Equal(t, testCase.expectedResult.Nanoseconds(), d.Nanoseconds())
 			}
 
 			if err != nil {

dataavailability/datacommittee/datacommittee.go

@@ -2,10 +2,10 @@ package datacommittee
 
 import (
 	"crypto/ecdsa"
+	"crypto/rand"
 	"errors"
 	"fmt"
 	"math/big"
-	"math/rand"
 	"sort"
 	"strings"
 
@@ -91,7 +91,11 @@ func (d *Backend) Init() error {
 	if committee != nil {
 		d.committeeMembers = committee.Members
 		if len(committee.Members) > 0 {
-			selectedCommitteeMember = rand.Intn(len(committee.Members)) //nolint:gosec
+			nBig, err := rand.Int(rand.Reader, big.NewInt(int64(len(committee.Members))))
+			if err != nil {
+				return err
+			}
+			selectedCommitteeMember = int(nBig.Int64())
 		}
 	}
 	d.selectedCommitteeMember = selectedCommitteeMember
@@ -304,7 +308,7 @@ func requestSignatureFromMember(ctx context.Context, signedSequence daTypes.Sign
 	// request
 	c := client.New(member.URL)
 	log.Infof("sending request to sign the sequence to %s at %s", member.Addr.Hex(), member.URL)
-	//funcSign must call something like that  c.SignSequenceBanana(ctx, signedSequence)
+	// funcSign must call something like that  c.SignSequenceBanana(ctx, signedSequence)
 	signature, err := funcSign(c)
 
 	if err != nil {