From 45494190e0ff30a8332bd1563281573a38af04d4 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 13 Jun 2022 14:45:46 +0100 Subject: [PATCH 1/4] fix: zero out client before processing upgraded client proof. --- modules/light-clients/07-tendermint/types/upgrade.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/upgrade.go b/modules/light-clients/07-tendermint/types/upgrade.go index 5e23c8d9036..4b08cc449c6 100644 --- a/modules/light-clients/07-tendermint/types/upgrade.go +++ b/modules/light-clients/07-tendermint/types/upgrade.go @@ -18,7 +18,7 @@ import ( // in client state that must be the same across all valid Tendermint clients for the new chain. // VerifyUpgrade will return an error if: // - the upgradedClient is not a Tendermint ClientState -// - the lastest height of the client state does not have the same revision number or has a greater +// - the latest height of the client state does not have the same revision number or has a greater // height than the committed client. // - the height of upgraded client is not greater than that of current client // - the latest height of the new client does not match or is greater than the height in committed client @@ -73,7 +73,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( } // Verify client proof - bz, err := cdc.MarshalInterface(upgradedClient) + bz, err := cdc.MarshalInterface(upgradedClient.ZeroCustomFields()) if err != nil { return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClient, "could not marshal client state: %v", err) } From 717642548f854f78411ad6320cd981234dc99fbc Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 13 Jun 2022 14:53:50 +0100 Subject: [PATCH 2/4] chore: adding changelog entry. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff0d73923e3..30ed68ade2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,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 specific channel did not follow the same format as the rest of queries. * (apps/29-fee)[\#1343](https://github.com/cosmos/ibc-go/pull/1523) Fixed an issue where a bad refund address would prevent channel closure. +* (07-tendermint) [\#1530](https://github.com/cosmos/ibc-go/pull/1530) Submitted client state is zeroed out before checking the proof. ## [v3.0.0](https://github.com/cosmos/ibc-go/releases/tag/v3.0.0) - 2022-03-15 From c79e48cba1fad1e6d39eb9a70dd88fd45a691494 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 13 Jun 2022 15:17:57 +0100 Subject: [PATCH 3/4] test: updated existing tests to remove zeroing in test. --- modules/light-clients/07-tendermint/types/upgrade_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/upgrade_test.go b/modules/light-clients/07-tendermint/types/upgrade_test.go index 6ad81df3520..d86e198a7ca 100644 --- a/modules/light-clients/07-tendermint/types/upgrade_test.go +++ b/modules/light-clients/07-tendermint/types/upgrade_test.go @@ -452,9 +452,6 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { cs := suite.chainA.GetClientState(path.EndpointA.ClientID) clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) - // Call ZeroCustomFields on upgraded clients to clear any client-chosen parameters in test-case upgradedClient - upgradedClient = upgradedClient.ZeroCustomFields() - clientState, consensusState, err := cs.VerifyUpgradeAndUpdateState( suite.chainA.GetContext(), suite.cdc, From 84d8dedf5b63ec880d09bd46d4daa564ba2d9733 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 13 Jun 2022 15:50:00 +0100 Subject: [PATCH 4/4] chore: adding more descriptive changelog entry and comment. --- CHANGELOG.md | 2 +- modules/light-clients/07-tendermint/types/upgrade.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30ed68ade2f..1cc4fe96ae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,7 +81,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 specific channel did not follow the same format as the rest of queries. * (apps/29-fee)[\#1343](https://github.com/cosmos/ibc-go/pull/1523) Fixed an issue where a bad refund address would prevent channel closure. -* (07-tendermint) [\#1530](https://github.com/cosmos/ibc-go/pull/1530) Submitted client state is zeroed out before checking the proof. +* (07-tendermint) [\#1530](https://github.com/cosmos/ibc-go/pull/1530) Submitted client state is zeroed out before checking the proof in order to prevent the proposal from 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/light-clients/07-tendermint/types/upgrade.go b/modules/light-clients/07-tendermint/types/upgrade.go index 4b08cc449c6..2e2bd988cbb 100644 --- a/modules/light-clients/07-tendermint/types/upgrade.go +++ b/modules/light-clients/07-tendermint/types/upgrade.go @@ -14,8 +14,9 @@ import ( ) // VerifyUpgradeAndUpdateState checks if the upgraded client has been committed by the current client -// It will zero out all client-specific fields (e.g. TrustingPeriod and verify all data +// It will zero out all client-specific fields (e.g. TrustingPeriod) and verify all data // in client state that must be the same across all valid Tendermint clients for the new chain. +// Zeroing out the submitted client prevents the proposal from containing information governance is not actually voting on. // VerifyUpgrade will return an error if: // - the upgradedClient is not a Tendermint ClientState // - the latest height of the client state does not have the same revision number or has a greater