Merge pull request #338 from bandprotocol/cleanup-stricter

[Chore] Cleanup with more lint rules

.github/workflows/golangci-lint.yml

@@ -12,5 +12,5 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.49
+          version: v1.55.2
           args: --timeout=5m0s

.golangci.yml

@@ -1,15 +1,103 @@
-linters-settings:
+run:
+  timeout: 5m
+  go: '1.19'
+  skip-files:
+    - ".*\\.pb\\.go$"
+    - ".*\\.pb\\.gw\\.\\.go$"
+    - ".*\\.pulsar\\.go$"
 
 linters:
   disable-all: true
   enable:
     - bodyclose
-    - gofmt
-    - goimports
+    - whitespace
+    - errcheck
+    - exportloopref
+    - gci
+    - gocritic
+    - gofumpt
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
     - misspell
+    - nolintlint
+    - revive
+    - staticcheck
+    - stylecheck
+    - unconvert
     - typecheck
-    - whitespace
 
-run:
-  timeout: 5m
-  go: '1.19'
+issues:
+  exclude-rules:
+    - text: 'Use of weak random number generator'
+      linters:
+        - gosec
+    - text: 'ST1003:'
+      linters:
+        - stylecheck
+
+linters-settings:
+  gci:
+    custom-order: true
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - prefix(github.com/bandprotocol/chain)
+  gocritic:
+    disabled-checks:
+      - regexpMust
+      - appendAssign
+      - ifElseChain
+  maligned:
+    # print struct with more effective memory layout or not, false by default
+    suggest-new: true
+  nolintlint:
+    allow-unused: false
+    allow-leading-space: true
+    require-explanation: false
+    require-specific: false
+  revive:
+    rules:
+      - name: unused-parameter
+        disabled: true
+  gosec:
+    # To select a subset of rules to run.
+    # Available rules: https://github.com/securego/gosec#available-rules
+    # Default: [] - means include all rules
+    includes:
+      #  - G101 # Look for hard coded credentials
+      - G102 # Bind to all interfaces
+      - G103 # Audit the use of unsafe block
+      - G104 # Audit errors not checked
+      - G106 # Audit the use of ssh.InsecureIgnoreHostKey
+      - G107 # Url provided to HTTP request as taint input
+      - G108 # Profiling endpoint automatically exposed on /debug/pprof
+      - G109 # Potential Integer overflow made by strconv.Atoi result conversion to int16/32
+      - G110 # Potential DoS vulnerability via decompression bomb
+      - G111 # Potential directory traversal
+      - G112 # Potential slowloris attack
+      - G113 # Usage of Rat.SetString in math/big with an overflow (CVE-2022-23772)
+      - G114 # Use of net/http serve function that has no support for setting timeouts
+      - G201 # SQL query construction using format string
+      - G202 # SQL query construction using string concatenation
+      - G203 # Use of unescaped data in HTML templates
+      - G204 # Audit use of command execution
+      - G301 # Poor file permissions used when creating a directory
+      - G302 # Poor file permissions used with chmod
+      - G303 # Creating tempfile using a predictable path
+      - G304 # File path provided as taint input
+      - G305 # File traversal when extracting zip/tar archive
+      - G306 # Poor file permissions used when writing to a new file
+      - G307 # Deferring a method which returns an error
+      - G401 # Detect the usage of DES, RC4, MD5 or SHA1
+      - G402 # Look for bad TLS connection settings
+      - G403 # Ensure minimum RSA key length of 2048 bits
+      - G404 # Insecure random number source (rand)
+      - G501 # Import blocklist: crypto/md5
+      - G502 # Import blocklist: crypto/des
+      - G503 # Import blocklist: crypto/rc4
+      - G504 # Import blocklist: net/http/cgi
+      - G505 # Import blocklist: crypto/sha1
+      - G601 # Implicit memory aliasing of items from a range statement
+

app/app.go

@@ -303,6 +303,7 @@ func NewBandApp(
 	if err != nil {
 		panic(err)
 	}
+
 	// Initialize params keeper and module subspaces.
 	app.ParamsKeeper = initParamsKeeper(
 		appCodec,
@@ -340,6 +341,7 @@ func NewBandApp(
 		Bech32MainPrefix,
 		authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)
+
 	// wrappedBankerKeeper overrides burn token behavior to instead transfer to community pool.
 	app.BankKeeper = bandbankkeeper.NewWrappedBankKeeperBurnToCommunityPool(
 		bankkeeper.NewBaseKeeper(
@@ -351,13 +353,15 @@ func NewBandApp(
 		),
 		app.AccountKeeper,
 	)
+
 	app.StakingKeeper = stakingkeeper.NewKeeper(
 		appCodec,
 		keys[stakingtypes.StoreKey],
 		app.AccountKeeper,
 		app.BankKeeper,
 		authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)
+
 	app.MintKeeper = mintkeeper.NewKeeper(
 		appCodec,
 		keys[minttypes.StoreKey],
@@ -367,6 +371,7 @@ func NewBandApp(
 		authtypes.FeeCollectorName,
 		authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)
+
 	app.DistrKeeper = distrkeeper.NewKeeper(
 		appCodec,
 		keys[distrtypes.StoreKey],
@@ -378,6 +383,7 @@ func NewBandApp(
 	)
 	// DistrKeeper must be set afterward due to the circular reference between banker-staking-distr.
 	app.BankKeeper.SetDistrKeeper(&app.DistrKeeper)
+
 	app.SlashingKeeper = slashingkeeper.NewKeeper(
 		appCodec,
 		legacyAmino,
@@ -488,7 +494,6 @@ func NewBandApp(
 		scopedICAHostKeeper,
 		app.MsgServiceRouter(),
 	)
-
 	icaModule := ica.NewAppModule(nil, &app.ICAHostKeeper)
 	icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper)
 
@@ -536,7 +541,7 @@ func NewBandApp(
 	/****  Module Options ****/
 	// NOTE: we may consider parsing `appOpts` inside module constructors. For the moment
 	// we prefer to be more strict in what arguments the modules expect.
-	var skipGenesisInvariants = cast.ToBool(appOpts.Get(crisis.FlagSkipGenesisInvariants))
+	skipGenesisInvariants := cast.ToBool(appOpts.Get(crisis.FlagSkipGenesisInvariants))
 
 	// NOTE: Any module instantiated in the module manager that is later modified
 	// must be passed by reference here.
@@ -602,11 +607,11 @@ func NewBandApp(
 		oracleModule,
 		globalfee.NewAppModule(app.GlobalfeeKeeper),
 	)
+
 	// NOTE: Oracle module must occur before distr as it takes some fee to distribute to active oracle validators.
 	// NOTE: During begin block slashing happens after distr.BeginBlocker so that there is nothing left
 	// over in the validator fee pool, so as to keep the CanWithdrawInvariant invariant.
 	// NOTE: capability module's beginblocker must come before any modules using capabilities (e.g. IBC)
-
 	app.mm.SetOrderBeginBlockers(
 		upgradetypes.ModuleName,
 		capabilitytypes.ModuleName,
@@ -632,6 +637,7 @@ func NewBandApp(
 		consensusparamtypes.ModuleName,
 		globalfeetypes.ModuleName,
 	)
+
 	app.mm.SetOrderEndBlockers(
 		crisistypes.ModuleName,
 		govtypes.ModuleName,
@@ -765,6 +771,7 @@ func NewBandApp(
 	app.ScopedIBCKeeper = scopedIBCKeeper
 	app.ScopedTransferKeeper = scopedTransferKeeper
 	app.ScopedOracleKeeper = scopedOracleKeeper
+	app.ScopedICAHostKeeper = scopedICAHostKeeper
 
 	return app
 }
@@ -961,7 +968,7 @@ func initParamsKeeper(
 	paramsKeeper.Subspace(minttypes.ModuleName)
 	paramsKeeper.Subspace(distrtypes.ModuleName)
 	paramsKeeper.Subspace(slashingtypes.ModuleName)
-	paramsKeeper.Subspace(govtypes.ModuleName).WithKeyTable(govv1.ParamKeyTable())
+	paramsKeeper.Subspace(govtypes.ModuleName).WithKeyTable(govv1.ParamKeyTable()) //nolint:staticcheck
 	paramsKeeper.Subspace(crisistypes.ModuleName)
 	paramsKeeper.Subspace(ibcexported.ModuleName)
 	paramsKeeper.Subspace(ibctransfertypes.ModuleName)
@@ -996,9 +1003,9 @@ func (app *BandApp) setupUpgradeStoreLoaders() {
 		return
 	}
 
-	for _, upgrade := range Upgrades {
+	for idx, upgrade := range Upgrades {
 		if upgradeInfo.Name == upgrade.UpgradeName {
-			app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &upgrade.StoreUpgrades))
+			app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &Upgrades[idx].StoreUpgrades))
 		}
 	}
 }

app/export.go

@@ -2,10 +2,10 @@ package band
 
 import (
 	"encoding/json"
+	"fmt"
 	"log"
 
 	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
-
 	servertypes "github.com/cosmos/cosmos-sdk/server/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
@@ -88,7 +88,10 @@ func (app *BandApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [
 		if err != nil {
 			panic(err)
 		}
-		_, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr)
+
+		if _, err = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr); err != nil {
+			panic(err)
+		}
 	}
 
 	// clear validator slash events
@@ -109,7 +112,9 @@ func (app *BandApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [
 		feePool.CommunityPool = feePool.CommunityPool.Add(scraps...)
 		app.DistrKeeper.SetFeePool(ctx, feePool)
 
-		app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, val.GetOperator())
+		if err := app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, val.GetOperator()); err != nil {
+			panic(err)
+		}
 		return false
 	})
 
@@ -119,12 +124,17 @@ func (app *BandApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [
 		if err != nil {
 			panic(err)
 		}
-		delAddr, err := sdk.AccAddressFromBech32(del.DelegatorAddress)
-		if err != nil {
-			panic(err)
+		delAddr := sdk.MustAccAddressFromBech32(del.DelegatorAddress)
+
+		if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil {
+			// never called as BeforeDelegationCreated always returns nil
+			panic(fmt.Errorf("error while incrementing period: %w", err))
+		}
+
+		if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil {
+			// never called as AfterDelegationModified always returns nil
+			panic(fmt.Errorf("error while creating a new delegation period record: %w", err))
 		}
-		app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr)
-		app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr)
 	}
 
 	// reset context height
@@ -175,7 +185,10 @@ func (app *BandApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs [
 		counter++
 	}
 
-	iter.Close()
+	if err := iter.Close(); err != nil {
+		app.Logger().Error("error while closing the key-value store reverse prefix iterator: ", err)
+		return
+	}
 
 	_, err := app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
 	if err != nil {

app/upgrades/v2_4/upgrades.go

@@ -86,7 +86,9 @@ func CreateUpgradeHandler(
 		}
 
 		// Oracle DefaultParams only upgrade BaseRequestGas to 50000
-		keepers.OracleKeeper.SetParams(ctx, oracletypes.DefaultParams())
+		if err := keepers.OracleKeeper.SetParams(ctx, oracletypes.DefaultParams()); err != nil {
+			return nil, err
+		}
 
 		consensusParam := am.GetConsensusParams(ctx)
 		consensusParam.Block.MaxGas = 50_000_000

app/upgrades/v2_5/upgrades.go

@@ -12,8 +12,8 @@ import (
 func CreateUpgradeHandler(
 	mm *module.Manager,
 	configurator module.Configurator,
-	_ upgrades.AppManager,
-	_ *keepers.AppKeepers,
+	am upgrades.AppManager,
+	keepers *keepers.AppKeepers,
 ) upgradetypes.UpgradeHandler {
 	return func(ctx sdk.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
 		ctx.Logger().Info("Starting module migrations...")

app/upgrades/v2_6/upgrades.go

@@ -1,7 +1,6 @@
 package v2_6
 
 import (
-	oracletypes "github.com/bandprotocol/chain/v2/x/oracle/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
@@ -21,6 +20,7 @@ import (
 	"github.com/bandprotocol/chain/v2/app/keepers"
 	"github.com/bandprotocol/chain/v2/app/upgrades"
 	globalfeetypes "github.com/bandprotocol/chain/v2/x/globalfee/types"
+	oracletypes "github.com/bandprotocol/chain/v2/x/oracle/types"
 )
 
 func CreateUpgradeHandler(
@@ -41,7 +41,7 @@ func CreateUpgradeHandler(
 			case banktypes.ModuleName:
 				keyTable = banktypes.ParamKeyTable() //nolint:staticcheck
 			case stakingtypes.ModuleName:
-				keyTable = stakingtypes.ParamKeyTable() //nolint:staticcheck
+				keyTable = stakingtypes.ParamKeyTable()
 			case minttypes.ModuleName:
 				keyTable = minttypes.ParamKeyTable() //nolint:staticcheck
 			case distrtypes.ModuleName:
@@ -54,11 +54,11 @@ func CreateUpgradeHandler(
 				keyTable = crisistypes.ParamKeyTable() //nolint:staticcheck
 			// ibc types
 			case ibctransfertypes.ModuleName:
-				keyTable = ibctransfertypes.ParamKeyTable() //nolint:staticcheck
+				keyTable = ibctransfertypes.ParamKeyTable()
 			case icahosttypes.SubModuleName:
-				keyTable = icahosttypes.ParamKeyTable() //nolint:staticcheck
+				keyTable = icahosttypes.ParamKeyTable()
 			case oracletypes.ModuleName:
-				keyTable = oracletypes.ParamKeyTable() //nolint:staticcheck
+				keyTable = oracletypes.ParamKeyTable()
 			default:
 				continue
 			}