Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,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

Expand Down
12 changes: 6 additions & 6 deletions modules/light-clients/07-tendermint/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,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
Expand Down
21 changes: 18 additions & 3 deletions modules/light-clients/07-tendermint/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,35 @@ func (suite *TendermintTestSuite) TestValidate() {
expPass: false,
},
{
name: "invalid trusting period",
name: "invalid zero trusting period",
clientState: ibctm.NewClientState(chainID, ibctm.DefaultTrustLevel, 0, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath),
expPass: false,
},
{
name: "invalid unbonding period",
name: "invalid negative trusting period",
clientState: ibctm.NewClientState(chainID, ibctm.DefaultTrustLevel, -1, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath),
expPass: false,
},
{
name: "invalid zero unbonding period",
clientState: ibctm.NewClientState(chainID, ibctm.DefaultTrustLevel, trustingPeriod, 0, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath),
expPass: false,
},
{
name: "invalid max clock drift",
name: "invalid negative unbonding period",
clientState: ibctm.NewClientState(chainID, ibctm.DefaultTrustLevel, trustingPeriod, -1, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath),
expPass: false,
},
{
name: "invalid zero max clock drift",
clientState: ibctm.NewClientState(chainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, 0, height, commitmenttypes.GetSDKSpecs(), upgradePath),
expPass: false,
},
{
name: "invalid negative max clock drift",
clientState: ibctm.NewClientState(chainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, -1, height, commitmenttypes.GetSDKSpecs(), upgradePath),
expPass: false,
},
{
name: "invalid revision number",
clientState: ibctm.NewClientState(chainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clienttypes.NewHeight(1, 1), commitmenttypes.GetSDKSpecs(), upgradePath),
Expand Down