diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 3ab40ceee..39f05fde7 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/x/ibc/02-client/keeper/client.go @@ -132,18 +132,29 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour ex if !found { return sdkerrors.Wrapf(types.ErrClientNotFound, "cannot check misbehaviour for client with ID %s", misbehaviour.GetClientID()) } + if err := misbehaviour.ValidateBasic(); err != nil { + return sdkerrors.Wrap(err, "IBC misbehaviour failed validate basic") + } var err error switch e := misbehaviour.(type) { case ibctmtypes.Evidence: - // Get ConsensusState at TrustedHeight - consensusState, found := k.GetClientConsensusState(ctx, misbehaviour.GetClientID(), e.Header1.TrustedHeight) + // Get consensus states at TrustedHeight for each header + consensusState1, found := k.GetClientConsensusState(ctx, misbehaviour.GetClientID(), e.Header1.TrustedHeight) if !found { - return sdkerrors.Wrapf(types.ErrConsensusStateNotFound, "cannot check misbehaviour for client with ID %s", misbehaviour.GetClientID()) + return sdkerrors.Wrapf(types.ErrConsensusStateNotFound, "could not find ConsensusState for clientID %s at TrustedHeight (%d) for first header", + misbehaviour.GetClientID(), e.Header1.TrustedHeight) + } + consensusState2, found := k.GetClientConsensusState(ctx, misbehaviour.GetClientID(), e.Header2.TrustedHeight) + if !found { + return sdkerrors.Wrapf(types.ErrConsensusStateNotFound, "could not find ConsensusState for clientID %s at TrustedHeight (%d) for second header", + misbehaviour.GetClientID(), e.Header2.TrustedHeight) } + // TODO: Retrieve consensusparams from client and not context + // Issue #6516: https://github.com/cosmos/cosmos-sdk/issues/6516 clientState, err = tendermint.CheckMisbehaviourAndUpdateState( - clientState, consensusState, misbehaviour, consensusState.GetHeight(), ctx.BlockTime(), ctx.ConsensusParams(), + clientState, consensusState1, consensusState2, misbehaviour, ctx.BlockTime(), ctx.ConsensusParams(), ) default: diff --git a/x/ibc/02-client/keeper/client_test.go b/x/ibc/02-client/keeper/client_test.go index c4c810b2d..f7f335b4f 100644 --- a/x/ibc/02-client/keeper/client_test.go +++ b/x/ibc/02-client/keeper/client_test.go @@ -248,8 +248,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { suite.Require().NoError(err) altVal := tmtypes.NewValidator(altPubKey, 4) + // Set valSet here with suite.valSet so it doesn't get reset on each testcase + valSet := suite.valSet + valsHash := valSet.Hash() + // Create bothValSet with both suite validator and altVal bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + bothValsHash := bothValSet.Hash() // Create alternative validator set with only altVal altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) @@ -266,6 +271,9 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { altSigners := []tmtypes.PrivValidator{altPrivVal} + // Create valid Misbehaviour by making a duplicate header that signs over different block time + altTime := suite.ctx.BlockTime().Add(time.Minute) + testCases := []struct { name string evidence ibctmtypes.Evidence @@ -275,13 +283,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { { "trusting period misbehavior should pass", ibctmtypes.Evidence{ - Header1: ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - Header2: ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - ChainID: testClientID, + Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + ChainID: testChainID, ClientID: testClientID, }, func() error { - suite.consensusState.NextValidatorsHash = bothValSet.Hash() + suite.consensusState.NextValidatorsHash = bothValsHash clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs()) _, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) @@ -292,13 +300,13 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { { "misbehavior at later height should pass", ibctmtypes.Evidence{ - Header1: ibctmtypes.CreateTestHeader(testClientID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - Header2: ibctmtypes.CreateTestHeader(testClientID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - ChainID: testClientID, + Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners), + ChainID: testChainID, ClientID: testClientID, }, func() error { - suite.consensusState.NextValidatorsHash = bothValSet.Hash() + suite.consensusState.NextValidatorsHash = valsHash clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs()) _, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) @@ -317,6 +325,86 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { }, true, }, + { + "misbehavior at later height with different trusted heights should pass", + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + ChainID: testChainID, + ClientID: testClientID, + }, + func() error { + suite.consensusState.NextValidatorsHash = valsHash + clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs()) + _, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + + // store trusted consensus state for Header2 + intermediateConsState := &ibctmtypes.ConsensusState{ + Height: testClientHeight + 3, + Timestamp: suite.now.Add(time.Minute), + NextValidatorsHash: bothValsHash, + } + suite.keeper.SetClientConsensusState(suite.ctx, testClientID, testClientHeight+3, intermediateConsState) + + clientState.LatestHeight = testClientHeight + 3 + suite.keeper.SetClientState(suite.ctx, testClientID, clientState) + + return err + }, + true, + }, + { + "misbehaviour fails validatebasic", + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+1, testClientHeight, altTime, bothValSet, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + ChainID: testChainID, + ClientID: testClientID, + }, + func() error { + suite.consensusState.NextValidatorsHash = bothValsHash + clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs()) + _, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + + return err + }, + false, + }, + { + "trusted ConsensusState1 not found", + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, altTime, bothValSet, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, suite.ctx.BlockTime(), bothValSet, valSet, bothSigners), + ChainID: testChainID, + ClientID: testClientID, + }, + func() error { + suite.consensusState.NextValidatorsHash = valsHash + clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs()) + _, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + // intermediate consensus state at height + 3 is not created + return err + }, + false, + }, + { + "trusted ConsensusState2 not found", + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight, altTime, bothValSet, valSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight+5, testClientHeight+3, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + ChainID: testChainID, + ClientID: testClientID, + }, + func() error { + suite.consensusState.NextValidatorsHash = valsHash + clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs()) + _, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + // intermediate consensus state at height + 3 is not created + return err + }, + false, + }, + { "client state not found", ibctmtypes.Evidence{}, @@ -324,41 +412,32 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { false, }, { - "consensus state not found", + "client already frozen at earlier height", ibctmtypes.Evidence{ - Header1: ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - Header2: ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - ChainID: testClientID, + Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), + ChainID: testChainID, ClientID: testClientID, }, func() error { - clientState := &ibctmtypes.ClientState{FrozenHeight: 1, LatestHeight: testClientHeight} + suite.consensusState.NextValidatorsHash = bothValsHash + clientState := ibctmtypes.NewClientState(testChainID, ibctmtypes.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs()) + _, err := suite.keeper.CreateClient(suite.ctx, testClientID, clientState, suite.consensusState) + + clientState.FrozenHeight = 1 suite.keeper.SetClientState(suite.ctx, testClientID, clientState) - return nil - }, - false, - }, - { - "consensus state not found", - ibctmtypes.Evidence{ - Header1: ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - Header2: ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - ChainID: testClientID, - ClientID: testClientID, - }, - func() error { - clientState := &ibctmtypes.ClientState{FrozenHeight: 1, LatestHeight: testClientHeight} - suite.keeper.SetClientState(suite.ctx, testClientID, clientState) - return nil + + return err }, false, }, + { "misbehaviour check failed", ibctmtypes.Evidence{ - Header1: ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), bothValSet, bothValSet, bothSigners), - Header2: ibctmtypes.CreateTestHeader(testClientID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners), - ChainID: testClientID, + Header1: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, altTime, bothValSet, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(testChainID, testClientHeight, testClientHeight, suite.ctx.BlockTime(), altValSet, bothValSet, altSigners), + ChainID: testChainID, ClientID: testClientID, }, func() error { @@ -391,6 +470,8 @@ func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { clientState, found := suite.keeper.GetClientState(suite.ctx, testClientID) suite.Require().True(found, "valid test case %d failed: %s", i, tc.name) suite.Require().True(clientState.IsFrozen(), "valid test case %d failed: %s", i, tc.name) + suite.Require().Equal(uint64(tc.evidence.GetHeight()), clientState.GetFrozenHeight(), + "valid test case %d failed: %s. Expected FrozenHeight %d got %d", tc.evidence.GetHeight(), clientState.GetFrozenHeight()) } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name) } diff --git a/x/ibc/07-tendermint/misbehaviour.go b/x/ibc/07-tendermint/misbehaviour.go index 6e4f45a7d..02ab9cfa5 100644 --- a/x/ibc/07-tendermint/misbehaviour.go +++ b/x/ibc/07-tendermint/misbehaviour.go @@ -1,7 +1,6 @@ package tendermint import ( - "bytes" "time" abci "github.com/tendermint/tendermint/abci/types" @@ -15,13 +14,14 @@ import ( // CheckMisbehaviourAndUpdateState determines whether or not two conflicting // headers at the same height would have convinced the light client. // -// NOTE: assumes provided height is the height at which the consensusState is -// stored. +// NOTE: consensusState1 is the trusted consensus state that corresponds to the TrustedHeight +// of misbehaviour.Header1 +// Similarly, consensusState2 is the trusted consensus state that corresponds +// to misbehaviour.Header2 func CheckMisbehaviourAndUpdateState( clientState clientexported.ClientState, - consensusState clientexported.ConsensusState, + consensusState1, consensusState2 clientexported.ConsensusState, misbehaviour clientexported.Misbehaviour, - height uint64, // height at which the consensus state was loaded currentTimestamp time.Time, consensusParams *abci.ConsensusParams, ) (clientexported.ClientState, error) { @@ -38,9 +38,13 @@ func CheckMisbehaviourAndUpdateState( "client is already frozen at earlier height %d than misbehaviour height %d", tmClientState.FrozenHeight, misbehaviour.GetHeight()) } - tmConsensusState, ok := consensusState.(*types.ConsensusState) + tmConsensusState1, ok := consensusState1.(*types.ConsensusState) if !ok { - return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", &types.ConsensusState{}, consensusState) + return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "invalid consensus state type for first header: expected type %T, got %T", &types.ConsensusState{}, consensusState1) + } + tmConsensusState2, ok := consensusState2.(*types.ConsensusState) + if !ok { + return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "invalid consensus state for second header: expected type %T, got %T", &types.ConsensusState{}, consensusState2) } tmEvidence, ok := misbehaviour.(types.Evidence) @@ -48,40 +52,20 @@ func CheckMisbehaviourAndUpdateState( return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected type %T, got %T", misbehaviour, types.Evidence{}) } - if err := checkMisbehaviour( - tmClientState, tmConsensusState, tmEvidence, height, currentTimestamp, consensusParams, - ); err != nil { - return nil, err + // use earliest height of trusted consensus states to verify ageBlocks + var height uint64 + if tmConsensusState1.Height < tmConsensusState2.Height { + height = tmConsensusState1.Height + } else { + height = tmConsensusState2.Height } - tmClientState.FrozenHeight = uint64(tmEvidence.GetHeight()) - return tmClientState, nil -} - -// checkMisbehaviour checks if the evidence provided is a valid light client misbehaviour -func checkMisbehaviour( - clientState *types.ClientState, consensusState *types.ConsensusState, evidence types.Evidence, - height uint64, currentTimestamp time.Time, consensusParams *abci.ConsensusParams, -) error { // calculate the age of the misbehaviour evidence - infractionHeight := evidence.GetHeight() - infractionTime := evidence.GetTime() + infractionHeight := tmEvidence.GetHeight() + infractionTime := tmEvidence.GetTime() ageDuration := currentTimestamp.Sub(infractionTime) ageBlocks := uint64(infractionHeight) - height - // assert that trustedVals is NextValidators of last trusted header - // to do this, we check that trustedVals.Hash() == consState.NextValidatorsHash - trustedValsetHash1 := evidence.Header1.TrustedValidators.Hash() - trustedValsetHash2 := evidence.Header2.TrustedValidators.Hash() - - if !bytes.Equal(consensusState.NextValidatorsHash, trustedValsetHash1) || !bytes.Equal(consensusState.NextValidatorsHash, trustedValsetHash2) { - return sdkerrors.Wrapf( - types.ErrInvalidValidatorSet, - "header's trusted validators %s, does not hash to either consensus state's trusted validator set. Expected: %X, got: TrustedValSet Header1 %X, TrustedValSet Header2 %X", - evidence.Header1.TrustedValidators, consensusState.NextValidatorsHash, trustedValsetHash1, trustedValsetHash2, - ) - } - // Reject misbehaviour if the age is too old. Evidence is considered stale // if the difference in time and number of blocks is greater than the allowed // parameters defined. @@ -94,47 +78,58 @@ func checkMisbehaviour( consensusParams.Evidence != nil && ageDuration > consensusParams.Evidence.MaxAgeDuration && ageBlocks > uint64(consensusParams.Evidence.MaxAgeNumBlocks) { - return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, + return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "age duration (%s) and age blocks (%d) are greater than max consensus params for duration (%s) and block (%d)", ageDuration, ageBlocks, consensusParams.Evidence.MaxAgeDuration, consensusParams.Evidence.MaxAgeNumBlocks, ) } - // check if provided height matches the headers' height - if height > uint64(evidence.GetHeight()) { - return sdkerrors.Wrapf( - sdkerrors.ErrInvalidHeight, - "height > evidence header height (%d > %d)", height, evidence.GetHeight(), - ) + // Check the validity of the two conflicting headers against their respective + // trusted consensus states + // NOTE: header height and commitment root assertions are checked in + // evidence.ValidateBasic by the client keeper and msg.ValidateBasic + // by the base application. + if err := checkMisbehaviourHeader( + tmClientState, tmConsensusState1, tmEvidence.Header1, currentTimestamp, + ); err != nil { + return nil, sdkerrors.Wrap(err, "verifying Header1 in Evidence failed") + } + if err := checkMisbehaviourHeader( + tmClientState, tmConsensusState2, tmEvidence.Header2, currentTimestamp, + ); err != nil { + return nil, sdkerrors.Wrap(err, "verifying Header2 in Evidence failed") } - // NOTE: header height and commitment root assertions are checked with the - // evidence and msg ValidateBasic functions at the AnteHandler level. + tmClientState.FrozenHeight = uint64(tmEvidence.GetHeight()) + return tmClientState, nil +} + +// checkMisbehaviourHeader checks that a Header in Misbehaviour is valid evidence given +// a trusted ConsensusState +func checkMisbehaviourHeader( + clientState *types.ClientState, consState *types.ConsensusState, header types.Header, currentTimestamp time.Time, +) error { + // check the trusted fields for the header against ConsensusState + if err := checkTrustedHeader(header, consState); err != nil { + return err + } // assert that the timestamp is not from more than an unbonding period ago - if currentTimestamp.Sub(consensusState.Timestamp) >= clientState.UnbondingPeriod { + if currentTimestamp.Sub(consState.Timestamp) >= clientState.UnbondingPeriod { return sdkerrors.Wrapf( types.ErrUnbondingPeriodExpired, "current timestamp minus the latest consensus state timestamp is greater than or equal to the unbonding period (%s >= %s)", - currentTimestamp.Sub(consensusState.Timestamp), clientState.UnbondingPeriod, + currentTimestamp.Sub(consState.Timestamp), clientState.UnbondingPeriod, ) } // - ValidatorSet must have 2/3 similarity with trusted FromValidatorSet // - ValidatorSets on both headers are valid given the last trusted ValidatorSet - if err := evidence.Header1.TrustedValidators.VerifyCommitLightTrusting( - evidence.ChainID, evidence.Header1.Commit.BlockID, evidence.Header1.Height, - evidence.Header1.Commit, clientState.TrustLevel.ToTendermint(), + if err := header.TrustedValidators.VerifyCommitLightTrusting( + clientState.GetChainID(), header.Commit.BlockID, header.Height, + header.Commit, clientState.TrustLevel.ToTendermint(), ); err != nil { - return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header 1 has too much change from trusted validator set: %v", err) + return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header has too much change from trusted validator set: %v", err) } - - if err := evidence.Header2.TrustedValidators.VerifyCommitLightTrusting( - evidence.ChainID, evidence.Header2.Commit.BlockID, evidence.Header2.Height, - evidence.Header2.Commit, clientState.TrustLevel.ToTendermint(), - ); err != nil { - return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header 2 has too much change from trusted validator set: %v", err) - } - return nil } diff --git a/x/ibc/07-tendermint/misbehaviour_test.go b/x/ibc/07-tendermint/misbehaviour_test.go index 1953cc4ec..bdd492f43 100644 --- a/x/ibc/07-tendermint/misbehaviour_test.go +++ b/x/ibc/07-tendermint/misbehaviour_test.go @@ -15,7 +15,7 @@ import ( commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types" ) -func (suite *TendermintTestSuite) TestCheckMisbehaviour() { +func (suite *TendermintTestSuite) TestCheckMisbehaviourAndUpdateState() { altPrivVal := tmtypes.NewMockPV() altPubKey, err := altPrivVal.GetPubKey() suite.Require().NoError(err) @@ -45,10 +45,10 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { testCases := []struct { name string clientState clientexported.ClientState - consensusState clientexported.ConsensusState + consensusState1 clientexported.ConsensusState + consensusState2 clientexported.ConsensusState evidence clientexported.Misbehaviour consensusParams *abci.ConsensusParams - height uint64 timestamp time.Time expPass bool }{ @@ -56,6 +56,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "valid misbehavior evidence", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), @@ -63,7 +64,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, true, }, @@ -71,6 +71,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "valid misbehavior at height greater than last consensusState", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), @@ -78,29 +79,74 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height - 1, + suite.now, + true, + }, + { + "valid misbehavior evidence with different trusted heights", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-3, suite.valsHash), + types.Evidence{ + Header1: types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: types.CreateTestHeader(chainID, height, height-3, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, suite.now, true, }, { "consensus state's valset hash different from evidence should still pass", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), - types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, suite.valsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, suite.valsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, suite.valsHash), types.Evidence{ - Header1: types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, suite.valSet, bothSigners), + Header1: types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, suite.valSet, bothSigners), + Header2: types.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, + suite.now, + true, + }, + { + "invalid misbehavior evidence with trusted height different from trusted consensus state", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-3, suite.valsHash), + types.Evidence{ + Header1: types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), ChainID: chainID, ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, - true, + false, + }, + { + "invalid misbehavior evidence with trusted validators different from trusted consensus state", + types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-1, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height-3, suite.valsHash), + types.Evidence{ + Header1: types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners), + Header2: types.CreateTestHeader(chainID, height, height-3, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, + suite.now, + false, }, { "invalid tendermint client state", nil, types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, altValSet, bothSigners), @@ -108,13 +154,13 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, false, }, { "already frozen client state", - &types.ClientState{FrozenHeight: 1}, + types.ClientState{FrozenHeight: 1}, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners), @@ -123,7 +169,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, false, }, @@ -131,6 +176,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "invalid tendermint consensus state", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), nil, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), @@ -138,7 +184,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, false, }, @@ -146,9 +191,9 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "invalid tendermint misbehaviour evidence", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), nil, simapp.DefaultConsensusParams, - height, suite.now, false, }, @@ -156,6 +201,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "rejected misbehaviour due to expired age", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, int64(2*height+uint64(simapp.DefaultConsensusParams.Evidence.MaxAgeNumBlocks)), height, suite.now, bothValSet, bothValSet, bothSigners), @@ -165,7 +211,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now.Add(2 * time.Minute).Add(simapp.DefaultConsensusParams.Evidence.MaxAgeDuration), false, }, @@ -173,6 +218,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "provided height > header height", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, bothValSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), @@ -180,14 +226,14 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height + 10, suite.now, false, }, { "unbonding period expired", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), - types.NewConsensusState(time.Time{}, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.ConsensusState{Timestamp: time.Time{}, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), NextValidatorsHash: bothValsHash}, + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), @@ -195,7 +241,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, false, }, @@ -203,6 +248,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "trusted validators is incorrect for given consensus state", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, suite.valSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, suite.valSet, bothSigners), @@ -210,7 +256,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, false, }, @@ -218,6 +263,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "first valset has too much change", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, altValSet, bothValSet, altSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), bothValSet, bothValSet, bothSigners), @@ -225,7 +271,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, false, }, @@ -233,6 +278,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "second valset has too much change", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, bothValSet, bothValSet, bothSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), altValSet, bothValSet, altSigners), @@ -240,7 +286,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, false, }, @@ -248,6 +293,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { "both valsets have too much change", types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs()), types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), + types.NewConsensusState(suite.now, commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), height, bothValsHash), types.Evidence{ Header1: types.CreateTestHeader(chainID, height, height, suite.now, altValSet, bothValSet, altSigners), Header2: types.CreateTestHeader(chainID, height, height, suite.now.Add(time.Minute), altValSet, bothValSet, altSigners), @@ -255,7 +301,6 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ClientID: chainID, }, simapp.DefaultConsensusParams, - height, suite.now, false, }, @@ -264,12 +309,14 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { for i, tc := range testCases { tc := tc - clientState, err := tendermint.CheckMisbehaviourAndUpdateState(tc.clientState, tc.consensusState, tc.evidence, tc.height, tc.timestamp, tc.consensusParams) + clientState, err := tendermint.CheckMisbehaviourAndUpdateState(tc.clientState, tc.consensusState1, tc.consensusState2, tc.evidence, tc.timestamp, tc.consensusParams) if tc.expPass { 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.IsFrozen(), "valid test case %d failed: %s", i, tc.name) + suite.Require().Equal(uint64(tc.evidence.GetHeight()), clientState.GetFrozenHeight(), + "valid test case %d failed: %s. Expected FrozenHeight %d got %d", tc.evidence.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/x/ibc/07-tendermint/types/evidence.go b/x/ibc/07-tendermint/types/evidence.go index cdb638f5c..1346ef4de 100644 --- a/x/ibc/07-tendermint/types/evidence.go +++ b/x/ibc/07-tendermint/types/evidence.go @@ -1,7 +1,6 @@ package types import ( - "bytes" "math" "time" @@ -86,17 +85,19 @@ func (ev Evidence) GetTime() time.Time { // ValidateBasic implements Evidence interface func (ev Evidence) ValidateBasic() error { - if ev.Header1.TrustedHeight != ev.Header2.TrustedHeight { - return sdkerrors.Wrapf(ErrInvalidHeaderHeight, "evidence headers must share the same trusted height. got height1: %d, height2: %d", - ev.Header1.TrustedHeight, ev.Header2.TrustedHeight) + if ev.Header1.TrustedHeight == 0 { + return sdkerrors.Wrap(ErrInvalidHeaderHeight, "evidence Header1 must have non-zero trusted height") } - if !bytes.Equal(ev.Header1.TrustedValidators.Hash(), ev.Header2.TrustedValidators.Hash()) { - return sdkerrors.Wrapf(ErrInvalidValidatorSet, "trusted validators on both submitted headers must be the same. Got valset1: %s, valset2: %s", - ev.Header1.TrustedValidators, ev.Header2.TrustedValidators) + if ev.Header2.TrustedHeight == 0 { + return sdkerrors.Wrap(ErrInvalidHeaderHeight, "evidence Header2 must have non-zero trusted height") } if ev.Header1.TrustedValidators == nil { - return sdkerrors.Wrap(ErrInvalidValidatorSet, "trusted validator set cannot be empty") + return sdkerrors.Wrap(ErrInvalidValidatorSet, "trusted validator set in Header1 cannot be empty") } + if ev.Header2.TrustedValidators == nil { + return sdkerrors.Wrap(ErrInvalidValidatorSet, "trusted validator set in Header2 cannot be empty") + } + if err := host.ClientIdentifierValidator(ev.ClientID); err != nil { return sdkerrors.Wrap(err, "evidence client ID is invalid") } diff --git a/x/ibc/07-tendermint/types/evidence_test.go b/x/ibc/07-tendermint/types/evidence_test.go index 9e99b3bed..fce3cd16e 100644 --- a/x/ibc/07-tendermint/types/evidence_test.go +++ b/x/ibc/07-tendermint/types/evidence_test.go @@ -19,11 +19,11 @@ func (suite *TendermintTestSuite) TestEvidence() { Header1: suite.header, Header2: ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now, suite.valSet, suite.valSet, signers), ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, } suite.Require().Equal(ev.ClientType(), clientexported.Tendermint) - suite.Require().Equal(ev.GetClientID(), "gaiamainnet") + suite.Require().Equal(ev.GetClientID(), clientID) suite.Require().Equal(ev.Route(), "client") suite.Require().Equal(ev.Type(), "client_misbehaviour") suite.Require().Equal(ev.Hash(), tmbytes.HexBytes(tmhash.Sum(suite.aminoCdc.MustMarshalBinaryBare(ev)))) @@ -69,29 +69,62 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { Header1: suite.header, Header2: ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers), ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { return nil }, true, }, { - "trusted heights don't match", + "valid evidence with different trusted headers", ibctmtypes.Evidence{ Header1: suite.header, - Header2: ibctmtypes.CreateTestHeader(chainID, height, height-2, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers), + Header2: ibctmtypes.CreateTestHeader(chainID, height, height-3, suite.now.Add(time.Minute), suite.valSet, bothValSet, signers), ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, + }, + func(ev *ibctmtypes.Evidence) error { return nil }, + true, + }, + { + "trusted height is 0 in Header1", + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(chainID, height, 0, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers), + Header2: suite.header, + ChainID: chainID, + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { return nil }, false, }, { - "trusted valsets don't match", + "trusted height is 0 in Header2", ibctmtypes.Evidence{ Header1: suite.header, - Header2: ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), suite.valSet, bothValSet, signers), + Header2: ibctmtypes.CreateTestHeader(chainID, height, 0, suite.now.Add(time.Minute), suite.valSet, suite.valSet, signers), ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, + }, + func(ev *ibctmtypes.Evidence) error { return nil }, + false, + }, + { + "trusted valset is nil in Header1", + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), suite.valSet, nil, signers), + Header2: suite.header, + ChainID: chainID, + ClientID: clientID, + }, + func(ev *ibctmtypes.Evidence) error { return nil }, + false, + }, + { + "trusted valset is nil in Header2", + ibctmtypes.Evidence{ + Header1: suite.header, + Header2: ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now.Add(time.Minute), suite.valSet, nil, signers), + ChainID: chainID, + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { return nil }, false, @@ -113,7 +146,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { Header1: suite.header, Header2: ibctmtypes.CreateTestHeader("ethermint", height, height-1, suite.now, suite.valSet, suite.valSet, signers), ChainID: "ethermint", - ClientID: "gaiamainnet", + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { return nil }, false, @@ -124,7 +157,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { Header1: suite.header, Header2: ibctmtypes.CreateTestHeader("ethermint", height, height-1, suite.now, suite.valSet, suite.valSet, signers), ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { return nil }, false, @@ -135,7 +168,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { Header1: suite.header, Header2: ibctmtypes.CreateTestHeader(chainID, 6, 4, suite.now, suite.valSet, suite.valSet, signers), ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { return nil }, false, @@ -146,7 +179,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { Header1: suite.header, Header2: suite.header, ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { return nil }, false, @@ -157,7 +190,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { Header1: ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, suite.valSet, bothSigners), Header2: suite.header, ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { // voteSet contains only altVal which is less than 2/3 of total power (height/1height) @@ -174,7 +207,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { Header1: suite.header, Header2: ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, suite.valSet, bothSigners), ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { // voteSet contains only altVal which is less than 2/3 of total power (height/1height) @@ -191,7 +224,7 @@ func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { Header1: suite.header, Header2: ibctmtypes.CreateTestHeader(chainID, height, height-1, suite.now, bothValSet, suite.valSet, bothSigners), ChainID: chainID, - ClientID: "gaiamainnet", + ClientID: clientID, }, func(ev *ibctmtypes.Evidence) error { ev.Header2.Commit.BlockID = ibctmtypes.MakeBlockID(tmhash.Sum([]byte("other_hash")), 3, tmhash.Sum([]byte("other_partset"))) diff --git a/x/ibc/07-tendermint/types/tendermint_test.go b/x/ibc/07-tendermint/types/tendermint_test.go index 82b381bc6..372d30aeb 100644 --- a/x/ibc/07-tendermint/types/tendermint_test.go +++ b/x/ibc/07-tendermint/types/tendermint_test.go @@ -18,6 +18,7 @@ import ( const ( chainID = "gaia" + clientID = "gaiamainnet" height = 4 trustingPeriod time.Duration = time.Hour * 24 * 7 * 2 ubdPeriod time.Duration = time.Hour * 24 * 7 * 3 diff --git a/x/ibc/07-tendermint/update.go b/x/ibc/07-tendermint/update.go index 415edc97a..2f3217cef 100644 --- a/x/ibc/07-tendermint/update.go +++ b/x/ibc/07-tendermint/update.go @@ -66,12 +66,15 @@ func CheckValidityAndUpdateState( return tmClientState, consensusState, nil } -// checkValidity checks if the Tendermint header is valid. -// CONTRACT: consState.Height == header.TrustedHeight -func checkValidity( - clientState *types.ClientState, consState *types.ConsensusState, - header types.Header, currentTimestamp time.Time, -) error { +// checkTrustedHeader checks that consensus state matches trusted fields of Header +func checkTrustedHeader(header types.Header, consState *types.ConsensusState) error { + if header.TrustedHeight != consState.Height { + return sdkerrors.Wrapf( + types.ErrInvalidHeaderHeight, + "trusted header height %d does not match consensus state height %d", + header.TrustedHeight, consState.Height, + ) + } // assert that trustedVals is NextValidators of last trusted header // to do this, we check that trustedVals.Hash() == consState.NextValidatorsHash tvalHash := header.TrustedValidators.Hash() @@ -82,30 +85,17 @@ func checkValidity( header.TrustedValidators, consState.NextValidatorsHash, tvalHash, ) } - // assert trusting period has not yet passed - if currentTimestamp.Sub(consState.Timestamp) >= clientState.TrustingPeriod { - return sdkerrors.Wrapf( - types.ErrTrustingPeriodExpired, - "current timestamp minus the consensus state timestamp is greater than or equal to the trusting period (%s >= %s)", - currentTimestamp.Sub(consState.Timestamp), clientState.TrustingPeriod, - ) - } + return nil +} - // assert header timestamp is not past the trusting period - if header.Time.Sub(consState.Timestamp) >= clientState.TrustingPeriod { - return sdkerrors.Wrap( - clienttypes.ErrInvalidHeader, - "header timestamp is beyond trusting period in relation to the consensus state timestamp", - ) - } - - // assert header timestamp is past latest stored consensus state timestamp - if header.Time.Unix() <= consState.Timestamp.Unix() { - return sdkerrors.Wrapf( - clienttypes.ErrInvalidHeader, - "header timestamp ≤ consensus state timestamp (%s ≤ %s)", - header.Time.UTC(), consState.Timestamp.UTC(), - ) +// checkValidity checks if the Tendermint header is valid. +// CONTRACT: consState.Height == header.TrustedHeight +func checkValidity( + clientState *types.ClientState, consState *types.ConsensusState, + header types.Header, currentTimestamp time.Time, +) error { + if err := checkTrustedHeader(header, consState); err != nil { + return err } // assert header height is newer than consensus state @@ -128,6 +118,10 @@ func checkValidity( } // Verify next header with the passed-in trustedVals + // - asserts trusting period not passed + // - assert header timestamp is not past the trusting period + // - assert header timestamp is past latest stored consensus state timestamp + // - assert that a TrustLevel proportion of TrustedValidators signed new Commit err := lite.Verify( clientState.GetChainID(), &signedHeader, header.TrustedValidators, &header.SignedHeader, header.ValidatorSet,