From 89e57649ddf09fee767ca7daf02b5464a5f038dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 15 Feb 2024 13:08:47 +0100 Subject: [PATCH] refactor!: remove `ExportMetadata` inteface function (#5782) * refactor: remove interface function from ClientState * imp: add note about performance * docs: fix ordering * docs: update godoc * Apply suggestions from code review Co-authored-by: Damian Nolan * test: add additional metadata keys * imp: make IterateMetadata private * docs: remove link reference * lint: remove unused arg in private func * docs: add migration doc entry * markdown lint: remove empty line * refactor: remove ExportMetadata from 08-wasm * fix: correctly skip client and consensus keys + hardening tests * fix: broken link * rm: unused IterateConsensusMetadata func * test: add code cov on panic case * lint: take a break * lint: fix lint directive --------- Co-authored-by: Damian Nolan --- .../01-developer-guide/08-genesis.md | 45 ---------- .../{09-setup.md => 08-setup.md} | 2 +- .../04-wasm/03-integration.md | 2 +- .../03-light-clients/04-wasm/07-contracts.md | 3 - docs/docs/05-migrations/13-v8-to-v9.md | 5 ++ modules/core/02-client/genesis.go | 2 + modules/core/02-client/keeper/keeper.go | 69 +++++++++----- modules/core/02-client/keeper/keeper_test.go | 33 ++++++- .../02-client/migrations/v7/solomachine.go | 5 -- modules/core/02-client/types/genesis.pb.go | 4 +- modules/core/exported/client.go | 3 - .../06-solomachine/client_state.go | 5 -- .../light-clients/07-tendermint/genesis.go | 22 ----- .../07-tendermint/genesis_test.go | 89 ------------------- modules/light-clients/07-tendermint/store.go | 37 -------- .../08-wasm/testing/mock_engine.go | 6 +- .../08-wasm/types/client_state_test.go | 1 - .../08-wasm/types/contract_api.go | 11 +-- .../light-clients/08-wasm/types/genesis.go | 30 ------- .../08-wasm/types/genesis_test.go | 88 ------------------ .../08-wasm/types/upgrade_test.go | 1 - .../09-localhost/client_state.go | 5 -- .../09-localhost/client_state_test.go | 5 -- proto/ibc/core/client/v1/genesis.proto | 4 +- 24 files changed, 94 insertions(+), 383 deletions(-) delete mode 100644 docs/docs/03-light-clients/01-developer-guide/08-genesis.md rename docs/docs/03-light-clients/01-developer-guide/{09-setup.md => 08-setup.md} (99%) delete mode 100644 modules/light-clients/07-tendermint/genesis.go delete mode 100644 modules/light-clients/07-tendermint/genesis_test.go diff --git a/docs/docs/03-light-clients/01-developer-guide/08-genesis.md b/docs/docs/03-light-clients/01-developer-guide/08-genesis.md deleted file mode 100644 index 5c7c611e729..00000000000 --- a/docs/docs/03-light-clients/01-developer-guide/08-genesis.md +++ /dev/null @@ -1,45 +0,0 @@ ---- -title: Handling Genesis -sidebar_label: Handling Genesis -sidebar_position: 8 -slug: /ibc/light-clients/genesis ---- - -# Genesis metadata - -:::note Synopsis -Learn how to implement the `ExportMetadata` interface -::: - -:::note - -## Pre-requisite readings - -- [Cosmos SDK module genesis](https://docs.cosmos.network/v0.47/building-modules/genesis) - -::: - -`ClientState` instances are provided their own isolated and namespaced client store upon initialisation. `ClientState` implementations may choose to store any amount of arbitrary metadata in order to verify counterparty consensus state and perform light client updates correctly. - -The `ExportMetadata` method of the [`ClientState` interface](https://github.com/cosmos/ibc-go/blob/v7.0.0/modules/core/exported/client.go#L47) provides light client modules with the ability to persist metadata in genesis exports. - -```go -ExportMetadata(clientStore sdk.KVStore) []GenesisMetadata -``` - -`ExportMetadata` is provided the client store and returns an array of `GenesisMetadata`. For maximum flexibility, `GenesisMetadata` is defined as a simple interface containing two distinct `Key` and `Value` accessor methods. - -```go -type GenesisMetadata interface { - // return store key that contains metadata without clientID-prefix - GetKey() []byte - // returns metadata value - GetValue() []byte -} -``` - -This allows `ClientState` instances to retrieve and export any number of key-value pairs which are maintained within the store in their raw `[]byte` form. - -When a chain is started with a `genesis.json` file which contains `ClientState` metadata (for example, when performing manual upgrades using an exported `genesis.json`) the `02-client` submodule of core IBC will handle setting the key-value pairs within their respective client stores. [See `02-client` `InitGenesis`](https://github.com/cosmos/ibc-go/blob/v7.0.0/modules/core/02-client/genesis.go#L18-L22). - -Please refer to the [Tendermint light client implementation](https://github.com/cosmos/ibc-go/blob/v7.0.0/modules/light-clients/07-tendermint/genesis.go#L12) for an example. diff --git a/docs/docs/03-light-clients/01-developer-guide/09-setup.md b/docs/docs/03-light-clients/01-developer-guide/08-setup.md similarity index 99% rename from docs/docs/03-light-clients/01-developer-guide/09-setup.md rename to docs/docs/03-light-clients/01-developer-guide/08-setup.md index 31481ea7642..3c55dc902a9 100644 --- a/docs/docs/03-light-clients/01-developer-guide/09-setup.md +++ b/docs/docs/03-light-clients/01-developer-guide/08-setup.md @@ -1,7 +1,7 @@ --- title: Setup sidebar_label: Setup -sidebar_position: 9 +sidebar_position: 8 slug: /ibc/light-clients/setup --- diff --git a/docs/docs/03-light-clients/04-wasm/03-integration.md b/docs/docs/03-light-clients/04-wasm/03-integration.md index 3827203e6cd..7cbe7929f13 100644 --- a/docs/docs/03-light-clients/04-wasm/03-integration.md +++ b/docs/docs/03-light-clients/04-wasm/03-integration.md @@ -335,7 +335,7 @@ func CreateWasmUpgradeHandler( } ``` -Or alternatively the parameter can be updated via a governance proposal (see at the bottom of section [`Creating clients`](../01-developer-guide/09-setup.md#creating-clients) for an example of how to do this). +Or alternatively the parameter can be updated via a governance proposal (see at the bottom of section [`Creating clients`](../01-developer-guide/08-setup.md#creating-clients) for an example of how to do this). ## Adding the module to the store diff --git a/docs/docs/03-light-clients/04-wasm/07-contracts.md b/docs/docs/03-light-clients/04-wasm/07-contracts.md index 9edddff1385..b73978a184d 100644 --- a/docs/docs/03-light-clients/04-wasm/07-contracts.md +++ b/docs/docs/03-light-clients/04-wasm/07-contracts.md @@ -34,7 +34,6 @@ The Wasm light client contract is expected to store the client and consensus sta ```go type QueryMsg struct { Status *StatusMsg `json:"status,omitempty"` - ExportMetadata *ExportMetadataMsg `json:"export_metadata,omitempty"` TimestampAtHeight *TimestampAtHeightMsg `json:"timestamp_at_height,omitempty"` VerifyClientMessage *VerifyClientMessageMsg `json:"verify_client_message,omitempty"` CheckForMisbehaviour *CheckForMisbehaviourMsg `json:"check_for_misbehaviour,omitempty"` @@ -45,7 +44,6 @@ type QueryMsg struct { #[cw_serde] pub enum QueryMsg { Status(StatusMsg), - ExportMetadata(ExportMetadataMsg), TimestampAtHeight(TimestampAtHeightMsg), VerifyClientMessage(VerifyClientMessageRaw), CheckForMisbehaviour(CheckForMisbehaviourMsgRaw), @@ -55,7 +53,6 @@ pub enum QueryMsg { To learn what it is expected from the Wasm light client contract when processing each message, please read the corresponding section of the [Light client developer guide](../01-developer-guide/01-overview.md): - For `StatusMsg`, see the section [`Status` method](../01-developer-guide/02-client-state.md#status-method). -- For `ExportMetadataMsg`, see the section [Genesis metadata](../01-developer-guide/08-genesis.md#genesis-metadata). - For `TimestampAtHeightMsg`, see the section [`GetTimestampAtHeight` method](../01-developer-guide/02-client-state.md#gettimestampatheight-method). - For `VerifyClientMessageMsg`, see the section [`VerifyClientMessage`](../01-developer-guide/04-updates-and-misbehaviour.md#verifyclientmessage). - For `CheckForMisbehaviourMsg`, see the section [`CheckForMisbehaviour` method](../01-developer-guide/02-client-state.md#checkformisbehaviour-method). diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index 00f6a0bde63..a169973861c 100644 --- a/docs/docs/05-migrations/13-v8-to-v9.md +++ b/docs/docs/05-migrations/13-v8-to-v9.md @@ -53,4 +53,9 @@ Please use the new functions `path.Setup`, `path.SetupClients`, `path.SetupConne ### API removals +The `ExportMetadata` interface function has been removed from the `ClientState` interface. Core IBC will export all key/value's within the 02-client store. + +### 08-wasm + +The `ExportMetadataMsg` has been removed and is no longer required for contracts to implement. Core IBC will handle exporting all key/value's written to the store by a light client contract. The `ZeroCustomFields` interface function has been removed from the `ClientState` interface. Core IBC only used this function to set tendermint client states when scheduling an IBC software upgrade. The interface function has been replaced by a type assertion. diff --git a/modules/core/02-client/genesis.go b/modules/core/02-client/genesis.go index 17b6e663364..3127d665d55 100644 --- a/modules/core/02-client/genesis.go +++ b/modules/core/02-client/genesis.go @@ -59,6 +59,8 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) { } // ExportGenesis returns the ibc client submodule's exported genesis. +// NOTE: the export process is not optimized, it will iterate three +// times over the 02-client sub-store. func ExportGenesis(ctx sdk.Context, k keeper.Keeper) types.GenesisState { genClients := k.GetAllGenesisClients(ctx) clientsMetadata, err := k.GetAllClientMetadata(ctx, genClients) diff --git a/modules/core/02-client/keeper/keeper.go b/modules/core/02-client/keeper/keeper.go index 009910adff5..2ef23ee5e58 100644 --- a/modules/core/02-client/keeper/keeper.go +++ b/modules/core/02-client/keeper/keeper.go @@ -154,6 +154,42 @@ func (k Keeper) IterateConsensusStates(ctx sdk.Context, cb func(clientID string, } } +// iterateMetadata provides an iterator over all stored metadata keys in the client store. +// For each metadata object, it will perform a callback. +func (k Keeper) iterateMetadata(ctx sdk.Context, cb func(clientID string, key, value []byte) bool) { + store := ctx.KVStore(k.storeKey) + iterator := storetypes.KVStorePrefixIterator(store, host.KeyClientStorePrefix) + + defer sdk.LogDeferred(ctx.Logger(), func() error { return iterator.Close() }) + for ; iterator.Valid(); iterator.Next() { + split := strings.Split(string(iterator.Key()), "/") + if len(split) == 3 && split[2] == string(host.KeyClientState) { + // skip client state keys + continue + } + + if len(split) == 4 && split[2] == string(host.KeyConsensusStatePrefix) { + // skip consensus state keys + continue + } + + if split[0] != string(host.KeyClientStorePrefix) { + panic(errorsmod.Wrapf(host.ErrInvalidPath, "path does not begin with client store prefix: expected %s, got %s", host.KeyClientStorePrefix, split[0])) + } + if strings.TrimSpace(split[1]) == "" { + panic(errorsmod.Wrap(host.ErrInvalidPath, "clientID is empty")) + } + + clientID := split[1] + + key := []byte(strings.Join(split[2:], "/")) + + if cb(clientID, key, iterator.Value()) { + break + } + } +} + // GetAllGenesisClients returns all the clients in state with their client ids returned as IdentifiedClientState func (k Keeper) GetAllGenesisClients(ctx sdk.Context) types.IdentifiedClientStates { var genClients types.IdentifiedClientStates @@ -169,30 +205,23 @@ func (k Keeper) GetAllGenesisClients(ctx sdk.Context) types.IdentifiedClientStat // of IdentifiedGenesisMetadata necessary for exporting and importing client metadata // into the client store. func (k Keeper) GetAllClientMetadata(ctx sdk.Context, genClients []types.IdentifiedClientState) ([]types.IdentifiedGenesisMetadata, error) { + metadataMap := make(map[string][]types.GenesisMetadata) + k.iterateMetadata(ctx, func(clientID string, key, value []byte) bool { + metadataMap[clientID] = append(metadataMap[clientID], types.NewGenesisMetadata(key, value)) + return false + }) + genMetadata := make([]types.IdentifiedGenesisMetadata, 0) for _, ic := range genClients { - cs, err := types.UnpackClientState(ic.ClientState) - if err != nil { - return nil, err + metadata := metadataMap[ic.ClientId] + if len(metadata) != 0 { + genMetadata = append(genMetadata, types.NewIdentifiedGenesisMetadata( + ic.ClientId, + metadata, + )) } - gms := cs.ExportMetadata(k.ClientStore(ctx, ic.ClientId)) - if len(gms) == 0 { - continue - } - clientMetadata := make([]types.GenesisMetadata, len(gms)) - for i, metadata := range gms { - cmd, ok := metadata.(types.GenesisMetadata) - if !ok { - return nil, errorsmod.Wrapf(types.ErrInvalidClientMetadata, "expected metadata type: %T, got: %T", - types.GenesisMetadata{}, cmd) - } - clientMetadata[i] = cmd - } - genMetadata = append(genMetadata, types.NewIdentifiedGenesisMetadata( - ic.ClientId, - clientMetadata, - )) } + return genMetadata, nil } diff --git a/modules/core/02-client/keeper/keeper_test.go b/modules/core/02-client/keeper/keeper_test.go index c8d217edc6f..c879cdd0920 100644 --- a/modules/core/02-client/keeper/keeper_test.go +++ b/modules/core/02-client/keeper/keeper_test.go @@ -23,6 +23,7 @@ import ( "github.com/cosmos/ibc-go/v8/modules/core/02-client/keeper" "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" "github.com/cosmos/ibc-go/v8/modules/core/exported" solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" @@ -252,26 +253,45 @@ func (suite KeeperTestSuite) TestGetAllGenesisClients() { //nolint:govet // this } func (suite KeeperTestSuite) TestGetAllGenesisMetadata() { //nolint:govet // this is a test, we are okay with copying locks + clientA, clientB := "07-tendermint-1", "clientB" + + // create some starting state + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), clientA, &ibctm.ClientState{}) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientA, types.NewHeight(0, 1), &ibctm.ConsensusState{}) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientA, types.NewHeight(0, 2), &ibctm.ConsensusState{}) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientA, types.NewHeight(0, 3), &ibctm.ConsensusState{}) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientA, types.NewHeight(2, 300), &ibctm.ConsensusState{}) + + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), clientB, &ibctm.ClientState{}) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientB, types.NewHeight(1, 100), &ibctm.ConsensusState{}) + suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientB, types.NewHeight(2, 300), &ibctm.ConsensusState{}) + + // NOTE: correct ordering of expected value is required + // Ordering is typically determined by the lexographic ordering of the height passed into each key. expectedGenMetadata := []types.IdentifiedGenesisMetadata{ types.NewIdentifiedGenesisMetadata( - "07-tendermint-1", + clientA, []types.GenesisMetadata{ + types.NewGenesisMetadata([]byte(fmt.Sprintf("%s/%s", host.KeyClientState, "clientMetadata")), []byte("value")), types.NewGenesisMetadata(ibctm.ProcessedTimeKey(types.NewHeight(0, 1)), []byte("foo")), types.NewGenesisMetadata(ibctm.ProcessedTimeKey(types.NewHeight(0, 2)), []byte("bar")), types.NewGenesisMetadata(ibctm.ProcessedTimeKey(types.NewHeight(0, 3)), []byte("baz")), + types.NewGenesisMetadata(ibctm.ProcessedHeightKey(types.NewHeight(2, 300)), []byte(types.NewHeight(1, 100).String())), }, ), types.NewIdentifiedGenesisMetadata( - "clientB", + clientB, []types.GenesisMetadata{ types.NewGenesisMetadata(ibctm.ProcessedTimeKey(types.NewHeight(1, 100)), []byte("val1")), + types.NewGenesisMetadata(ibctm.ProcessedHeightKey(types.NewHeight(2, 300)), []byte(types.NewHeight(1, 100).String())), types.NewGenesisMetadata(ibctm.ProcessedTimeKey(types.NewHeight(2, 300)), []byte("val2")), + types.NewGenesisMetadata([]byte("key"), []byte("value")), }, ), } genClients := []types.IdentifiedClientState{ - types.NewIdentifiedClientState("07-tendermint-1", &ibctm.ClientState{}), types.NewIdentifiedClientState("clientB", &ibctm.ClientState{}), + types.NewIdentifiedClientState(clientA, &ibctm.ClientState{}), types.NewIdentifiedClientState(clientB, &ibctm.ClientState{}), } suite.chainA.App.GetIBCKeeper().ClientKeeper.SetAllClientMetadata(suite.chainA.GetContext(), expectedGenMetadata) @@ -279,6 +299,13 @@ func (suite KeeperTestSuite) TestGetAllGenesisMetadata() { //nolint:govet // thi actualGenMetadata, err := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAllClientMetadata(suite.chainA.GetContext(), genClients) suite.Require().NoError(err, "get client metadata returned error unexpectedly") suite.Require().Equal(expectedGenMetadata, actualGenMetadata, "retrieved metadata is unexpected") + + // set invalid key in client store which will cause panic during iteration + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), "") + clientStore.Set([]byte("key"), []byte("val")) + suite.Require().Panics(func() { + suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAllClientMetadata(suite.chainA.GetContext(), genClients) //nolint:errcheck // we expect a panic + }) } func (suite KeeperTestSuite) TestGetConsensusState() { //nolint:govet // this is a test, we are okay with copying locks diff --git a/modules/core/02-client/migrations/v7/solomachine.go b/modules/core/02-client/migrations/v7/solomachine.go index bdee6ff6494..418ea1f1c8a 100644 --- a/modules/core/02-client/migrations/v7/solomachine.go +++ b/modules/core/02-client/migrations/v7/solomachine.go @@ -77,11 +77,6 @@ func (ClientState) Initialize(_ sdk.Context, _ codec.BinaryCodec, _ storetypes.K panic(errors.New("legacy solo machine is deprecated")) } -// ExportMetadata panics! -func (ClientState) ExportMetadata(_ storetypes.KVStore) []exported.GenesisMetadata { - panic(errors.New("legacy solo machine is deprecated")) -} - // CheckForMisbehaviour panics! func (ClientState) CheckForMisbehaviour(_ sdk.Context, _ codec.BinaryCodec, _ storetypes.KVStore, _ exported.ClientMessage) bool { panic(errors.New("legacy solo machine is deprecated")) diff --git a/modules/core/02-client/types/genesis.pb.go b/modules/core/02-client/types/genesis.pb.go index 96c97088173..53d6386600f 100644 --- a/modules/core/02-client/types/genesis.pb.go +++ b/modules/core/02-client/types/genesis.pb.go @@ -115,8 +115,8 @@ func (m *GenesisState) GetNextClientSequence() uint64 { return 0 } -// GenesisMetadata defines the genesis type for metadata that clients may return -// with ExportMetadata +// GenesisMetadata defines the genesis type for metadata that will be used +// to export all client store keys that are not client or consensus states. type GenesisMetadata struct { // store key of metadata without clientID-prefix Key []byte `protobuf:"bytes,1,opt,name=key,proto3" json:"key,omitempty"` diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index de109efc3eb..46a5deba852 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -55,9 +55,6 @@ type ClientState interface { // Status must return the status of the client. Only Active clients are allowed to process packets. Status(ctx sdk.Context, clientStore storetypes.KVStore, cdc codec.BinaryCodec) Status - // ExportMetadata must export metadata stored within the clientStore for genesis export - ExportMetadata(clientStore storetypes.KVStore) []GenesisMetadata - // GetTimestampAtHeight must return the timestamp for the consensus state associated with the provided height. GetTimestampAtHeight( ctx sdk.Context, diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go index 051cd81507c..88f41ae07a0 100644 --- a/modules/light-clients/06-solomachine/client_state.go +++ b/modules/light-clients/06-solomachine/client_state.go @@ -87,11 +87,6 @@ func (cs ClientState) Initialize(_ sdk.Context, cdc codec.BinaryCodec, clientSto return nil } -// ExportMetadata is a no-op since solomachine does not store any metadata in client store -func (ClientState) ExportMetadata(_ storetypes.KVStore) []exported.GenesisMetadata { - return nil -} - // VerifyUpgradeAndUpdateState returns an error since solomachine client does not support upgrades func (ClientState) VerifyUpgradeAndUpdateState( _ sdk.Context, _ codec.BinaryCodec, _ storetypes.KVStore, diff --git a/modules/light-clients/07-tendermint/genesis.go b/modules/light-clients/07-tendermint/genesis.go deleted file mode 100644 index 62e8660fef3..00000000000 --- a/modules/light-clients/07-tendermint/genesis.go +++ /dev/null @@ -1,22 +0,0 @@ -package tendermint - -import ( - storetypes "cosmossdk.io/store/types" - - clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - "github.com/cosmos/ibc-go/v8/modules/core/exported" -) - -// ExportMetadata exports all the consensus metadata in the client store so they can be included in clients genesis -// and imported by a ClientKeeper -func (ClientState) ExportMetadata(store storetypes.KVStore) []exported.GenesisMetadata { - gm := make([]exported.GenesisMetadata, 0) - IterateConsensusMetadata(store, func(key, val []byte) bool { - gm = append(gm, clienttypes.NewGenesisMetadata(key, val)) - return false - }) - if len(gm) == 0 { - return nil - } - return gm -} diff --git a/modules/light-clients/07-tendermint/genesis_test.go b/modules/light-clients/07-tendermint/genesis_test.go deleted file mode 100644 index 56344c14dac..00000000000 --- a/modules/light-clients/07-tendermint/genesis_test.go +++ /dev/null @@ -1,89 +0,0 @@ -package tendermint_test - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - - clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" - ibctesting "github.com/cosmos/ibc-go/v8/testing" -) - -// expected export ordering: -// processed height and processed time per height -// then all iteration keys -func (suite *TendermintTestSuite) TestExportMetadata() { - // test initializing client and exporting metadata - path := ibctesting.NewPath(suite.chainA, suite.chainB) - path.SetupClients() - clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) - clientState := path.EndpointA.GetClientState() - height := clientState.GetLatestHeight() - - initIteration := ibctm.GetIterationKey(clientStore, height) - suite.Require().NotEqual(0, len(initIteration)) - initProcessedTime, found := ibctm.GetProcessedTime(clientStore, height) - suite.Require().True(found) - initProcessedHeight, found := ibctm.GetProcessedHeight(clientStore, height) - suite.Require().True(found) - - gm := clientState.ExportMetadata(suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID)) - suite.Require().NotNil(gm, "client with metadata returned nil exported metadata") - suite.Require().Len(gm, 3, "exported metadata has unexpected length") - - suite.Require().Equal(ibctm.ProcessedHeightKey(height), gm[0].GetKey(), "metadata has unexpected key") - actualProcessedHeight, err := clienttypes.ParseHeight(string(gm[0].GetValue())) - suite.Require().NoError(err) - suite.Require().Equal(initProcessedHeight, actualProcessedHeight, "metadata has unexpected value") - - suite.Require().Equal(ibctm.ProcessedTimeKey(height), gm[1].GetKey(), "metadata has unexpected key") - suite.Require().Equal(initProcessedTime, sdk.BigEndianToUint64(gm[1].GetValue()), "metadata has unexpected value") - - suite.Require().Equal(ibctm.IterationKey(height), gm[2].GetKey(), "metadata has unexpected key") - suite.Require().Equal(initIteration, gm[2].GetValue(), "metadata has unexpected value") - - // test updating client and exporting metadata - err = path.EndpointA.UpdateClient() - suite.Require().NoError(err) - - clientState = path.EndpointA.GetClientState() - updateHeight := clientState.GetLatestHeight() - - iteration := ibctm.GetIterationKey(clientStore, updateHeight) - suite.Require().NotEqual(0, len(initIteration)) - processedTime, found := ibctm.GetProcessedTime(clientStore, updateHeight) - suite.Require().True(found) - processedHeight, found := ibctm.GetProcessedHeight(clientStore, updateHeight) - suite.Require().True(found) - - gm = clientState.ExportMetadata(suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID)) - suite.Require().NotNil(gm, "client with metadata returned nil exported metadata") - suite.Require().Len(gm, 6, "exported metadata has unexpected length") - - // expected ordering: - // initProcessedHeight, initProcessedTime, processedHeight, processedTime, initIteration, iteration - - // check init processed height and time - suite.Require().Equal(ibctm.ProcessedHeightKey(height), gm[0].GetKey(), "metadata has unexpected key") - actualProcessedHeight, err = clienttypes.ParseHeight(string(gm[0].GetValue())) - suite.Require().NoError(err) - suite.Require().Equal(initProcessedHeight, actualProcessedHeight, "metadata has unexpected value") - - suite.Require().Equal(ibctm.ProcessedTimeKey(height), gm[1].GetKey(), "metadata has unexpected key") - suite.Require().Equal(initProcessedTime, sdk.BigEndianToUint64(gm[1].GetValue()), "metadata has unexpected value") - - // check processed height and time after update - suite.Require().Equal(ibctm.ProcessedHeightKey(updateHeight), gm[2].GetKey(), "metadata has unexpected key") - actualProcessedHeight, err = clienttypes.ParseHeight(string(gm[2].GetValue())) - suite.Require().NoError(err) - suite.Require().Equal(processedHeight, actualProcessedHeight, "metadata has unexpected value") - - suite.Require().Equal(ibctm.ProcessedTimeKey(updateHeight), gm[3].GetKey(), "metadata has unexpected key") - suite.Require().Equal(processedTime, sdk.BigEndianToUint64(gm[3].GetValue()), "metadata has unexpected value") - - // check iteration keys - suite.Require().Equal(ibctm.IterationKey(height), gm[4].GetKey(), "metadata has unexpected key") - suite.Require().Equal(initIteration, gm[4].GetValue(), "metadata has unexpected value") - - suite.Require().Equal(ibctm.IterationKey(updateHeight), gm[5].GetKey(), "metadata has unexpected key") - suite.Require().Equal(iteration, gm[5].GetValue(), "metadata has unexpected value") -} diff --git a/modules/light-clients/07-tendermint/store.go b/modules/light-clients/07-tendermint/store.go index 1dce3bbf521..eaa256c73fc 100644 --- a/modules/light-clients/07-tendermint/store.go +++ b/modules/light-clients/07-tendermint/store.go @@ -3,7 +3,6 @@ package tendermint import ( "bytes" "encoding/binary" - "strings" "cosmossdk.io/store/prefix" storetypes "cosmossdk.io/store/types" @@ -76,42 +75,6 @@ func deleteConsensusState(clientStore storetypes.KVStore, height exported.Height clientStore.Delete(key) } -// IterateConsensusMetadata iterates through the prefix store and applies the callback. -// If the cb returns true, then iterator will close and stop. -func IterateConsensusMetadata(store storetypes.KVStore, cb func(key, val []byte) bool) { - iterator := storetypes.KVStorePrefixIterator(store, []byte(host.KeyConsensusStatePrefix)) - - // iterate over processed time and processed height - defer iterator.Close() - for ; iterator.Valid(); iterator.Next() { - keySplit := strings.Split(string(iterator.Key()), "/") - // processed time key in prefix store has format: "consensusState//processedTime" - if len(keySplit) != 3 { - // ignore all consensus state keys - continue - } - - if keySplit[2] != "processedTime" && keySplit[2] != "processedHeight" { - // only perform callback on consensus metadata - continue - } - - if cb(iterator.Key(), iterator.Value()) { - break - } - } - - // iterate over iteration keys - iter := storetypes.KVStorePrefixIterator(store, []byte(KeyIterateConsensusStatePrefix)) - - defer iter.Close() - for ; iter.Valid(); iter.Next() { - if cb(iter.Key(), iter.Value()) { - break - } - } -} - // ProcessedTimeKey returns the key under which the processed time will be stored in the client store. func ProcessedTimeKey(height exported.Height) []byte { return append(host.ConsensusStateKey(height), KeyProcessedTime...) diff --git a/modules/light-clients/08-wasm/testing/mock_engine.go b/modules/light-clients/08-wasm/testing/mock_engine.go index b8cba81b430..4754a907cfa 100644 --- a/modules/light-clients/08-wasm/testing/mock_engine.go +++ b/modules/light-clients/08-wasm/testing/mock_engine.go @@ -20,7 +20,7 @@ var ( _ ibcwasm.WasmEngine = (*MockWasmEngine)(nil) // queryTypes contains all the possible query message types. - queryTypes = [...]any{types.StatusMsg{}, types.ExportMetadataMsg{}, types.TimestampAtHeightMsg{}, types.VerifyClientMessageMsg{}, types.CheckForMisbehaviourMsg{}} + queryTypes = [...]any{types.StatusMsg{}, types.TimestampAtHeightMsg{}, types.VerifyClientMessageMsg{}, types.CheckForMisbehaviourMsg{}} // sudoTypes contains all the possible sudo message types. sudoTypes = [...]any{types.UpdateStateMsg{}, types.UpdateStateOnMisbehaviourMsg{}, types.VerifyUpgradeAndUpdateStateMsg{}, types.VerifyMembershipMsg{}, types.VerifyNonMembershipMsg{}, types.MigrateClientStoreMsg{}} @@ -229,10 +229,6 @@ func getQueryMsgPayloadTypeName(queryMsgBz []byte) string { payloadField = *payload.CheckForMisbehaviour } - if payload.ExportMetadata != nil { - payloadField = *payload.ExportMetadata - } - if payload.TimestampAtHeight != nil { payloadField = *payload.TimestampAtHeight } diff --git a/modules/light-clients/08-wasm/types/client_state_test.go b/modules/light-clients/08-wasm/types/client_state_test.go index b2da5984a31..5b7f4d94d3a 100644 --- a/modules/light-clients/08-wasm/types/client_state_test.go +++ b/modules/light-clients/08-wasm/types/client_state_test.go @@ -114,7 +114,6 @@ func (suite *TypesTestSuite) TestGetTimestampAtHeight() { suite.Require().NotNil(payload.TimestampAtHeight) suite.Require().Nil(payload.CheckForMisbehaviour) suite.Require().Nil(payload.Status) - suite.Require().Nil(payload.ExportMetadata) suite.Require().Nil(payload.VerifyClientMessage) resp, err := json.Marshal(types.TimestampAtHeightResult{Timestamp: expectedTimestamp}) diff --git a/modules/light-clients/08-wasm/types/contract_api.go b/modules/light-clients/08-wasm/types/contract_api.go index 0247951f6ed..217818d439e 100644 --- a/modules/light-clients/08-wasm/types/contract_api.go +++ b/modules/light-clients/08-wasm/types/contract_api.go @@ -18,7 +18,6 @@ type InstantiateMessage struct { // Only one field should be set at a time. type QueryMsg struct { Status *StatusMsg `json:"status,omitempty"` - ExportMetadata *ExportMetadataMsg `json:"export_metadata,omitempty"` TimestampAtHeight *TimestampAtHeightMsg `json:"timestamp_at_height,omitempty"` VerifyClientMessage *VerifyClientMessageMsg `json:"verify_client_message,omitempty"` CheckForMisbehaviour *CheckForMisbehaviourMsg `json:"check_for_misbehaviour,omitempty"` @@ -27,9 +26,6 @@ type QueryMsg struct { // StatusMsg is a queryMsg sent to the contract to query the status of the wasm client. type StatusMsg struct{} -// ExportMetadataMsg is a queryMsg sent to the contract to query the exported metadata of the wasm client. -type ExportMetadataMsg struct{} - // TimestampAtHeightMsg is a queryMsg sent to the contract to query the timestamp at a given height. type TimestampAtHeightMsg struct { Height clienttypes.Height `json:"height"` @@ -100,7 +96,7 @@ type MigrateClientStoreMsg struct{} // ContractResult is a type constraint that defines the expected results that can be returned by a contract call/query. type ContractResult interface { - EmptyResult | StatusResult | ExportMetadataResult | TimestampAtHeightResult | CheckForMisbehaviourResult | UpdateStateResult + EmptyResult | StatusResult | TimestampAtHeightResult | CheckForMisbehaviourResult | UpdateStateResult } // EmptyResult is the default return type of any contract call that does not require a custom return type. @@ -111,11 +107,6 @@ type StatusResult struct { Status string `json:"status"` } -// ExportMetadataResult is the expected return type of the exportMetadataMsg query. It returns the exported metadata of the wasm client. -type ExportMetadataResult struct { - GenesisMetadata []clienttypes.GenesisMetadata `json:"genesis_metadata"` -} - // TimestampAtHeightResult is the expected return type of the timestampAtHeightMsg query. It returns the timestamp for a light client // at a given height. type TimestampAtHeightResult struct { diff --git a/modules/light-clients/08-wasm/types/genesis.go b/modules/light-clients/08-wasm/types/genesis.go index 2b93f168623..00f2478d535 100644 --- a/modules/light-clients/08-wasm/types/genesis.go +++ b/modules/light-clients/08-wasm/types/genesis.go @@ -1,16 +1,7 @@ package types import ( - "time" - errorsmod "cosmossdk.io/errors" - storetypes "cosmossdk.io/store/types" - - sdk "github.com/cosmos/cosmos-sdk/types" - - cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" - - "github.com/cosmos/ibc-go/v8/modules/core/exported" ) // NewGenesisState creates an 08-wasm GenesisState instance. @@ -29,24 +20,3 @@ func (gs GenesisState) Validate() error { return nil } - -// ExportMetadata exports all the consensus metadata in the client store so they -// can be included in clients genesis and imported by a ClientKeeper -func (cs ClientState) ExportMetadata(store storetypes.KVStore) []exported.GenesisMetadata { - payload := QueryMsg{ - ExportMetadata: &ExportMetadataMsg{}, - } - - ctx := sdk.NewContext(nil, cmtproto.Header{Height: 1, Time: time.Now()}, true, nil) // context with infinite gas meter - result, err := wasmQuery[ExportMetadataResult](ctx, store, &cs, payload) - if err != nil { - panic(err) - } - - genesisMetadata := make([]exported.GenesisMetadata, len(result.GenesisMetadata)) - for i, metadata := range result.GenesisMetadata { - genesisMetadata[i] = metadata - } - - return genesisMetadata -} diff --git a/modules/light-clients/08-wasm/types/genesis_test.go b/modules/light-clients/08-wasm/types/genesis_test.go index 026b6a901c8..ab5c6d73af5 100644 --- a/modules/light-clients/08-wasm/types/genesis_test.go +++ b/modules/light-clients/08-wasm/types/genesis_test.go @@ -1,17 +1,7 @@ package types_test import ( - "encoding/json" - - wasmvm "github.com/CosmWasm/wasmvm" - wasmvmtypes "github.com/CosmWasm/wasmvm/types" - - errorsmod "cosmossdk.io/errors" - - wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" - clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - "github.com/cosmos/ibc-go/v8/modules/core/exported" ) func (suite *TypesTestSuite) TestValidateGenesis() { @@ -46,81 +36,3 @@ func (suite *TypesTestSuite) TestValidateGenesis() { } } } - -func (suite *TypesTestSuite) TestExportMetatada() { - mockMetadata := clienttypes.NewGenesisMetadata([]byte("key"), []byte("value")) - - testCases := []struct { - name string - malleate func() - expPanic error - expMetadata []exported.GenesisMetadata - }{ - { - "success", - func() { - suite.mockVM.RegisterQueryCallback(types.ExportMetadataMsg{}, func(_ wasmvm.Checksum, _ wasmvmtypes.Env, queryMsg []byte, _ wasmvm.KVStore, _ wasmvm.GoAPI, _ wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) ([]byte, uint64, error) { - var msg types.QueryMsg - - err := json.Unmarshal(queryMsg, &msg) - suite.Require().NoError(err) - - suite.Require().NotNil(msg.ExportMetadata) - suite.Require().Nil(msg.VerifyClientMessage) - suite.Require().Nil(msg.Status) - suite.Require().Nil(msg.CheckForMisbehaviour) - suite.Require().Nil(msg.TimestampAtHeight) - - resp, err := json.Marshal(types.ExportMetadataResult{ - GenesisMetadata: []clienttypes.GenesisMetadata{mockMetadata}, - }) - suite.Require().NoError(err) - - return resp, wasmtesting.DefaultGasUsed, nil - }) - }, - nil, - []exported.GenesisMetadata{mockMetadata}, - }, - { - "failure: contract returns an error", - func() { - suite.mockVM.RegisterQueryCallback(types.ExportMetadataMsg{}, func(_ wasmvm.Checksum, _ wasmvmtypes.Env, queryMsg []byte, _ wasmvm.KVStore, _ wasmvm.GoAPI, _ wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) ([]byte, uint64, error) { - return nil, 0, wasmtesting.ErrMockContract - }) - }, - errorsmod.Wrapf(types.ErrWasmContractCallFailed, wasmtesting.ErrMockContract.Error()), - nil, - }, - } - - for _, tc := range testCases { - tc := tc - suite.Run(tc.name, func() { - suite.SetupWasmWithMockVM() - - endpoint := wasmtesting.NewWasmEndpoint(suite.chainA) - err := endpoint.CreateClient() - suite.Require().NoError(err) - - clientState := endpoint.GetClientState() - - tc.malleate() - - store := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), endpoint.ClientID) - - var metadata []exported.GenesisMetadata - exportMetadata := func() { - metadata = clientState.ExportMetadata(store) - } - - if tc.expPanic == nil { - exportMetadata() - - suite.Require().Equal(tc.expMetadata, metadata) - } else { - suite.Require().PanicsWithError(tc.expPanic.Error(), exportMetadata) - } - }) - } -} diff --git a/modules/light-clients/08-wasm/types/upgrade_test.go b/modules/light-clients/08-wasm/types/upgrade_test.go index 7c75a90870f..243b6a6fb06 100644 --- a/modules/light-clients/08-wasm/types/upgrade_test.go +++ b/modules/light-clients/08-wasm/types/upgrade_test.go @@ -46,7 +46,6 @@ func (suite *TypesTestSuite) TestVerifyClientMessage() { suite.Require().Nil(msg.Status) suite.Require().Nil(msg.CheckForMisbehaviour) suite.Require().Nil(msg.TimestampAtHeight) - suite.Require().Nil(msg.ExportMetadata) suite.Require().Equal(env.Contract.Address, defaultWasmClientID) diff --git a/modules/light-clients/09-localhost/client_state.go b/modules/light-clients/09-localhost/client_state.go index d30cad5c34d..0d4376b7d0c 100644 --- a/modules/light-clients/09-localhost/client_state.go +++ b/modules/light-clients/09-localhost/client_state.go @@ -171,11 +171,6 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client return []exported.Height{height} } -// ExportMetadata is a no-op for the 09-localhost client. -func (ClientState) ExportMetadata(_ storetypes.KVStore) []exported.GenesisMetadata { - return nil -} - // CheckSubstituteAndUpdateState returns an error. The localhost cannot be modified by // proposals. func (ClientState) CheckSubstituteAndUpdateState(_ sdk.Context, _ codec.BinaryCodec, _, _ storetypes.KVStore, _ exported.ClientState) error { diff --git a/modules/light-clients/09-localhost/client_state_test.go b/modules/light-clients/09-localhost/client_state_test.go index c9618da5412..f7b1928bf7c 100644 --- a/modules/light-clients/09-localhost/client_state_test.go +++ b/modules/light-clients/09-localhost/client_state_test.go @@ -418,11 +418,6 @@ func (suite *LocalhostTestSuite) TestUpdateState() { suite.Require().True(heights[0].EQ(clientState.GetLatestHeight())) } -func (suite *LocalhostTestSuite) TestExportMetadata() { - clientState := localhost.NewClientState(clienttypes.NewHeight(1, 10)) - suite.Require().Nil(clientState.ExportMetadata(nil)) -} - func (suite *LocalhostTestSuite) TestCheckSubstituteAndUpdateState() { clientState := localhost.NewClientState(clienttypes.NewHeight(1, 10)) err := clientState.CheckSubstituteAndUpdateState(suite.chain.GetContext(), suite.chain.Codec, nil, nil, nil) diff --git a/proto/ibc/core/client/v1/genesis.proto b/proto/ibc/core/client/v1/genesis.proto index 43610b0d464..4abd943cdbc 100644 --- a/proto/ibc/core/client/v1/genesis.proto +++ b/proto/ibc/core/client/v1/genesis.proto @@ -25,8 +25,8 @@ message GenesisState { uint64 next_client_sequence = 6; } -// GenesisMetadata defines the genesis type for metadata that clients may return -// with ExportMetadata +// GenesisMetadata defines the genesis type for metadata that will be used +// to export all client store keys that are not client or consensus states. message GenesisMetadata { option (gogoproto.goproto_getters) = false;