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.

Sign up for GitHub

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

Jump to bottom

feat: add custom max gas for block for sim config #16656

Merged
merged 10 commits into from
Jun 27, 2023
Merged

feat: add custom max gas for block for sim config #16656

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
98d8a19
avoid invalid consensus params when set unlimited max gas for sim test
mmsqe Jun 21, 2023
25790ad
Revert "avoid invalid consensus params when set unlimited max gas for…
mmsqe Jun 22, 2023
00b13c5
add custom BlockMaxGas for sim test
mmsqe Jun 22, 2023
741e481
add change doc
mmsqe Jun 22, 2023
d9337b4
fix config
mmsqe Jun 22, 2023
86b7150
use 0 as -1
mmsqe Jun 23, 2023
e9edb5b
Merge remote-tracking branch 'origin/main' into fix_sim
mmsqe Jun 26, 2023
80c1415
Apply suggestions from code review
mmsqe Jun 26, 2023
63c3b6c
Merge remote-tracking branch 'origin/main' into fix_sim
mmsqe Jun 27, 2023
edd55bd
update change doc
mmsqe Jun 27, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (cli) [#16209](https://github.com/cosmos/cosmos-sdk/pull/16209) Make `StartCmd` more customizable.
* (types) [#16257](https://github.com/cosmos/cosmos-sdk/pull/16257) Allow setting the base denom in the denom registry.
* (genutil) [#16046](https://github.com/cosmos/cosmos-sdk/pull/16046) Add "module-name" flag to genutil add-genesis-account to enable intializing module accounts at genesis.
* (sims) [#16656](https://github.com/cosmos/cosmos-sdk/pull/16656) Add custom max gas for block for sim config with unlimited as default.
Copy link
Member

Choose a reason for hiding this comment

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

nit, can you place the changelog under unreleased?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, is it possible that we backport to v0.46.x as well


### Improvements

Expand Down
3 changes: 2 additions & 1 deletion types/simulation/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ type Config struct {
OnOperation bool // run slow invariants every operation
AllInvariants bool // print all failed invariants if a broken invariant is found

DBBackend string // custom db backend type
DBBackend string // custom db backend type
BlockMaxGas *int64 // custom max gas for block
}
5 changes: 2 additions & 3 deletions x/simulation/params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (w WeightedProposalContent) ContentSimulatorFn() simulation.ContentSimulato
// Consensus Params

// randomConsensusParams returns random simulation consensus parameters, it extracts the Evidence from the Staking genesis state.
func randomConsensusParams(r *rand.Rand, appState json.RawMessage, cdc codec.JSONCodec) *cmtproto.ConsensusParams {
func randomConsensusParams(r *rand.Rand, appState json.RawMessage, cdc codec.JSONCodec, maxGas int64) *cmtproto.ConsensusParams {
var genesisState map[string]json.RawMessage
err := json.Unmarshal(appState, &genesisState)
if err != nil {
Expand All @@ -187,7 +187,7 @@ func randomConsensusParams(r *rand.Rand, appState json.RawMessage, cdc codec.JSO
consensusParams := &cmtproto.ConsensusParams{
Block: &cmtproto.BlockParams{
MaxBytes: int64(simulation.RandIntBetween(r, 20000000, 30000000)),
MaxGas: -1,
MaxGas: maxGas,
},
Validator: &cmtproto.ValidatorParams{
PubKeyTypes: []string{types.ABCIPubKeyTypeEd25519},
Expand All @@ -197,7 +197,6 @@ func randomConsensusParams(r *rand.Rand, appState json.RawMessage, cdc codec.JSO
MaxAgeDuration: stakingGenesisState.Params.UnbondingTime,
},
}

odeke-em marked this conversation as resolved.
Show resolved Hide resolved
bz, err := json.MarshalIndent(&consensusParams, "", " ")
if err != nil {
panic(err)
Expand Down
6 changes: 5 additions & 1 deletion x/simulation/simulate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ func initChain(
config simulation.Config,
cdc codec.JSONCodec,
) (mockValidators, time.Time, []simulation.Account, string) {
blockMaxGas := int64(-1)
if config.BlockMaxGas != nil {
blockMaxGas = *config.BlockMaxGas
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The zero value of int64 is 0, but 0 is a valid value for max gas. If we don't use a pointer, we cannot maintain a default behaviour of maxGas with value -1

Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't the default be -1 w/o using a pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, but we can't set maxGas as 0 unless pointer

appState, accounts, chainID, genesisTimestamp := appStateFn(r, accounts, config)
consensusParams := randomConsensusParams(r, appState, cdc)
consensusParams := randomConsensusParams(r, appState, cdc, blockMaxGas)
req := abci.RequestInitChain{
AppStateBytes: appState,
ChainId: chainID,
Expand Down