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

fix!: add check for min height of evidence (backport #2007) #2008

Merged
merged 5 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add missing check for the minimum height of evidence in the consumer double-vote handler.
[#2008](https://github.com/cosmos/interchain-security/pull/2008)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Add missing check for the minimum height of evidence in the consumer double-vote handler.
[#2008](https://github.com/cosmos/interchain-security/pull/2008)
21 changes: 15 additions & 6 deletions tests/integration/double_vote.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
s.consumerChain.ChainID,
)

// create a vote using the consumer validator key
// with block height that is smaller than the equivocation evidence min height
consuVoteOld := testutil.MakeAndSignVote(
// create two votes using the consumer validator key that both have
// the same block height that is smaller than the equivocation evidence min height
consuVoteOld1 := testutil.MakeAndSignVote(
blockID1,
int64(equivocationEvidenceMinHeight-1),
s.consumerCtx().BlockTime(),
Expand All @@ -97,6 +97,15 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
s.consumerChain.ChainID,
)

consuVoteOld2 := testutil.MakeAndSignVote(
blockID2,
int64(equivocationEvidenceMinHeight-1),
s.consumerCtx().BlockTime(),
consuValSet,
consuSigner,
s.consumerChain.ChainID,
)

testCases := []struct {
name string
ev *tmtypes.DuplicateVoteEvidence
Expand All @@ -120,8 +129,8 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
{
"evidence is older than equivocation evidence min height - shouldn't pass",
&tmtypes.DuplicateVoteEvidence{
VoteA: consuVoteOld,
VoteB: consuBadVote,
VoteA: consuVoteOld1,
VoteB: consuVoteOld2,
ValidatorPower: consuVal.VotingPower,
TotalVotingPower: consuVal.VotingPower,
Timestamp: s.consumerCtx().BlockTime(),
Expand All @@ -134,7 +143,7 @@ func (s *CCVTestSuite) TestHandleConsumerDoubleVoting() {
"the votes in the evidence are for different height - shouldn't pass",
&tmtypes.DuplicateVoteEvidence{
VoteA: consuVote,
VoteB: consuVoteOld,
VoteB: consuVoteOld1,
ValidatorPower: consuVal.VotingPower,
TotalVotingPower: consuVal.VotingPower,
Timestamp: s.consumerCtx().BlockTime(),
Expand Down
11 changes: 10 additions & 1 deletion tests/integration/misbehaviour.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,16 @@ func (s *CCVTestSuite) TestCheckMisbehaviour() {
clientTMValset,
altSigners,
),
Header2: clientHeader,
Header2: s.consumerChain.CreateTMClientHeader(
s.consumerChain.ChainID,
int64(equivocationEvidenceMinHeight-1),
clientHeight,
headerTs,
clientTMValset,
clientTMValset,
clientTMValset,
clientSigners,
),
},
false,
},
Expand Down
61 changes: 50 additions & 11 deletions x/ccv/provider/keeper/consumer_equivocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/binary"
"fmt"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

ibcclienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
ibctmtypes "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
Expand Down Expand Up @@ -35,6 +36,27 @@ func (k Keeper) HandleConsumerDoubleVoting(
chainID string,
pubkey cryptotypes.PubKey,
) error {
// check that the evidence is for an ICS consumer chain
if _, found := k.GetConsumerClientId(ctx, chainID); !found {
return errorsmod.Wrapf(
ccvtypes.ErrInvalidDoubleVotingEvidence,
"cannot find consumer chain %s",
chainID,
)
}

// check that the evidence is not too old
minHeight := k.GetEquivocationEvidenceMinHeight(ctx, chainID)
if uint64(evidence.VoteA.Height) < minHeight {
return errorsmod.Wrapf(
ccvtypes.ErrInvalidDoubleVotingEvidence,
"evidence for consumer chain %s is too old - evidence height (%d), min (%d)",
chainID,
evidence.VoteA.Height,
minHeight,
)
}

// verifies the double voting evidence using the consumer chain public key
if err := k.VerifyDoubleVotingEvidence(*evidence, chainID, pubkey); err != nil {
return err
Expand Down Expand Up @@ -269,34 +291,51 @@ func headerToLightBlock(h ibctmtypes.Header) (*tmtypes.LightBlock, error) {
}

// CheckMisbehaviour checks that headers in the given misbehaviour forms
// a valid light client attack on a light client that tracks an ICS consumer chain
// a valid light client attack from an ICS consumer chain and that the light client isn't expired
func (k Keeper) CheckMisbehaviour(ctx sdk.Context, misbehaviour ibctmtypes.Misbehaviour) error {
consumerChainID := misbehaviour.Header1.Header.ChainID

// check that the misbehaviour is for an ICS consumer chain
clientId, found := k.GetConsumerClientId(ctx, misbehaviour.Header1.Header.ChainID)
clientId, found := k.GetConsumerClientId(ctx, consumerChainID)
if !found {
return fmt.Errorf("incorrect misbehaviour with conflicting headers from a non-existent consumer chain: %s", misbehaviour.Header1.Header.ChainID)
return fmt.Errorf("incorrect misbehaviour with conflicting headers from a non-existent consumer chain: %s", consumerChainID)
} else if misbehaviour.ClientId != clientId {
return fmt.Errorf("incorrect misbehaviour: expected client ID for consumer chain %s is %s got %s",
misbehaviour.Header1.Header.ChainID,
consumerChainID,
clientId,
misbehaviour.ClientId,
)
}

// Check that the headers are at the same height to ensure that
// the misbehaviour is for a light client attack and not a time violation,
// see ibc-go/modules/light-clients/07-tendermint/types/misbehaviour_handle.go
if !misbehaviour.Header1.GetHeight().EQ(misbehaviour.Header2.GetHeight()) {
return sdkerrors.Wrap(ibcclienttypes.ErrInvalidMisbehaviour, "headers are not at same height")
}

// Check that the evidence is not too old
minHeight := k.GetEquivocationEvidenceMinHeight(ctx, consumerChainID)
evidenceHeight := misbehaviour.Header1.GetHeight().GetRevisionHeight()
// Note that the revision number is not relevant for checking the age of evidence
// as it's already part of the chain ID and the minimum height is mapped to chain IDs
if evidenceHeight < minHeight {
return errorsmod.Wrapf(
ccvtypes.ErrInvalidDoubleVotingEvidence,
"evidence for consumer chain %s is too old - evidence height (%d), min (%d)",
consumerChainID,
evidenceHeight,
minHeight,
)
}

clientState, found := k.clientKeeper.GetClientState(ctx, clientId)
if !found {
return errorsmod.Wrapf(ibcclienttypes.ErrClientNotFound, "cannot find client state for client with ID %s", clientId)
}

clientStore := k.clientKeeper.ClientStore(ctx, clientId)

// Check that the headers are at the same height to ensure that
// the misbehaviour is for a light client attack and not a time violation,
// see CheckForMisbehaviour in ibc-go/blob/v7.3.0/modules/light-clients/07-tendermint/misbehaviour_handle.go#L73
if !misbehaviour.Header1.GetHeight().EQ(misbehaviour.Header2.GetHeight()) {
return errorsmod.Wrap(ibcclienttypes.ErrInvalidMisbehaviour, "headers are not at same height")
}

// CheckForMisbehaviour verifies that the headers have different blockID hashes
ok := clientState.CheckForMisbehaviour(ctx, k.cdc, clientStore, &misbehaviour)
if !ok {
Expand Down
4 changes: 4 additions & 0 deletions x/ccv/provider/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, prop *types.ConsumerAdditi
fmt.Sprintf("cannot create client for existent consumer chain: %s", chainID))
}

// Set minimum height for equivocation evidence from this consumer chain
k.SetEquivocationEvidenceMinHeight(ctx, chainID, prop.InitialHeight.RevisionHeight)

// Consumers start out with the unbonding period from the consumer addition prop
consumerUnbondingPeriod := prop.UnbondingPeriod

Expand Down Expand Up @@ -163,6 +166,7 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, closeChan boo
k.DeleteInitTimeoutTimestamp(ctx, chainID)
// Note: this call panics if the key assignment state is invalid
k.DeleteKeyAssignments(ctx, chainID)
k.DeleteEquivocationEvidenceMinHeight(ctx, chainID)

// close channel and delete the mappings between chain ID and channel ID
if channelID, found := k.GetChainToChannel(ctx, chainID); found {
Expand Down
Loading