WIP: Demonstrate staking bug

[ValarDragon]

This PR basically just demonstrates that there is a problem, and this causes simulation to fail.
Something is happening that is causing a non-existent validator to get deleted.

---

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)

[ValarDragon]

Its failing in test_sim_modules by itself: https://circleci.com/gh/cosmos/cosmos-sdk/25616?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (dev/benchmark_simulation@b3d08bc). Click here to learn what that means.
The diff coverage is n/a.

@@                     Coverage Diff                     @@
##             dev/benchmark_simulation    #2188   +/-   ##
===========================================================
  Coverage                            ?   63.62%           
===========================================================
  Files                               ?      136           
  Lines                               ?     8270           
  Branches                            ?        0           
===========================================================
  Hits                                ?     5262           
  Misses                              ?     2652           
  Partials                            ?      356

[cwgoes]

I don't think this is necessarily caused by a particular issue in staking, this is always how we've handled validator updates, but if the TM side has changed (for the better!) we might need to update our end too.

If a validator goes from zero power to nonzero power and back to zero power in the same block (through a few transactions), the validator will be passed back to Tendermint with zero power. In this case Tendermint used to do nothing (delete a key that wasn't there), but it looks like that behavior may have changed - https://github.com/tendermint/tendermint/blob/master/state/execution.go#L290 - which could be a problem here.

[rigelrozanski]

@ValarDragon / @cwgoes . nice catch/Interesting- well we could always attempt to keep track of things on the state machine side too... we would just need to store/check the power at the previous height for each validator which is being sent as a tendermint update with 0 power - if the validator wasn't bonded at the previous height then we just don't send the update to Tendermint... seems simple enough - I'd propose we handle it this way - thoughts?

[ValarDragon]

That seems like a good fix to me

[cwgoes]

If the validator wasn't bonded at the previous height then we just don't send the update to Tendermint.

I think this is fine. How do we track whether a validator was bonded at the start of the block, though? We can't use ABCIRequestBeginBlock.SigningValidators, because that will be offset. I guess we'll have to use another transient substore to track this?

[rigelrozanski]

@cwgoes we can tease out whether the validator was bonded or not at the previous height using on validator.BondHeight and validator.UnbondingHeight (new in unbonding validator PR)

[cwgoes]

@cwgoes we can tease out whether the validator was bonded or not at the previous height using on validator.BondHeight and validator.UnbondingHeight (new in unbonding validator PR)

Great, let's definitely get a fix for this in the next release.

[rigelrozanski]

it'd be good to just work from this PR and change the merge base to develop

[cwgoes]

cc @alexanderbez

[alexanderbez]

Closing this in favor of #2238.