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 7 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 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 @@ -320,7 +320,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
28 changes: 13 additions & 15 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 @@ -80,7 +79,10 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
if err != nil {
panic(err)
}
_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, valBz)
_, err = app.DistrKeeper.WithdrawValidatorCommission(ctx, valBz)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should check the error, it fails tests when a validator has no commission.
Could you revert?

if err != nil {
panic(err)
}
return false
})
if err != nil {
Expand All @@ -94,14 +96,13 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
}

for _, delegation := range dels {
valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
valAddr:= sdk.MustValAddressFromBech32(delegation.ValidatorAddress)
delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress)

_, err = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr)
if err != nil {
panic(err)
}

delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress)

_, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr)
}

// clear validator slash events
Expand Down Expand Up @@ -151,10 +152,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 +190,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 +235,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{}),
},
},
})
)
4 changes: 3 additions & 1 deletion simapp/v2/app_di.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -69,6 +70,7 @@ type SimApp[T transaction.Tx] struct {
ConsensusParamsKeeper consensuskeeper.Keeper
CircuitBreakerKeeper circuitkeeper.Keeper
PoolKeeper poolkeeper.Keeper
EpochsKeeper *epochskeeper.Keeper
}

func init() {
Expand Down Expand Up @@ -177,6 +179,7 @@ func NewSimApp[T transaction.Tx](
&app.ConsensusParamsKeeper,
&app.CircuitBreakerKeeper,
&app.PoolKeeper,
&app.EpochsKeeper,
); err != nil {
panic(err)
}
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion simapp/v2/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
89 changes: 89 additions & 0 deletions simapp/v2/genesis_account_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
3 changes: 2 additions & 1 deletion simapp/v2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ require (
cosmossdk.io/x/circuit v0.0.0-20230613133644-0a778132a60f
cosmossdk.io/x/consensus v0.0.0-00010101000000-000000000000
cosmossdk.io/x/distribution v0.0.0-20230925135524-a1bc045b3190
cosmossdk.io/x/epochs v0.0.0-20240522060652-a1ae4c3e0337
cosmossdk.io/x/evidence v0.0.0-20230613133644-0a778132a60f
cosmossdk.io/x/feegrant v0.0.0-20230613133644-0a778132a60f
cosmossdk.io/x/gov v0.0.0-20231113122742-912390d5fc4a
Expand Down Expand Up @@ -61,7 +62,6 @@ require (
cosmossdk.io/store v1.1.1-0.20240418092142-896cdf1971bc // indirect
cosmossdk.io/x/accounts/defaults/lockup v0.0.0-20240417181816-5e7aae0db1f5 // indirect
cosmossdk.io/x/accounts/defaults/multisig v0.0.0-00010101000000-000000000000 // indirect
cosmossdk.io/x/epochs v0.0.0-20240522060652-a1ae4c3e0337 // indirect
cosmossdk.io/x/tx v0.13.4 // indirect
filippo.io/edwards25519 v1.1.0 // indirect
github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
Expand Down Expand Up @@ -260,6 +260,7 @@ replace (
cosmossdk.io/x/circuit => ../../x/circuit
cosmossdk.io/x/consensus => ../../x/consensus
cosmossdk.io/x/distribution => ../../x/distribution
cosmossdk.io/x/epochs => ../../x/epochs
cosmossdk.io/x/evidence => ../../x/evidence
cosmossdk.io/x/feegrant => ../../x/feegrant
cosmossdk.io/x/gov => ../../x/gov
Expand Down
2 changes: 0 additions & 2 deletions simapp/v2/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ cosmossdk.io/math v1.3.0 h1:RC+jryuKeytIiictDslBP9i1fhkVm6ZDmZEoNP316zE=
cosmossdk.io/math v1.3.0/go.mod h1:vnRTxewy+M7BtXBNFybkuhSH4WfedVAAnERHgVFhp3k=
cosmossdk.io/schema v0.1.1 h1:I0M6pgI7R10nq+/HCQfbO6BsGBZA8sQy+duR1Y3aKcA=
cosmossdk.io/schema v0.1.1/go.mod h1:RDAhxIeNB4bYqAlF4NBJwRrgtnciMcyyg0DOKnhNZQQ=
cosmossdk.io/x/epochs v0.0.0-20240522060652-a1ae4c3e0337 h1:GuBrfHsK3RD5vlD4DuBz3DXslR6VlnzrYmHOC3L679Q=
cosmossdk.io/x/epochs v0.0.0-20240522060652-a1ae4c3e0337/go.mod h1:PhLn1pMBilyRC4GfRkoYhm+XVAYhF4adVrzut8AdpJI=
dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
Expand Down
3 changes: 0 additions & 3 deletions simapp/v2/simdv2/cmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,8 @@ func queryCommand() *cobra.Command {

cmd.AddCommand(
rpc.QueryEventForTxCmd(),
server.QueryBlockCmd(),
authcmd.QueryTxsByEventsCmd(),
server.QueryBlocksCmd(),
authcmd.QueryTxCmd(),
server.QueryBlockResultsCmd(),
)

return cmd
Expand Down
Loading
Loading