refactor(x/mint): unexport keeper methods (backport #2417)

CHANGELOG.md

@@ -42,6 +42,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+#### Golang API breaks
+* [#2422](https://github.com/osmosis-labs/osmosis/pull/2422) x/mint unexport keeper `SetLastReductionEpochNum`, `getLastReductionEpochNum`, `CreateDeveloperVestingModuleAccount`, and `MintCoins`
+
 ## v11.0.1
 
 #### Golang API breaks

app/apptesting/test_suite.go

@@ -28,6 +28,7 @@ import (
 	gammtypes "github.com/osmosis-labs/osmosis/v11/x/gamm/types"
 	lockupkeeper "github.com/osmosis-labs/osmosis/v11/x/lockup/keeper"
 	lockuptypes "github.com/osmosis-labs/osmosis/v11/x/lockup/types"
+	minttypes "github.com/osmosis-labs/osmosis/v11/x/mint/types"
 )
 
 type KeeperTestHelper struct {
@@ -77,6 +78,11 @@ func (s *KeeperTestHelper) FundAcc(acc sdk.AccAddress, amounts sdk.Coins) {
 	s.Require().NoError(err)
 }
 
+func (s *KeeperTestHelper) MintCoins(coins sdk.Coins) {
+	err := s.App.BankKeeper.MintCoins(s.Ctx, minttypes.ModuleName, coins)
+	s.Require().NoError(err)
+}
+
 func (s *KeeperTestHelper) SetupValidator(bondStatus stakingtypes.BondStatus) sdk.ValAddress {
 	valPub := secp256k1.GenPrivKey().PubKey()
 	valAddr := sdk.ValAddress(valPub.Address())

x/mint/keeper/export_test.go

@@ -20,6 +20,10 @@ var (
 	GetProportions = getProportions
 )
 
+func (k Keeper) CreateDeveloperVestingModuleAccount(ctx sdk.Context, amount sdk.Coin) error {
+	return k.createDeveloperVestingModuleAccount(ctx, amount)
+}
+
 func (k Keeper) DistributeToModule(ctx sdk.Context, recipientModule string, mintedCoin sdk.Coin, proportion sdk.Dec) (sdk.Int, error) {
 	return k.distributeToModule(ctx, recipientModule, mintedCoin, proportion)
 }
@@ -28,6 +32,18 @@ func (k Keeper) DistributeDeveloperRewards(ctx sdk.Context, totalMintedCoin sdk.
 	return k.distributeDeveloperRewards(ctx, totalMintedCoin, developerRewardsProportion, developerRewardsReceivers)
 }
 
+func (k Keeper) GetLastHalvenEpochNum(ctx sdk.Context) int64 {
+	return k.getLastHalvenEpochNum(ctx)
+}
+
+func (k Keeper) SetLastHalvenEpochNum(ctx sdk.Context, epochNum int64) {
+	k.setLastHalvenEpochNum(ctx, epochNum)
+}
+
+func (k Keeper) MintCoins(ctx sdk.Context, newCoins sdk.Coins) error {
+	return k.mintCoins(ctx, newCoins)
+}
+
 // Set the mint hooks. This is used for testing purposes only.
 func (k *Keeper) SetMintHooksUnsafe(h types.MintHooks) *Keeper {
 	k.hooks = h

x/mint/keeper/genesis.go

@@ -27,14 +27,14 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data *types.GenesisState) {
 	if !k.accountKeeper.HasAccount(ctx, k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName)) {
 		totalDeveloperVestingCoins := sdk.NewCoin(data.Params.MintDenom, sdk.NewInt(developerVestingAmount))
 
-		if err := k.CreateDeveloperVestingModuleAccount(ctx, totalDeveloperVestingCoins); err != nil {
+		if err := k.createDeveloperVestingModuleAccount(ctx, totalDeveloperVestingCoins); err != nil {
 			panic(err)
 		}
 
 		k.bankKeeper.AddSupplyOffset(ctx, data.Params.MintDenom, sdk.NewInt(developerVestingAmount).Neg())
 	}
 
-	k.SetLastHalvenEpochNum(ctx, data.HalvenStartedEpoch)
+	k.setLastHalvenEpochNum(ctx, data.HalvenStartedEpoch)
 }
 
 // ExportGenesis returns a GenesisState for a given context and keeper.
@@ -46,6 +46,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 		params.WeightedDeveloperRewardsReceivers = make([]types.WeightedAddress, 0)
 	}
 
-	lastHalvenEpoch := k.GetLastHalvenEpochNum(ctx)
+	lastHalvenEpoch := k.getLastHalvenEpochNum(ctx)
 	return types.NewGenesisState(minter, params, lastHalvenEpoch)
 }

x/mint/keeper/hooks.go

@@ -30,7 +30,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb
 		if epochNumber < params.MintingRewardsDistributionStartEpoch {
 			return
 		} else if epochNumber == params.MintingRewardsDistributionStartEpoch {
-			k.SetLastHalvenEpochNum(ctx, epochNumber)
+			k.setLastHalvenEpochNum(ctx, epochNumber)
 		}
 		// fetch stored minter & params
 		minter := k.GetMinter(ctx)
@@ -40,19 +40,19 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb
 		// This avoids issues with measuring in block numbers, as epochs have fixed intervals, with very
 		// low variance at the relevant sizes. As a result, it is safe to store the epoch number
 		// of the last reduction to be later retrieved for comparison.
-		if epochNumber >= params.ReductionPeriodInEpochs+k.GetLastHalvenEpochNum(ctx) {
+		if epochNumber >= params.ReductionPeriodInEpochs+k.getLastHalvenEpochNum(ctx) {
 			// Reduce the reward per reduction period
 			minter.EpochProvisions = minter.NextEpochProvisions(params)
 			k.SetMinter(ctx, minter)
-			k.SetLastHalvenEpochNum(ctx, epochNumber)
+			k.setLastHalvenEpochNum(ctx, epochNumber)
 		}
 
 		// 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)
+		err := k.mintCoins(ctx, mintedCoins)
 		if err != nil {
 			panic(err)
 		}

x/mint/keeper/keeper.go

@@ -92,34 +92,6 @@ func (k Keeper) SetInitialSupplyOffsetDuringMigration(ctx sdk.Context) error {
 	return nil
 }
 
-// CreateDeveloperVestingModuleAccount creates the developer vesting module account
-// and mints amount of tokens to it.
-// Should only be called during the initial genesis creation, never again. Returns nil on success.
-// Returns error in the following cases:
-// - amount is nil or zero.
-// - if ctx has block height greater than 0.
-// - developer vesting module account is already created prior to calling this method.
-func (k Keeper) CreateDeveloperVestingModuleAccount(ctx sdk.Context, amount sdk.Coin) error {
-	if amount.IsNil() || amount.Amount.IsZero() {
-		return sdkerrors.Wrap(types.ErrAmountNilOrZero, "amount cannot be nil or zero")
-	}
-	if k.accountKeeper.HasAccount(ctx, k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName)) {
-		return sdkerrors.Wrapf(types.ErrModuleAccountAlreadyExist, "%s vesting module account already exist", types.DeveloperVestingModuleAcctName)
-	}
-
-	moduleAcc := authtypes.NewEmptyModuleAccount(
-		types.DeveloperVestingModuleAcctName, authtypes.Minter)
-	k.accountKeeper.SetModuleAccount(ctx, moduleAcc)
-
-	err := k.bankKeeper.MintCoins(ctx, types.DeveloperVestingModuleAcctName, sdk.NewCoins(amount))
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
-// _____________________________________________________________________
-
 // Logger returns a module-specific logger.
 func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 	return ctx.Logger().With("module", "x/"+types.ModuleName)
@@ -136,24 +108,7 @@ func (k *Keeper) SetHooks(h types.MintHooks) *Keeper {
 	return k
 }
 
-// GetLastHalvenEpochNum returns last halven epoch number.
-func (k Keeper) GetLastHalvenEpochNum(ctx sdk.Context) int64 {
-	store := ctx.KVStore(k.storeKey)
-	b := store.Get(types.LastHalvenEpochKey)
-	if b == nil {
-		return 0
-	}
-
-	return int64(sdk.BigEndianToUint64(b))
-}
-
-// SetLastHalvenEpochNum set last halven epoch number.
-func (k Keeper) SetLastHalvenEpochNum(ctx sdk.Context, epochNum int64) {
-	store := ctx.KVStore(k.storeKey)
-	store.Set(types.LastHalvenEpochKey, sdk.Uint64ToBigEndian(uint64(epochNum)))
-}
-
-// get the minter.
+// GetMinter gets the minter.
 func (k Keeper) GetMinter(ctx sdk.Context) (minter types.Minter) {
 	store := ctx.KVStore(k.storeKey)
 	b := store.Get(types.MinterKey)
@@ -165,15 +120,13 @@ func (k Keeper) GetMinter(ctx sdk.Context) (minter types.Minter) {
 	return
 }
 
-// set the minter.
+// SetMinter sets the minter.
 func (k Keeper) SetMinter(ctx sdk.Context, minter types.Minter) {
 	store := ctx.KVStore(k.storeKey)
 	b := k.cdc.MustMarshal(&minter)
 	store.Set(types.MinterKey, b)
 }
 
-// _____________________________________________________________________
-
 // GetParams returns the total set of minting parameters.
 func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) {
 	k.paramSpace.GetParamSet(ctx, &params)
@@ -185,20 +138,7 @@ func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
 	k.paramSpace.SetParamSet(ctx, &params)
 }
 
-// _____________________________________________________________________
-
-// MintCoins implements an alias call to the underlying supply keeper's
-// MintCoins to be used in BeginBlocker.
-func (k Keeper) MintCoins(ctx sdk.Context, newCoins sdk.Coins) error {
-	if newCoins.Empty() {
-		// skip as no coins need to be minted
-		return nil
-	}
-
-	return k.bankKeeper.MintCoins(ctx, types.ModuleName, newCoins)
-}
-
-// DistributeMintedCoins implements distribution of minted coins from mint to external modules.
+// DistributeMintedCoin implements distribution of a minted coin from mint to external modules.
 func (k Keeper) DistributeMintedCoin(ctx sdk.Context, mintedCoin sdk.Coin) error {
 	params := k.GetParams(ctx)
 	proportions := params.DistributionProportions
@@ -234,6 +174,34 @@ func (k Keeper) DistributeMintedCoin(ctx sdk.Context, mintedCoin sdk.Coin) error
 	return err
 }
 
+// getLastHalvenEpochNum returns last reduction epoch number.
+func (k Keeper) getLastHalvenEpochNum(ctx sdk.Context) int64 {
+	store := ctx.KVStore(k.storeKey)
+	b := store.Get(types.LastHalvenEpochKey)
+	if b == nil {
+		return 0
+	}
+
+	return int64(sdk.BigEndianToUint64(b))
+}
+
+// setLastHalvenEpochNum set last reduction epoch number.
+func (k Keeper) setLastHalvenEpochNum(ctx sdk.Context, epochNum int64) {
+	store := ctx.KVStore(k.storeKey)
+	store.Set(types.LastHalvenEpochKey, sdk.Uint64ToBigEndian(uint64(epochNum)))
+}
+
+// mintCoins implements an alias call to the underlying bank keeper's
+// MintCoins to be used in BeginBlocker.
+func (k Keeper) mintCoins(ctx sdk.Context, newCoins sdk.Coins) error {
+	if newCoins.Empty() {
+		// skip as no coins need to be minted
+		return nil
+	}
+
+	return k.bankKeeper.MintCoins(ctx, types.ModuleName, newCoins)
+}
+
 // distributeToModule distributes mintedCoin multiplied by proportion to the recepientModule account.
 func (k Keeper) distributeToModule(ctx sdk.Context, recipientModule string, mintedCoin sdk.Coin, proportion sdk.Dec) (sdk.Int, error) {
 	distributionCoin, err := getProportions(mintedCoin, proportion)
@@ -338,3 +306,29 @@ func getProportions(mintedCoin sdk.Coin, ratio sdk.Dec) (sdk.Coin, error) {
 	}
 	return sdk.NewCoin(mintedCoin.Denom, mintedCoin.Amount.ToDec().Mul(ratio).TruncateInt()), nil
 }
+
+// createDeveloperVestingModuleAccount creates the developer vesting module account
+// and mints amount of tokens to it.
+// Should only be called during the initial genesis creation, never again. Returns nil on success.
+// Returns error in the following cases:
+// - amount is nil or zero.
+// - if ctx has block height greater than 0.
+// - developer vesting module account is already created prior to calling this method.
+func (k Keeper) createDeveloperVestingModuleAccount(ctx sdk.Context, amount sdk.Coin) error {
+	if amount.IsNil() || amount.Amount.IsZero() {
+		return sdkerrors.Wrap(types.ErrAmountNilOrZero, "amount cannot be nil or zero")
+	}
+	if k.accountKeeper.HasAccount(ctx, k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName)) {
+		return sdkerrors.Wrapf(types.ErrModuleAccountAlreadyExist, "%s vesting module account already exist", types.DeveloperVestingModuleAcctName)
+	}
+
+	moduleAcc := authtypes.NewEmptyModuleAccount(
+		types.DeveloperVestingModuleAcctName, authtypes.Minter)
+	k.accountKeeper.SetModuleAccount(ctx, moduleAcc)
+
+	err := k.bankKeeper.MintCoins(ctx, types.DeveloperVestingModuleAcctName, sdk.NewCoins(amount))
+	if err != nil {
+		return err
+	}
+	return nil
+}

x/mint/keeper/keeper_test.go

@@ -230,11 +230,10 @@ func (suite *KeeperTestSuite) TestDistributeMintedCoin() {
 			}
 
 			// mints coins so supply exists on chain
-			err := mintKeeper.MintCoins(ctx, sdk.NewCoins(tc.mintCoin))
-			suite.Require().NoError(err)
+			suite.MintCoins(sdk.NewCoins(tc.mintCoin))
 
 			// System under test.
-			err = mintKeeper.DistributeMintedCoin(ctx, tc.mintCoin)
+			err := mintKeeper.DistributeMintedCoin(ctx, tc.mintCoin)
 			suite.Require().NoError(err)
 
 			// validate that AfterDistributeMintedCoin hook was called once.
@@ -336,6 +335,10 @@ func (suite *KeeperTestSuite) TestSetInitialSupplyOffsetDuringMigration() {
 			bankKeeper := suite.App.BankKeeper
 			mintKeeper := suite.App.MintKeeper
 
+			// in order to ensure the offset is correctly calculated, we need to mint the supply + 1
+			// this is because a negative supply offset will always return zero
+			// by setting this to the supply + 1, we ensure we are correctly calculating the offset by keeping it delta positive
+			suite.MintCoins(sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(keeper.DeveloperVestingAmount+1))))
 			supplyWithOffsetBefore := bankKeeper.GetSupplyWithOffset(ctx, sdk.DefaultBondDenom)
 			supplyOffsetBefore := bankKeeper.GetSupplyOffset(ctx, sdk.DefaultBondDenom)
 
@@ -436,7 +439,7 @@ func (suite *KeeperTestSuite) TestDistributeToModule() {
 				ctx := suite.Ctx
 
 				// Setup.
-				suite.Require().NoError(mintKeeper.MintCoins(ctx, sdk.NewCoins(tc.preMintCoin)))
+				suite.MintCoins(sdk.NewCoins(tc.preMintCoin))
 
 				// TODO: Should not be truncated. Remove truncation after rounding errors are addressed and resolved.
 				// Ref: https://github.com/osmosis-labs/osmosis/issues/1917

x/pool-incentives/keeper/distr_test.go

@@ -26,10 +26,9 @@ func (suite *KeeperTestSuite) TestAllocateAssetToCommunityPoolWhenNoDistrRecords
 	// At this time, there is no distr record, so the asset should be allocated to the community pool.
 	mintCoin := sdk.NewCoin("stake", sdk.NewInt(100000))
 	mintCoins := sdk.Coins{mintCoin}
-	err := mintKeeper.MintCoins(suite.Ctx, mintCoins)
-	suite.NoError(err)
+	suite.MintCoins(mintCoins)
 
-	err = mintKeeper.DistributeMintedCoin(suite.Ctx, mintCoin) // this calls AllocateAsset via hook
+	err := mintKeeper.DistributeMintedCoin(suite.Ctx, mintCoin) // this calls AllocateAsset via hook
 	suite.NoError(err)
 
 	distribution.BeginBlocker(suite.Ctx, abci.RequestBeginBlock{}, *suite.App.DistrKeeper)
@@ -42,8 +41,7 @@ func (suite *KeeperTestSuite) TestAllocateAssetToCommunityPoolWhenNoDistrRecords
 	// Community pool should be increased
 	mintCoin = sdk.NewCoin("stake", sdk.NewInt(100000))
 	mintCoins = sdk.Coins{mintCoin}
-	err = mintKeeper.MintCoins(suite.Ctx, mintCoins)
-	suite.NoError(err)
+	suite.MintCoins(mintCoins)
 	err = mintKeeper.DistributeMintedCoin(suite.Ctx, mintCoin) // this calls AllocateAsset via hook
 	suite.NoError(err)
 
@@ -102,8 +100,7 @@ func (suite *KeeperTestSuite) TestAllocateAsset() {
 	// In this time, there are 3 records, so the assets to be allocated to the gauges proportionally.
 	mintCoin := sdk.NewCoin("stake", sdk.NewInt(100000))
 	mintCoins := sdk.Coins{mintCoin}
-	err = mintKeeper.MintCoins(suite.Ctx, mintCoins)
-	suite.NoError(err)
+	suite.MintCoins(mintCoins)
 
 	err = mintKeeper.DistributeMintedCoin(suite.Ctx, mintCoin) // this calls AllocateAsset via hook
 	suite.NoError(err)
@@ -127,7 +124,7 @@ func (suite *KeeperTestSuite) TestAllocateAsset() {
 	// Allocate more.
 	mintCoin = sdk.NewCoin("stake", sdk.NewInt(50000))
 	mintCoins = sdk.Coins{mintCoin}
-	err = mintKeeper.MintCoins(suite.Ctx, mintCoins)
+	suite.MintCoins(mintCoins)
 	suite.NoError(err)
 	err = mintKeeper.DistributeMintedCoin(suite.Ctx, mintCoin) // this calls AllocateAsset via hook
 	suite.NoError(err)
@@ -171,7 +168,7 @@ func (suite *KeeperTestSuite) TestAllocateAsset() {
 	// In this time, there are 3 records, so the assets to be allocated to the gauges proportionally.
 	mintCoin = sdk.NewCoin("stake", sdk.NewInt(100000))
 	mintCoins = sdk.Coins{mintCoin}
-	err = mintKeeper.MintCoins(suite.Ctx, mintCoins)
+	suite.MintCoins(mintCoins)
 	suite.NoError(err)
 	err = mintKeeper.DistributeMintedCoin(suite.Ctx, mintCoin) // this calls AllocateAsset via hook
 	suite.NoError(err)
@@ -194,7 +191,7 @@ func (suite *KeeperTestSuite) TestAllocateAsset() {
 	// In this time, all should be allocated to community pool
 	mintCoin = sdk.NewCoin("stake", sdk.NewInt(100000))
 	mintCoins = sdk.Coins{mintCoin}
-	err = mintKeeper.MintCoins(suite.Ctx, mintCoins)
+	suite.MintCoins(mintCoins)
 	suite.NoError(err)
 	err = mintKeeper.DistributeMintedCoin(suite.Ctx, mintCoin) // this calls AllocateAsset via hook
 	suite.NoError(err)