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

add randomized consensus params into simulation #6051

Merged
merged 33 commits into from
Apr 22, 2020

Conversation

jgimeno
Copy link
Contributor

@jgimeno jgimeno commented Apr 22, 2020

Continues: #6002

Description


For contributor use:

  • 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.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Apr 22, 2020

Codecov Report

Merging #6051 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6051   +/-   ##
=======================================
  Coverage   54.64%   54.64%           
=======================================
  Files         425      425           
  Lines       25815    25815           
=======================================
  Hits        14106    14106           
  Misses      10735    10735           
  Partials      974      974           

@jgimeno jgimeno marked this pull request as ready for review April 22, 2020 15:46
@jgimeno jgimeno self-assigned this Apr 22, 2020
baseapp/baseapp.go Show resolved Hide resolved
Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
@@ -139,6 +139,7 @@ func TestAppImportExport(t *testing.T) {
ctxA := app.NewContext(true, abci.Header{Height: app.LastBlockHeight()})
ctxB := newApp.NewContext(true, abci.Header{Height: app.LastBlockHeight()})
newApp.mm.InitGenesis(ctxB, app.Codec(), genesisState)
newApp.StoreConsensusParams(ctxB, consensusParams)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of having to make this method public (maybe it doesn't matter), perhaps instead we call newApp.InitChain() with the consensus params?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that on the beginning, but found several issues that was making the boilerplate too bid with the InitChainer. I can follow that but was too much iin my opinion.

I tried other ways too, using options, etc, but this was the less painful.

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 alexanderbez added the A:automerge Automatically merge PR once all prerequisites pass. label Apr 22, 2020
@mergify mergify bot merged commit 550c3e9 into master Apr 22, 2020
@mergify mergify bot deleted the jonathan/5988-sim-params-consensus-2 branch April 22, 2020 18:24
@alessio alessio changed the title Jonathan/5988 sim params consensus 2 add randomized consensus params into simulation Nov 10, 2020
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
* temporal commit

* add random consensus params

* make format

* make random pass

* remove comment

* revert change

* update ub key types

* extract Evidence params from state

* extract the random parameters from state

* clean the code

* update imports!

* mispelled back

* mispelled back

* add misspelled command

* update changelog

* remove useless linter to avoid misspell

* remove function

* use tendermint constants

* update import test

* remove debug comment

* Update baseapp/baseapp.go

Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:Simulations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants