Fix the umber upgrades.

CHANGELOG.md

@@ -120,6 +120,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * The `add-net-asset-values` command now correctly uses the from `flag`'s `AccAddress` [#1995](https://github.com/provenance-io/provenance/issues/1995).
 * Fix the sim tests [#2015](https://github.com/provenance-io/provenance/pull/2015).
+* Fix the `umber` and `umber-rc1` upgrades [#2033](https://github.com/provenance-io/provenance/pull/2033).
 
 ### Deprecated
 

app/app.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/CosmWasm/wasmd/x/wasm"
 	wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
+	wasmv2 "github.com/CosmWasm/wasmd/x/wasm/migrations/v2"
 	wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
 	"github.com/gorilla/mux"
 	"github.com/rakyll/statik/fs"
@@ -92,6 +93,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/group"
 	groupkeeper "github.com/cosmos/cosmos-sdk/x/group/keeper"
@@ -332,13 +334,14 @@ func New(
 	logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool,
 	appOpts servertypes.AppOptions, baseAppOptions ...func(*baseapp.BaseApp),
 ) *App {
+	sdkConfig := sdk.GetConfig()
+	addrPrefix := sdkConfig.GetBech32AccountAddrPrefix()
+	valAddrPrefix := sdkConfig.GetBech32ValidatorAddrPrefix()
+	consAddrPrefix := sdkConfig.GetBech32ConsensusAddrPrefix()
+
 	signingOptions := signing.Options{
-		AddressCodec: address.Bech32Codec{
-			Bech32Prefix: sdk.GetConfig().GetBech32AccountAddrPrefix(),
-		},
-		ValidatorAddressCodec: address.Bech32Codec{
-			Bech32Prefix: sdk.GetConfig().GetBech32ValidatorAddrPrefix(),
-		},
+		AddressCodec:          address.Bech32Codec{Bech32Prefix: addrPrefix},
+		ValidatorAddressCodec: address.Bech32Codec{Bech32Prefix: valAddrPrefix},
 	}
 	exchange.DefineCustomGetSigners(&signingOptions)
 	interfaceRegistry, _ := types.NewInterfaceRegistryWithOptions(types.InterfaceRegistryOptions{
@@ -359,7 +362,6 @@ func New(
 	bApp.SetInterfaceRegistry(interfaceRegistry)
 	sdk.SetCoinDenomRegex(SdkCoinDenomRegex)
 
-	// TODO[1760]: Add the circuit breaker module to the app.
 	keys := storetypes.NewKVStoreKeys(
 		authtypes.StoreKey, banktypes.StoreKey, stakingtypes.StoreKey,
 		minttypes.StoreKey, distrtypes.StoreKey, slashingtypes.StoreKey,
@@ -434,13 +436,8 @@ func New(
 	// capability keeper must be sealed after scope to module registrations are completed.
 	app.CapabilityKeeper.Seal()
 
-	// Obtain prefixes so running tests can use "cosmos" while we use "pb"
-	addrPrefix := sdk.GetConfig().GetBech32AccountAddrPrefix()
-	valAddrPrefix := sdk.GetConfig().GetBech32ValidatorAddrPrefix()
-	consAddrPrefix := sdk.GetConfig().GetBech32ConsensusAddrPrefix()
-
 	// add keepers
-	app.AccountKeeper = authkeeper.NewAccountKeeper(appCodec, runtime.NewKVStoreService(keys[authtypes.StoreKey]), authtypes.ProtoBaseAccount, maccPerms, authcodec.NewBech32Codec(addrPrefix), addrPrefix, govAuthority)
+	app.AccountKeeper = authkeeper.NewAccountKeeper(appCodec, runtime.NewKVStoreService(keys[authtypes.StoreKey]), authtypes.ProtoBaseAccount, maccPerms, signingOptions.AddressCodec, addrPrefix, govAuthority)
 
 	app.BankKeeper = bankkeeper.NewBaseKeeper(
 		appCodec,
@@ -957,11 +954,6 @@ func New(
 		panic(err)
 	}
 
-	// Register upgrade handlers and set the store loader.
-	// This must be done after the module manager and configurator are set,
-	// but before the baseapp is sealed via LoadLatestVersion() down below.
-	app.registerUpgradeHandlers(appOpts)
-
 	autocliv1.RegisterQueryServer(app.GRPCQueryRouter(), runtimeservices.NewAutoCLIQueryService(app.mm.Modules))
 
 	reflectionSvc, err := runtimeservices.NewReflectionService()
@@ -993,6 +985,11 @@ func New(
 	app.setFeeHandler()
 	app.SetAggregateEventsFunc(piohandlers.AggregateEvents)
 
+	// Register upgrade handlers and set the store loader.
+	// This must be done after the module manager, configurator, and pre-blocker are set,
+	// but before the baseapp is sealed via LoadLatestVersion() below.
+	app.registerUpgradeHandlers(appOpts)
+
 	if loadLatest {
 		if err := app.LoadLatestVersion(); err != nil {
 			cmtos.Exit(err.Error())
@@ -1346,31 +1343,33 @@ func GetMaccPerms() map[string][]string {
 func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino, key, tkey storetypes.StoreKey) paramskeeper.Keeper {
 	paramsKeeper := paramskeeper.NewKeeper(appCodec, legacyAmino, key, tkey)
 
-	paramsKeeper.Subspace(authtypes.ModuleName)
+	//nolint:staticcheck // The ParamKeyTable functions are deprecated, but needed here for the migrations.
+	paramsKeeper.Subspace(authtypes.ModuleName).WithKeyTable(authtypes.ParamKeyTable())
 	paramsKeeper.Subspace(banktypes.ModuleName)
-	paramsKeeper.Subspace(stakingtypes.ModuleName)
-	paramsKeeper.Subspace(minttypes.ModuleName)
-	paramsKeeper.Subspace(distrtypes.ModuleName)
-	paramsKeeper.Subspace(slashingtypes.ModuleName)
-	paramsKeeper.Subspace(govtypes.ModuleName)
-	paramsKeeper.Subspace(crisistypes.ModuleName)
+	paramsKeeper.Subspace(stakingtypes.ModuleName).WithKeyTable(stakingtypes.ParamKeyTable())   //nolint:staticcheck
+	paramsKeeper.Subspace(minttypes.ModuleName).WithKeyTable(minttypes.ParamKeyTable())         //nolint:staticcheck
+	paramsKeeper.Subspace(distrtypes.ModuleName).WithKeyTable(distrtypes.ParamKeyTable())       //nolint:staticcheck
+	paramsKeeper.Subspace(slashingtypes.ModuleName).WithKeyTable(slashingtypes.ParamKeyTable()) //nolint:staticcheck
+	paramsKeeper.Subspace(govtypes.ModuleName).WithKeyTable(govtypesv1.ParamKeyTable())         //nolint:staticcheck
+	paramsKeeper.Subspace(crisistypes.ModuleName).WithKeyTable(crisistypes.ParamKeyTable())     //nolint:staticcheck
 
-	paramsKeeper.Subspace(wasmtypes.ModuleName)
+	paramsKeeper.Subspace(wasmtypes.ModuleName).WithKeyTable(wasmv2.ParamKeyTable()) //nolint:staticcheck
 
 	// register the key tables for legacy param subspaces
 	keyTable := ibcclienttypes.ParamKeyTable()
 	keyTable.RegisterParamSet(&ibcconnectiontypes.Params{})
 	paramsKeeper.Subspace(ibcexported.ModuleName).WithKeyTable(keyTable)
 	paramsKeeper.Subspace(ibctransfertypes.ModuleName).WithKeyTable(ibctransfertypes.ParamKeyTable())
 	paramsKeeper.Subspace(icahosttypes.SubModuleName).WithKeyTable(icahosttypes.ParamKeyTable())
+	paramsKeeper.Subspace(icqtypes.ModuleName).WithKeyTable(icqtypes.ParamKeyTable())
 
 	return paramsKeeper
 }
 
 // injectUpgrade causes the named upgrade to be run as the chain starts.
 //
-// To use this, add a call to it in New after the call to InstallCustomUpgradeHandlers
-// but before the line that looks for an upgrade file.
+// To use this, add a call to it in registerUpgradeHandlers immediately after the
+// call to InstallCustomUpgradeHandlers, but before the line that looks for an upgrade file.
 //
 // This function is for testing an upgrade against an existing chain's data (e.g. mainnet).
 // Here's how:
@@ -1401,15 +1400,16 @@ func (app *App) injectUpgrade(name string) { //nolint:unused // This is designed
 	if err := app.UpgradeKeeper.DumpUpgradeInfoToDisk(plan.Height, plan); err != nil {
 		panic(err)
 	}
-	// Define a new BeginBlocker that will inject the upgrade.
+
+	// Define a new PreBlocker that will inject the upgrade.
 	injected := false
-	app.SetBeginBlocker(func(ctx sdk.Context) (sdk.BeginBlock, error) {
+	app.SetPreBlocker(func(ctx sdk.Context, req *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) {
 		if !injected {
 			app.Logger().Info("Injecting upgrade plan", "plan", plan)
 			// Ideally, we'd just call ScheduleUpgrade(ctx, plan) here (and panic on error).
-			// But the upgrade keeper often its own migration stuff that change some store key stuff.
-			// ScheduleUpgrade tries to read some of that changed store stuff and fails if the migration hasn't
-			// been applied yet. So we're doing things the hard way here.
+			// But the upgrade keeper often has its own migration stuff that change some store stuff.
+			// ScheduleUpgrade would try to read some of that old state using the update pattern,
+			// causing a failure. So we're doing things the hard way here.
 			if err := app.UpgradeKeeper.ClearUpgradePlan(ctx); err != nil {
 				panic(err)
 			}
@@ -1418,6 +1418,6 @@ func (app *App) injectUpgrade(name string) { //nolint:unused // This is designed
 			store.Set(upgradetypes.PlanKey(), bz)
 			injected = true
 		}
-		return app.BeginBlocker(ctx)
+		return app.PreBlocker(ctx, req)
 	})
 }

app/prefix.go

@@ -26,23 +26,25 @@ var (
 
 // SetConfig sets the configuration for the network using mainnet or testnet
 func SetConfig(testnet bool, seal bool) {
-	// not the default (mainnet) so reset with testnet config
+	AccountAddressPrefix = AccountAddressPrefixMainNet
+	CoinType = CoinTypeMainNet
 	if testnet {
 		AccountAddressPrefix = AccountAddressPrefixTestNet
-		AccountPubKeyPrefix = AccountAddressPrefixTestNet + "pub"
-		ValidatorAddressPrefix = AccountAddressPrefixTestNet + "valoper"
-		ValidatorPubKeyPrefix = AccountAddressPrefixTestNet + "valoperpub"
-		ConsNodeAddressPrefix = AccountAddressPrefixTestNet + "valcons"
-		ConsNodePubKeyPrefix = AccountAddressPrefixTestNet + "valconspub"
 		CoinType = CoinTypeTestNet
 	}
+	AccountPubKeyPrefix = AccountAddressPrefix + "pub"
+	ValidatorAddressPrefix = AccountAddressPrefix + "valoper"
+	ValidatorPubKeyPrefix = AccountAddressPrefix + "valoperpub"
+	ConsNodeAddressPrefix = AccountAddressPrefix + "valcons"
+	ConsNodePubKeyPrefix = AccountAddressPrefix + "valconspub"
 
 	config := sdk.GetConfig()
 	config.SetCoinType(uint32(CoinType))
 	config.SetPurpose(Purpose)
 	config.SetBech32PrefixForAccount(AccountAddressPrefix, AccountPubKeyPrefix)
 	config.SetBech32PrefixForValidator(ValidatorAddressPrefix, ValidatorPubKeyPrefix)
 	config.SetBech32PrefixForConsensusNode(ConsNodeAddressPrefix, ConsNodePubKeyPrefix)
+
 	if seal {
 		config.Seal()
 	}

app/prefix_test/prefix_test.go

@@ -22,62 +22,139 @@ package prefix_test
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/provenance-io/provenance/app"
 )
 
-func TestFullBIP44Path(t *testing.T) {
+func TestSetConfig(t *testing.T) {
 	cases := []struct {
-		name         string
-		expected     string
-		isTestnet    bool
-		seal         bool
-		panicMessage string
+		name        string
+		isTestnet   bool
+		seal        bool
+		expPath     string
+		expHRP      string
+		expCoinType int
+		expPanic    string
 	}{
 		{
-			// Note: The way our SetConfig function works, calling it with isTestnet = true makes some changes that aren't undone
-			// when later calling it with isTestnet = false. So to test the mainnet BIP44, we need to do it first.
-			name:         "has correct bip44th path for mainnet",
-			expected:     "m/44'/505'/0'/0/0",
-			isTestnet:    false,
-			seal:         false,
-			panicMessage: "",
+			name:        "mainnet",
+			isTestnet:   false,
+			seal:        false,
+			expPath:     "m/44'/505'/0'/0/0",
+			expHRP:      app.AccountAddressPrefixMainNet,
+			expCoinType: app.CoinTypeMainNet,
 		},
 		{
-			// Note: This is the 2nd to last test, so we're doing seal = true.
-			name:         "has correct bip44th path for testnet",
-			expected:     "m/44'/1'/0'/0/0",
-			isTestnet:    true,
-			seal:         true,
-			panicMessage: "",
+			name:        "testnet",
+			isTestnet:   true,
+			seal:        false,
+			expPath:     "m/44'/1'/0'/0/0",
+			expHRP:      app.AccountAddressPrefixTestNet,
+			expCoinType: app.CoinTypeTestNet,
 		},
 		{
-			// Note: The previous test should have had seal = true, making this an
+			name:        "back to mainnet",
+			isTestnet:   false,
+			seal:        false,
+			expPath:     "m/44'/505'/0'/0/0",
+			expHRP:      app.AccountAddressPrefixMainNet,
+			expCoinType: app.CoinTypeMainNet,
+		},
+		{
+			// This is the last valid test, so we're doing seal = true.
+			name:        "back to testnet with seal",
+			isTestnet:   true,
+			seal:        true,
+			expPath:     "m/44'/1'/0'/0/0",
+			expHRP:      app.AccountAddressPrefixTestNet,
+			expCoinType: app.CoinTypeTestNet,
+		},
+		{
+			// Note: A previous test should have had seal = true, making this an
+			// attempt to change the config after sealing it.
+			name:      "already sealed: mainnet, no reseal",
+			isTestnet: false,
+			seal:      false,
+			expPanic:  "Config is sealed",
+		},
+		{
+			// Note: A previous test should have had seal = true, making this an
+			// attempt to change the config after sealing it.
+			name:      "already sealed: testnet, no reseal",
+			isTestnet: true,
+			seal:      false,
+			expPanic:  "Config is sealed",
+		},
+		{
+			// Note: A previous test should have had seal = true, making this an
 			// attempt to change the config after sealing it.
-			name:         "cannot double seal",
-			expected:     "",
-			isTestnet:    false,
-			seal:         true,
-			panicMessage: "Config is sealed",
+			name:      "already sealed: mainnet, with reseal",
+			isTestnet: false,
+			seal:      true,
+			expPanic:  "Config is sealed",
+		},
+		{
+			// Note: A previous test should have had seal = true, making this an
+			// attempt to change the config after sealing it.
+			name:      "already sealed: testnet, with reseal",
+			isTestnet: true,
+			seal:      true,
+			expPanic:  "Config is sealed",
 		},
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			if len(tc.panicMessage) > 0 {
-				require.PanicsWithValue(t, tc.panicMessage, func() {
-					app.SetConfig(tc.isTestnet, tc.seal)
-				}, "SetConfig")
+			testFunc := func() {
+				app.SetConfig(tc.isTestnet, tc.seal)
+			}
+			if len(tc.expPanic) > 0 {
+				require.PanicsWithValue(t, tc.expPanic, testFunc, "SetConfig")
 			} else {
-				require.NotPanics(t, func() {
-					app.SetConfig(tc.isTestnet, tc.seal)
-				}, "SetConfig")
-				config := sdk.GetConfig()
-				fullBIP44Path := config.GetFullBIP44Path()
-				require.Equal(t, tc.expected, fullBIP44Path, "GetFullBIP44Path")
+				require.NotPanics(t, testFunc, "SetConfig")
+
+				expAddrPre := tc.expHRP
+				expValPre := tc.expHRP + "valoper"
+				expValPubPre := tc.expHRP + "valoperpub"
+				expAddrPrePub := tc.expHRP + "pub"
+				expConsPre := tc.expHRP + "valcons"
+				expConsPubPre := tc.expHRP + "valconspub"
+
+				sdkConfig := sdk.GetConfig()
+				fullBIP44Path := sdkConfig.GetFullBIP44Path()
+				addrPre := sdkConfig.GetBech32AccountAddrPrefix()
+				addrPubPre := sdkConfig.GetBech32AccountPubPrefix()
+				valPre := sdkConfig.GetBech32ValidatorAddrPrefix()
+				valPubPre := sdkConfig.GetBech32ValidatorPubPrefix()
+				consPre := sdkConfig.GetBech32ConsensusAddrPrefix()
+				consPubPre := sdkConfig.GetBech32ConsensusPubPrefix()
+				coinType := sdkConfig.GetCoinType()
+				purpose := sdkConfig.GetPurpose()
+
+				// Using require to check the main HRPs because if they're wrong, the rest will almost certainly also be wrong.
+				require.Equal(t, expAddrPre, app.AccountAddressPrefix, "AccountAddressPrefix")
+				require.Equal(t, expAddrPre, addrPre, "sdkConfig.GetBech32AccountAddrPrefix()")
+				// Asserts from here on out so we get a larger picture upon failure.
+				assert.Equal(t, expAddrPrePub, app.AccountPubKeyPrefix, "AccountPubKeyPrefix")
+				assert.Equal(t, expAddrPrePub, addrPubPre, "sdkConfig.GetBech32AccountPubPrefix()")
+				assert.Equal(t, expValPre, app.ValidatorAddressPrefix, "ValidatorAddressPrefix")
+				assert.Equal(t, expValPre, valPre, "sdkConfig.GetBech32ValidatorAddrPrefix()")
+				assert.Equal(t, expValPubPre, app.ValidatorPubKeyPrefix, "ValidatorPubKeyPrefix")
+				assert.Equal(t, expValPubPre, valPubPre, "sdkConfig.GetBech32ValidatorPubPrefix()")
+				assert.Equal(t, expConsPre, app.ConsNodeAddressPrefix, "ConsNodeAddressPrefix")
+				assert.Equal(t, expConsPre, consPre, "sdkConfig.GetBech32ConsensusAddrPrefix()")
+				assert.Equal(t, expConsPubPre, app.ConsNodePubKeyPrefix, "ConsNodePubKeyPrefix")
+				assert.Equal(t, expConsPubPre, consPubPre, "sdkConfig.GetBech32ConsensusPubPrefix()")
+
+				assert.Equal(t, tc.expPath, fullBIP44Path, "sdkConfig.GetFullBIP44Path()")
+				assert.Equal(t, tc.expCoinType, app.CoinType, "CoinType")
+				assert.Equal(t, tc.expCoinType, int(coinType), "sdkConfig.GetCoinType()")
+				assert.Equal(t, 44, app.Purpose, "Purpose")
+				assert.Equal(t, 44, int(purpose), "sdkConfig.GetPurpose()")
 			}
 		})
 	}