Cosmos-sdk: Unify iteration approach

[rigelrozanski]

Going to through the staking code it's obvious that the way we choose to iterate across store records is totally inconsistent, sometimes we use a "retrieveAll" functionality which iterates and returns and array of all the records, which is then received and simply looped over. Other times we use an iterator function which takes an "operating function". Other times we a special keeper function which returns and sdk.Iterator which is then utilized in higher level function. There appears to be duplication of Iterators based on fulfilling the sdk.ValidatorSet/ sdk.DelegationSet

I'm not saying there isn't reason to have some variability in our iteration approach, it just seem that things need to be consolidated and re-evaluated as they've been slapped together.

I'm assuming that this also exists in other modules and should also be rectified.

[alexanderbez]

I'm a big fan of the functional callback approach.

[cwgoes]

I'm a big fan of the functional callback approach.

Likewise, and it's more efficient (far less allocation) when we don't need to collect records in a list.

[jackzampolin]

Sounds like the actionable here is to unify around the callback approach. Maybe this is something @fedekunze or @sabau could pick up?

[alexanderbez]

Correct -- callback approach.

/cc @aayushijain23

[fedekunze]

Agree on the callback approach, it's cleaner and more flexible

[sabau]

I found now this issue (thanks @fedekunze ) I love the callback approach idea, was just thinking about the mixed naming I've found across the codebase handle, fn, cb
https://github.com/cosmos/cosmos-sdk/pull/4460#discussion_r290226971

Why not call all those callbacks done as far as I've seen all those iterators use those callbacks at the end with a format similar to:

if done(...params) {
    break
}

[alexanderbez]

I don't think that reads well at all @sabau imho. Using handler seems like a good compromise, no?

[sabau]

Could work too, that's personal taste, I felt natural the phrase if done then break.
I was mainly trying to suggest to uniform it.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.