From 4575fe05d91499980140e87c38471a8e628969a0 Mon Sep 17 00:00:00 2001 From: tbjump <103955289+tbjump@users.noreply.github.com> Date: Tue, 21 Jun 2022 12:18:16 -0700 Subject: [PATCH] Better lint and formatting (#1263) Enforce goimports, add tooling and documentation, and run it on the repo. --- .github/workflows/build.yml | 26 +-- CONTRIBUTING.md | 21 +++ Dockerfile.lint | 9 +- lint.sh | 8 - node/cmd/debug/vaa.go | 5 +- node/cmd/guardiand/adminnodes.go | 7 +- node/cmd/guardiand/adminserver.go | 13 +- node/cmd/guardiand/logging.go | 3 +- node/cmd/guardiand/publicrpc.go | 3 +- node/cmd/guardiand/publicweb.go | 9 +- node/cmd/guardiand/systemd.go | 3 +- node/cmd/root.go | 3 +- node/cmd/spy/spy.go | 11 +- node/hack/discord_test/discord.go | 1 + node/hack/findmissing/findmissing.go | 3 +- node/hack/repair_solana/repair.go | 11 +- node/pkg/algorand/watcher.go | 3 +- node/pkg/common/chainlock.go | 3 +- node/pkg/common/grpc.go | 8 +- node/pkg/common/guardianset_test.go | 3 +- node/pkg/common/nodekey.go | 5 +- node/pkg/common/symmetric_test.go | 3 +- node/pkg/common/sysutils.go | 3 +- node/pkg/db/db.go | 5 +- node/pkg/devnet/deterministic_p2p_key_test.go | 3 +- node/pkg/ethereum/by_transaction.go | 3 +- node/pkg/ethereum/moonbeamfin.go | 3 +- node/pkg/ethereum/utils_test.go | 3 +- node/pkg/notify/discord/notify.go | 5 +- node/pkg/p2p/netmetrics.go | 7 +- node/pkg/p2p/p2p.go | 5 +- node/pkg/p2p/registry.go | 3 +- node/pkg/p2p/registry_test.go | 3 +- node/pkg/processor/broadcast.go | 3 +- node/pkg/processor/cleanup.go | 3 +- node/pkg/processor/injection.go | 1 + node/pkg/processor/observation_test.go | 5 +- node/pkg/processor/processor.go | 3 +- node/pkg/processor/quorum_test.go | 3 +- node/pkg/publicrpc/publicrpcserver_test.go | 3 +- node/pkg/solana/client.go | 3 +- node/pkg/telemetry/telemetry.go | 5 +- node/pkg/terra/watcher.go | 7 +- node/pkg/vaa/governance_test.go | 9 +- node/pkg/vaa/payloads.go | 1 + node/pkg/vaa/payloads_test.go | 3 +- node/pkg/vaa/structs_test.go | 9 +- node/pkg/vaa/types_test.go | 5 +- scripts/lint.sh | 152 ++++++++++++++++++ 49 files changed, 319 insertions(+), 100 deletions(-) delete mode 100755 lint.sh create mode 100755 scripts/lint.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2700a70f4..af71a9d39 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,6 +8,9 @@ on: jobs: # Run the full Tilt build and wait for it to converge tilt: + # in the future, we may want to run cheap lints, tests, and builds before firing up the expensive tilt test. + # But for now, we'll kick-off everything at once + # needs: [go-lint-and-tests, node, algorand, ethereum, terra, rust-lint-and-tests] runs-on: tilt-kube-public # Cancel previous builds on the same branch/ref. Full runs are expensive @@ -72,7 +75,6 @@ jobs: with: node-version: "16" - run: cd terra && make test - terra-2: runs-on: ubuntu-20.04 steps: @@ -82,7 +84,7 @@ jobs: node-version: "16" - run: cd cosmwasm && make test - # Run linters, Go tests and other outside-of-Tilt things. + # Run Go linters, Go tests and other outside-of-Tilt things. lint-and-tests: # The linter is slow enough that we want to run it on the self-hosted runner runs-on: tilt-kube-public @@ -94,14 +96,20 @@ jobs: - uses: actions/setup-go@v2 with: go-version: "1.17.5" - # ensure that code is formatted - - run: GOFMT_OUTPUT="$(gofmt -l `find ./node ./event_database -name '*.go' | grep -v vendor` 2>&1)"; if [ -n "$GOFMT_OUTPUT" ]; then printf "All the following files are not correctly formatted\n${GOFMT_OUTPUT}\n"; exit 1; fi - # run linters - - run: make generate && ./lint.sh - # The go-ethereum and celo-blockchain packages both implement secp256k1 using the exact same header, but that causes duplicate symbols. - - run: cd node && go test -v -ldflags '-extldflags "-Wl,--allow-multiple-definition" ' ./... + - name: Install formatter + run: go install golang.org/x/tools/cmd/goimports@latest + - name: Formatting checks + run: ./scripts/lint.sh -l -g format + - name: Install linters + run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.46.2 + - name: Run linters + run: make generate && ./scripts/lint.sh -g lint - # Run rust lints and tests + # The go-ethereum and celo-blockchain packages both implement secp256k1 using the exact same header, but that causes duplicate symbols. + - name: Run golang tests + run: cd node && go test -v -ldflags '-extldflags "-Wl,--allow-multiple-definition" ' ./... + + # Run Rust lints and tests rust-lint-and-tests: runs-on: ubuntu-20.04 strategy: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 137d9be23..9461d877b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -66,3 +66,24 @@ of communication (like transfers). It is likely that you can use the existing Wo own features on top of, without requiring any changes in Wormhole itself. Please open a GitHub issue outlining your use case, and we can help you build it! + +# Pre-Commit checks +Run `./scripts/lint.sh -d format` and `./scripts/lint.sh lint`. + +## IDE Integration +### Golang formatting +You must format your code with `goimports` before submitting. +You can install it with `go install golang.org/x/tools/cmd/goimports@latest` and run it with `goimports -d ./`. +You can enable it in VSCode with the following in your `settings.json`. +```json + "go.useLanguageServer": true, + "go.formatTool": "goimports", + "[go]": { + "editor.defaultFormatter": "golang.go", + "editor.formatOnSaveMode": "file", + "editor.codeActionsOnSave": { + "source.fixAll": true, + "source.organizeImports": true + } + }, +``` diff --git a/Dockerfile.lint b/Dockerfile.lint index b37da0892..4e6a2c7d8 100644 --- a/Dockerfile.lint +++ b/Dockerfile.lint @@ -5,8 +5,9 @@ RUN useradd -u 1000 -U -m -d /home/lint lint USER 1000 WORKDIR /home/lint -RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | \ - sh -s -- -b ~ v1.42.0 +# install goimports +RUN go install golang.org/x/tools/cmd/goimports@latest -RUN --mount=type=bind,target=/app,source=node cd /app && \ - GOGC=off ~/golangci-lint run --skip-dirs pkg/supervisor --timeout=10m --out-format=github-actions ./... +# install golangci-lint +RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | \ + sh -s -- -b $(go env GOPATH)/bin v1.46.2 diff --git a/lint.sh b/lint.sh deleted file mode 100755 index 630d1e674..000000000 --- a/lint.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/usr/bin/env bash - -# fail if any command fails -set -e -set -o pipefail - -# we duplicate stderr to stdout and then filter and parse stdout to only include errors that are readable as github annotations -DOCKER_BUILDKIT=1 docker build -f Dockerfile.lint . 2>&1 | while read line; do echo $line; echo $line >&2; done | (grep "::" || true) | cut -f3- -d " " diff --git a/node/cmd/debug/vaa.go b/node/cmd/debug/vaa.go index e1e9ffd20..4456be024 100644 --- a/node/cmd/debug/vaa.go +++ b/node/cmd/debug/vaa.go @@ -2,11 +2,12 @@ package debug import ( "encoding/hex" + "log" + "strings" + "github.com/certusone/wormhole/node/pkg/vaa" "github.com/davecgh/go-spew/spew" "github.com/spf13/cobra" - "log" - "strings" ) var decodeVaaCmd = &cobra.Command{ diff --git a/node/cmd/guardiand/adminnodes.go b/node/cmd/guardiand/adminnodes.go index 2b0a14920..bafd17086 100644 --- a/node/cmd/guardiand/adminnodes.go +++ b/node/cmd/guardiand/adminnodes.go @@ -3,15 +3,16 @@ package guardiand import ( "context" "fmt" - publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" - "github.com/certusone/wormhole/node/pkg/vaa" - "github.com/spf13/cobra" "log" "os" "sort" "strings" "text/tabwriter" "time" + + publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" + "github.com/certusone/wormhole/node/pkg/vaa" + "github.com/spf13/cobra" ) // How to test in container: diff --git a/node/cmd/guardiand/adminserver.go b/node/cmd/guardiand/adminserver.go index 604783378..8d6eef4c6 100644 --- a/node/cmd/guardiand/adminserver.go +++ b/node/cmd/guardiand/adminserver.go @@ -7,6 +7,13 @@ import ( "encoding/json" "errors" "fmt" + "math" + "math/rand" + "net" + "net/http" + "os" + "time" + "github.com/certusone/wormhole/node/pkg/db" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" @@ -15,12 +22,6 @@ import ( "go.uber.org/zap" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "math" - "math/rand" - "net" - "net/http" - "os" - "time" "github.com/certusone/wormhole/node/pkg/common" nodev1 "github.com/certusone/wormhole/node/pkg/proto/node/v1" diff --git a/node/cmd/guardiand/logging.go b/node/cmd/guardiand/logging.go index 53f360964..d3e8d4b92 100644 --- a/node/cmd/guardiand/logging.go +++ b/node/cmd/guardiand/logging.go @@ -1,9 +1,10 @@ package guardiand import ( + "unicode" + "go.uber.org/zap/buffer" "go.uber.org/zap/zapcore" - "unicode" ) type consoleEncoder struct { diff --git a/node/cmd/guardiand/publicrpc.go b/node/cmd/guardiand/publicrpc.go index 4cb541509..08b58185d 100644 --- a/node/cmd/guardiand/publicrpc.go +++ b/node/cmd/guardiand/publicrpc.go @@ -2,6 +2,8 @@ package guardiand import ( "fmt" + "net" + "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" @@ -9,7 +11,6 @@ import ( "github.com/certusone/wormhole/node/pkg/supervisor" "go.uber.org/zap" "google.golang.org/grpc" - "net" ) func publicrpcServiceRunnable(logger *zap.Logger, listenAddr string, db *db.Database, gst *common.GuardianSetState) (supervisor.Runnable, *grpc.Server, error) { diff --git a/node/cmd/guardiand/publicweb.go b/node/cmd/guardiand/publicweb.go index 53db80f40..507316b6e 100644 --- a/node/cmd/guardiand/publicweb.go +++ b/node/cmd/guardiand/publicweb.go @@ -3,7 +3,11 @@ package guardiand import ( "context" "fmt" - "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" + "net" + "net/http" + "strings" + + publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" "github.com/certusone/wormhole/node/pkg/supervisor" "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" "github.com/improbable-eng/grpc-web/go/grpcweb" @@ -11,9 +15,6 @@ import ( "golang.org/x/crypto/acme" "golang.org/x/crypto/acme/autocert" "google.golang.org/grpc" - "net" - "net/http" - "strings" ) func allowCORSWrapper(h http.Handler) http.Handler { diff --git a/node/cmd/guardiand/systemd.go b/node/cmd/guardiand/systemd.go index d7165ae45..2eefba923 100644 --- a/node/cmd/guardiand/systemd.go +++ b/node/cmd/guardiand/systemd.go @@ -2,8 +2,9 @@ package guardiand import ( "fmt" - "github.com/coreos/go-systemd/activation" "net" + + "github.com/coreos/go-systemd/activation" ) func getSDListeners() ([]net.Listener, error) { diff --git a/node/cmd/root.go b/node/cmd/root.go index 3a5cbe3dd..a4b39502f 100644 --- a/node/cmd/root.go +++ b/node/cmd/root.go @@ -2,10 +2,11 @@ package cmd import ( "fmt" + "os" + "github.com/certusone/wormhole/node/cmd/debug" "github.com/certusone/wormhole/node/cmd/spy" "github.com/certusone/wormhole/node/pkg/version" - "os" "github.com/spf13/cobra" diff --git a/node/cmd/spy/spy.go b/node/cmd/spy/spy.go index 4579e82c3..2d186eda0 100644 --- a/node/cmd/spy/spy.go +++ b/node/cmd/spy/spy.go @@ -4,10 +4,15 @@ import ( "context" "encoding/hex" "fmt" + "net" + "net/http" + "os" + "sync" + "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/p2p" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" - "github.com/certusone/wormhole/node/pkg/proto/spy/v1" + spyv1 "github.com/certusone/wormhole/node/pkg/proto/spy/v1" "github.com/certusone/wormhole/node/pkg/supervisor" "github.com/certusone/wormhole/node/pkg/vaa" "github.com/google/uuid" @@ -20,10 +25,6 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "net" - "net/http" - "os" - "sync" ) var ( diff --git a/node/hack/discord_test/discord.go b/node/hack/discord_test/discord.go index e475491e6..eeeda8559 100644 --- a/node/hack/discord_test/discord.go +++ b/node/hack/discord_test/discord.go @@ -3,6 +3,7 @@ package main import ( "encoding/hex" "flag" + "github.com/certusone/wormhole/node/pkg/notify/discord" "github.com/certusone/wormhole/node/pkg/vaa" "go.uber.org/zap" diff --git a/node/hack/findmissing/findmissing.go b/node/hack/findmissing/findmissing.go index 1100684ba..7066fe047 100644 --- a/node/hack/findmissing/findmissing.go +++ b/node/hack/findmissing/findmissing.go @@ -4,11 +4,12 @@ import ( "context" "flag" "fmt" + "log" + "github.com/certusone/wormhole/node/pkg/common" nodev1 "github.com/certusone/wormhole/node/pkg/proto/node/v1" "github.com/certusone/wormhole/node/pkg/vaa" "google.golang.org/grpc" - "log" ) var ( diff --git a/node/hack/repair_solana/repair.go b/node/hack/repair_solana/repair.go index 0eaabb6bc..76bcf779b 100644 --- a/node/hack/repair_solana/repair.go +++ b/node/hack/repair_solana/repair.go @@ -5,6 +5,12 @@ import ( "encoding/hex" "flag" "fmt" + "log" + "net/http" + "strconv" + "strings" + "time" + "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" @@ -14,11 +20,6 @@ import ( "github.com/gagliardetto/solana-go/rpc" "golang.org/x/time/rate" "google.golang.org/grpc" - "log" - "net/http" - "strconv" - "strings" - "time" ) var ( diff --git a/node/pkg/algorand/watcher.go b/node/pkg/algorand/watcher.go index 6e97d7a2e..8619f519c 100644 --- a/node/pkg/algorand/watcher.go +++ b/node/pkg/algorand/watcher.go @@ -7,6 +7,8 @@ import ( "encoding/hex" "encoding/json" "fmt" + "time" + "github.com/algorand/go-algorand-sdk/client/v2/algod" "github.com/algorand/go-algorand-sdk/client/v2/indexer" "github.com/algorand/go-algorand-sdk/crypto" @@ -21,7 +23,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "go.uber.org/zap" - "time" ) type ( diff --git a/node/pkg/common/chainlock.go b/node/pkg/common/chainlock.go index b7742b026..aaa74433d 100644 --- a/node/pkg/common/chainlock.go +++ b/node/pkg/common/chainlock.go @@ -1,9 +1,10 @@ package common import ( - "github.com/certusone/wormhole/node/pkg/vaa" "time" + "github.com/certusone/wormhole/node/pkg/vaa" + "github.com/ethereum/go-ethereum/common" ) diff --git a/node/pkg/common/grpc.go b/node/pkg/common/grpc.go index cc8960994..a314e8144 100644 --- a/node/pkg/common/grpc.go +++ b/node/pkg/common/grpc.go @@ -1,10 +1,10 @@ package common import ( - "github.com/grpc-ecosystem/go-grpc-middleware" - "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap" - "github.com/grpc-ecosystem/go-grpc-middleware/tags" - "github.com/grpc-ecosystem/go-grpc-prometheus" + grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" + grpc_zap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap" + grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags" + grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" "go.uber.org/zap" "google.golang.org/grpc" ) diff --git a/node/pkg/common/guardianset_test.go b/node/pkg/common/guardianset_test.go index d19aed2f2..5ef2c4a39 100644 --- a/node/pkg/common/guardianset_test.go +++ b/node/pkg/common/guardianset_test.go @@ -1,9 +1,10 @@ package common import ( + "testing" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" - "testing" ) func TestKeyIndex(t *testing.T) { diff --git a/node/pkg/common/nodekey.go b/node/pkg/common/nodekey.go index 4485c4fff..a80ce863f 100644 --- a/node/pkg/common/nodekey.go +++ b/node/pkg/common/nodekey.go @@ -2,11 +2,12 @@ package common import ( "fmt" + "io/ioutil" + "os" + "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" "go.uber.org/zap" - "io/ioutil" - "os" ) func GetOrCreateNodeKey(logger *zap.Logger, path string) (crypto.PrivKey, error) { diff --git a/node/pkg/common/symmetric_test.go b/node/pkg/common/symmetric_test.go index 701debfca..36226720b 100644 --- a/node/pkg/common/symmetric_test.go +++ b/node/pkg/common/symmetric_test.go @@ -2,8 +2,9 @@ package common import ( "encoding/hex" - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestAESGCM(t *testing.T) { diff --git a/node/pkg/common/sysutils.go b/node/pkg/common/sysutils.go index ed834742a..1e7de4526 100644 --- a/node/pkg/common/sysutils.go +++ b/node/pkg/common/sysutils.go @@ -2,9 +2,10 @@ package common import ( "fmt" - "golang.org/x/sys/unix" "os" "syscall" + + "golang.org/x/sys/unix" ) // LockMemory locks current and future pages in memory to protect secret keys from being swapped out to disk. diff --git a/node/pkg/db/db.go b/node/pkg/db/db.go index 515d792d4..1cf763404 100644 --- a/node/pkg/db/db.go +++ b/node/pkg/db/db.go @@ -3,10 +3,11 @@ package db import ( "errors" "fmt" - "github.com/certusone/wormhole/node/pkg/vaa" - "github.com/dgraph-io/badger/v3" "strconv" "strings" + + "github.com/certusone/wormhole/node/pkg/vaa" + "github.com/dgraph-io/badger/v3" ) type Database struct { diff --git a/node/pkg/devnet/deterministic_p2p_key_test.go b/node/pkg/devnet/deterministic_p2p_key_test.go index 63be85127..747d1544e 100644 --- a/node/pkg/devnet/deterministic_p2p_key_test.go +++ b/node/pkg/devnet/deterministic_p2p_key_test.go @@ -3,8 +3,9 @@ package devnet import ( "encoding/hex" "fmt" - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestDeterministicP2PPrivKeyByIndex(t *testing.T) { diff --git a/node/pkg/ethereum/by_transaction.go b/node/pkg/ethereum/by_transaction.go index 0192824e2..19bd2ecf4 100644 --- a/node/pkg/ethereum/by_transaction.go +++ b/node/pkg/ethereum/by_transaction.go @@ -3,10 +3,11 @@ package ethereum import ( "context" "fmt" + "time" + "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/vaa" eth_common "github.com/ethereum/go-ethereum/common" - "time" ) var ( diff --git a/node/pkg/ethereum/moonbeamfin.go b/node/pkg/ethereum/moonbeamfin.go index 2306e3791..c2f27e1f8 100644 --- a/node/pkg/ethereum/moonbeamfin.go +++ b/node/pkg/ethereum/moonbeamfin.go @@ -8,10 +8,11 @@ package ethereum import ( "context" + "time" + common "github.com/certusone/wormhole/node/pkg/common" ethRpc "github.com/ethereum/go-ethereum/rpc" "go.uber.org/zap" - "time" ) type MoonbeamFinalizer struct { diff --git a/node/pkg/ethereum/utils_test.go b/node/pkg/ethereum/utils_test.go index b540f5f6b..598b8243c 100644 --- a/node/pkg/ethereum/utils_test.go +++ b/node/pkg/ethereum/utils_test.go @@ -1,10 +1,11 @@ package ethereum import ( + "testing" + "github.com/certusone/wormhole/node/pkg/vaa" "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" - "testing" ) func TestPadAddress(t *testing.T) { diff --git a/node/pkg/notify/discord/notify.go b/node/pkg/notify/discord/notify.go index 8b9c02aa5..9f0c16853 100644 --- a/node/pkg/notify/discord/notify.go +++ b/node/pkg/notify/discord/notify.go @@ -3,12 +3,13 @@ package discord import ( "bytes" "fmt" + "strings" + "sync" + "github.com/certusone/wormhole/node/pkg/vaa" "github.com/diamondburned/arikawa/v3/api" "github.com/diamondburned/arikawa/v3/discord" "go.uber.org/zap" - "strings" - "sync" ) type DiscordNotifier struct { diff --git a/node/pkg/p2p/netmetrics.go b/node/pkg/p2p/netmetrics.go index eb926de54..55c44002c 100644 --- a/node/pkg/p2p/netmetrics.go +++ b/node/pkg/p2p/netmetrics.go @@ -1,6 +1,10 @@ package p2p import ( + "math" + "regexp" + "strconv" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/vaa" "github.com/certusone/wormhole/node/pkg/version" @@ -8,9 +12,6 @@ import ( "github.com/libp2p/go-libp2p-core/peer" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "math" - "regexp" - "strconv" ) var ( diff --git a/node/pkg/p2p/p2p.go b/node/pkg/p2p/p2p.go index 724564fb7..3269233ba 100644 --- a/node/pkg/p2p/p2p.go +++ b/node/pkg/p2p/p2p.go @@ -5,6 +5,9 @@ import ( "crypto/ecdsa" "errors" "fmt" + "strings" + "time" + node_common "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/vaa" "github.com/certusone/wormhole/node/pkg/version" @@ -12,8 +15,6 @@ import ( ethcrypto "github.com/ethereum/go-ethereum/crypto" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "strings" - "time" "github.com/libp2p/go-libp2p-core/peer" "github.com/multiformats/go-multiaddr" diff --git a/node/pkg/p2p/registry.go b/node/pkg/p2p/registry.go index 3eb4a910d..cc5394e2d 100644 --- a/node/pkg/p2p/registry.go +++ b/node/pkg/p2p/registry.go @@ -1,9 +1,10 @@ package p2p import ( + "sync" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/vaa" - "sync" ) // The p2p package implements a simple global metrics registry singleton for node status values transmitted on-chain. diff --git a/node/pkg/p2p/registry_test.go b/node/pkg/p2p/registry_test.go index 039bc7087..ca6c21a17 100644 --- a/node/pkg/p2p/registry_test.go +++ b/node/pkg/p2p/registry_test.go @@ -3,11 +3,12 @@ package p2p import ( "crypto/ed25519" "crypto/rand" + "testing" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/vaa" "github.com/mr-tron/base58" "github.com/stretchr/testify/assert" - "testing" ) func TestNewRegistry(t *testing.T) { diff --git a/node/pkg/processor/broadcast.go b/node/pkg/processor/broadcast.go index 9b7e812c4..d2b532e03 100644 --- a/node/pkg/processor/broadcast.go +++ b/node/pkg/processor/broadcast.go @@ -2,9 +2,10 @@ package processor import ( "encoding/hex" + "time" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "time" ethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" diff --git a/node/pkg/processor/cleanup.go b/node/pkg/processor/cleanup.go index b11632a3b..69385daf1 100644 --- a/node/pkg/processor/cleanup.go +++ b/node/pkg/processor/cleanup.go @@ -3,12 +3,13 @@ package processor import ( "context" "encoding/hex" + "time" + "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/db" "github.com/certusone/wormhole/node/pkg/vaa" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "time" "go.uber.org/zap" ) diff --git a/node/pkg/processor/injection.go b/node/pkg/processor/injection.go index e6826f5d6..afc387442 100644 --- a/node/pkg/processor/injection.go +++ b/node/pkg/processor/injection.go @@ -3,6 +3,7 @@ package processor import ( "context" "encoding/hex" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" diff --git a/node/pkg/processor/observation_test.go b/node/pkg/processor/observation_test.go index 86016365b..636092836 100644 --- a/node/pkg/processor/observation_test.go +++ b/node/pkg/processor/observation_test.go @@ -4,6 +4,9 @@ import ( "context" "crypto/ecdsa" "crypto/rand" + "testing" + "time" + "github.com/certusone/wormhole/node/pkg/common" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/vaa" @@ -12,8 +15,6 @@ import ( "github.com/stretchr/testify/assert" "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" - "testing" - "time" ) func getVAA() vaa.VAA { diff --git a/node/pkg/processor/processor.go b/node/pkg/processor/processor.go index 7e446aec9..9afa2ed8e 100644 --- a/node/pkg/processor/processor.go +++ b/node/pkg/processor/processor.go @@ -3,9 +3,10 @@ package processor import ( "context" "crypto/ecdsa" - "github.com/certusone/wormhole/node/pkg/notify/discord" "time" + "github.com/certusone/wormhole/node/pkg/notify/discord" + "github.com/certusone/wormhole/node/pkg/db" ethcommon "github.com/ethereum/go-ethereum/common" diff --git a/node/pkg/processor/quorum_test.go b/node/pkg/processor/quorum_test.go index ddd801730..7c769112d 100644 --- a/node/pkg/processor/quorum_test.go +++ b/node/pkg/processor/quorum_test.go @@ -2,8 +2,9 @@ package processor import ( "fmt" - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestCalculateQuorum(t *testing.T) { diff --git a/node/pkg/publicrpc/publicrpcserver_test.go b/node/pkg/publicrpc/publicrpcserver_test.go index 70d93c1a4..2f95fc3dd 100644 --- a/node/pkg/publicrpc/publicrpcserver_test.go +++ b/node/pkg/publicrpc/publicrpcserver_test.go @@ -2,12 +2,13 @@ package publicrpc import ( "context" + "testing" + publicrpcv1 "github.com/certusone/wormhole/node/pkg/proto/publicrpc/v1" "github.com/stretchr/testify/assert" "go.uber.org/zap" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "testing" ) func TestGetSignedVAANoMessage(t *testing.T) { diff --git a/node/pkg/solana/client.go b/node/pkg/solana/client.go index 9e7637e6c..188b038be 100644 --- a/node/pkg/solana/client.go +++ b/node/pkg/solana/client.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "time" + "github.com/certusone/wormhole/node/pkg/common" "github.com/certusone/wormhole/node/pkg/p2p" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" @@ -19,7 +21,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "go.uber.org/zap" - "time" ) type SolanaWatcher struct { diff --git a/node/pkg/telemetry/telemetry.go b/node/pkg/telemetry/telemetry.go index 7acdebd2d..0ced3c304 100644 --- a/node/pkg/telemetry/telemetry.go +++ b/node/pkg/telemetry/telemetry.go @@ -1,16 +1,17 @@ package telemetry import ( - "cloud.google.com/go/logging" "context" "encoding/json" "fmt" + "io/ioutil" + + "cloud.google.com/go/logging" "github.com/blendle/zapdriver" "go.uber.org/zap" "go.uber.org/zap/buffer" "go.uber.org/zap/zapcore" "google.golang.org/api/option" - "io/ioutil" ) type Telemetry struct { diff --git a/node/pkg/terra/watcher.go b/node/pkg/terra/watcher.go index 0bd88eaf3..d851dc05d 100644 --- a/node/pkg/terra/watcher.go +++ b/node/pkg/terra/watcher.go @@ -5,14 +5,15 @@ import ( "encoding/base64" "encoding/hex" "fmt" - "github.com/certusone/wormhole/node/pkg/p2p" - gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" - "github.com/prometheus/client_golang/prometheus/promauto" "io/ioutil" "net/http" "strconv" "time" + "github.com/certusone/wormhole/node/pkg/p2p" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prometheus/client_golang/prometheus" eth_common "github.com/ethereum/go-ethereum/common" diff --git a/node/pkg/vaa/governance_test.go b/node/pkg/vaa/governance_test.go index 4ec3a73b4..ed93c21af 100644 --- a/node/pkg/vaa/governance_test.go +++ b/node/pkg/vaa/governance_test.go @@ -1,8 +1,11 @@ package vaa -import "testing" -import "time" -import "github.com/stretchr/testify/assert" +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) // Testing the expected default behavior of a CreateGovernanceVAA func TestCreateGovernanceVAA(t *testing.T) { diff --git a/node/pkg/vaa/payloads.go b/node/pkg/vaa/payloads.go index fcd56394b..68b4b3190 100644 --- a/node/pkg/vaa/payloads.go +++ b/node/pkg/vaa/payloads.go @@ -3,6 +3,7 @@ package vaa import ( "bytes" "encoding/binary" + "github.com/ethereum/go-ethereum/common" ) diff --git a/node/pkg/vaa/payloads_test.go b/node/pkg/vaa/payloads_test.go index c40e41d08..832e6e8c9 100644 --- a/node/pkg/vaa/payloads_test.go +++ b/node/pkg/vaa/payloads_test.go @@ -2,9 +2,10 @@ package vaa import ( "encoding/hex" + "testing" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" - "testing" ) func TestCoreModule(t *testing.T) { diff --git a/node/pkg/vaa/structs_test.go b/node/pkg/vaa/structs_test.go index 229c89040..0fdd894dc 100644 --- a/node/pkg/vaa/structs_test.go +++ b/node/pkg/vaa/structs_test.go @@ -5,14 +5,15 @@ import ( "crypto/elliptic" "crypto/rand" "encoding/hex" - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/crypto" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "math/big" "reflect" "testing" "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/crypto" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestChainIDFromString(t *testing.T) { diff --git a/node/pkg/vaa/types_test.go b/node/pkg/vaa/types_test.go index 14794ffa4..677d769b3 100644 --- a/node/pkg/vaa/types_test.go +++ b/node/pkg/vaa/types_test.go @@ -4,11 +4,12 @@ import ( "crypto/ecdsa" "crypto/rand" "encoding/hex" + "testing" + "time" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" "github.com/stretchr/testify/require" - "testing" - "time" ) func TestSerializeDeserialize(t *testing.T) { diff --git a/scripts/lint.sh b/scripts/lint.sh new file mode 100755 index 000000000..a3e4b0020 --- /dev/null +++ b/scripts/lint.sh @@ -0,0 +1,152 @@ +#!/usr/bin/env bash + +# fail if any command fails +set -eo pipefail -o nounset + +ROOT="$(dirname "$(dirname "$(realpath "$0")")")" +DOCKERFILE="$ROOT/Dockerfile.lint" + +VALID_COMMANDS=("lint" "format") + +SELF_ARGS_WITHOUT_DOCKER="" +GOIMPORTS_ARGS="" +GOLANGCI_LINT_ARGS="" + +print_help() { + cat <<-EOF >&2 + Usage: $(basename "$0") [-h] [-c] [-w] [-d] [-l] COMMAND + COMMAND can be one of: "${VALID_COMMANDS[*]}" + -h Print this help. + -c Run in docker and don't worry about dependencies + -w Automatically fix all formatting issues + -d Print diff for all formatting issues + -l List files that have formatting issues + -g Format output to be parsed by github actions + EOF +} + +format(){ + + if [ "$GOIMPORTS_ARGS" == "" ]; then + GOIMPORTS_ARGS="-l" + fi + + # only -l supports output as github action + if [ "$GITHUB_ACTION" == "true" ]; then + GOIMPORTS_ARGS="-l" + fi + + # Check for dependencies + if ! command -v goimports >/dev/null 2>&1; then + printf "%s\n" "Require goimports. You can run this command in a docker container instead with '-c' and not worry about it or install it: \n\tgo install golang.org/x/tools/cmd/goimports@latest" >&2 + exit 1 + fi + + # Use -exec because of pitfall #1 in http://mywiki.wooledge.org/BashPitfalls + GOFMT_OUTPUT="$(find ./node ./event_database -type f -name '*.go' -not -path './node/pkg/proto/*' -print0 | xargs -r -0 goimports $GOIMPORTS_ARGS 2>&1)" + + if [ -n "$GOFMT_OUTPUT" ]; then + if [ "$GITHUB_ACTION" == "true" ]; then + GOFMT_OUTPUT="$(echo "$GOFMT_OUTPUT" | awk '{print "::error file="$0"::Formatting error. Please format using ./scripts/lint.sh -d format."}')" + fi + echo "$GOFMT_OUTPUT" >&2 + exit 1 + fi +} + +lint(){ + # Check for dependencies + if ! command -v golangci-lint >/dev/null 2>&1; then + printf "%s\n" "Require golangci-lint. You can run this command in a docker container instead with '-c' and not worry about it or install it: https://golangci-lint.run/usage/install/" + fi + + # Do the actual linting! + cd "$ROOT"/node + golangci-lint run --skip-dirs pkg/supervisor --timeout=10m --path-prefix=node $GOLANGCI_LINT_ARGS ./... +} + +DOCKER="false" +GITHUB_ACTION="false" + +while getopts 'cwdlgh' opt; do + case "$opt" in + c) + DOCKER="true" + ;; + w) + GOIMPORTS_ARGS+="-w " + SELF_ARGS_WITHOUT_DOCKER+="-w " + ;; + d) + GOIMPORTS_ARGS+="-d " + SELF_ARGS_WITHOUT_DOCKER+="-d " + ;; + l) + GOIMPORTS_ARGS+="-l " + SELF_ARGS_WITHOUT_DOCKER+="-l " + ;; + g) + GOLANGCI_LINT_ARGS+="--out-format=github-actions " + GITHUB_ACTION="true" + SELF_ARGS_WITHOUT_DOCKER+="-g " + ;; + h) + print_help + exit 0 + ;; + ?) + echo "Invalid command option." >&2 + print_help + exit 1 + ;; + esac +done +shift $((OPTIND - 1)) + +if [ "$#" -ne "1" ]; then + echo "Need to specify COMMAND." >&2 + print_help + exit 1 +fi + +COMMAND="$1" + +if [[ ! " ${VALID_COMMANDS[*]} " == *" $COMMAND "* ]]; then + echo "Invalid command $COMMAND." >&2 + print_help + exit 1 +fi + +# run this script recursively inside docker, if requested +if [ "$DOCKER" == "true" ]; then + # The easy thing to do here would be to use a bind mount to share the code with the container. + # But this doesn't work in scenarios where we are in a container already. + # But it's easy so we just won't support that case for now. + # If we wanted to support it, my idea would be to `docker run`, `docker cp`, `docker exec`, `docker rm`. + + if grep -Esq 'docker|lxc|kubepods' /proc/1/cgroup; then + echo "Already running inside a container. This situation isn't supported (yet)." >&2 + exit 1 + fi + + DOCKER_IMAGE="$(docker build -q -f "$DOCKERFILE" .)" + DOCKER_EXEC="./scripts/$(basename "$0")" + MOUNT="--mount=type=bind,target=/app,source=$PWD" + + # for safety, mount as readonly unless -w flag was given + if ! [[ "$GOIMPORTS_ARGS" =~ "w" ]]; then + MOUNT+=",readonly" + fi + docker run --workdir /app "$MOUNT" "$DOCKER_IMAGE" "$DOCKER_EXEC" $SELF_ARGS_WITHOUT_DOCKER "$COMMAND" + exit "$?" +fi + +case $COMMAND in + "lint") + lint + ;; + + "format") + format + ;; +esac