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

Mismatch of client type returned by ClientState and ConsensusState in 07-celestia #6061

Closed
crodriguezvega opened this issue Mar 26, 2024 · 2 comments
Assignees
Labels
07-celestia needs discussion Issues that need discussion before they can be worked on

Comments

@crodriguezvega
Copy link
Contributor

crodriguezvega commented Mar 26, 2024

In the PR that adds a light client module for 07-celestia we have temporarily disabled this check in ValidateBasic of MsgCreateClient because for this light client we have a ClientState implementation that returns "07-celestia" when calling ClientType, but we use Tendermint's implementation of ConsensusState, whose ClientType function returns "07-tendermint".

Potential solutions discussed so far:

  • Removing the check form ValidateBasic.
  • Removing ClientType function from ConsensusState if not really needed, then we can remove the check in ValidateBasic. Light client modules should make sure that ConsensusState bytes unmarshal to the expected type.
  • Adding a customs ConsensusState type to 07-celestia that wraps Tendermint's ConsensusState (in a similar way as it is done with the ClientState type).

ref: #6053 (comment)

@crodriguezvega crodriguezvega added type: bug Something isn't working as expected needs discussion Issues that need discussion before they can be worked on 07-celestia labels Mar 26, 2024
@crodriguezvega crodriguezvega added this to the Rollkit integration milestone Mar 26, 2024
@damiannolan
Copy link
Contributor

Discussed with @AdityaSripal and grepped the codebase. Should be possible to remove ClientType() from ConsensusState and the checks in ValidateBasics.

If any incompatible consensus state is passed to the target light client module retrieve from clientState.ClientType() then unmarshalling will fail

@crodriguezvega
Copy link
Contributor Author

Closed by #6137

@github-project-automation github-project-automation bot moved this from In progress 👷 to Done 🥳 in ibc-go Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
07-celestia needs discussion Issues that need discussion before they can be worked on
Projects
Archived in project
Development

No branches or pull requests

2 participants