diff --git a/tests/integration/double_vote.go b/tests/integration/double_vote.go index 112671ec67..afdd0894a0 100644 --- a/tests/integration/double_vote.go +++ b/tests/integration/double_vote.go @@ -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 @@ -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())) } }) } diff --git a/tests/integration/misbehaviour.go b/tests/integration/misbehaviour.go index c3b590d5c1..f7cebc84c0 100644 --- a/tests/integration/misbehaviour.go +++ b/tests/integration/misbehaviour.go @@ -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 @@ -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)) } } diff --git a/testutil/crypto/evidence.go b/testutil/crypto/evidence.go index 544210a218..050c17331a 100644 --- a/testutil/crypto/evidence.go +++ b/testutil/crypto/evidence.go @@ -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" ) @@ -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, - }, - } -} diff --git a/x/ccv/provider/keeper/double_vote.go b/x/ccv/provider/keeper/double_vote.go index a398c02316..1f18de8e54 100644 --- a/x/ccv/provider/keeper/double_vote.go +++ b/x/ccv/provider/keeper/double_vote.go @@ -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( @@ -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", @@ -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), - ) - } + + // 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 || diff --git a/x/ccv/provider/keeper/double_vote_test.go b/x/ccv/provider/keeper/double_vote_test.go index 2d15ed4be6..9bb789c381 100644 --- a/x/ccv/provider/keeper/double_vote_test.go +++ b/x/ccv/provider/keeper/double_vote_test.go @@ -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) @@ -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{ @@ -289,8 +262,6 @@ func TestVerifyDoubleVotingEvidence(t *testing.T) { }, } - ctx = ctx.WithConsensusParams(testutil.GetDefaultConsensusEvidenceParams()) - for _, tc := range testCases { err = keeper.VerifyDoubleVotingEvidence( ctx, diff --git a/x/ccv/provider/keeper/misbehaviour.go b/x/ccv/provider/keeper/misbehaviour.go index 675d22c3e0..35d9219324 100644 --- a/x/ccv/provider/keeper/misbehaviour.go +++ b/x/ccv/provider/keeper/misbehaviour.go @@ -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) } diff --git a/x/ccv/provider/keeper/punish_validator.go b/x/ccv/provider/keeper/punish_validator.go index 3510483363..95dced731d 100644 --- a/x/ccv/provider/keeper/punish_validator.go +++ b/x/ccv/provider/keeper/punish_validator.go @@ -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 @@ -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 @@ -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 } diff --git a/x/ccv/provider/keeper/punish_validator_test.go b/x/ccv/provider/keeper/punish_validator_test.go index f768f8ee1b..748b463b8a 100644 --- a/x/ccv/provider/keeper/punish_validator_test.go +++ b/x/ccv/provider/keeper/punish_validator_test.go @@ -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 @@ -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), } }, }, @@ -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), } }, }, @@ -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() }