feat: add option to output metadata to a file #61

trusch · 2023-02-01T15:39:46Z

This adds a --output flag to the metadata subcommand. If specified, the prefixed scale-encoded metadata is written to the file as expected by for example subxt or the polkadotjs/api augmentation scripts. If the global --json flag is set, the json of the prefixed metadata is written to the file.

The normal json output is not changed to not break backward compability.

We need this because we want to expose the metadata of our chain directly from CI and don't want to spin up a local parachain setup just to query the metadata from a live node.

Ps: I really like this tool, getting the metadata directly from the wasm is super nice!

edit: Fixes #58

chevdor

Thank you for this PR @trusch, this is a great addition.
I would like to add this feature but I find the assymetry it introduces potentially confusing:

--json outputs json (ie non scale) to stdout (former behavior)
--output <file> stores the scale encoded version to
--json --output <file> stores the scale encoded as json to file

I fear this last one will be confusing and people may not expect getting the scale encoded version from the last one.

How about:

--json: for json output
--scale: for scale output, can be called with --json as well
--output <file> takes care of writting to disk any of the options above:
- --json
- --json --scale
- --scale
without --output, we use stdout

The benefit of not merging --scale and --output is that a user could also do something like scale_encoded=$(subwasm meta --scale --json) which can be useful for CI and does not require saving into a file.

trusch · 2023-02-02T09:34:52Z

Currently (in this PR) subwasm meta --output <output-file> --json <wasm-blob> would write the json encoded version of the metadata into and would not store the metadata as hex encoded scale within a json object..

But actually I like the idea of combining --json and --scale to produce something like {"result": "0xdeadbeaf..."}, thats exactly the format that polkadotjs expects.

Ok, let summarize:

subwasm meta <wasm>
- outputs json formatted RuntimeMetadata to stdout
subwasm meta --json <wasm>
- same as above
subwasm meta --scale <wasm>
- outputs hex(scale(prefixed_metadata)), i.e. 0xdeadbeaf...
subwasm meta --scale --json <wasm>
- outputs a json object with result key set to hex(scale(prefixed_metadata)), i.e. {"result": "0xdeadbeaf..."}
subwasm meta --output <file> <wasm>
- outputs json formatted RuntimeMetadata to
subwasm meta --output <file> --json <wasm>
- same as above
subwasm meta --output <file> --scale <wasm>
- outputs scale(prefixed_metadata) to
- this is the format expected by subxt
subwasm meta --output <file> --scale --json <wasm>
- outputs a json object with result key set to hex(scale(prefixed_metadata)) to , i.e. {"result": "0xdeadbeaf..."}
- this is the format expected by polkadotjs

If we agree on that I'll restructure the main to reflect that :)

chevdor · 2023-02-02T09:46:46Z

subwasm meta <wasm>

outputs json formatted RuntimeMetadata to stdout

No this is not the current behavior, currently it outputs a human friendly list of pallets and the user can dig in using an extra -m (for module)

subwasm meta --json <wasm>

same as above

Not same as above, this one shows the raw json version

subwasm meta --scale <wasm>

outputs hex(scale(prefixed_metadata)), i.e. 0xdeadbeaf...

yep, that would be great, I guess you mean to stdout here

subwasm meta --scale --json <wasm>

outputs a json object with result key set to hex(scale(prefixed_metadata)), i.e. {"result": "0xdeadbeaf..."}

fine by me

subwasm meta --output <file> <wasm>

outputs json formatted RuntimeMetadata to

We may drop --output totally I think, and we let users do subwasm meta ... > output.ext.

subwasm meta --output <file> --json <wasm>

same as above

any "same than above" is likley not required

subwasm meta --output <file> --scale <wasm>

outputs scale(prefixed_metadata) to

see above

this is the format expected by subxt

subwasm meta --output <file> --scale --json <wasm>

can we not drop --output and simply use:

subwasm meta --scale --json <wasm> > <file>

outputs a json object with result key set to hex(scale(prefixed_metadata)) to , i.e. {"result": "0xdeadbeaf..."}

this is the format expected by polkadotjs

If we have several "format" for --scale we may be better off using:
-- scale <format> with romat one of: subxt | polkadotjs (or whatever names...)

trusch · 2023-02-02T12:06:52Z

I just discovered that there are even emojis in the human readable output when specifying a module, lfg! Ok, new UX proposal:

We have a flag --format <human | json | scale | json+scale | hex+scale > that defaults to human and we always print to stdout (eventually refusing to write raw scale to a terminal). Thats the least confusing in my opinion.

Does that sound better?

chevdor · 2023-02-02T14:21:19Z

Yes that sounds very good actually 👍

I indeed would like to keep human as default. Automation can do the effort of adding flags but humans are usually lazy :)

I am not against some --output file, it was so far just not required but I see it could benefit some use case and prevent a few subwasm.... | tee <file>. If we introduce it, it should however just do what users expect: send the output (whaetver it is) to the provided file, and not make any assumption on the format.

In theory, you may want to even do stuff like --format human --output /tmp/foo.bar

Having such a new output flag, we could make it mandatory for raw scale and set a default filename such as metadata_scale_raw.bin to keep it simple for instance.

Cherry on top, for the default file, we may want to keep track of what is the appropriate extension for a given format. For instance:

human: .txt
json: .json
scale: .scale
scale/json: .jscale ?
scale/hex: .hscale ?

Those will be user overridable anyway to we help making it easy without getting in the way.

trusch · 2023-02-03T10:01:26Z

@chevdor that took a little more effort than anticipated, but besides a few minor warnings (error handling in those macros is hard) it should now do exactly what we want.

Only thing I don't know what to do is with the --json flag. Should we respect it for backward compability and translate it to "--format json"?

chevdor

I want to test carefully but that looks very good already, I also appreciate the extra cleanup !

chevdor · 2023-02-03T10:20:23Z

lib/src/subwasm.rs

-		// The following fails if piped to another command that truncates the output.
-		// Typical use case here is: subwasm meta | head
-		// The failure is due to https://github.com/rust-lang/rust/issues/46016
-		// TODO: Once the above is fixed, we can remove the dependency on calm_io
-		// println!("{}", serialized);


Did you test that part ?
I see the issue is still open.

The root cause of this issue is that we can't ignore the broken pipe error in a println! call. Since all printing is now done through writeln macro calls I just improved error propagation and added a top-level check to ignore the broken pipe error. I consequently removed the calm_io dependency.

While doing so I saw you already started using color_eyre, so I properly installed it in the beginning of main and also replaced a lot of unwraps and panics with better error propagation.

You will be happy seeing the next error. That eyre crate is really nice, I didn't used it before.

If you do RUST_LOG=subwasm=debug you can see a debug line when the broken pipe error is ignored.

chevdor · 2023-02-03T10:25:11Z

@chevdor that took a little more effort than anticipated, but besides a few minor warnings (error handling in those macros is hard) it should now do exactly what we want.

Yes, I do see the extra work, thanks a lot for that !

Only thing I don't know what to do is with the --json flag. Should we respect it for backward compability and translate it to "--format json"?

Yes I want to be extra cautious with that since parity and probably other users may be counting on that.
We could either:

deprecate --json, ie we still support it but show a fat warning mentioning to switch to --format json but keep it for now
simply keep --json as alias for --format json but... meh... :)

I think the first option is cleaner.

… of panics and unwraps

trusch · 2023-02-06T08:47:47Z

@chevdor Broken Pipe errors are now ignored again + there is now a warning about the deprecation of --json when you use it in the context of retrieving metadata. Error handling is also much better now.

Hope you like it now :)

chevdor · 2023-02-07T09:51:11Z

Nice work on that PR @trusch, thank you!

dzmitry-lahoda · 2023-05-09T14:11:55Z

tried to use output of subwasm as input to subxt. not worked.

chevdor · 2023-05-09T15:03:05Z

Can you please open a new issue and provide more details ?
I just found out that this PR introduces a regression that I am fixing now.
If this is the issue you are running into, I may have a workaround for you for now.

dzmitry-lahoda · 2023-05-09T15:53:47Z

#69

ref #61

add option to output metadata to a file in either scale or json encoding

31437e4

chevdor requested changes Feb 2, 2023

View reviewed changes

trusch added 2 commits February 3, 2023 10:54

refactor metadata printing

95165a8

cargo clippy --fix and cargo fmt

bb8e420

chevdor reviewed Feb 3, 2023

View reviewed changes

trusch added 3 commits February 6, 2023 08:49

don't panic on broken pipe

4bd9a98

update cargo lock

7319b51

improve error handling by properly installing eyre and replacing lots…

665b75b

… of panics and unwraps

trusch requested a review from chevdor February 6, 2023 08:48

chevdor and others added 6 commits February 7, 2023 09:35

Merge branch 'master' into feature-output-metadata-to-file

4a8bcbe

Merge branch 'master' into feature-output-metadata-to-file

20f0d7a

Minor fixes

88e7600

Add doc and user goodie

9cef6e0

cargo +nightly clippy --fix

8a06c2c

fix typo

ff06701

chevdor approved these changes Feb 7, 2023

View reviewed changes

Generate doc

ff30673

chevdor merged commit cfed868 into chevdor:master Feb 7, 2023

ntn-x2 mentioned this pull request Feb 9, 2023

chore: update subwasm dependency to 0.19 paritytech/srtool#60

Merged

chevdor mentioned this pull request May 10, 2023

Use RuntimeMetadataPrefixed instead of RuntimeMetadata #70

Merged

chevdor added a commit that referenced this pull request May 10, 2023

WIP: Fix regression introduced in #61

5d77a9b

chevdor mentioned this pull request May 10, 2023

WIP: Major rework and new differ #71

Merged

chevdor added a commit that referenced this pull request May 10, 2023

Fix regression related to broken pipe issues

d788745

ref #61

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add option to output metadata to a file #61

feat: add option to output metadata to a file #61

trusch commented Feb 1, 2023 •

edited

Loading

chevdor left a comment

trusch commented Feb 2, 2023

chevdor commented Feb 2, 2023

trusch commented Feb 2, 2023

chevdor commented Feb 2, 2023

trusch commented Feb 3, 2023

chevdor left a comment

chevdor Feb 3, 2023

trusch Feb 6, 2023

trusch Feb 6, 2023

chevdor commented Feb 3, 2023

trusch commented Feb 6, 2023

chevdor commented Feb 7, 2023

dzmitry-lahoda commented May 9, 2023

chevdor commented May 9, 2023

dzmitry-lahoda commented May 9, 2023

feat: add option to output metadata to a file #61

feat: add option to output metadata to a file #61

Conversation

trusch commented Feb 1, 2023 • edited Loading

chevdor left a comment

Choose a reason for hiding this comment

trusch commented Feb 2, 2023

chevdor commented Feb 2, 2023

trusch commented Feb 2, 2023

chevdor commented Feb 2, 2023

trusch commented Feb 3, 2023

chevdor left a comment

Choose a reason for hiding this comment

chevdor Feb 3, 2023

Choose a reason for hiding this comment

trusch Feb 6, 2023

Choose a reason for hiding this comment

trusch Feb 6, 2023

Choose a reason for hiding this comment

chevdor commented Feb 3, 2023

trusch commented Feb 6, 2023

chevdor commented Feb 7, 2023

dzmitry-lahoda commented May 9, 2023

chevdor commented May 9, 2023

dzmitry-lahoda commented May 9, 2023

trusch commented Feb 1, 2023 •

edited

Loading