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

fix(simulation): Fix all problems make test-sim-custom-genesis-fast for simulation test. #17911

Merged
merged 9 commits into from
Oct 20, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Bug Fixes

* (x/gov) [#17873](https://github.com/cosmos/cosmos-sdk/pull/17873) Fail any inactive and active proposals whose messages cannot be decoded.
* (simulation) [#17911](https://github.com/cosmos/cosmos-sdk/pull/17911) Fix all problems with executing command `make test-sim-custom-genesis-fast` for simulation test.

### API Breaking Changes

Expand Down
10 changes: 5 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ test-sim-nondeterminism-streaming:

test-sim-custom-genesis-fast:
@echo "Running custom genesis simulation..."
@echo "By default, ${HOME}/.gaiad/config/genesis.json will be used."
@cd ${CURRENT_DIR}/simapp && go test -mod=readonly -run TestFullAppSimulation -Genesis=${HOME}/.gaiad/config/genesis.json \
-Enabled=true -NumBlocks=100 -BlockSize=200 -Commit=true -Seed=99 -Period=5 -v -timeout 24h
@echo "By default, ${HOME}/.simapp/config/genesis.json will be used."
@cd ${CURRENT_DIR}/simapp && go test -mod=readonly -run TestFullAppSimulation -Genesis=${HOME}/.simapp/config/genesis.json \
-Enabled=true -NumBlocks=100 -BlockSize=200 -Commit=true -Seed=99 -Period=5 -SigverifyTx=false -v -timeout 24h

test-sim-import-export: runsim
@echo "Running application import/export simulation. This may take several minutes..."
Expand All @@ -284,8 +284,8 @@ test-sim-after-import: runsim

test-sim-custom-genesis-multi-seed: runsim
@echo "Running multi-seed custom genesis simulation..."
@echo "By default, ${HOME}/.gaiad/config/genesis.json will be used."
@cd ${CURRENT_DIR}/simapp && $(BINDIR)/runsim -Genesis=${HOME}/.gaiad/config/genesis.json -SimAppPkg=. -ExitOnFail 400 5 TestFullAppSimulation
@echo "By default, ${HOME}/.simapp/config/genesis.json will be used."
@cd ${CURRENT_DIR}/simapp && $(BINDIR)/runsim -Genesis=${HOME}/.simapp/config/genesis.json -SigverifyTx=false -SimAppPkg=. -ExitOnFail 400 5 TestFullAppSimulation

test-sim-multi-seed-long: runsim
@echo "Running long multi-seed application simulation. This may take awhile!"
Expand Down
4 changes: 4 additions & 0 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type BaseApp struct {
addrPeerFilter sdk.PeerFilter // filter peers by address and port
idPeerFilter sdk.PeerFilter // filter peers by node ID
fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed.
sigverifyTx bool // whether to sigverify check for transaction
Copy link
Member

Choose a reason for hiding this comment

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

can we precise here, as in context what it is used for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


// manages snapshots, i.e. dumps of app state at certain intervals
snapshotManager *snapshots.Manager
Expand Down Expand Up @@ -198,6 +199,7 @@ func NewBaseApp(
msgServiceRouter: NewMsgServiceRouter(),
txDecoder: txDecoder,
fauxMerkleMode: false,
sigverifyTx: true,
queryGasLimit: math.MaxUint64,
}

Expand Down Expand Up @@ -642,6 +644,8 @@ func (app *BaseApp) getContextForTx(mode execMode, txBytes []byte) sdk.Context {
WithTxBytes(txBytes)
// WithVoteInfos(app.voteInfos) // TODO: identify if this is needed

ctx = ctx.WithIsSigverifyTx(app.sigverifyTx)

ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx))

if mode == execModeReCheck {
Expand Down
5 changes: 5 additions & 0 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ func (app *BaseApp) SetFauxMerkleMode() {
app.fauxMerkleMode = true
}

// SetNotSigverify during simulation testing, transaction signature verification needs to be ignored.
func (app *BaseApp) SetNotSigverifyTx() {
app.sigverifyTx = false
}

// SetCommitMultiStoreTracer sets the store tracer on the BaseApp's underlying
// CommitMultiStore.
func (app *BaseApp) SetCommitMultiStoreTracer(w io.Writer) {
Expand Down
4 changes: 2 additions & 2 deletions baseapp/test_helpers.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package baseapp

import (
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"

errorsmod "cosmossdk.io/errors"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
Copy link
Member

Choose a reason for hiding this comment

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

please rn make lint-fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -35,6 +34,7 @@ func (app *BaseApp) SimDeliver(txEncoder sdk.TxEncoder, tx sdk.Tx) (sdk.GasInfo,
if err != nil {
return sdk.GasInfo{}, nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "%s", err)
}

gasInfo, result, _, err := app.runTx(execModeFinalize, bz)
return gasInfo, result, err
}
Expand Down
12 changes: 12 additions & 0 deletions simapp/sim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ func TestFullAppSimulation(t *testing.T) {
appOptions[server.FlagInvCheckPeriod] = simcli.FlagPeriodValue

app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt, baseapp.SetChainID(SimAppChainID))
if !simcli.FlagSigverifyTxValue {
app.SetNotSigverifyTx()
}
require.Equal(t, "SimApp", app.Name())

// run randomized simulation
Expand Down Expand Up @@ -121,6 +124,9 @@ func TestAppImportExport(t *testing.T) {
appOptions[server.FlagInvCheckPeriod] = simcli.FlagPeriodValue

app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt, baseapp.SetChainID(SimAppChainID))
if !simcli.FlagSigverifyTxValue {
app.SetNotSigverifyTx()
}
require.Equal(t, "SimApp", app.Name())

// Run randomized simulation
Expand Down Expand Up @@ -240,6 +246,9 @@ func TestAppSimulationAfterImport(t *testing.T) {
appOptions[server.FlagInvCheckPeriod] = simcli.FlagPeriodValue

app := NewSimApp(logger, db, nil, true, appOptions, fauxMerkleModeOpt, baseapp.SetChainID(SimAppChainID))
if !simcli.FlagSigverifyTxValue {
app.SetNotSigverifyTx()
}
require.Equal(t, "SimApp", app.Name())

// Run randomized simulation
Expand Down Expand Up @@ -362,6 +371,9 @@ func TestAppStateDeterminism(t *testing.T) {

db := dbm.NewMemDB()
app := NewSimApp(logger, db, nil, true, appOptions, interBlockCacheOpt(), baseapp.SetChainID(SimAppChainID))
if !simcli.FlagSigverifyTxValue {
app.SetNotSigverifyTx()
}

fmt.Printf(
"running non-determinism simulation; seed %d: %d/%d, attempt: %d/%d\n",
Expand Down
23 changes: 15 additions & 8 deletions testutil/sims/state_helpers.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package sims

import (
"bufio"
"encoding/json"
"fmt"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
"io"
"math/rand"
"os"
"path/filepath"
"time"

"github.com/cosmos/gogoproto/proto"
Expand Down Expand Up @@ -250,19 +253,23 @@ func AppStateRandomizedFn(
// AppStateFromGenesisFileFn util function to generate the genesis AppState
// from a genesis.json file.
func AppStateFromGenesisFileFn(r io.Reader, cdc codec.JSONCodec, genesisFile string) (genutiltypes.AppGenesis, []simtypes.Account, error) {
bytes, err := os.ReadFile(genesisFile)
file, err := os.Open(filepath.Clean(genesisFile))
if err != nil {
panic(err)
}

var genesis genutiltypes.AppGenesis
if err = json.Unmarshal(bytes, &genesis); err != nil {
return genesis, nil, err
genesis, err := genutiltypes.AppGenesisFromReader(bufio.NewReader(file))
if err != nil {
return *genesis, nil, err
}

if err := file.Close(); err != nil {
return *genesis, nil, err
}

var appState map[string]json.RawMessage
if err = json.Unmarshal(genesis.AppState, &appState); err != nil {
return genesis, nil, err
return *genesis, nil, err
}

var authGenesis authtypes.GenesisState
Expand All @@ -284,13 +291,13 @@ func AppStateFromGenesisFileFn(r io.Reader, cdc codec.JSONCodec, genesisFile str

a, ok := acc.GetCachedValue().(sdk.AccountI)
if !ok {
return genesis, nil, fmt.Errorf("expected account")
return *genesis, nil, fmt.Errorf("expected account")
}

// create simulator accounts
simAcc := simtypes.Account{PrivKey: privKey, PubKey: privKey.PubKey(), Address: a.GetAddress()}
simAcc := simtypes.Account{PrivKey: privKey, PubKey: privKey.PubKey(), Address: a.GetAddress(), ConsKey: ed25519.GenPrivKeyFromSecret(privkeySeed)}
newAccs[i] = simAcc
}

return genesis, newAccs, nil
return *genesis, newAccs, nil
}
9 changes: 9 additions & 0 deletions types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type Context struct {
blockGasMeter storetypes.GasMeter
checkTx bool
recheckTx bool // if recheckTx == true, then checkTx must also be true
sigverifyTx bool // when run simulation, because the private key corresponding to the account in the genesis.json randomly generated, we must skip the sigverify.
execMode ExecMode
minGasPrice DecCoins
consParams cmtproto.ConsensusParams
Expand Down Expand Up @@ -78,6 +79,7 @@ func (c Context) GasMeter() storetypes.GasMeter { return c.gasMe
func (c Context) BlockGasMeter() storetypes.GasMeter { return c.blockGasMeter }
func (c Context) IsCheckTx() bool { return c.checkTx }
func (c Context) IsReCheckTx() bool { return c.recheckTx }
func (c Context) IsSigverifyTx() bool { return c.sigverifyTx }
func (c Context) ExecMode() ExecMode { return c.execMode }
func (c Context) MinGasPrices() DecCoins { return c.minGasPrice }
func (c Context) EventManager() EventManagerI { return c.eventManager }
Expand Down Expand Up @@ -127,6 +129,7 @@ func NewContext(ms storetypes.MultiStore, isCheckTx bool, logger log.Logger) Con
header: h,
chainID: h.ChainID,
checkTx: isCheckTx,
sigverifyTx: true,
logger: logger,
gasMeter: storetypes.NewInfiniteGasMeter(),
minGasPrice: DecCoins{},
Expand Down Expand Up @@ -254,6 +257,12 @@ func (c Context) WithIsReCheckTx(isRecheckTx bool) Context {
return c
}

// WithIsSigverifyTx called with true will sigverify in auth module
func (c Context) WithIsSigverifyTx(isSigverifyTx bool) Context {
c.sigverifyTx = isSigverifyTx
return c
}

// WithExecMode returns a Context with an updated ExecMode.
func (c Context) WithExecMode(m ExecMode) Context {
c.execMode = m
Expand Down
4 changes: 2 additions & 2 deletions x/auth/ante/sigverify.go
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a test in sigverify_test to harden this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done! Please help me review the code again.

Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
pk = simSecp256k1Pubkey
}
// Only make check if simulate=false
if !simulate && !bytes.Equal(pk.Address(), signers[i]) {
if !simulate && !bytes.Equal(pk.Address(), signers[i]) && ctx.IsSigverifyTx() {
return ctx, errorsmod.Wrapf(sdkerrors.ErrInvalidPubKey,
"pubKey does not match signer address %s with signer index: %d", signerStrs[i], i)
}
Expand Down Expand Up @@ -302,7 +302,7 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul
}

// no need to verify signatures on recheck tx
if !simulate && !ctx.IsReCheckTx() {
if !simulate && !ctx.IsReCheckTx() && ctx.IsSigverifyTx() {
anyPk, _ := codectypes.NewAnyWithValue(pubKey)

signerData := txsigning.SignerData{
Expand Down
10 changes: 8 additions & 2 deletions x/group/simulation/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -1229,15 +1229,21 @@ func randomGroup(r *rand.Rand, k keeper.Keeper, ak group.AccountKeeper,

switch {
case groupID > initialGroupID:
// select a random ID between [initialGroupID, groupID]
groupID = uint64(simtypes.RandIntBetween(r, int(initialGroupID), int(groupID)))
// select a random ID between (initialGroupID, groupID]
// if there is at least one group information, then the groupID at this time must be greater than or equal to 1
groupID = uint64(simtypes.RandIntBetween(r, int(initialGroupID+1), int(groupID+1)))

default:
// This is called on the first call to this function
// in order to update the global variable
initialGroupID = groupID
}

// when groupID is 0, it proves that SimulateMsgCreateGroup has never been called. that is, no group exists in the chain
if groupID == 0 {
return nil, simtypes.Account{}, nil, nil
}

res, err := k.GroupInfo(ctx, &group.QueryGroupInfoRequest{GroupId: groupID})
if err != nil {
return nil, simtypes.Account{}, nil, err
Expand Down
2 changes: 2 additions & 0 deletions x/simulation/client/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
FlagVerboseValue bool
FlagPeriodValue uint
FlagGenesisTimeValue int64
FlagSigverifyTxValue bool
)

// GetSimulatorFlags gets the values of all the available simulation flags
Expand All @@ -56,6 +57,7 @@ func GetSimulatorFlags() {
flag.BoolVar(&FlagVerboseValue, "Verbose", false, "verbose log output")
flag.UintVar(&FlagPeriodValue, "Period", 0, "run slow invariants only once every period assertions")
flag.Int64Var(&FlagGenesisTimeValue, "GenesisTime", 0, "override genesis UNIX time instead of using a random UNIX time")
flag.BoolVar(&FlagSigverifyTxValue, "SigverifyTx", true, "whether to sigverify check for transaction ")
}

// NewConfigFromFlags creates a simulation from the retrieved values of the flags.
Expand Down
6 changes: 4 additions & 2 deletions x/simulation/simulate.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,10 @@ func SimulateFromSeed(
// Second variable to keep pending validator set (delayed one block since
// TM 0.24) Initially this is the same as the initial validator set
validators, blockTime, accs, chainID := initChain(r, params, accs, app, appStateFn, config, cdc)
if len(accs) == 0 {
return true, params, fmt.Errorf("must have greater than zero genesis accounts")
// At least 2 accounts must be added here, otherwise when executing SimulateMsgSend
// two accounts will be selected to meet the conditions from != to and it will fall into an infinite loop.
if len(accs) <= 1 {
return true, params, fmt.Errorf("at least two genesis accounts are required")
}

config.ChainID = chainID
Expand Down
Loading