diff --git a/tests/integration/double_vote.go b/tests/integration/double_vote.go index afdd0894a0..04d976ed61 100644 --- a/tests/integration/double_vote.go +++ b/tests/integration/double_vote.go @@ -28,6 +28,8 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { blockID1 := testutil.MakeBlockID([]byte("blockhash"), 1000, []byte("partshash")) blockID2 := testutil.MakeBlockID([]byte("blockhash2"), 1000, []byte("partshash")) + // Note that votes are signed along with the chain ID + // see VoteSignBytes in https://github.com/cometbft/cometbft/blob/main/types/vote.go#L139 vote1 := testutil.MakeAndSignVote( blockID1, s.consumerCtx().BlockHeight(), @@ -80,7 +82,8 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() { { // In order to create an evidence for a consumer chain, // we create two votes that only differ by their Block IDs and - // signed them using the same validator and chain ID of the consumer chain + // signed them using the same validator private key and chain ID + // of the consumer chain "valid double voting evidence - should pass", &tmtypes.DuplicateVoteEvidence{ VoteA: vote1, diff --git a/x/ccv/provider/keeper/double_vote.go b/x/ccv/provider/keeper/double_vote.go index 4ee443ea50..ee47e233ee 100644 --- a/x/ccv/provider/keeper/double_vote.go +++ b/x/ccv/provider/keeper/double_vote.go @@ -46,18 +46,18 @@ func (k Keeper) HandleConsumerDoubleVoting(ctx sdk.Context, evidence *tmtypes.Du } // VerifyDoubleVotingEvidence verifies a double voting evidence -// for a given chain id and validator public key +// for a given chain id and a validator public key func (k Keeper) VerifyDoubleVotingEvidence( ctx sdk.Context, evidence tmtypes.DuplicateVoteEvidence, chainID string, pubkey tmprotocrypto.PublicKey, ) error { - // 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 + // Note that since we're only jailing validators for double voting on a consumer chain, + // the age of the evidence is irrelevant and therefore isn't checked. - // TODO: check the age of the evidence once we integrate - // the slashing to HandleConsumerDoubleVoting + // TODO: check the age of the evidence once we slash + // validators for double voting on a consumer chain // H/R/S must be the same if evidence.VoteA.Height != evidence.VoteB.Height || diff --git a/x/ccv/provider/keeper/punish_validator.go b/x/ccv/provider/keeper/punish_validator.go index 95dced731d..f4648cc641 100644 --- a/x/ccv/provider/keeper/punish_validator.go +++ b/x/ccv/provider/keeper/punish_validator.go @@ -6,10 +6,10 @@ import ( "github.com/cosmos/interchain-security/v2/x/ccv/provider/types" ) -// 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. +// JailValidator jails the validator with the given provider consensus address +// Note that the tombstoning is temporarily removed until we slash validator +// for double signing on a consumer chain, see comment +// https://github.com/cosmos/interchain-security/pull/1232#issuecomment-1693127641. func (k Keeper) JailValidator(ctx sdk.Context, providerAddr types.ProviderConsAddress) { logger := k.Logger(ctx) @@ -20,7 +20,7 @@ func (k Keeper) JailValidator(ctx sdk.Context, providerAddr types.ProviderConsAd return } - // check that validator isn't tombstoned + // check that the validator isn't tombstoned if k.slashingKeeper.IsTombstoned(ctx, providerAddr.ToSdkConsAddr()) { logger.Info("validator is already tombstoned", "provider cons addr", providerAddr.String()) return diff --git a/x/ccv/provider/keeper/punish_validator_test.go b/x/ccv/provider/keeper/punish_validator_test.go index 748b463b8a..50da9ae4bb 100644 --- a/x/ccv/provider/keeper/punish_validator_test.go +++ b/x/ccv/provider/keeper/punish_validator_test.go @@ -12,6 +12,8 @@ import ( "github.com/golang/mock/gomock" ) +// TestJailValidator tests that the jailing of a validator is only executed +// under the conditions that the validator is neither unbonded, already jailed, nor tombstoned. func TestJailValidator(t *testing.T) { providerConsAddr := cryptotestutil.NewCryptoIdentityFromIntSeed(7842334).ProviderConsAddress() testCases := []struct {