Versioning

[colin-axner]

With each layer we stack upon new pieces of technology we increase the complexity of our dependency graphs and the versioning requirements that go along with it.

Traditional software development might use semantic versioning, but the concept of "backwards compatibility" starts to break down once you introduce decentralized systems. It's possible a change is backwards compatible for clients interacting with a process, but the change is not backwards compatible for processes within the same cluster communicating with each other. We might call this a state machine breaking change and the lack of external clients breaking a non API breaking change.

Now that we have IBC, we introduce another layer of complexity. One can think of the versioning here like the ability for a cluster of processes to communicate with another cluster of processes. That is, chains (constructed of many nodes) communicating with other chains (constructed of many nodes). We have the IBC specification version to handle this complexity.

What isn't well defined at this layer of the stack are the individual choices an implementation may make/change which still support the IBC specification version. This is very burdensome for relayers who may be reliant on unspecified/versioned code of these implementations. Even more so, relayers are needed to support interaction between chains using uncompatible code. This is to say that a relayer may need to introduce exceptions based on the version a chain is using. This is already enormously complex when using a single implementation of ibc-go which may have breaking API/state changes between the oldest version (stargate) and the newest version. This becomes exponentially complex as new implementations of IBC are introduced

#408 introduces are outlined meaning of ibc-go semantic versioning. It allows us to follow a straightforward decision tree. This is a good starting point, but it intentionally does not encapsulate all current concerns. This is because we don't have the current insight to make a best/informed decision at the moment, but as time progresses we can iron out problem areas and increase the strength of our backwards compatibility guarantees beyond just the IBC specification version

Just as it is unsustainable for us to try to account for backwards compatibility with all of our user groups, it is unsustainable for relayers to maintain full compatibility with all connected implementations and versions of those implementations. To the best of my knowledge, the stop gap solution in hermes is to track the code version of a node it is communicating with and add custom if statements to handle new logic based on new functionality, with the idea of eventually deprecating support for the oldest versions of the code. Deprecating support for old versions is idealistic and not practical, we should not rely on this as a solution.

In order to push development further, breaking changes will need to be made eventually, but there should be a natural give/take to ensure we are only causing disruption when necessary. Ensuring that we aren't causing disruption requires being aware of what might cause disruption. This will also allow us to increase the strength of our backwards compatibility guarantees and potentially amend the meaning of our semantic versioning

As I've outlined before, I believe there are four user groups of ibc-go:

• chains
• IBC app developers
• IBC light client developers
• Relayers

Chains care about API breaking changes, state machine breaking changes, and IBC spec versioning. They are accounted for in the current semantic versioning proposal.

IBC app developers care primarily about API breaking changes (to the app module interface). They are accounted for in the current semantic versioning proposal.

IBC light client developers care primarily about API breaking changes (to the light client interface). They are accounted for in the current semantic versioning proposal

Relayers care about many aspects of our code. All the dependencies are not entirely clear to me. Relayers certainly care about the gRPCs we expose and how they function (this should be added to IBC specs). They may care about our internal handling of IBC spec recommendations. For example, the portID limit or the supported amount for ics20 packet transfers. They may want to optimistically drop sending packets which cannot succeed based on the chain they are connected to. They may care about how IBC governance related actions are implemented. They may care about which IBC applications are supported

In general I think we should outline which dependencies are absolutely necessary for relayers to successfully relay between old versions of IBC implementations and new version of IBC implementations. These should be added as requirements or at least recommendations to the IBC specification. Having a clear understanding of what changes might cause breakage in relayers will allow us to give more care when making potentially disruptive changes

Opening for discussion. What changes since the stargate release have been disruptive? What changes have been ok to support? What part of our API is most used? Is there any areas of consideration I haven't mentioned?

[adizere]

In order to push development further, breaking changes will need to be made eventually, but there should be a natural give/take to ensure we are only causing disruption when necessary.

I agree.

Many thanks for the discussion, this is very well written and already provides some clarifications!

What changes since the stargate release have been disruptive? What changes have been ok to support? What part of our API is most used? Is there any areas of consideration I haven't mentioned?

In general the releases have not been that disruptive so far. I can mention here two areas of consideration, though they are not related strictly to versioning, and would be curious to hear other's opinion:

1. Mentioned in Fix ibc-proto recompilation bug informalsystems/hermes#802. Concretely: When we generate gRPC Rust files from .proto files, we can get into a weird scenario because some cosmos-sdk .proto file can be a dependency of some ibc-go .proto file. This means that the same Rust gRPC file, namely, upgrade.proto, would be generated twice (1) directly from cosmo-sdk .proto definitions and (2) as a transitive dependency of the ibc-go .proto definitions, which leads to obtaining two (potentially different) versions of the same Rust code. We haven't noticed this problem since ~April, though.

2. Relayers need to be aware of three separate release cycles: cosmos-sdk, ibc-go, and gaiad (plus other chain binaries). I know this is not up to ibc-go, but it would improve our support for changes if the relayer teams get a clearer understanding (probably from chain binary release managers e.g., @okwme) into when each ibc-go and cosmos-sdk version is slated to land into the chain binary. There are two sub-problems here:

   1. the issue of pre-empting upcoming changes, so that the relayer is ready to support them;
      • not sure what is a good way to do this, or if there are standard ways of handling it, maybe RC announcements?
   2. the issue of understanding, for each binary, what are its dependencies, for instance reading through gaiad releases the SDK is often specified, but the ibc-go version is obscured. If a relayer does a curl cosmoshub-4:1317/node_info it can parse the SDK version from there, but I would have also expected to see the ibc-go version. Or should the relayer only care about the SDK version?

[ancazamfir]

1. Not sure I understand what "need to assume 64-limits" means. If a chanOpenInit event is issued by v1.2.0 with port-ids that have len < 64 then the handshake should "just" work. If any of the ID is > 64 then the relayer can:

• just hope for the best, send the MsgChanOpenTry, and deal with the consequences (this should fail in CheckTx I believe but then we need to extract the reason)
• check the counterparty version and if <v1.2.0 then drop the event (forever)

2. I don't think a pre-v1.2.0 will be able to deserialize the new QueryPacketAcknowledgementsRequest if packet_commitment_sequences field is present (includes something). A post-v1.2.0 will be able to process a pre-v1.2.0 QueryPacketAcknowledgementsRequest of course.

[colin-axner]

In response to the first two points:

1. I don't have too many thoughts here. In the future, our proto files/definitions should be the slowest moving part of ibc-go so hopefully issues don't arise. This seems to be more related to interacting with proto (recompiling definitions yourself), but I think it makes the point that our proto files/definitions need the most consideration when changes are being made. Ideally we move some of them to cosmos/ibc
2. This is a great point and I think a difficult one to manage. We can certainly work to create a predictable release cycle and do the initial pre-emptive testing ourselves and opening issues as they arise. We also maintain our own chain binary (simd) so this should be usable for testing out new releases (without having to upgrade gaia). Maybe it is best to only do pre-emptive testing with simd and wait for gaia to prepare a release to do testing with gaia?

If a relayer does a curl cosmoshub-4:1317/node_info it can parse the SDK version from there, but I would have also expected to see the ibc-go version. Or should the relayer only care about the SDK version?

This is an issue that we should fix. I wasn't aware that the ibc-go version wasn't in that response. The ibc-go version is arguably more useful for relayers than the SDK version. @marbar3778 do you know how we can get ibc-go in this node info response?

@ancazamfir covered the answers perfectly for your other questions. For chains which don't support > 64 portID, the tx will be dropped from the mempool, but you could end up with unfinished handshakes.

I believe deserialization will fail for pre-v1.2.0 chains because of the policy adopted by the SDK for unknown fields. It looks like it might be possible to change the default functionality. We should likely investigate this as it would help ease the burden of introducing new functionality to gRPCs (cc @damiannolan)

[tac0turtle]

This is an issue that we should fix. I wasn't aware that the ibc-go version wasn't in that response. The ibc-go version is arguably more useful for relayers than the SDK version. @marbar3778 do you know how we can get ibc-go in this node info response?

there is a way to do this now. A user can crawl https://api.cosmos.network/node_info for ibc-go. The hub and many other chains use 0.42 so once they upgrade the ibc-go version should be present. If youd like to add it directly above, we can look at adding something like this in the sdk.

[adizere]

This is an issue that we should fix. I wasn't aware that the ibc-go version wasn't in that response. The ibc-go version is arguably more useful for relayers than the SDK version. @marbar3778 do you know how we can get ibc-go in this node info response?

there is a way to do this now. A user can crawl https://api.cosmos.network/node_info for ibc-go. The hub and many other chains use 0.42 so once they upgrade the ibc-go version should be present. If youd like to add it directly above, we can look at adding something like this in the sdk.

Confirm we're now seeing the ibc-go dependency when crawling the build_deps of /node_info endpoint for a gaiad 6.0.0-rc1. We'll add a check in Hermes to infer the ibc-go version by searching for the string "github.com/cosmos/ibc-go" within the list of all build_deps. Thanks for pointing that out!

Now there is a separate question I have:

I noticed there is a new cosmos_sdk_version field specified in the VersionInfo, which didn't exist earlier (e.g., in v0.40.0). Currently Hermes ignores this field, and infers the SDK version by traversing the build_deps and greping for a hardcoded string "cosmos/cosmos-sdk". (Ref in Hermes.)

Can you confirm our current approach is not the the advised one -- and instead it would be more appropriate to look at this new field cosmos_sdk_version to find the canonical place specifying the SDK version?

Thank you!

[adizere]

Can you confirm our current approach is not the the advised one -- and instead it would be more appropriate to look at this new field cosmos_sdk_version to find the canonical place specifying the SDK version?

Just to answer here after sync-ing with Marko in person: we need not switch to cosmos_sdk_version and we can continue using "cosmos/cosmos-sdk".

[ancazamfir]

Relayers certainly care about the gRPCs we expose and how they function (this should be added to IBC specs).

A concrete example: QueryPacketAcknowledgementsRequest has a new field packet_commitment_sequences. This will go in SDK version 43.x (?). Earlier versions do not have it. The protocol version is still v1. A relayer needs to get the chain SDK version before building the request. So this change is "breaking" for the relayer.
As these types of changes pile up it will become a nightmare for the relayer to provide service for chains that span multiple releases.

Not sure what the looking forward solution is but maybe we should consider something similar to networking protocols where we could preemptively add some fields (very similar to the Any with the type-url) to be used by future extensions. Ideally these should carry some ignore/error action flag if received by an implementation that doesn't support it. So for example if the relayer would send QueryPacketAcknowledgementsRequest with non-empty packet_commitment_sequences this could be ignored by versions < 43.x (assuming packet_commitment_sequences is marked "ignored if not understood").

[colin-axner]

This is an interesting idea. This would be the same as version < 43.x not erroring on unknown fields?

The change done for QueryPacketAcknowledgementRequest seems to be a good format to follow if we can get pre v1.2.0 chains to accept the new request format. That is:

If the new field is unused: old functionality should remain unchanged
If the new field is present: functionality may adjust (assuming it still fulfills the functions purpose)

But we need to ensure the new Request type won't be rejected by older nodes, otherwise this is problematic. If the older nodes do accept the new Request type by simply ignoring the unknown fields then Hermes could update to use the new functionality without checking which ibc-go version it is talking to

[ancazamfir]

Yes, something along those lines. We want any new fields to be added as some sort of Any and ignored or error-ed by a chain if a version doesn't implement it (behavior needs to be specified in yet another flag). And of course we need to add this in the spec and it would only work after SDK implements this type of handling and all chains upgrade to at least that version.
It would mean adding some options vector to all queries and maybe all protocol messages, for future use. It is a big change and needs to be justified.