refactor(x/mint): avoid over-allocation and then burning dev rewards from mint module account

[p0mvn]

Background

Currently, we mint the number of tokens equal to epoch provisions:

osmosis/x/mint/keeper/hooks.go

Lines 42 to 47 in 8cb9470

	// mint coins, update supply
	mintedCoin := minter.EpochProvision(params)
	mintedCoins := sdk.NewCoins(mintedCoin)
	// We over-allocate by the developer vesting portion, and burn this later
	err := k.MintCoins(ctx, mintedCoins)

Epoch provisions include the developer rewards proportion. However, the developer rewards must be allocated from the developer vesting module account. As a result, we over-allocate from mint module account.

To mitigate, we instead burn tokens here while distributing them to dev reward receivers:

osmosis/x/mint/keeper/keeper.go

Lines 218 to 220 in 8cb9470

	// This is supposed to come from the developer vesting module address, not the mint module address
	// we over-allocated to the mint module address earlier though, so we burn it right here.
	if err := k.bankKeeper.BurnCoins(ctx, types.ModuleName, devRewardCoins); err != nil {

This is called temporal decomposition where execution order is reflected in the code structure:

• More info (search for temporal decomposition): https://neexee.github.io/posts-en/philosophy-of-software-design/

Suggested Design

Instead, we should be minting the correct amount in the first place where we exclude the developer rewards from being minted by the mint module account. By doing so, we would avoid having to later burn the amount.

The suggested design should reduce coupling and remove the temporal decomposition code smell.

Acceptance Criteria

• mint module account does not mint developer rewards
• developer rewards are only minted by the developer vesting module account
• unit test added to cover all new logic

[p0mvn]

This is currently blocked by #1916 and #1917 . Making this issue to keep track of.

There are other constraints that should be considered after the blocking issues are addressed.

For example, one constraint is that DistributedMintedCoin:

osmosis/x/mint/keeper/keeper.go

Line 197 in 8cb9470

func (k Keeper) DistributeMintedCoin(ctx sdk.Context, mintedCoin sdk.Coin) error {

assumes that mintedCoin includes the dev rewards amount. It might have to be refactored further to change the assumption.

[stackman27]

I can take a shot at this, i'll follow up with the progress in #1916 and #1917