From 81a7cae633ab36a04ff3d0ec613ea8a641afbc31 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 11 Jul 2022 15:31:04 +0100 Subject: [PATCH] Check that client state is zeroed out for ibc client upgrade proposals (#1676) --- CHANGELOG.md | 1 + modules/core/02-client/types/proposal.go | 7 ++++++- modules/core/02-client/types/proposal_test.go | 17 ++++++++++++++--- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c021f77df94..1459234ad64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (modules/core/04-channel) [\#1130](https://github.com/cosmos/ibc-go/pull/1130) Call `packet.GetSequence()` rather than passing func in `WriteAcknowledgement` log output * (apps/29-fee) [\#1278](https://github.com/cosmos/ibc-go/pull/1278) The URI path for the query to get all incentivized packets for a specifc channel did not follow the same format as the rest of queries. +* (modules/core/02-client)[\#1676](https://github.com/cosmos/ibc-go/pull/1676) ClientState must be zeroed out for `UpgradeProposals` to pass validation. This prevents a proposal containing information governance is not actually voting on. ## [v3.0.0](https://github.com/cosmos/ibc-go/releases/tag/v3.0.0) - 2022-03-15 diff --git a/modules/core/02-client/types/proposal.go b/modules/core/02-client/types/proposal.go index 75c9778e8c9..02dc3c517ee 100644 --- a/modules/core/02-client/types/proposal.go +++ b/modules/core/02-client/types/proposal.go @@ -2,6 +2,7 @@ package types import ( "fmt" + "reflect" codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -111,11 +112,15 @@ func (up *UpgradeProposal) ValidateBasic() error { return sdkerrors.Wrap(ErrInvalidUpgradeProposal, "upgraded client state cannot be nil") } - _, err := UnpackClientState(up.UpgradedClientState) + clientState, err := UnpackClientState(up.UpgradedClientState) if err != nil { return sdkerrors.Wrap(err, "failed to unpack upgraded client state") } + if !reflect.DeepEqual(clientState, clientState.ZeroCustomFields()) { + return sdkerrors.Wrap(ErrInvalidUpgradeProposal, "upgraded client state is not zeroed out") + } + return nil } diff --git a/modules/core/02-client/types/proposal_test.go b/modules/core/02-client/types/proposal_test.go index a32dcdac4e8..ad5be469961 100644 --- a/modules/core/02-client/types/proposal_test.go +++ b/modules/core/02-client/types/proposal_test.go @@ -109,13 +109,24 @@ func (suite *TypesTestSuite) TestUpgradeProposalValidateBasic() { }{ { "success", func() { - proposal, err = types.NewUpgradeProposal(ibctesting.Title, ibctesting.Description, plan, cs) + proposal, err = types.NewUpgradeProposal(ibctesting.Title, ibctesting.Description, plan, cs.ZeroCustomFields()) suite.Require().NoError(err) }, true, }, { "fails validate abstract - empty title", func() { - proposal, err = types.NewUpgradeProposal("", ibctesting.Description, plan, cs) + proposal, err = types.NewUpgradeProposal("", ibctesting.Description, plan, cs.ZeroCustomFields()) + suite.Require().NoError(err) + + }, false, + }, + { + "non zeroed fields", func() { + proposal, err = types.NewUpgradeProposal(ibctesting.Title, ibctesting.Description, plan, &ibctmtypes.ClientState{ + FrozenHeight: types.Height{ + RevisionHeight: 10, + }, + }) suite.Require().NoError(err) }, false, @@ -123,7 +134,7 @@ func (suite *TypesTestSuite) TestUpgradeProposalValidateBasic() { { "plan height is zero", func() { invalidPlan := upgradetypes.Plan{Name: "ibc upgrade", Height: 0} - proposal, err = types.NewUpgradeProposal(ibctesting.Title, ibctesting.Description, invalidPlan, cs) + proposal, err = types.NewUpgradeProposal(ibctesting.Title, ibctesting.Description, invalidPlan, cs.ZeroCustomFields()) suite.Require().NoError(err) }, false, },