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

OnChanOpenInit Implementation may not follow IBCModule docs #1693

Closed
chipshort opened this issue Nov 3, 2023 · 1 comment · Fixed by #1697
Closed

OnChanOpenInit Implementation may not follow IBCModule docs #1693

chipshort opened this issue Nov 3, 2023 · 1 comment · Fixed by #1697
Assignees
Milestone

Comments

@chipshort
Copy link
Collaborator

Maybe this is not relevant in practice, but it seems to me that our implementation of OnChanOpenInit doesn't uphold the contract specified in the doc comment of the IBCModule
:

If the version string is empty, OnChanOpenInit is expected to
return a default version string representing the version(s) it supports.
If there is no default version string for the application,
it should return an error if provided version is empty string.

In our implementation, we let the contract return a version or default to the provided one if the contract does not return a version, but we do not handle the case mentioned above, where the contract does not return a version and the provided version is empty.

@alpe
Copy link
Contributor

alpe commented Nov 8, 2023

Thanks for bringing this up! Your observation is correct that we do not error in case of the empty version. I am surprised to see this not handled by the ibc-go module themself. 🤷 I may open an issue with ibc-go repo to discuss this
After re-reading this, I think the key point here is "if there is no default version string...", which is the case when the contract returns an empty string. I will add a fix to handle this in wasmd

@alpe alpe added this to the v0.50.0 milestone Nov 8, 2023
@alpe alpe self-assigned this Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants