Skip to content

Commit

Permalink
refactor(x/mint): unexport keeper methods (#2417)
Browse files Browse the repository at this point in the history 
* refactor(x/mint): unexport keeper methods

* changelog

* Update x/mint/keeper/keeper.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Update x/mint/keeper/keeper.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Update x/mint/keeper/keeper.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Update x/mint/keeper/keeper.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

Co-authored-by: Adam Tucker <adam@osmosis.team>
  • Loading branch information
@p0mvn @czarcas7ic
p0mvn and czarcas7ic authored Aug 16, 2022
1 parent 91c7830 commit 4176b28
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 85 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#1857](https://github.com/osmosis-labs/osmosis/pull/1857) x/mint rename GetLastHalvenEpochNum to GetLastReductionEpochNum
* [#2394](https://github.com/osmosis-labs/osmosis/pull/2394) Remove unused interface methods from expected keepers of each module
* [#2390](https://github.com/osmosis-labs/osmosis/pull/2390) x/mint remove unused mintCoins parameter from AfterDistributeMintedCoin
* [#2417](https://github.com/osmosis-labs/osmosis/pull/2417) x/mint unexport keeper `SetLastReductionEpochNum`, `getLastReductionEpochNum`, `CreateDeveloperVestingModuleAccount`, and `MintCoins`

### Features

Expand Down
6 changes: 6 additions & 0 deletions app/apptesting/test_suite.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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 {
Expand Down Expand Up @@ -95,6 +96,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)
}

// SetupValidator sets up a validator and returns the ValAddress.
func (s *KeeperTestHelper) SetupValidator(bondStatus stakingtypes.BondStatus) sdk.ValAddress {
valPub := secp256k1.GenPrivKey().PubKey()
Expand Down
16 changes: 16 additions & 0 deletions x/mint/keeper/export_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -28,6 +32,18 @@ func (k Keeper) DistributeDeveloperRewards(ctx sdk.Context, totalMintedCoin sdk.
return k.distributeDeveloperRewards(ctx, totalMintedCoin, developerRewardsProportion, developerRewardsReceivers)
}

func (k Keeper) GetLastReductionEpochNum(ctx sdk.Context) int64 {
return k.getLastReductionEpochNum(ctx)
}

func (k Keeper) SetLastReductionEpochNum(ctx sdk.Context, epochNum int64) {
k.setLastReductionEpochNum(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
Expand Down
6 changes: 3 additions & 3 deletions x/mint/keeper/genesis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.SetLastReductionEpochNum(ctx, data.ReductionStartedEpoch)
k.setLastReductionEpochNum(ctx, data.ReductionStartedEpoch)
}

// ExportGenesis returns a GenesisState for a given context and keeper.
Expand All @@ -46,6 +46,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
params.WeightedDeveloperRewardsReceivers = make([]types.WeightedAddress, 0)
}

lastHalvenEpoch := k.GetLastReductionEpochNum(ctx)
lastHalvenEpoch := k.getLastReductionEpochNum(ctx)
return types.NewGenesisState(minter, params, lastHalvenEpoch)
}
8 changes: 4 additions & 4 deletions x/mint/keeper/hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.SetLastReductionEpochNum(ctx, epochNumber)
k.setLastReductionEpochNum(ctx, epochNumber)
}
// fetch stored minter & params
minter := k.GetMinter(ctx)
Expand All @@ -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.GetLastReductionEpochNum(ctx) {
if epochNumber >= params.ReductionPeriodInEpochs+k.getLastReductionEpochNum(ctx) {
// Reduce the reward per reduction period
minter.EpochProvisions = minter.NextEpochProvisions(params)
k.SetMinter(ctx, minter)
k.SetLastReductionEpochNum(ctx, epochNumber)
k.setLastReductionEpochNum(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)
}
Expand Down
120 changes: 57 additions & 63 deletions x/mint/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -136,24 +108,7 @@ func (k *Keeper) SetHooks(h types.MintHooks) *Keeper {
return k
}

// GetLastReductionEpochNum returns last reduction epoch number.
func (k Keeper) GetLastReductionEpochNum(ctx sdk.Context) int64 {
store := ctx.KVStore(k.storeKey)
b := store.Get(types.LastReductionEpochKey)
if b == nil {
return 0
}

return int64(sdk.BigEndianToUint64(b))
}

// SetLastReductionEpochNum set last reduction epoch number.
func (k Keeper) SetLastReductionEpochNum(ctx sdk.Context, epochNum int64) {
store := ctx.KVStore(k.storeKey)
store.Set(types.LastReductionEpochKey, 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)
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -234,6 +174,34 @@ func (k Keeper) DistributeMintedCoin(ctx sdk.Context, mintedCoin sdk.Coin) error
return err
}

// getLastReductionEpochNum returns last reduction epoch number.
func (k Keeper) getLastReductionEpochNum(ctx sdk.Context) int64 {
store := ctx.KVStore(k.storeKey)
b := store.Get(types.LastReductionEpochKey)
if b == nil {
return 0
}

return int64(sdk.BigEndianToUint64(b))
}

// setLastReductionEpochNum set last reduction epoch number.
func (k Keeper) setLastReductionEpochNum(ctx sdk.Context, epochNum int64) {
store := ctx.KVStore(k.storeKey)
store.Set(types.LastReductionEpochKey, 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)
Expand Down Expand Up @@ -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
}
9 changes: 4 additions & 5 deletions x/mint/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -339,7 +338,7 @@ func (suite *KeeperTestSuite) TestSetInitialSupplyOffsetDuringMigration() {
// 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
mintKeeper.MintCoins(ctx, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(keeper.DeveloperVestingAmount+1))))
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)
Expand Down Expand Up @@ -442,7 +441,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
Expand Down
17 changes: 7 additions & 10 deletions x/pool-incentives/keeper/distr_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit 4176b28

Please sign in to comment.