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

Encode metadata version in the metadata itself? #941

Closed
ascjones opened this issue Sep 23, 2021 · 5 comments
Closed

Encode metadata version in the metadata itself? #941

ascjones opened this issue Sep 23, 2021 · 5 comments
Labels
A-ink_metadata [ink_metadata] Work item

Comments

@ascjones
Copy link
Collaborator

In use-ink/cargo-contract#347 I am extracting the version of the ink_metadata crate to specify the version of the metadata.

This assumes that metadata version == ink_metadata crate version, which would be a safe assumption if we are following semver for wire-format compatibility.

However, at the moment we are just bumping ink_metadata along with the other ink! crates. If that will be our policy moving forwards, then we will need to encode the version in the metadata itself, instead of using the crate version.

We could do that using an enum e.g. enum Metadata { V1(Vec<u8>), V2(V2Metadata) } like with frame_metadata, which allows supporting multiple metadata versions. Or we can just add a version field to the contract metadata struct and populate it with a constant value.

It would be good to provide some mapping of crate versions to metadata versions, possibly in the README e.g.

ink_metadata metadata version
3.0.0 V1
4.0.0 V1
5.0.0 V2
@Robbepop
Copy link
Collaborator

Robbepop commented Sep 23, 2021

An argument against using ink_metadata crate version to represent ink! metadata format version is that this would unnecessary make it much harder for us to do changes to the ink_metadata crate that do not interfere with the format but interfere with Rust specific details. The effect was that we could no longer really do that without disrespecting semver again.

So for the sake of maintainability keeping both ink_metadata crate version and ink! metadata format version separate is the only sane choice here.

@ascjones
Copy link
Collaborator Author

ascjones commented Sep 23, 2021

So do you think the aforementioned approach of an enum (same as frame-metadata),

pub enum Deprecated {}

enum Metadata { 
  V0(Deprecated), 
  V1(InkProjectV1)
}

or having a version field:

pub struct InkProject {
  version: semver::Version
  ...
}

Also the question of backwards compatibility for the existing format which doesn't include the version number. We could add a special prefix to specify that this is now the "versioned" metadata.

@jacogr
Copy link

jacogr commented Sep 23, 2021

Discussed this with @ascjones over PM. Basically on Substrate metadata we had the same thing - V0 was not an enum, then from V1 onwards it was. So we can actually use that info and the lack of the same fields inside the new to detect, so basically...

This is V0, check for existence of metadataVersion to detect this path -

{
  "metadataVersion": "0.1.0",
   ...
}

This is V1, if we cannot find the above detection field, we parse it as an enum.

{
  "V1": { ... }
}

So basically - if we find metadataVersion parse as V0, otherwise parse via the new enum.

@cmichi
Copy link
Collaborator

cmichi commented Nov 26, 2021

@ascjones I think we can close this issue?

@jacogr
Copy link

jacogr commented Nov 26, 2021

I believe so, we have the versioning enum implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ink_metadata [ink_metadata] Work item
Projects
None yet
Development

No branches or pull requests

4 participants