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

ICS03: Connection versioning for Handshake #5079

Closed
1 task
mossid opened this issue Sep 20, 2019 · 10 comments · Fixed by #6534
Closed
1 task

ICS03: Connection versioning for Handshake #5079

mossid opened this issue Sep 20, 2019 · 10 comments · Fixed by #6534
Assignees

Comments

@mossid
Copy link
Contributor

mossid commented Sep 20, 2019

getCompatibleVersion and pickVersion are defined under Versioning in ICS03. The current implementation includes version information in the datagram but lacks version negotiation. Both functions need to be implemented as a hardcoded function and called by the handshake functions.

TODO:

  • Performance issues
@mossid mossid added the x/ibc label Sep 20, 2019
@mossid mossid changed the title ICS03: Connection versioning ICS03: Connection versioning for Handshaking Sep 20, 2019
@mossid mossid mentioned this issue Oct 1, 2019
6 tasks
@alexanderbez alexanderbez added this to the IBC Implementation & Integration milestone Oct 1, 2019
@fedekunze
Copy link
Collaborator

@mossid (cc: @cwgoes) How would each hardcoded function look like?

@jackzampolin jackzampolin changed the title ICS03: Connection versioning for Handshaking ICS03: Connection versioning for Handshake Oct 15, 2019
@fedekunze fedekunze modified the milestones: IBC Implementation & Integration, IBC Implementation Dec 10, 2019
@fedekunze fedekunze removed their assignment Jan 3, 2020
@cwgoes
Copy link
Contributor

cwgoes commented Jan 15, 2020

@mossid What is the status of this?

@fedekunze
Copy link
Collaborator

The function just needs to be updated to increase it’s performance + add tests.

@jackzampolin
Copy link
Member

Is this still applicable? Seems like a small task.

@cwgoes cwgoes assigned cwgoes and unassigned cwgoes Mar 17, 2020
@cwgoes
Copy link
Contributor

cwgoes commented Mar 20, 2020

This should be tested in conjunction with dynamic IBC, e.g. with CosmWASM, cc @ethanfrey.

@cwgoes cwgoes added the feature label Mar 20, 2020
@ethanfrey
Copy link
Contributor

Is version:
(1) version of IBC or (eg. 1.0)
(2) protocol name and version ( eg. ICS20-1)

Since this is done on a Connection level, not a Channel level, I assume it is (1). This has no effect on CosmWasm. If we have a similar discussion on the Channel level, that is very interesting. I can easily imagine for example, one contract that supports both ICS20 and some other custom protocol, and needs to know which one is intended when opening a channel.

@cwgoes
Copy link
Contributor

cwgoes commented Mar 20, 2020

Ah yes, you are completely correct - this particular issue is for connection-level versioning, but we should also ensure that our implementation of channel-level versioning is up to snuff - #5846.

@cwgoes
Copy link
Contributor

cwgoes commented Jun 16, 2020

Specifically, what we probably want is:

  • Handshake initiating chain supplies list of compatible versions
  • Handshake receiving chain chooses version from list (or set of versions / features)
  • Handshake initiation chain agrees

Then both chains agree on version string ~ featureset.

Also, we should

  • Pick a fixed version string for 1.0
  • Perhaps add channel types (ordered, unordered) for features

@colin-axner
Copy link
Contributor

should an error be returned in OpenTry if the chain decides it cannot support any of the provided versions from the counterparty list?

@cwgoes
Copy link
Contributor

cwgoes commented Jun 29, 2020

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants