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

Sim refactor 2: move GenesisState generators to modules #4838

Merged
merged 70 commits into from
Aug 28, 2019

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Aug 2, 2019

Closes: #4298

  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added a relevant changelog entry: clog add [section] [-t <tag>] [-m <msg>]

  • Re-reviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@fedekunze fedekunze added WIP Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. C:Simulations labels Aug 2, 2019
@fedekunze fedekunze changed the title move sims GenesisState generators to modules Sim refactor 2: move sims GenesisState generators to modules Aug 8, 2019
@fedekunze fedekunze changed the title Sim refactor 2: move sims GenesisState generators to modules Sim refactor 2: move GenesisState generators to modules Aug 8, 2019
Copy link
Contributor

@rigelrozanski rigelrozanski left a comment

Choose a reason for hiding this comment

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

New changes look good!

simapp/sim_test.go Outdated Show resolved Hide resolved
simapp/sim_test.go Outdated Show resolved Hide resolved
simapp/state.go Outdated Show resolved Hide resolved
simapp/state.go Outdated Show resolved Hide resolved
simapp/state.go Outdated Show resolved Hide resolved
types/module/simulation.go Outdated Show resolved Hide resolved
x/auth/simulation/genesis.go Outdated Show resolved Hide resolved
x/auth/simulation/genesis.go Outdated Show resolved Hide resolved
x/bank/genesis.go Show resolved Hide resolved
x/bank/simulation/genesis.go Outdated Show resolved Hide resolved
simapp/state.go Outdated Show resolved Hide resolved
types/module/simulation.go Outdated Show resolved Hide resolved
x/bank/simulation/genesis.go Show resolved Hide resolved
x/bank/simulation/params.go Outdated Show resolved Hide resolved
x/bank/simulation/params.go Outdated Show resolved Hide resolved
x/genaccounts/simulation/genesis.go Outdated Show resolved Hide resolved
x/mint/simulation/genesis.go Outdated Show resolved Hide resolved
x/mint/simulation/genesis.go Outdated Show resolved Hide resolved
x/mint/simulation/params.go Outdated Show resolved Hide resolved
x/supply/simulation/genesis.go Outdated Show resolved Hide resolved
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

ACK 🎉

@alexanderbez
Copy link
Contributor

Let's not merge until we figure out why test_sim_nondeterminism is 2x slower (~40m+)

@alexanderbez
Copy link
Contributor

@fedekunze Let's turn off invariants for test_sim_nondeterminism. They're checked in regular simulations anyway.

@fedekunze
Copy link
Collaborator Author

Interestingly enough, the invariants aren't run on test_sim_nondeterminism

@fedekunze
Copy link
Collaborator Author

@alexanderbez @rigelrozanski I'd vote for merging this as it and fix #4963 in a separate PR. cc: @tnachen

@alexanderbez
Copy link
Contributor

alexanderbez commented Aug 28, 2019

Interestingly enough, the invariants aren't run on test_sim_nondeterminism

Are you sure?

test_sim_nondeterminism:
	@echo "Running non-determinism test..."
	@go test -mod=readonly $(SIMAPP) -run TestAppStateDeterminism -Enabled=true \
	    -NumBlocks=100 -BlockSize=200 -Commit=true -v -timeout 24h
flag.IntVar(&flagPeriodValue, "Period", 1, "run slow invariants only once every period assertions")

Seems like the default is to run every block? Also, looks like this PR updates it to run every 5 blocks now.

@alexanderbez alexanderbez merged commit 45b25ce into master Aug 28, 2019
@alexanderbez alexanderbez deleted the fedekunze/cleanup-genState-generators branch August 28, 2019 14:59
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:Simulations Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Random Genesis Initialization to Module Pattern
4 participants