refactor(simapp): Audit simapp (backport #21311) (#21470)

Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>

.github/workflows/test.yml

@@ -605,7 +605,7 @@ jobs:
           simdv2 start &
           SIMD_PID=$!
           cnt=0
-          while ! simdv2 query block --type=height 5; do
+          while ! simdv2 query comet block --type=height 5; do
             cnt=$((cnt + 1))
             if [ $cnt -gt 30 ]; then
               kill -9 "$SIMD_PID"

server/v2/cometbft/commands.go

@@ -13,7 +13,6 @@ import (
 	"github.com/cometbft/cometbft/p2p"
 	pvm "github.com/cometbft/cometbft/privval"
 	rpchttp "github.com/cometbft/cometbft/rpc/client/http"
-	"github.com/cometbft/cometbft/rpc/client/local"
 	cmtversion "github.com/cometbft/cometbft/version"
 	"github.com/spf13/cobra"
 	"google.golang.org/protobuf/encoding/protojson"
@@ -28,35 +27,25 @@ import (
 	"github.com/cosmos/cosmos-sdk/version"
 )
 
-func (s *CometBFTServer[T]) rpcClient(cmd *cobra.Command) (rpc.CometRPC, error) {
-	if s.config.AppTomlConfig.Standalone {
-		client, err := rpchttp.New(client.GetConfigFromCmd(cmd).RPC.ListenAddress)
-		if err != nil {
-			return nil, err
-		}
-		return client, nil
+func rpcClient(cmd *cobra.Command) (rpc.CometRPC, error) {
+	rpcURI, err := cmd.Flags().GetString(FlagNode)
+	if err != nil {
+		return nil, err
 	}
-
-	if s.Node == nil || cmd.Flags().Changed(FlagNode) {
-		rpcURI, err := cmd.Flags().GetString(FlagNode)
-		if err != nil {
-			return nil, err
-		}
-		if rpcURI != "" {
-			return rpchttp.New(rpcURI)
-		}
+	if rpcURI == "" {
+		return nil, fmt.Errorf("rpc URI is empty")
 	}
 
-	return local.New(s.Node), nil
+	return rpchttp.New(rpcURI)
 }
 
 // StatusCommand returns the command to return the status of the network.
-func (s *CometBFTServer[T]) StatusCommand() *cobra.Command {
+func StatusCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "status",
 		Short: "Query remote node for status",
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			rpcclient, err := s.rpcClient(cmd)
+			rpcclient, err := rpcClient(cmd)
 			if err != nil {
 				return err
 			}
@@ -82,7 +71,7 @@ func (s *CometBFTServer[T]) StatusCommand() *cobra.Command {
 }
 
 // ShowNodeIDCmd - ported from CometBFT, dump node ID to stdout
-func (s *CometBFTServer[T]) ShowNodeIDCmd() *cobra.Command {
+func ShowNodeIDCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "show-node-id",
 		Short: "Show this node's ID",
@@ -100,7 +89,7 @@ func (s *CometBFTServer[T]) ShowNodeIDCmd() *cobra.Command {
 }
 
 // ShowValidatorCmd - ported from CometBFT, show this node's validator info
-func (s *CometBFTServer[T]) ShowValidatorCmd() *cobra.Command {
+func ShowValidatorCmd() *cobra.Command {
 	cmd := cobra.Command{
 		Use:   "show-validator",
 		Short: "Show this node's CometBFT validator info",
@@ -134,7 +123,7 @@ func (s *CometBFTServer[T]) ShowValidatorCmd() *cobra.Command {
 }
 
 // ShowAddressCmd - show this node's validator address
-func (s *CometBFTServer[T]) ShowAddressCmd() *cobra.Command {
+func ShowAddressCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "show-address",
 		Short: "Shows this node's CometBFT validator consensus address",
@@ -153,7 +142,7 @@ func (s *CometBFTServer[T]) ShowAddressCmd() *cobra.Command {
 }
 
 // VersionCmd prints CometBFT and ABCI version numbers.
-func (s *CometBFTServer[T]) VersionCmd() *cobra.Command {
+func VersionCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "version",
 		Short: "Print CometBFT libraries' version",
@@ -181,7 +170,7 @@ func (s *CometBFTServer[T]) VersionCmd() *cobra.Command {
 }
 
 // QueryBlocksCmd returns a command to search through blocks by events.
-func (s *CometBFTServer[T]) QueryBlocksCmd() *cobra.Command {
+func QueryBlocksCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "blocks",
 		Short: "Query for paginated blocks that match a set of events",
@@ -196,7 +185,7 @@ for. Each module documents its respective events under 'xx_events.md'.
 			version.AppName,
 		),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			rpcclient, err := s.rpcClient(cmd)
+			rpcclient, err := rpcClient(cmd)
 			if err != nil {
 				return err
 			}
@@ -231,7 +220,7 @@ for. Each module documents its respective events under 'xx_events.md'.
 }
 
 // QueryBlockCmd implements the default command for a Block query.
-func (s *CometBFTServer[T]) QueryBlockCmd() *cobra.Command {
+func QueryBlockCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "block --type={height|hash} <height|hash>",
 		Short: "Query for a committed block by height, hash, or event(s)",
@@ -246,7 +235,8 @@ $ %s query block --%s=%s <hash>
 		RunE: func(cmd *cobra.Command, args []string) error {
 			typ, _ := cmd.Flags().GetString(FlagType)
 
-			rpcclient, err := s.rpcClient(cmd)
+			rpcclient, err := rpcClient(cmd)
+			fmt.Println("rpcclient", rpcclient, err)
 			if err != nil {
 				return err
 			}
@@ -318,14 +308,14 @@ $ %s query block --%s=%s <hash>
 }
 
 // QueryBlockResultsCmd implements the default command for a BlockResults query.
-func (s *CometBFTServer[T]) QueryBlockResultsCmd() *cobra.Command {
+func QueryBlockResultsCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "block-results [height]",
 		Short: "Query for a committed block's results by height",
 		Long:  "Query for a specific committed block's results using the CometBFT RPC `block_results` method",
 		Args:  cobra.RangeArgs(0, 1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			node, err := s.rpcClient(cmd)
+			node, err := rpcClient(cmd)
 			if err != nil {
 				return err
 			}

server/v2/cometbft/config.go

@@ -64,3 +64,4 @@ func getConfigTomlFromViper(v *viper.Viper) *cmtcfg.Config {
 
 	return conf.SetRoot(rootDir)
 }
+

server/v2/cometbft/server.go

@@ -233,18 +233,19 @@ func (s *CometBFTServer[T]) StartCmdFlags() *pflag.FlagSet {
 func (s *CometBFTServer[T]) CLICommands() serverv2.CLIConfig {
 	return serverv2.CLIConfig{
 		Commands: []*cobra.Command{
-			s.StatusCommand(),
-			s.ShowNodeIDCmd(),
-			s.ShowValidatorCmd(),
-			s.ShowAddressCmd(),
-			s.VersionCmd(),
+			StatusCommand(),
+			ShowNodeIDCmd(),
+			ShowValidatorCmd(),
+			ShowAddressCmd(),
+			VersionCmd(),
+			s.BootstrapStateCmd(),
 			cmtcmd.ResetAllCmd,
 			cmtcmd.ResetStateCmd,
 		},
 		Queries: []*cobra.Command{
-			s.QueryBlockCmd(),
-			s.QueryBlocksCmd(),
-			s.QueryBlockResultsCmd(),
+			QueryBlockCmd(),
+			QueryBlocksCmd(),
+			QueryBlockResultsCmd(),
 		},
 	}
 }

simapp/CHANGELOG.md

@@ -24,7 +24,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 # Changelog
 
 `SimApp` is an application built using the Cosmos SDK for testing and educational purposes.
-It won't be tagged or intented to be imported in an application.
+It won't be tagged or intended to be imported in an application.
 This changelog is aimed to help developers understand the wiring changes between SDK versions.
 It is an exautive list of changes that completes the SimApp section in the [UPGRADING.md](https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md#simapp)
 

simapp/app.go

@@ -516,7 +516,6 @@ func NewSimApp(
 		group.ModuleName,
 		upgradetypes.ModuleName,
 		vestingtypes.ModuleName,
-		consensustypes.ModuleName,
 		circuittypes.ModuleName,
 		pooltypes.ModuleName,
 		epochstypes.ModuleName,

simapp/app_config.go

@@ -284,7 +284,7 @@ var (
 				Name:   epochstypes.ModuleName,
 				Config: appconfig.WrapAny(&epochsmodulev1.Module{}),
 			},
-			// This module is used for testing the depinject gogo x pulsar module registration.
+			// This module is only used for testing the depinject gogo x pulsar module registration.
 			{
 				Name:   countertypes.ModuleName,
 				Config: appconfig.WrapAny(&countertypes.Module{}),

simapp/app_di.go

@@ -288,7 +288,7 @@ func NewSimApp(
 	return app
 }
 
-// overwrite default ante handlers with custom ante handlers
+// setCustomAnteHandler overwrites default ante handlers with custom ante handlers
 // set SkipAnteHandler to true in app config and set custom ante handler on baseapp
 func (app *SimApp) setCustomAnteHandler() {
 	anteHandler, err := NewAnteHandler(

simapp/export.go

@@ -3,7 +3,6 @@ package simapp
 import (
 	"encoding/json"
 	"fmt"
-	"log"
 
 	cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
 
@@ -50,7 +49,7 @@ func (app *SimApp) ExportAppStateAndValidators(forZeroHeight bool, jailAllowedAd
 	}, err
 }
 
-// prepare for fresh start at zero height
+// prepForZeroHeightGenesis prepares for fresh start at zero height
 // NOTE zero height genesis is a temporary feature which will be deprecated
 //
 //	in favor of export at a block height
@@ -67,7 +66,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
 	for _, addr := range jailAllowedAddrs {
 		_, err := sdk.ValAddressFromBech32(addr)
 		if err != nil {
-			log.Fatal(err)
+			panic(err)
 		}
 		allowedAddrsMap[addr] = true
 	}
@@ -94,11 +93,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
 	}
 
 	for _, delegation := range dels {
-		valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
-		if err != nil {
-			panic(err)
-		}
-
+		valAddr:= sdk.MustValAddressFromBech32(delegation.ValidatorAddress)
 		delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress)
 
 		_, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr)
@@ -151,10 +146,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
 
 	// reinitialize all delegations
 	for _, del := range dels {
-		valAddr, err := sdk.ValAddressFromBech32(del.ValidatorAddress)
-		if err != nil {
-			panic(err)
-		}
+		valAddr := sdk.MustValAddressFromBech32(del.ValidatorAddress)
 		delAddr := sdk.MustAccAddressFromBech32(del.DelegatorAddress)
 
 		if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil {
@@ -192,7 +184,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
 	err = app.StakingKeeper.UnbondingDelegations.Walk(
 		ctx,
 		nil,
-		func(key collections.Pair[[]byte, []byte], ubd stakingtypes.UnbondingDelegation) (stop bool, err error) {
+		func(_ collections.Pair[[]byte, []byte], ubd stakingtypes.UnbondingDelegation) (stop bool, err error) {
 			for i := range ubd.Entries {
 				ubd.Entries[i].CreationHeight = 0
 			}
@@ -237,7 +229,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
 
 	_, err = app.StakingKeeper.ApplyAndReturnValidatorSetUpdates(ctx)
 	if err != nil {
-		log.Fatal(err)
+		panic(err)
 	}
 
 	/* Handle slashing state. */

simapp/genesis_account.go

@@ -31,7 +31,7 @@ type SimGenesisAccount struct {
 func (sga SimGenesisAccount) Validate() error {
 	if !sga.OriginalVesting.IsZero() {
 		if sga.StartTime >= sga.EndTime {
-			return errors.New("vesting start-time cannot be before end-time")
+			return errors.New("vesting start-time cannot be after end-time")
 		}
 	}
 

simapp/simd/cmd/commands.go

@@ -146,17 +146,16 @@ func appExport(
 
 	// overwrite the FlagInvCheckPeriod
 	viperAppOpts.Set(server.FlagInvCheckPeriod, 1)
-	appOpts = viperAppOpts
 
 	var simApp *simapp.SimApp
 	if height != -1 {
-		simApp = simapp.NewSimApp(logger, db, traceStore, false, appOpts)
+		simApp = simapp.NewSimApp(logger, db, traceStore, false, viperAppOpts)
 
 		if err := simApp.LoadHeight(height); err != nil {
 			return servertypes.ExportedApp{}, err
 		}
 	} else {
-		simApp = simapp.NewSimApp(logger, db, traceStore, true, appOpts)
+		simApp = simapp.NewSimApp(logger, db, traceStore, true, viperAppOpts)
 	}
 
 	return simApp.ExportAppStateAndValidators(forZeroHeight, jailAllowedAddrs, modulesToExport)

simapp/simd/cmd/config.go

@@ -15,7 +15,7 @@ import (
 func initCometBFTConfig() *cmtcfg.Config {
 	cfg := cmtcfg.DefaultConfig()
 
-	// only display only error logs by default except for p2p and state
+	// display only error logs by default except for p2p and state
 	cfg.LogLevel = "*:error,p2p:info,state:info"
 
 	// these values put a higher strain on node memory

simapp/simd/cmd/testnet.go

@@ -544,11 +544,7 @@ func writeFile(name, dir string, contents []byte) error {
 		return fmt.Errorf("could not create directory %q: %w", dir, err)
 	}
 
-	if err := os.WriteFile(file, contents, 0o600); err != nil {
-		return err
-	}
-
-	return nil
+	return os.WriteFile(file, contents, 0o600)
 }
 
 // startTestnet starts an in-process testnet

simapp/v2/app_config.go

@@ -15,6 +15,7 @@ import (
 	circuitmodulev1 "cosmossdk.io/api/cosmos/circuit/module/v1"
 	consensusmodulev1 "cosmossdk.io/api/cosmos/consensus/module/v1"
 	distrmodulev1 "cosmossdk.io/api/cosmos/distribution/module/v1"
+	epochsmodulev1 "cosmossdk.io/api/cosmos/epochs/module/v1"
 	evidencemodulev1 "cosmossdk.io/api/cosmos/evidence/module/v1"
 	feegrantmodulev1 "cosmossdk.io/api/cosmos/feegrant/module/v1"
 	genutilmodulev1 "cosmossdk.io/api/cosmos/genutil/module/v1"
@@ -45,6 +46,8 @@ import (
 	consensustypes "cosmossdk.io/x/consensus/types"
 	_ "cosmossdk.io/x/distribution" // import for side-effects
 	distrtypes "cosmossdk.io/x/distribution/types"
+	_ "cosmossdk.io/x/epochs" // import for side-effects
+	epochstypes "cosmossdk.io/x/epochs/types"
 	_ "cosmossdk.io/x/evidence" // import for side-effects
 	evidencetypes "cosmossdk.io/x/evidence/types"
 	"cosmossdk.io/x/feegrant"
@@ -121,6 +124,7 @@ var (
 						evidencetypes.ModuleName,
 						stakingtypes.ModuleName,
 						authz.ModuleName,
+						epochstypes.ModuleName,
 					},
 					EndBlockers: []string{
 						govtypes.ModuleName,
@@ -158,6 +162,7 @@ var (
 						vestingtypes.ModuleName,
 						circuittypes.ModuleName,
 						pooltypes.ModuleName,
+						epochstypes.ModuleName,
 					},
 					// When ExportGenesis is not specified, the export genesis module order
 					// is equal to the init genesis order
@@ -270,6 +275,10 @@ var (
 				Name:   pooltypes.ModuleName,
 				Config: appconfig.WrapAny(&poolmodulev1.Module{}),
 			},
+			{
+				Name:   epochstypes.ModuleName,
+				Config: appconfig.WrapAny(&epochsmodulev1.Module{}),
+			},
 		},
 	})
 )