Refactor x/staking Unbonding Validator Queue

[alexanderbez]

Description

• Refactor x/staking unbonding validator queue logic to use height and time
• Godoc cleanup

closes: #6478

/cc @cwgoes

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• 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
• Review Codecov Report in the comment section below once CI passes

[cwgoes]

I think it is possible to use a single key like you have started to do here, just note that the iteration over safe-to-unbond validators must be non-linear, you have to check the second value in the key (since unbonding height is first in the key, and it's possible that a validator will be past their unbonding height but not past their unbonding time).

[alexanderbez]

Thanks for pointing that out @cwgoes. Do you believe the single key approach here is the ideal approach? I can swap the composite key s.t. the time comes first (which we'll have to length-prefix), so that'll make the iteration linear. If so, we'll still have to check the height too though, no?

[cwgoes]

Thanks for pointing that out @cwgoes. Do you believe the single key approach here is the ideal approach? I can swap the composite key s.t. the time comes first (which we'll have to length-prefix), so that'll make the iteration linear. If so, we'll still have to check the height too though, no?

If the key is prefixed by time, you'll have to check the height; if the key is prefixed by height, you'll have to check the time. I can't immediately think of a way to avoid checking one of the two values without more control over the tree (e.g. a custom ordering function). In expectation, prefixing by time is probably better, since the height requirement is intended to only kick in under exceptional circumstances (where the validators are misbehaving & accelerating the timestamp).

[codecov]

Codecov Report

Merging #6844 into master will increase coverage by 0.03%.
The diff coverage is 83.09%.

@@            Coverage Diff             @@
##           master    #6844      +/-   ##
==========================================
+ Coverage   61.42%   61.46%   +0.03%     
==========================================
  Files         517      517              
  Lines       32060    32077      +17     
==========================================
+ Hits        19694    19716      +22     
+ Misses      10799    10791       -8     
- Partials     1567     1570       +3     

[cwgoes]

utACK