From 79e1c9abcca212f647e2411da1710043528a04cd Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Tue, 25 Apr 2023 18:34:44 +0200 Subject: [PATCH 1/2] feat: update the slashing and evidence modules to work with ICS (#15908) Co-authored-by: Julien Robert Co-authored-by: Jacob Gadikian (cherry picked from commit 97e4978f0f773d49a122a353ad117e58ff6ee50c) # Conflicts: # CHANGELOG.md # x/evidence/keeper/infraction.go --- CHANGELOG.md | 33 ++++++++++++++++++++++++++++++++ x/evidence/keeper/infraction.go | 26 +++++++++++++++++++++++++ x/slashing/keeper/infractions.go | 3 --- 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76b8d6f082fd..231a33d2587e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,40 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## Improvements +<<<<<<< HEAD * (store) [#15683](https://github.com/cosmos/cosmos-sdk/pull/15683) `rootmulti.Store.CacheMultiStoreWithVersion` now can handle loading archival states that don't persist any of the module stores the current state has. +======= +* (x/evidence) [#15908](https://github.com/cosmos/cosmos-sdk/pull/15908) Update the equivocation handler to work with ICS by removing a pubkey check that was performing a no-op for consumer chains. +* (x/slashing) [#15908](https://github.com/cosmos/cosmos-sdk/pull/15908) Remove the validators' pubkey check in the signature handler in order to work with ICS. +* (runtime) [#15818](https://github.com/cosmos/cosmos-sdk/pull/15818) Provide logger through `depinject` instead of appBuilder. +* (client) [#15597](https://github.com/cosmos/cosmos-sdk/pull/15597) Add status endpoint for clients. +* (testutil/integration) [#15556](https://github.com/cosmos/cosmos-sdk/pull/15556) Introduce `testutil/integration` package for module integration testing. +* (types) [#15735](https://github.com/cosmos/cosmos-sdk/pull/15735) Make `ValidateBasic() error` method of `Msg` interface optional. Modules should validate messages directly in their message handlers ([RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation)). +* (x/genutil) [#15679](https://github.com/cosmos/cosmos-sdk/pull/15679) Allow applications to specify a custom genesis migration function for the `genesis migrate` command. +* (client) [#15458](https://github.com/cosmos/cosmos-sdk/pull/15458) Add a `CmdContext` field to client.Context initialized to cobra command's context. +* (core) [#15133](https://github.com/cosmos/cosmos-sdk/pull/15133) Implement RegisterServices in the module manager. +* (x/gov) [#14373](https://github.com/cosmos/cosmos-sdk/pull/14373) Add new proto field `constitution` of type `string` to gov module genesis state, which allows chain builders to lay a strong foundation by specifying purpose. +* (x/genutil) [#15301](https://github.com/cosmos/cosmos-sdk/pull/15031) Add application genesis. The genesis is now entirely managed by the application and passed to CometBFT at note instantiation. Functions that were taking a `cmttypes.GenesisDoc{}` now takes a `genutiltypes.AppGenesis{}`. +* (cli) [#14659](https://github.com/cosmos/cosmos-sdk/pull/14659) Added ability to query blocks by events with queries directly passed to Tendermint, which will allow for full query operator support, e.g. `>`. +* (x/gov) [#14720](https://github.com/cosmos/cosmos-sdk/pull/14720) Upstream expedited proposals from Osmosis. +* (x/auth) [#14650](https://github.com/cosmos/cosmos-sdk/pull/14650) Add Textual SignModeHandler. It is however **NOT** enabled by default, and should only be used for **TESTING** purposes until `SIGN_MODE_TEXTUAL` is fully released. +* (x/crisis) [#14588](https://github.com/cosmos/cosmos-sdk/pull/14588) Use CacheContext() in AssertInvariants() +* (client) [#14342](https://github.com/cosmos/cosmos-sdk/pull/14342) Add ` config` command is now a sub-command, for setting, getting and migrating Cosmos SDK configuration files. +* (query) [#14468](https://github.com/cosmos/cosmos-sdk/pull/14468) Implement pagination for collections. +* (x/distribution) [#14322](https://github.com/cosmos/cosmos-sdk/pull/14322) Introduce a new gRPC message handler, `DepositValidatorRewardsPool`, that allows explicit funding of a validator's reward pool. +* [#13473](https://github.com/cosmos/cosmos-sdk/pull/13473) ADR-038: Go plugin system proposal +* (mempool) [#14484](https://github.com/cosmos/cosmos-sdk/pull/14484) Add priority nonce mempool option for transaction replacement. +* (x/bank) [#14894](https://github.com/cosmos/cosmos-sdk/pull/14894) Return a human readable denomination for IBC vouchers when querying bank balances. Added a `ResolveDenom` parameter to `types.QueryAllBalancesRequest` and `--resolve-denom` flag to `GetBalancesCmd()`. +* (x/gov) [#15151](https://github.com/cosmos/cosmos-sdk/pull/15151) Add `burn_vote_quorum`, `burn_proposal_deposit_prevote` and `burn_vote_veto` params to allow applications to decide if they would like to burn deposits +* (runtime) [#15547](https://github.com/cosmos/cosmos-sdk/pull/15547) Allow runtime to pass event core api service to modules +* (telemetry) [#15657](https://github.com/cosmos/cosmos-sdk/pull/15657) Emit more data (go version, sdk version, upgrade height) in prom metrics +* (modulemanager) [#15829](https://github.com/cosmos/cosmos-sdk/pull/15829) add new endblocker interface to handle valset updates +* (core) [#14860](https://github.com/cosmos/cosmos-sdk/pull/14860) Add `Precommit` and `PrepareCheckState` AppModule callbacks. + +### Improvements + +* (x/slashing) [#15580](https://github.com/cosmos/cosmos-sdk/pull/15580) Refactor the validator's missed block signing window to be a chunked bitmap instead of a "logical" bitmap, significantly reducing the storage footprint. +>>>>>>> 97e4978f0 (feat: update the slashing and evidence modules to work with ICS (#15908)) * [#15448](https://github.com/cosmos/cosmos-sdk/pull/15448) Automatically populate the block timestamp for historical queries. In contexts where the block timestamp is needed for previous states, the timestamp will now be set. Note, when querying against a node it must be re-synced in order to be able to automatically populate the block timestamp. Otherwise, the block timestamp will be populated for heights going forward once upgraded. * [#14019](https://github.com/cosmos/cosmos-sdk/issues/14019) Remove the interface casting to allow other implementations of a `CommitMultiStore`. * (simtestutil) [#15903](https://github.com/cosmos/cosmos-sdk/pull/15903) Add `AppStateFnWithExtendedCbs` with moduleStateCb callback function to allow access moduleState. diff --git a/x/evidence/keeper/infraction.go b/x/evidence/keeper/infraction.go index 3b6d17042846..3a1dbd8abc02 100644 --- a/x/evidence/keeper/infraction.go +++ b/x/evidence/keeper/infraction.go @@ -27,6 +27,7 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi logger := k.Logger(ctx) consAddr := evidence.GetConsensusAddress() +<<<<<<< HEAD if _, err := k.slashingKeeper.GetPubkey(ctx, consAddr.Bytes()); err != nil { // Ignore evidence that cannot be handled. // @@ -37,9 +38,31 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi // allowable but none of the disallowed evidence types. Instead of // getting this coordination right, it is easier to relax the // constraints and ignore evidence that cannot be handled. +======= + validator := k.stakingKeeper.ValidatorByConsAddr(ctx, consAddr) + if validator == nil || validator.IsUnbonded() { + // Defensive: Simulation doesn't take unbonding periods into account, and + // CometBFT might break this assumption at some point. +>>>>>>> 97e4978f0 (feat: update the slashing and evidence modules to work with ICS (#15908)) return } + if !validator.GetOperator().Empty() { + if _, err := k.slashingKeeper.GetPubkey(ctx, consAddr.Bytes()); err != nil { + // Ignore evidence that cannot be handled. + // + // NOTE: We used to panic with: + // `panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr))`, + // but this couples the expectations of the app to both CometBFT and + // the simulator. Both are expected to provide the full range of + // allowable but none of the disallowed evidence types. Instead of + // getting this coordination right, it is easier to relax the + // constraints and ignore evidence that cannot be handled. + logger.Error(fmt.Sprintf("ignore evidence; expected public key for validator %s not found", consAddr)) + return + } + } + // calculate the age of the evidence infractionHeight := evidence.GetHeight() infractionTime := evidence.GetTime() @@ -64,6 +87,7 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi } } +<<<<<<< HEAD validator := k.stakingKeeper.ValidatorByConsAddr(ctx, consAddr) if validator == nil || validator.IsUnbonded() { // Defensive: Simulation doesn't take unbonding periods into account, and @@ -86,6 +110,8 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi } } +======= +>>>>>>> 97e4978f0 (feat: update the slashing and evidence modules to work with ICS (#15908)) if ok := k.slashingKeeper.HasValidatorSigningInfo(ctx, consAddr); !ok { panic(fmt.Sprintf("expected signing info for validator %s but not found", consAddr)) } diff --git a/x/slashing/keeper/infractions.go b/x/slashing/keeper/infractions.go index 17f56d04c6cc..b4f473d6a378 100644 --- a/x/slashing/keeper/infractions.go +++ b/x/slashing/keeper/infractions.go @@ -16,9 +16,6 @@ func (k Keeper) HandleValidatorSignature(ctx sdk.Context, addr cryptotypes.Addre // fetch the validator public key consAddr := sdk.ConsAddress(addr) - if _, err := k.GetPubkey(ctx, addr); err != nil { - panic(fmt.Sprintf("Validator consensus-address %s not found", consAddr)) - } // don't update missed blocks when validator's jailed if k.sk.IsValidatorJailed(ctx, consAddr) { From 6c6fa064b729336b0856414b771bee3b791cd78b Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 25 Apr 2023 19:24:33 +0200 Subject: [PATCH 2/2] fix conflicts --- CHANGELOG.md | 34 +++-------------------------- x/evidence/keeper/infraction.go | 38 --------------------------------- 2 files changed, 3 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 231a33d2587e..309a7f085c03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,42 +37,14 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] -## Improvements +## Features -<<<<<<< HEAD -* (store) [#15683](https://github.com/cosmos/cosmos-sdk/pull/15683) `rootmulti.Store.CacheMultiStoreWithVersion` now can handle loading archival states that don't persist any of the module stores the current state has. -======= * (x/evidence) [#15908](https://github.com/cosmos/cosmos-sdk/pull/15908) Update the equivocation handler to work with ICS by removing a pubkey check that was performing a no-op for consumer chains. * (x/slashing) [#15908](https://github.com/cosmos/cosmos-sdk/pull/15908) Remove the validators' pubkey check in the signature handler in order to work with ICS. -* (runtime) [#15818](https://github.com/cosmos/cosmos-sdk/pull/15818) Provide logger through `depinject` instead of appBuilder. -* (client) [#15597](https://github.com/cosmos/cosmos-sdk/pull/15597) Add status endpoint for clients. -* (testutil/integration) [#15556](https://github.com/cosmos/cosmos-sdk/pull/15556) Introduce `testutil/integration` package for module integration testing. -* (types) [#15735](https://github.com/cosmos/cosmos-sdk/pull/15735) Make `ValidateBasic() error` method of `Msg` interface optional. Modules should validate messages directly in their message handlers ([RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation)). -* (x/genutil) [#15679](https://github.com/cosmos/cosmos-sdk/pull/15679) Allow applications to specify a custom genesis migration function for the `genesis migrate` command. -* (client) [#15458](https://github.com/cosmos/cosmos-sdk/pull/15458) Add a `CmdContext` field to client.Context initialized to cobra command's context. -* (core) [#15133](https://github.com/cosmos/cosmos-sdk/pull/15133) Implement RegisterServices in the module manager. -* (x/gov) [#14373](https://github.com/cosmos/cosmos-sdk/pull/14373) Add new proto field `constitution` of type `string` to gov module genesis state, which allows chain builders to lay a strong foundation by specifying purpose. -* (x/genutil) [#15301](https://github.com/cosmos/cosmos-sdk/pull/15031) Add application genesis. The genesis is now entirely managed by the application and passed to CometBFT at note instantiation. Functions that were taking a `cmttypes.GenesisDoc{}` now takes a `genutiltypes.AppGenesis{}`. -* (cli) [#14659](https://github.com/cosmos/cosmos-sdk/pull/14659) Added ability to query blocks by events with queries directly passed to Tendermint, which will allow for full query operator support, e.g. `>`. -* (x/gov) [#14720](https://github.com/cosmos/cosmos-sdk/pull/14720) Upstream expedited proposals from Osmosis. -* (x/auth) [#14650](https://github.com/cosmos/cosmos-sdk/pull/14650) Add Textual SignModeHandler. It is however **NOT** enabled by default, and should only be used for **TESTING** purposes until `SIGN_MODE_TEXTUAL` is fully released. -* (x/crisis) [#14588](https://github.com/cosmos/cosmos-sdk/pull/14588) Use CacheContext() in AssertInvariants() -* (client) [#14342](https://github.com/cosmos/cosmos-sdk/pull/14342) Add ` config` command is now a sub-command, for setting, getting and migrating Cosmos SDK configuration files. -* (query) [#14468](https://github.com/cosmos/cosmos-sdk/pull/14468) Implement pagination for collections. -* (x/distribution) [#14322](https://github.com/cosmos/cosmos-sdk/pull/14322) Introduce a new gRPC message handler, `DepositValidatorRewardsPool`, that allows explicit funding of a validator's reward pool. -* [#13473](https://github.com/cosmos/cosmos-sdk/pull/13473) ADR-038: Go plugin system proposal -* (mempool) [#14484](https://github.com/cosmos/cosmos-sdk/pull/14484) Add priority nonce mempool option for transaction replacement. -* (x/bank) [#14894](https://github.com/cosmos/cosmos-sdk/pull/14894) Return a human readable denomination for IBC vouchers when querying bank balances. Added a `ResolveDenom` parameter to `types.QueryAllBalancesRequest` and `--resolve-denom` flag to `GetBalancesCmd()`. -* (x/gov) [#15151](https://github.com/cosmos/cosmos-sdk/pull/15151) Add `burn_vote_quorum`, `burn_proposal_deposit_prevote` and `burn_vote_veto` params to allow applications to decide if they would like to burn deposits -* (runtime) [#15547](https://github.com/cosmos/cosmos-sdk/pull/15547) Allow runtime to pass event core api service to modules -* (telemetry) [#15657](https://github.com/cosmos/cosmos-sdk/pull/15657) Emit more data (go version, sdk version, upgrade height) in prom metrics -* (modulemanager) [#15829](https://github.com/cosmos/cosmos-sdk/pull/15829) add new endblocker interface to handle valset updates -* (core) [#14860](https://github.com/cosmos/cosmos-sdk/pull/14860) Add `Precommit` and `PrepareCheckState` AppModule callbacks. -### Improvements +## Improvements -* (x/slashing) [#15580](https://github.com/cosmos/cosmos-sdk/pull/15580) Refactor the validator's missed block signing window to be a chunked bitmap instead of a "logical" bitmap, significantly reducing the storage footprint. ->>>>>>> 97e4978f0 (feat: update the slashing and evidence modules to work with ICS (#15908)) +* (store) [#15683](https://github.com/cosmos/cosmos-sdk/pull/15683) `rootmulti.Store.CacheMultiStoreWithVersion` now can handle loading archival states that don't persist any of the module stores the current state has. * [#15448](https://github.com/cosmos/cosmos-sdk/pull/15448) Automatically populate the block timestamp for historical queries. In contexts where the block timestamp is needed for previous states, the timestamp will now be set. Note, when querying against a node it must be re-synced in order to be able to automatically populate the block timestamp. Otherwise, the block timestamp will be populated for heights going forward once upgraded. * [#14019](https://github.com/cosmos/cosmos-sdk/issues/14019) Remove the interface casting to allow other implementations of a `CommitMultiStore`. * (simtestutil) [#15903](https://github.com/cosmos/cosmos-sdk/pull/15903) Add `AppStateFnWithExtendedCbs` with moduleStateCb callback function to allow access moduleState. diff --git a/x/evidence/keeper/infraction.go b/x/evidence/keeper/infraction.go index 3a1dbd8abc02..f71f967b79dd 100644 --- a/x/evidence/keeper/infraction.go +++ b/x/evidence/keeper/infraction.go @@ -27,23 +27,10 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi logger := k.Logger(ctx) consAddr := evidence.GetConsensusAddress() -<<<<<<< HEAD - if _, err := k.slashingKeeper.GetPubkey(ctx, consAddr.Bytes()); err != nil { - // Ignore evidence that cannot be handled. - // - // NOTE: We used to panic with: - // `panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr))`, - // but this couples the expectations of the app to both Tendermint and - // the simulator. Both are expected to provide the full range of - // allowable but none of the disallowed evidence types. Instead of - // getting this coordination right, it is easier to relax the - // constraints and ignore evidence that cannot be handled. -======= validator := k.stakingKeeper.ValidatorByConsAddr(ctx, consAddr) if validator == nil || validator.IsUnbonded() { // Defensive: Simulation doesn't take unbonding periods into account, and // CometBFT might break this assumption at some point. ->>>>>>> 97e4978f0 (feat: update the slashing and evidence modules to work with ICS (#15908)) return } @@ -87,31 +74,6 @@ func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi } } -<<<<<<< HEAD - validator := k.stakingKeeper.ValidatorByConsAddr(ctx, consAddr) - if validator == nil || validator.IsUnbonded() { - // Defensive: Simulation doesn't take unbonding periods into account, and - // Tendermint might break this assumption at some point. - return - } - - if !validator.GetOperator().Empty() { - if _, err := k.slashingKeeper.GetPubkey(ctx, consAddr.Bytes()); err != nil { - // Ignore evidence that cannot be handled. - // - // NOTE: We used to panic with: - // `panic(fmt.Sprintf("Validator consensus-address %v not found", consAddr))`, - // but this couples the expectations of the app to both Tendermint and - // the simulator. Both are expected to provide the full range of - // allowable but none of the disallowed evidence types. Instead of - // getting this coordination right, it is easier to relax the - // constraints and ignore evidence that cannot be handled. - return - } - } - -======= ->>>>>>> 97e4978f0 (feat: update the slashing and evidence modules to work with ICS (#15908)) if ok := k.slashingKeeper.HasValidatorSigningInfo(ctx, consAddr); !ok { panic(fmt.Sprintf("expected signing info for validator %s but not found", consAddr)) }