refactor: adding CheckForMisbehaviour to 07-tendermint client

modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go

@@ -40,7 +40,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() {
 		height1         clienttypes.Height
 		consensusState2 exported.ConsensusState
 		height2         clienttypes.Height
-		misbehaviour   exported.ClientMessage 
+		misbehaviour    exported.ClientMessage
 		timestamp       time.Time
 		expPass         bool
 	}{

modules/light-clients/07-tendermint/types/update.go

@@ -261,3 +261,70 @@ func update(ctx sdk.Context, clientStore sdk.KVStore, clientState *ClientState,
 
 	return clientState, consensusState
 }
+
+// CheckForMisbehaviour detects duplicate height misbehaviour and BFT time violation misbehaviour
+func (cs ClientState) CheckForMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, msg exported.ClientMessage) bool {
+	switch msg := msg.(type) {
+	case *Header:
+		tmHeader := msg
+		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
+			}
+
+			// 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
+		}
+
+		// 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
+		}
+		// 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
+		}
+	case *Misbehaviour:
+		tmMisbehaviour := msg
+
+		// 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 false
+			}
+			blockID2, err := tmtypes.BlockIDFromProto(&tmMisbehaviour.Header2.SignedHeader.Commit.BlockID)
+			if err != nil {
+				return false
+			}
+
+			// Ensure that Commit Hashes are different
+			if !bytes.Equal(blockID1.Hash, blockID2.Hash) {
+				return true
+			}
+		} 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.Before(tmMisbehaviour.Header2.SignedHeader.Header.Time) {
+				return true
+			}
+		}
+	}
+
+	return false
+}

modules/light-clients/07-tendermint/types/update_test.go

@@ -448,3 +448,172 @@ func (suite *TendermintTestSuite) TestPruneConsensusState() {
 	consKey = types.GetIterationKey(clientStore, expiredHeight)
 	suite.Require().Equal(expectedConsKey, consKey, "iteration key incorrectly pruned")
 }
+
+func (suite *TendermintTestSuite) TestCheckForMisbehaviour() {
+	var (
+		path          *ibctesting.Path
+		clientMessage exported.ClientMessage
+	)
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expPass  bool
+	}{
+		{
+			"valid update no misbehaviour",
+			func() {},
+			false,
+		},
+		{
+			"consensus state already exists, already updated",
+			func() {
+				header, ok := clientMessage.(*types.Header)
+				suite.Require().True(ok)
+
+				consensusState := &types.ConsensusState{
+					Timestamp:          header.GetTime(),
+					Root:               commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()),
+					NextValidatorsHash: header.Header.NextValidatorsHash,
+				}
+
+				suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientMessage.GetHeight(), consensusState)
+			},
+			false,
+		},
+		{
+			"consensus state already exists, app hash mismatch",
+			func() {
+				header, ok := clientMessage.(*types.Header)
+				suite.Require().True(ok)
+
+				consensusState := &types.ConsensusState{
+					Timestamp:          header.GetTime(),
+					Root:               commitmenttypes.NewMerkleRoot([]byte{}), // empty bytes
+					NextValidatorsHash: header.Header.NextValidatorsHash,
+				}
+
+				suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientMessage.GetHeight(), consensusState)
+			},
+			true,
+		},
+		{
+			"previous consensus state exists and header time is before previous consensus state time",
+			func() {
+				header, ok := clientMessage.(*types.Header)
+				suite.Require().True(ok)
+
+				// offset header timestamp before previous consensus state timestamp
+				header.Header.Time = header.GetTime().Add(-time.Hour)
+			},
+			true,
+		},
+		{
+			"next consensus state exists and header time is after next consensus state time",
+			func() {
+				header, ok := clientMessage.(*types.Header)
+				suite.Require().True(ok)
+
+				// commit block and update client, adding a new consensus state
+				suite.coordinator.CommitBlock(suite.chainB)
+				err := path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+
+				// increase timestamp of current header
+				header.Header.Time = header.Header.Time.Add(time.Hour)
+			},
+			true,
+		},
+		{
+			"valid fork misbehaviour",
+			func() {
+				header1, err := path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID)
+				suite.Require().NoError(err)
+
+				// commit block and update client
+				suite.coordinator.CommitBlock(suite.chainB)
+				err = path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+
+				header2, err := path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID)
+				suite.Require().NoError(err)
+
+				// assign the same height, each header will have a different commit hash
+				header1.Header.Height = header2.Header.Height
+
+				clientMessage = &types.Misbehaviour{
+					Header1:  header1,
+					Header2:  header2,
+					ClientId: chainID,
+				}
+			},
+			true,
+		},
+		{
+			"valid time violation misbehaviour",
+			func() {
+				header1, err := path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID)
+				suite.Require().NoError(err)
+
+				// commit block and update client
+				suite.coordinator.CommitBlock(suite.chainB)
+				err = path.EndpointA.UpdateClient()
+				suite.Require().NoError(err)
+
+				header2, err := path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID)
+				suite.Require().NoError(err)
+
+				// set Height(Header1) > Height(Header2), ValidateBasic ensures Height(Header1) >= Height(Header2)
+				// timestamp of Header1 is before Header2, therefore causing of a violation of monotonic time.
+				header1.Header.Height = header1.Header.Height + 42
+
+				clientMessage = &types.Misbehaviour{
+					Header1:  header1,
+					Header2:  header2,
+					ClientId: chainID,
+				}
+			},
+			true,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		suite.Run(tc.name, func() {
+			// reset suite to create fresh application state
+			suite.SetupTest()
+			path = ibctesting.NewPath(suite.chainA, suite.chainB)
+
+			err := path.EndpointA.CreateClient()
+			suite.Require().NoError(err)
+
+			// ensure counterparty state is committed
+			suite.coordinator.CommitBlock(suite.chainB)
+			clientMessage, err = path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID)
+			suite.Require().NoError(err)
+
+			tc.malleate()
+
+			clientState := path.EndpointA.GetClientState()
+
+			// TODO: remove casting when 'UpdateState' is an interface function.
+			tmClientState, ok := clientState.(*types.ClientState)
+			suite.Require().True(ok)
+
+			clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID)
+
+			foundMisbehaviour := tmClientState.CheckForMisbehaviour(
+				suite.chainA.GetContext(),
+				suite.chainA.App.AppCodec(),
+				clientStore, // pass in clientID prefixed clientStore
+				clientMessage,
+			)
+
+			if tc.expPass {
+				suite.Require().True(foundMisbehaviour)
+			} else {
+				suite.Require().False(foundMisbehaviour)
+			}
+		})
+	}
+}