From 43be3815cce07b715359dc22f5b8e8e8ffa4cc12 Mon Sep 17 00:00:00 2001 From: Draco Date: Tue, 3 Oct 2023 04:10:22 +0800 Subject: [PATCH] feat: disable inflation upgrade logic (#1706) * feat: disable inflation upgrade logic * improve disable inflation comments * add upgrade tests * update changelog * split disable inflation upgrade and the upgrade check * remove pay rewards logic * clean up incentives test * add abci test * refactor upgradeTime and blockTime check * fix abci test * fix wrong pr in changelog * refactor disable inflation tests, behavior, and implementation - Unit tests are now shared between keeper and abci begin blocker since behavior is 100% the same - ABCI is unaware of keeper initial keeper logic branch (keeper methods required to be called in certain order by begin blocker) - Behavior of zero time is changed -- this now doesn't run for the zero time. This is more ideal for new chains (genesis should set all correct state instead of relying on inflation disable logic), and allows for a simpler implementation. - Begin blocker now panics if parameters are not in state * remove previous tests and implementation * remove previous block time keeper state -- not needed for inflation disalbing * move inflation disabling to private method and add more comments * remove unused key * use correct spelling for idempotence --------- Co-authored-by: Nick DeLuca --- CHANGELOG.md | 10 +- app/app.go | 27 ++-- app/test_common.go | 6 +- x/community/abci.go | 17 ++ x/community/disable_inflation_abci_test.go | 19 +++ x/community/keeper/disable_inflation.go | 53 +++++++ x/community/keeper/disable_inflation_test.go | 18 +++ x/community/keeper/keeper.go | 26 ++-- x/community/module.go | 4 +- x/community/testutil/disable_inflation.go | 156 +++++++++++++++++++ x/community/types/expected_keepers.go | 12 ++ 11 files changed, 324 insertions(+), 24 deletions(-) create mode 100644 x/community/abci.go create mode 100644 x/community/disable_inflation_abci_test.go create mode 100644 x/community/keeper/disable_inflation.go create mode 100644 x/community/keeper/disable_inflation_test.go create mode 100644 x/community/testutil/disable_inflation.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 75ab30e0..d77795f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,20 +37,26 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] ### Features + - (metrics) [#1668] Adds non-state breaking x/metrics module for custom telemetry. - (metrics) [#1669] Add performance timing metrics to all Begin/EndBlockers - (community) [#1704] Add module params +- (community) [#1706] Add disable inflation upgrade ### Bug Fixes + - (evmutil) [#1655] Initialize x/evmutil module account in InitGenesis ## State Machine Breaking + - (community) [#1704] Add param to control when inflation will be disabled - (community) [#1707] Default staking rewards per second set to `744191` +- (community) [#1706] Add disable inflation upgrade to begin blocker that updates x/mint and x/kavadist params. ## [v0.24.0] ### Features + - (evmutil) [#1590] & [#1596] Add allow list param of sdk native denoms that can be transferred to evm - (evmutil) [#1591] & [#1596] Configure module to support deploying ERC20KavaWrappedCosmosCoin contracts - (evmutil) [#1598] Track deployed ERC20 contract addresses for representing cosmos coins in module state @@ -61,6 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ - (evmutil) [#1610] Add new invariant checking that ERC20s are fully backed by sdk.Coins ### Client Breaking + - (evmutil) [#1603] Renamed error `ErrConversionNotEnabled` to `ErrEVMConversionNotEnabled` - (evmutil) [#1604] Renamed event types - `convert_erc20_to_coin` -> `convert_evm_erc20_to_coin` @@ -72,8 +79,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ - (cli) [#1624] Removes unused, no-op `migrate` CLI command. ### Bug Fixes -- (cli) [#1624] Fix `assert-invariants` CLI command. +- (cli) [#1624] Fix `assert-invariants` CLI command. ## [v0.23.2] @@ -286,6 +293,7 @@ the [changelog](https://github.com/cosmos/cosmos-sdk/blob/v0.38.4/CHANGELOG.md). large-scale simulations remotely using aws-batch [#1707]: https://github.com/Kava-Labs/kava/pull/1707 +[#1706]: https://github.com/Kava-Labs/kava/pull/1706 [#1668]: https://github.com/Kava-Labs/kava/pull/1668 [#1669]: https://github.com/Kava-Labs/kava/pull/1669 [#1655]: https://github.com/Kava-Labs/kava/pull/1655 diff --git a/app/app.go b/app/app.go index c449d0a4..c2ac222e 100644 --- a/app/app.go +++ b/app/app.go @@ -635,16 +635,6 @@ func NewApp( &app.distrKeeper, ) - // x/community's deposit/withdraw to lend proposals depend on hard keeper. - app.communityKeeper = communitykeeper.NewKeeper( - appCodec, - keys[communitytypes.StoreKey], - app.accountKeeper, - app.bankKeeper, - &cdpKeeper, - app.distrKeeper, - &hardKeeper, - ) app.kavadistKeeper = kavadistkeeper.NewKeeper( appCodec, keys[kavadisttypes.StoreKey], @@ -665,6 +655,19 @@ func NewApp( authtypes.FeeCollectorName, ) + // x/community's deposit/withdraw to lend proposals depend on hard keeper. + app.communityKeeper = communitykeeper.NewKeeper( + appCodec, + keys[communitytypes.StoreKey], + app.accountKeeper, + app.bankKeeper, + &cdpKeeper, + app.distrKeeper, + &hardKeeper, + &app.mintKeeper, + &app.kavadistKeeper, + ) + app.incentiveKeeper = incentivekeeper.NewKeeper( appCodec, keys[incentivetypes.StoreKey], @@ -809,6 +812,9 @@ func NewApp( // Committee begin blocker changes module params by enacting proposals. // Run before to ensure params are updated together before state changes. committeetypes.ModuleName, + // Community begin blocker should run before x/mint and x/kavadist since + // the disable inflation upgrade will update those modules' params. + communitytypes.ModuleName, minttypes.ModuleName, distrtypes.ModuleName, // During begin block slashing happens after distr.BeginBlocker so that @@ -820,7 +826,6 @@ func NewApp( feemarkettypes.ModuleName, evmtypes.ModuleName, kavadisttypes.ModuleName, - communitytypes.ModuleName, // Auction begin blocker will close out expired auctions and pay debt back to cdp. // It should be run before cdp begin blocker which cancels out debt with stable and starts more auctions. auctiontypes.ModuleName, diff --git a/app/test_common.go b/app/test_common.go index 338633d9..e969743b 100644 --- a/app/test_common.go +++ b/app/test_common.go @@ -15,6 +15,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + storetypes "github.com/cosmos/cosmos-sdk/store/types" "github.com/cosmos/cosmos-sdk/testutil/mock" sdk "github.com/cosmos/cosmos-sdk/types" authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" @@ -111,7 +112,6 @@ func (tApp TestApp) GetDistrKeeper() distkeeper.Keeper { return tApp.di func (tApp TestApp) GetGovKeeper() govkeeper.Keeper { return tApp.govKeeper } func (tApp TestApp) GetCrisisKeeper() crisiskeeper.Keeper { return tApp.crisisKeeper } func (tApp TestApp) GetParamsKeeper() paramskeeper.Keeper { return tApp.paramsKeeper } - func (tApp TestApp) GetKavadistKeeper() kavadistkeeper.Keeper { return tApp.kavadistKeeper } func (tApp TestApp) GetAuctionKeeper() auctionkeeper.Keeper { return tApp.auctionKeeper } func (tApp TestApp) GetIssuanceKeeper() issuancekeeper.Keeper { return tApp.issuanceKeeper } @@ -131,6 +131,10 @@ func (tApp TestApp) GetEarnKeeper() earnkeeper.Keeper { return tApp.ea func (tApp TestApp) GetRouterKeeper() routerkeeper.Keeper { return tApp.routerKeeper } func (tApp TestApp) GetCommunityKeeper() communitykeeper.Keeper { return tApp.communityKeeper } +func (tApp TestApp) GetKVStoreKey(key string) *storetypes.KVStoreKey { + return tApp.keys[key] +} + // LegacyAmino returns the app's amino codec. func (app *App) LegacyAmino() *codec.LegacyAmino { return app.legacyAmino diff --git a/x/community/abci.go b/x/community/abci.go new file mode 100644 index 00000000..729a2547 --- /dev/null +++ b/x/community/abci.go @@ -0,0 +1,17 @@ +package community + +import ( + "time" + + "github.com/cosmos/cosmos-sdk/telemetry" + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/kava-labs/kava/x/community/keeper" + "github.com/kava-labs/kava/x/community/types" +) + +func BeginBlocker(ctx sdk.Context, k keeper.Keeper) { + defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) + + k.CheckAndDisableMintAndKavaDistInflation(ctx) +} diff --git a/x/community/disable_inflation_abci_test.go b/x/community/disable_inflation_abci_test.go new file mode 100644 index 00000000..67c387c7 --- /dev/null +++ b/x/community/disable_inflation_abci_test.go @@ -0,0 +1,19 @@ +package community_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" + + "github.com/kava-labs/kava/x/community" + "github.com/kava-labs/kava/x/community/keeper" + "github.com/kava-labs/kava/x/community/testutil" +) + +func TestABCIDisableInflation(t *testing.T) { + testFunc := func(ctx sdk.Context, k keeper.Keeper) { + community.BeginBlocker(ctx, k) + } + suite.Run(t, testutil.NewDisableInflationTestSuite(testFunc)) +} diff --git a/x/community/keeper/disable_inflation.go b/x/community/keeper/disable_inflation.go new file mode 100644 index 00000000..f921d376 --- /dev/null +++ b/x/community/keeper/disable_inflation.go @@ -0,0 +1,53 @@ +package keeper + +import ( + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// CheckAndDisableMintAndKavaDistInflation compares the disable inflation time and block time, +// and disables inflation if time is set and before block time. Inflation time is reset, +// so this method is safe to call more than once. +func (k Keeper) CheckAndDisableMintAndKavaDistInflation(ctx sdk.Context) { + params, found := k.GetParams(ctx) + if !found { + // panic since this can only be reached if chain state is corrupted or method is ran at an invalid height + panic("invalid state: module parameters not found") + } + + // if upgrade time is in the future or zero there is nothing to do, so return + if params.UpgradeTimeDisableInflation.IsZero() || params.UpgradeTimeDisableInflation.After(ctx.BlockTime()) { + return + } + + // run disable inflation logic + k.disableInflation(ctx) + + // reset disable inflation time to ensure next call is a no-op + params.UpgradeTimeDisableInflation = time.Time{} + k.SetParams(ctx, params) + +} + +// TODO: double check this is correct method for disabling inflation in kavadist without +// affecting rewards. In addition, inflation periods in kavadist should be removed. +func (k Keeper) disableInflation(ctx sdk.Context) { + logger := k.Logger(ctx) + logger.Info("disable inflation upgrade started") + + // set x/min inflation to 0 + mintParams := k.mintKeeper.GetParams(ctx) + mintParams.InflationMin = sdk.ZeroDec() + mintParams.InflationMax = sdk.ZeroDec() + k.mintKeeper.SetParams(ctx, mintParams) + logger.Info("x/mint inflation set to 0") + + // disable kavadist inflation + kavadistParams := k.kavadistKeeper.GetParams(ctx) + kavadistParams.Active = false + k.kavadistKeeper.SetParams(ctx, kavadistParams) + logger.Info("x/kavadist inflation disabled") + + logger.Info("disable inflation upgrade finished successfully!") +} diff --git a/x/community/keeper/disable_inflation_test.go b/x/community/keeper/disable_inflation_test.go new file mode 100644 index 00000000..94e19097 --- /dev/null +++ b/x/community/keeper/disable_inflation_test.go @@ -0,0 +1,18 @@ +package keeper_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" + + "github.com/kava-labs/kava/x/community/keeper" + "github.com/kava-labs/kava/x/community/testutil" +) + +func TestKeeperDisableInflation(t *testing.T) { + testFunc := func(ctx sdk.Context, k keeper.Keeper) { + k.CheckAndDisableMintAndKavaDistInflation(ctx) + } + suite.Run(t, testutil.NewDisableInflationTestSuite(testFunc)) +} diff --git a/x/community/keeper/keeper.go b/x/community/keeper/keeper.go index 89d69cfb..a9db95fe 100644 --- a/x/community/keeper/keeper.go +++ b/x/community/keeper/keeper.go @@ -16,11 +16,13 @@ type Keeper struct { key storetypes.StoreKey cdc codec.Codec - bankKeeper types.BankKeeper - cdpKeeper types.CdpKeeper - distrKeeper types.DistributionKeeper - hardKeeper types.HardKeeper - moduleAddress sdk.AccAddress + bankKeeper types.BankKeeper + cdpKeeper types.CdpKeeper + distrKeeper types.DistributionKeeper + hardKeeper types.HardKeeper + moduleAddress sdk.AccAddress + mintKeeper types.MintKeeper + kavadistKeeper types.KavadistKeeper legacyCommunityPoolAddress sdk.AccAddress } @@ -34,6 +36,8 @@ func NewKeeper( ck types.CdpKeeper, dk types.DistributionKeeper, hk types.HardKeeper, + mk types.MintKeeper, + kk types.KavadistKeeper, ) Keeper { // ensure community module account is set addr := ak.GetModuleAddress(types.ModuleAccountName) @@ -49,11 +53,13 @@ func NewKeeper( key: key, cdc: cdc, - bankKeeper: bk, - cdpKeeper: ck, - distrKeeper: dk, - hardKeeper: hk, - moduleAddress: addr, + bankKeeper: bk, + cdpKeeper: ck, + distrKeeper: dk, + hardKeeper: hk, + mintKeeper: mk, + kavadistKeeper: kk, + moduleAddress: addr, legacyCommunityPoolAddress: legacyAddr, } diff --git a/x/community/module.go b/x/community/module.go index ec409a2e..be697fc9 100644 --- a/x/community/module.go +++ b/x/community/module.go @@ -152,7 +152,9 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw } // BeginBlock module begin-block -func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} +func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { + BeginBlocker(ctx, am.keeper) +} // EndBlock module end-block func (am AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { diff --git a/x/community/testutil/disable_inflation.go b/x/community/testutil/disable_inflation.go new file mode 100644 index 00000000..21bb6c0f --- /dev/null +++ b/x/community/testutil/disable_inflation.go @@ -0,0 +1,156 @@ +package testutil + +import ( + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" + minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" + "github.com/stretchr/testify/suite" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + tmtime "github.com/tendermint/tendermint/types/time" + + "github.com/kava-labs/kava/app" + "github.com/kava-labs/kava/x/community" + "github.com/kava-labs/kava/x/community/keeper" + "github.com/kava-labs/kava/x/community/types" + kavadisttypes "github.com/kava-labs/kava/x/kavadist/types" +) + +type testFunc func(sdk.Context, keeper.Keeper) + +// Test suite used for all abci inflation tests +type disableInflationTestSuite struct { + suite.Suite + + App app.TestApp + Ctx sdk.Context + Keeper keeper.Keeper + + genesisMintState *minttypes.GenesisState + genesisKavadistState *kavadisttypes.GenesisState + + testFunc testFunc +} + +func NewDisableInflationTestSuite(tf testFunc) *disableInflationTestSuite { + suite := &disableInflationTestSuite{} + suite.testFunc = tf + return suite +} + +// The default state used by each test +func (suite *disableInflationTestSuite) SetupTest() { + app.SetSDKConfig() + tApp := app.NewTestApp() + suite.App = tApp + suite.Ctx = suite.App.NewContext(true, tmproto.Header{Height: 1, Time: tmtime.Now()}) + suite.Keeper = suite.App.GetCommunityKeeper() + + // Set up x/mint and x/kavadist gen state + mintGen := minttypes.DefaultGenesisState() + mintGen.Params.InflationMax = sdk.NewDecWithPrec(595, 3) + mintGen.Params.InflationMin = sdk.NewDecWithPrec(595, 3) + suite.genesisMintState = mintGen + + kavadistGen := kavadisttypes.DefaultGenesisState() + kavadistGen.Params.Active = true + suite.genesisKavadistState = kavadistGen + + appCodec := tApp.AppCodec() + suite.App.InitializeFromGenesisStates( + app.GenesisState{minttypes.ModuleName: appCodec.MustMarshalJSON(mintGen)}, + app.GenesisState{kavadisttypes.ModuleName: appCodec.MustMarshalJSON(kavadistGen)}, + ) +} + +func (suite *disableInflationTestSuite) TestDisableInflation() { + validateState := func(upgraded bool, expectedDisableTime time.Time, msg string) { + params, found := suite.Keeper.GetParams(suite.Ctx) + suite.Require().True(found) + mintParams := suite.App.GetMintKeeper().GetParams(suite.Ctx) + kavadistParams := suite.App.GetKavadistKeeper().GetParams(suite.Ctx) + + disableTimeMsg := "expected inflation disable time to match" + expectedMintState := suite.genesisMintState + expectedKavadistState := suite.genesisKavadistState + msgSuffix := "before upgrade" + + // The state expected after upgrade time is reached + if upgraded { + // Disable upgrade time is reset when run. + // + // This allows the time to be set and run again if required. + // In addition, with zero time not upgrading, achieves idempotence + // without extra logic or state. + expectedDisableTime = time.Time{} + disableTimeMsg = "expected inflation disable time to be reset" + + expectedMintState.Params.InflationMin = sdk.ZeroDec() + expectedMintState.Params.InflationMax = sdk.ZeroDec() + + expectedKavadistState.Params.Active = false + msgSuffix = "after upgrade" + } + + suite.Require().Equal(expectedMintState.Params.InflationMin, mintParams.InflationMin, msg+": expected mint inflation min to match state "+msgSuffix) + suite.Require().Equal(expectedMintState.Params.InflationMax, mintParams.InflationMax, msg+": expected mint inflation max to match state "+msgSuffix) + suite.Require().Equal(expectedKavadistState.Params.Active, kavadistParams.Active, msg+":expected kavadist active flag match state "+msgSuffix) + suite.Require().Equal(expectedDisableTime, params.UpgradeTimeDisableInflation, msg+": "+disableTimeMsg) + } + + blockTime := suite.Ctx.BlockTime() + testCases := []struct { + name string + upgradeTime time.Time + shouldUpgrade bool + }{ + {"zero upgrade time -- should not upgrade", time.Time{}, false}, + {"upgrade time in future -- should not upgrade", blockTime.Add(1 * time.Second), false}, + {"upgrade time in past -- should upgrade", blockTime.Add(-1 * time.Second), true}, + {"upgrade time equal to block time -- should upgrade", blockTime, true}, + } + + for _, tc := range testCases { + suite.Run(tc.name, func() { + suite.SetupTest() + // ensure state is as we expect before running upgrade or updating time + validateState(false, time.Time{}, "initial state") + + // set inflation disable time + params, found := suite.Keeper.GetParams(suite.Ctx) + suite.Require().True(found) + params.UpgradeTimeDisableInflation = tc.upgradeTime + suite.Keeper.SetParams(suite.Ctx, params) + + // run test function + suite.testFunc(suite.Ctx, suite.Keeper) + + // run assertions to ensure upgrade did or did not run + validateState(tc.shouldUpgrade, tc.upgradeTime, "first begin blocker run") + + // test idempotence only if upgrade should have been ran + if tc.shouldUpgrade { + // reset mint and kavadist state to their initial values + suite.App.GetMintKeeper().SetParams(suite.Ctx, suite.genesisMintState.Params) + suite.App.GetKavadistKeeper().SetParams(suite.Ctx, suite.genesisKavadistState.Params) + + // run begin blocker again + community.BeginBlocker(suite.Ctx, suite.Keeper) + + // ensure begin blocker is impodent and never runs twice + validateState(false, time.Time{}, "second begin blocker run") + } + }) + } +} + +func (suite *disableInflationTestSuite) TestPanicsOnMissingParameters() { + suite.SetupTest() + + store := suite.Ctx.KVStore(suite.App.GetKVStoreKey(types.StoreKey)) + store.Delete(types.ParamsKey) + + suite.PanicsWithValue("invalid state: module parameters not found", func() { + suite.testFunc(suite.Ctx, suite.Keeper) + }) +} diff --git a/x/community/types/expected_keepers.go b/x/community/types/expected_keepers.go index f642dfe0..a07788ff 100644 --- a/x/community/types/expected_keepers.go +++ b/x/community/types/expected_keepers.go @@ -3,6 +3,8 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" + kavadisttypes "github.com/kava-labs/kava/x/kavadist/types" ) // AccountKeeper defines the contract required for account APIs. @@ -37,3 +39,13 @@ type DistributionKeeper interface { FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error GetFeePoolCommunityCoins(ctx sdk.Context) sdk.DecCoins } + +type MintKeeper interface { + GetParams(ctx sdk.Context) (params minttypes.Params) + SetParams(ctx sdk.Context, params minttypes.Params) +} + +type KavadistKeeper interface { + GetParams(ctx sdk.Context) (params kavadisttypes.Params) + SetParams(ctx sdk.Context, params kavadisttypes.Params) +}