finality: move voting power dist update to finality module

[SebastianElvis]

First step of #24

This PR moves the voting power distribution update algorithm to x/finality. This is a major refactoring that includes

• moving max_active_finality_providers parameter to x/finality
• moving many functions in x/btcstaking/power_dist_change.go and relevant tests to x/finality
• moving test utilities under x/btcstaking/keeper_test.go to a new module testutil/btcstaking-helper, and use it for existing tests of x/btcstaking and x/finality
• removing the cyclic dependency between x/btcstaking and x/finality, as well as their hooks. Now the hooks in these 2 modules are not necessary, as the only dependency between them is that x/finality will invoke x/btcstaking, but not the other way

TODOs in subsequent PRs:

• moving some relevant query APIs to x/finality
• moving the voting power dist cache and voting power table KV stores to x/finality

[KonradStaniec]

shouldn't this whole migration be done first on feature branch ?

[KonradStaniec]

as both k.IndexBTCHeight(ctx) and k.UpdatePowerDist(ctx) are happening in BeginBlocker, it means in our v0.9.0 chain there is already data created and saved to database due to its usage.

This will probably means we require some data migration during upgrade

[SebastianElvis]

Compared to TGE chain, not really actually. The IndexBTCHeight is same in TGE chain and this version. The only difference is the voting power dist cache. As in TGE chain the code always triggers here, there is no voting power dist cache or voting power table recorded. Thus even after we move both KV stores to x/finality, the upgrade handler does not need to do any extra thing for it.

This is probably why the upgrade e2e test works properly.

[KonradStaniec]

ahh true ! so in tge chain prefix, there is no distribution caches anywhere as there is no staker/fp with voting power. thanks for explanantion 👍

[SebastianElvis]

shouldn't this whole migration be done first on feature branch ?

Fine with me on having a feature branch for this. However, given that 1) this PR is self-contained, and 2) this PR breaks a lot of things while we have a fast development schedule, I'm more inclined to merge it first. Do you have any concern on having it in main before finishing the entire feature?

[KonradStaniec]

Fine with me on having a feature branch for this. However, given that 1) this PR is self-contained, and 2) this PR breaks a lot of things while we have a fast development schedule, I'm more inclined to merge it first. Do you have any concern on having it in main before finishing the entire feature?

My main concern is that it is not yet finished feature/change which leads to few problems:

• it is now hard to asses how the final change will look like after all kv stores move etc.
• later after merge there will several commits on main branch corresponding to this one conceptual change.

I know we are on fast development schedule, but tbh if this whole feature ends up devnet5 then imo it will be fine. Or maybe we can delay devnet4 by one day if it will be possible to land this. Do you think it is feasible ?

[SebastianElvis]

My main concern is that it is not yet finished feature/change which leads to few problems:

• it is now hard to asses how the final change will look like after all kv stores move etc.
• later after merge there will several commits on main branch corresponding to this one conceptual change.

I know we are on fast development schedule, but tbh if this whole feature ends up devnet5 then imo it will be fine. Or maybe we can delay devnet4 by one day if it will be possible to land this. Do you think it is feasible ?

Alright, make sense. Targeted this PR to a feature branch then. I don't think we need to bother to delay devnet 4 for this feature though. Let's stick to the original schedule and target to have this feature in devnet 5.