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

WIP: Distr-simulation OnValidatorPowerDidChange panic fix #2632

Closed

Conversation

rigelrozanski
Copy link
Contributor

closes #2618

  • 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 entries in PENDING.md with issue #

  • rereviewed 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)

@rigelrozanski rigelrozanski changed the base branch from develop to cwgoes/simulation-transubstantiated October 30, 2018 16:32
@codecov
Copy link

codecov bot commented Oct 31, 2018

Codecov Report

Merging #2632 into cwgoes/simulation-transubstantiated will increase coverage by 0.14%.
The diff coverage is 81.25%.

@@                           Coverage Diff                           @@
##           cwgoes/simulation-transubstantiated    #2632      +/-   ##
=======================================================================
+ Coverage                                58.82%   58.96%   +0.14%     
=======================================================================
  Files                                      152      152              
  Lines                                     9425     9434       +9     
=======================================================================
+ Hits                                      5544     5563      +19     
+ Misses                                    3511     3500      -11     
- Partials                                   370      371       +1

@rigelrozanski
Copy link
Contributor Author

rigelrozanski commented Oct 31, 2018

running go test ./cmd/gaia/app -run TestFullGaiaSimulation -SimulationEnabled=true -SimulationNumBlocks=400 -SimulationBlockSize=200 -SimulationCommit=true -SimulationSeed=3 -v -timeout 24h

@rigelrozanski
Copy link
Contributor Author

So it looks like what is happening is that power is being updated in block 5 (likely due to a delayed update) but block 5 has no transactions hence, none of the fees have been withdrawn - this issue looks intimately linked to the delayed validator set changes logic

@rigelrozanski
Copy link
Contributor Author

yeah basically something is modifying the power which is not being logged, hard to determine what is happening - possibly has to do with a slashing occurrence? Anyways we need better debugging tooling to determine this (CC @cwgoes)

@cwgoes
Copy link
Contributor

cwgoes commented Oct 31, 2018

@rigelrozanski Did you try using the tracing KV store? Might help, or at least help isolate when the write is occurring.

Agreed on better debugging tooling, what specific tooling do you think would be useful here?

@rigelrozanski
Copy link
Contributor Author

Note that this issue was somehow resolved in seed 124 but is still present in seed 3

@cwgoes
Copy link
Contributor

cwgoes commented Nov 5, 2018

@rigelrozanski I think we can close this since the issue was fixed in #2677.

@rigelrozanski
Copy link
Contributor Author

absolutely

@rigelrozanski rigelrozanski deleted the rigel/distr-gen-fix branch November 6, 2018 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants