From 9a85dbafe57b8b3076663eb14fab3d4ef69526af Mon Sep 17 00:00:00 2001 From: Leo Date: Mon, 30 Aug 2021 20:40:38 +0200 Subject: [PATCH] Add Go linting stage to CI rustfmt appears to be a little more complicated since it wants to download dependencies and needs nightly Rust. Change-Id: Ia348def30a6459ae2ab6c29a8c3a413216f5eb4b --- Dockerfile.lint | 12 +++++++ jenkins-presubmit.groovy | 37 ++++++++++++++++++-- lint.sh | 3 ++ node/cmd/guardiand/adminnodes.go | 4 +-- node/cmd/guardiand/adminserver.go | 3 ++ node/cmd/guardiand/node.go | 14 +++----- node/pkg/devnet/guardianset_vaa.go | 2 +- node/pkg/processor/cleanup.go | 2 +- node/pkg/processor/observation.go | 10 ------ node/pkg/readiness/health.go | 2 +- terra/contracts/token-bridge/src/contract.rs | 6 ++-- 11 files changed, 67 insertions(+), 28 deletions(-) create mode 100644 Dockerfile.lint create mode 100755 lint.sh diff --git a/Dockerfile.lint b/Dockerfile.lint new file mode 100644 index 000000000..f74329288 --- /dev/null +++ b/Dockerfile.lint @@ -0,0 +1,12 @@ +# syntax=docker.io/docker/dockerfile:experimental@sha256:de85b2f3a3e8a2f7fe48e8e84a65f6fdd5cd5183afa6412fff9caa6871649c44 +FROM docker.io/golang:1.17.0@sha256:06e92e576fc7a7067a268d47727f3083c0a564331bfcbfdde633157fc91fb17d AS go + +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 + +RUN --mount=type=bind,target=/app,source=node cd /app && \ + GOGC=off ~/golangci-lint run --skip-dirs pkg/supervisor ./... diff --git a/jenkins-presubmit.groovy b/jenkins-presubmit.groovy index 8e6dea928..c4a0bfd36 100644 --- a/jenkins-presubmit.groovy +++ b/jenkins-presubmit.groovy @@ -4,6 +4,7 @@ pipeline { agent none stages { stage('Parallel') { + failFast true parallel { stage('Test') { agent { @@ -35,11 +36,9 @@ pipeline { } post { success { - gerritReview labels: [Verified: 1] gerritCheck checks: ['jenkins:test': 'SUCCESSFUL'] } unsuccessful { - gerritReview labels: [Verified: -1] gerritCheck checks: ['jenkins:test': 'FAILED'] } cleanup { @@ -47,6 +46,40 @@ pipeline { } } } + stage('Lint') { + agent { + node { + label "" + customWorkspace '/home/ci/wormhole' + } + } + steps { + gerritCheck checks: ['jenkins:linters': 'RUNNING'], message: "Running on ${env.NODE_NAME}" + + echo "Gerrit change: ${GERRIT_CHANGE_URL}" + + timeout(time: 1, unit: 'MINUTES') { + sh "make generate" + sh "./lint.sh" + } + } + post { + success { + gerritCheck checks: ['jenkins:linters': 'SUCCESSFUL'] + } + unsuccessful { + gerritCheck checks: ['jenkins:linters': 'FAILED'] + } + } + } + } + post { + success { + gerritReview labels: [Verified: 1] + } + unsuccessful { + gerritReview labels: [Verified: -1] + } } } } diff --git a/lint.sh b/lint.sh new file mode 100755 index 000000000..53dbc2b06 --- /dev/null +++ b/lint.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env bash + +DOCKER_BUILDKIT=1 tilt docker build -- -f Dockerfile.lint . diff --git a/node/cmd/guardiand/adminnodes.go b/node/cmd/guardiand/adminnodes.go index 9254205ff..223187509 100644 --- a/node/cmd/guardiand/adminnodes.go +++ b/node/cmd/guardiand/adminnodes.go @@ -65,9 +65,9 @@ func runListNodes(cmd *cobra.Command, args []string) { w := tabwriter.NewWriter(os.Stdout, 0, 8, 2, ' ', 0) if showDetails { - w.Write([]byte("Node key\tGuardian key\tNode name\tVersion\tLast seen\tUptime\tSolana\tEthereum\tTerra\tBSC\n")) + _, _ = w.Write([]byte("Node key\tGuardian key\tNode name\tVersion\tLast seen\tUptime\tSolana\tEthereum\tTerra\tBSC\n")) } else { - w.Write([]byte("Node key\tGuardian key\tNode name\tVersion\tLast seen\tSolana\tEthereum\tTerra\tBSC\n")) + _, _ = w.Write([]byte("Node key\tGuardian key\tNode name\tVersion\tLast seen\tSolana\tEthereum\tTerra\tBSC\n")) } for _, h := range nodes { diff --git a/node/cmd/guardiand/adminserver.go b/node/cmd/guardiand/adminserver.go index 0998c065d..822889d19 100644 --- a/node/cmd/guardiand/adminserver.go +++ b/node/cmd/guardiand/adminserver.go @@ -158,6 +158,9 @@ func adminServiceRunnable(logger *zap.Logger, socketPath string, injectC chan<- // The umask avoids a race condition between file creation and chmod. laddr, err := net.ResolveUnixAddr("unix", socketPath) + if err != nil { + return nil, fmt.Errorf("invalid listen address: %v", err) + } l, err := net.ListenUnix("unix", laddr) if err != nil { return nil, fmt.Errorf("failed to listen on %s: %w", socketPath, err) diff --git a/node/cmd/guardiand/node.go b/node/cmd/guardiand/node.go index ac1bfc6b5..42ca25c80 100644 --- a/node/cmd/guardiand/node.go +++ b/node/cmd/guardiand/node.go @@ -536,18 +536,14 @@ func runNode(cmd *cobra.Command, args []string) { logger.Info("Started internal services") - select { - case <-ctx.Done(): - return nil - } + <-ctx.Done() + return nil }, // It's safer to crash and restart the process in case we encounter a panic, // rather than attempting to reschedule the runnable. supervisor.WithPropagatePanic) - select { - case <-rootCtx.Done(): - logger.Info("root context cancelled, exiting...") - // TODO: wait for things to shut down gracefully - } + <-rootCtx.Done() + logger.Info("root context cancelled, exiting...") + // TODO: wait for things to shut down gracefully } diff --git a/node/pkg/devnet/guardianset_vaa.go b/node/pkg/devnet/guardianset_vaa.go index c7e9d2d4b..fcec61eb2 100644 --- a/node/pkg/devnet/guardianset_vaa.go +++ b/node/pkg/devnet/guardianset_vaa.go @@ -82,7 +82,7 @@ func GetKeyedTransactor(ctx context.Context) *bind.TransactOpts { panic(err) } - kt := bind.NewKeyedTransactor(key) + kt := bind.NewKeyedTransactor(key) // nolint kt.Context = ctx return kt diff --git a/node/pkg/processor/cleanup.go b/node/pkg/processor/cleanup.go index 74095f116..d67c4f9f9 100644 --- a/node/pkg/processor/cleanup.go +++ b/node/pkg/processor/cleanup.go @@ -49,7 +49,7 @@ func (p *Processor) handleCleanup(ctx context.Context) { aggregationStateEntries.Set(float64(len(p.state.vaaSignatures))) for hash, s := range p.state.vaaSignatures { - delta := time.Now().Sub(s.firstObserved) + delta := time.Since(s.firstObserved) switch { case !s.settled && delta.Seconds() >= 30: diff --git a/node/pkg/processor/observation.go b/node/pkg/processor/observation.go index 2548e0443..2564731d0 100644 --- a/node/pkg/processor/observation.go +++ b/node/pkg/processor/observation.go @@ -39,16 +39,6 @@ var ( Name: "wormhole_observations_unknown_total", Help: "Total number of verified observations we haven't seen ourselves", }) - observationsDirectSubmissionsTotal = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "wormhole_observations_direct_submissions_queued_total", - Help: "Total number of observations for a specific target chain that were queued for direct submission", - }, []string{"target_chain"}) - observationsDirectSubmissionSuccessTotal = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "wormhole_observations_direct_submission_success_total", - Help: "Total number of observations for a specific target chain that succeeded", - }, []string{"target_chain"}) ) // handleObservation processes a remote VAA observation, verifies it, checks whether the VAA has met quorum, diff --git a/node/pkg/readiness/health.go b/node/pkg/readiness/health.go index 698fdffab..a14233ada 100644 --- a/node/pkg/readiness/health.go +++ b/node/pkg/readiness/health.go @@ -55,7 +55,7 @@ func Handler(w http.ResponseWriter, r *http.Request) { mu.Lock() for k, v := range registry { - _, err = fmt.Fprintln(resp, fmt.Sprintf("%s\t%v", k, v)) + _, err = fmt.Fprintf(resp, "%s\t%v\n", k, v) if err != nil { panic(err) } diff --git a/terra/contracts/token-bridge/src/contract.rs b/terra/contracts/token-bridge/src/contract.rs index 6f0a9b63e..cc6fc3ff9 100644 --- a/terra/contracts/token-bridge/src/contract.rs +++ b/terra/contracts/token-bridge/src/contract.rs @@ -562,11 +562,13 @@ fn handle_initiate_transfer( amount = Uint128( amount .u128() - .checked_sub(amount.u128().checked_rem(multiplier).unwrap()).unwrap(), + .checked_sub(amount.u128().checked_rem(multiplier).unwrap()) + .unwrap(), ); fee = Uint128( fee.u128() - .checked_sub(fee.u128().checked_rem(multiplier).unwrap()).unwrap(), + .checked_sub(fee.u128().checked_rem(multiplier).unwrap()) + .unwrap(), ); // This is a regular asset, transfer its balance