From b8e612ad984a21472d71468436c44a1ff92913a3 Mon Sep 17 00:00:00 2001 From: Vritra4 Date: Wed, 17 Jul 2024 17:14:52 +0900 Subject: [PATCH 1/3] add workflow to lint and fix lint errors --- .github/workflows/lint.yml | 93 ++++++++++++++++++++++++++++++++++++++ .golangci.yml | 47 +++++++++++++++++++ Makefile | 39 ++++++++++++++++ executor/executor.go | 15 +++++- executor/types/config.go | 2 +- node/account.go | 2 +- node/node.go | 3 ++ node/tx.go | 6 +-- 8 files changed, 198 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/lint.yml create mode 100644 .golangci.yml create mode 100644 Makefile diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 0000000..22e32e1 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,93 @@ +name: Lint +# This workflow is run on every pull request and push to master +# The `golangci` will pass without running if no *.{go, mod, sum} files have been changed. +on: + pull_request: + paths: + - "**.go" + - "go.mod" + - "go.sum" + push: + branches: + - main + - "release/*" + paths: + - "**.go" + - "go.mod" + - "go.sum" + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + golangci: + env: + # for private repo access + GOPRIVATE: github.com/initia-labs,github.com/skip-mev/slinky + GITHUB_ACCESS_TOKEN: ${{ secrets.GH_READ_TOKEN }} + GOLANGCI_LINT_VERSION: v1.59.1 + name: golangci-lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: 1.22 + check-latest: true + - uses: technote-space/get-diff-action@v6.1.2 + id: git_diff + with: + PATTERNS: | + **/**.go + go.mod + go.sum + # for private repo access + - run: git config --global url.https://${GITHUB_ACCESS_TOKEN}:x-oauth-basic@github.com/.insteadOf https://github.com/ + # install golangci-lint + - run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_LINT_VERSION} + - name: run go linters (long) + if: env.GIT_DIFF + id: lint_long + run: | + make lint + - uses: technote-space/get-diff-action@v6.1.2 + if: steps.lint_long.outcome == 'skipped' + with: + PATTERNS: | + **/**.go + go.mod + go.sum + - name: run go linters (short) + if: steps.lint_long.outcome == 'skipped' && env.GIT_DIFF + run: | + make lint + env: + GIT_DIFF: ${{ env.GIT_DIFF }} + LINT_DIFF: 1 + # Use --check or --exit-code when available (Go 1.19?) + # https://github.com/golang/go/issues/27005 + tidy: + env: + # for private repo access + GOPRIVATE: github.com/initia-labs,github.com/skip-mev/slinky + GITHUB_ACCESS_TOKEN: ${{ secrets.GH_READ_TOKEN }} + runs-on: ubuntu-latest + name: tidy + steps: + - uses: actions/checkout@v4 + - name: Setup go + uses: actions/setup-go@v5 + with: + go-version: 1.22 + check-latest: true + # for private repo access + - run: git config --global url.https://${GITHUB_ACCESS_TOKEN}:x-oauth-basic@github.com/.insteadOf https://github.com/ + - run: | + go mod tidy + CHANGES_IN_REPO=$(git status --porcelain) + if [[ -n "$CHANGES_IN_REPO" ]]; then + echo "Repository is dirty. Showing 'git status' and 'git --no-pager diff' for debugging now:" + git status && git --no-pager diff + exit 1 + fi diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..497ee05 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,47 @@ +linters: + enable: + - asciicheck + - bodyclose + - dogsled + - dupl + - errcheck + - exportloopref + - goconst + - gofmt + - goimports + - gosec + - gosimple + - govet + - ineffassign + - misspell + # - nakedret + - nolintlint + # - prealloc + - staticcheck + # - structcheck // to be fixed by golangci-lint + - stylecheck + - typecheck + - unconvert + - unused + +issues: + exclude-rules: + - path: _test\.go + linters: + - gosec + - linters: + - stylecheck + text: "ST1003:" + max-same-issues: 50 + +linters-settings: + dogsled: + max-blank-identifiers: 3 + #golint: + # min-confidence: 0 + goconst: + ignore-tests: true + #maligned: + # suggest-new: true + misspell: + locale: US diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..d84282d --- /dev/null +++ b/Makefile @@ -0,0 +1,39 @@ +#!/usr/bin/make -f + +DOCKER := $(shell which docker) + + +############################################################################### +### Linting ### +############################################################################### + +lint: + golangci-lint run --out-format=tab --timeout=15m + +lint-fix: + golangci-lint run --fix --out-format=tab --timeout=15m + +.PHONY: lint lint-fix + + +############################################################################### +### Tests +############################################################################### + +test: test-unit + +test-all: test-unit test-race test-cover + +test-unit: + @VERSION=$(VERSION) go test -mod=readonly -tags='ledger test_ledger_mock' ./... + +test-race: + @VERSION=$(VERSION) go test -mod=readonly -race -tags='ledger test_ledger_mock' ./... + +test-cover: + @go test -mod=readonly -timeout 30m -race -coverprofile=coverage.txt -covermode=atomic -tags='ledger test_ledger_mock' ./... + +benchmark: + @go test -timeout 20m -mod=readonly -bench=. ./... + +.PHONY: test test-all test-cover test-unit test-race benchmark diff --git a/executor/executor.go b/executor/executor.go index 73fa853..9259f22 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -60,10 +60,21 @@ func NewExecutor(cfg *types.Config, logger *zap.Logger, cdc codec.Codec, txConfi func (ex Executor) Start(cmdCtx context.Context) error { hostCtx, hostDone := context.WithCancel(cmdCtx) - go ex.hostNode.BlockProcessLooper(hostCtx) + go func() { + err := ex.hostNode.BlockProcessLooper(hostCtx) + if err != nil { + panic(err) + } + }() go ex.hostNode.TxBroadCastLooper(hostCtx) + childCtx, childDone := context.WithCancel(cmdCtx) - go ex.childNode.BlockProcessLooper(childCtx) + go func() { + err := ex.childNode.BlockProcessLooper(childCtx) + if err != nil { + panic(err) + } + }() go ex.childNode.TxBroadCastLooper(childCtx) <-cmdCtx.Done() diff --git a/executor/types/config.go b/executor/types/config.go index 62665e3..20d438d 100644 --- a/executor/types/config.go +++ b/executor/types/config.go @@ -46,7 +46,7 @@ func (cfg Config) Validate() error { } if cfg.BridgeId == 0 { - return errors.New("Bridge ID is required") + return errors.New("Bridge ID is required") //nolint:stylecheck } return nil diff --git a/node/account.go b/node/account.go index 51059de..051797a 100644 --- a/node/account.go +++ b/node/account.go @@ -52,7 +52,7 @@ func (n *Node) GetAccountWithHeight(_ client.Context, addr sdk.AccAddress) (clie return nil, 0, fmt.Errorf("failed to parse block height: %w", err) } - var acc authtypes.AccountI + var acc authtypes.AccountI //nolint:staticcheck if err := n.cdc.UnpackAny(res.Account, &acc); err != nil { return nil, 0, err } diff --git a/node/node.go b/node/node.go index 494010a..18d9d98 100644 --- a/node/node.go +++ b/node/node.go @@ -51,6 +51,9 @@ type Node struct { func NewNode(name string, cfg NodeConfig, logger *zap.Logger, cdc codec.Codec, txConfig client.TxConfig) (*Node, error) { client, err := client.NewClientFromNode(cfg.RPC) + if err != nil { + return nil, err + } // Use memory keyring for now // TODO: may use os keyring later diff --git a/node/tx.go b/node/tx.go index ad0b19d..b6abf6b 100644 --- a/node/tx.go +++ b/node/tx.go @@ -10,8 +10,6 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/crypto/keyring" - "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" txtypes "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" @@ -163,9 +161,7 @@ func BuildSimTx(info *keyring.Record, txf tx.Factory, msgs ...sdk.Msg) ([]byte, return nil, err } - var pk cryptotypes.PubKey = &secp256k1.PubKey{} // use default public key type - - pk, err = info.GetPubKey() + pk, err := info.GetPubKey() if err != nil { return nil, err } From 53dc34ee71e09d5a005b74b1274ed480c59f4766 Mon Sep 17 00:00:00 2001 From: Vritra4 Date: Wed, 17 Jul 2024 17:16:57 +0900 Subject: [PATCH 2/3] remove unnecessary goprivate from workflow --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 22e32e1..b7f66a7 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -24,7 +24,7 @@ jobs: golangci: env: # for private repo access - GOPRIVATE: github.com/initia-labs,github.com/skip-mev/slinky + GOPRIVATE: github.com/initia-labs GITHUB_ACCESS_TOKEN: ${{ secrets.GH_READ_TOKEN }} GOLANGCI_LINT_VERSION: v1.59.1 name: golangci-lint From 142375b129c12133a548d289d31ffc8e17ffe7eb Mon Sep 17 00:00:00 2001 From: sh-cha Date: Tue, 6 Aug 2024 19:48:03 +0900 Subject: [PATCH 3/3] lint --- executor/batch/handler.go | 2 +- executor/child/handler.go | 2 +- executor/child/withdraw.go | 2 +- merkle/merkle.go | 1 - node/broadcaster/account.go | 8 -------- node/broadcaster/broadcaster.go | 4 ++-- node/process.go | 2 +- 7 files changed, 6 insertions(+), 15 deletions(-) diff --git a/executor/batch/handler.go b/executor/batch/handler.go index d58b6f6..b17f6b2 100644 --- a/executor/batch/handler.go +++ b/executor/batch/handler.go @@ -177,7 +177,7 @@ func (bs *BatchSubmitter) finalizeBatch(blockHeight uint64) error { Save: true, }) - for offset := int64(0); ; offset += int64(bs.batchCfg.MaxChunkSize) { + for offset := int64(0); ; offset += bs.batchCfg.MaxChunkSize { readLength, err := bs.batchFile.ReadAt(batchBuffer, offset) if err != nil && err != io.EOF { return err diff --git a/executor/child/handler.go b/executor/child/handler.go index aef5623..ac1416d 100644 --- a/executor/child/handler.go +++ b/executor/child/handler.go @@ -30,7 +30,7 @@ func (ch *Child) beginBlockHandler(args nodetypes.BeginBlockArgs) (err error) { func (ch *Child) endBlockHandler(args nodetypes.EndBlockArgs) error { blockHeight := uint64(args.Block.Header.Height) batchKVs := make([]types.RawKV, 0) - treeKVs, storageRoot, err := ch.handleTree(blockHeight, uint64(args.LatestHeight), args.BlockID, args.Block.Header) + treeKVs, storageRoot, err := ch.handleTree(blockHeight, args.LatestHeight, args.BlockID, args.Block.Header) if err != nil { return err } diff --git a/executor/child/withdraw.go b/executor/child/withdraw.go index abc584a..3780276 100644 --- a/executor/child/withdraw.go +++ b/executor/child/withdraw.go @@ -93,7 +93,7 @@ func (ch *Child) prepareTree(blockHeight uint64) error { err := ch.mk.LoadWorkingTree(blockHeight - 1) if err == dbtypes.ErrNotFound { // must not happened - // TOOD: if user want to start from a specific height, we need to provide a way to do so + // TODO: if user want to start from a specific height, we need to provide a way to do so panic(fmt.Errorf("working tree not found at height: %d, current: %d", blockHeight-1, blockHeight)) } else if err != nil { return err diff --git a/merkle/merkle.go b/merkle/merkle.go index 33049b0..0def1ce 100644 --- a/merkle/merkle.go +++ b/merkle/merkle.go @@ -186,7 +186,6 @@ func (m *Merkle) fillLeaves() error { } lastLeaf := m.workingTree.LastSiblings[0] - //nolint:typecheck for range numRestLeaves { if err := m.InsertLeaf(lastLeaf); err != nil { return err diff --git a/node/broadcaster/account.go b/node/broadcaster/account.go index 20a37fa..231d9d2 100644 --- a/node/broadcaster/account.go +++ b/node/broadcaster/account.go @@ -68,16 +68,8 @@ func (b *Broadcaster) GetAccountWithHeight(_ client.Context, addr sdk.AccAddress return nil, 0, fmt.Errorf("failed to parse block height: %w", err) } -<<<<<<< HEAD:node/account.go - var acc authtypes.AccountI //nolint:staticcheck - if err := n.cdc.UnpackAny(res.Account, &acc); err != nil { -||||||| 168b4ff:node/account.go - var acc authtypes.AccountI - if err := n.cdc.UnpackAny(res.Account, &acc); err != nil { -======= var acc sdk.AccountI if err := b.cdc.UnpackAny(res.Account, &acc); err != nil { ->>>>>>> main:node/broadcaster/account.go return nil, 0, err } diff --git a/node/broadcaster/broadcaster.go b/node/broadcaster/broadcaster.go index 3a3caff..8eaddab 100644 --- a/node/broadcaster/broadcaster.go +++ b/node/broadcaster/broadcaster.go @@ -125,8 +125,8 @@ func (b Broadcaster) getClientCtx() client.Context { WithFromAddress(b.keyAddress) } -func (n Broadcaster) GetTxf() tx.Factory { - return n.txf +func (b Broadcaster) GetTxf() tx.Factory { + return b.txf } func (b *Broadcaster) prepareBroadcaster(_ /*lastBlockHeight*/ uint64, lastBlockTime time.Time) error { diff --git a/node/process.go b/node/process.go index b09eedb..bb219a3 100644 --- a/node/process.go +++ b/node/process.go @@ -232,7 +232,7 @@ func (n *Node) txChecker(ctx context.Context) error { } } - err = n.broadcaster.RemovePendingTx(int64(res.Height), pendingTx.TxHash, pendingTx.Sequence) + err = n.broadcaster.RemovePendingTx(res.Height, pendingTx.TxHash, pendingTx.Sequence) if err != nil { return err }