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

chore: publish metadata as gitlab artifact #459

Merged
merged 11 commits into from
May 2, 2023
Merged

chore: publish metadata as gitlab artifact #459

merged 11 commits into from
May 2, 2023

Conversation

ggera
Copy link
Member

@ggera ggera commented Jan 26, 2023

fixes KILTProtocol/ticket#2241

Generates metadata and publishes json value to s3 bucket
Sample here https://kilt-protocol.org/peregrine/peregrine-metadata.json

Pending

@ggera ggera marked this pull request as draft January 26, 2023 15:07
@ggera ggera marked this pull request as ready for review January 27, 2023 10:47
@ggera ggera changed the title chore: [DONT MERGE] chore: publish metadata as gitlab artifact Jan 27, 2023
Copy link
Member

@ntn-x2 ntn-x2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see this is definitely good. But I cannot get the SDK to use this properly inside the augment-api package. @rflechtner do you have any insights into how we could use this directly instead of pulling the metadata from a running node?

aws configure set region eu-central-1

if [[ "$1" == "spiritnet" ]]; then
aws s3 cp /tmp/spiritnet-metadata.json s3://$S3_BUCKET/spiritnet/$2/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use the value of "$1" as part of the path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

already added $1 == spiritnet

@ntn-x2
Copy link
Member

ntn-x2 commented Jan 27, 2023

Probably scope of a different PR, but subwasm also allows to run diffs between two metadata. We can discuss what is the best strategy to execute those checks, but I would say that running it when a PR is merged into master might be a good time.

@ggera
Copy link
Member Author

ggera commented Jan 27, 2023

As far as I can see this is definitely good. But I cannot get the SDK to use this properly inside the augment-api package. @rflechtner do you have any insights into how we could use this directly instead of pulling the metadata from a running node?

This is expected metadata as json https://kilt-protocol.org/peregrine/peregrine-metadata.json

@rflechtner
Copy link
Contributor

As far as I can see this is definitely good. But I cannot get the SDK to use this properly inside the augment-api package. @rflechtner do you have any insights into how we could use this directly instead of pulling the metadata from a running node?

The augment-api script expects the result of the state_getMetadata rpc call, which as far as I can see a JSON object with a result property whose value is the scale encoded metadata in hex encoding.
I don't know what the best way to scale-encode the JSON example Gera posted is; in theory it should work with

import { Metadata, TypeRegistry} from '@polkadot/types';
reg = new TypeRegistry()
meta = new Metadata(reg, json)
meta.toHex()

Doesn't really work for me though rn, but that does seem to do the trick:

import { TypeRegistry } from '@polkadot/types';
meta = new TypeRegistry().createType('MetadataV14', json.V14)
meta.toHex()

@rflechtner
Copy link
Contributor

rflechtner commented Jan 30, 2023

I assume subwasm also has a binary export format for the metadata if you're passing the --json flag? That could be just what we need. Should we try uploading that?
You could also try updating subwasm; something's definitely off with the JSON export. The interface for the V14 data model for the metadata looks as follows:

export interface MetadataV14 extends Struct {
    readonly lookup: PortableRegistry;
    readonly pallets: Vec<PalletMetadataV14>;
    readonly extrinsic: ExtrinsicMetadataV14;
    readonly type: SiLookupTypeId;
}

But in the JSON file we've got the keys types, pallets, extrinsics, and ty instead.

@ntn-x2
Copy link
Member

ntn-x2 commented Jan 31, 2023

I have been playing on the rust server with the latest version of subwasm, and I also get a JSON with a single V14 key with properties types, pallets, extrinsic, and ty. I think the difference could be in how the RPC nodes returns this information. There's also such a thing as a magicNumber, which I guess the script expects when querying from the RPC node state_getMetadata. We should bypass this and simply parse a metadata document somehow. I will dedicate a bit more time to that this morning, and see if I make any progress on that front.

@ntn-x2
Copy link
Member

ntn-x2 commented Jan 31, 2023

@ntn-x2
Copy link
Member

ntn-x2 commented Jan 31, 2023

Another related StackExchange issue: polkadot-js/api#4293

Copy link
Contributor

@weichweich weichweich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might be enough if we have them as an artifact. Not sure if we need to upload them to s3. Gitlab also stores them for a while.

@trusch
Copy link
Contributor

trusch commented Feb 9, 2023

https://github.com/chevdor/subwasm/releases/tag/v0.19.0 contains the needed functionality to output all kinds of metadata formats.

Especially there is --format=json+scale which produces the metadata as polkadotjs expects it and --format=scale which outputs the format that subxt needs.

@ntn-x2
Copy link
Member

ntn-x2 commented Feb 9, 2023

@trusch so as a quick recap, does --format=json+scale produce a file with the metadata? That we can serve directly as an artifact? Can we output multiple things at the same time? Can we also easily check what changed between two metadata versions?

@trusch
Copy link
Contributor

trusch commented Feb 9, 2023

@ntn-x2 the format flag just specifies the format, but there is now also a --output flag where you can specify a file.

It's not possible to output two different formats at the same time, but running the command is fast enough to do it two times.

The diff subcommand was not changed since the last release.

@ntn-x2
Copy link
Member

ntn-x2 commented Feb 9, 2023

@trusch thanks for the clarifications. I would say that we can update this PR to export the new format so that the SDK can use it. We will tackle metadata diffs in a separate ticket.

.gitlab-ci.yml Outdated
image: amazon/aws-cli:2.9.18
script:
- aws configure set region $AWS_DEFAULT_REGION
- aws s3 cp /out/peregrine-metadata.json s3://$S3_BUCKET/peregrine/${CI_COMMIT_TAG}/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ggera as far as I know, CI_COMMIT_TAG could be empty if the job was simply triggered by a commit push. Would it make sense to have the same logic we have for Docker image tagging? Or maybe push something like <branch-commit> if a regular push and <tag> if a tag.

@ntn-x2
Copy link
Member

ntn-x2 commented Feb 9, 2023

I pushed a new change with the new format for subwasm, but we still need to wait for a new SRtool release that includes the new subwasm 0.19.

@ntn-x2
Copy link
Member

ntn-x2 commented Feb 9, 2023

Tracking PR: paritytech/srtool#60

@ntn-x2 ntn-x2 added the ✋on hold status: on hold label Feb 17, 2023
@ggera
Copy link
Member Author

ggera commented Apr 27, 2023

it might be enough if we have them as an artifact. Not sure if we need to upload them to s3. Gitlab also stores them for a while.

Removed the s3 upload and updated srtool since new image is pushed @ntn-x2 @trusch

@ntn-x2
Copy link
Member

ntn-x2 commented Apr 27, 2023

The artifacts only include *.wasm outputs. Should also the *.json be added as artifacts? So that we have a URL to pull the metadata from.

Copy link
Member

@ntn-x2 ntn-x2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good enough! Let's give it a try!

@ntn-x2 ntn-x2 merged commit 8575a87 into develop May 2, 2023
@ntn-x2 ntn-x2 deleted the subwasm branch May 2, 2023 09:31
ggera added a commit that referenced this pull request May 3, 2023
ggera added a commit that referenced this pull request May 3, 2023
Reverts #459
Reverts until stable toolchain supporter
webguru9178 pushed a commit to webguru9178/kilt-node that referenced this pull request Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✋on hold status: on hold
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants