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.

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): remove SetInitialSupplyOffsetDuringMigration keeper method #2418

Merged
merged 4 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
* [#2418](https://github.com/osmosis-labs/osmosis/pull/2418) x/mint remove SetInitialSupplyOffsetDuringMigration from keeper
* [#2417](https://github.com/osmosis-labs/osmosis/pull/2417) x/mint unexport keeper `SetLastReductionEpochNum`, `getLastReductionEpochNum`, `CreateDeveloperVestingModuleAccount`, and `MintCoins`

### Features
Expand Down
10 changes: 6 additions & 4 deletions app/upgrades/v7/upgrades.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,12 @@ func CreateUpgradeHandler(
}
keepers.SuperfluidKeeper.AddNewSuperfluidAsset(ctx, superfluidAsset)

// Set the supply offset from the developer vesting account
if err := keepers.MintKeeper.SetInitialSupplyOffsetDuringMigration(ctx); err != nil {
panic(err)
}
// N.B.: This is left for historic reasons.
// After the v7 upgrade, there was no need for this function anymore so it was removed.
// // Set the supply offset from the developer vesting account
// if err := keepers.MintKeeper.SetInitialSupplyOffsetDuringMigration(ctx); err != nil {
// panic(err)
// }

return newVM, err
}
Expand Down
17 changes: 0 additions & 17 deletions x/mint/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,23 +75,6 @@ func NewKeeper(
}
}

// SetInitialSupplyOffsetDuringMigration sets the supply offset based on the balance of the
// developer vesting module account. CreateDeveloperVestingModuleAccount must be called
// prior to calling this method. That is, developer vesting module account must exist when
// SetInitialSupplyOffsetDuringMigration is called. Also, SetInitialSupplyOffsetDuringMigration
// should only be called one time during the initial migration to v7. This is done so because
// we would like to ensure that unvested developer tokens are not returned as part of the supply
// queries. The method returns an error if current height in ctx is greater than the v7 upgrade height.
func (k Keeper) SetInitialSupplyOffsetDuringMigration(ctx sdk.Context) error {
if !k.accountKeeper.HasAccount(ctx, k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName)) {
return sdkerrors.Wrapf(types.ErrModuleDoesnotExist, "%s vesting module account doesnot exist", types.DeveloperVestingModuleAcctName)
}

moduleAccBalance := k.bankKeeper.GetBalance(ctx, k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName), k.GetParams(ctx).MintDenom)
k.bankKeeper.AddSupplyOffset(ctx, moduleAccBalance.Denom, moduleAccBalance.Amount.Neg())
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 Down
51 changes: 0 additions & 51 deletions x/mint/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,57 +311,6 @@ func (suite *KeeperTestSuite) TestCreateDeveloperVestingModuleAccount() {
}
}

func (suite *KeeperTestSuite) TestSetInitialSupplyOffsetDuringMigration() {
testcases := map[string]struct {
blockHeight int64
isDeveloperModuleAccountCreated bool

expectedError error
}{
"valid call": {
blockHeight: 1,
isDeveloperModuleAccountCreated: true,
},
"dev vesting module account does not exist": {
blockHeight: 1,
expectedError: sdkerrors.Wrapf(types.ErrModuleDoesnotExist, "%s vesting module account doesnot exist", types.DeveloperVestingModuleAcctName),
},
}

for name, tc := range testcases {
suite.Run(name, func() {
suite.setupDeveloperVestingModuleAccountTest(tc.blockHeight, tc.isDeveloperModuleAccountCreated)
ctx := suite.Ctx
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)

// Test
actualError := mintKeeper.SetInitialSupplyOffsetDuringMigration(ctx)

if tc.expectedError != nil {
suite.Require().Error(actualError)
suite.Require().ErrorIs(actualError, tc.expectedError)

suite.Require().Equal(supplyWithOffsetBefore.Amount, bankKeeper.GetSupplyWithOffset(ctx, sdk.DefaultBondDenom).Amount)
suite.Require().Equal(supplyOffsetBefore, bankKeeper.GetSupplyOffset(ctx, sdk.DefaultBondDenom))
return
}
suite.Require().NoError(actualError)

suite.Require().Equal(supplyWithOffsetBefore.Amount.Sub(sdk.NewInt(keeper.DeveloperVestingAmount)), bankKeeper.GetSupplyWithOffset(ctx, sdk.DefaultBondDenom).Amount)
suite.Require().Equal(supplyOffsetBefore.Sub(sdk.NewInt(keeper.DeveloperVestingAmount)), bankKeeper.GetSupplyOffset(ctx, sdk.DefaultBondDenom))
})
}
}

// TestDistributeToModule tests that distribution from mint module to another module helper
// function is working as expected.
func (suite *KeeperTestSuite) TestDistributeToModule() {
Expand Down