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 <nickdeluca08@gmail.com>

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

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,

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

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)
+}

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))
+}

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!")
+}

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))
+}

x/community/keeper/keeper.go

@@ -21,6 +21,8 @@ type Keeper struct {
 	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)
@@ -53,6 +57,8 @@ func NewKeeper(
 		cdpKeeper:      ck,
 		distrKeeper:    dk,
 		hardKeeper:     hk,
+		mintKeeper:     mk,
+		kavadistKeeper: kk,
 		moduleAddress:  addr,
 
 		legacyCommunityPoolAddress: legacyAddr,

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 {

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)
+	})
+}

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)
+}