Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update the slashing and evidence modules to work with ICS #15908

Merged
merged 12 commits into from
Apr 25, 2023
13 changes: 0 additions & 13 deletions x/evidence/keeper/infraction.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,6 @@ 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.
return
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do not panic l75, can we at least log something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we move the validator and pubkey check above this and we can short-circuit everything else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can replicate this no-op in the slashing infractions.go ?

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as duplicate.

This comment was marked as off-topic.

// calculate the age of the evidence
infractionHeight := evidence.GetHeight()
infractionTime := evidence.GetTime()
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 {

This comment was marked as off-topic.

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
4 changes: 4 additions & 0 deletions yarn.lock
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1