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

Human readable client registration for 08-wasm #5844

Open
3 tasks
colin-axner opened this issue Feb 15, 2024 · 0 comments
Open
3 tasks

Human readable client registration for 08-wasm #5844

colin-axner opened this issue Feb 15, 2024 · 0 comments
Labels
02-client 08-wasm needs discussion Issues that need discussion before they can be worked on

Comments

@colin-axner
Copy link
Contributor

Summary

This issue touches on the topic of adding a human readable sub-type to the 08-wasm module.

Current approach

The 02-client acts as a router to light client modules. The 08-wasm light client module is then a second router to light client contracts. The 02-client module uses the clientType which is set in the client identifier as the route to the light client module. The 08-wasm module uses the codehash (currently set within the client state) as the route to the contract.

If a relayer wants to interact with 07-tendermint contract. It must know the codehash for this contract. This could be determined by adding an API to contracts to return its client type, asking the contract uploader to specify the human readable client type for the codehash, or introspecting the uploaded code.

Adding a registry into 08-wasm

When registering/uploading a new contract, we could allow a human readable client type to be specified. The 08-wasm module would then add a stateful mapping from client type -> codehash (or vice versa, depending on usage)

Cons:

  • additional state storage
  • requires state import/export logic
  • requires a manual migration if it is not possible to introspect the code for the client type

Allowing relayers to pass in a human readable client type

This is where things get tricky. Currently, the codehash is embedded into the 08-wasm client state (the client type is also derived from the client state), so if you want to pass in a human readable client type to reference the 08-wasm contract, we need to add a new MsgCreateClient type which supports this.

Based on the work done in #5806, the current Initialize API for invoking a light client module to initialize a client is:

Initialize(ctx sdk.Context, clientID string, clientState, consensusState []byte) error

There are two possible ways to passing in a human readable sub client type:

  • squeezing the new argument into one of the existing arguments
  • modifying the interface to accept a third argument

I explored these possibilities with @damiannolan, here are our findings:

Putting additional information into existing arguments

The current arguments we have are:

  • clientID
  • clientState, consensusState

The existing approach is to use the client state to set the codehash. Given that we are decoupling routing/encoding, this is not the approach we want to take for passing in human readable client types.

Lets look at how things would work if we put the human readable sub client type into the client identifier.

Upon contract registration, the 08-wasm module would register a new client type on the 02-client router with the human readable sub type. Upon client creation, the relayer would pass in the human readable sub client type and this would be used to generate the client identifier. This sounds great, but there are important side effects:

  • client identifiers are not mutable
  • existing 08-wasm clients will not work with this construction (not backwards compatible)
  • increase in state storage, import/export + migration logic necessary

These difficulties lead me to ask what we are trying to achieve? We are trying to take in a new relayer provided argument and pass this information to the 08-wasm module. I believe the lack of backwards compatible + making 02-client router non statically defined makes this approach cautionable to me.

Modifying the Initialize interface

Given the above section, we see that putting the human readable sub type into the existing arguments is not straight forward. Lets explore the approach of adding this additional argument into the Initialize interface

I see a couple approaches here:

  1. add an additional argument
  2. bundle clientState, consensusState, subClientType into a struct
  3. accept a initMsg of type []byte which is the marshaled struct taking in the {clientState, consensusState, subClientType}

approach 1

Initialize(ctx sdk.Context, clientID string, clientState, consensusState []byte, subClientType string) error

Pros:

  • not much functional change

Cons:

  • design API around 08-wasm
  • requires breaking changes to handle any other light client implementation requests
  • MsgCreateClient will have: clientType and subClientType which will be confusing (maybe there is better naming available)

approach 2

Initialize(ctx sdk.Context, clientID string, initMsg types.InitMsg) error

Pros:

  • types.InitMsg can be extended without breaking API

Cons:

  • introduces a new type that light clients need to interpret
  • light clients which require a different field must ask us to extend the types.InitMsg

approach 3

Initialize(ctx sdk.Context, clientID string, initMsg []byte) error

Pros:

  • light clients can use their own custom init msg type without a change from core IBC
  • 02-client acting as pure pass through

Cons:

  • relayers will need to adopt this type
  • introduces a new type that light clients need to interpret
  • the initMsg type may differ per light client implementation (same occurs when light clients update with ClientMessage)

Conclusion

Creating a human readable sub client type that can be used for client creation is not straight forward. We can begin by adding a registry to the 08-wasm module which allows for relayers to easily query for the codehash given a human readable sub client type, but allowing relayers to pass in human readable sub client types likely needs more discussion and consideration before we can be confident about the proposed changes


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
02-client 08-wasm needs discussion Issues that need discussion before they can be worked on
Projects
Status: No status
Development

No branches or pull requests

1 participant