From 01b4f3a5ad8c7e8a1a1d3f10fa7ba1ed02e4d67d Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 1 Nov 2022 16:26:15 +0000 Subject: [PATCH] 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 (cherry picked from commit eab24e82447007cf8bd3efd926d0f0ccdbd44263) # 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 --- CHANGELOG.md | 1 + .../07-tendermint/types/client_state.go | 12 +++++------ .../07-tendermint/types/client_state_test.go | 21 ++++++++++++++++--- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfba45fb782..d3f450bb73a 100644 --- 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 index 6deced02718..d19f8d5f7e8 100644 --- 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 index bb1b15a8de1..d780f20813d 100644 --- 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),