fix: forbid negative values for trusting period, unbonding period and…

… max clock drift (backport #2555) (#2620) * 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 * fix conflicts * delete files Co-authored-by: Carlos Rodriguez <[email protected]>
cosmos · Nov 1, 2022 · 01b4f3a · 01b4f3a
1 parent 12e1322
commit 01b4f3a
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (27-interchain-accounts) [\#2580](https://github.com/cosmos/ibc-go/issues/2580) Removing port prefix requirement from the ICA host channel handshake
 * (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
 

diff --git a/modules/light-clients/07-tendermint/types/client_state.go b/modules/light-clients/07-tendermint/types/client_state.go
@@ -115,14 +115,14 @@ func (cs ClientState) Validate() error {
 	if err := light.ValidateTrustLevel(cs.TrustLevel.ToTendermint()); err != nil {
 		return err
 	}
-	if cs.TrustingPeriod == 0 {
-		return sdkerrors.Wrap(ErrInvalidTrustingPeriod, "trusting period cannot be zero")
+	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 cannot be 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 cannot be 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

diff --git a/modules/light-clients/07-tendermint/types/client_state_test.go b/modules/light-clients/07-tendermint/types/client_state_test.go
@@ -112,20 +112,35 @@ func (suite *TendermintTestSuite) TestValidate() {
 			expPass:     false,
 		},
 		{
-			name:        "invalid trusting period",
+			name:        "invalid zero trusting period",
 			clientState: types.NewClientState(chainID, types.DefaultTrustLevel, 0, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false),
 			expPass:     false,
 		},
 		{
-			name:        "invalid unbonding period",
+			name:        "invalid negative trusting period",
+			clientState: types.NewClientState(chainID, types.DefaultTrustLevel, -1, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false),
+			expPass:     false,
+		},
+		{
+			name:        "invalid zero unbonding period",
 			clientState: types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, 0, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false),
 			expPass:     false,
 		},
 		{
-			name:        "invalid max clock drift",
+			name:        "invalid negative unbonding period",
+			clientState: types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, -1, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false),
+			expPass:     false,
+		},
+		{
+			name:        "invalid zero max clock drift",
 			clientState: types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, 0, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false),
 			expPass:     false,
 		},
+		{
+			name:        "invalid negative max clock drift",
+			clientState: types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, -1, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false),
+			expPass:     false,
+		},
 		{
 			name:        "invalid revision number",
 			clientState: types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clienttypes.NewHeight(1, 1), commitmenttypes.GetSDKSpecs(), upgradePath, false, false),