[TODO] Blockers @okdas (#674)

Co-authored-by: Bryan White <[email protected]>

.golangci.yml

@@ -1,27 +1,23 @@
 linters-settings:
   govet:
-    check-shadowing: true
+    enable:
+      - shadow
 
-# TODO_TECHDEBT: Enable each linter listed, 1 by 1, fixing issues as they appear.
-# Don't forget to delete the `disable-all: true` line as well.
 linters:
   disable-all: true
   enable:
-#    - govet
-#    - revive
-#    - errcheck
-#    - unused
+    - errcheck
+    - gosimple
     - goimports
+    - govet
+    - ineffassign
+    - staticcheck
+    - unused
 
 issues:
   exclude-use-default: true
   max-issues-per-linter: 0
   max-same-issues: 0
-  # TODO_CONSIDERATION/TODO_TECHDEBT: Perhaps we should prefer enforcing the best
-  # practices suggested by the linters over convention or the default in generated
-  # code (where possible). The more exceptions we have, the bigger the gaps will be
-  # in our linting coverage. We could eliminate or reduce these exceptions step-
-  # by-step.
   exclude-rules:
     # Exclude cosmos-sdk module genesis.go files as they are generated with an
     # empty import block containing a comment used by ignite CLI.
@@ -48,6 +44,7 @@ issues:
     - path: ^x/.+/codec.go$
       linters:
         - revive
+    # Exclude test files from errcheck linter
     - path: _test\.go$
       linters:
         - errcheck

Makefile

@@ -85,11 +85,10 @@ endif
 ####################
 
 # TODO_IMPROVE(@okdas): Add other dependencies (ignite, docker, k8s, etc) here
-# TODO_BLOCKER(@okdas): bump `golangci-lint` when we upgrade golang to 1.21+
 .PHONY: install_ci_deps
 install_ci_deps: ## Installs `mockgen` and other go tools
 	go install "github.com/golang/mock/[email protected]" && mockgen --version
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3 && golangci-lint --version
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.59.1 && golangci-lint --version
 	go install golang.org/x/tools/cmd/goimports@latest
 	go install github.com/mikefarah/yq/v4@latest
 

Tiltfile

@@ -150,20 +150,20 @@ configmap_create(
 secret_create_generic(
     "poktrolld-keys", from_file=listdir("localnet/poktrolld/keyring-test/")
 )
+
+# Import validator keys for the poktrolld helm chart to consume
+secret_create_generic(
+    "poktrolld-validator-keys",
+    from_file=[
+        "localnet/poktrolld/config/node_key.json",
+        "localnet/poktrolld/config/priv_validator_key.json",
+    ],
+)
+
 # Import configuration files into Kubernetes ConfigMap
 configmap_create(
     "poktrolld-configs", from_file=listdir("localnet/poktrolld/config/"), watch=True
 )
-# TODO_BLOCKER(@okdas): Import validator keys when we switch to `poktrolld` helm chart
-# by uncommenting the following lines:
-# load("ext://secret", "secret_create_generic")
-# secret_create_generic(
-#     "poktrolld-validator-keys",
-#     from_file=[
-#         "localnet/poktrolld/config/node_key.json",
-#         "localnet/poktrolld/config/priv_validator_key.json",
-#     ],
-# )
 
 # Hot reload protobuf changes
 local_resource(

app/app.go

@@ -190,7 +190,7 @@ func New(
 		appBuilder *runtime.AppBuilder
 
 		// merge the AppConfig and other configuration in one config
-		appConfig = depinject.Configs(
+		deps = depinject.Configs(
 			AppConfig(),
 			depinject.Supply(
 				// Supply the application options
@@ -246,7 +246,7 @@ func New(
 		)
 	)
 
-	if err := depinject.Inject(appConfig,
+	if err := depinject.Inject(deps,
 		&appBuilder,
 		&app.appCodec,
 		&app.legacyAmino,

app/export.go

@@ -94,9 +94,9 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 	}
 
 	for _, delegation := range dels {
-		valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
-		if err != nil {
-			panic(err)
+		valAddr, bech32Err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
+		if bech32Err != nil {
+			panic(bech32Err)
 		}
 
 		delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress)
@@ -116,44 +116,44 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 
 	// reinitialize all validators
 	err = app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
-		valBz, err := app.StakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
-		if err != nil {
-			panic(err)
+		valBz, addrErr := app.StakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
+		if addrErr != nil {
+			panic(addrErr)
 		}
 		// donate any unwithdrawn outstanding reward fraction tokens to the community pool
-		scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, valBz)
-		if err != nil {
-			panic(err)
+		scraps, rewardsErr := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, valBz)
+		if rewardsErr != nil {
+			panic(rewardsErr)
 		}
-		feePool, err := app.DistrKeeper.FeePool.Get(ctx)
-		if err != nil {
-			panic(err)
+		feePool, feeErr := app.DistrKeeper.FeePool.Get(ctx)
+		if feeErr != nil {
+			panic(feeErr)
 		}
 		feePool.CommunityPool = feePool.CommunityPool.Add(scraps...)
-		if err := app.DistrKeeper.FeePool.Set(ctx, feePool); err != nil {
+		if err = app.DistrKeeper.FeePool.Set(ctx, feePool); err != nil {
 			panic(err)
 		}
 
-		if err := app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, valBz); err != nil {
+		if err = app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, valBz); err != nil {
 			panic(err)
 		}
 		return false
 	})
 
 	// reinitialize all delegations
 	for _, del := range dels {
-		valAddr, err := sdk.ValAddressFromBech32(del.ValidatorAddress)
-		if err != nil {
-			panic(err)
+		valAddr, bech32Err := sdk.ValAddressFromBech32(del.ValidatorAddress)
+		if bech32Err != nil {
+			panic(bech32Err)
 		}
 		delAddr := sdk.MustAccAddressFromBech32(del.DelegatorAddress)
 
-		if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil {
+		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 {
+		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))
 		}
@@ -165,7 +165,7 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 	/* Handle staking state. */
 
 	// iterate through redelegations, reset creation height
-	app.StakingKeeper.IterateRedelegations(ctx, func(_ int64, red stakingtypes.Redelegation) (stop bool) {
+	err = app.StakingKeeper.IterateRedelegations(ctx, func(_ int64, red stakingtypes.Redelegation) (stop bool) {
 		for i := range red.Entries {
 			red.Entries[i].CreationHeight = 0
 		}
@@ -175,9 +175,12 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 		}
 		return false
 	})
+	if err != nil {
+		panic(fmt.Errorf("unable to iterate reldelegations: %w", err))
+	}
 
 	// iterate through unbonding delegations, reset creation height
-	app.StakingKeeper.IterateUnbondingDelegations(ctx, func(_ int64, ubd stakingtypes.UnbondingDelegation) (stop bool) {
+	err = app.StakingKeeper.IterateUnbondingDelegations(ctx, func(_ int64, ubd stakingtypes.UnbondingDelegation) (stop bool) {
 		for i := range ubd.Entries {
 			ubd.Entries[i].CreationHeight = 0
 		}
@@ -187,6 +190,9 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 		}
 		return false
 	})
+	if err != nil {
+		panic(fmt.Errorf("unable to iterate unbonding delegations: %w", err))
+	}
 
 	// Iterate through validators by power descending, reset bond heights, and
 	// update bond intra-tx counters.
@@ -196,8 +202,8 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 
 	for ; iter.Valid(); iter.Next() {
 		addr := sdk.ValAddress(stakingtypes.AddressFromValidatorsKey(iter.Key()))
-		validator, err := app.StakingKeeper.GetValidator(ctx, addr)
-		if err != nil {
+		validator, valErr := app.StakingKeeper.GetValidator(ctx, addr)
+		if valErr != nil {
 			panic("expected validator, not found")
 		}
 
@@ -206,11 +212,13 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 			validator.Jailed = true
 		}
 
-		app.StakingKeeper.SetValidator(ctx, validator)
+		if err = app.StakingKeeper.SetValidator(ctx, validator); err != nil {
+			panic(fmt.Errorf("unable to set validator: %v: %w", validator, err))
+		}
 		counter++
 	}
 
-	if err := iter.Close(); err != nil {
+	if err = iter.Close(); err != nil {
 		app.Logger().Error("error while closing the key-value store reverse prefix iterator: ", err)
 		return
 	}
@@ -223,12 +231,16 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 	/* Handle slashing state. */
 
 	// reset start height on signing infos
-	app.SlashingKeeper.IterateValidatorSigningInfos(
+	if err = app.SlashingKeeper.IterateValidatorSigningInfos(
 		ctx,
 		func(addr sdk.ConsAddress, info slashingtypes.ValidatorSigningInfo) (stop bool) {
 			info.StartHeight = 0
-			app.SlashingKeeper.SetValidatorSigningInfo(ctx, addr, info)
+			if err = app.SlashingKeeper.SetValidatorSigningInfo(ctx, addr, info); err != nil {
+				panic(fmt.Errorf("unable to set validator signing info: %w", err))
+			}
 			return false
 		},
-	)
+	); err != nil {
+		panic(fmt.Errorf("unable to iterate validator signing infos: %w", err))
+	}
 }

app/ibc.go

@@ -26,7 +26,7 @@ import (
 	ibctransferkeeper "github.com/cosmos/ibc-go/v8/modules/apps/transfer/keeper"
 	ibctransfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
 	ibc "github.com/cosmos/ibc-go/v8/modules/core"
-	ibcclienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
+	ibcclienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" //nolint:staticcheck // SA1019
 	ibcconnectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
 	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 	ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"

cmd/poktrolld/cmd/commands.go

@@ -177,7 +177,7 @@ func appExport(
 			return servertypes.ExportedApp{}, err
 		}
 
-		if err := bApp.LoadHeight(height); err != nil {
+		if err = bApp.LoadHeight(height); err != nil {
 			return servertypes.ExportedApp{}, err
 		}
 	} else {

cmd/poktrolld/cmd/root.go

@@ -1,14 +1,16 @@
 package cmd
 
 import (
+	"errors"
+	"log"
 	"os"
 	"strings"
 
 	"cosmossdk.io/client/v2/autocli"
 	clientv2keyring "cosmossdk.io/client/v2/autocli/keyring"
 	"cosmossdk.io/core/address"
 	"cosmossdk.io/depinject"
-	"cosmossdk.io/log"
+	cosmoslog "cosmossdk.io/log"
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/config"
 	"github.com/cosmos/cosmos-sdk/client/flags"
@@ -43,7 +45,7 @@ func NewRootCmd() *cobra.Command {
 	if err := depinject.Inject(
 		depinject.Configs(app.AppConfig(),
 			depinject.Supply(
-				log.NewNopLogger(),
+				cosmoslog.NewNopLogger(),
 			),
 			depinject.Provide(
 				ProvideClientContext,
@@ -62,13 +64,13 @@ func NewRootCmd() *cobra.Command {
 		Use:           app.Name + "d",
 		Short:         "Start poktroll node",
 		SilenceErrors: true,
-		PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
+		PersistentPreRunE: func(cmd *cobra.Command, _ []string) (err error) {
 			// set the default command outputs
 			cmd.SetOut(cmd.OutOrStdout())
 			cmd.SetErr(cmd.ErrOrStderr())
 
 			clientCtx = clientCtx.WithCmdContext(cmd.Context())
-			clientCtx, err := client.ReadPersistentCommandFlags(clientCtx, cmd.Flags())
+			clientCtx, err = client.ReadPersistentCommandFlags(clientCtx, cmd.Flags())
 			if err != nil {
 				return err
 			}
@@ -91,11 +93,11 @@ func NewRootCmd() *cobra.Command {
 			}
 
 			clientCtx = clientCtx.WithTxConfig(txConfigWithTextual)
-			if err := client.SetCmdClientContextHandler(clientCtx, cmd); err != nil {
+			if err = client.SetCmdClientContextHandler(clientCtx, cmd); err != nil {
 				return err
 			}
 
-			if err := client.SetCmdClientContextHandler(clientCtx, cmd); err != nil {
+			if err = client.SetCmdClientContextHandler(clientCtx, cmd); err != nil {
 				return err
 			}
 
@@ -115,10 +117,12 @@ func NewRootCmd() *cobra.Command {
 	}
 	initRootCmd(rootCmd, clientCtx.TxConfig, clientCtx.InterfaceRegistry, clientCtx.Codec, moduleBasicManager)
 
-	overwriteFlagDefaults(rootCmd, map[string]string{
+	if err := overwriteFlagDefaults(rootCmd, map[string]string{
 		flags.FlagChainID:        strings.ReplaceAll(app.Name, "-", ""),
 		flags.FlagKeyringBackend: "test",
-	})
+	}); err != nil {
+		log.Fatal(err)
+	}
 
 	if err := autoCliOpts.EnhanceRootCommand(rootCmd); err != nil {
 		panic(err)
@@ -137,20 +141,24 @@ func NewRootCmd() *cobra.Command {
 	return rootCmd
 }
 
-func overwriteFlagDefaults(c *cobra.Command, defaults map[string]string) {
-	set := func(s *pflag.FlagSet, key, val string) {
+func overwriteFlagDefaults(c *cobra.Command, defaults map[string]string) (err error) {
+	set := func(s *pflag.FlagSet, key, val string) error {
 		if f := s.Lookup(key); f != nil {
 			f.DefValue = val
-			f.Value.Set(val)
+			if err = f.Value.Set(val); err != nil {
+				return err
+			}
 		}
+		return nil
 	}
 	for key, val := range defaults {
-		set(c.Flags(), key, val)
-		set(c.PersistentFlags(), key, val)
+		err = errors.Join(err, set(c.Flags(), key, val))
+		err = errors.Join(err, set(c.PersistentFlags(), key, val))
 	}
 	for _, c := range c.Commands() {
-		overwriteFlagDefaults(c, defaults)
+		err = errors.Join(err, overwriteFlagDefaults(c, defaults))
 	}
+	return err
 }
 
 func ProvideClientContext(

docs/docs.go

@@ -29,7 +29,7 @@ func handler(title string) http.HandlerFunc {
 	t, _ := httptemplate.ParseFS(template, indexFile)
 
 	return func(w http.ResponseWriter, req *http.Request) {
-		t.Execute(w, struct {
+		_ = t.Execute(w, struct {
 			Title string
 			URL   string
 		}{