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

Implement IBC version negotiations in IBC java #809

Open
AntonAndell opened this issue Jan 3, 2024 · 4 comments
Open

Implement IBC version negotiations in IBC java #809

AntonAndell opened this issue Jan 3, 2024 · 4 comments
Labels
iBriz iBriz Team ICON ICON Chain related task

Comments

@AntonAndell
Copy link
Collaborator

Add logic for version negotiation to IBC Java.
Take care to not break the connection to wasm

@AntonAndell AntonAndell added ICON ICON Chain related task iBriz iBriz Team labels Jan 3, 2024
@AntonAndell
Copy link
Collaborator Author

@sabinchitrakar @izyak
How do you feel about version negotiation?
Problem as raise in audit

Description

ICON's implementation of Inter-Blockchain Communication (IBC) currently adopts a simplistic approach to version
selection for counterparty chains, differing significantly from the methodology advised in the IBC specifications. In
the standard IBC-go implementation, the chosen version is an intersection of supported versions and counterparty
versions. However, in ICON's implementation, the version is directly taken from the message without considering
an intersection, as evidenced by the code comment // We currently support no official version
in the isSupportedVersion function. This approach overlooks the nuanced and critical process of version
negotiation outlined in the IBC specifications, particularly in ICS-003 connection semantics.

Problem Scenarios

  • Lack of Version Negotiation: ICON’s approach bypasses the negotiation process, potentially leading to
    compatibility issues, especially as ICON aims to connect to IBC via ICS-08. Directly accepting versions from
    messages without an intersection check may result in operational inefficiencies or mismatches in protocol
    capabilities.
  • Siloed IBC Environment Risks: While ICON's current environment may be somewhat isolated, with only one
    version in use, this design may not be sustainable in a broader IBC ecosystem. As ICON connects with more diverse
    chains, the lack of a robust versioning strategy could pose integration and compatibility challenges.

https://github.com/cosmos/ibc/tree/main/spec/core/ics-003-connection-semantics#versioning

I have a feeling adding this might just cause us problems as we try to connect to more chains. Since we do not expect people to open random connection to ICON it might be a bad idea to add. What do you guys feel?

@ibrizsabin
Copy link
Collaborator

I think it will start to make sense once we have to create patches for different chain based on their version. But i dont think other chains will be making any patches on their end based on our versioning. The doc states Host state machine MUST utilise the version data to negotiate encodings, priorities, or connection-specific metadata related to custom logic on top of IBC. In my opinion for us its better to just reflect versionings of chain that we are connecting to since we would have made necessary changes based on their versioning.

@AntonAndell
Copy link
Collaborator Author

Yeah but we should not need to create patches for the different chains(hopefully) And if we do, since we establish the connection we can still chose the correct version we want. It would instead be a very annoying scenario if we are prevented to connect to a chain only due to a missing string

@izyak
Copy link
Collaborator

izyak commented Feb 13, 2024

Yeah, I agree.

Think for now, it should be alright, if we just check the implementation of the ibc-go before connecting to ICON. The cosmos implementation of ibc-go, as well as the composablefi fork, I believe only has one default version supported anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iBriz iBriz Team ICON ICON Chain related task
Projects
None yet
Development

No branches or pull requests

3 participants