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

Cosmos SDK 0.44 support #865

Closed
ethanfrey opened this issue Aug 3, 2021 · 15 comments
Closed

Cosmos SDK 0.44 support #865

ethanfrey opened this issue Aug 3, 2021 · 15 comments

Comments

@ethanfrey
Copy link
Contributor

ethanfrey commented Aug 3, 2021

CosmJS 0.24, 0.25 and 0.26 all plan to support Cosmos SDK 0.40-0.42 under the name "Stargate".

Cosmos SDK 0.43 introduced some breaking changes in protobuf types, such that a client lib may only support either 0.42 or 0.43, but not both with the same code. It would require either making a new post-stargate package (which will become quite heavy to maintain, especially when 0.44 breaks it again), or making Stargate only support 0.43 and deprecating 0.42 support.

The most important chain to support is the Cosmos Hub. And ideally the majority of other mainnets. The question is when to enable 0.43 support and how. This issue is here to collect discussions on that point, as I know other projects have questions on this.

@ethanfrey
Copy link
Contributor Author

I believe one metric could be "when Cosmos Hub has a Gaia version (and pending proposal) to upgrade to a new SDK version". Maybe it will jump right to 0.44 and never use 0.43 and we don't need to support 0.43 at all. The only SDK versions to receive LTS seem to be those deployed to the Cosmos Hub.

Another possibility would be that the SDK Core team make some changes such that the 0.42 types can be used in 0.43 without breaking anything. We are happy to add some optional fields (especially in queries) and extra message types that only work in 0.43 (such as fee grants), but the message types that work in both should be easy to support with one code-base (same protocol types).

@amaury1093
Copy link

amaury1093 commented Aug 3, 2021

Another possibility would be that the SDK Core team make some changes such that the 0.42 types can be used in 0.43 without breaking anything.

We did introduce some breaking changes in the dev phase of 0.43, but noticed they are a headache for client libs, so reverted them before the RCs. A summary can be found in this draft ADR.

AFAIK there are no more breaking changes in Msgs between 0.42 and 0.43, but lmk if you found some, and we should probably revert them.

@webmaster128
Copy link
Member

webmaster128 commented Aug 3, 2021

In confio/cosmjs-types#4 I regenerated the types. The diff show what is overridden when 0.43 types are generated over 0.42 types. It looks much better now. I marked one place where I think there is a breaking change. For the majority of changes it is just unclear what behaviour to expect for which backend and what queries are available where. But overall, this is probably good enough.

@robert-zaremba
Copy link

I responded in the comment there - I agree that this is a proto breaking change, but most likely the response is always empty. I'm checking this with others.

@webmaster128
Copy link
Member

Okay, now my main question is: should we just continue to use Cosmos SDK 0.42 types for both backends to ensure only common functionality can be accessed?

@amaury1093
Copy link

Okay, now my main question is: should we just continue to use Cosmos SDK 0.42 types for both backends to ensure only common functionality can be accessed?

I personally think the proto breaking change should not be there, and we should revert it. I created a draft PR in the SDK which should be merged in the RCs: cosmos/cosmos-sdk#9847

@webmaster128
Copy link
Member

I'm not talking about the change addressed in cosmos/cosmos-sdk#9847 but all the additions like the added Accounts query. If I use the 0.43 types it is very unclear which of the functionality of cosmos/auth/v1beta1/query.ts is vailable in 0.42. Same for the added fields.

@amaury1093
Copy link

Ah I see. I don't have a good answer for this.

My assumption was that CosmJS could be agnostic of backend, and the users of CosmJS should know their backend and which relevant RPCs and fields are accessible. But maybe that's risky.

If you have any ideas how to improve that, we're happy to discuss. Just throwing some quick ideas :

  • maybe add a proto option in proto files to denote from when new fields/rpcs are added: rpc NewMethod(InputArgs) ResponseType {option (cosmos.since_version) = "0.43"}
  • or maybe for now add proto comments "Available from 0.43", which will be displayed in IDE intellisense

@ethanfrey
Copy link
Contributor Author

ethanfrey commented Aug 4, 2021

Comments that make the path from proto to cosmjs types to wrapper methods would be a good addition.

Open for other ideas as well

@webmaster128 webmaster128 changed the title Cosmos SDK 0.43 support Cosmos SDK 0.44 support Oct 11, 2021
@webmaster128
Copy link
Member

I can see a path forward as follows:

  • Make cosmjs-types version 1 with the types compatible to Cosmos SDK 0.42
  • Make cosmjs-types version 2 with the types compatible to Cosmos SDK 0.44
  • Make cosmjs-types version 3 with the types compatible to Cosmos SDK 0.45

Then a caller project needs to decide if they want cosmjs-types ^1, ^2 or ^3. Here in CosmJS we create different client packages for a 0.42 client (@cosmjs/stargate), a 0.44 client (which name??), a 0.45 client (which name??).

@ethanfrey
Copy link
Contributor Author

Here in CosmJS we create different client packages for a 0.42 client (@cosmjs/stargate), a 0.44 client (which name??), a 0.45 client (which name??).

This seems like a fair plan.

I have been pinging the sdk team for codenames to distinguish eg. the 0.40, 0.41, 0.42 release series (Stargate) from the 0.43, 0.44 release series. Numbers keep changing, codenames remain. However, they haven't given any suggestions after a few months.

If they don't I guess we get to decide on the (un)official names?

@webmaster128
Copy link
Member

In #909 I'm connecting to a 0.44 backend with the current @cosmjs/stargate now – not updating the types and only using what is available in 0.42. This works surpridingly well.

Biggest issues so far:

  • All light-client verified queries rely on an exact database storage key. This is not stable across SDK versions.
  • simapp from Cosmos SDK has no IBC support anymore (Remove IBC from the SDK cosmos-sdk#8735)

But overall it should be safe to claim the current client works with 0.44.

@ethanfrey
Copy link
Contributor Author

All light-client verified queries rely on an exact database storage key. This is not stable across SDK versions.

That is an interesting issue.
Maybe we just give up on that idea for everything besides ibc (where the keys are well-defined and must be consistent over all versions)

@webmaster128
Copy link
Member

Maybe we just give up on that idea for everything besides ibc (where the keys are well-defined and must be consistent over all versions)

Yes, will remove them. This is clearly not maintainable:
Bildschirmfoto 2021-10-20 um 22 55 47
Also it can take up to 4 blocks to get a response. This type of queries need to be implemented in an SDK-specific client.

@webmaster128
Copy link
Member

Closing this as mostly done. CosmJS is tested againat simapp 0.42 and 0.44 backends in CI. With CosmJS 0.27 we use cosmjs-types 0.3 which include Cosmos SDK 0.44 types.

Support for new features in transaction signing or query support should be tracked in more specific tickets.

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

No branches or pull requests

4 participants