Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

02-client routing: remove GetLatestHeight from ClientState interface and add it to LightClientModule #5866

Merged
merged 31 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
8d38948
wip: remove GetLatestHeight from ClientState interface and add it to …
crodriguezvega Feb 19, 2024
97895c0
Merge branch 'feat/02-client-router' into carlos/move-latest-height
damiannolan Mar 5, 2024
7eb5fef
chore: add godoc for GetLatestHeight in 02-client keeper
damiannolan Mar 6, 2024
4ee7800
chore: add args to event emission funcs in 02-client
damiannolan Mar 6, 2024
0460d53
chore: add args to usage in 02-client event emissions
damiannolan Mar 6, 2024
c5ff602
chore: put back logging in upgrade client
damiannolan Mar 6, 2024
26cb94c
chore: deprecate and refactor 04-channel client utils QueryLatestCons…
damiannolan Mar 6, 2024
4dfb2ea
chore: rename lightClientModule to clientModule in core
damiannolan Mar 6, 2024
16e4ec0
chore: undo faulty renames
damiannolan Mar 6, 2024
d819121
fix: godoc for LatestHeight
damiannolan Mar 6, 2024
1664b6a
chore: add godocs
damiannolan Mar 6, 2024
041f364
chore: readding sequence checking in solomachine RecoverClient
damiannolan Mar 6, 2024
2a2890e
fix: testing chain helper to use keeper getter for latest height
damiannolan Mar 6, 2024
c7383d2
chore: fix compilation errors in tests
chatton Mar 6, 2024
8d14457
fix: import ordering waaah
damiannolan Mar 6, 2024
814224d
imp: adding GetClientLatestHeight to testing endpoint/chain
damiannolan Mar 6, 2024
e2936d1
test: cleanup test funcs with path.Endpoint.GetClientLatestHeight
damiannolan Mar 6, 2024
cec25a6
test: cleanup tests in 04-channel to use path.Endpoint.GetClientLates…
damiannolan Mar 6, 2024
d100df9
test: cleanup ante_test
damiannolan Mar 6, 2024
b4350ca
chore: add godocs for light client modules LatestHeight method
damiannolan Mar 6, 2024
d47da6e
chore: update endpoint.go to use the new GetClientLatestHeight helper fn
chatton Mar 7, 2024
46f87fe
test: usage of path.Endpoint.GetClientLatestHeight in core/keeper and…
damiannolan Mar 7, 2024
37152f1
test: another one
damiannolan Mar 7, 2024
718947a
chore: rm GetLatestHeight from solomachine ClientState
damiannolan Mar 7, 2024
fac0967
test: update QueryConsensusStateProof in testing lib to use test helper
damiannolan Mar 8, 2024
fe6caf8
Update testing/chain.go
damiannolan Mar 8, 2024
a96b618
fix typo in variable name
crodriguezvega Mar 8, 2024
5dd271f
chore: consolidate two calls to LatestHeight to initialHeight var on …
damiannolan Mar 11, 2024
27a43ae
chore: extract to latestHeight var in VerifyUpgradeAndUpdateState to …
damiannolan Mar 11, 2024
b91af50
chore: rm greater than or equal height check in solomachine as alread…
damiannolan Mar 11, 2024
e2150d9
chore: address todo on GetLatestHeight in 02-client keeper
damiannolan Mar 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ Learn how to implement the [`ClientState`](https://github.com/cosmos/ibc-go/blob
`ClientType` should return a unique string identifier of the light client. This will be used when generating a client identifier.
The format is created as follows: `ClientType-{N}` where `{N}` is the unique global nonce associated with a specific client.

## `GetLatestHeight` method

`GetLatestHeight` should return the latest block height that the client state represents.

## `Validate` method

`Validate` should validate every client state field and should return an error if any value is invalid. The light client
Expand Down
8 changes: 4 additions & 4 deletions e2e/tests/core/02-client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,8 @@ func (s *ClientTestSuite) TestClient_Update_Misbehaviour() {
tmClientState, ok := clientState.(*ibctm.ClientState)
s.Require().True(ok)

trustedHeight, ok = tmClientState.GetLatestHeight().(clienttypes.Height)
s.Require().True(ok)
trustedHeight := tmClientState.LatestHeight
s.Require().True(trustedHeight.GT(clienttypes.ZeroHeight()))
})

t.Run("update clients", func(t *testing.T) {
Expand All @@ -377,8 +377,8 @@ func (s *ClientTestSuite) TestClient_Update_Misbehaviour() {
tmClientState, ok := clientState.(*ibctm.ClientState)
s.Require().True(ok)

latestHeight, ok = tmClientState.GetLatestHeight().(clienttypes.Height)
s.Require().True(ok)
latestHeight := tmClientState.LatestHeight
s.Require().True(latestHeight.GT(clienttypes.ZeroHeight()))
})

t.Run("create validator set", func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions e2e/tests/transfer/localhost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ func (s *LocalhostTransferTestSuite) TestMsgTransfer_Localhost() {
t.Run("verify begin blocker was executed", func(t *testing.T) {
cs, err := s.QueryClientState(ctx, chainA, exported.LocalhostClientID)
s.Require().NoError(err)
originalHeight := cs.GetLatestHeight()
originalHeight := cs.(*localhost.ClientState).LatestHeight

s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA), "failed to wait for blocks")

cs, err = s.QueryClientState(ctx, chainA, exported.LocalhostClientID)
s.Require().NoError(err)
s.Require().True(cs.GetLatestHeight().GT(originalHeight), "client state height was not incremented")
s.Require().True(cs.(*localhost.ClientState).LatestHeight.GT(originalHeight), "client state height was not incremented")
})

t.Run("channel open init localhost", func(t *testing.T) {
Expand Down
44 changes: 25 additions & 19 deletions modules/core/02-client/keeper/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ 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"
)

Expand All @@ -33,28 +32,28 @@ func (k Keeper) CreateClient(

clientID := k.GenerateClientIdentifier(ctx, clientType)

lightClientModule, found := k.router.GetRoute(clientID)
clientModule, found := k.router.GetRoute(clientID)
if !found {
return "", errorsmod.Wrap(types.ErrRouteNotFound, clientID)
}

if err := lightClientModule.Initialize(ctx, clientID, clientState, consensusState); err != nil {
if err := clientModule.Initialize(ctx, clientID, clientState, consensusState); err != nil {
return "", err
}

if status := k.GetClientStatus(ctx, clientID); status != exported.Active {
return "", errorsmod.Wrapf(types.ErrClientNotActive, "cannot create client (%s) with status %s", clientID, status)
}

// k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientState.GetLatestHeight().String())
k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientModule.LatestHeight(ctx, clientID).String())
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

defer telemetry.IncrCounterWithLabels(
[]string{"ibc", "client", "create"},
1,
[]metrics.Label{telemetry.NewLabel(types.LabelClientType, clientType)},
)

emitCreateClientEvent(ctx, clientID, clientType)
emitCreateClientEvent(ctx, clientID, clientType, clientModule.LatestHeight(ctx, clientID))

return clientID, nil
}
Expand All @@ -70,18 +69,18 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte
return errorsmod.Wrapf(types.ErrClientNotFound, "clientID (%s)", clientID)
}

lightClientModule, found := k.router.GetRoute(clientID)
clientModule, found := k.router.GetRoute(clientID)
if !found {
return errorsmod.Wrap(types.ErrRouteNotFound, clientID)
}

if err := lightClientModule.VerifyClientMessage(ctx, clientID, clientMsg); err != nil {
if err := clientModule.VerifyClientMessage(ctx, clientID, clientMsg); err != nil {
return err
}

foundMisbehaviour := lightClientModule.CheckForMisbehaviour(ctx, clientID, clientMsg)
foundMisbehaviour := clientModule.CheckForMisbehaviour(ctx, clientID, clientMsg)
if foundMisbehaviour {
lightClientModule.UpdateStateOnMisbehaviour(ctx, clientID, clientMsg)
clientModule.UpdateStateOnMisbehaviour(ctx, clientID, clientMsg)

k.Logger(ctx).Info("client frozen due to misbehaviour", "client-id", clientID)

Expand All @@ -100,7 +99,7 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte
return nil
}

consensusHeights := lightClientModule.UpdateState(ctx, clientID, clientMsg)
consensusHeights := clientModule.UpdateState(ctx, clientID, clientMsg)

k.Logger(ctx).Info("client state updated", "client-id", clientID, "heights", consensusHeights)

Expand Down Expand Up @@ -136,21 +135,22 @@ 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)
}

// TODO: This code is removed in https://github.com/cosmos/ibc-go/pull/5827
// last height of current counterparty chain must be client's latest height
lastHeight := clientState.GetLatestHeight()
// lastHeight := k.GetLatestHeight(ctx, clientID)

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 !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)
// }
Comment on lines +139 to +146
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to leave this and address in #5827

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't supposed to be removed 😄 Happy to see it rectified after though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! For sure, just wanted to punt until this PR was merged


if err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore,
upgradedClient, upgradedConsState, upgradeClientProof, upgradeConsensusStateProof,
); err != nil {
return errorsmod.Wrapf(err, "cannot upgrade client with ID %s", clientID)
}

k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", upgradedClient.GetLatestHeight().String())
k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", k.GetLatestHeight(ctx, clientID).String())
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

defer telemetry.IncrCounterWithLabels(
[]string{"ibc", "client", "upgrade"},
Expand All @@ -161,7 +161,7 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e
},
)

emitUpgradeClientEvent(ctx, clientID, upgradedClient)
emitUpgradeClientEvent(ctx, clientID, upgradedClient.ClientType(), k.GetLatestHeight(ctx, clientID))

return nil
}
Expand All @@ -187,12 +187,18 @@ func (k Keeper) RecoverClient(ctx sdk.Context, subjectClientID, substituteClient
return errorsmod.Wrapf(types.ErrClientNotFound, "clientID (%s)", subjectClientID)
}

lightClientModule, found := k.router.GetRoute(subjectClientID)
clientModule, found := k.router.GetRoute(subjectClientID)
if !found {
return errorsmod.Wrap(types.ErrRouteNotFound, subjectClientID)
}

if err := lightClientModule.RecoverClient(ctx, subjectClientID, substituteClientID); err != nil {
subjectLatestHeight := clientModule.LatestHeight(ctx, subjectClientID)
substituteLatestHeight := clientModule.LatestHeight(ctx, substituteClientID)
if subjectLatestHeight.GTE(substituteLatestHeight) {
return errorsmod.Wrapf(types.ErrInvalidHeight, "subject client state latest height is greater or equal to substitute client state latest height (%s >= %s)", subjectLatestHeight, substituteLatestHeight)
}

if err := clientModule.RecoverClient(ctx, subjectClientID, substituteClientID); err != nil {
return err
}

Expand Down
Loading
Loading