Collin/v8 migration fixes (#324)

* Add missing SDK 0.47 upgrade steps

* Remove msg event boilerplate replace in 0.47

* Add missing param migrations and additional IBC client type

* Fixing cellarfees params migration

* More cellarfees params fixing

* Even more cellarfees params fixing

* Hopefully last cellarfees params fix

* Hopefully the last last cellarfees fix

* Remove write to old cellarfees param subspace

* Use correct method to retrieve legacy cellarfees subspace

* Add missing IBC migration stuff

* Test commit for debugging

* More debugging

* Please

* Debugging

* AHHHH

* just trying stuff

* .

* ..

* Trying with legacySubspace passed in via app module, and deleting param by key

* Fix method call

* Fix method call

* Move upgrade handler setup after store mount

* Hardcode cellarfees params

* Incentives params test

* Cellarfees params migration fix test

* Clean up auction params migration

* Explicitly set new auction module account permissions

* Fix axelarcork export genesis bug that results in null values for CorkResults and ScheduledCorks

* Actually add the upgrade fix...

* Move auction permission to end of upgrade handler

* Wow.

* Tweak auction test

* Fix params validation in cellarfees

app/app.go

@@ -66,6 +66,7 @@ import (
 	govclient "github.com/cosmos/cosmos-sdk/x/gov/client"
 	govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+	govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
 	govtypesv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
 	"github.com/cosmos/cosmos-sdk/x/mint"
 	mintkeeper "github.com/cosmos/cosmos-sdk/x/mint/keeper"
@@ -609,7 +610,7 @@ func NewSommelierApp(
 		authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry),
 		cork.NewAppModule(app.CorkKeeper, appCodec),
 		incentives.NewAppModule(app.IncentivesKeeper, app.DistrKeeper, app.BankKeeper, app.MintKeeper, appCodec),
-		cellarfees.NewAppModule(app.CellarFeesKeeper, appCodec, app.AccountKeeper, app.BankKeeper, app.MintKeeper, app.CorkKeeper, app.AuctionKeeper),
+		cellarfees.NewAppModule(app.CellarFeesKeeper, appCodec, app.AccountKeeper, app.BankKeeper, app.MintKeeper, app.CorkKeeper, app.AuctionKeeper, app.GetSubspace(cellarfeestypes.ModuleName)),
 		auction.NewAppModule(app.AuctionKeeper, app.BankKeeper, app.AccountKeeper, appCodec),
 		pubsub.NewAppModule(appCodec, app.PubsubKeeper, app.StakingKeeper, app.GravityKeeper),
 		addresses.NewAppModule(appCodec, app.AddressesKeeper),
@@ -750,7 +751,7 @@ func NewSommelierApp(
 		authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry),
 		cork.NewAppModule(app.CorkKeeper, appCodec),
 		incentives.NewAppModule(app.IncentivesKeeper, app.DistrKeeper, app.BankKeeper, app.MintKeeper, appCodec),
-		cellarfees.NewAppModule(app.CellarFeesKeeper, appCodec, app.AccountKeeper, app.BankKeeper, app.MintKeeper, app.CorkKeeper, app.AuctionKeeper),
+		cellarfees.NewAppModule(app.CellarFeesKeeper, appCodec, app.AccountKeeper, app.BankKeeper, app.MintKeeper, app.CorkKeeper, app.AuctionKeeper, app.GetSubspace(cellarfeestypes.ModuleName)),
 		auction.NewAppModule(app.AuctionKeeper, app.BankKeeper, app.AccountKeeper, appCodec),
 		pubsub.NewAppModule(appCodec, app.PubsubKeeper, app.StakingKeeper, app.GravityKeeper),
 		addresses.NewAppModule(appCodec, app.AddressesKeeper),
@@ -1038,6 +1039,39 @@ func (app *SommelierApp) setupUpgradeStoreLoaders() {
 }
 
 func (app *SommelierApp) setupUpgradeHandlers() {
+	// Set param key table for params module migration
+	for _, subspace := range app.ParamsKeeper.GetSubspaces() {
+		subspace := subspace
+		found := true
+		var keyTable paramstypes.KeyTable
+		switch subspace.Name() {
+		case authtypes.ModuleName:
+			keyTable = authtypes.ParamKeyTable() //nolint: staticcheck // deprecated but required for upgrade
+		case banktypes.ModuleName:
+			keyTable = banktypes.ParamKeyTable() //nolint: staticcheck // deprecated but required for upgrade
+		case stakingtypes.ModuleName:
+			keyTable = stakingtypes.ParamKeyTable()
+		case minttypes.ModuleName:
+			keyTable = minttypes.ParamKeyTable() //nolint: staticcheck // deprecated but required for upgrade
+		case distrtypes.ModuleName:
+			keyTable = distrtypes.ParamKeyTable() //nolint: staticcheck // deprecated but required for upgrade
+		case slashingtypes.ModuleName:
+			keyTable = slashingtypes.ParamKeyTable() //nolint: staticcheck // deprecated but required for upgrade
+		case govtypes.ModuleName:
+			keyTable = govtypesv1.ParamKeyTable() //nolint: staticcheck // deprecated but required for upgrade
+		case crisistypes.ModuleName:
+			keyTable = crisistypes.ParamKeyTable() //nolint: staticcheck // deprecated but required for upgrade
+		case ibctransfertypes.ModuleName:
+			keyTable = ibctransfertypes.ParamKeyTable()
+		default:
+			// subspace not handled
+			found = false
+		}
+		if found && !subspace.HasKeyTable() {
+			subspace.WithKeyTable(keyTable)
+		}
+	}
+
 	baseAppLegacySS := app.ParamsKeeper.Subspace(baseapp.Paramspace).WithKeyTable(paramstypes.ConsensusParamsKeyTable())
 
 	// TODO: Add v8 upgrade handle
@@ -1048,6 +1082,10 @@ func (app *SommelierApp) setupUpgradeHandlers() {
 			app.configurator,
 			&baseAppLegacySS,
 			&app.ConsensusParamsKeeper,
+			app.IBCKeeper,
+			app.appCodec,
+			app.IBCKeeper.ClientKeeper,
+			&app.AccountKeeper,
 		),
 	)
 }

app/upgrades/v8/upgrades.go

@@ -1,26 +1,83 @@
 package v8
 
 import (
+	"fmt"
+
 	"github.com/cosmos/cosmos-sdk/baseapp"
+	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
+	authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	consensusparamkeeper "github.com/cosmos/cosmos-sdk/x/consensus/keeper"
 	paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
+	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
+	ibckeeper "github.com/cosmos/ibc-go/v7/modules/core/keeper"
+	ibctmmigrations "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint/migrations"
+	auctiontypes "github.com/peggyjv/sommelier/v8/x/auction/types"
 )
 
 func CreateUpgradeHandler(
 	mm *module.Manager,
 	configurator module.Configurator,
 	baseAppLegacySS *paramstypes.Subspace,
 	consensusParamsKeeper *consensusparamkeeper.Keeper,
+	ibcKeeper *ibckeeper.Keeper,
+	cdc codec.BinaryCodec,
+	clientKeeper ibctmmigrations.ClientKeeper,
+	accountKeeper *authkeeper.AccountKeeper,
 ) upgradetypes.UpgradeHandler {
 	return func(ctx sdk.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
 		ctx.Logger().Info("v8 upgrade: entering handler and running migrations")
 
+		// Include this when migrating to ibc-go v7 (optional)
+		// source: https://github.com/cosmos/ibc-go/blob/v7.2.0/docs/migrations/v6-to-v7.md
+		// prune expired tendermint consensus states to save storage space
+		if _, err := ibctmmigrations.PruneExpiredConsensusStates(ctx, cdc, clientKeeper); err != nil {
+			return nil, err
+		}
+
 		// new x/consensus module params migration
 		baseapp.MigrateParams(ctx, baseAppLegacySS, consensusParamsKeeper)
 
-		return mm.RunMigrations(ctx, configurator, vm)
+		// explicitly update the IBC 02-client params, adding the localhost client type
+		params := ibcKeeper.ClientKeeper.GetParams(ctx)
+		params.AllowedClients = append(params.AllowedClients, ibcexported.Localhost)
+		ibcKeeper.ClientKeeper.SetParams(ctx, params)
+
+		vm, err := mm.RunMigrations(ctx, configurator, vm)
+		if err != nil {
+			return nil, err
+		}
+
+		// add burner permission to auction account
+		if err := MigrateAuctionAccountPermissions(ctx, accountKeeper); err != nil {
+			return nil, err
+		}
+
+		return vm, nil
 	}
 }
+
+func MigrateAuctionAccountPermissions(ctx sdk.Context, accountKeeper *authkeeper.AccountKeeper) error {
+	ctx.Logger().Info("Migrating auction account permissions")
+	oldAcctI := accountKeeper.GetModuleAccount(ctx, auctiontypes.ModuleName)
+
+	if oldAcctI == nil {
+		return fmt.Errorf("module account not found")
+	}
+
+	newAcct := authtypes.NewEmptyModuleAccount(auctiontypes.ModuleName, authtypes.Burner)
+	newAcct.AccountNumber = oldAcctI.GetAccountNumber()
+	newAcct.Address = oldAcctI.GetAddress().String()
+	newAcct.Sequence = oldAcctI.GetSequence()
+	newAcct.Name = oldAcctI.GetName()
+	newAcctI := (accountKeeper.NewAccount(ctx, newAcct)).(authtypes.ModuleAccountI)
+
+	accountKeeper.SetModuleAccount(ctx, newAcctI)
+
+	ctx.Logger().Info("Auction account permissions migrated")
+
+	return nil
+}

integration_tests/auction_test.go

@@ -315,6 +315,7 @@ func (s *IntegrationTestSuite) TestAuction() {
 
 		// Calculate the expected burn amount (50% of total SOMM paid in auction)
 		expectedBurnAmount := totalSommPaid.Quo(sdk.NewInt(2))
+		s.Require().NotZero(expectedBurnAmount.Int64(), "Expected burn amount should not be zero")
 
 		// Calculate the expected new total supply
 		expectedNewSupply := supplyRes.Amount.Amount.Sub(expectedBurnAmount)
@@ -326,6 +327,10 @@ func (s *IntegrationTestSuite) TestAuction() {
 		// Verify that the new supply matches the expected new supply
 		s.Require().Equal(expectedNewSupply.Int64(), newSupplyRes.Amount.Amount.Int64(), "Total supply of usomm should be reduced by the amount burnt")
 
+		// Get auction module account
+		auctionModuleAccount := authtypes.NewModuleAddress(types.ModuleName)
+		s.T().Logf("Auction module account: %s", auctionModuleAccount.String())
+
 		s.T().Log("Total supply of usomm has been correctly reduced!")
 
 		s.T().Log("--Test completed successfully--")

x/auction/keeper/migrations.go

@@ -2,7 +2,7 @@ package keeper
 
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	v1 "github.com/peggyjv/sommelier/v8/x/auction/migrations/v1"
+	"github.com/peggyjv/sommelier/v8/x/auction/types"
 )
 
 // Migrator is a struct for handling in-place store migrations.
@@ -17,5 +17,13 @@ func NewMigrator(keeper Keeper) Migrator {
 
 // Migrate1to2 migrates from consensus version 1 to 2.
 func (m Migrator) Migrate1to2(ctx sdk.Context) error {
-	return v1.MigrateParamStore(ctx, m.keeper.paramSpace)
+	ctx.Logger().Info("auction v1 to v2: New params")
+	subspace := m.keeper.paramSpace
+
+	if !subspace.Has(ctx, types.KeyAuctionBurnRate) {
+		subspace.Set(ctx, types.KeyAuctionBurnRate, types.DefaultParams().AuctionBurnRate)
+	}
+
+	ctx.Logger().Info("auction v1 to v2: Params migration complete")
+	return nil
 }