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.

Sign up for GitHub

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

chore: pull out get light client module to helper methods #6712

Merged
merged 4 commits into from
Jul 3, 2024
Merged

chore: pull out get light client module to helper methods #6712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6041052
chore: pull out get light client module to helper methods
gjermundgaraba Jun 26, 2024
4da89a0
another place to use getLightClientModuleRoute
gjermundgaraba Jun 26, 2024
5ce4d31
use getLightClientModule in other 02-client functions
crodriguezvega Jul 2, 2024
bea2935
lint
crodriguezvega Jul 2, 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
26 changes: 9 additions & 17 deletions modules/core/02-client/keeper/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,11 @@ func (k *Keeper) CreateClient(ctx sdk.Context, clientType string, clientState, c
return "", errorsmod.Wrapf(types.ErrInvalidClientType, "cannot create client of type: %s", clientType)
}

params := k.GetParams(ctx)
if !params.IsAllowedClient(clientType) {
return "", errorsmod.Wrapf(
types.ErrInvalidClientType,
"client state type %s is not registered in the allowlist", clientType,
)
}

clientID := k.GenerateClientIdentifier(ctx, clientType)

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

if err := clientModule.Initialize(ctx, clientID, clientState, consensusState); err != nil {
Expand Down Expand Up @@ -70,9 +62,9 @@ func (k *Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg export
return errorsmod.Wrapf(err, "unable to parse client identifier %s", clientID)
}

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

if err := clientModule.VerifyClientMessage(ctx, clientID, clientMsg); err != nil {
Expand Down Expand Up @@ -136,9 +128,9 @@ func (k *Keeper) UpgradeClient(
return errorsmod.Wrapf(err, "unable to parse client identifier %s", clientID)
}

clientModule, found := k.router.GetRoute(clientID)
if !found {
return errorsmod.Wrap(types.ErrRouteNotFound, clientID)
clientModule, err := k.getLightClientModule(ctx, clientID)
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to leave the parsing of the client ID above, because the client type is used in the telemetry below. Same thing in UpdateClient. If somebody has a proposal to get rid of it (since it is also done in getLightClientModule), I'm all ears.

if err != nil {
return err
}

if err := clientModule.VerifyUpgradeAndUpdateState(ctx, clientID, upgradedClient, upgradedConsState, upgradeClientProof, upgradeConsensusStateProof); err != nil {
Expand Down
61 changes: 28 additions & 33 deletions modules/core/02-client/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,8 @@ func (k *Keeper) HasClientConsensusState(ctx sdk.Context, clientID string, heigh

// GetLatestClientConsensusState gets the latest ConsensusState stored for a given client
func (k *Keeper) GetLatestClientConsensusState(ctx sdk.Context, clientID string) (exported.ConsensusState, bool) {
clientModule, found := k.router.GetRoute(clientID)
if !found {
clientModule, err := k.getLightClientModule(ctx, clientID)
if err != nil {
return nil, false
}

Expand Down Expand Up @@ -393,40 +393,22 @@ func (k *Keeper) ClientStore(ctx sdk.Context, clientID string) storetypes.KVStor
// GetClientStatus returns the status for a client state given a client identifier. If the client type is not in the allowed
// clients param field, Unauthorized is returned, otherwise the client state status is returned.
func (k *Keeper) GetClientStatus(ctx sdk.Context, clientID string) exported.Status {
clientType, _, err := types.ParseClientIdentifier(clientID)
clientModule, err := k.getLightClientModule(ctx, clientID)
if err != nil {
return exported.Unauthorized
}

if !k.GetParams(ctx).IsAllowedClient(clientType) {
return exported.Unauthorized
}

clientModule, found := k.router.GetRoute(clientID)
if !found {
return exported.Unauthorized
}

return clientModule.Status(ctx, clientID)
}

// GetClientLatestHeight returns the latest height of a client state for a given client identifier. If the client type is not in the allowed
// clients param field, a zero value height is returned, otherwise the client state latest height is returned.
func (k *Keeper) GetClientLatestHeight(ctx sdk.Context, clientID string) types.Height {
clientType, _, err := types.ParseClientIdentifier(clientID)
clientModule, err := k.getLightClientModule(ctx, clientID)
if err != nil {
return types.ZeroHeight()
}

if !k.GetParams(ctx).IsAllowedClient(clientType) {
return types.ZeroHeight()
}

clientModule, found := k.router.GetRoute(clientID)
if !found {
return types.ZeroHeight()
}

var latestHeight types.Height
latestHeight, ok := clientModule.LatestHeight(ctx, clientID).(types.Height)
if !ok {
Expand All @@ -437,18 +419,9 @@ func (k *Keeper) GetClientLatestHeight(ctx sdk.Context, clientID string) types.H

// GetClientTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.
func (k *Keeper) GetClientTimestampAtHeight(ctx sdk.Context, clientID string, height exported.Height) (uint64, error) {
clientType, _, err := types.ParseClientIdentifier(clientID)
clientModule, err := k.getLightClientModule(ctx, clientID)
if err != nil {
return 0, errorsmod.Wrapf(err, "clientID (%s)", clientID)
}

if !k.GetParams(ctx).IsAllowedClient(clientType) {
return 0, errorsmod.Wrapf(types.ErrInvalidClientType, "client state type %s is not registered in the allowlist", clientType)
}

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

return clientModule.TimestampAtHeight(ctx, clientID, height)
Expand Down Expand Up @@ -503,3 +476,25 @@ func (k *Keeper) ScheduleIBCSoftwareUpgrade(ctx sdk.Context, plan upgradetypes.P

return nil
}

// getLightClientModule checks that the clientType of clientID is allowed and returns the corresponding light client module
func (k *Keeper) getLightClientModule(ctx sdk.Context, clientID string) (exported.LightClientModule, error) {
clientType, _, err := types.ParseClientIdentifier(clientID)
if err != nil {
return nil, errorsmod.Wrapf(err, "unable to parse client identifier %s", clientID)
}

if !k.GetParams(ctx).IsAllowedClient(clientType) {
return nil, errorsmod.Wrapf(
types.ErrInvalidClientType,
"client (%s) type %s is not in the allowed client list", clientID, clientType,
)
}

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

return clientModule, nil
}
4 changes: 2 additions & 2 deletions modules/light-clients/09-localhost/client_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (ClientState) VerifyMembership(
}

// The commitment prefix (eg: "ibc") is omitted when operating on the core IBC store
bz := store.Get([]byte(merklePath.KeyPath[1]))
bz := store.Get(merklePath.KeyPath[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

Linter was complaining about this.

if bz == nil {
return errorsmod.Wrapf(clienttypes.ErrFailedMembershipVerification, "value not found for path %s", path)
}
Expand Down Expand Up @@ -129,7 +129,7 @@ func (ClientState) VerifyNonMembership(
}

// The commitment prefix (eg: "ibc") is omitted when operating on the core IBC store
if store.Has([]byte(merklePath.KeyPath[1])) {
if store.Has(merklePath.KeyPath[1]) {
return errorsmod.Wrapf(clienttypes.ErrFailedNonMembershipVerification, "value found for path %s", path)
}

Expand Down
Loading