diff --git a/x/ibc/02-client/handler.go b/x/ibc/02-client/handler.go index 0a7cefcd5..294c79ed6 100644 --- a/x/ibc/02-client/handler.go +++ b/x/ibc/02-client/handler.go @@ -86,7 +86,9 @@ func HandlerClientMisbehaviour(k Keeper) evidencetypes.Handler { return func(ctx sdk.Context, evidence evidenceexported.Evidence) error { misbehaviour, ok := evidence.(exported.Misbehaviour) if !ok { - return types.ErrInvalidEvidence + return sdkerrors.Wrapf(types.ErrInvalidEvidence, + "expected evidence to implement client Misbehaviour interface, got %T", evidence, + ) } return k.CheckMisbehaviourAndUpdateState(ctx, misbehaviour) diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 481659a77..871d61516 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/x/ibc/02-client/keeper/client.go @@ -140,7 +140,7 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour ex switch e := misbehaviour.(type) { case ibctmtypes.Evidence: clientState, err = tendermint.CheckMisbehaviourAndUpdateState( - clientState, consensusState, misbehaviour, consensusState.GetHeight(), ctx.BlockTime(), + clientState, consensusState, misbehaviour, consensusState.GetHeight(), ctx.BlockTime(), ctx.ConsensusParams(), ) default: diff --git a/x/ibc/07-tendermint/misbehaviour.go b/x/ibc/07-tendermint/misbehaviour.go index d7e0d1e32..dae1e36af 100644 --- a/x/ibc/07-tendermint/misbehaviour.go +++ b/x/ibc/07-tendermint/misbehaviour.go @@ -3,7 +3,7 @@ package tendermint import ( "time" - lite "github.com/tendermint/tendermint/lite2" + abci "github.com/tendermint/tendermint/abci/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" clientexported "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" @@ -22,6 +22,7 @@ func CheckMisbehaviourAndUpdateState( misbehaviour clientexported.Misbehaviour, height uint64, // height at which the consensus state was loaded currentTimestamp time.Time, + consensusParams *abci.ConsensusParams, ) (clientexported.ClientState, error) { // cast the interface to specific types before checking for misbehaviour @@ -47,9 +48,9 @@ func CheckMisbehaviourAndUpdateState( } if err := checkMisbehaviour( - tmClientState, tmConsensusState, tmEvidence, height, currentTimestamp, + tmClientState, tmConsensusState, tmEvidence, height, currentTimestamp, consensusParams, ); err != nil { - return nil, sdkerrors.Wrap(clienttypes.ErrInvalidEvidence, err.Error()) + return nil, err } tmClientState.FrozenHeight = uint64(tmEvidence.GetHeight()) @@ -59,8 +60,32 @@ func CheckMisbehaviourAndUpdateState( // 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, + height uint64, currentTimestamp time.Time, consensusParams *abci.ConsensusParams, ) error { + // calculate the age of the misbehaviour evidence + infractionHeight := evidence.GetHeight() + infractionTime := evidence.GetTime() + ageDuration := currentTimestamp.Sub(infractionTime) + ageBlocks := height - uint64(infractionHeight) + + // 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. + // + // NOTE: The first condition is a safety check as the consensus params cannot + // be nil since the previous param values will be used in case they can't be + // retreived. If they are not set during initialization, Tendermint will always + // use the default values. + if consensusParams != nil && + consensusParams.Evidence != nil && + ageDuration > consensusParams.Evidence.MaxAgeDuration && + ageBlocks > uint64(consensusParams.Evidence.MaxAgeNumBlocks) { + return 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( @@ -81,21 +106,18 @@ func checkMisbehaviour( ) } - // TODO: Evidence must be within trusting period - // Blocked on https://github.com/cosmos/ics/issues/379 - // - ValidatorSet must have 2/3 similarity with trusted FromValidatorSet // - ValidatorSets on both headers are valid given the last trusted ValidatorSet if err := consensusState.ValidatorSet.VerifyCommitTrusting( evidence.ChainID, evidence.Header1.Commit.BlockID, evidence.Header1.Height, - evidence.Header1.Commit, lite.DefaultTrustLevel, + evidence.Header1.Commit, clientState.TrustLevel, ); err != nil { return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header 1 has too much change from last known validator set: %v", err) } if err := consensusState.ValidatorSet.VerifyCommitTrusting( evidence.ChainID, evidence.Header2.Commit.BlockID, evidence.Header2.Height, - evidence.Header2.Commit, lite.DefaultTrustLevel, + evidence.Header2.Commit, clientState.TrustLevel, ); err != nil { return sdkerrors.Wrapf(clienttypes.ErrInvalidEvidence, "validator set in header 2 has too much change from last known validator set: %v", err) } diff --git a/x/ibc/07-tendermint/misbehaviour_test.go b/x/ibc/07-tendermint/misbehaviour_test.go index 95e39b746..5cd54baa6 100644 --- a/x/ibc/07-tendermint/misbehaviour_test.go +++ b/x/ibc/07-tendermint/misbehaviour_test.go @@ -4,10 +4,13 @@ import ( "bytes" "time" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto/tmhash" lite "github.com/tendermint/tendermint/lite2" tmtypes "github.com/tendermint/tendermint/types" + "github.com/cosmos/cosmos-sdk/simapp" + clientexported "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" tendermint "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint" ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types" commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types" @@ -40,12 +43,14 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { altSigners := []tmtypes.PrivValidator{altPrivVal} testCases := []struct { - name string - clientState ibctmtypes.ClientState - consensusState ibctmtypes.ConsensusState - evidence ibctmtypes.Evidence - height uint64 - expPass bool + name string + clientState clientexported.ClientState + consensusState clientexported.ConsensusState + evidence clientexported.Misbehaviour + consensusParams *abci.ConsensusParams + height uint64 + timestamp time.Time + expPass bool }{ { "valid misbehavior evidence", @@ -57,7 +62,9 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ChainID: chainID, ClientID: chainID, }, + simapp.DefaultConsensusParams, height, + suite.now, true, }, { @@ -70,7 +77,9 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ChainID: chainID, ClientID: chainID, }, + simapp.DefaultConsensusParams, height - 1, + suite.now, true, }, { @@ -83,9 +92,111 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ChainID: chainID, ClientID: chainID, }, + simapp.DefaultConsensusParams, height - 1, + suite.now, true, }, + { + "invalid tendermint client state", + nil, + ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, altValSet, altSigners), + Header2: ibctmtypes.CreateTestHeader(chainID, height, suite.now.Add(time.Minute), bothValSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, + height, + suite.now, + false, + }, + { + "already frozen client state", + ibctmtypes.ClientState{FrozenHeight: 1}, + ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(chainID, height, suite.now.Add(time.Minute), bothValSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, + height, + suite.now, + false, + }, + { + "invalid tendermint consensus state", + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), + nil, + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, altValSet, altSigners), + Header2: ibctmtypes.CreateTestHeader(chainID, height, suite.now.Add(time.Minute), bothValSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, + height, + suite.now, + false, + }, + { + "invalid tendermint misbehaviour evidence", + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), + ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, + nil, + simapp.DefaultConsensusParams, + height, + suite.now, + false, + }, + { + "rejected misbehaviour due to expired age", + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), + ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(chainID, height, suite.now.Add(time.Minute), bothValSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, + 2*height + uint64(simapp.DefaultConsensusParams.Evidence.MaxAgeNumBlocks), + suite.now.Add(2 * time.Minute).Add(simapp.DefaultConsensusParams.Evidence.MaxAgeDuration), + false, + }, + { + "provided height ≠ header height", + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), + ibctmtypes.ConsensusState{Timestamp: suite.now, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(chainID, height, suite.now.Add(time.Minute), bothValSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, + height + 10, + suite.now, + false, + }, + { + "unbonding period expired", + ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), + ibctmtypes.ConsensusState{Timestamp: time.Time{}, Root: commitmenttypes.NewMerkleRoot(tmhash.Sum([]byte("app_hash"))), ValidatorSet: bothValSet}, + ibctmtypes.Evidence{ + Header1: ibctmtypes.CreateTestHeader(chainID, height, suite.now, bothValSet, bothSigners), + Header2: ibctmtypes.CreateTestHeader(chainID, height, suite.now.Add(time.Minute), bothValSet, bothSigners), + ChainID: chainID, + ClientID: chainID, + }, + simapp.DefaultConsensusParams, + height, + suite.now, + false, + }, { "first valset has too much change", ibctmtypes.NewClientState(chainID, lite.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, suite.header, commitmenttypes.GetSDKSpecs()), @@ -96,7 +207,9 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ChainID: chainID, ClientID: chainID, }, + simapp.DefaultConsensusParams, height, + suite.now, false, }, { @@ -109,7 +222,9 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ChainID: chainID, ClientID: chainID, }, + simapp.DefaultConsensusParams, height, + suite.now, false, }, { @@ -122,7 +237,9 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { ChainID: chainID, ClientID: chainID, }, + simapp.DefaultConsensusParams, height, + suite.now, false, }, } @@ -130,7 +247,7 @@ func (suite *TendermintTestSuite) TestCheckMisbehaviour() { for i, tc := range testCases { tc := tc - clientState, err := tendermint.CheckMisbehaviourAndUpdateState(tc.clientState, tc.consensusState, tc.evidence, tc.height, suite.now) + clientState, err := tendermint.CheckMisbehaviourAndUpdateState(tc.clientState, tc.consensusState, tc.evidence, tc.height, tc.timestamp, tc.consensusParams) if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name) diff --git a/x/ibc/07-tendermint/types/evidence.go b/x/ibc/07-tendermint/types/evidence.go index ebfe51879..d63a49665 100644 --- a/x/ibc/07-tendermint/types/evidence.go +++ b/x/ibc/07-tendermint/types/evidence.go @@ -2,6 +2,7 @@ package types import ( "math" + "time" yaml "gopkg.in/yaml.v2" @@ -73,6 +74,14 @@ func (ev Evidence) GetHeight() int64 { return int64(math.Min(float64(ev.Header1.Height), float64(ev.Header2.Height))) } +// GetTime returns the timestamp at which misbehaviour occurred. It uses the +// maximum value from both headers to prevent producing an invalid header outside +// of the evidence age range. +func (ev Evidence) GetTime() time.Time { + minTime := int64(math.Max(float64(ev.Header1.Time.UnixNano()), float64(ev.Header2.Time.UnixNano()))) + return time.Unix(0, minTime) +} + // ValidateBasic implements Evidence interface func (ev Evidence) ValidateBasic() error { if err := host.ClientIdentifierValidator(ev.ClientID); err != nil {