-
Notifications
You must be signed in to change notification settings - Fork 170
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
simulation: vanilla simulation tests #71
Conversation
@SebastianElvis thank you for the detailed description and the log output, amazing job! I see this:
which points here. It looks like Or are you saying this doesn't work because the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic! It would be great if @vitsalis could have a look at what exactly is happening during the Genesis setup of the BTC light client to cause a panic, but this is super useful!
simapp/utils.go
Outdated
// NOTE: this function is identical to https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/simapp/utils.go. The reason of migrating it here is that it uses the modules' flags initialised in `init()`. Otherwise, if using `sdksimapp.SetupSimulation`, then `sdksimapp.FlagEnabledValue` and `sdksimapp.FlagVerboseValue` will be used, rather than those in `config.go` of our module. | ||
// The other functions under https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/simapp/utils.go do not access flags and thus can be reused safely. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// NOTE: this function is identical to https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/simapp/utils.go. The reason of migrating it here is that it uses the modules' flags initialised in `init()`. Otherwise, if using `sdksimapp.SetupSimulation`, then `sdksimapp.FlagEnabledValue` and `sdksimapp.FlagVerboseValue` will be used, rather than those in `config.go` of our module. | |
// The other functions under https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/simapp/utils.go do not access flags and thus can be reused safely. | |
// NOTE: this function is identical to https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/simapp/utils.go. | |
// The reason of migrating it here is that it uses the modules' flags initialised in `init()`. Otherwise, | |
// if using `sdksimapp.SetupSimulation`, then `sdksimapp.FlagEnabledValue` and `sdksimapp.FlagVerboseValue` | |
// will be used, rather than those in `config.go` of our module. The other functions under | |
// https://github.com/cosmos/cosmos-sdk/blob/v0.45.5/simapp/utils.go do not access flags and thus can be reused safely. |
@SebastianElvis I took a look at the error and turns out that genesis was not setup properly for the
|
Thanks for the fix!
This issue is because Babylon rejects the existing message types in the staking module and instead requires their wrapped versions. Will investigate and fix (either in this PR or the subsequent ones depending on the complexity and scope). Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, other than the error that is generated when running the sim commands. Nice catches with the sdksimapp
structure!
I have fixed the failure caused by unwrapped message types by ruling out simulated messages in the staking module. In addition, I disabled changing the epoch interval on-the-fly in the simulation since this causes errors in the simulation but won't happen in MVP. Now the simulation can run, but still not successfully. I believe the issue is out of this PR's scope -- it will be gone after all modules implement the simulation support. Sample log is attached. |
Partially fixes BM-79
This PR implements the support of all simulation tests specified in
Makefile
. Specifically, one can usemake test-sim-<sim_test_name>
to run a simulation test, which bootstraps a Babylon SimApp with random parameters and genesis files, triggers certain random operations, and verifies certain correctness properties. Note thatmake test
and thus the current CI program does not trigger any simulation test.The current
Makefile
provides the following simulation tests:make test-sim-import-export
make test-sim-after-import
make test-sim-multi-seed-long
make test-sim-multi-seed-short
make test-sim-custom-genesis-multi-seed
make test-sim-profile
make test-sim-custom-genesis-fast
make test-sim-nondeterminism
make test-sim-benchmark
make test-sim-benchmark-invariants
At the moment, none of them is successful, since we haven't finished the implementation yet. Some sample simulation tests are attached below. This does not mean to be a blocker for our current work -- the integration test phase will happen after we finish implementing all the functionalities. It would be useful for us to find bugs early though.
I also marked @vitsalis as reviewer since Vitalis did the initial migration of
/simapp
so may have experience on debugging it. Thanks in advance!Results of
make test-sim-import-export
Results of
make test-sim-benchmark