feat: Linters Fix (#62)

* wip linters

* add SonarQube config

* linter fix bridgesync

* linter fix aggOracle

* linter fix claimsponsor

* linter fix cmd

* linter fix common config

* linter fix dataavailability

* etherman lint

* hex lint

* l1bridge2infoindexsync lint

* l1infotree lint fix

* l1infotreesync lastgersync lint fix

* log lint fix

* merkletree lint fix

* merkletree lint fix

* reorgdetactor rpc lint fix

* sequencesender lint fix

* state lint fix

* sync tree lint fix

* wip

* wip

* lint fix

* addressed feedback

* wip

* wip

* sonar cloud fix (#66)

* sonar cloud fix

* wip

* addressed feedback

* upgrade actions

* reorg detactor errors

.github/workflows/codeql.yml

@@ -46,7 +46,7 @@ jobs:
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL

.github/workflows/lint.yml

@@ -0,0 +1,23 @@
+name: Lint
+on:
+  push:
+    branches:
+      - main
+      - develop
+      - update-external-dependencies
+      - 'release/**'
+  pull_request:
+jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install Go
+        uses: actions/setup-go@v3
+        with:
+          go-version: 1.21.x
+      - name: Checkout code
+        uses: actions/checkout@v3
+      - name: Lint
+        run: |
+          make install-linter
+          make lint

.github/workflows/test-e2e.yml

@@ -20,7 +20,7 @@ jobs:
       uses: actions/checkout@v4
 
     - name: Install Go
-      uses: actions/setup-go@v3
+      uses: actions/setup-go@v5
       with:
         go-version: ${{ matrix.go-version }}
       env:

.github/workflows/test-resequence.yml

@@ -94,7 +94,7 @@ jobs:
 
       - name: Upload logs
         if: always()
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: logs_${{ github.run_id }}
           path: ./kurtosis-cdk/ci_logs

.github/workflows/test-unit.yml

@@ -0,0 +1,40 @@
+name: Test Unit and SonarCloud analysis
+
+on:
+  push:
+    branches:
+      - main
+      - develop
+      - 'release/**'
+  pull_request:
+  workflow_dispatch: {}
+
+jobs:
+  test-unit:
+    strategy:
+      fail-fast: false
+      matrix:
+        go-version: [1.22.4]
+        goarch: ["amd64"]
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0  # Shallow clones should be disabled for a better relevancy of analysis
+
+      - name: Install Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: ${{ matrix.go-version }}
+        env:
+          GOARCH: ${{ matrix.goarch }}
+
+      - name: Test
+        run: make test-unit
+
+      - name: Analyze with SonarCloud
+        uses: sonarsource/sonarcloud-github-action@master
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

.golangci.yml

@@ -8,70 +8,79 @@ run:
   skip-dirs-use-default: true
   skip-dirs:
     - tests
+    - aggregator/db/migrations
 
 service:
   golangci-lint-version: 1.59.1
-
+    
 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
-    - 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
+  - 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
 
 linters-settings:
   gofmt:
     simplify: true
-  goconst:
-    min-len: 3
-    min-occurrences: 3
   gocritic:
     enabled-checks:
       - ruleguard
-    settings:
-      ruleguard:
-        rules: "./gorules/rules.go"
+    # settings:
+    #   ruleguard:
+    #     rules: "./gorules/rules.go"
+  revive:
+    rules:
+    - name: exported
+      arguments:
+      - disableStutteringCheck
+  goconst:
+    min-len: 3
+    min-occurrences: 3
 
 issues:
-  new-from-rev: origin/develop # report only new issues with reference to develop branch
+  # new-from-rev: origin/develop # report only new issues with reference to develop branch
   whole-files: true
   exclude-rules:
-    - path: _test\.go
+    - path: '(_test\.go|^test/.*)'
       linters:
         - gosec
         - unparam
         - lll
-    - path: gen_sc_data\.go
+    - path: 'etherman/contracts/contracts_(banana|elderberry)\.go'
       linters:
-        - wsl
-        - lll
-        - stylecheck
+        - dupl
   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
+

Makefile

@@ -80,9 +80,9 @@ build-docker-nc: ## Builds a docker image with the cdk binary - but without buil
 stop: ## Stops all services
 	docker-compose down
 
-.PHONY: test
-test:
-	trap '$(STOP)' EXIT; MallocNanoZone=0 go test -count=1 -short -race -p 1 -covermode=atomic -coverprofile=../coverage.out  -coverpkg ./... -timeout 200s ./...
+.PHONY: test-unit
+test-unit:
+	trap '$(STOP)' EXIT; MallocNanoZone=0 go test -count=1 -short -race -p 1 -covermode=atomic -coverprofile=coverage.out  -coverpkg ./... -timeout 200s ./...
 
 .PHONY: test-seq_sender
 test-seq_sender:

aggoracle/chaingersender/evm.go

@@ -25,9 +25,18 @@ type EthClienter interface {
 
 type EthTxManager interface {
 	Remove(ctx context.Context, id common.Hash) error
-	ResultsByStatus(ctx context.Context, statuses []ethtxmanager.MonitoredTxStatus) ([]ethtxmanager.MonitoredTxResult, error)
+	ResultsByStatus(ctx context.Context,
+		statuses []ethtxmanager.MonitoredTxStatus,
+	) ([]ethtxmanager.MonitoredTxResult, error)
 	Result(ctx context.Context, id common.Hash) (ethtxmanager.MonitoredTxResult, error)
-	Add(ctx context.Context, to *common.Address, forcedNonce *uint64, value *big.Int, data []byte, gasOffset uint64, sidecar *types.BlobTxSidecar) (common.Hash, error)
+	Add(ctx context.Context,
+		to *common.Address,
+		forcedNonce *uint64,
+		value *big.Int,
+		data []byte,
+		gasOffset uint64,
+		sidecar *types.BlobTxSidecar,
+	) (common.Hash, error)
 }
 
 type EVMChainGERSender struct {
@@ -61,6 +70,7 @@ func NewEVMChainGERSender(
 	if err != nil {
 		return nil, err
 	}
+
 	return &EVMChainGERSender{
 		gerContract:         gerContract,
 		gerAddr:             l2GlobalExitRoot,
@@ -77,6 +87,7 @@ func (c *EVMChainGERSender) IsGERAlreadyInjected(ger common.Hash) (bool, error)
 	if err != nil {
 		return false, fmt.Errorf("error calling gerContract.GlobalExitRootMap: %w", err)
 	}
+
 	return timestamp.Cmp(big.NewInt(0)) != 0, nil
 }
 

aggoracle/config.go

@@ -19,7 +19,7 @@ type Config struct {
 	TargetChainType TargetChainType `mapstructure:"TargetChainType"`
 	URLRPCL1        string          `mapstructure:"URLRPCL1"`
 	// BlockFinality indicates the status of the blocks that will be queried in order to sync
-	BlockFinality     string                   `jsonschema:"enum=LatestBlock, enum=SafeBlock, enum=PendingBlock, enum=FinalizedBlock, enum=EarliestBlock" mapstructure:"BlockFinality"`
+	BlockFinality     string                   `jsonschema:"enum=LatestBlock, enum=SafeBlock, enum=PendingBlock, enum=FinalizedBlock, enum=EarliestBlock" mapstructure:"BlockFinality"` //nolint:lll
 	WaitPeriodNextGER types.Duration           `mapstructure:"WaitPeriodNextGER"`
 	EVMSender         chaingersender.EVMConfig `mapstructure:"EVMSender"`
 }

aggoracle/e2e_test.go

@@ -27,6 +27,8 @@ func runTest(
 	l1Client *simulated.Backend,
 	authL1 *bind.TransactOpts,
 ) {
+	t.Helper()
+
 	for i := 0; i < 10; i++ {
 		_, err := gerL1Contract.UpdateExitRoot(authL1, common.HexToHash(strconv.Itoa(i)))
 		require.NoError(t, err)

aggoracle/oracle.go

@@ -2,6 +2,7 @@ package aggoracle
 
 import (
 	"context"
+	"errors"
 	"math/big"
 	"time"
 
@@ -41,6 +42,7 @@ func New(
 	if err != nil {
 		return nil, err
 	}
+
 	return &AggOracle{
 		ticker:        ticker,
 		l1Client:      l1Client,
@@ -61,26 +63,30 @@ func (a *AggOracle) Start(ctx context.Context) {
 		case <-a.ticker.C:
 			blockNumToFetch, gerToInject, err = a.getLastFinalisedGER(ctx, blockNumToFetch)
 			if err != nil {
-				if err == l1infotreesync.ErrBlockNotProcessed {
+				if errors.Is(err, l1infotreesync.ErrBlockNotProcessed) {
 					log.Debugf("syncer is not ready for the block %d", blockNumToFetch)
-				} else if err == l1infotreesync.ErrNotFound {
+				} else if errors.Is(err, l1infotreesync.ErrNotFound) {
 					blockNumToFetch = 0
 					log.Debugf("syncer has not found any GER until block %d", blockNumToFetch)
 				} else {
 					log.Error("error calling getLastFinalisedGER: ", err)
 				}
+
 				continue
 			}
 			if alreadyInjected, err := a.chainSender.IsGERAlreadyInjected(gerToInject); err != nil {
 				log.Error("error calling isGERAlreadyInjected: ", err)
+
 				continue
 			} else if alreadyInjected {
 				log.Debugf("GER %s already injected", gerToInject.Hex())
+
 				continue
 			}
 			log.Infof("injecting new GER: %s", gerToInject.Hex())
 			if err := a.chainSender.UpdateGERWaitUntilMined(ctx, gerToInject); err != nil {
 				log.Errorf("error calling updateGERWaitUntilMined, when trying to inject GER %s: %v", gerToInject.Hex(), err)
+
 				continue
 			}
 			log.Infof("GER %s injected", gerToInject.Hex())
@@ -106,5 +112,6 @@ func (a *AggOracle) getLastFinalisedGER(ctx context.Context, blockNumToFetch uin
 	if err != nil {
 		return blockNumToFetch, common.Hash{}, err
 	}
+
 	return 0, info.GlobalExitRoot, nil
 }

aggregator/agglayer_tx.go

@@ -41,6 +41,7 @@ func (t *Tx) Sign(privateKey *ecdsa.PrivateKey) (*SignedTx, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	return &SignedTx{
 		Tx:        *t,
 		Signature: sig,
@@ -59,5 +60,6 @@ func (s *SignedTx) Signer() (common.Address, error) {
 	if err != nil {
 		return common.Address{}, err
 	}
+
 	return crypto.PubkeyToAddress(*pubKey), nil
 }