Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(simapp): Audit simapp #21311

Merged
merged 23 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update a little bit CI and server/v2/cometbft since we using cometbft queries

cnt=$((cnt + 1))
if [ $cnt -gt 30 ]; then
kill -9 "$SIMD_PID"
Expand Down
54 changes: 30 additions & 24 deletions server/v2/cometbft/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ 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"
"sigs.k8s.io/yaml"

serverv2 "cosmossdk.io/server/v2"
"cosmossdk.io/server/v2/cometbft/client/rpc"

"github.com/cosmos/cosmos-sdk/client"
Expand All @@ -28,35 +28,40 @@ import (
"github.com/cosmos/cosmos-sdk/version"
)

func (s *CometBFTServer[T]) rpcClient(cmd *cobra.Command) (rpc.CometRPC, error) {
if s.config.AppTomlConfig.Standalone {
func rpcClient(cmd *cobra.Command) (rpc.CometRPC, error) {
v := client.GetViperFromCmd(cmd)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when doing queries it shouldn't need to read the app.toml actually right, but the client.toml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only for Standalone check, should we remove? I think FlagNode is enough

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah flag node will get if from client.toml, so that's great. Not sure what a standalone check means for query tbh.

appTomlConfig := AppTomlConfig{}
if v != nil {
if err := serverv2.UnmarshalSubConfig(v, ServerName, &appTomlConfig); err != nil {
return nil, fmt.Errorf("failed to unmarshal config: %w", err)
}
}
if appTomlConfig.Standalone {
client, err := rpchttp.New(client.GetConfigFromCmd(cmd).RPC.ListenAddress)
if err != nil {
return nil, err
}
return client, nil
}

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)
}
rpcURI, err := cmd.Flags().GetString(FlagNode)
if err != nil {
return nil, err
}
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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comet queries should not relayed on CometBFTServer anymore.

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
}
Expand All @@ -82,7 +87,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",
Expand All @@ -100,7 +105,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",
Expand Down Expand Up @@ -134,7 +139,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",
Expand All @@ -153,7 +158,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",
Expand Down Expand Up @@ -181,7 +186,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",
Expand All @@ -196,7 +201,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
}
Expand Down Expand Up @@ -231,7 +236,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)",
Expand All @@ -246,7 +251,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
}
Expand Down Expand Up @@ -318,14 +324,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
}
Expand Down
1 change: 1 addition & 0 deletions server/v2/cometbft/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,4 @@ func getConfigTomlFromViper(v *viper.Viper) *cmtcfg.Config {

return conf.SetRoot(rootDir)
}

17 changes: 9 additions & 8 deletions server/v2/cometbft/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion simapp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 0 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,6 @@ func NewSimApp(
group.ModuleName,
upgradetypes.ModuleName,
vestingtypes.ModuleName,
consensustypes.ModuleName,
circuittypes.ModuleName,
pooltypes.ModuleName,
epochstypes.ModuleName,
Expand Down
2 changes: 1 addition & 1 deletion simapp/app_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}),
Expand Down
2 changes: 1 addition & 1 deletion simapp/app_di.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
20 changes: 6 additions & 14 deletions simapp/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package simapp
import (
"encoding/json"
"fmt"
"log"

cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"

Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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. */
Expand Down
2 changes: 1 addition & 1 deletion simapp/genesis_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
5 changes: 2 additions & 3 deletions simapp/simd/cmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion simapp/simd/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 1 addition & 5 deletions simapp/simd/cmd/testnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions simapp/v2/app_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -121,6 +124,7 @@ var (
evidencetypes.ModuleName,
stakingtypes.ModuleName,
authz.ModuleName,
epochstypes.ModuleName,
},
EndBlockers: []string{
govtypes.ModuleName,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -270,6 +275,10 @@ var (
Name: pooltypes.ModuleName,
Config: appconfig.WrapAny(&poolmodulev1.Module{}),
},
{
Name: epochstypes.ModuleName,
Config: appconfig.WrapAny(&epochsmodulev1.Module{}),
},
},
})
)
Loading
Loading