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

CW2: adding supported interfaces value #842

Open
eledra89 opened this issue Nov 3, 2022 · 7 comments
Open

CW2: adding supported interfaces value #842

eledra89 opened this issue Nov 3, 2022 · 7 comments

Comments

@eledra89
Copy link

eledra89 commented Nov 3, 2022

Following some of the discussion on Twitter: https://twitter.com/EledraNguyen/status/1587640345701281792

Motivation

There should be an easy way to detect what interfaces a cosmwasm contract implements (similar to EIP-165). There are several situations both on-chain and off-chain for such feature:

  • An indexer wants to categorize different types of contract.
  • A multichain application (wallet, marketplace, etc.) that already support ERC-165 from EVM chain wants to render different UX for different type of contracts.
  • A contract wants different ways to interact with other contracts depending on their types.

Implementation

We propose to add an optional map containing supported interfaces in the ContractVersion struct for these values.
The supported interface value should be a string of pre-defined format: :

For example: ["crates.io:cw721","crates.io:cw2"]

Notes

There is no way to enforce the string format or forcing the contract to have actual implementation of the declared interfaces. Thus, this variable is entirely optional for both the implementor as well as the caller.

@eledra89 eledra89 changed the title adding supported interfaces value to cw2 CW2: adding supported interfaces value Nov 3, 2022
@ethanfrey
Copy link
Member

Interesting idea.
it will have to be optional, as it will not exist for all currently deployed contracts but does seem like a nice hint.

We also discussed adding source code link, repeatable compilation Metadata, etc in a wasmd issue. This falls into that general category for me.

@uint I would love to hear you opinions here as well

@eledra89
Copy link
Author

eledra89 commented Nov 7, 2022

Yes, it is optional. We have opened a pull request on it.

For source code link, binaries, etc. I think the block explorer should be the place to submit those information (like Etherscan did). I think they are just informative data while the supported interface value is quite helpful in interacting with other program.

We have built a small utility on our explorer for general information of a wasm contract here: https://github.com/aura-nw/contract-source-code-verifier.

@uint
Copy link
Contributor

uint commented Nov 7, 2022

@uint I would love to hear you opinions here as well

If we want contracts to self-describe their interfaces and we're trying to make things modular with Sylvia, it makes total sense IMO to be able to reason which common interfaces (cw20 etc) they implement.

The cool thing is Sylvia might be able to autogenerate these in the future and it'd fit right into its model.

@eledra89
Copy link
Author

eledra89 commented Nov 8, 2022

So what is the general consensus on this ? @uint @ethanfrey

Should we modify the current cw2 interface or we make a new cw2-support-interface crate ?

@uint
Copy link
Contributor

uint commented Nov 8, 2022

Should we modify the current cw2 interface or we make a new cw2-support-interface crate ?

I worry about one scenario. Say there's an old client somewhere that doesn't know about this new supported_interface field. It goes and tries to deserialize contract_info with a deserializer that fails on unknown fields, and bam. It can't get the name/version of newer contracts because it doesn't know how to deal with some unrelated data.

I think I'd prefer if this was a separate spec with a separate storage item. @ethanfrey? is this a non-issue?

@ethanfrey
Copy link
Member

It goes and tries to deserialize contract_info with a deserializer that fails on unknown fields, and bam.

Since we don't use #[deny_unknown_fields] here afaik, I think this is safe if it is an extra Option<Vec<String>> field to give proper backwards compatibility.

We could also add another cw22 spec or something to just store this info in a different storage key.

I am fine with both solutions and pretty far from the code to see if there is a better one. I will defer to @uint on that point.

Idea: Another option is to extend cw2 to define 2 keys and helpers - get_version / set_version as well as get_interfaces / set_interfaces. Using the same spec (logical consistency) and separate storage key (ultimate backwards compatibility) may get us the best of both worlds.

@uint
Copy link
Contributor

uint commented Nov 9, 2022

Since we don't use #[deny_unknown_fields] here afaik, I think this is safe if it is an extra Option<Vec> field to give proper backwards compatibility.

Okay, in that case I'm also fine with either. Let's get this in!

@uint uint added this to the 1.0.0 milestone Nov 9, 2022
@uint uint removed this from the 1.0.0 milestone Nov 22, 2022
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

3 participants