Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Aug 16, 2022
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
94d866e
refactor(x/mint): unexport keeper methods
p0mvn Aug 15, 2022
3ef76d5
changelog
p0mvn Aug 15, 2022
249b3c8
Update x/mint/keeper/keeper.go
p0mvn Aug 15, 2022
e841d69
Update x/mint/keeper/keeper.go
p0mvn Aug 15, 2022
f6f1644
Update x/mint/keeper/keeper.go
p0mvn Aug 15, 2022
562f43b
Update x/mint/keeper/keeper.go
p0mvn Aug 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
118 changes: 56 additions & 62 deletions x/mint/keeper/keeper.go
View file
Open in desktop
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 {
Copy link
Member Author

@p0mvn p0mvn Aug 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers: some methods in this file were shuffled around so that exported methods go first, and, then, unexported follow

None of the methods were actually removed

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 get the minter.
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
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 set the minter.
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
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,19 +138,6 @@ 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.
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
func (k Keeper) DistributeMintedCoin(ctx sdk.Context, mintedCoin sdk.Coin) error {
params := k.GetParams(ctx)
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
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
// 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
Open in desktop
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 @@ -441,7 +440,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
Open in desktop
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