Skip to content

Commit

Permalink
feat: update the slashing and evidence modules to work with ICS (#15908)
Browse files Browse the repository at this point in the history
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Jacob Gadikian <faddat@users.noreply.github.com>
  • Loading branch information
3 people authored Apr 25, 2023
1 parent ebcad58 commit 97e4978
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 35 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
52 changes: 20 additions & 32 deletions x/evidence/keeper/infraction.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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))
}
Expand Down
3 changes: 0 additions & 3 deletions x/slashing/keeper/infractions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 97e4978

Please sign in to comment.