diff --git a/x/ibc/light-clients/06-solomachine/spec/01_concepts.md b/x/ibc/light-clients/06-solomachine/spec/01_concepts.md index 1723c8740e..fd8a4f71b2 100644 --- a/x/ibc/light-clients/06-solomachine/spec/01_concepts.md +++ b/x/ibc/light-clients/06-solomachine/spec/01_concepts.md @@ -154,6 +154,9 @@ If the misbehaviour is successfully processed: - the client is frozen by setting the frozen sequence to the misbehaviour sequence +NOTE: Misbehaviour processing is data processing order dependent. A misbehaving solo machine +could update to a new public key to prevent being frozen before misbehaviour is submitted. + ## Upgrades Upgrades to solo machine light clients are not supported since an entirely different type of diff --git a/x/ibc/light-clients/06-solomachine/types/misbehaviour_handle.go b/x/ibc/light-clients/06-solomachine/types/misbehaviour_handle.go index 721b84f82e..83335ec59f 100644 --- a/x/ibc/light-clients/06-solomachine/types/misbehaviour_handle.go +++ b/x/ibc/light-clients/06-solomachine/types/misbehaviour_handle.go @@ -11,6 +11,9 @@ import ( // CheckMisbehaviourAndUpdateState determines whether or not the currently registered // public key signed over two different messages with the same sequence. If this is true // the client state is updated to a frozen status. +// NOTE: Misbehaviour is not tracked for previous public keys, a solo machine may update to +// a new public key before the misbehaviour is processed. Therefore, misbehaviour is data +// order processing dependent. func (cs ClientState) CheckMisbehaviourAndUpdateState( ctx sdk.Context, cdc codec.BinaryMarshaler, @@ -49,14 +52,10 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( // verifySignatureAndData verifies that the currently registered public key has signed // over the provided data and that the data is valid. The data is valid if it can be -// unmarshaled into the specified data type or the timestamp of the signature is less -// than the consensus state timestamp. +// unmarshaled into the specified data type. func verifySignatureAndData(cdc codec.BinaryMarshaler, clientState ClientState, misbehaviour *Misbehaviour, sigAndData *SignatureAndData) error { - // timestamp less than consensus state would always fail and not succeed in fooling the - // light client - if sigAndData.Timestamp < clientState.ConsensusState.Timestamp { - return sdkerrors.Wrapf(clienttypes.ErrInvalidMisbehaviour, "timestamp is less than consensus state timestamp (%d < %d)", sigAndData.Timestamp, clientState.ConsensusState.Timestamp) - } + + // do not check misbehaviour timestamp since we want to allow processing of past misbehaviour // ensure data can be unmarshaled to the specified data type if _, err := UnmarshalDataByType(cdc, sigAndData.DataType, sigAndData.Data); err != nil { diff --git a/x/ibc/light-clients/06-solomachine/types/misbehaviour_handle_test.go b/x/ibc/light-clients/06-solomachine/types/misbehaviour_handle_test.go index f247ed2332..89692c5c4d 100644 --- a/x/ibc/light-clients/06-solomachine/types/misbehaviour_handle_test.go +++ b/x/ibc/light-clients/06-solomachine/types/misbehaviour_handle_test.go @@ -29,6 +29,14 @@ func (suite *SoloMachineTestSuite) TestCheckMisbehaviourAndUpdateState() { }, true, }, + { + "old misbehaviour is successful (timestamp is less than current consensus state)", + func() { + clientState = solomachine.ClientState() + solomachine.Time = solomachine.Time - 5 + misbehaviour = solomachine.CreateMisbehaviour() + }, true, + }, { "client is frozen", func() { @@ -95,14 +103,6 @@ func (suite *SoloMachineTestSuite) TestCheckMisbehaviourAndUpdateState() { misbehaviour = m }, false, }, - { - "timestamp is less than consensus state timestamp", - func() { - clientState = solomachine.ClientState() - solomachine.Time = solomachine.Time - 5 - misbehaviour = solomachine.CreateMisbehaviour() - }, false, - }, { "invalid first signature data", func() {