From 8110d6771a09370ccc8c862ab9c96a80ebca4827 Mon Sep 17 00:00:00 2001 From: Ankur Banerjee Date: Tue, 10 Jan 2023 18:40:57 +0000 Subject: [PATCH] fix(upgrade): Bring UpgradeHandler and InitChainer in-line with best practices [DEV-2115] (#505) * Update app.go * Remove SetOrderMigrations * Fix debug printings * Removed special RegisterMigration for DID module * Set testing log level to debug * Add pagination to DID and resource queries * make proto-gen * gofumpt * Switch timestamps to protobuf timestamp * Revert "Switch timestamps to protobuf timestamp" This reverts commit 7ae11ed77f13cfdcffddf1c87004bacc63cb670e. * Revert "Add pagination to DID and resource queries" This reverts commit 462edc6b840fbde2a7abc79aaeb5afbe073a60de. * Prune v1 protogen paths * Add clarifying comment * Update protoc-pulsar-gen.sh Use a similar loop to exclude v1 directories for Pulsar generation * Run proto-gen Co-authored-by: Andrew Nikitin --- app/app.go | 223 +++++++++++--------------- docker/localnet/gen-network-config.sh | 1 + scripts/protoc-pulsar-gen.sh | 13 +- scripts/protocgen.sh | 4 +- 4 files changed, 106 insertions(+), 135 deletions(-) diff --git a/app/app.go b/app/app.go index ba2e0fb81..e87809dcb 100644 --- a/app/app.go +++ b/app/app.go @@ -673,33 +673,6 @@ func New( paramstypes.ModuleName, ) - // Uncomment if you want to set a custom migration order here. - app.mm.SetOrderMigrations( - upgradetypes.ModuleName, - capabilitytypes.ModuleName, - minttypes.ModuleName, - distrtypes.ModuleName, - slashingtypes.ModuleName, - evidencetypes.ModuleName, - stakingtypes.ModuleName, - authtypes.ModuleName, - banktypes.ModuleName, - govtypes.ModuleName, - crisistypes.ModuleName, - ibchost.ModuleName, - ibctransfertypes.ModuleName, - icatypes.ModuleName, - ibcfeetypes.ModuleName, - genutiltypes.ModuleName, - authz.ModuleName, - group.ModuleName, - feegrant.ModuleName, - paramstypes.ModuleName, - vestingtypes.ModuleName, - didtypes.ModuleName, - resourcetypes.ModuleName, - ) - app.mm.RegisterInvariants(&app.CrisisKeeper) app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino) @@ -740,16 +713,6 @@ func New( app.SetBeginBlocker(app.BeginBlocker) app.SetEndBlocker(app.EndBlocker) - // TODO: Remove this after cheqd-node release v1.x is successful - //nolint: errcheck - app.configurator.RegisterMigration( - didtypes.ModuleName, - 3, - func(ctx sdk.Context) error { - return nil - }, - ) - // Upgrade handler for the next release app.UpgradeKeeper.SetUpgradeHandler(UpgradeName, func(ctx sdk.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { @@ -757,105 +720,99 @@ func New( // Fix lack of version map initialization in InitChainer for new chains if len(fromVM) == 0 { - println("Initializing version map") + ctx.Logger().Info("Version map is empty. Initialising to required values.") + + // Set these explicitly to avoid calling InitGenesis on modules that don't need it. + // Explicitly skipping x/auth migrations. + fromVM = map[string]uint64{ + "auth": 2, + "authz": 1, + "bank": 2, + "capability": 1, + "cheqd": 3, + "crisis": 1, + "distribution": 2, + "evidence": 1, + "feegrant": 1, + "genutil": 1, + "gov": 2, + "ibc": 2, + "interchainaccounts": 1, + "mint": 1, + "params": 1, + "resource": 1, + "slashing": 2, + "staking": 2, + "transfer": 1, + "upgrade": 1, + "vesting": 1, + } + } - // Add defaults for did subspace - didSubspace := app.GetSubspace(didtypes.ModuleName) - didSubspace.Set(ctx, didtypes.ParamStoreKeyFeeParams, didtypes.DefaultFeeParams()) + ctx.Logger().Info("Version map is populated. Running migrations.") + ctx.Logger().Debug(fmt.Sprintf("fromVm: %v", fromVM)) - // Add defaults for resource subspace - resourceSubspace := app.GetSubspace(resourcetypes.ModuleName) - resourceSubspace.Set(ctx, resourcetypes.ParamStoreKeyFeeParams, resourcetypes.DefaultFeeParams()) + // Get current version map + newVM := app.mm.GetVersionMap() + ctx.Logger().Debug(fmt.Sprintf("newVM: %v", newVM)) - // Add defaults for staking subspace - stakingSubspace, _ := app.ParamsKeeper.GetSubspace(stakingtypes.ModuleName) - stakingSubspace.Set(ctx, stakingtypes.KeyMinCommissionRate, sdk.NewDec(0)) + // Set cheqd/DID module to ConsensusVersion + fromVM[didtypes.ModuleName] = newVM[didtypes.ModuleName] - // Fix version map - versionMap := app.mm.GetVersionMap() + // Set resource module to ConsensusVersion + fromVM[resourcetypes.ModuleName] = newVM[resourcetypes.ModuleName] - for moduleName := range versionMap { - fromVM[moduleName] = versionMap[moduleName] - } - } else { - println("Version map already initialized") - - // Add defaults for staking subspace - stakingSubspace, _ := app.ParamsKeeper.GetSubspace(stakingtypes.ModuleName) - stakingSubspace.Set(ctx, stakingtypes.KeyMinCommissionRate, sdk.NewDec(0)) - - // Add defaults for did subspace - didSubspace := app.GetSubspace(didtypes.ModuleName) - didSubspace.Set(ctx, didtypes.ParamStoreKeyFeeParams, didtypes.DefaultFeeParams()) - - // Add defaults for resource subspace - resourceSubspace := app.GetSubspace(resourcetypes.ModuleName) - resourceSubspace.Set(ctx, resourcetypes.ParamStoreKeyFeeParams, resourcetypes.DefaultFeeParams()) - - // Re-register ICA module with correct ICA controller and host params & store keys - // This is required because the ICA module was registered with the wrong params & store keys in the previous release - // What's changed: - // Added: []string{ - // icahosttypes.StoreKey, - // icacontrollertypes.StoreKey, // <-- this is the new store key ehich was missing in the previous release - // }, - - // set the ICS27 consensus version so InitGenesis is not run - fromVM[icatypes.ModuleName] = icaModule.ConsensusVersion() - - // create ICS27 Controller submodule params - controllerParams := icacontrollertypes.Params{ - ControllerEnabled: true, - } + // Add defaults for DID module subspace + didSubspace := app.GetSubspace(didtypes.ModuleName) + didSubspace.Set(ctx, didtypes.ParamStoreKeyFeeParams, didtypes.DefaultFeeParams()) - // create ICS27 Host submodule params - hostParams := icahosttypes.Params{ - HostEnabled: true, - AllowMessages: []string{ - authzMsgExec, - authzMsgGrant, - authzMsgRevoke, - bankMsgSend, - bankMsgMultiSend, - distrMsgSetWithdrawAddr, - distrMsgWithdrawValidatorCommission, - distrMsgFundCommunityPool, - distrMsgWithdrawDelegatorReward, - feegrantMsgGrantAllowance, - feegrantMsgRevokeAllowance, - govMsgVoteWeighted, - govMsgSubmitProposal, - govMsgDeposit, - govMsgVote, - stakingMsgEditValidator, - stakingMsgDelegate, - stakingMsgUndelegate, - stakingMsgBeginRedelegate, - stakingMsgCreateValidator, - vestingMsgCreateVestingAccount, - ibcMsgTransfer, - // cheqd namespace - didMsgCreateDidDoc, - didMsgUpdateDidDoc, - didMsgDeactivateDidDoc, - resourceMsgCreateResource, - }, - } + // Add defaults for resource subspace + resourceSubspace := app.GetSubspace(resourcetypes.ModuleName) + resourceSubspace.Set(ctx, resourcetypes.ParamStoreKeyFeeParams, resourcetypes.DefaultFeeParams()) - // initialize ICS27 module - ctx.Logger().Info("start to init interchainaccount module...") - icaModule.InitModule(ctx, controllerParams, hostParams) - - // Get the current version map - versionMap := app.mm.GetVersionMap() - - fmt.Println("Current version map: ", fromVM) - fmt.Println("Expected version map: ", versionMap) + // create ICS27 Controller submodule params + controllerParams := icacontrollertypes.Params{ + ControllerEnabled: true, + } - // Skip resource module InitGenesis (was not present in v0.6.x) - fromVM[resourcetypes.ModuleName] = versionMap[resourcetypes.ModuleName] + // create ICS27 Host submodule params + hostParams := icahosttypes.Params{ + HostEnabled: true, + AllowMessages: []string{ + authzMsgExec, + authzMsgGrant, + authzMsgRevoke, + bankMsgSend, + bankMsgMultiSend, + distrMsgSetWithdrawAddr, + distrMsgWithdrawValidatorCommission, + distrMsgFundCommunityPool, + distrMsgWithdrawDelegatorReward, + feegrantMsgGrantAllowance, + feegrantMsgRevokeAllowance, + govMsgVoteWeighted, + govMsgSubmitProposal, + govMsgDeposit, + govMsgVote, + stakingMsgEditValidator, + stakingMsgDelegate, + stakingMsgUndelegate, + stakingMsgBeginRedelegate, + stakingMsgCreateValidator, + vestingMsgCreateVestingAccount, + ibcMsgTransfer, + // cheqd namespace + didMsgCreateDidDoc, + didMsgUpdateDidDoc, + didMsgDeactivateDidDoc, + resourceMsgCreateResource, + }, } + // initialize ICS27 module + ctx.Logger().Debug("Initialise interchainaccount module...") + icaModule.InitModule(ctx, controllerParams, hostParams) + // cheqd migrations migrationContext := migrations.NewMigrationContext( app.appCodec, @@ -865,6 +822,7 @@ func New( app.GetSubspace(resourcetypes.ModuleName), ) + ctx.Logger().Debug("Initialise cheqd DID and Resource module migrations...") cheqdMigrator := migrations.NewMigrator( migrationContext, []migrations.Migration{ @@ -898,9 +856,11 @@ func New( panic(err) } - // ibc v3 -> v4 migration - // transfer module consensus version has been bumped to 2 - return app.mm.RunMigrations(ctx, app.configurator, fromVM) + // Run all default migrations + ctx.Logger().Debug(fmt.Sprintf("fromVM (for default RunMigrations): %v", fromVM)) + toVM, err := app.mm.RunMigrations(ctx, app.configurator, fromVM) + ctx.Logger().Debug(fmt.Sprintf("toVM (version map after RunMigrations): %v", toVM)) + return toVM, err }) if loadLatest { @@ -932,14 +892,11 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo // InitChainer application update at chain initialization func (app *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain { - // FIXME: This should have been done from the beginning. - // Now this would break consensus with existing networks. - // so ModuleVersionMap is initialized as part of upgrade xxx. var genesisState GenesisState if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil { panic(err) } - // app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()) + app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()) return app.mm.InitGenesis(ctx, app.appCodec, genesisState) } diff --git a/docker/localnet/gen-network-config.sh b/docker/localnet/gen-network-config.sh index 914855638..52096b53f 100644 --- a/docker/localnet/gen-network-config.sh +++ b/docker/localnet/gen-network-config.sh @@ -46,6 +46,7 @@ function configure_node() { sed -i $SED_EXT 's/timeout_prevote = "1s"/timeout_prevote = "500ms"/g' "${CONFIG_TOML}" sed -i $SED_EXT 's/timeout_precommit = "1s"/timeout_precommit = "500ms"/g' "${CONFIG_TOML}" sed -i $SED_EXT 's/timeout_commit = "5s"/timeout_commit = "500ms"/g' "${CONFIG_TOML}" + sed -i $SED_EXT 's/log_level = "info"/log_level = "debug"/g' "${CONFIG_TOML}" } function configure_genesis() { diff --git a/scripts/protoc-pulsar-gen.sh b/scripts/protoc-pulsar-gen.sh index 442146c4f..28861e8fc 100755 --- a/scripts/protoc-pulsar-gen.sh +++ b/scripts/protoc-pulsar-gen.sh @@ -13,5 +13,16 @@ protoc_gen_install() { protoc_gen_install echo "Generating API module" + +# Remove old generated API/Pulsar files (cd api; find ./ -type f \( -iname \*.pulsar.go -o -iname \*.pb.go -o -iname \*.cosmos_orm.go -o -iname \*.pb.gw.go \) -delete; find . -empty -type d -delete; cd ..) -(cd proto; buf generate --path cheqd/did/v2 --path cheqd/resource/v2 --template buf.gen.pulsar.yaml) +cd proto + +# Exclude "v1" paths from API/Pulsar generation +proto_dirs=$(find ./ -type f -path '*/v1/*' -o -name '*.proto' -print0 | xargs -0 -n1 dirname | sort | uniq) +for proto_dir in $proto_dirs; do + proto_files=$(find "${proto_dir}" -maxdepth 1 -name '*.proto') + for f in $proto_files; do + buf generate --template buf.gen.pulsar.yaml "$f" + done +done diff --git a/scripts/protocgen.sh b/scripts/protocgen.sh index 2d83b7e07..5a8724f85 100755 --- a/scripts/protocgen.sh +++ b/scripts/protocgen.sh @@ -7,7 +7,9 @@ go get github.com/cosmos/gogoproto 2>/dev/null echo "Generating gogo proto code" cd proto -proto_dirs=$(find ./ -path -prune -o -name '*.proto' -print0 | xargs -0 -n1 dirname | sort | uniq) + +# Exclude "v1" paths from Protobuf generation +proto_dirs=$(find ./ -type f -path '*/v1/*' -o -name '*.proto' -print0 | xargs -0 -n1 dirname | sort | uniq) for proto_dir in $proto_dirs; do proto_files=$(find "${proto_dir}" -maxdepth 1 -name '*.proto') for f in $proto_files; do