Skip to content

Commit

Permalink
Fix the umber upgrades. (#2033)
Browse files Browse the repository at this point in the history 
* Move the call to registerUpgradeHandlers back down to where that stuff was originally so that we can have it inject an upgrade which now overwrites the pre-blocker (instead of begin-blocker).

* Add the consensus params to the list of added modules for umber.

* inject the umber upgrade (and add a TODO to remove it).

* Use the correct type for the marker module's max supply param entry.

* Fix the os locator params migration. Make it just set it to the default. Those params never got defined in testnet, and mainnet has an empty entry for it. So both were just using the default value there anyway.

* Fix the name params migration to properly identify the keytable.

* Fix the ibchooks params migration.

* Small cleanup in app.New to reuse stuff from the sdk config.

* Refactor app.SetConfig(bool, bool) to be able to switch back to mainnet after initially being changed to testnet.

* Move the EnvTypeFlag and CoinTypeFlag variables into the interceptor with some other flags so that we can use them in there as needed.

* Add key tables to all the params subspaces. Needed now because the migrations all failed because the needed keys weren't registered.

* Tweak the log messages in migrateBaseappParams to better reflect what's being done (all of the params stuff is 'legacy' now, so it didn't really describe what was happening.

* Delete the call to injectUpgrade.

* Add nolint:staticcheck to all the newly added calls to the deprecated ParamKeyTable functions.

* Fix double import.

* Fix the umber tests to have the expected log messages for the consensus params migration (that I updated a few commits ago).

* Add changelog entry.
  • Loading branch information
@SpicyLemon
SpicyLemon authored Jun 14, 2024
1 parent 2058ec2 commit f40ef8f
Show file tree
Hide file tree
Showing 10 changed files with 182 additions and 109 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
68 changes: 34 additions & 34 deletions app/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
})
}
14 changes: 8 additions & 6 deletions app/prefix.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
147 changes: 112 additions & 35 deletions app/prefix_test/prefix_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()")
}
})
}
Expand Down
Loading

0 comments on commit f40ef8f

Please sign in to comment.