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

Create new interface version #1367

Closed
3 tasks
webmaster128 opened this issue Jul 16, 2022 · 8 comments
Closed
3 tasks

Create new interface version #1367

webmaster128 opened this issue Jul 16, 2022 · 8 comments
Assignees
Milestone

Comments

@webmaster128
Copy link
Member

In order to solve the compatibility issue discussed in #1356, we can create a new interface_version_9. 1.1 contracts than require interface_version_9 and 1.1 hosts support both interface_version_8 and interface_version_9.

  • Bump interface_version_8 to interface_version_9 in cosmwasm-std
  • Let cosmwasm-vm support both interface_version_8 and interface_version_9 (see SUPPORTED_INTERFACE_VERSIONS)
  • Update table and changelog in packages/vm/README.md

The drawback of this approach is contract developers who upgrade cosmwasm-std can only deploy to upgraded chains. But I think this is a minor short term issue and better than polluting the features list or letting contracts fail at runtime due to missing query implementations.

@ethanfrey
Copy link
Member

I strongly disagree with this.

This will cause many issues throughout the ecosystem. It is not in line with previous discussions on versioning. And it will delay the usage of the new schema in cw-plus and other contracts by a few months.

It takes quite some time for other chains to upgrade (and we need a wasmd release before then, which is unlikely to happen soon).

If the issue is the one query, let's just leave this an Osmosis specific custom query for now, not add it to CosmWasm, and then add a proper feature flag with "token factory" along with this query when we get that standardised.

I assumed we could easily add this with a feature flag. if you make the much more ecosystem-impacting version bump, I would avoid merging that new query at all. I think it is very important to get the new schema out into a CosmWasm release and cw-plus and the tooling sooner rather than later.

@uint
Copy link
Contributor

uint commented Jul 20, 2022

If we did go the "interface version bump" route, maybe it would be possible for contracts to use cosmwasm-schema 1.1 and cosmwasm-std 1.0. They then have the new schema format, export interface_version_8, and should run just fine on current blockchains. Just a thought.

@ethanfrey
Copy link
Member

That would allow the schema to get out faster, which is true.

However, I don't like the interface version bump in general. This breaks all previous discussions about 1.x versioning and such a major change should only be undertaken after discussing with all core devs. Also, given holidays, a new wasmd version is unlikely until early September, which is required here as well before chains can think about upgrades.

Basically, it causes lots of incompatibility for contract and chain devs for minor aesthetic improvements in the cosmwasm-std Cargo.toml. We can discuss such an approach in the future, but now is not the time.

@larry0x
Copy link
Contributor

larry0x commented Jul 29, 2022

If the issue is the one query, let's just leave this an Osmosis specific custom query for now, not add it to CosmWasm, and then add a proper feature flag with "token factory" along with this query when we get that standardised.

@ethanfrey just want to clarify that the new query added in #1356 is for the x/bank module. it's unrelated to tokenfactory and is not osmosis-specific.

tokenfactory was mentioned in the PR only because contracts that mint coins using the module will likely need to query the total supply for some of their logics.

@ethanfrey
Copy link
Member

Yes, I think it makes sense to make this custom osmosis query for now.

we can standardize this with official cosmwasm api for many chains later when working to standardize tokenfactory (that deserves a feature flag)

@webmaster128
Copy link
Member Author

webmaster128 commented Aug 1, 2022

Jupp, makes sense @ethanfrey. Especially the timing aspect of it.

I think what have never been discussed in details is all the different requrements for 1.x compatibility. This proposal is the best implementation I could think of right now. It ensures existing 1.0 contracts continue to run in 1.1 VMs, which is the main requirement I think. 1.1 contract will fail to upload to 1.0 chains, which I considered okay. But I agree with the issues discribed.

So what we have left is

  1. Using a new feature. This is a heavy tool and requires us to maintain it long term for a little thing. It also violates this principle of features: A good feature makes sense to be disabled. [...] When functionality is always present in the VM [...], we should not use features. They just create fragmentation in the CosmWasm ecosystem and increase the barrier for adoption.
  2. Fail at runtime instead of upload. By just adding the query in contract and chain, the contract will fail at runtime if it tries to use a query that is not yet implemented.
  3. Don't add the query now.

Maybe a good compromise is going with 2. and using a Rust feature to prevent the usage by default.

If we did go the "interface version bump" route, maybe it would be possible for contracts to use cosmwasm-schema 1.1 and cosmwasm-std 1.0.

The issue with this is that due to semver rules, the cosmwasm-std dependencies in projects will increase to 1.1 automatically in many cases (upgrades, removal of lockfiles, using cosmwasm-schema 1.1 which depends on cosmwasm-std 1.1).

@ethanfrey
Copy link
Member

I think we all agree on 3 and not adding it now. The requested posted that he would be fine with just leaving it custom on osmosis.

I thought it would be a simple add, but not worth any delay or confusion

@webmaster128 webmaster128 closed this as not planned Won't fix, can't repro, duplicate, stale Aug 1, 2022
@webmaster128
Copy link
Member Author

It would be great to find a long term solution for gradual additions. The supply query makes sense, many other queries could follow. Also things like MsgVoteWeighted support should be straight forward.

What those additions have in common is that they need to be available in wasmvm, wasmd and the chain first but are not somthing we want to allow to disable like the staking feature. We could only develop the chain side and shipt the std component months later. This has the disadvantage that we cannot test the functionality.

What about using a Rust feature preview that hides recently added functionality under a non-default Rust feature while we develop and ship the chain side? Ater a few months or even a year, this functionality can be moved out of this feature gate to become available to developers by default. If the chain still does not implement the feature, the contract fails at runtime. By using only one feature for all the little things, we keep the maintenance and communication overhead low.

ps.: we should find a better name for the VM-contract features which are very different from the Rust project features.

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

Successfully merging a pull request may close this issue.

4 participants