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

Scalable version queries #136

Closed
2 of 3 tasks
Tracked by #378
colin-axner opened this issue Apr 23, 2021 · 5 comments · Fixed by #384
Closed
2 of 3 tasks
Tracked by #378

Scalable version queries #136

colin-axner opened this issue Apr 23, 2021 · 5 comments · Fixed by #384
Assignees
Milestone

Comments

@colin-axner
Copy link
Contributor

colin-axner commented Apr 23, 2021

Summary

Querying for an application version is currently unsupported (during handshakes). My initial thoughts indicate that there are two solutions:

  • modifying the module interface to return the supported versions
  • supporting gRPC routes for every application version

Both have their draw backs and (2) isn't scalable. I'd like to give this some more thought to determine if there's a way we could abstract it so that it can scale without introducing more constraints required for IBC execution


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner self-assigned this May 26, 2021
@colin-axner colin-axner added this to the 2.0.0 milestone May 28, 2021
@colin-axner colin-axner removed their assignment Aug 12, 2021
@colin-axner
Copy link
Contributor Author

colin-axner commented Aug 12, 2021

To close this issue, we should modify the application module interface by adding another function. This function should perform application version negotiation and return the version to be used in the OpenTry step.

The relayer will choose the initial version in the OpenInit step, this will likely be chosen by the user controlling the relayer or by the application which triggers the OpenInit step (interchain accounts)

The version submitted in the OpenInit step will be submitted as an argument to the function being adding to the module interface. The function should look at the version selected in OpenInit and return the matching version to be used for OpenTry. Applications may choose to implement this in however fashion they choose.

ICS20 would simply return the "ics20-1" or perhaps an error if the counterparty version is unsupported. Assuming a second version of ICS20 eventually arises, we may expect this negotiation only to return a valid version if the counterparty version is supported. So if the counterparty used "ics20-1" in OpenInit, it returns "ics20-1". The same would be true if it used "ics20-2". If the counterparty used an unsupported version, say "ics20-5", then an error should be returned

ICS27 would have similar version negotiation. Assuming ICS27 also decides to interject the host chain account address into the version, then we would expect this function to generate interchain account adddress using the counterparty port ID and append this to the existing version (or use the associated protobuf version)

This function signature may look something like:

func NegotiateAppVersion(ctx sdk.Context, portID string, counterparty channeltypes.Counterparty, counterpartyVersion string) (version string, err error) 

The name may be different (I haven't given it to much thought atm). I left off channelID in the function signature since it may not be known in advance by a relayer, but the portID should be known. I'm unsure if we should include the connection hops as well

We should assume some applications may return versions based on current state and thus access to the context may be useful

We also want to add a gRPC to 05-port which specifies an standard gRPC relayers can use which routes the request to the appropriate module callback. This way each module does not need to define their own gRPC

@seantking
Copy link
Contributor

wdyt about adding this to the interchain-accounts milestone?

@colin-axner
Copy link
Contributor Author

wdyt about adding this to the interchain-accounts milestone?

No preference, but this change should be made to the main branch

@seantking
Copy link
Contributor

@damiannolan the version string passed into OnChannelOpenTry will look like ics27-1-<interchain-account-address>

@damiannolan
Copy link
Member

I'll include the ICS27 version negotiation as part of #378 targeted for the interchain-accounts branch

faddat referenced this issue in notional-labs/ibc-go Feb 23, 2022
CosmosCar pushed a commit to caelus-labs/ibc-go that referenced this issue Nov 6, 2023
Bumps [github.com/libp2p/go-libp2p](https://github.com/libp2p/go-libp2p) from 0.15.0 to 0.15.1.
- [Release notes](https://github.com/libp2p/go-libp2p/releases)
- [Commits](libp2p/go-libp2p@v0.15.0...v0.15.1)

---
updated-dependencies:
- dependency-name: github.com/libp2p/go-libp2p
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

4 participants