Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Linters warning fixes #74

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v6
with:
version: v1.60
version: v1.61
86 changes: 41 additions & 45 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/.*)'
Expand All @@ -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

9 changes: 6 additions & 3 deletions aggoracle/oracle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
32 changes: 20 additions & 12 deletions aggregator/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}
}
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
12 changes: 6 additions & 6 deletions aggregator/profitabilitychecker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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 {
Expand All @@ -89,4 +89,4 @@ func (pc *TxProfitabilityCheckerAcceptAll) IsProfitable(ctx context.Context, pol
// }
//
// return false, err
//}
// }
14 changes: 7 additions & 7 deletions aggregator/prover/prover.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
10 changes: 7 additions & 3 deletions claimsponsor/claimsponsor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}()
Expand Down
2 changes: 1 addition & 1 deletion config/types/duration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions dataavailability/datacommittee/datacommittee.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
if committee != nil {
d.committeeMembers = committee.Members
if len(committee.Members) > 0 {
selectedCommitteeMember = rand.Intn(len(committee.Members)) //nolint:gosec
selectedCommitteeMember = rand.Intn(len(committee.Members))

Check failure on line 94 in dataavailability/datacommittee/datacommittee.go

View workflow job for this annotation

GitHub Actions / lint

G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand) (gosec)
}
}
d.selectedCommitteeMember = selectedCommitteeMember
Expand Down Expand Up @@ -304,7 +304,7 @@
// 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 {
Expand Down
4 changes: 2 additions & 2 deletions etherman/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ var (
ErrGasRequiredExceedsAllowance = errors.New("gas required exceeds allowance")
// ErrContentLengthTooLarge content length is too large
ErrContentLengthTooLarge = errors.New("content length too large")
//ErrTimestampMustBeInsideRange Timestamp must be inside range
// ErrTimestampMustBeInsideRange Timestamp must be inside range
ErrTimestampMustBeInsideRange = errors.New("timestamp must be inside range")
//ErrInsufficientAllowance insufficient allowance
// ErrInsufficientAllowance insufficient allowance
ErrInsufficientAllowance = errors.New("insufficient allowance")
// ErrBothGasPriceAndMaxFeeGasAreSpecified both gasPrice and (maxFeePerGas or maxPriorityFeePerGas) specified
ErrBothGasPriceAndMaxFeeGasAreSpecified = errors.New(
Expand Down
Loading
Loading