Skip to content

Commit

Permalink
remove tombstoning and evidence age check
Browse files Browse the repository at this point in the history
  • Loading branch information
sainoe committed Aug 25, 2023
1 parent f0b44c2 commit 18ae136
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 80 deletions.
15 changes: 6 additions & 9 deletions tests/integration/double_vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

// TestHandleConsumerDoubleVoting verifies that handling a double voting evidence
// of a consumer chain results in the expected jailing and tombstoning of the malicious validator
// of a consumer chain results in the expected jailing of the malicious validator
func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
s.SetupCCVChannel(s.path)
// required to have the consumer client revision height greater than 0
Expand Down Expand Up @@ -98,25 +98,22 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
provAddr := s.providerApp.GetProviderKeeper().GetProviderAddrFromConsumerAddr(s.providerCtx(), s.consumerChain.ChainID, consuAddr)

for _, tc := range testCases {
ctx := s.providerCtx().WithConsensusParams(testutil.GetDefaultConsensusEvidenceParams())
s.Run(tc.name, func() {
err = s.providerApp.GetProviderKeeper().HandleConsumerDoubleVoting(
ctx,
s.providerCtx(),
tc.ev,
tc.chainID,
)
if tc.expPass {
s.Require().NoError(err)

// verifies that the jailing and tombstoning has occurred
s.Require().True(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(ctx, provAddr.ToSdkConsAddr()))
s.Require().True(s.providerApp.GetTestSlashingKeeper().IsTombstoned(ctx, provAddr.ToSdkConsAddr()))
// verifies that the jailing has occurred
s.Require().True(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(s.providerCtx(), provAddr.ToSdkConsAddr()))
} else {
s.Require().Error(err)

// verifies that no jailing and tombstoning has occurred
s.Require().False(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(ctx, provAddr.ToSdkConsAddr()))
s.Require().False(s.providerApp.GetTestSlashingKeeper().IsTombstoned(ctx, provAddr.ToSdkConsAddr()))
// verifies that no jailing and has occurred
s.Require().False(s.providerApp.GetTestStakingKeeper().IsValidatorJailed(s.providerCtx(), provAddr.ToSdkConsAddr()))
}
})
}
Expand Down
3 changes: 1 addition & 2 deletions tests/integration/misbehaviour.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

// TestHandleConsumerMisbehaviour tests that handling a valid misbehaviour,
// with conflicting headers forming an equivocation, results in the jailing and tombstoning of the validators
// with conflicting headers forming an equivocation, results in the jailing of the validators
func (s *CCVTestSuite) TestHandleConsumerMisbehaviour() {
s.SetupCCVChannel(s.path)
// required to have the consumer client revision height greater than 0
Expand Down Expand Up @@ -63,7 +63,6 @@ func (s *CCVTestSuite) TestHandleConsumerMisbehaviour() {
val, ok := s.providerApp.GetTestStakingKeeper().GetValidatorByConsAddr(s.providerCtx(), provAddr.Address)
s.Require().True(ok)
s.Require().True(val.Jailed)
s.Require().True(s.providerApp.GetTestSlashingKeeper().IsTombstoned(s.providerCtx(), provAddr.Address))
}
}

Expand Down
12 changes: 0 additions & 12 deletions testutil/crypto/evidence.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ package crypto
import (
"time"

abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/crypto/tmhash"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
tmtypes "github.com/tendermint/tendermint/types"
)

Expand Down Expand Up @@ -56,13 +54,3 @@ func MakeAndSignVote(
vote.Signature = v.Signature
return vote
}

func GetDefaultConsensusEvidenceParams() *abci.ConsensusParams {
return &abci.ConsensusParams{
Evidence: &tmproto.EvidenceParams{
MaxAgeNumBlocks: 302400,
MaxAgeDuration: 504 * time.Hour, // 3 weeks is the max duration
MaxBytes: 10000,
},
}
}
23 changes: 9 additions & 14 deletions x/ccv/provider/keeper/double_vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

// HandleConsumerDoubleVoting verifies a double voting evidence for a given a consumer chain and,
// if successful, executes the the jailing and the tombstoning of the malicious validator.
// if successful, executes the jailing and of the malicious validator.
func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.DuplicateVoteEvidence, chainID string) error {
// get the validator's consensus address on the provider
providerAddr := k.GetProviderAddrFromConsumerAddr(
Expand All @@ -34,8 +34,8 @@ func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.Du
return err
}

// execute the jailing and tombstoning
k.JailAndTombstoneValidator(ctx, providerAddr)
// execute the jailing
k.JailValidator(ctx, providerAddr)

k.Logger(ctx).Info(
"confirmed equivocation",
Expand All @@ -53,17 +53,12 @@ func (k Keeper) VerifyDoubleVotingEvidence(
chainID string,
pubkey tmprotocrypto.PublicKey,
) error {
// check evidence age duration
maxAgeDuration := ctx.ConsensusParams().Evidence.MaxAgeDuration
ageDuration := ctx.BlockHeader().Time.Sub(evidence.Time())

if ageDuration > maxAgeDuration {
return fmt.Errorf(
"evidence created at: %v is too old; evidence can not be older than %v",
evidence.Time(),
ctx.BlockHeader().Time.Add(maxAgeDuration),
)
}

Check failure on line 56 in x/ccv/provider/keeper/double_vote.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gofumpt`-ed with `-extra` (gofumpt)
// Note that since we don't slash validators for double signing
// on a consumer chain, we don't need to check the age of the evidence

// TODO: check the age of the evidence once we integrate
// the slashing to HandleConsumerDoubleVoting

// H/R/S must be the same
if evidence.VoteA.Height != evidence.VoteB.Height ||
Expand Down
29 changes: 0 additions & 29 deletions x/ccv/provider/keeper/double_vote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ func TestVerifyDoubleVotingEvidence(t *testing.T) {
blockID2 := testutil.MakeBlockID([]byte("blockhash2"), 1000, []byte("partshash"))

ctx = ctx.WithBlockTime(time.Now())
oldTime := ctx.BlockTime().
Add(-testutil.GetDefaultConsensusEvidenceParams().Evidence.MaxAgeDuration).
Add(-1 * time.Hour)

valPubkey1, err := tmencoding.PubKeyToProto(val1.PubKey)
require.NoError(t, err)
Expand All @@ -47,30 +44,6 @@ func TestVerifyDoubleVotingEvidence(t *testing.T) {
pubkey tmprotocrypto.PublicKey
expPass bool
}{
{
"evidence is too old - shouldn't pass",
[]*tmtypes.Vote{
testutil.MakeAndSignVote(
blockID1,
ctx.BlockHeight(),
oldTime,
valSet,
signer1,
chainID,
),
testutil.MakeAndSignVote(
blockID2,
ctx.BlockHeight(),
oldTime,
valSet,
signer1,
chainID,
),
},
chainID,
valPubkey1,
false,
},
{
"evidence has votes with different block height - shouldn't pass",
[]*tmtypes.Vote{
Expand Down Expand Up @@ -289,8 +262,6 @@ func TestVerifyDoubleVotingEvidence(t *testing.T) {
},
}

ctx = ctx.WithConsensusParams(testutil.GetDefaultConsensusEvidenceParams())

for _, tc := range testCases {
err = keeper.VerifyDoubleVotingEvidence(
ctx,
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/misbehaviour.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (k Keeper) HandleConsumerMisbehaviour(ctx sdk.Context, misbehaviour ibctmty
misbehaviour.Header1.Header.ChainID,
types.NewConsumerConsAddress(sdk.ConsAddress(v.Address.Bytes())),
)
k.JailAndTombstoneValidator(ctx, providerAddr)
k.JailValidator(ctx, providerAddr)
provAddrs = append(provAddrs, providerAddr)
}

Expand Down
13 changes: 8 additions & 5 deletions x/ccv/provider/keeper/punish_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ import (
"github.com/cosmos/interchain-security/v2/x/ccv/provider/types"
)

// JailAndTombstoneValidator jails and tombstones the validator for the given validator consensus address
func (k Keeper) JailAndTombstoneValidator(ctx sdk.Context, providerAddr types.ProviderConsAddress) {
// JailValidator jails the validator for the given validator consensus address
// Note that the tombstoning is temporarily removed until we integrate the slashing
// for consumer double signing and light client attacks,
// see comments https://github.com/cosmos/interchain-security/pull/1232#issuecomment-1693127641.
func (k Keeper) JailValidator(ctx sdk.Context, providerAddr types.ProviderConsAddress) {
logger := k.Logger(ctx)

// get validator
Expand All @@ -17,7 +20,7 @@ func (k Keeper) JailAndTombstoneValidator(ctx sdk.Context, providerAddr types.Pr
return
}

// check that validator isn't already tombstoned
// check that validator isn't tombstoned
if k.slashingKeeper.IsTombstoned(ctx, providerAddr.ToSdkConsAddr()) {
logger.Info("validator is already tombstoned", "provider cons addr", providerAddr.String())
return
Expand All @@ -30,6 +33,6 @@ func (k Keeper) JailAndTombstoneValidator(ctx sdk.Context, providerAddr types.Pr

// update jail time to end after double sign jail duration
k.slashingKeeper.JailUntil(ctx, providerAddr.ToSdkConsAddr(), evidencetypes.DoubleSignJailEndTime)
// tombstone validator
k.slashingKeeper.Tombstone(ctx, providerAddr.ToSdkConsAddr())

// TODO: add tombstoning back once we integrate the slashing
}
10 changes: 2 additions & 8 deletions x/ccv/provider/keeper/punish_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/golang/mock/gomock"
)

func TestJailAndTombstoneValidator(t *testing.T) {
func TestJailValidator(t *testing.T) {
providerConsAddr := cryptotestutil.NewCryptoIdentityFromIntSeed(7842334).ProviderConsAddress()
testCases := []struct {
name string
Expand Down Expand Up @@ -86,9 +86,6 @@ func TestJailAndTombstoneValidator(t *testing.T) {
mocks.MockSlashingKeeper.EXPECT().JailUntil(
ctx, providerConsAddr.ToSdkConsAddr(), evidencetypes.DoubleSignJailEndTime).
Times(1),
mocks.MockSlashingKeeper.EXPECT().Tombstone(
ctx, providerConsAddr.ToSdkConsAddr()).
Times(1),
}
},
},
Expand All @@ -113,9 +110,6 @@ func TestJailAndTombstoneValidator(t *testing.T) {
mocks.MockSlashingKeeper.EXPECT().JailUntil(
ctx, providerConsAddr.ToSdkConsAddr(), evidencetypes.DoubleSignJailEndTime).
Times(1),
mocks.MockSlashingKeeper.EXPECT().Tombstone(
ctx, providerConsAddr.ToSdkConsAddr()).
Times(1),
}
},
},
Expand All @@ -129,7 +123,7 @@ func TestJailAndTombstoneValidator(t *testing.T) {
gomock.InOrder(tc.expectedCalls(ctx, mocks, tc.provAddr)...)

// Execute method and assert expected mock calls
providerKeeper.JailAndTombstoneValidator(ctx, tc.provAddr)
providerKeeper.JailValidator(ctx, tc.provAddr)

ctrl.Finish()
}
Expand Down

0 comments on commit 18ae136

Please sign in to comment.