diff --git a/CHANGELOG.md b/CHANGELOG.md index 458dc36a1f6..4bd8c4266e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking * (modules/light-clients/07-tendermint) [\#99](https://github.com/cosmos/ibc-go/pull/99) Enforce maximum chain-id length for tendermint client. +* (modules/light-clients/07-tendermint) [\#141](https://github.com/cosmos/ibc-go/pull/141) Allow a new form of misbehaviour that proves counterparty chain breaks time monotonicity, automatically enforce monotonicity in UpdateClient and freeze client if monotonicity is broken. +* (modules/light-clients/07-tendermint) [\#141](https://github.com/cosmos/ibc-go/pull/141) Freeze the client if there's a conflicting header submitted for an existing consensus state. * (modules/core/02-client) [\#8405](https://github.com/cosmos/cosmos-sdk/pull/8405) Refactor IBC client update governance proposals to use a substitute client to update a frozen or expired client. * (modules/core/02-client) [\#8673](https://github.com/cosmos/cosmos-sdk/pull/8673) IBC upgrade logic moved to 02-client and an IBC UpgradeProposal is added. @@ -60,6 +62,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (modules/core/04-channel) [\#7949](https://github.com/cosmos/cosmos-sdk/issues/7949) Standardized channel `Acknowledgement` moved to its own file. Codec registration redundancy removed. * (modules/core/04-channel) [\#144](https://github.com/cosmos/ibc-go/pull/144) Introduced a `packet_data_hex` attribute to emit the hex-encoded packet data in events. This allows for raw binary (proto-encoded message) to be sent over events and decoded correctly on relayer. Original `packet_data` is DEPRECATED. All relayers and IBC event consumers are encouraged to switch to `packet_data_hex` as soon as possible. * (modules/light-clients/07-tendermint) [\#125](https://github.com/cosmos/ibc-go/pull/125) Implement efficient iteration of consensus states and pruning of earliest expired consensus state on UpdateClient. +* (modules/light-clients/07-tendermint) [\#141](https://github.com/cosmos/ibc-go/pull/141) Return early in case there's a duplicate update call to save Gas. + ## IBC in the Cosmos SDK Repository diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index c5416b9e803..ddb9aab9f16 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -67,58 +67,86 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H return sdkerrors.Wrapf(types.ErrClientNotActive, "cannot update client (%s) with status %s", clientID, status) } - clientState, consensusState, err := clientState.CheckHeaderAndUpdateState(ctx, k.cdc, clientStore, header) + eventType := types.EventTypeUpdateClient + + // Any writes made in CheckHeaderAndUpdateState are persisted on both valid updates and misbehaviour updates. + // Light client implementations are responsible for writing the correct metadata (if any) in either case. + newClientState, newConsensusState, err := clientState.CheckHeaderAndUpdateState(ctx, k.cdc, clientStore, header) if err != nil { return sdkerrors.Wrapf(err, "cannot update client with ID %s", clientID) } - k.SetClientState(ctx, clientID, clientState) - - var consensusHeight exported.Height - - // we don't set consensus state for localhost client - if header != nil && clientID != exported.Localhost { - k.SetClientConsensusState(ctx, clientID, header.GetHeight(), consensusState) - consensusHeight = header.GetHeight() - } else { - consensusHeight = types.GetSelfHeight(ctx) - } - - k.Logger(ctx).Info("client state updated", "client-id", clientID, "height", consensusHeight.String()) - - defer func() { - telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "update"}, - 1, - []metrics.Label{ - telemetry.NewLabel("client-type", clientState.ClientType()), - telemetry.NewLabel("client-id", clientID), - telemetry.NewLabel("update-type", "msg"), - }, - ) - }() - // emit the full header in events - var headerStr string + var ( + headerStr string + consensusHeight exported.Height + ) if header != nil { // Marshal the Header as an Any and encode the resulting bytes to hex. // This prevents the event value from containing invalid UTF-8 characters // which may cause data to be lost when JSON encoding/decoding. headerStr = hex.EncodeToString(types.MustMarshalHeader(k.cdc, header)) + // set default consensus height with header height + consensusHeight = header.GetHeight() } + // set new client state regardless of if update is valid update or misbehaviour + k.SetClientState(ctx, clientID, newClientState) + // If client state is not frozen after clientState CheckHeaderAndUpdateState, + // then update was valid. Write the update state changes, and set new consensus state. + // Else the update was proof of misbehaviour and we must emit appropriate misbehaviour events. + if status := newClientState.Status(ctx, clientStore, k.cdc); status != exported.Frozen { + // if update is not misbehaviour then update the consensus state + // we don't set consensus state for localhost client + if header != nil && clientID != exported.Localhost { + k.SetClientConsensusState(ctx, clientID, header.GetHeight(), newConsensusState) + } else { + consensusHeight = types.GetSelfHeight(ctx) + } + + k.Logger(ctx).Info("client state updated", "client-id", clientID, "height", consensusHeight.String()) + + defer func() { + telemetry.IncrCounterWithLabels( + []string{"ibc", "client", "update"}, + 1, + []metrics.Label{ + telemetry.NewLabel("client-type", clientState.ClientType()), + telemetry.NewLabel("client-id", clientID), + telemetry.NewLabel("update-type", "msg"), + }, + ) + }() + } else { + // set eventType to SubmitMisbehaviour + eventType = types.EventTypeSubmitMisbehaviour + + k.Logger(ctx).Info("client frozen due to misbehaviour", "client-id", clientID, "height", header.GetHeight().String()) + + defer func() { + telemetry.IncrCounterWithLabels( + []string{"ibc", "client", "misbehaviour"}, + 1, + []metrics.Label{ + telemetry.NewLabel("client-type", clientState.ClientType()), + telemetry.NewLabel("client-id", clientID), + telemetry.NewLabel("msg-type", "update"), + }, + ) + }() + } + // emitting events in the keeper emits for both begin block and handler client updates ctx.EventManager().EmitEvent( sdk.NewEvent( - types.EventTypeUpdateClient, + eventType, sdk.NewAttribute(types.AttributeKeyClientID, clientID), sdk.NewAttribute(types.AttributeKeyClientType, clientState.ClientType()), sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()), sdk.NewAttribute(types.AttributeKeyHeader, headerStr), ), ) - return nil } @@ -186,6 +214,10 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour ex return sdkerrors.Wrapf(types.ErrClientNotActive, "cannot process misbehaviour for client (%s) with status %s", misbehaviour.GetClientID(), status) } + if err := misbehaviour.ValidateBasic(); err != nil { + return err + } + clientState, err := clientState.CheckMisbehaviourAndUpdateState(ctx, k.cdc, clientStore, misbehaviour) if err != nil { return err diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index 1e1f393df9a..96aed366bc3 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -5,9 +5,9 @@ import ( "fmt" "time" - upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" tmtypes "github.com/tendermint/tendermint/types" + upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" "github.com/cosmos/ibc-go/modules/core/02-client/types" clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/modules/core/23-commitment/types" @@ -53,7 +53,6 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { suite.Require().NoError(err) return header } - createPastUpdateFn := func(fillHeight, trustedHeight clienttypes.Height) *ibctmtypes.Header { consState, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, trustedHeight) suite.Require().True(found) @@ -61,10 +60,12 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { return suite.chainB.CreateTMClientHeader(suite.chainB.ChainID, int64(fillHeight.RevisionHeight), trustedHeight, consState.(*ibctmtypes.ConsensusState).Timestamp.Add(time.Second*5), suite.chainB.Vals, suite.chainB.Vals, suite.chainB.Signers) } + cases := []struct { - name string - malleate func() - expPass bool + name string + malleate func() + expPass bool + expFreeze bool }{ {"valid update", func() { clientState := path.EndpointA.GetClientState().(*ibctmtypes.ClientState) @@ -74,7 +75,7 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { path.EndpointA.UpdateClient() updateHeader = createFutureUpdateFn(trustHeight) - }, true}, + }, true, false}, {"valid past update", func() { clientState := path.EndpointA.GetClientState() trustedHeight := clientState.GetLatestHeight().(types.Height) @@ -95,12 +96,89 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { // updateHeader will fill in consensus state between prevConsState and suite.consState // clientState should not be updated updateHeader = createPastUpdateFn(fillHeight, trustedHeight) - }, true}, + }, true, false}, + {"valid duplicate update", func() { + clientID := path.EndpointA.ClientID + + height1 := types.NewHeight(0, 1) + + // store previous consensus state + prevConsState := &ibctmtypes.ConsensusState{ + Timestamp: suite.past, + NextValidatorsHash: suite.chainB.Vals.Hash(), + } + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, height1, prevConsState) + + height5 := types.NewHeight(0, 5) + // store next consensus state to check that trustedHeight does not need to be hightest consensus state before header height + nextConsState := &ibctmtypes.ConsensusState{ + Timestamp: suite.past.Add(time.Minute), + NextValidatorsHash: suite.chainB.Vals.Hash(), + } + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, height5, nextConsState) + + height3 := types.NewHeight(0, 3) + // updateHeader will fill in consensus state between prevConsState and suite.consState + // clientState should not be updated + updateHeader = createPastUpdateFn(height3, height1) + // set updateHeader's consensus state in store to create duplicate UpdateClient scenario + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, updateHeader.GetHeight(), updateHeader.ConsensusState()) + }, true, false}, + {"misbehaviour detection: conflicting header", func() { + clientID := path.EndpointA.ClientID + + height1 := types.NewHeight(0, 1) + // store previous consensus state + prevConsState := &ibctmtypes.ConsensusState{ + Timestamp: suite.past, + NextValidatorsHash: suite.chainB.Vals.Hash(), + } + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, height1, prevConsState) + + height5 := types.NewHeight(0, 5) + // store next consensus state to check that trustedHeight does not need to be hightest consensus state before header height + nextConsState := &ibctmtypes.ConsensusState{ + Timestamp: suite.past.Add(time.Minute), + NextValidatorsHash: suite.chainB.Vals.Hash(), + } + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, height5, nextConsState) + + height3 := types.NewHeight(0, 3) + // updateHeader will fill in consensus state between prevConsState and suite.consState + // clientState should not be updated + updateHeader = createPastUpdateFn(height3, height1) + // set conflicting consensus state in store to create misbehaviour scenario + conflictConsState := updateHeader.ConsensusState() + conflictConsState.Root = commitmenttypes.NewMerkleRoot([]byte("conflicting apphash")) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, updateHeader.GetHeight(), conflictConsState) + }, true, true}, + {"misbehaviour detection: monotonic time violation", func() { + clientState := path.EndpointA.GetClientState().(*ibctmtypes.ClientState) + clientID := path.EndpointA.ClientID + trustedHeight := clientState.GetLatestHeight().(types.Height) + + // store intermediate consensus state at a time greater than updateHeader time + // this will break time monotonicity + incrementedClientHeight := clientState.GetLatestHeight().Increment().(types.Height) + intermediateConsState := &ibctmtypes.ConsensusState{ + Timestamp: suite.coordinator.CurrentTime.Add(2 * time.Hour), + NextValidatorsHash: suite.chainB.Vals.Hash(), + } + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, incrementedClientHeight, intermediateConsState) + // set iteration key + clientStore := suite.keeper.ClientStore(suite.ctx, clientID) + ibctmtypes.SetIterationKey(clientStore, incrementedClientHeight) + + clientState.LatestHeight = incrementedClientHeight + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), clientID, clientState) + + updateHeader = createFutureUpdateFn(trustedHeight) + }, true, true}, {"client state not found", func() { updateHeader = createFutureUpdateFn(path.EndpointA.GetClientState().GetLatestHeight().(types.Height)) path.EndpointA.ClientID = ibctesting.InvalidID - }, false}, + }, false, false}, {"consensus state not found", func() { clientState := path.EndpointA.GetClientState() tmClient, ok := clientState.(*ibctmtypes.ClientState) @@ -109,17 +187,17 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientState) updateHeader = createFutureUpdateFn(clientState.GetLatestHeight().(types.Height)) - }, false}, + }, false, false}, {"client is not active", func() { clientState := path.EndpointA.GetClientState().(*ibctmtypes.ClientState) clientState.FrozenHeight = types.NewHeight(0, 1) suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientState) updateHeader = createFutureUpdateFn(clientState.GetLatestHeight().(types.Height)) - }, false}, + }, false, false}, {"invalid header", func() { updateHeader = createFutureUpdateFn(path.EndpointA.GetClientState().GetLatestHeight().(types.Height)) updateHeader.TrustedHeight = updateHeader.TrustedHeight.Increment().(types.Height) - }, false}, + }, false, false}, } for _, tc := range cases { @@ -141,28 +219,32 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() { if tc.expPass { suite.Require().NoError(err, err) - expConsensusState := &ibctmtypes.ConsensusState{ - Timestamp: updateHeader.GetTime(), - Root: commitmenttypes.NewMerkleRoot(updateHeader.Header.GetAppHash()), - NextValidatorsHash: updateHeader.Header.NextValidatorsHash, - } - newClientState := path.EndpointA.GetClientState() - consensusState, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, updateHeader.GetHeight()) - suite.Require().True(found) - - // Determine if clientState should be updated or not - if updateHeader.GetHeight().GT(clientState.GetLatestHeight()) { - // Header Height is greater than clientState latest Height, clientState should be updated with header.GetHeight() - suite.Require().Equal(updateHeader.GetHeight(), newClientState.GetLatestHeight(), "clientstate height did not update") + if tc.expFreeze { + suite.Require().True(!newClientState.(*ibctmtypes.ClientState).FrozenHeight.IsZero(), "client did not freeze after conflicting header was submitted to UpdateClient") } else { - // Update will add past consensus state, clientState should not be updated at all - suite.Require().Equal(clientState.GetLatestHeight(), newClientState.GetLatestHeight(), "client state height updated for past header") + expConsensusState := &ibctmtypes.ConsensusState{ + Timestamp: updateHeader.GetTime(), + Root: commitmenttypes.NewMerkleRoot(updateHeader.Header.GetAppHash()), + NextValidatorsHash: updateHeader.Header.NextValidatorsHash, + } + + consensusState, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, updateHeader.GetHeight()) + suite.Require().True(found) + + // Determine if clientState should be updated or not + if updateHeader.GetHeight().GT(clientState.GetLatestHeight()) { + // Header Height is greater than clientState latest Height, clientState should be updated with header.GetHeight() + suite.Require().Equal(updateHeader.GetHeight(), newClientState.GetLatestHeight(), "clientstate height did not update") + } else { + // Update will add past consensus state, clientState should not be updated at all + suite.Require().Equal(clientState.GetLatestHeight(), newClientState.GetLatestHeight(), "client state height updated for past header") + } + + suite.Require().NoError(err) + suite.Require().Equal(expConsensusState, consensusState, "consensus state should have been updated on case %s", tc.name) } - - suite.Require().NoError(err) - suite.Require().Equal(expConsensusState, consensusState, "consensus state should have been updated on case %s", tc.name) } else { suite.Require().Error(err) } @@ -381,8 +463,24 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { { "trusting period misbehavior should pass", &ibctmtypes.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight+1), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight+1), testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + ClientId: clientID, + }, + func() error { + suite.consensusState.NextValidatorsHash = bothValsHash + clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) + + return err + }, + true, + }, + { + "time misbehavior should pass", + &ibctmtypes.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight+5), testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight+1), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), ClientId: clientID, }, func() error { @@ -397,8 +495,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { { "misbehavior at later height should pass", &ibctmtypes.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, altTime, bothValSet, valSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight+1), testClientHeight, altTime, bothValSet, valSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight+1), testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners), ClientId: clientID, }, func() error { @@ -409,7 +507,7 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { // store intermediate consensus state to check that trustedHeight does not need to be highest consensus state before header height intermediateConsState := &ibctmtypes.ConsensusState{ Timestamp: suite.now.Add(time.Minute), - NextValidatorsHash: suite.valSetHash, + NextValidatorsHash: suite.chainB.Vals.Hash(), } suite.keeper.SetClientConsensusState(suite.ctx, clientID, heightPlus3, intermediateConsState) @@ -423,8 +521,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { { "misbehavior at later height with different trusted heights should pass", &ibctmtypes.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, altTime, bothValSet, valSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), heightPlus3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight+1), testClientHeight, altTime, bothValSet, valSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight+1), heightPlus3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), ClientId: clientID, }, func() error { @@ -446,11 +544,27 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { }, true, }, + { + "misbehavior ValidateBasic fails: misbehaviour height is at same height as trusted height", + &ibctmtypes.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + ClientId: clientID, + }, + func() error { + suite.consensusState.NextValidatorsHash = bothValsHash + clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false) + clientID, err = suite.keeper.CreateClient(suite.ctx, clientState, suite.consensusState) + + return err + }, + false, + }, { "trusted ConsensusState1 not found", &ibctmtypes.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), heightPlus3, altTime, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight+1), heightPlus3, altTime, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight+1), testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners), ClientId: clientID, }, func() error { @@ -465,8 +579,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { { "trusted ConsensusState2 not found", &ibctmtypes.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), testClientHeight, altTime, bothValSet, valSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight), heightPlus3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight+1), testClientHeight, altTime, bothValSet, valSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(heightPlus5.RevisionHeight+1), heightPlus3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), ClientId: clientID, }, func() error { @@ -487,8 +601,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { { "client already is not active - client is frozen", &ibctmtypes.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight+1), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight+1), testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), ClientId: clientID, }, func() error { @@ -506,8 +620,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { { "misbehaviour check failed", &ibctmtypes.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight), testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners), + Header1: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight+1), testClientHeight, altTime, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(testChainID, int64(testClientHeight.RevisionHeight+1), testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners), ClientId: clientID, }, func() error { @@ -544,8 +658,6 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { clientState, found := suite.keeper.GetClientState(suite.ctx, clientID) suite.Require().True(found, "valid test case %d failed: %s", i, tc.name) suite.Require().True(!clientState.(*ibctmtypes.ClientState).FrozenHeight.IsZero(), "valid test case %d failed: %s", i, tc.name) - suite.Require().Equal(tc.misbehaviour.GetHeight(), clientState.GetFrozenHeight(), - "valid test case %d failed: %s. Expected FrozenHeight %s got %s", tc.misbehaviour.GetHeight(), clientState.GetFrozenHeight()) } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name) } diff --git a/modules/light-clients/07-tendermint/types/header.go b/modules/light-clients/07-tendermint/types/header.go index 9bd59708ee5..d3662bb72a8 100644 --- a/modules/light-clients/07-tendermint/types/header.go +++ b/modules/light-clients/07-tendermint/types/header.go @@ -62,10 +62,9 @@ func (h Header) ValidateBasic() error { return sdkerrors.Wrap(err, "header failed basic validation") } - // TrustedHeight is less than Header for updates - // and less than or equal to Header for misbehaviour - if h.TrustedHeight.GT(h.GetHeight()) { - return sdkerrors.Wrapf(ErrInvalidHeaderHeight, "TrustedHeight %d must be less than or equal to header height %d", + // TrustedHeight is less than Header for updates and misbehaviour + if h.TrustedHeight.GTE(h.GetHeight()) { + return sdkerrors.Wrapf(ErrInvalidHeaderHeight, "TrustedHeight %d must be less than header height %d", h.TrustedHeight, h.GetHeight()) } diff --git a/modules/light-clients/07-tendermint/types/header_test.go b/modules/light-clients/07-tendermint/types/header_test.go index 487b27943dc..37a7082d62a 100644 --- a/modules/light-clients/07-tendermint/types/header_test.go +++ b/modules/light-clients/07-tendermint/types/header_test.go @@ -43,8 +43,8 @@ func (suite *TendermintTestSuite) TestHeaderValidateBasic() { header = suite.chainA.LastHeader header.SignedHeader.Commit = nil }, false}, - {"trusted height is greater than header height", func() { - header.TrustedHeight = header.GetHeight().(clienttypes.Height).Increment().(clienttypes.Height) + {"trusted height is equal to header height", func() { + header.TrustedHeight = header.GetHeight().(clienttypes.Height) }, false}, {"validator set nil", func() { header.ValidatorSet = nil diff --git a/modules/light-clients/07-tendermint/types/misbehaviour.go b/modules/light-clients/07-tendermint/types/misbehaviour.go index 51e59612f11..9a5ef74c948 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour.go @@ -1,7 +1,6 @@ package types import ( - "bytes" "time" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" @@ -15,6 +14,9 @@ import ( var _ exported.Misbehaviour = &Misbehaviour{} +// Use the same FrozenHeight for all misbehaviour +var FrozenHeight = clienttypes.NewHeight(0, 1) + // NewMisbehaviour creates a new Misbehaviour instance. func NewMisbehaviour(clientID string, header1, header2 *Header) *Misbehaviour { return &Misbehaviour{ @@ -35,8 +37,6 @@ func (misbehaviour Misbehaviour) GetClientID() string { } // GetHeight returns the height at which misbehaviour occurred -// -// NOTE: assumes that misbehaviour headers have the same height func (misbehaviour Misbehaviour) GetHeight() exported.Height { return misbehaviour.Header1.GetHeight() } @@ -93,9 +93,9 @@ func (misbehaviour Misbehaviour) ValidateBasic() error { sdkerrors.Wrap(err, "header 2 failed validation").Error(), ) } - // Ensure that Heights are the same - if misbehaviour.Header1.GetHeight() != misbehaviour.Header2.GetHeight() { - return sdkerrors.Wrapf(clienttypes.ErrInvalidMisbehaviour, "headers in misbehaviour are on different heights (%d ≠ %d)", misbehaviour.Header1.GetHeight(), misbehaviour.Header2.GetHeight()) + // Ensure that Height1 is greater than or equal to Height2 + if misbehaviour.Header1.GetHeight().LT(misbehaviour.Header2.GetHeight()) { + return sdkerrors.Wrapf(clienttypes.ErrInvalidMisbehaviour, "Header1 height is less than Header2 height (%s < %s)", misbehaviour.Header1.GetHeight(), misbehaviour.Header2.GetHeight()) } blockID1, err := tmtypes.BlockIDFromProto(&misbehaviour.Header1.SignedHeader.Commit.BlockID) @@ -107,10 +107,6 @@ func (misbehaviour Misbehaviour) ValidateBasic() error { return 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 sdkerrors.Wrap(clienttypes.ErrInvalidMisbehaviour, "headers block hashes are equal") - } if err := validCommit(misbehaviour.Header1.Header.ChainID, *blockID1, misbehaviour.Header1.Commit, misbehaviour.Header1.ValidatorSet); err != nil { return err diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go index fa00dad99e3..286977491f8 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle.go @@ -1,6 +1,7 @@ package types import ( + "bytes" "time" tmtypes "github.com/tendermint/tendermint/types" @@ -19,6 +20,7 @@ import ( // of misbehaviour.Header1 // Similarly, consensusState2 is the trusted consensus state that corresponds // to misbehaviour.Header2 +// Misbehaviour sets frozen height to {0, 1} since it is only used as a boolean value (zero or non-zero). func (cs ClientState) CheckMisbehaviourAndUpdateState( ctx sdk.Context, cdc codec.BinaryCodec, @@ -32,6 +34,32 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( // 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") + } + } + + // Regardless of the type of misbehaviour, ensure that both headers are valid and would have been accepted by light-client + // Retrieve trusted consensus states for each Header in misbehaviour // and unmarshal from clientStore @@ -63,7 +91,8 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState( return nil, sdkerrors.Wrap(err, "verifying Header2 in Misbehaviour failed") } - cs.FrozenHeight = tmMisbehaviour.GetHeight().(clienttypes.Height) + cs.FrozenHeight = FrozenHeight + return &cs, nil } diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go index 1ce0b154ed5..a257a4b80c7 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go @@ -50,15 +50,45 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { expPass bool }{ { - "valid misbehavior misbehaviour", + "valid fork misbehaviour", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + { + "valid time misbehaviour", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + true, + }, + { + "valid time misbehaviour header 1 stricly less than header 2", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Hour), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now, @@ -72,8 +102,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), heightMinus1, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now, @@ -87,8 +117,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), heightMinus3, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus3, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus3, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), ClientId: chainID, }, suite.now, @@ -102,8 +132,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), heightMinus3, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainIDRevision0, int64(height.RevisionHeight), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainIDRevision0, int64(height.RevisionHeight), heightMinus3, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainIDRevision0, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainIDRevision0, int64(height.RevisionHeight+1), heightMinus3, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), ClientId: chainID, }, suite.now, @@ -147,13 +177,43 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now, bothValSet, suite.valSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, suite.valSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), ClientId: chainID, }, suite.now, true, }, + { + "invalid fork misbehaviour: identical headers", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, + { + "invalid time misbehaviour: monotonically increasing time", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), + height, + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+3), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothSigners), + ClientId: chainID, + }, + suite.now, + false, + }, { "invalid misbehavior misbehaviour from different chain", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false), @@ -162,8 +222,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader("ethermint", int64(height.RevisionHeight), height, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader("ethermint", int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader("ethermint", int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader("ethermint", int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now, @@ -177,8 +237,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), heightMinus3, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), ClientId: chainID, }, suite.now, @@ -192,8 +252,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), suite.valsHash), heightMinus3, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus3, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now, @@ -207,8 +267,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now, @@ -222,8 +282,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now, @@ -248,8 +308,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now, @@ -263,8 +323,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), heightMinus1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now.Add(trustingPeriod), @@ -278,8 +338,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now, bothValSet, suite.valSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, suite.valSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), ClientId: chainID, }, suite.now, @@ -293,8 +353,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now, altValSet, bothValSet, altSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, altValSet, bothValSet, altSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), ClientId: chainID, }, suite.now, @@ -308,8 +368,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now, bothValSet, bothValSet, bothSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), altValSet, bothValSet, altSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, bothValSet, bothValSet, bothSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), altValSet, bothValSet, altSigners), ClientId: chainID, }, suite.now, @@ -323,8 +383,8 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), bothValsHash), height, &types.Misbehaviour{ - Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now, altValSet, bothValSet, altSigners), - Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), height, suite.now.Add(time.Minute), altValSet, bothValSet, altSigners), + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now, altValSet, bothValSet, altSigners), + Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+1), height, suite.now.Add(time.Minute), altValSet, bothValSet, altSigners), ClientId: chainID, }, suite.now, @@ -352,7 +412,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { clientState, err := tc.clientState.CheckMisbehaviourAndUpdateState( ctx, - suite.cdc, + suite.chainA.App.AppCodec(), suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, clientID), // pass in clientID prefixed clientStore tc.misbehaviour, ) @@ -361,8 +421,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name) suite.Require().NotNil(clientState, "valid test case %d failed: %s", i, tc.name) suite.Require().True(!clientState.(*types.ClientState).FrozenHeight.IsZero(), "valid test case %d failed: %s", i, tc.name) - suite.Require().Equal(tc.misbehaviour.GetHeight(), clientState.GetFrozenHeight(), - "valid test case %d failed: %s. Expected FrozenHeight %s got %s", tc.misbehaviour.GetHeight(), clientState.GetFrozenHeight()) } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name) suite.Require().Nil(clientState, "invalid test case %d passed: %s", i, tc.name) diff --git a/modules/light-clients/07-tendermint/types/misbehaviour_test.go b/modules/light-clients/07-tendermint/types/misbehaviour_test.go index 1b67b72973c..30deec2cb90 100644 --- a/modules/light-clients/07-tendermint/types/misbehaviour_test.go +++ b/modules/light-clients/07-tendermint/types/misbehaviour_test.go @@ -60,7 +60,7 @@ func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { expPass bool }{ { - "valid misbehaviour", + "valid fork misbehaviour, two headers at same height have different time", &types.Misbehaviour{ Header1: suite.header, Header2: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight), heightMinus1, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers), @@ -69,6 +69,16 @@ func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { func(misbehaviour *types.Misbehaviour) error { return nil }, true, }, + { + "valid time misbehaviour, both headers at different heights are at same time", + &types.Misbehaviour{ + Header1: suite.chainA.CreateTMClientHeader(chainID, int64(height.RevisionHeight+5), heightMinus1, suite.now, suite.valSet, suite.valSet, signers), + Header2: suite.header, + ClientId: clientID, + }, + func(misbehaviour *types.Misbehaviour) error { return nil }, + true, + }, { "misbehaviour Header1 is nil", types.NewMisbehaviour(clientID, nil, suite.header), @@ -152,20 +162,10 @@ func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { false, }, { - "mismatched heights", + "header2 height is greater", &types.Misbehaviour{ Header1: suite.header, - Header2: suite.chainA.CreateTMClientHeader(chainID, 6, clienttypes.NewHeight(0, 4), suite.now, suite.valSet, suite.valSet, signers), - ClientId: clientID, - }, - func(misbehaviour *types.Misbehaviour) error { return nil }, - false, - }, - { - "same block id", - &types.Misbehaviour{ - Header1: suite.header, - Header2: suite.header, + Header2: suite.chainA.CreateTMClientHeader(chainID, 6, clienttypes.NewHeight(0, height.RevisionHeight+1), suite.now, suite.valSet, suite.valSet, signers), ClientId: clientID, }, func(misbehaviour *types.Misbehaviour) error { return nil }, diff --git a/modules/light-clients/07-tendermint/types/store.go b/modules/light-clients/07-tendermint/types/store.go index 68793621547..f261f4dc146 100644 --- a/modules/light-clients/07-tendermint/types/store.go +++ b/modules/light-clients/07-tendermint/types/store.go @@ -1,6 +1,7 @@ package types import ( + "bytes" "encoding/binary" "strings" @@ -170,6 +171,8 @@ func GetHeightFromIterationKey(iterKey []byte) exported.Height { return clienttypes.NewHeight(revision, height) } +// IterateConsensusStateAscending iterates through the consensus states in ascending order. It calls the provided +// callback on each height, until stop=true is returned. func IterateConsensusStateAscending(clientStore sdk.KVStore, cb func(height exported.Height) (stop bool)) error { iterator := sdk.KVStorePrefixIterator(clientStore, []byte(KeyIterateConsensusStatePrefix)) defer iterator.Close() @@ -186,17 +189,25 @@ func IterateConsensusStateAscending(clientStore sdk.KVStore, cb func(height expo // GetNextConsensusState returns the lowest consensus state that is larger than the given height. // The Iterator returns a storetypes.Iterator which iterates from start (inclusive) to end (exclusive). -// Thus, to get the next consensus state, we must first call iterator.Next() and then get the value. +// If the starting height exists in store, we need to call iterator.Next() to get the next consenus state. +// Otherwise, the iterator is already at the next consensus state so we can call iterator.Value() immediately. func GetNextConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, height exported.Height) (*ConsensusState, bool) { iterateStore := prefix.NewStore(clientStore, []byte(KeyIterateConsensusStatePrefix)) iterator := iterateStore.Iterator(bigEndianHeightBytes(height), nil) defer iterator.Close() - // ignore the consensus state at current height and get next height - iterator.Next() if !iterator.Valid() { return nil, false } + // if iterator is at current height, ignore the consensus state at current height and get next height + // if iterator value is not at current height, it is already at next height. + if bytes.Equal(iterator.Value(), host.ConsensusStateKey(height)) { + iterator.Next() + if !iterator.Valid() { + return nil, false + } + } + csKey := iterator.Value() return getTmConsensusState(clientStore, cdc, csKey) diff --git a/modules/light-clients/07-tendermint/types/tendermint_test.go b/modules/light-clients/07-tendermint/types/tendermint_test.go index 7bb8977c68a..071f6f6cca1 100644 --- a/modules/light-clients/07-tendermint/types/tendermint_test.go +++ b/modules/light-clients/07-tendermint/types/tendermint_test.go @@ -10,12 +10,12 @@ import ( tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" - "github.com/cosmos/ibc-go/testing/simapp" sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types" ibctmtypes "github.com/cosmos/ibc-go/modules/light-clients/07-tendermint/types" ibctesting "github.com/cosmos/ibc-go/testing" ibctestingmock "github.com/cosmos/ibc-go/testing/mock" + "github.com/cosmos/ibc-go/testing/simapp" ) const ( @@ -90,6 +90,19 @@ func (suite *TendermintTestSuite) SetupTest() { suite.ctx = app.BaseApp.NewContext(checkTx, tmproto.Header{Height: 1, Time: suite.now}) } +func getSuiteSigners(suite *TendermintTestSuite) []tmtypes.PrivValidator { + return []tmtypes.PrivValidator{suite.privVal} +} + +func getBothSigners(suite *TendermintTestSuite, altVal *tmtypes.Validator, altPrivVal tmtypes.PrivValidator) (*tmtypes.ValidatorSet, []tmtypes.PrivValidator) { + // Create bothValSet with both suite validator and altVal. Would be valid update + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + // Create signer array and ensure it is in same order as bothValSet + _, suiteVal := suite.valSet.GetByIndex(0) + bothSigners := ibctesting.CreateSortedSignerArray(altPrivVal, suite.privVal, altVal, suiteVal) + return bothValSet, bothSigners +} + func TestTendermintTestSuite(t *testing.T) { suite.Run(t, new(TendermintTestSuite)) } diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index bfabb324d7c..e1104e8d54b 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -2,6 +2,7 @@ package types import ( "bytes" + "reflect" "time" "github.com/tendermint/tendermint/light" @@ -38,6 +39,12 @@ import ( // Tendermint client validity checking uses the bisection algorithm described // in the [Tendermint spec](https://github.com/tendermint/spec/blob/master/spec/consensus/light-client.md). // +// Misbehaviour Detection: +// UpdateClient will detect implicit misbehaviour by enforcing certain invariants on any new update call and will return a frozen client. +// 1. Any valid update that creates a different consensus state for an already existing height is evidence of misbehaviour and will freeze client. +// 2. Any valid update that breaks time monotonicity with respect to its neighboring consensus states is evidence of misbehaviour and will freeze client. +// Misbehaviour sets frozen height to {0, 1} since it is only used as a boolean value (zero or non-zero). +// // Pruning: // UpdateClient will additionally retrieve the earliest consensus state for this clientID and check if it is expired. If it is, // that consensus state will be pruned from store along with all associated metadata. This will prevent the client store from @@ -53,18 +60,56 @@ 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 - tmConsState, err := GetConsensusState(clientStore, cdc, tmHeader.TrustedHeight) + trustedConsState, err := GetConsensusState(clientStore, cdc, tmHeader.TrustedHeight) if err != nil { return nil, nil, sdkerrors.Wrapf( err, "could not get consensus state from clientstore at TrustedHeight: %s", tmHeader.TrustedHeight, ) } - if err := checkValidity(&cs, tmConsState, tmHeader, ctx.BlockTime()); err != nil { + if err := checkValidity(&cs, trustedConsState, tmHeader, ctx.BlockTime()); err != nil { 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 + } + // Check the earliest consensus state to see if it is expired, if so then set the prune height // so that we can delete consensus state and all associated metadata. var ( diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index 14c34645c2c..95a159ef97c 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/modules/light-clients/07-tendermint/types/update_test.go @@ -1,6 +1,7 @@ package types_test import ( + "fmt" "time" tmtypes "github.com/tendermint/tendermint/types" @@ -40,13 +41,13 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { altVal := tmtypes.NewValidator(altPubKey, revisionHeight) // Create alternative validator set with only altVal, invalid update (too much change in valSet) altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) - altSigners := []tmtypes.PrivValidator{altPrivVal} testCases := []struct { - name string - setup func(suite *TendermintTestSuite) - expPass bool + name string + setup func(*TendermintTestSuite) + expFrozen bool + expPass bool }{ { name: "successful update with next height and same validator set", @@ -56,7 +57,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.headerTime, suite.valSet, suite.valSet, signers) currentTime = suite.now }, - expPass: true, + expFrozen: false, + expPass: true, }, { name: "successful update with future height and different validator set", @@ -66,7 +68,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus5.RevisionHeight), height, suite.headerTime, bothValSet, suite.valSet, bothSigners) currentTime = suite.now }, - expPass: true, + expFrozen: false, + expPass: true, }, { name: "successful update with next height and different validator set", @@ -76,7 +79,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.headerTime, bothValSet, bothValSet, bothSigners) currentTime = suite.now }, - expPass: true, + expFrozen: false, + expPass: true, }, { name: "successful update for a previous height", @@ -87,7 +91,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightMinus1.RevisionHeight), heightMinus3, suite.headerTime, bothValSet, suite.valSet, bothSigners) currentTime = suite.now }, - expPass: true, + expFrozen: false, + expPass: true, }, { name: "successful update for a previous revision", @@ -100,6 +105,72 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { }, expPass: true, }, + { + name: "successful update with identical header to a previous update", + setup: func(suite *TendermintTestSuite) { + clientState = types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, heightPlus1, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) + consensusState = types.NewConsensusState(suite.clientTime, commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()), suite.valsHash) + newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.headerTime, suite.valSet, suite.valSet, signers) + currentTime = suite.now + ctx := suite.chainA.GetContext().WithBlockTime(currentTime) + // Store the header's consensus state in client store before UpdateClient call + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(ctx, clientID, heightPlus1, newHeader.ConsensusState()) + }, + expFrozen: false, + expPass: true, + }, + { + name: "misbehaviour detection: header conflicts with existing consensus state", + setup: func(suite *TendermintTestSuite) { + clientState = types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, heightPlus1, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) + consensusState = types.NewConsensusState(suite.clientTime, commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()), suite.valsHash) + newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.headerTime, suite.valSet, suite.valSet, signers) + currentTime = suite.now + ctx := suite.chainA.GetContext().WithBlockTime(currentTime) + // Change the consensus state of header and store in client store to create a conflict + conflictConsState := newHeader.ConsensusState() + conflictConsState.Root = commitmenttypes.NewMerkleRoot([]byte("conflicting apphash")) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(ctx, clientID, heightPlus1, conflictConsState) + }, + expFrozen: true, + expPass: true, + }, + { + name: "misbehaviour detection: previous consensus state time is not before header time. time monotonicity violation", + setup: func(suite *TendermintTestSuite) { + clientState = types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) + // create an intermediate consensus state with the same time as the newHeader to create a time violation. + // header time is after client time + consensusState = types.NewConsensusState(suite.clientTime, commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()), suite.valsHash) + newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus5.RevisionHeight), height, suite.headerTime, suite.valSet, suite.valSet, signers) + currentTime = suite.now + prevConsensusState := types.NewConsensusState(suite.headerTime, commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()), suite.valsHash) + ctx := suite.chainA.GetContext().WithBlockTime(currentTime) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(ctx, clientID, heightPlus1, prevConsensusState) + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, clientID) + types.SetIterationKey(clientStore, heightPlus1) + }, + expFrozen: true, + expPass: true, + }, + { + name: "misbehaviour detection: next consensus state time is not after header time. time monotonicity violation", + setup: func(suite *TendermintTestSuite) { + clientState = types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false) + // create the next consensus state with the same time as the intermediate newHeader to create a time violation. + // header time is after clientTime + consensusState = types.NewConsensusState(suite.clientTime, commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()), suite.valsHash) + newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.headerTime, suite.valSet, suite.valSet, signers) + currentTime = suite.now + nextConsensusState := types.NewConsensusState(suite.headerTime, commitmenttypes.NewMerkleRoot(suite.header.Header.GetAppHash()), suite.valsHash) + ctx := suite.chainA.GetContext().WithBlockTime(currentTime) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(ctx, clientID, heightPlus5, nextConsensusState) + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, clientID) + types.SetIterationKey(clientStore, heightPlus5) + }, + expFrozen: true, + expPass: true, + }, { name: "unsuccessful update with incorrect header chain-id", setup: func(suite *TendermintTestSuite) { @@ -108,7 +179,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader("ethermint", int64(heightPlus1.RevisionHeight), height, suite.headerTime, suite.valSet, suite.valSet, signers) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "unsuccessful update to a future revision", @@ -128,7 +200,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainIDRevision1, 3, height, suite.headerTime, suite.valSet, suite.valSet, signers) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "unsuccessful update with next height: update header mismatches nextValSetHash", @@ -138,7 +211,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.headerTime, bothValSet, suite.valSet, bothSigners) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "unsuccessful update with next height: update header mismatches different nextValSetHash", @@ -148,7 +222,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.headerTime, suite.valSet, bothValSet, signers) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "unsuccessful update with future height: too much change in validator set", @@ -158,7 +233,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus5.RevisionHeight), height, suite.headerTime, altValSet, suite.valSet, altSigners) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "unsuccessful updates, passed in incorrect trusted validators for given consensus state", @@ -168,7 +244,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus5.RevisionHeight), height, suite.headerTime, bothValSet, bothValSet, bothSigners) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "unsuccessful update: trusting period has passed since last client timestamp", @@ -179,7 +256,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { // make current time pass trusting period from last timestamp on clientstate currentTime = suite.now.Add(trustingPeriod) }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "unsuccessful update: header timestamp is past current timestamp", @@ -189,7 +267,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "unsuccessful update: header timestamp is not past last client timestamp", @@ -199,7 +278,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightPlus1.RevisionHeight), height, suite.clientTime, suite.valSet, suite.valSet, signers) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "header basic validation failed", @@ -211,7 +291,8 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader.SignedHeader.Commit.Height = revisionHeight - 1 currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, { name: "header height < consensus height", @@ -222,13 +303,14 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { newHeader = suite.chainA.CreateTMClientHeader(chainID, int64(heightMinus1.RevisionHeight), height, suite.headerTime, suite.valSet, suite.valSet, signers) currentTime = suite.now }, - expPass: false, + expFrozen: false, + expPass: false, }, } for i, tc := range testCases { tc := tc - suite.Run(tc.name, func() { + suite.Run(fmt.Sprintf("Case: %s", tc.name), func() { suite.SetupTest() // reset metadata writes // Create bothValSet with both suite validator and altVal. Would be valid update bothValSet = tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) @@ -265,17 +347,22 @@ func (suite *TendermintTestSuite) TestCheckHeaderAndUpdateState() { if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name) - // Determine if clientState should be updated or not - // TODO: check the entire Height struct once GetLatestHeight returns clienttypes.Height - if height.GT(clientState.LatestHeight) { - // Header Height is greater than clientState latest Height, clientState should be updated with header.GetHeight() - suite.Require().Equal(height, newClientState.GetLatestHeight(), "clientstate height did not update") - } else { - // Update will add past consensus state, clientState should not be updated at all - suite.Require().Equal(clientState.LatestHeight, newClientState.GetLatestHeight(), "client state height updated for past header") - } + suite.Require().Equal(tc.expFrozen, !newClientState.(*types.ClientState).FrozenHeight.IsZero(), "client state status is unexpected after update") + + // further writes only happen if update is not misbehaviour + if !tc.expFrozen { + // Determine if clientState should be updated or not + // TODO: check the entire Height struct once GetLatestHeight returns clienttypes.Height + if height.GT(clientState.LatestHeight) { + // Header Height is greater than clientState latest Height, clientState should be updated with header.GetHeight() + suite.Require().Equal(height, newClientState.GetLatestHeight(), "clientstate height did not update") + } else { + // Update will add past consensus state, clientState should not be updated at all + suite.Require().Equal(clientState.LatestHeight, newClientState.GetLatestHeight(), "client state height updated for past header") + } - suite.Require().Equal(expectedConsensus, consensusState, "valid test case %d failed: %s", i, tc.name) + suite.Require().Equal(expectedConsensus, consensusState, "valid test case %d failed: %s", i, tc.name) + } } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name) suite.Require().Nil(newClientState, "invalid test case %d passed: %s", i, tc.name)