Move fork and upgrade logic into sub-directory structure (#680)

* Move fork logic into sub-directory structure

* Move upgrade handlers into sub-directories

* Move upgrade logic to end of keeper initialization

* Add v4 migration test

app/app.go

@@ -18,7 +18,6 @@ import (
 
 	ibcclient "github.com/cosmos/ibc-go/v2/modules/core/02-client"
 	ibcclienttypes "github.com/cosmos/ibc-go/v2/modules/core/02-client/types"
-	ibcconnectiontypes "github.com/cosmos/ibc-go/v2/modules/core/03-connection/types"
 
 	"github.com/cosmos/cosmos-sdk/x/authz"
 	paramproposal "github.com/cosmos/cosmos-sdk/x/params/types/proposal"
@@ -93,6 +92,8 @@ import (
 	"github.com/gorilla/mux"
 
 	appparams "github.com/osmosis-labs/osmosis/app/params"
+	v4 "github.com/osmosis-labs/osmosis/app/upgrades/v4"
+	v5 "github.com/osmosis-labs/osmosis/app/upgrades/v5"
 	_ "github.com/osmosis-labs/osmosis/client/docs/statik"
 	"github.com/osmosis-labs/osmosis/x/claim"
 	claimkeeper "github.com/osmosis-labs/osmosis/x/claim/keeper"
@@ -128,7 +129,6 @@ import (
 )
 
 const appName = "OsmosisApp"
-const v5UpgradeName = "v5"
 
 var (
 	// DefaultNodeHome default home directories for the application daemon
@@ -348,95 +348,6 @@ func NewOsmosisApp(
 		app.BaseApp,
 	)
 
-	// this configures a no-op upgrade handler for the v4 upgrade,
-	// which improves the lockup module's store management.
-	app.UpgradeKeeper.SetUpgradeHandler(
-		"v4", func(ctx sdk.Context, _plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
-			// // Upgrade all of the lock storages
-			// locks, err := app.LockupKeeper.GetLegacyPeriodLocks(ctx)
-			// if err != nil {
-			// 	panic(err)
-			// }
-			// // clear all lockup module locking / unlocking queue items
-			// app.LockupKeeper.ClearAllLockRefKeys(ctx)
-			// app.LockupKeeper.ClearAllAccumulationStores(ctx)
-
-			// // reset all lock and references
-			// if err := app.LockupKeeper.ResetAllLocks(ctx, locks); err != nil {
-			// 	panic(err)
-			// }
-
-			// // configure upgrade for gamm module's pool creation fee param add
-			// app.GAMMKeeper.SetParams(ctx, gammtypes.NewParams(sdk.Coins{sdk.NewInt64Coin("uosmo", 1)})) // 1 uOSMO
-			// // execute prop12. See implementation in
-			// prop12(ctx, app)
-			return vm, nil
-		})
-
-	app.UpgradeKeeper.SetUpgradeHandler(
-		v5UpgradeName,
-		func(ctx sdk.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
-			// Set IBC updates from {inside SDK} to v1
-			// https://github.com/cosmos/ibc-go/blob/main/docs/migrations/ibc-migration-043.md#in-place-store-migrations
-			app.IBCKeeper.ConnectionKeeper.SetParams(ctx, ibcconnectiontypes.DefaultParams())
-
-			totalLiquidity := app.GAMMKeeper.GetLegacyTotalLiquidity(ctx)
-			app.GAMMKeeper.DeleteLegacyTotalLiquidity(ctx)
-			app.GAMMKeeper.SetTotalLiquidity(ctx, totalLiquidity)
-
-			// Set all modules "old versions" to 1.
-			// Then the run migrations logic will handle running their upgrade logics
-			fromVM := make(map[string]uint64)
-			for moduleName := range app.mm.Modules {
-				fromVM[moduleName] = 1
-			}
-			// EXCEPT Auth needs to run _after_ staking (https://github.com/cosmos/cosmos-sdk/issues/10591),
-			// and it seems bank as well (https://github.com/provenance-io/provenance/blob/407c89a7d73854515894161e1526f9623a94c368/app/upgrades.go#L86-L122).
-			// So we do this by making auth run last.
-			// This is done by setting auth's consensus version to 2, running RunMigrations,
-			// then setting it back to 1, and then running migrations again.
-			fromVM[authtypes.ModuleName] = 2
-
-			// override versions for authz & bech32ibctypes module as to not skip their InitGenesis
-			// for txfees module, we will override txfees ourselves.
-			delete(fromVM, authz.ModuleName)
-			delete(fromVM, bech32ibctypes.ModuleName)
-
-			newVM, err := app.mm.RunMigrations(ctx, app.configurator, fromVM)
-			if err != nil {
-				return nil, err
-			}
-
-			// Override txfees genesis here
-			ctx.Logger().Info("Setting txfees module genesis with actual v5 desired genesis")
-			feeTokens := initialWhitelistedFeetokens(ctx, app)
-			txfees.InitGenesis(ctx, app.TxFeesKeeper, txfeestypes.GenesisState{
-				Basedenom: app.StakingKeeper.BondDenom(ctx),
-				Feetokens: feeTokens,
-			})
-
-			// now update auth version back to v1, to run auth migration last
-			newVM[authtypes.ModuleName] = 1
-
-			ctx.Logger().Info("Now running migrations just for auth, to get auth migration to be last. " +
-				"(CC https://github.com/cosmos/cosmos-sdk/issues/10591)")
-			return app.mm.RunMigrations(ctx, app.configurator, newVM)
-		})
-
-	upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
-	if err != nil {
-		panic(fmt.Sprintf("failed to read upgrade info from disk %s", err))
-	}
-
-	if upgradeInfo.Name == v5UpgradeName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
-		storeUpgrades := store.StoreUpgrades{
-			Added: []string{authz.ModuleName, txfees.ModuleName, bech32ibctypes.ModuleName},
-		}
-
-		// configure store loader that checks if version == upgradeHeight and applies store upgrades
-		app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades))
-	}
-
 	// Create IBC Keeper
 	app.IBCKeeper = ibckeeper.NewKeeper(
 		appCodec,
@@ -481,6 +392,8 @@ func NewOsmosisApp(
 
 	app.ClaimKeeper = claimkeeper.NewKeeper(appCodec, keys[claimtypes.StoreKey], app.AccountKeeper, app.BankKeeper, stakingKeeper, app.DistrKeeper)
 
+	app.setupUpgrades()
+
 	// register the staking hooks
 	// NOTE: stakingKeeper above is passed by reference, so that it will contain these hooks
 	app.StakingKeeper = *stakingKeeper.SetHooks(
@@ -732,7 +645,7 @@ func (app *OsmosisApp) Name() string { return app.BaseApp.Name() }
 
 // BeginBlocker application updates every begin block
 func (app *OsmosisApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
-	forks(ctx, app)
+	BeginBlockForks(ctx, app)
 	return app.mm.BeginBlock(ctx, req)
 }
 
@@ -899,6 +812,37 @@ func (app *OsmosisApp) RegisterTendermintService(clientCtx client.Context) {
 	tmservice.RegisterTendermintService(app.BaseApp.GRPCQueryRouter(), clientCtx, app.interfaceRegistry)
 }
 
+// RegisterTendermintService implements the Application.RegisterTendermintService method.
+func (app *OsmosisApp) setupUpgrades() {
+	// this configures a no-op upgrade handler for the v4 upgrade,
+	// which improves the lockup module's store management.
+	app.UpgradeKeeper.SetUpgradeHandler(
+		v4.UpgradeName, v4.CreateUpgradeHandler(
+			app.mm, app.configurator,
+			&app.BankKeeper, &app.DistrKeeper, &app.GAMMKeeper))
+
+	app.UpgradeKeeper.SetUpgradeHandler(
+		v5.UpgradeName,
+		v5.CreateUpgradeHandler(
+			app.mm, app.configurator,
+			&app.IBCKeeper.ConnectionKeeper, &app.TxFeesKeeper,
+			&app.GAMMKeeper, &app.StakingKeeper))
+
+	upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
+	if err != nil {
+		panic(fmt.Sprintf("failed to read upgrade info from disk %s", err))
+	}
+
+	if upgradeInfo.Name == v5.UpgradeName && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
+		storeUpgrades := store.StoreUpgrades{
+			Added: []string{authz.ModuleName, txfees.ModuleName, bech32ibctypes.ModuleName},
+		}
+
+		// configure store loader that checks if version == upgradeHeight and applies store upgrades
+		app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades))
+	}
+}
+
 // RegisterSwaggerAPI registers swagger route with API Server
 func RegisterSwaggerAPI(ctx client.Context, rtr *mux.Router) {
 	statikFS, err := fs.New()

app/forks.go

@@ -2,48 +2,19 @@ package app
 
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	v3 "github.com/osmosis-labs/osmosis/app/upgrades/v3"
+	v6 "github.com/osmosis-labs/osmosis/app/upgrades/v6"
 )
 
-func forks(ctx sdk.Context, app *OsmosisApp) {
+// BeginBlockForks is intended to be ran in
+func BeginBlockForks(ctx sdk.Context, app *OsmosisApp) {
 	switch ctx.BlockHeight() {
-	case 712000:
-		fix_min_deposit_denom(ctx, app)
-		fix_min_commision_rate(ctx, app)
-	case 2464000:
-		ctx.Logger().Info("Applying emergency hard fork for v6, allows IBC to create new channels.")
+	case v3.UpgradeHeight:
+		v3.RunForkLogic(ctx, &app.GovKeeper, &app.StakingKeeper)
+	case v6.UpgradeHeight:
+		v6.RunForkLogic(ctx)
 	default:
 		// do nothing
 		return
 	}
 }
-
-// Fixes an error where minimum deposit was set to "500 osmo"
-// This denom does not exist, which makes it impossible for a proposal to go to a vote
-func fix_min_deposit_denom(ctx sdk.Context, app *OsmosisApp) {
-	var params = app.GovKeeper.GetDepositParams(ctx)
-	params.MinDeposit = sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(500000000)))
-	app.GovKeeper.SetDepositParams(ctx, params)
-}
-
-// Fixes an error where validators can be created with a commission rate
-// less than the network minimum rate.
-func fix_min_commision_rate(ctx sdk.Context, app *OsmosisApp) {
-	// Upgrade every validators min-commission rate
-	validators := app.StakingKeeper.GetAllValidators(ctx)
-	minCommissionRate := app.StakingKeeper.GetParams(ctx).MinCommissionRate
-	for _, v := range validators {
-		if v.Commission.Rate.LT(minCommissionRate) {
-			comm, err := app.StakingKeeper.MustUpdateValidatorCommission(
-				ctx, v, minCommissionRate)
-			if err != nil {
-				panic(err)
-			}
-			v.Commission = comm
-
-			// call the before-modification hook since we're about to update the commission
-			app.StakingKeeper.BeforeValidatorModified(ctx, v.GetOperator())
-
-			app.StakingKeeper.SetValidator(ctx, v)
-		}
-	}
-}

app/upgrades/README.md

@@ -0,0 +1,11 @@
+# Osmosis Upgrades
+
+This folder contains logic for every osmosis upgrade. (Both state migrations, and hard forks)
+
+* v1 - Initial version
+* v3 - short blurb on prop19 and the fork
+* v4 - Berylium State Migration
+* v5 - Boron State migration
+* v6 - hard fork for IBC bug fix
+
+## TODO: Make a fork-upgrade struct and a state-migration upgrade struct

app/upgrades/v3/README.md

@@ -0,0 +1,3 @@
+# TODO Write stuff
+
+Should include description about this version, compatibility with v1 until height {...}, and fork code here.

app/upgrades/v3/constants.go

@@ -0,0 +1,4 @@
+package v3
+
+const UpgradeName = "v3"
+const UpgradeHeight = 712000

app/upgrades/v3/forks.go

@@ -0,0 +1,46 @@
+package v3
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
+	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
+)
+
+func RunForkLogic(ctx sdk.Context, gov *govkeeper.Keeper, staking *stakingkeeper.Keeper) {
+	ctx.Logger().Info("Applying Osmosis v3 upgrade." +
+		" Fixing governance deposit so proposals can be voted upon," +
+		" and fixing validator min commission rate.")
+	FixMinDepositDenom(ctx, gov)
+	FixMinCommisionRate(ctx, staking)
+}
+
+// Fixes an error where minimum deposit was set to "500 osmo"
+// This denom does not exist, which makes it impossible for a proposal to go to a vote
+func FixMinDepositDenom(ctx sdk.Context, gov *govkeeper.Keeper) {
+	var params = gov.GetDepositParams(ctx)
+	params.MinDeposit = sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(500000000)))
+	gov.SetDepositParams(ctx, params)
+}
+
+// Fixes an error where validators can be created with a commission rate
+// less than the network minimum rate.
+func FixMinCommisionRate(ctx sdk.Context, staking *stakingkeeper.Keeper) {
+	// Upgrade every validators min-commission rate
+	validators := staking.GetAllValidators(ctx)
+	minCommissionRate := staking.GetParams(ctx).MinCommissionRate
+	for _, v := range validators {
+		if v.Commission.Rate.LT(minCommissionRate) {
+			comm, err := staking.MustUpdateValidatorCommission(
+				ctx, v, minCommissionRate)
+			if err != nil {
+				panic(err)
+			}
+			v.Commission = comm
+
+			// call the before-modification hook since we're about to update the commission
+			staking.BeforeValidatorModified(ctx, v.GetOperator())
+
+			staking.SetValidator(ctx, v)
+		}
+	}
+}

app/upgrades/v4/constants.go

@@ -0,0 +1,3 @@
+package v4
+
+const UpgradeName = "v4"

app/prop12.go → app/upgrades/v4/prop12.go

@@ -1,14 +1,16 @@
-package app
+package v4
 
 import (
 	"strconv"
 	"strings"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
-)
 
-func prop12(ctx sdk.Context, app *OsmosisApp) {
+	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
+	distrkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper"
+)
 
+func Prop12(ctx sdk.Context, bank *bankkeeper.Keeper, distr *distrkeeper.Keeper) {
 	payments := GetProp12Payments()
 
 	var total = int64(0)
@@ -23,15 +25,15 @@ func prop12(ctx sdk.Context, app *OsmosisApp) {
 			panic(err)
 		}
 		coins := sdk.NewCoins(sdk.NewInt64Coin("uosmo", amount))
-		if err := app.BankKeeper.SendCoinsFromModuleToAccount(ctx, "distribution", addr, coins); err != nil {
+		if err := (*bank).SendCoinsFromModuleToAccount(ctx, "distribution", addr, coins); err != nil {
 			panic(err)
 		}
 		total += amount
 	}
 
 	//deduct from the feePool tracker
-	feePool := app.DistrKeeper.GetFeePool(ctx)
+	feePool := distr.GetFeePool(ctx)
 	feePool.CommunityPool = feePool.CommunityPool.Sub(sdk.NewDecCoins(sdk.NewInt64DecCoin("uosmo", total)))
-	app.DistrKeeper.SetFeePool(ctx, feePool)
+	distr.SetFeePool(ctx, feePool)
 
 }

app/prop12_data.go → app/upgrades/v4/prop12_data.go

@@ -1,4 +1,4 @@
-package app
+package v4
 
 import (
 	"encoding/csv"