fix: RewardBallotWinners determinism

[robert-zaremba]

Description

Fixes RC1 Canon consensus failure.

Bug was in: https://github.com/umee-network/umee/pull/1433/files#diff-971aeedb004816fd5770d86dfa2d9ee39c5e53d7802773dd9e81cbc2cf890f62L60

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• added appropriate labels to the PR
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[adamewozniak]

approved 👍 was this introduced in #1391 or was this in the oracle previously / should we have seen that in the latest beta?

[robert-zaremba]

approved +1 was this introduced in #1391 or was this in the oracle previously / should we have seen that in the latest beta?

No. #1381 didn't touch RewardBallotWinners. Basically, the bug is that we iterate over a map for k.distrKeeper.AllocateTokensToValidator to send distributions in an AVL tree. I will make it more clear in few secs

[adamewozniak]

approved +1 was this introduced in #1391 or was this in the oracle previously / should we have seen that in the latest beta?

No. #1381 didn't touch RewardBallotWinners. Basically, the bug is that we iterate over a map for k.distrKeeper.AllocateTokensToValidator to send distributions in an AVL tree. I will make it more clear in few secs

How come we didn't see this in any of the betas then?

[toteki]

Map nondeterminism bugs are elusive - they'll only reveal themselves if the underlying data were arranged differently, and the conditions when that will appear in practice are undefined.

Probably didn't appear in betas due to the small number of validators, operating under similar conditions

[codecov-commenter]

Codecov Report

Merging #1433 (19e1e01) into main (1f8bc6f) will increase coverage by 0.00%.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##             main    #1433   +/-   ##
=======================================
  Coverage   52.80%   52.81%           
=======================================
  Files          70       70           
  Lines        6751     6752    +1     
=======================================
+ Hits         3565     3566    +1     
  Misses       2916     2916           
  Partials      270      270           

Impacted Files	Coverage Δ
ante/fee.go	78.35% <33.33%> (ø)
app/upgrades/v3/min_commission.go	85.71% <100.00%> (ø)
x/oracle/client/tests/suite.go	98.38% <100.00%> (ø)
x/oracle/keeper/reward.go	86.36% <100.00%> (+0.31%)	⬆️

[RafilxTenfen]

ohhh, so in the sum part it didn't matter that much, because sum the whole amount in map or slice is the same thing,
but when we iterate again here to send rewards, it matters

@robert-zaremba is that right?

[robert-zaremba]

@RafilxTenfen correct.