diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index d17d41679b8..732bd9a442f 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -9,6 +9,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" ) @@ -128,6 +129,14 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e return errorsmod.Wrapf(types.ErrClientNotActive, "cannot upgrade client (%s) with status %s", clientID, status) } + // last height of current counterparty chain must be client's latest height + lastHeight := clientState.GetLatestHeight() + + if !upgradedClient.GetLatestHeight().GT(lastHeight) { + return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", + upgradedClient.GetLatestHeight(), lastHeight) + } + if err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore, upgradedClient, upgradedConsState, proofUpgradeClient, proofUpgradeConsState, ); err != nil { diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index 0f0786fa873..7e9a5731e31 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -437,6 +437,35 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { }, expPass: false, }, + { + name: "unsuccessful upgrade: upgraded height is not greater than current height", + setup: func() { + // last Height is at next block + lastHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) + + // zero custom fields and store in upgrade store + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) + suite.Require().NoError(err) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) + suite.Require().NoError(err) + + // change upgradedClient height to be lower than current client state height + tmClient := upgradedClient.(*ibctm.ClientState) + tmClient.LatestHeight = clienttypes.NewHeight(0, 1) + upgradedClient = tmClient + + suite.coordinator.CommitBlock(suite.chainB) + err = path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) + suite.Require().True(found) + + proofUpgradedClient, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + proofUpgradedConsState, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + }, + expPass: false, + }, } for _, tc := range testCases { diff --git a/modules/light-clients/07-tendermint/upgrade.go b/modules/light-clients/07-tendermint/upgrade.go index a3159459816..61124f6c2f5 100644 --- a/modules/light-clients/07-tendermint/upgrade.go +++ b/modules/light-clients/07-tendermint/upgrade.go @@ -12,7 +12,6 @@ import ( clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" - ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" ) @@ -36,14 +35,6 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( return errorsmod.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade client, no upgrade path set") } - // last height of current counterparty chain must be client's latest height - lastHeight := cs.GetLatestHeight() - - if !upgradedClient.GetLatestHeight().GT(lastHeight) { - return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", - upgradedClient.GetLatestHeight(), lastHeight) - } - // upgraded client state and consensus state must be IBC tendermint client state and consensus state // this may be modified in the future to upgrade to a new IBC tendermint type // counterparty must also commit to the upgraded consensus state at a sub-path under the upgrade path specified @@ -67,6 +58,9 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(commitmenttypes.ErrInvalidProof, "could not unmarshal consensus state merkle proof: %v", err) } + // last height of current counterparty chain must be client's latest height + lastHeight := cs.GetLatestHeight() + // Must prove against latest consensus state to ensure we are verifying against latest upgrade plan // This verifies that upgrade is intended for the provided revision, since committed client must exist // at this consensus state