You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
02-client had direct references to each light client, it stored/grabbed client/consensus states on their behalf. The direct references to each light client was problematic as adding a light client was not modular.
IBC launch
By IBC launch, 02-client had been refactored adding client and consensus state interfaces. The interfaces were acted upon by grabbing the client and consensus states from the store, unpacking into an any and using the client and consensus state interface functions to perform light client functionality.
Improvements enabled:
light clients could be developed in isolation from core IBC
v7
02-client was refactored once more to move setting of client and consensus states to the light client implementations. This removed many assumptions on how light clients might work as generalizing this is near impossible. All IBC light client implementations thus far have functioned in very different ways.
Improvements enabled:
generalized verify membership and non membership queries
clear interfaces for light client development + a guide
ability to pass in alternative client msg types (enabled on going work)
solomachine timeout support enabled
decoupling encoding/routing
The crux of the issues we have faced in light client development thus far is the deep coupling of how the values in the client store are encoded and how the light client modules are routed to. A poc was done to show we could decouple these two concerns. This removes the root architectural issue and gives light clients much more freedom.
Structurally speaking, the 02-client module is only strictly necessary for core IBC having a router to the light client modules based on its assigned identifier. A side benefit is that the 02-client module can also be used as a single gateway to and from the light client modules. This allows it to be an easy blocking point if it wishes to disable a light client module. It also creates a single unified rpc for external usage rather than external apps/relayers needing to comprehend where to find a light client module based on its client identifier.
The poc mentioned above was implemented with the existing API's available and continued the assumption that light clients have types which fulfill the client and consensus states which can be decoded using the codec given to the 02-client router.
This is fine for the existing light client implementations in ibc-go as their types can easily be registered on the codec. It is problematic however in order to enable wasm contracts to skip using the wasmtypes.ClientState in order to wrap their own custom types.
There appear to be two benefits for removing this extra wrapper type:
alignment of encoding types across implementations (tendermint client implemented in contract vs native 07-tendermint implementation)
easier external interaction (less wasm specific logic is required when interacting with light clients written as contracts)
There are some difficulties that arise when trying to fully decouple encoding from routing at the 08-wasm level (a secondary router after 02-client).
Removing the ability for 02-client to decode client and consensus states requires:
removing the interfaces entirely (deleting non-strictly required function such as ExportMetadata, ZeroCustomFields or moving essential functions to the light client module)
migrating genesis types to take in []byte instead of google.protobuf.Any
adding a query interface to each of the light client modules (the grpcs on 02-client must be capable of delegating to the respective light client modules)
the initialize interface function must be capable of taking in additional information required for 08-wasm (codehash or human readable sub client type)
adding v2 msg types for client creation
Issue 1
Following the merge of #5086, the following interface functions will remain on the ClientState:
typeClientStateinterface {
proto.MessageClientType() stringValidate() error// ExportMetadata must export metadata stored within the clientStore for genesis exportExportMetadata(clientStore storetypes.KVStore) []GenesisMetadata// ZeroCustomFields zeroes out any client customizable fields in client state// Ledger enforced fields are maintained while all custom fields are zero values// Used to verify upgradesZeroCustomFields() ClientState
}
ExportMetadata and ZeroCustomFields are in the process of being removed.
It should be possible to remove Validate() and ClientType() should remain and only be used by the v1 MsgCreateClient.
Issue 2
Currently the 02-client keeper handles importing and exporting state on behalf of the light clients. It asserts that the client and consensus states are of type google.protobuf.Any, but this notion needs to be removed in order to allow any client state type to be written into state.
The GenesisState would need to be replaced to take in []byte for all keys to be set in the 02-client store. It would also need to export these key/values as []byte pairs as it should not try to inspect the existing values.
This is quite a bit of work.
Issue 3
In addition to importing/exporting state, the 02-client also acts a server responding to queries. If the 02-client is no longer capable of unmarshaling client store state, it must be capable of delegating to the light client implementations.
This requires us to add a light client module querier interface for all gRPC's in the 02-client gRPC. The 02-client module would pass through to these light clients.
As it is undesirable for 08-wasm to add more interfaces to the contract_api.go, 08-wasm would likely fulfill this querier by wrapping the bytes in its substore in a 08-wasm type (for readability) or returning the bytes directly (as desired by users).
The queries that need to be implemented:
// ClientState queries an IBC light client.rpcClientState(QueryClientStateRequest) returns (QueryClientStateResponse) {
option (google.api.http).get="/ibc/core/client/v1/client_states/{client_id}";
}
// ClientStates queries all the IBC light clients of a chain.rpcClientStates(QueryClientStatesRequest) returns (QueryClientStatesResponse) {
option (google.api.http).get="/ibc/core/client/v1/client_states";
}
// ConsensusState queries a consensus state associated with a client state at// a given height.rpcConsensusState(QueryConsensusStateRequest) returns (QueryConsensusStateResponse) {
option (google.api.http).get="/ibc/core/client/v1/consensus_states/""{client_id}/revision/{revision_number}/""height/{revision_height}";
}
// ConsensusStates queries all the consensus state associated with a given// client.rpcConsensusStates(QueryConsensusStatesRequest) returns (QueryConsensusStatesResponse) {
option (google.api.http).get="/ibc/core/client/v1/consensus_states/{client_id}";
}
// ConsensusStateHeights queries the height of every consensus states associated with a given client.// TODO: this may not be requiredrpcConsensusStateHeights(QueryConsensusStateHeightsRequest) returns (QueryConsensusStateHeightsResponse) {
option (google.api.http).get="/ibc/core/client/v1/consensus_states/{client_id}/heights";
}
Issue 4
I explored this problem in-depth here, please refer to that issue for more information.
Issue 5
Assuming the above issues have been handled, then we will need a new message type for MsgCreateClient in order to take in []byte for the client and consensus states rather than a google.protobuf.Any.
The current proto definition of MsgCreateClient:
// MsgCreateClient defines a message to create an IBC clientmessageMsgCreateClient {
option(cosmos.msg.v1.signer)="signer";
option(gogoproto.goproto_getters)=false;
// light client stategoogle.protobuf.Anyclient_state=1;
// consensus state associated with the client that corresponds to a given// height.google.protobuf.Anyconsensus_state=2;
// signer addressstringsigner=3;
}
This would likely need to be updated to look something like (depending on the solution to issue #4):
// MsgCreateClient defines a message to create an IBC clientmessageMsgCreateClient {
option(cosmos.msg.v1.signer)="signer";
option(gogoproto.goproto_getters)=false;
stringclient_type=1;
bytesclient_state=2;
bytesconsensus_state=3;
stringsigner=4;
}
This msg type will be required for any light client usage which does not set its client and consensus states as a google.protobuf.Any.
Additional notes
If the connection handshake is reworked to allow the underlying chain to have a counterparty which represents it not using a google.protobuf.Any type, then we would need new fields or types for MsgConnectionOpenTry and MsgConnectionOpenAck (for client and consensus state proof validation during the connection handshake).
Improvements enabled:
08-wasm secondary routing structure removed allowing for compatibility with native client implemenations (connection handshake, upgrades were broken previously)
no opinionated encoding of client and consensus states (02-client acts as a pure passthrough router)
Conclusion
The 02-client sub-module has been difficult to define and understand. It has required many revisions, each with a whole hearted attempt to make it easier to work with. The decoupling of encoding and routing is a good architectural fix that will ease a lot of the pressure it has caused. The poc for the 02-client router also has shown that fixing this architectural issue is not too difficult and has minimal disruption to our users.
I am less enticed by the proposal to drop opinionated encoding structure used to encode client and consensus state types. I understand that this proposal introduces a benefit that also comes with a clear consequence. I do believe that at the moment it is useful that the 02-client module is able to decode the values set in its store by light clients.
I recognize that working with google.protobuf.Any's is problematic, especially externally, and that it would likely ease interaction. In addition, I think there is clear isolation between light client implementations and having to have some shared format for client and consensus states doesn't seem beneficial (outside of querying).
Given that there are several large changes required to achieve this goal, I would recommend holding off on this refactoring until there is clear agreement that dropping the loosely opinionated structuring of client and consensus states is the right choice and that it can be done in a minimally disruptive manner. It would be beneficial to continue to outline concrete improvements in the IBC workflow that would benefit from such a large structural change.
My main concerns with dropping the ability for self introspection is around queries. Does ibc-go return blobs of bytes for the client state and consensus state which it requires external users to understand? Is it possible to ask light client modules to take in a blob of bytes and return some human readable structure which could be returned back? I would feel more comfortable, if the narrative around this is solved, as I would like to avoid any major disruptions to existing relayers.
For Admin Use
Not duplicate issue
Appropriate labels applied
Appropriate contributors tagged/assigned
The text was updated successfully, but these errors were encountered:
In regard to this issue, the following context has been brought up internally with the ibc-go team. Copying here for our future reference:
Concerning the ibc-starknet project, Cairo's contract environment already offers a unique serialization method well-suited for their STARK proof generation. We're currently working on implementing ICS-02 (essentially a client router component). Use of the Any type for the client_state and consensus_state fields under the MsgCreateClient forces Protobuf decoding. This requires to add an extra layer of deserialization in Cairo, which is inefficient. Therefore, a MsgCreateClient with a client_type field and []byte type for client/consensus_state appears be offering greater compatibility across different hosting chains.
Context
For more in-depth context read 02-client refactor ADR
Pre-IBC launch
02-client had direct references to each light client, it stored/grabbed client/consensus states on their behalf. The direct references to each light client was problematic as adding a light client was not modular.
IBC launch
By IBC launch, 02-client had been refactored adding client and consensus state interfaces. The interfaces were acted upon by grabbing the client and consensus states from the store, unpacking into an any and using the client and consensus state interface functions to perform light client functionality.
Improvements enabled:
v7
02-client was refactored once more to move setting of client and consensus states to the light client implementations. This removed many assumptions on how light clients might work as generalizing this is near impossible. All IBC light client implementations thus far have functioned in very different ways.
Improvements enabled:
decoupling encoding/routing
The crux of the issues we have faced in light client development thus far is the deep coupling of how the values in the client store are encoded and how the light client modules are routed to. A poc was done to show we could decouple these two concerns. This removes the root architectural issue and gives light clients much more freedom.
Structurally speaking, the 02-client module is only strictly necessary for core IBC having a router to the light client modules based on its assigned identifier. A side benefit is that the 02-client module can also be used as a single gateway to and from the light client modules. This allows it to be an easy blocking point if it wishes to disable a light client module. It also creates a single unified rpc for external usage rather than external apps/relayers needing to comprehend where to find a light client module based on its client identifier.
Improvements enabled:
allowing unopinionated encoding structures
The poc mentioned above was implemented with the existing API's available and continued the assumption that light clients have types which fulfill the client and consensus states which can be decoded using the codec given to the 02-client router.
This is fine for the existing light client implementations in ibc-go as their types can easily be registered on the codec. It is problematic however in order to enable wasm contracts to skip using the
wasmtypes.ClientState
in order to wrap their own custom types.There appear to be two benefits for removing this extra wrapper type:
There are some difficulties that arise when trying to fully decouple encoding from routing at the 08-wasm level (a secondary router after 02-client).
Removing the ability for 02-client to decode client and consensus states requires:
ExportMetadata
,ZeroCustomFields
or moving essential functions to the light client module)[]byte
instead ofgoogle.protobuf.Any
Issue 1
Following the merge of #5086, the following interface functions will remain on the
ClientState
:ExportMetadata
andZeroCustomFields
are in the process of being removed.It should be possible to remove
Validate()
andClientType()
should remain and only be used by the v1MsgCreateClient
.Issue 2
Currently the 02-client keeper handles importing and exporting state on behalf of the light clients. It asserts that the client and consensus states are of type
google.protobuf.Any
, but this notion needs to be removed in order to allow any client state type to be written into state.The
GenesisState
would need to be replaced to take in[]byte
for all keys to be set in the 02-client store. It would also need to export these key/values as[]byte
pairs as it should not try to inspect the existing values.This is quite a bit of work.
Issue 3
In addition to importing/exporting state, the 02-client also acts a server responding to queries. If the 02-client is no longer capable of unmarshaling client store state, it must be capable of delegating to the light client implementations.
This requires us to add a light client module querier interface for all gRPC's in the 02-client gRPC. The 02-client module would pass through to these light clients.
As it is undesirable for 08-wasm to add more interfaces to the contract_api.go, 08-wasm would likely fulfill this querier by wrapping the bytes in its substore in a 08-wasm type (for readability) or returning the bytes directly (as desired by users).
The queries that need to be implemented:
Issue 4
I explored this problem in-depth here, please refer to that issue for more information.
Issue 5
Assuming the above issues have been handled, then we will need a new message type for
MsgCreateClient
in order to take in[]byte
for the client and consensus states rather than agoogle.protobuf.Any
.The current proto definition of
MsgCreateClient
:This would likely need to be updated to look something like (depending on the solution to issue #4):
This msg type will be required for any light client usage which does not set its client and consensus states as a
google.protobuf.Any
.Additional notes
If the connection handshake is reworked to allow the underlying chain to have a counterparty which represents it not using a
google.protobuf.Any
type, then we would need new fields or types forMsgConnectionOpenTry
andMsgConnectionOpenAck
(for client and consensus state proof validation during the connection handshake).Improvements enabled:
Conclusion
The 02-client sub-module has been difficult to define and understand. It has required many revisions, each with a whole hearted attempt to make it easier to work with. The decoupling of encoding and routing is a good architectural fix that will ease a lot of the pressure it has caused. The poc for the 02-client router also has shown that fixing this architectural issue is not too difficult and has minimal disruption to our users.
I am less enticed by the proposal to drop opinionated encoding structure used to encode client and consensus state types. I understand that this proposal introduces a benefit that also comes with a clear consequence. I do believe that at the moment it is useful that the 02-client module is able to decode the values set in its store by light clients.
I recognize that working with
google.protobuf.Any
's is problematic, especially externally, and that it would likely ease interaction. In addition, I think there is clear isolation between light client implementations and having to have some shared format for client and consensus states doesn't seem beneficial (outside of querying).Given that there are several large changes required to achieve this goal, I would recommend holding off on this refactoring until there is clear agreement that dropping the loosely opinionated structuring of client and consensus states is the right choice and that it can be done in a minimally disruptive manner. It would be beneficial to continue to outline concrete improvements in the IBC workflow that would benefit from such a large structural change.
My main concerns with dropping the ability for self introspection is around queries. Does ibc-go return blobs of bytes for the client state and consensus state which it requires external users to understand? Is it possible to ask light client modules to take in a blob of bytes and return some human readable structure which could be returned back? I would feel more comfortable, if the narrative around this is solved, as I would like to avoid any major disruptions to existing relayers.
For Admin Use
The text was updated successfully, but these errors were encountered: