refactor(simapp): Audit simapp

.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

@@ -29,7 +29,8 @@ import (
 )
 
 func (s *CometBFTServer[T]) rpcClient(cmd *cobra.Command) (rpc.CometRPC, error) {
-	if s.config.AppTomlConfig.Standalone {
+	appTomlConfig := getAppTomlFromViper(client.GetViperFromCmd(cmd))
+	if appTomlConfig.Standalone {
 		client, err := rpchttp.New(client.GetConfigFromCmd(cmd).RPC.ListenAddress)
 		if err != nil {
 			return nil, err

server/v2/cometbft/config.go

@@ -64,3 +64,12 @@ func getConfigTomlFromViper(v *viper.Viper) *cmtcfg.Config {
 
 	return conf.SetRoot(rootDir)
 }
+
+func getAppTomlFromViper(v *viper.Viper) *AppTomlConfig {
+	appTomlConfig := DefaultAppTomlConfig()
+	if err := serverv2.UnmarshalSubConfig(v, ServerName, &appTomlConfig); err != nil {
+		return DefaultAppTomlConfig()
+	}
+
+	return appTomlConfig
+}

server/v2/cometbft/server.go

@@ -58,15 +58,9 @@ func New[T transaction.Tx](txCodec transaction.Codec[T], serverOptions ServerOpt
 
 func (s *CometBFTServer[T]) Init(appI serverv2.AppI[T], v *viper.Viper, logger log.Logger) error {
 	// get configs (app.toml + config.toml) from viper
-	appTomlConfig := s.Config().(*AppTomlConfig)
-	if v != nil {
-		if err := serverv2.UnmarshalSubConfig(v, s.Name(), &appTomlConfig); err != nil {
-			return fmt.Errorf("failed to unmarshal config: %w", err)
-		}
-	}
 	s.config = Config{
 		ConfigTomlConfig: getConfigTomlFromViper(v),
-		AppTomlConfig:    appTomlConfig,
+		AppTomlConfig:    getAppTomlFromViper(v),
 	}
 
 	chainID := v.GetString(FlagChainID)

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

@@ -518,7 +518,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

@@ -287,7 +287,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{}),
+			},
 		},
 	})
 )

simapp/v2/app_di.go

@@ -19,6 +19,7 @@ import (
 	circuitkeeper "cosmossdk.io/x/circuit/keeper"
 	consensuskeeper "cosmossdk.io/x/consensus/keeper"
 	distrkeeper "cosmossdk.io/x/distribution/keeper"
+	epochskeeper "cosmossdk.io/x/epochs/keeper"
 	evidencekeeper "cosmossdk.io/x/evidence/keeper"
 	feegrantkeeper "cosmossdk.io/x/feegrant/keeper"
 	govkeeper "cosmossdk.io/x/gov/keeper"
@@ -69,6 +70,7 @@ type SimApp[T transaction.Tx] struct {
 	ConsensusParamsKeeper consensuskeeper.Keeper
 	CircuitBreakerKeeper  circuitkeeper.Keeper
 	PoolKeeper            poolkeeper.Keeper
+	EpochsKeeper          *epochskeeper.Keeper
 }
 
 func init() {
@@ -177,6 +179,7 @@ func NewSimApp[T transaction.Tx](
 		&app.ConsensusParamsKeeper,
 		&app.CircuitBreakerKeeper,
 		&app.PoolKeeper,
+		&app.EpochsKeeper,
 	); err != nil {
 		panic(err)
 	}
@@ -194,7 +197,6 @@ func NewSimApp[T transaction.Tx](
 
 	// TODO (here or in runtime/v2)
 	// wire simulation manager
-	// wire snapshot manager
 	// wire unordered tx manager
 
 	if err := app.LoadLatest(); err != nil {

simapp/v2/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/v2/genesis_account_test.go

@@ -0,0 +1,89 @@
+package simapp_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/cometbft/cometbft/crypto"
+	"github.com/stretchr/testify/require"
+
+	"cosmossdk.io/simapp/v2"
+	authtypes "cosmossdk.io/x/auth/types"
+
+	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
+func TestSimGenesisAccountValidate(t *testing.T) {
+	pubkey := secp256k1.GenPrivKey().PubKey()
+	addr := sdk.AccAddress(pubkey.Address())
+
+	vestingStart := time.Now().UTC()
+
+	coins := sdk.NewCoins(sdk.NewInt64Coin("test", 1000))
+	baseAcc := authtypes.NewBaseAccount(addr, pubkey, 0, 0)
+
+	testCases := []struct {
+		name    string
+		sga     simapp.SimGenesisAccount
+		wantErr bool
+	}{
+		{
+			"valid basic account",
+			simapp.SimGenesisAccount{
+				BaseAccount: baseAcc,
+			},
+			false,
+		},
+		{
+			"invalid basic account with mismatching address/pubkey",
+			simapp.SimGenesisAccount{
+				BaseAccount: authtypes.NewBaseAccount(addr, secp256k1.GenPrivKey().PubKey(), 0, 0),
+			},
+			true,
+		},
+		{
+			"valid basic account with module name",
+			simapp.SimGenesisAccount{
+				BaseAccount: authtypes.NewBaseAccount(sdk.AccAddress(crypto.AddressHash([]byte("testmod"))), nil, 0, 0),
+				ModuleName:  "testmod",
+			},
+			false,
+		},
+		{
+			"valid basic account with invalid module name/pubkey pair",
+			simapp.SimGenesisAccount{
+				BaseAccount: baseAcc,
+				ModuleName:  "testmod",
+			},
+			true,
+		},
+		{
+			"valid basic account with valid vesting attributes",
+			simapp.SimGenesisAccount{
+				BaseAccount:     baseAcc,
+				OriginalVesting: coins,
+				StartTime:       vestingStart.Unix(),
+				EndTime:         vestingStart.Add(1 * time.Hour).Unix(),
+			},
+			false,
+		},
+		{
+			"valid basic account with invalid vesting end time",
+			simapp.SimGenesisAccount{
+				BaseAccount:     baseAcc,
+				OriginalVesting: coins,
+				StartTime:       vestingStart.Add(2 * time.Hour).Unix(),
+				EndTime:         vestingStart.Add(1 * time.Hour).Unix(),
+			},
+			true,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.wantErr, tc.sga.Validate() != nil)
+		})
+	}
+}