Skip to content

Commit

Permalink
Move check for upgraded client state height to 02-client layer.
Browse files Browse the repository at this point in the history
  • Loading branch information
DimitrisJim committed Aug 16, 2023
1 parent 9b7d01e commit 4841ad2
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 9 deletions.
9 changes: 9 additions & 0 deletions modules/core/02-client/keeper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
ibcerrors "github.com/cosmos/ibc-go/v7/modules/core/errors"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
)

Expand Down Expand Up @@ -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 {
Expand Down
29 changes: 29 additions & 0 deletions modules/core/02-client/keeper/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 3 additions & 9 deletions modules/light-clients/07-tendermint/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/v7/modules/core/23-commitment/types"
ibcerrors "github.com/cosmos/ibc-go/v7/modules/core/errors"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
)

Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 4841ad2

Please sign in to comment.