x/gov: slash nonvoting validators error.

[MrXJC]

Summary of Bug

There is a big error in gov. It will lead to the consensus failure, because every node maybe have entirely different nonVotingVals which is from the map currValidators. Then, slash validators in different order and make the Apphash different.

for _, valAddr := range nonVotingVals {
	val := keeper.ds.GetValidatorSet().Validator(ctx, valAddr)
	keeper.ds.GetValidatorSet().Slash(....)
}

Steps to Reproduce

Just submit a proposal and all of the validators don't vote.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[ValarDragon]

While I agree that non-deterministic ordering of slashes is sub-optimal, I think the slashing logic should be independent of the order in which slashes were applied. (I also agree with fixing this) Unless IAVL root hash depends on insertion order, in which case then this is a problem.

However this shouldn't be a consensus failure. I would like it if we could actually use this (or something similar) in a test somewhere to ensure that the order of slashes isn't important.

/cc @sunnya97 @cwgoes

[cwgoes]

Unless IAVL root hash depends on insertion order, in which case then this is a problem.

It does but we resort the keys at the end of the block when the cache is flushed. We should still avoid any sort of non-deterministic Golang map iteration.

[alexanderbez]

++

Sounds like this is a nice-to-have, but not an immediate need. Feel free to submit a PR @MrXJC where iteration happens by sorted map keys 👍

Edit: Ahhh, nvm, I see you opened #2159 -- thank you!

[ValarDragon]

closing, thanks for the PR!

Made #2288 as a follow-up for making a test case for this.