diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go index 5d0fc3e10e5..54f9508d37b 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go @@ -1,7 +1,6 @@ package types import ( - "bytes" "time" "github.com/cosmos/cosmos-sdk/codec" @@ -32,39 +31,16 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", misbehaviour, &Misbehaviour{}) } - // The status of the client is checked in 02-client - - // if heights are equal check that this is valid misbehaviour of a fork - // otherwise if heights are unequal check that this is valid misbehavior of BFT time violation - if tmMisbehaviour.Header1.GetHeight().EQ(tmMisbehaviour.Header2.GetHeight()) { - blockID1, err := tmtypes.BlockIDFromProto(&tmMisbehaviour.Header1.SignedHeader.Commit.BlockID) - if err != nil { - return nil, sdkerrors.Wrap(err, "invalid block ID from header 1 in misbehaviour") - } - blockID2, err := tmtypes.BlockIDFromProto(&tmMisbehaviour.Header2.SignedHeader.Commit.BlockID) - if err != nil { - return nil, sdkerrors.Wrap(err, "invalid block ID from header 2 in misbehaviour") - } - - // Ensure that Commit Hashes are different - if bytes.Equal(blockID1.Hash, blockID2.Hash) { - return nil, sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers block hashes are equal") - } - } else { - // Header1 is at greater height than Header2, therefore Header1 time must be less than or equal to - // Header2 time in order to be valid misbehaviour (violation of monotonic time). - if tmMisbehaviour.Header1.SignedHeader.Header.Time.After(tmMisbehaviour.Header2.SignedHeader.Header.Time) { - return nil, sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers are not at same height and are monotonically increasing") - } + foundMisbehaviour, err := cs.CheckForMisbehaviour(ctx, cdc, clientStore, tmMisbehaviour) + if foundMisbehaviour { + return nil, err } if err := cs.ValidateClientMessage(ctx, clientStore, cdc, nil, misbehaviour, ctx.BlockTime()); err != nil { return nil, err } - cs.FrozenHeight = FrozenHeight - - return &cs, nil + return cs.UpdateStateOnMisbehaviour(ctx, cdc, clientStore), nil } // checkMisbehaviourHeader checks that a Header in Misbehaviour is valid misbehaviour given diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index 34568bd772d..7384fbf2a67 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -60,22 +60,6 @@ func (cs ClientState) CheckHeaderAndUpdateState( ) } - // Check if the Client store already has a consensus state for the header's height - // If the consensus state exists, and it matches the header then we return early - // since header has already been submitted in a previous UpdateClient. - var conflictingHeader bool - prevConsState, _ := GetConsensusState(clientStore, cdc, header.GetHeight()) - if prevConsState != nil { - // This header has already been submitted and the necessary state is already stored - // in client store, thus we can return early without further validation. - if reflect.DeepEqual(prevConsState, tmHeader.ConsensusState()) { - return &cs, prevConsState, nil - } - // A consensus state already exists for this height, but it does not match the provided header. - // Thus, we must check that this header is valid, and if so we will freeze the client. - conflictingHeader = true - } - // get consensus state from clientStore trustedConsState, err := GetConsensusState(clientStore, cdc, tmHeader.TrustedHeight) if err != nil { @@ -88,26 +72,9 @@ func (cs ClientState) CheckHeaderAndUpdateState( return nil, nil, err } - consState := tmHeader.ConsensusState() - // Header is different from existing consensus state and also valid, so freeze the client and return - if conflictingHeader { - cs.FrozenHeight = FrozenHeight - return &cs, consState, nil - } - // Check that consensus state timestamps are monotonic - prevCons, prevOk := GetPreviousConsensusState(clientStore, cdc, header.GetHeight()) - nextCons, nextOk := GetNextConsensusState(clientStore, cdc, header.GetHeight()) - // if previous consensus state exists, check consensus state time is greater than previous consensus state time - // if previous consensus state is not before current consensus state, freeze the client and return. - if prevOk && !prevCons.Timestamp.Before(consState.Timestamp) { - cs.FrozenHeight = FrozenHeight - return &cs, consState, nil - } - // if next consensus state exists, check consensus state time is less than next consensus state time - // if next consensus state is not after current consensus state, freeze the client and return. - if nextOk && !nextCons.Timestamp.After(consState.Timestamp) { - cs.FrozenHeight = FrozenHeight - return &cs, consState, nil + foundMisbehaviour, err := cs.CheckForMisbehaviour(ctx, cdc, clientStore, header) + if foundMisbehaviour { + return cs.UpdateStateOnMisbehaviour(ctx, cdc, clientStore), nil, err } // Check the earliest consensus state to see if it is expired, if so then set the prune height @@ -283,6 +250,85 @@ func (cs *ClientState) ValidateClientMessage( return nil } +// CheckForMisbehaviour returns true with an optional error if a misbehaviour is found for the given Header or Misbehaviour type +func (cs ClientState) CheckForMisbehaviour( + ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, + msg exported.ClientMessage, +) (bool, error) { + switch msg.(type) { + case *Header: + tmHeader := msg.(*Header) + consState := tmHeader.ConsensusState() + + // Check if the Client store already has a consensus state for the header's height + // If the consensus state exists, and it matches the header then we return early + // since header has already been submitted in a previous UpdateClient. + prevConsState, _ := GetConsensusState(clientStore, cdc, tmHeader.GetHeight()) + if prevConsState != nil { + // This header has already been submitted and the necessary state is already stored + // in client store, thus we can return early without further validation. + if reflect.DeepEqual(prevConsState, tmHeader.ConsensusState()) { + return false, nil + } + + // A consensus state already exists for this height, but it does not match the provided header. + // The assumption is that Header has already been validated. Thus we can return true as misbehaviour is present + return true, nil + } + + // Check that consensus state timestamps are monotonic + prevCons, prevOk := GetPreviousConsensusState(clientStore, cdc, tmHeader.GetHeight()) + nextCons, nextOk := GetNextConsensusState(clientStore, cdc, tmHeader.GetHeight()) + // if previous consensus state exists, check consensus state time is greater than previous consensus state time + // if previous consensus state is not before current consensus state return true + if prevOk && !prevCons.Timestamp.Before(consState.Timestamp) { + return true, nil + } + // if next consensus state exists, check consensus state time is less than next consensus state time + // if next consensus state is not after current consensus state return true + if nextOk && !nextCons.Timestamp.After(consState.Timestamp) { + return true, nil + } + case *Misbehaviour: + tmMisbehaviour := msg.(*Misbehaviour) + + // if heights are equal check that this is valid misbehaviour of a fork + // otherwise if heights are unequal check that this is valid misbehavior of BFT time violation + if tmMisbehaviour.Header1.GetHeight().EQ(tmMisbehaviour.Header2.GetHeight()) { + blockID1, err := tmtypes.BlockIDFromProto(&tmMisbehaviour.Header1.SignedHeader.Commit.BlockID) + if err != nil { + return true, sdkerrors.Wrap(err, "invalid block ID from header 1 in misbehaviour") + } + blockID2, err := tmtypes.BlockIDFromProto(&tmMisbehaviour.Header2.SignedHeader.Commit.BlockID) + if err != nil { + return true, sdkerrors.Wrap(err, "invalid block ID from header 2 in misbehaviour") + } + + // Ensure that Commit Hashes are different + if bytes.Equal(blockID1.Hash, blockID2.Hash) { + return true, sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers block hashes are equal") + } + } else { + // Header1 is at greater height than Header2, therefore Header1 time must be less than or equal to + // Header2 time in order to be valid misbehaviour (violation of monotonic time). + if tmMisbehaviour.Header1.SignedHeader.Header.Time.After(tmMisbehaviour.Header2.SignedHeader.Header.Time) { + return true, sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers are not at same height and are monotonically increasing") + } + } + } + + return false, nil +} + +// UpdateStateOnMisbehaviour updates state upon misbehaviour. This method should only be called on misbehaviour +// as it does not perform any misbehaviour checks. +func (cs ClientState) UpdateStateOnMisbehaviour( + _ sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, +) *ClientState { + cs.FrozenHeight = FrozenHeight + return &cs +} + // update the consensus state from a new header and set processed time metadata func update(ctx sdk.Context, clientStore sdk.KVStore, clientState *ClientState, header *Header) (*ClientState, *ConsensusState) { height := header.GetHeight().(clienttypes.Height)