From 107ffbbdb3eccc2dbf0931524a35a45d7547e823 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> Date: Wed, 14 Aug 2024 22:20:58 +0200 Subject: [PATCH] feat!: Bump ICS to include inactive validators (#3263) * replace gaia/v19 with gaia/v20 * add v20 upgrade handler * update old gaia in upgrade-test * Upgrade ICS version * Change app wiring * Add migration * Fix linting * Revert changes * Upgrade ICS version * Change app wiring * Add migration * Fix linting * Add initialization for LastProviderConsensusValSet * Use CamelCase for const names * fix linter * Remove unnecessary migration and parameter check * Adjust comment to mention LastProviderConsensusValidatorSet initialization * Remove panics and replace with error returns * Improve logging * Lint * feat!: Bump ICS to include inactive validators (#3259) * Upgrade ICS version * Change app wiring * Add migration * Fix linting * Add initialization for LastProviderConsensusValSet * Use CamelCase for const names * Remove unnecessary migration and parameter check * Adjust comment to mention LastProviderConsensusValidatorSet initialization * Remove panics and replace with error returns * Improve logging * Lint * Revert "feat!: Bump ICS to include inactive validators (#3259)" This reverts commit 06f58d3c74caf521704f5abab6d10c4834881b3a. * add changelog entries * Update app/upgrades/v20/upgrades.go --------- Co-authored-by: mpoke --- .../feature/3263-max-provider-consensus.md | 4 + .../unreleased/feature/3263-max-validators.md | 5 + app/keepers/keepers.go | 64 +++++++------ app/modules.go | 10 +- app/upgrades/v20/upgrades.go | 96 ++++++++++++++++++- go.mod | 4 +- go.sum | 8 +- 7 files changed, 154 insertions(+), 37 deletions(-) create mode 100644 .changelog/unreleased/feature/3263-max-provider-consensus.md create mode 100644 .changelog/unreleased/feature/3263-max-validators.md diff --git a/.changelog/unreleased/feature/3263-max-provider-consensus.md b/.changelog/unreleased/feature/3263-max-provider-consensus.md new file mode 100644 index 00000000000..8a3843b3904 --- /dev/null +++ b/.changelog/unreleased/feature/3263-max-provider-consensus.md @@ -0,0 +1,4 @@ +- Set the `MaxProviderConsensusValidators` parameter of the provider module to 180. + This parameter will be used to govern the number of validators participating in consensus, + and takes over this role from the `MaxValidators` parameter of the staking module. + ([\#3263](https://github.com/cosmos/gaia/pull/3263)) \ No newline at end of file diff --git a/.changelog/unreleased/feature/3263-max-validators.md b/.changelog/unreleased/feature/3263-max-validators.md new file mode 100644 index 00000000000..a7aca6c9877 --- /dev/null +++ b/.changelog/unreleased/feature/3263-max-validators.md @@ -0,0 +1,5 @@ +- Set the `MaxValidators` parameter of the staking module to 200, which is the current number of 180 plus 20. + This is done as a result of introducing the inactive-validators feature of Interchain Security, + which entails that the number of validators participating in consensus will be governed by the + `MaxProviderConsensusValidators` parameter in the provider module. + ([\#3263](https://github.com/cosmos/gaia/pull/3263)) \ No newline at end of file diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index 1670eb38757..781fb648c1f 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -254,16 +254,6 @@ func NewAppKeeper( authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ConsensusAddrPrefix()), ) - appKeepers.MintKeeper = mintkeeper.NewKeeper( - appCodec, - runtime.NewKVStoreService(appKeepers.keys[minttypes.StoreKey]), - appKeepers.StakingKeeper, - appKeepers.AccountKeeper, - appKeepers.BankKeeper, - authtypes.FeeCollectorName, - authtypes.NewModuleAddress(govtypes.ModuleName).String(), - ) - appKeepers.DistrKeeper = distrkeeper.NewKeeper( appCodec, runtime.NewKVStoreService(appKeepers.keys[distrtypes.StoreKey]), @@ -321,21 +311,6 @@ func NewAppKeeper( authtypes.NewModuleAddress(govtypes.ModuleName).String(), ) - // provider depends on gov, so gov must be registered first - govConfig := govtypes.DefaultConfig() - // set the MaxMetadataLen for proposals to the same value as it was pre-sdk v0.47.x - govConfig.MaxMetadataLen = 10200 - appKeepers.GovKeeper = govkeeper.NewKeeper( - appCodec, - runtime.NewKVStoreService(appKeepers.keys[govtypes.StoreKey]), - appKeepers.AccountKeeper, - appKeepers.BankKeeper, - appKeepers.StakingKeeper, - appKeepers.DistrKeeper, - bApp.MsgServiceRouter(), - govConfig, - authtypes.NewModuleAddress(govtypes.ModuleName).String(), - ) appKeepers.ProviderKeeper = icsproviderkeeper.NewKeeper( appCodec, appKeepers.keys[providertypes.StoreKey], @@ -350,14 +325,49 @@ func NewAppKeeper( appKeepers.AccountKeeper, appKeepers.DistrKeeper, appKeepers.BankKeeper, - *appKeepers.GovKeeper, + govkeeper.Keeper{}, // cyclic dependency between provider and governance, will be set later authtypes.NewModuleAddress(govtypes.ModuleName).String(), authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ValidatorAddrPrefix()), authcodec.NewBech32Codec(sdk.GetConfig().GetBech32ConsensusAddrPrefix()), authtypes.FeeCollectorName, ) - appKeepers.ProviderModule = icsprovider.NewAppModule(&appKeepers.ProviderKeeper, appKeepers.GetSubspace(providertypes.ModuleName)) + // gov depends on provider, so needs to be set after + govConfig := govtypes.DefaultConfig() + // set the MaxMetadataLen for proposals to the same value as it was pre-sdk v0.47.x + govConfig.MaxMetadataLen = 10200 + appKeepers.GovKeeper = govkeeper.NewKeeper( + appCodec, + runtime.NewKVStoreService(appKeepers.keys[govtypes.StoreKey]), + appKeepers.AccountKeeper, + appKeepers.BankKeeper, + // use the ProviderKeeper as StakingKeeper for gov + // because governance should be based on the consensus-active validators + appKeepers.ProviderKeeper, + appKeepers.DistrKeeper, + bApp.MsgServiceRouter(), + govConfig, + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + ) + + // mint keeper must be created after provider keeper + appKeepers.MintKeeper = mintkeeper.NewKeeper( + appCodec, + runtime.NewKVStoreService(appKeepers.keys[minttypes.StoreKey]), + appKeepers.ProviderKeeper, + appKeepers.AccountKeeper, + appKeepers.BankKeeper, + authtypes.FeeCollectorName, + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + ) + + appKeepers.ProviderKeeper.SetGovKeeper(*appKeepers.GovKeeper) + + appKeepers.ProviderModule = icsprovider.NewAppModule( + &appKeepers.ProviderKeeper, + appKeepers.GetSubspace(providertypes.ModuleName), + appKeepers.keys[providertypes.StoreKey], + ) // Register the proposal types // Deprecated: Avoid adding new handlers, instead use the new proposal flow diff --git a/app/modules.go b/app/modules.go index bc06c53bbca..ecac3b126a9 100644 --- a/app/modules.go +++ b/app/modules.go @@ -15,6 +15,8 @@ import ( ibc "github.com/cosmos/ibc-go/v8/modules/core" ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" + no_valupdates_genutil "github.com/cosmos/interchain-security/v5/x/ccv/no_valupdates_genutil" + no_valupdates_staking "github.com/cosmos/interchain-security/v5/x/ccv/no_valupdates_staking" icsproviderclient "github.com/cosmos/interchain-security/v5/x/ccv/provider/client" providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" @@ -89,7 +91,7 @@ func appModules( skipGenesisInvariants bool, ) []module.AppModule { return []module.AppModule{ - genutil.NewAppModule( + no_valupdates_genutil.NewAppModule( app.AccountKeeper, app.StakingKeeper, app, @@ -104,7 +106,7 @@ func appModules( mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, nil, app.GetSubspace(minttypes.ModuleName)), slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(slashingtypes.ModuleName), app.interfaceRegistry), distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(distrtypes.ModuleName)), - staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(stakingtypes.ModuleName)), + no_valupdates_staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(stakingtypes.ModuleName)), upgrade.NewAppModule(app.UpgradeKeeper, app.AccountKeeper.AddressCodec()), evidence.NewAppModule(app.EvidenceKeeper), feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry), @@ -276,7 +278,6 @@ func orderInitBlockers() []string { stakingtypes.ModuleName, slashingtypes.ModuleName, minttypes.ModuleName, - crisistypes.ModuleName, genutiltypes.ModuleName, ibctransfertypes.ModuleName, ibcexported.ModuleName, @@ -302,5 +303,8 @@ func orderInitBlockers() []string { consensusparamtypes.ModuleName, metaprotocolstypes.ModuleName, wasmtypes.ModuleName, + // crisis needs to be last so that the genesis state is consistent + // when it checks invariants + crisistypes.ModuleName, } } diff --git a/app/upgrades/v20/upgrades.go b/app/upgrades/v20/upgrades.go index 1f4d0cc3709..54b84b83da3 100644 --- a/app/upgrades/v20/upgrades.go +++ b/app/upgrades/v20/upgrades.go @@ -3,14 +3,36 @@ package v20 import ( "context" + providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper" + "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + + errorsmod "cosmossdk.io/errors" upgradetypes "cosmossdk.io/x/upgrade/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" + stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" "github.com/cosmos/gaia/v20/app/keepers" ) +// Constants for the new parameters in the v20 upgrade. +const ( + // MaxValidators will be set to 200 (up from 180), + // to allow the first 20 inactive validators + // to participate on consumer chains. + NewMaxValidators = 200 + // MaxProviderConsensusValidators will be set to 180, + // to preserve the behaviour of only the first 180 + // validators participating in consensus on the Cosmos Hub. + NewMaxProviderConsensusValidators = 180 +) + +// CreateUpgradeHandler returns an upgrade handler for Gaia v20. +// It performs module migrations, as well as the following tasks: +// - Initializes the MaxProviderConsensusValidators parameter in the provider module to 180. +// - Increases the MaxValidators parameter in the staking module to 200. +// - Initializes the last provider consensus validator set in the provider module func CreateUpgradeHandler( mm *module.Manager, configurator module.Configurator, @@ -22,10 +44,82 @@ func CreateUpgradeHandler( vm, err := mm.RunMigrations(ctx, configurator, vm) if err != nil { - return vm, err + return vm, errorsmod.Wrapf(err, "running module migrations") + } + + ctx.Logger().Info("Initializing MaxProviderConsensusValidators parameter...") + InitializeMaxProviderConsensusParam(ctx, keepers.ProviderKeeper) + + ctx.Logger().Info("Setting MaxValidators parameter...") + err = SetMaxValidators(ctx, *keepers.StakingKeeper) + if err != nil { + return vm, errorsmod.Wrapf(err, "setting MaxValidators during migration") + } + + ctx.Logger().Info("Initializing LastProviderConsensusValidatorSet...") + err = InitializeLastProviderConsensusValidatorSet(ctx, keepers.ProviderKeeper, *keepers.StakingKeeper) + if err != nil { + return vm, errorsmod.Wrapf(err, "initializing LastProviderConsensusValSet during migration") } ctx.Logger().Info("Upgrade v20 complete") return vm, nil } } + +// InitializeMaxProviderConsensusParam initializes the MaxProviderConsensusValidators parameter. +// It is set to 180, which is the current number of validators participating in consensus on the Cosmos Hub. +// This parameter will be used to govern the number of validators participating in consensus on the Cosmos Hub, +// and takes over this role from the MaxValidators parameter in the staking module. +func InitializeMaxProviderConsensusParam(ctx sdk.Context, providerKeeper providerkeeper.Keeper) { + params := providerKeeper.GetParams(ctx) + params.MaxProviderConsensusValidators = NewMaxProviderConsensusValidators + providerKeeper.SetParams(ctx, params) +} + +// SetMaxValidators sets the MaxValidators parameter in the staking module to 200, +// which is the current number of 180 plus 20. +// This is done in concert with the introduction of the inactive-validators feature +// in Interchain Security, after which the number of validators +// participating in consensus on the Cosmos Hub will be governed by the +// MaxProviderConsensusValidators parameter in the provider module. +func SetMaxValidators(ctx sdk.Context, stakingKeeper stakingkeeper.Keeper) error { + params, err := stakingKeeper.GetParams(ctx) + if err != nil { + return err + } + + params.MaxValidators = NewMaxValidators + + return stakingKeeper.SetParams(ctx, params) +} + +// InitializeLastProviderConsensusValidatorSet initializes the last provider consensus validator set +// by setting it to the first 180 validators from the current validator set of the staking module. +func InitializeLastProviderConsensusValidatorSet( + ctx sdk.Context, providerKeeper providerkeeper.Keeper, stakingKeeper stakingkeeper.Keeper, +) error { + vals, err := stakingKeeper.GetBondedValidatorsByPower(ctx) + if err != nil { + return err + } + + // cut the validator set to the first 180 validators + if len(vals) > NewMaxProviderConsensusValidators { + vals = vals[:NewMaxProviderConsensusValidators] + } + + // create consensus validators for the staking validators + lastValidators := []types.ConsensusValidator{} + for _, val := range vals { + consensusVal, err := providerKeeper.CreateProviderConsensusValidator(ctx, val) + if err != nil { + return err + } + + lastValidators = append(lastValidators, consensusVal) + } + + providerKeeper.SetLastProviderConsensusValSet(ctx, lastValidators) + return nil +} diff --git a/go.mod b/go.mod index bc65dc85281..279a0b20266 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( github.com/cosmos/ibc-apps/modules/rate-limiting/v8 v8.0.0 github.com/cosmos/ibc-go/modules/capability v1.0.0 github.com/cosmos/ibc-go/v8 v8.4.0 - github.com/cosmos/interchain-security/v5 v5.1.1 + github.com/cosmos/interchain-security/v5 v5.0.0-20240806104629-29327696b8e6 github.com/google/gofuzz v1.2.0 github.com/gorilla/mux v1.8.1 github.com/ory/dockertest/v3 v3.11.0 @@ -50,7 +50,7 @@ require ( github.com/golang/protobuf v1.5.4 // indirect github.com/grpc-ecosystem/grpc-gateway v1.16.0 google.golang.org/genproto v0.0.0-20240401170217-c3f982113cda // indirect - google.golang.org/grpc v1.64.0 // indirect + google.golang.org/grpc v1.65.0 // indirect ) require ( diff --git a/go.sum b/go.sum index bca079b879e..908be089d7d 100644 --- a/go.sum +++ b/go.sum @@ -441,8 +441,8 @@ github.com/cosmos/ibc-go/v8 v8.4.0 h1:K2PfX0AZ+1XKZytHGEMuSjQXG/MZshPb83RSTQt2+c github.com/cosmos/ibc-go/v8 v8.4.0/go.mod h1:zh6x1osR0hNvEcFrC/lhGD08sMfQmr9wHVvZ/mRWMCs= github.com/cosmos/ics23/go v0.10.0 h1:iXqLLgp2Lp+EdpIuwXTYIQU+AiHj9mOC2X9ab++bZDM= github.com/cosmos/ics23/go v0.10.0/go.mod h1:ZfJSmng/TBNTBkFemHHHj5YY7VAU/MBU980F4VU1NG0= -github.com/cosmos/interchain-security/v5 v5.1.1 h1:xmRRMeE4xoc+JAZUh0XzXFYWaGBtzFFj5SETuOgnEnY= -github.com/cosmos/interchain-security/v5 v5.1.1/go.mod h1:vmeTcTxFCl1eV0o6xpl/IRT7Basz0szVVGzbppnInMg= +github.com/cosmos/interchain-security/v5 v5.0.0-20240806104629-29327696b8e6 h1:aFwnbEdeMUaQg9ZU+Pm8fE3CzZM2gG5jSeAvYOd+hoU= +github.com/cosmos/interchain-security/v5 v5.0.0-20240806104629-29327696b8e6/go.mod h1:sT6a/OIwwkXuH9fBzt5IBa4lrlWO8etgQ+b59pIE8k4= github.com/cosmos/keyring v1.2.0 h1:8C1lBP9xhImmIabyXW4c3vFjjLiBdGCmfLUfeZlV1Yo= github.com/cosmos/keyring v1.2.0/go.mod h1:fc+wB5KTk9wQ9sDx0kFXB3A0MaeGHM9AwRStKOQ5vOA= github.com/cosmos/ledger-cosmos-go v0.13.3 h1:7ehuBGuyIytsXbd4MP43mLeoN2LTOEnk5nvue4rK+yM= @@ -1960,8 +1960,8 @@ google.golang.org/grpc v1.48.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACu google.golang.org/grpc v1.49.0/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI= google.golang.org/grpc v1.50.0/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI= google.golang.org/grpc v1.50.1/go.mod h1:ZgQEeidpAuNRZ8iRrlBKXZQP1ghovWIVhdJRyCDK+GI= -google.golang.org/grpc v1.64.0 h1:KH3VH9y/MgNQg1dE7b3XfVK0GsPSIzJwdF617gUSbvY= -google.golang.org/grpc v1.64.0/go.mod h1:oxjF8E3FBnjp+/gVFYdWacaLDx9na1aqy9oovLpxQYg= +google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc= +google.golang.org/grpc v1.65.0/go.mod h1:WgYC2ypjlB0EiQi6wdKixMqukr6lBc0Vo+oOgjrM5ZQ= google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=