From 97e4978f0f773d49a122a353ad117e58ff6ee50c Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Tue, 25 Apr 2023 18:34:44 +0200 Subject: [PATCH] feat: update the slashing and evidence modules to work with ICS (#15908) Co-authored-by: Julien Robert Co-authored-by: Jacob Gadikian --- CHANGELOG.md | 2 ++ x/evidence/keeper/infraction.go | 52 ++++++++++++-------------------- x/slashing/keeper/infractions.go | 3 -- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bd5a7efe6..e0237bfe66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* (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. diff --git a/x/evidence/keeper/infraction.go b/x/evidence/keeper/infraction.go index 9526c0a29f..eb4b517f4f 100644 --- a/x/evidence/keeper/infraction.go +++ b/x/evidence/keeper/infraction.go @@ -27,19 +27,29 @@ func (k Keeper) handleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi logger := k.Logger(ctx) consAddr := evidence.GetConsensusAddress() - 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. + 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. 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,28 +74,6 @@ func (k Keeper) handleEquivocationEvidence(ctx sdk.Context, evidence *types.Equi } } - 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. - 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. - return - } - } - 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 557a48a639..0ad33ab85b 100644 --- a/x/slashing/keeper/infractions.go +++ b/x/slashing/keeper/infractions.go @@ -18,9 +18,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) {