Skip to content

Commit

Permalink
Linter on CI/CD and on code (#89)
Browse files Browse the repository at this point in the history
<!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺
v           ✰  Thanks for creating a PR! You're awesome! ✰
v Please note that maintainers will only review those PRs with a
completed PR template.
☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >  -->

## Purpose of Changes and their Description
* Apply linter to CI/CD
* Apply all suggestions, `nolint` where applicable

## Link(s) to Ticket(s) or Issue(s) resolved by this PR

## Are these changes tested and documented?

- [ ] If tested, please describe how. If not, why tests are not needed.
-- needs more testing
- [x] If documented, please describe where. If not, describe why docs
are not needed. -- no need, just linting
- [x] Added to `Unreleased` section of `CHANGELOG.md`?

## Still Left Todo
Further testing and remove TODO on testing global nolints
  • Loading branch information
xmariachi authored Nov 30, 2024
2 parents 34c0c5f + f3bfbe3 commit f058947
Show file tree
Hide file tree
Showing 27 changed files with 271 additions and 126 deletions.
29 changes: 29 additions & 0 deletions .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: golangci-lint
on:
push:
branches:
- main
- dev
- release-*
pull_request:

permissions:
contents: read
# Optional: allow write access to checks to allow the action to annotate code in the PR.
checks: write

jobs:
golangci:
name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version: stable
- name: golangci-lint
uses: golangci/golangci-lint-action@v6
with:
version: v1.60.3
args: --timeout=10m

106 changes: 106 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
run:
timeout: 5m

linters:
disable-all: true
# Enable specific linter
# https://golangci-lint.run/usage/linters/#enabled-by-default-linters
enable:
- asciicheck
- bidichk
- durationcheck
- errcheck
- errname
- copyloopvar
- exhaustruct
- forcetypeassert
- goconst
- gofmt
- goimports
- goheader
- gomodguard
- goprintffuncname
- gosimple
- govet
- importas
- ineffassign
- makezero
- misspell
- nakedret
- nilnil
- promlinter
- staticcheck
- stylecheck
- tenv
- thelper
- tparallel
- typecheck
- thelper
- unconvert
- unused
- unparam
- revive
- gosec
- testifylint

linters-settings:
revive:
rules:
- name: var-naming
severity: warning
disabled: false
exclude: [""]
arguments:
- ["ID", "RPC", "IDS", "JSON"] # AllowList
- [] # DenyList
- - upperCaseConst: true
gosec:
config:
G101:
pattern: "(?i)passwd|pass|password|pwd|secret|private_key|token|pw|apiKey|bearer|cred|mneumonic|seed|seedphrase|entropy"
ignore_entropy: false
# Maximum allowed entropy of the string.
entropy_threshold: "80.0"
# Maximum allowed value of entropy/string length.
# Is taken into account if entropy >= entropy_threshold/2.
per_char_threshold: "3.0"
# Calculate entropy for first N chars of the string.
truncate: "32"

issues:
exclude-rules:
- linters:
- staticcheck
text: "SA1024: cutset contains duplicate characters" # proved to not provide much value, only false positives.
- linters:
- staticcheck
text: "SA9004: only the first constant in this group has an explicit type" # explicitly recommended in go syntax
- linters:
- stylecheck
text: "ST1003:" # requires identifiers with "id" to be "ID".
- linters:
- stylecheck
text: "ST1005:" # punctuation in error messages
- linters:
- staticcheck
text: "SA1019: \"github.com/cosmos/ibc-go/v8/modules/core/02-client/types\" is deprecated`"
- path: _test\.go
text: "var-naming: don't use underscores in Go names;"
linters:
- revive
- path: _test\.go
linters:
- goconst
- path: _test\.go
text: "G404: Use of weak random number generator"
linters:
- gosec

max-issues-per-linter: 10000
max-same-issues: 10000

exclude-files:
- ".*\\.pb\\.go"
- ".*\\.pb\\.gw\\.go"
- ".*\\.pulsar\\.go"
- ".*_mocks\\.go"
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

* [#87](https://github.com/allora-network/allora-offchain-node/pull/87) Update v0.7.0 chain + whitelist coverage
* [#89](https://github.com/allora-network/allora-offchain-node/pull/89) Added linter

### Removed

Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package api_worker_reputer
package apiadapter

import (
"allora_offchain_node/lib"
Expand Down Expand Up @@ -46,7 +46,7 @@ func replaceExtendedPlaceholders(urlTemplate string, params map[string]string, b

func requestEndpoint(url string) (string, error) {
// make request to url
resp, err := http.Get(url)
resp, err := http.Get(url) // nolint: gosec
if err != nil {
return "", fmt.Errorf("failed to make request to %s: %w", url, err)
}
Expand Down Expand Up @@ -145,7 +145,7 @@ func (a *AlloraAdapter) GroundTruth(node lib.ReputerConfig, blockHeight int64) (
}
}
log.Info().Str("url", url).Str("groundTruth", groundTruthDec.String()).Msg("Ground truth")
return lib.Truth(groundTruthDec.String()), nil
return groundTruthDec.String(), nil
}

func (a *AlloraAdapter) LossFunction(node lib.ReputerConfig, groundTruth string, inferenceValue string, options map[string]string) (string, error) {
Expand Down Expand Up @@ -178,7 +178,7 @@ func (a *AlloraAdapter) LossFunction(node lib.ReputerConfig, groundTruth string,
req.Header.Set("Content-Type", "application/json")

// Send the request
client := &http.Client{}
client := &http.Client{} // nolint: exhaustruct
resp, err := client.Do(req)
if err != nil {
return "", fmt.Errorf("failed to send request: %w", err)
Expand Down Expand Up @@ -235,7 +235,7 @@ func (a *AlloraAdapter) IsLossFunctionNeverNegative(node lib.ReputerConfig, opti
req.Header.Set("Content-Type", "application/json")

// Send the request
client := &http.Client{}
client := &http.Client{} // nolint: exhaustruct
resp, err := client.Do(req)
if err != nil {
return false, fmt.Errorf("failed to send request: %w", err)
Expand Down
4 changes: 2 additions & 2 deletions adapter_factory.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package main

import (
api_worker_reputer "allora_offchain_node/adapter/api/worker-reputer"
apiAdapter "allora_offchain_node/adapter/api/apiadapter"
lib "allora_offchain_node/lib"
"fmt"
)

func NewAlloraAdapter(name string) (lib.AlloraAdapter, error) {
switch name {
case "api-worker-reputer":
return api_worker_reputer.NewAlloraAdapter(), nil
return apiAdapter.NewAlloraAdapter(), nil
// Add other cases for different adapters here
default:
return nil, fmt.Errorf("unknown adapter name: %s", name)
Expand Down
2 changes: 1 addition & 1 deletion lib/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const (
)

// A struct that holds the name and help text for a prometheus counter
var COUNTER_DATA = []MetricsCounter{
var CounterData = []MetricsCounter{
{InferenceRequestCount, "The total number of times worker requests inference from source"},
{ForecastRequestCount, "The total number of times worker requests forecast from source"},
{TruthRequestCount, "The total number of times reputer requests truth from source"},
Expand Down
16 changes: 8 additions & 8 deletions lib/domain_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ type WorkerConfig struct {
}

// Implement TopicActor interface for WorkerConfig
func (w WorkerConfig) GetTopicId() emissions.TopicId {
return w.TopicId
func (workerConfig WorkerConfig) GetTopicId() emissions.TopicId {
return workerConfig.TopicId
}

type ReputerConfig struct {
Expand All @@ -81,8 +81,8 @@ type ReputerConfig struct {
}

// Implement TopicActor interface for ReputerConfig
func (r ReputerConfig) GetTopicId() emissions.TopicId {
return r.TopicId
func (reputerConfig ReputerConfig) GetTopicId() emissions.TopicId {
return reputerConfig.TopicId
}

type LossFunctionParameters struct {
Expand Down Expand Up @@ -153,16 +153,16 @@ func (c *UserConfig) ValidateConfigAdapters() error {

func (c *UserConfig) ValidateWalletConfig() error {
if c.Wallet.WindowCorrectionFactor < WindowCorrectionFactorSuggestedMin {
return errors.New(fmt.Sprintf("window correction factor lower than suggested minimum: %f < %f", c.Wallet.WindowCorrectionFactor, WindowCorrectionFactorSuggestedMin))
return fmt.Errorf("window correction factor lower than suggested minimum: %f < %f", c.Wallet.WindowCorrectionFactor, WindowCorrectionFactorSuggestedMin)
}
if c.Wallet.BlockDurationEstimated < BlockDurationEstimatedMin {
return errors.New(fmt.Sprintf("block duration estimated lower than the minimum: %f < %f", c.Wallet.BlockDurationEstimated, BlockDurationEstimatedMin))
return fmt.Errorf("block duration estimated lower than the minimum: %f < %f", c.Wallet.BlockDurationEstimated, BlockDurationEstimatedMin)
}
if c.Wallet.RetryDelay < RetryDelayMin {
return errors.New(fmt.Sprintf("retry delay lower than the minimum: %d < %d", c.Wallet.RetryDelay, RetryDelayMin))
return fmt.Errorf("retry delay lower than the minimum: %d < %d", c.Wallet.RetryDelay, RetryDelayMin)
}
if c.Wallet.AccountSequenceRetryDelay < AccountSequenceRetryDelayMin {
return errors.New(fmt.Sprintf("account sequence retry delay lower than the minimum: %d < %d", c.Wallet.AccountSequenceRetryDelay, AccountSequenceRetryDelayMin))
return fmt.Errorf("account sequence retry delay lower than the minimum: %d < %d", c.Wallet.AccountSequenceRetryDelay, AccountSequenceRetryDelayMin)
}

return nil
Expand Down
34 changes: 17 additions & 17 deletions lib/factory_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,33 +51,33 @@ func getAlloraClient(config *UserConfig) (*cosmosclient.Client, error) {
return &client, nil
}

func (config *UserConfig) GenerateNodeConfig() (*NodeConfig, error) {
client, err := getAlloraClient(config)
func (c *UserConfig) GenerateNodeConfig() (*NodeConfig, error) {
client, err := getAlloraClient(c)
if err != nil {
config.Wallet.SubmitTx = false
c.Wallet.SubmitTx = false
return nil, err
}
var account *cosmosaccount.Account
// if we're giving a keyring ring name, with no mnemonic restore
if config.Wallet.AddressRestoreMnemonic == "" && config.Wallet.AddressKeyName != "" {
if c.Wallet.AddressRestoreMnemonic == "" && c.Wallet.AddressKeyName != "" {
// get account from the keyring
acc, err := client.Account(config.Wallet.AddressKeyName)
acc, err := client.Account(c.Wallet.AddressKeyName)
if err != nil {
config.Wallet.SubmitTx = false
c.Wallet.SubmitTx = false
log.Error().Err(err).Msg("could not retrieve account from keyring")
} else {
account = &acc
}
} else if config.Wallet.AddressRestoreMnemonic != "" && config.Wallet.AddressKeyName != "" {
} else if c.Wallet.AddressRestoreMnemonic != "" && c.Wallet.AddressKeyName != "" {
// restore from mnemonic
acc, err := client.AccountRegistry.Import(config.Wallet.AddressKeyName, config.Wallet.AddressRestoreMnemonic, "")
acc, err := client.AccountRegistry.Import(c.Wallet.AddressKeyName, c.Wallet.AddressRestoreMnemonic, "")
if err != nil {
if err.Error() == "account already exists" {
acc, err = client.Account(config.Wallet.AddressKeyName)
acc, err = client.Account(c.Wallet.AddressKeyName)
}

if err != nil {
config.Wallet.SubmitTx = false
c.Wallet.SubmitTx = false
log.Err(err).Msg("could not restore account from mnemonic")
} else {
account = &acc
Expand All @@ -95,7 +95,7 @@ func (config *UserConfig) GenerateNodeConfig() (*NodeConfig, error) {

address, err := account.Address(ADDRESS_PREFIX)
if err != nil {
config.Wallet.SubmitTx = false
c.Wallet.SubmitTx = false
log.Err(err).Msg("could not retrieve allora blockchain address, transactions will not be submitted to chain")
} else {
log.Info().Str("address", address).Msg("allora blockchain address loaded")
Expand All @@ -107,12 +107,12 @@ func (config *UserConfig) GenerateNodeConfig() (*NodeConfig, error) {
// Create bank client
bankClient := banktypes.NewQueryClient(client.Context())

// this is terrible, no isConnected as part of this code path
// Check chainId is set
if client.Context().ChainID == "" {
return nil, nil
return nil, errors.New("ChainId is empty")
}

config.Wallet.Address = address // Overwrite the address with the one from the keystore
c.Wallet.Address = address // Overwrite the address with the one from the keystore

log.Info().Msg("Allora client created successfully")
log.Info().Msg("Wallet address: " + address)
Expand All @@ -129,9 +129,9 @@ func (config *UserConfig) GenerateNodeConfig() (*NodeConfig, error) {

Node := NodeConfig{
Chain: alloraChain,
Wallet: config.Wallet,
Worker: config.Worker,
Reputer: config.Reputer,
Wallet: c.Wallet,
Worker: c.Worker,
Reputer: c.Reputer,
}

return &Node, nil
Expand Down
13 changes: 11 additions & 2 deletions lib/metrics_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package lib
import (
"net/http"
"strconv"
"time"

"github.com/rs/zerolog/log"

Expand Down Expand Up @@ -31,7 +32,7 @@ func (metrics *Metrics) RegisterMetricsCounters() {

for _, counter := range metrics.Counters {
counterVec := prometheus.NewCounterVec(
prometheus.CounterOpts{
prometheus.CounterOpts{ // nolint: exhaustruct
Name: counter.Name,
Help: counter.Help,
},
Expand All @@ -47,7 +48,15 @@ func (metrics Metrics) StartMetricsServer(port string) {
http.Handle("/metrics", promhttp.Handler())
go func() {
log.Info().Msgf("Starting metrics server on %s", port)
if err := http.ListenAndServe(port, nil); err != nil {
srv := &http.Server{ // nolint: exhaustruct
Addr: port,
ReadTimeout: 30 * time.Second,
WriteTimeout: 30 * time.Second,
IdleTimeout: 60 * time.Second,
ReadHeaderTimeout: 10 * time.Second,
}

if err := srv.ListenAndServe(); err != nil {
log.Error().Err(err).Msg("Could not start metric server")
return
}
Expand Down
6 changes: 3 additions & 3 deletions lib/repo_query_actor_whitelist.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func (node *NodeConfig) IsWorkerWhitelisted(topicId emissionstypes.TopicId, addr
Address: address,
})
},
query.PageRequest{},
query.PageRequest{}, // nolint: exhaustruct
"check worker whitelist",
)
if err != nil {
Expand All @@ -46,7 +46,7 @@ func (node *NodeConfig) IsReputerWhitelisted(topicId emissionstypes.TopicId, add
Address: address,
})
},
query.PageRequest{},
query.PageRequest{}, // nolint: exhaustruct
"check reputer whitelist",
)
if err != nil {
Expand All @@ -69,7 +69,7 @@ func (node *NodeConfig) IsWhitelistedGlobalActor(address string) (bool, error) {
Address: address,
})
},
query.PageRequest{},
query.PageRequest{}, // nolint: exhaustruct
"check global actor whitelist",
)
if err != nil {
Expand Down
Loading

0 comments on commit f058947

Please sign in to comment.