fix: forbid negative values for trusting period, unbonding period and max clock drift (#2555)

Co-authored-by: Carlos Rodriguez <[email protected]>
(cherry picked from commit eab24e8)

# Conflicts:
#	modules/light-clients/07-tendermint/client_state.go
#	modules/light-clients/07-tendermint/client_state_test.go

Author: crodriguezvega

CHANGELOG.md

@@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### State Machine Breaking
 
 * (transfer) [\#2377](https://github.com/cosmos/ibc-go/pull/2377) Adding `sequence` to `MsgTransferResponse`.
+* (light-clients/07-tendermint) [\#2554](https://github.com/cosmos/ibc-go/pull/2554) Forbid negative values for `TrustingPeriod`, `UnbondingPeriod` and `MaxClockDrift` (as specified in ICS-07).
 
 ### Improvements
 

modules/light-clients/07-tendermint/client_state.go

@@ -0,0 +1,333 @@
+package tendermint
+
+import (
+	"strings"
+	"time"
+
+	ics23 "github.com/confio/ics23/go"
+	"github.com/cosmos/cosmos-sdk/codec"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/tendermint/tendermint/light"
+	tmtypes "github.com/tendermint/tendermint/types"
+
+	clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+	commitmenttypes "github.com/cosmos/ibc-go/v6/modules/core/23-commitment/types"
+	"github.com/cosmos/ibc-go/v6/modules/core/exported"
+)
+
+var _ exported.ClientState = (*ClientState)(nil)
+
+// NewClientState creates a new ClientState instance
+func NewClientState(
+	chainID string, trustLevel Fraction,
+	trustingPeriod, ubdPeriod, maxClockDrift time.Duration,
+	latestHeight clienttypes.Height, specs []*ics23.ProofSpec,
+	upgradePath []string,
+) *ClientState {
+	return &ClientState{
+		ChainId:         chainID,
+		TrustLevel:      trustLevel,
+		TrustingPeriod:  trustingPeriod,
+		UnbondingPeriod: ubdPeriod,
+		MaxClockDrift:   maxClockDrift,
+		LatestHeight:    latestHeight,
+		FrozenHeight:    clienttypes.ZeroHeight(),
+		ProofSpecs:      specs,
+		UpgradePath:     upgradePath,
+	}
+}
+
+// GetChainID returns the chain-id
+func (cs ClientState) GetChainID() string {
+	return cs.ChainId
+}
+
+// ClientType is tendermint.
+func (cs ClientState) ClientType() string {
+	return exported.Tendermint
+}
+
+// GetLatestHeight returns latest block height.
+func (cs ClientState) GetLatestHeight() exported.Height {
+	return cs.LatestHeight
+}
+
+// GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.
+func (cs ClientState) GetTimestampAtHeight(
+	ctx sdk.Context,
+	clientStore sdk.KVStore,
+	cdc codec.BinaryCodec,
+	height exported.Height,
+) (uint64, error) {
+	// get consensus state at height from clientStore to check for expiry
+	consState, found := GetConsensusState(clientStore, cdc, height)
+	if !found {
+		return 0, sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "height (%s)", height)
+	}
+	return consState.GetTimestamp(), nil
+}
+
+// Status returns the status of the tendermint client.
+// The client may be:
+// - Active: FrozenHeight is zero and client is not expired
+// - Frozen: Frozen Height is not zero
+// - Expired: the latest consensus state timestamp + trusting period <= current time
+//
+// A frozen client will become expired, so the Frozen status
+// has higher precedence.
+func (cs ClientState) Status(
+	ctx sdk.Context,
+	clientStore sdk.KVStore,
+	cdc codec.BinaryCodec,
+) exported.Status {
+	if !cs.FrozenHeight.IsZero() {
+		return exported.Frozen
+	}
+
+	// get latest consensus state from clientStore to check for expiry
+	consState, found := GetConsensusState(clientStore, cdc, cs.GetLatestHeight())
+	if !found {
+		// if the client state does not have an associated consensus state for its latest height
+		// then it must be expired
+		return exported.Expired
+	}
+
+	if cs.IsExpired(consState.Timestamp, ctx.BlockTime()) {
+		return exported.Expired
+	}
+
+	return exported.Active
+}
+
+// IsExpired returns whether or not the client has passed the trusting period since the last
+// update (in which case no headers are considered valid).
+func (cs ClientState) IsExpired(latestTimestamp, now time.Time) bool {
+	expirationTime := latestTimestamp.Add(cs.TrustingPeriod)
+	return !expirationTime.After(now)
+}
+
+// Validate performs a basic validation of the client state fields.
+func (cs ClientState) Validate() error {
+	if strings.TrimSpace(cs.ChainId) == "" {
+		return sdkerrors.Wrap(ErrInvalidChainID, "chain id cannot be empty string")
+	}
+
+	// NOTE: the value of tmtypes.MaxChainIDLen may change in the future.
+	// If this occurs, the code here must account for potential difference
+	// between the tendermint version being run by the counterparty chain
+	// and the tendermint version used by this light client.
+	// https://github.com/cosmos/ibc-go/issues/177
+	if len(cs.ChainId) > tmtypes.MaxChainIDLen {
+		return sdkerrors.Wrapf(ErrInvalidChainID, "chainID is too long; got: %d, max: %d", len(cs.ChainId), tmtypes.MaxChainIDLen)
+	}
+
+	if err := light.ValidateTrustLevel(cs.TrustLevel.ToTendermint()); err != nil {
+		return err
+	}
+	if cs.TrustingPeriod <= 0 {
+		return sdkerrors.Wrap(ErrInvalidTrustingPeriod, "trusting period must be greater than zero")
+	}
+	if cs.UnbondingPeriod <= 0 {
+		return sdkerrors.Wrap(ErrInvalidUnbondingPeriod, "unbonding period must be greater than zero")
+	}
+	if cs.MaxClockDrift <= 0 {
+		return sdkerrors.Wrap(ErrInvalidMaxClockDrift, "max clock drift must be greater than zero")
+	}
+
+	// the latest height revision number must match the chain id revision number
+	if cs.LatestHeight.RevisionNumber != clienttypes.ParseChainID(cs.ChainId) {
+		return sdkerrors.Wrapf(ErrInvalidHeaderHeight,
+			"latest height revision number must match chain id revision number (%d != %d)", cs.LatestHeight.RevisionNumber, clienttypes.ParseChainID(cs.ChainId))
+	}
+	if cs.LatestHeight.RevisionHeight == 0 {
+		return sdkerrors.Wrapf(ErrInvalidHeaderHeight, "tendermint client's latest height revision height cannot be zero")
+	}
+	if cs.TrustingPeriod >= cs.UnbondingPeriod {
+		return sdkerrors.Wrapf(
+			ErrInvalidTrustingPeriod,
+			"trusting period (%s) should be < unbonding period (%s)", cs.TrustingPeriod, cs.UnbondingPeriod,
+		)
+	}
+
+	if cs.ProofSpecs == nil {
+		return sdkerrors.Wrap(ErrInvalidProofSpecs, "proof specs cannot be nil for tm client")
+	}
+	for i, spec := range cs.ProofSpecs {
+		if spec == nil {
+			return sdkerrors.Wrapf(ErrInvalidProofSpecs, "proof spec cannot be nil at index: %d", i)
+		}
+	}
+	// UpgradePath may be empty, but if it isn't, each key must be non-empty
+	for i, k := range cs.UpgradePath {
+		if strings.TrimSpace(k) == "" {
+			return sdkerrors.Wrapf(clienttypes.ErrInvalidClient, "key in upgrade path at index %d cannot be empty", i)
+		}
+	}
+
+	return nil
+}
+
+// GetProofSpecs returns the format the client expects for proof verification
+// as a string array specifying the proof type for each position in chained proof
+func (cs ClientState) GetProofSpecs() []*ics23.ProofSpec {
+	return cs.ProofSpecs
+}
+
+// ZeroCustomFields returns a ClientState that is a copy of the current ClientState
+// with all client customizable fields zeroed out
+func (cs ClientState) ZeroCustomFields() exported.ClientState {
+	// copy over all chain-specified fields
+	// and leave custom fields empty
+	return &ClientState{
+		ChainId:         cs.ChainId,
+		UnbondingPeriod: cs.UnbondingPeriod,
+		LatestHeight:    cs.LatestHeight,
+		ProofSpecs:      cs.ProofSpecs,
+		UpgradePath:     cs.UpgradePath,
+	}
+}
+
+// Initialize will check that initial consensus state is a Tendermint consensus state
+// and will store ProcessedTime for initial consensus state as ctx.BlockTime()
+func (cs ClientState) Initialize(ctx sdk.Context, _ codec.BinaryCodec, clientStore sdk.KVStore, consState exported.ConsensusState) error {
+	if _, ok := consState.(*ConsensusState); !ok {
+		return sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "invalid initial consensus state. expected type: %T, got: %T",
+			&ConsensusState{}, consState)
+	}
+	// set metadata for initial consensus state.
+	setConsensusMetadata(ctx, clientStore, cs.GetLatestHeight())
+	return nil
+}
+
+// VerifyMembership is a generic proof verification method which verifies a proof of the existence of a value at a given CommitmentPath at the specified height.
+// The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
+func (cs ClientState) VerifyMembership(
+	ctx sdk.Context,
+	clientStore sdk.KVStore,
+	cdc codec.BinaryCodec,
+	height exported.Height,
+	delayTimePeriod uint64,
+	delayBlockPeriod uint64,
+	proof []byte,
+	path []byte,
+	value []byte,
+) error {
+	if cs.GetLatestHeight().LT(height) {
+		return sdkerrors.Wrapf(
+			sdkerrors.ErrInvalidHeight,
+			"client state height < proof height (%d < %d), please ensure the client has been updated", cs.GetLatestHeight(), height,
+		)
+	}
+
+	if err := verifyDelayPeriodPassed(ctx, clientStore, height, delayTimePeriod, delayBlockPeriod); err != nil {
+		return err
+	}
+
+	var merkleProof commitmenttypes.MerkleProof
+	if err := cdc.Unmarshal(proof, &merkleProof); err != nil {
+		return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal proof into ICS 23 commitment merkle proof")
+	}
+
+	var merklePath commitmenttypes.MerklePath
+	if err := cdc.Unmarshal(path, &merklePath); err != nil {
+		return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal path into ICS 23 commitment merkle path")
+	}
+
+	consensusState, found := GetConsensusState(clientStore, cdc, height)
+	if !found {
+		return sdkerrors.Wrap(clienttypes.ErrConsensusStateNotFound, "please ensure the proof was constructed against a height that exists on the client")
+	}
+
+	if err := merkleProof.VerifyMembership(cs.ProofSpecs, consensusState.GetRoot(), merklePath, value); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// VerifyNonMembership is a generic proof verification method which verifies the absence of a given CommitmentPath at a specified height.
+// The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24).
+func (cs ClientState) VerifyNonMembership(
+	ctx sdk.Context,
+	clientStore sdk.KVStore,
+	cdc codec.BinaryCodec,
+	height exported.Height,
+	delayTimePeriod uint64,
+	delayBlockPeriod uint64,
+	proof []byte,
+	path []byte,
+) error {
+	if cs.GetLatestHeight().LT(height) {
+		return sdkerrors.Wrapf(
+			sdkerrors.ErrInvalidHeight,
+			"client state height < proof height (%d < %d), please ensure the client has been updated", cs.GetLatestHeight(), height,
+		)
+	}
+
+	if err := verifyDelayPeriodPassed(ctx, clientStore, height, delayTimePeriod, delayBlockPeriod); err != nil {
+		return err
+	}
+
+	var merkleProof commitmenttypes.MerkleProof
+	if err := cdc.Unmarshal(proof, &merkleProof); err != nil {
+		return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal proof into ICS 23 commitment merkle proof")
+	}
+
+	var merklePath commitmenttypes.MerklePath
+	if err := cdc.Unmarshal(path, &merklePath); err != nil {
+		return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "failed to unmarshal path into ICS 23 commitment merkle path")
+	}
+
+	consensusState, found := GetConsensusState(clientStore, cdc, height)
+	if !found {
+		return sdkerrors.Wrap(clienttypes.ErrConsensusStateNotFound, "please ensure the proof was constructed against a height that exists on the client")
+	}
+
+	if err := merkleProof.VerifyNonMembership(cs.ProofSpecs, consensusState.GetRoot(), merklePath); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// verifyDelayPeriodPassed will ensure that at least delayTimePeriod amount of time and delayBlockPeriod number of blocks have passed
+// since consensus state was submitted before allowing verification to continue.
+func verifyDelayPeriodPassed(ctx sdk.Context, store sdk.KVStore, proofHeight exported.Height, delayTimePeriod, delayBlockPeriod uint64) error {
+	if delayTimePeriod != 0 {
+		// check that executing chain's timestamp has passed consensusState's processed time + delay time period
+		processedTime, ok := GetProcessedTime(store, proofHeight)
+		if !ok {
+			return sdkerrors.Wrapf(ErrProcessedTimeNotFound, "processed time not found for height: %s", proofHeight)
+		}
+
+		currentTimestamp := uint64(ctx.BlockTime().UnixNano())
+		validTime := processedTime + delayTimePeriod
+
+		// NOTE: delay time period is inclusive, so if currentTimestamp is validTime, then we return no error
+		if currentTimestamp < validTime {
+			return sdkerrors.Wrapf(ErrDelayPeriodNotPassed, "cannot verify packet until time: %d, current time: %d",
+				validTime, currentTimestamp)
+		}
+
+	}
+
+	if delayBlockPeriod != 0 {
+		// check that executing chain's height has passed consensusState's processed height + delay block period
+		processedHeight, ok := GetProcessedHeight(store, proofHeight)
+		if !ok {
+			return sdkerrors.Wrapf(ErrProcessedHeightNotFound, "processed height not found for height: %s", proofHeight)
+		}
+
+		currentHeight := clienttypes.GetSelfHeight(ctx)
+		validHeight := clienttypes.NewHeight(processedHeight.GetRevisionNumber(), processedHeight.GetRevisionHeight()+delayBlockPeriod)
+
+		// NOTE: delay block period is inclusive, so if currentHeight is validHeight, then we return no error
+		if currentHeight.LT(validHeight) {
+			return sdkerrors.Wrapf(ErrDelayPeriodNotPassed, "cannot verify packet until height: %s, current height: %s",
+				validHeight, currentHeight)
+		}
+	}
+
+	return nil
+}