From b62e7d4b72f090f0aca4b292c3fbf88b0ab7c6a5 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 9 Jul 2024 14:32:24 +0200 Subject: [PATCH] imp: remove `Route` method in favour of `Verify(Non)Membership` on client keeper (#6760) * chore: pull out get light client module to helper methods (#6712) * chore: pull out get light client module to helper methods * another place to use getLightClientModuleRoute * use getLightClientModule in other 02-client functions * lint --------- Co-authored-by: Carlos Rodriguez * add verify(non)membership functions to client keeper and removed route from interface * delete file * use VerifyMembershipProof function name until https://github.com/cosmos/ibc-go/issues/6775 * Update modules/core/02-client/keeper/keeper.go Co-authored-by: DimitrisJim --------- Co-authored-by: Gjermund Garaba Co-authored-by: DimitrisJim --- modules/core/02-client/keeper/client.go | 26 +++--- modules/core/02-client/keeper/keeper.go | 81 +++++++++++-------- modules/core/03-connection/keeper/verify.go | 77 +++--------------- .../03-connection/types/expected_keepers.go | 3 +- 4 files changed, 70 insertions(+), 117 deletions(-) diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index 07adefa2e53..3ed572520f0 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -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 { @@ -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 { @@ -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) + if err != nil { + return err } if err := clientModule.VerifyUpgradeAndUpdateState(ctx, clientID, upgradedClient, upgradedConsState, upgradeClientProof, upgradeConsensusStateProof); err != nil { diff --git a/modules/core/02-client/keeper/keeper.go b/modules/core/02-client/keeper/keeper.go index cb3097dbb89..76c7e3cccfa 100644 --- a/modules/core/02-client/keeper/keeper.go +++ b/modules/core/02-client/keeper/keeper.go @@ -295,8 +295,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 } @@ -318,6 +318,26 @@ func (k *Keeper) ValidateSelfClient(ctx sdk.Context, clientState exported.Client return k.consensusHost.ValidateSelfClient(ctx, clientState) } +// VerifyMembershipProof retrieves the light client module for the clientID and verifies the proof of the existence of a key-value pair at a specified height. +func (k *Keeper) VerifyMembershipProof(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path, value []byte) error { + clientModule, err := k.getLightClientModule(ctx, clientID) + if err != nil { + return err + } + + return clientModule.VerifyMembership(ctx, clientID, height, delayTimePeriod, delayBlockPeriod, proof, path, value) +} + +// VerifyNonMembership retrieves the light client module for the clientID and verifies the absence of a given key at a specified height. +func (k *Keeper) VerifyNonMembership(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path) error { + clientModule, err := k.getLightClientModule(ctx, clientID) + if err != nil { + return err + } + + return clientModule.VerifyNonMembership(ctx, clientID, height, delayTimePeriod, delayBlockPeriod, proof, path) +} + // GetUpgradePlan executes the upgrade keeper GetUpgradePlan function. func (k *Keeper) GetUpgradePlan(ctx sdk.Context) (upgradetypes.Plan, error) { return k.upgradeKeeper.GetUpgradePlan(ctx) @@ -383,40 +403,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 { @@ -427,18 +429,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) @@ -493,3 +486,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 +} diff --git a/modules/core/03-connection/keeper/verify.go b/modules/core/03-connection/keeper/verify.go index 31968c304c4..31d7fb32bb7 100644 --- a/modules/core/03-connection/keeper/verify.go +++ b/modules/core/03-connection/keeper/verify.go @@ -29,11 +29,6 @@ func (k *Keeper) VerifyClientState( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - merklePath := commitmenttypes.NewMerklePath(host.FullClientStateKey(connection.Counterparty.ClientId)) merklePath, err := commitmenttypes.ApplyPrefix(connection.Counterparty.Prefix, merklePath) if err != nil { @@ -45,7 +40,7 @@ func (k *Keeper) VerifyClientState( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, height, 0, 0, // skip delay period checks for non-packet processing verification proof, merklePath, bz, @@ -71,11 +66,6 @@ func (k *Keeper) VerifyClientConsensusState( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - merklePath := commitmenttypes.NewMerklePath(host.FullConsensusStateKey(connection.Counterparty.ClientId, consensusHeight)) merklePath, err := commitmenttypes.ApplyPrefix(connection.Counterparty.Prefix, merklePath) if err != nil { @@ -87,7 +77,7 @@ func (k *Keeper) VerifyClientConsensusState( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, height, 0, 0, // skip delay period checks for non-packet processing verification proof, merklePath, bz, @@ -113,11 +103,6 @@ func (k *Keeper) VerifyConnectionState( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - merklePath := commitmenttypes.NewMerklePath(host.ConnectionKey(connectionID)) merklePath, err := commitmenttypes.ApplyPrefix(connection.Counterparty.Prefix, merklePath) if err != nil { @@ -129,7 +114,7 @@ func (k *Keeper) VerifyConnectionState( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, height, 0, 0, // skip delay period checks for non-packet processing verification proof, merklePath, bz, @@ -156,11 +141,6 @@ func (k *Keeper) VerifyChannelState( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - merklePath := commitmenttypes.NewMerklePath(host.ChannelKey(portID, channelID)) merklePath, err := commitmenttypes.ApplyPrefix(connection.Counterparty.Prefix, merklePath) if err != nil { @@ -172,7 +152,7 @@ func (k *Keeper) VerifyChannelState( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, height, 0, 0, // skip delay period checks for non-packet processing verification proof, merklePath, bz, @@ -200,11 +180,6 @@ func (k *Keeper) VerifyPacketCommitment( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - // get time and block delays timeDelay := connection.DelayPeriod blockDelay := k.getBlockDelay(ctx, connection) @@ -215,7 +190,7 @@ func (k *Keeper) VerifyPacketCommitment( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, height, timeDelay, blockDelay, proof, merklePath, commitmentBytes, ); err != nil { return errorsmod.Wrapf(err, "failed packet commitment verification for client (%s)", clientID) @@ -241,11 +216,6 @@ func (k *Keeper) VerifyPacketAcknowledgement( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - // get time and block delays timeDelay := connection.DelayPeriod blockDelay := k.getBlockDelay(ctx, connection) @@ -256,7 +226,7 @@ func (k *Keeper) VerifyPacketAcknowledgement( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, height, timeDelay, blockDelay, proof, merklePath, channeltypes.CommitAcknowledgement(acknowledgement), ); err != nil { @@ -283,27 +253,17 @@ func (k *Keeper) VerifyPacketReceiptAbsence( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientType, _, err := clienttypes.ParseClientIdentifier(clientID) - if err != nil { - return err - } - - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientType) - } - // get time and block delays timeDelay := connection.DelayPeriod blockDelay := k.getBlockDelay(ctx, connection) merklePath := commitmenttypes.NewMerklePath(host.PacketReceiptKey(portID, channelID, sequence)) - merklePath, err = commitmenttypes.ApplyPrefix(connection.Counterparty.Prefix, merklePath) + merklePath, err := commitmenttypes.ApplyPrefix(connection.Counterparty.Prefix, merklePath) if err != nil { return err } - if err := clientModule.VerifyNonMembership( + if err := k.clientKeeper.VerifyNonMembership( ctx, clientID, height, timeDelay, blockDelay, proof, merklePath, ); err != nil { return errorsmod.Wrapf(err, "failed packet receipt absence verification for client (%s)", clientID) @@ -328,11 +288,6 @@ func (k *Keeper) VerifyNextSequenceRecv( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - // get time and block delays timeDelay := connection.DelayPeriod blockDelay := k.getBlockDelay(ctx, connection) @@ -343,7 +298,7 @@ func (k *Keeper) VerifyNextSequenceRecv( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, height, timeDelay, blockDelay, proof, merklePath, sdk.Uint64ToBigEndian(nextSequenceRecv), @@ -369,11 +324,6 @@ func (k *Keeper) VerifyChannelUpgradeError( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - merklePath := commitmenttypes.NewMerklePath(host.ChannelUpgradeErrorKey(portID, channelID)) merklePath, err := commitmenttypes.ApplyPrefix(connection.Counterparty.Prefix, merklePath) if err != nil { @@ -385,7 +335,7 @@ func (k *Keeper) VerifyChannelUpgradeError( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, height, 0, 0, // skip delay period checks for non-packet processing verification proof, merklePath, bz, @@ -411,11 +361,6 @@ func (k *Keeper) VerifyChannelUpgrade( return errorsmod.Wrapf(clienttypes.ErrClientNotActive, "client (%s) status is %s", clientID, status) } - clientModule, found := k.clientKeeper.Route(clientID) - if !found { - return errorsmod.Wrap(clienttypes.ErrRouteNotFound, clientID) - } - merklePath := commitmenttypes.NewMerklePath(host.ChannelUpgradeKey(portID, channelID)) merklePath, err := commitmenttypes.ApplyPrefix(connection.Counterparty.Prefix, merklePath) if err != nil { @@ -427,7 +372,7 @@ func (k *Keeper) VerifyChannelUpgrade( return err } - if err := clientModule.VerifyMembership( + if err := k.clientKeeper.VerifyMembershipProof( ctx, clientID, proofHeight, 0, 0, // skip delay period checks for non-packet processing verification upgradeProof, merklePath, bz, diff --git a/modules/core/03-connection/types/expected_keepers.go b/modules/core/03-connection/types/expected_keepers.go index 4aa018aa198..f216814e24d 100644 --- a/modules/core/03-connection/types/expected_keepers.go +++ b/modules/core/03-connection/types/expected_keepers.go @@ -16,9 +16,10 @@ type ClientKeeper interface { GetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height) (exported.ConsensusState, bool) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error + VerifyMembershipProof(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path, value []byte) error + VerifyNonMembership(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path) error IterateClientStates(ctx sdk.Context, prefix []byte, cb func(string, exported.ClientState) bool) ClientStore(ctx sdk.Context, clientID string) storetypes.KVStore - Route(clientID string) (exported.LightClientModule, bool) } // ParamSubspace defines the expected Subspace interface for module parameters.