From 425579a6c34e8352fa2b2a57d942d7079757dfbc Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 8 Jul 2022 09:59:33 +0100 Subject: [PATCH 1/4] fix: ValidateBasic now ensures ClientState is zeroed out --- modules/core/02-client/types/proposal.go | 7 ++++++- modules/core/02-client/types/proposal_test.go | 17 ++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) 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, }, From 9e8f6fd053c5589c088abc35a73462e0978e5408 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 8 Jul 2022 10:01:06 +0100 Subject: [PATCH 2/4] chore: Added entry to CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c021f77df94..32bc1402a9e 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)[\#X](https://github.com/cosmos/ibc-go/pull/X) ClientState must be zeroed out for `UpgradeProposals` to pass validation. This prevents the 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 From 0eb70ce431520e8882b0c77281345e22e1d5a796 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 8 Jul 2022 10:02:06 +0100 Subject: [PATCH 3/4] chore: updating PR number in CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32bc1402a9e..f38da87a928 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,7 +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)[\#X](https://github.com/cosmos/ibc-go/pull/X) ClientState must be zeroed out for `UpgradeProposals` to pass validation. This prevents the proposal containing information governance is not actually voting on. +* (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 the 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 From d9a9d46282d413201ce7881bd6c81151305dacd7 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 8 Jul 2022 11:12:01 +0100 Subject: [PATCH 4/4] Update CHANGELOG.md Co-authored-by: Charly --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f38da87a928..1459234ad64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,7 +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 the proposal containing information governance is not actually voting on. +* (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