From ac345ce694cf89a57227921d55b5384da14c13b4 Mon Sep 17 00:00:00 2001 From: Marko Date: Fri, 17 Mar 2023 18:20:32 +0100 Subject: [PATCH] refactor: evidence make equivocation private (#15420) --- .../evidence/keeper/infraction_test.go | 33 +++++++++++-------- x/evidence/CHANGELOG.md | 3 +- x/evidence/{ => keeper}/abci.go | 7 ++-- x/evidence/keeper/infraction.go | 2 +- x/evidence/module.go | 2 +- x/evidence/types/evidence.go | 2 +- x/evidence/types/evidence_test.go | 2 +- 7 files changed, 29 insertions(+), 22 deletions(-) rename x/evidence/{ => keeper}/abci.go (82%) diff --git a/tests/integration/evidence/keeper/infraction_test.go b/tests/integration/evidence/keeper/infraction_test.go index 3bf9fd9bd3..a539474860 100644 --- a/tests/integration/evidence/keeper/infraction_test.go +++ b/tests/integration/evidence/keeper/infraction_test.go @@ -10,6 +10,7 @@ import ( "cosmossdk.io/x/evidence/keeper" "cosmossdk.io/x/evidence/testutil" "cosmossdk.io/x/evidence/types" + abci "github.com/cometbft/cometbft/abci/types" cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" "gotest.tools/v3/assert" @@ -111,13 +112,16 @@ func TestHandleDoubleSign(t *testing.T) { // double sign less than max age oldTokens := f.stakingKeeper.Validator(ctx, operatorAddr).GetTokens() - evidence := &types.Equivocation{ - Height: 0, - Time: time.Unix(0, 0), - Power: power, - ConsensusAddress: sdk.ConsAddress(val.Address()).String(), + evidence := abci.RequestBeginBlock{ + ByzantineValidators: []abci.Misbehavior{{ + Validator: abci.Validator{Address: val.Address(), Power: power}, + Type: abci.MisbehaviorType_DUPLICATE_VOTE, + Time: time.Unix(0, 0), + Height: 0, + }}, } - f.evidenceKeeper.HandleEquivocationEvidence(ctx, evidence) + + f.evidenceKeeper.BeginBlocker(ctx, evidence) // should be jailed and tombstoned assert.Assert(t, f.stakingKeeper.Validator(ctx, operatorAddr).IsJailed()) @@ -128,7 +132,7 @@ func TestHandleDoubleSign(t *testing.T) { assert.Assert(t, newTokens.LT(oldTokens)) // submit duplicate evidence - f.evidenceKeeper.HandleEquivocationEvidence(ctx, evidence) + f.evidenceKeeper.BeginBlocker(ctx, evidence) // tokens should be the same (capped slash) assert.Assert(t, f.stakingKeeper.Validator(ctx, operatorAddr).GetTokens().Equal(newTokens)) @@ -175,11 +179,13 @@ func TestHandleDoubleSign_TooOld(t *testing.T) { ) assert.DeepEqual(t, amt, f.stakingKeeper.Validator(ctx, operatorAddr).GetBondedTokens()) - evidence := &types.Equivocation{ - Height: 0, - Time: ctx.BlockTime(), - Power: power, - ConsensusAddress: sdk.ConsAddress(val.Address()).String(), + evidence := abci.RequestBeginBlock{ + ByzantineValidators: []abci.Misbehavior{{ + Validator: abci.Validator{Address: val.Address(), Power: power}, + Type: abci.MisbehaviorType_DUPLICATE_VOTE, + Time: ctx.BlockTime(), + Height: 0, + }}, } cp := f.app.BaseApp.GetConsensusParams(ctx) @@ -187,7 +193,8 @@ func TestHandleDoubleSign_TooOld(t *testing.T) { ctx = ctx.WithConsensusParams(cp) ctx = ctx.WithBlockTime(ctx.BlockTime().Add(cp.Evidence.MaxAgeDuration + 1)) ctx = ctx.WithBlockHeight(ctx.BlockHeight() + cp.Evidence.MaxAgeNumBlocks + 1) - f.evidenceKeeper.HandleEquivocationEvidence(ctx, evidence) + + f.evidenceKeeper.BeginBlocker(ctx, evidence) assert.Assert(t, f.stakingKeeper.Validator(ctx, operatorAddr).IsJailed() == false) assert.Assert(t, f.slashingKeeper.IsTombstoned(ctx, sdk.ConsAddress(val.Address())) == false) diff --git a/x/evidence/CHANGELOG.md b/x/evidence/CHANGELOG.md index bf9e4f9ad7..c54edf89ad 100644 --- a/x/evidence/CHANGELOG.md +++ b/x/evidence/CHANGELOG.md @@ -27,4 +27,5 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features -* (x/evidence) [14724](https://github.com/cosmos/cosmos-sdk/pull/14724) The `x/evidence` module is extracted to have a separate go.mod file which allows it be a standalone module. \ No newline at end of file +* (x/evidence) [14724](https://github.com/cosmos/cosmos-sdk/pull/14724) The `x/evidence` module is extracted to have a separate go.mod file which allows it be a standalone module. +* (keeper) [#15420](https://github.com/cosmos/cosmos-sdk/pull/15420) Move `BeginBlocker` to the keeper folder & make HandleEquivocation private diff --git a/x/evidence/abci.go b/x/evidence/keeper/abci.go similarity index 82% rename from x/evidence/abci.go rename to x/evidence/keeper/abci.go index fcfeef3716..cfa9fffa30 100644 --- a/x/evidence/abci.go +++ b/x/evidence/keeper/abci.go @@ -1,10 +1,9 @@ -package evidence +package keeper import ( "fmt" "time" - "cosmossdk.io/x/evidence/keeper" "cosmossdk.io/x/evidence/types" abci "github.com/cometbft/cometbft/abci/types" @@ -14,7 +13,7 @@ import ( // BeginBlocker iterates through and handles any newly discovered evidence of // misbehavior submitted by CometBFT. Currently, only equivocation is handled. -func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) { +func (k Keeper) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) { defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyBeginBlocker) for _, tmEvidence := range req.ByzantineValidators { @@ -23,7 +22,7 @@ func BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock, k keeper.Keeper) // premeditation. So for now we agree to treat them in the same way. case abci.MisbehaviorType_DUPLICATE_VOTE, abci.MisbehaviorType_LIGHT_CLIENT_ATTACK: evidence := types.FromABCIEvidence(tmEvidence) - k.HandleEquivocationEvidence(ctx, evidence.(*types.Equivocation)) + k.handleEquivocationEvidence(ctx, evidence) default: k.Logger(ctx).Error(fmt.Sprintf("ignored unknown evidence type: %s", tmEvidence.Type)) diff --git a/x/evidence/keeper/infraction.go b/x/evidence/keeper/infraction.go index 9859ba91a9..b9713c01fc 100644 --- a/x/evidence/keeper/infraction.go +++ b/x/evidence/keeper/infraction.go @@ -23,7 +23,7 @@ import ( // // TODO: Some of the invalid constraints listed above may need to be reconsidered // in the case of a lunatic attack. -func (k Keeper) HandleEquivocationEvidence(ctx sdk.Context, evidence *types.Equivocation) { +func (k Keeper) handleEquivocationEvidence(ctx sdk.Context, evidence *types.Equivocation) { logger := k.Logger(ctx) consAddr := evidence.GetConsensusAddress() diff --git a/x/evidence/module.go b/x/evidence/module.go index 87e87fde56..755e31cfb2 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -164,7 +164,7 @@ func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock executes all ABCI BeginBlock logic respective to the evidence module. func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { - BeginBlocker(ctx, req, am.keeper) + am.keeper.BeginBlocker(ctx, req) } // AppModuleSimulation functions diff --git a/x/evidence/types/evidence.go b/x/evidence/types/evidence.go index 8d26304708..622907d683 100644 --- a/x/evidence/types/evidence.go +++ b/x/evidence/types/evidence.go @@ -74,7 +74,7 @@ func (e Equivocation) GetTotalPower() int64 { return 0 } // FromABCIEvidence converts a CometBFT concrete Evidence type to // SDK Evidence using Equivocation as the concrete type. -func FromABCIEvidence(e abci.Misbehavior) exported.Evidence { +func FromABCIEvidence(e abci.Misbehavior) *Equivocation { bech32PrefixConsAddr := sdk.GetConfig().GetBech32ConsensusAddrPrefix() consAddr, err := sdk.Bech32ifyAddressBytes(bech32PrefixConsAddr, e.Validator.Address) if err != nil { diff --git a/x/evidence/types/evidence_test.go b/x/evidence/types/evidence_test.go index 29ec7b442b..f5a174ca31 100644 --- a/x/evidence/types/evidence_test.go +++ b/x/evidence/types/evidence_test.go @@ -82,7 +82,7 @@ func TestEvidenceAddressConversion(t *testing.T) { Time: time.Now(), TotalVotingPower: 100, } - evidence := types.FromABCIEvidence(tmEvidence).(*types.Equivocation) + evidence := types.FromABCIEvidence(tmEvidence) consAddr := evidence.GetConsensusAddress() // Check the address is the same after conversion require.Equal(t, tmEvidence.Validator.Address, consAddr.Bytes())