Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

[feature request] Add metadata dump subcommand #10418

Open
dvdplm opened this issue Dec 3, 2021 · 15 comments
Open

[feature request] Add metadata dump subcommand #10418

dvdplm opened this issue Dec 3, 2021 · 15 comments

Comments

@dvdplm
Copy link
Contributor

dvdplm commented Dec 3, 2021

For testing in subxt it would be useful to have a metadata dump subcommand that outputs the metdata to stdout.

Options:
--format/-f: accepts json or hex (default)
--strip-docs: do not include doc strings
--output/-o: output to a file at the given path

Some context

@bkchr
Copy link
Member

bkchr commented Dec 3, 2021

Substrate itself can not decode the metadata and that would also break when there is a runtime upgrade and the node wasn't upgraded yet.

@dvdplm
Copy link
Contributor Author

dvdplm commented Dec 6, 2021

I'd imagine the command would essentially do:

		self.block_or_best(block).map_err(client_err).and_then(|block| {
			self.client
				.runtime_api()
				.metadata(&BlockId::Hash(block))
				.map(Into::into)
				.map_err(|e| Error::Client(Box::new(e)))
		})

Why wouldn't that work?

@bkchr
Copy link
Member

bkchr commented Dec 6, 2021

You only want the SCALE encoded metadata? I mean that could be fine, but then you also can just call the RPC method.

@dvdplm
Copy link
Contributor Author

dvdplm commented Dec 7, 2021

You only want the SCALE encoded metadata? I mean that could be fine, but then you also can just call the RPC method.

Yep. The context here is writing integration tests for subxt and this command would just be a quicker way of getting the metadata used by the tests.
The assumption is that the startup time would be shorter and we wouldn't have to mess with any network calls or even have an rpc client in the test setup. With a metadata dump command we could ensure the right metadata is available in CI rather than using a build.rs step.

@bkchr
Copy link
Member

bkchr commented Dec 7, 2021

available in CI rather than using a build.rs step.

Can you elaborate this?

About what node do we also actually speak here? The substrate one or Polkadot or what?

@dvdplm
Copy link
Contributor Author

dvdplm commented Dec 8, 2021

Can you elaborate this?

The context is this PR where we add a test-runtime crate that queries a substrate node for its metadata and writes it to disk. At the moment it's a plain old --dev --tmp chain but it could be any type of node against whose metadata we want to run tests.

With a metadata dump command we could do the job done in test-runtime's build.rs as a single step in CI instead which is a bit cleaner. It might also be handy while developing, just a quick&easy way to get the metadata rather than running the node and then curl it for the metadata.

Is your concern here that we add too many commands of dubious usefulness?

@bkchr
Copy link
Member

bkchr commented Dec 8, 2021

I thought about this again and realized that we already have CLI commands optional. So yeah, we could probably provide such a CLI command and chain implementors then can add this if required.

@bkchr
Copy link
Member

bkchr commented Dec 8, 2021

--format/-f: accepts json or hex (default)
--strip-docs: do not include doc strings
--output/-o: output to a file at the given path

However, we could not support strip docs, because we can not decode the metadata. For format I'm also not sure what you mean with json?

@dvdplm
Copy link
Contributor Author

dvdplm commented Dec 8, 2021

However, we could not support strip docs, because we can not decode the metadata. For format I'm also not sure what you mean with json?

Yeah that won't work. I think we might want to provide a way to strip docs in some form at some point – they are only useful for dynamic UIs – but right now we can't implement that. :/

As for the format, that's a brain fart on my end. I was thinking about decoding the metadata.

@shawntabrizi
Copy link
Member

Given the nature of Metadata potentially changing block to block, this is a very strange CLI command. I hope it doesnt trick people in the ecosystem to start using this in ways it shouldnt be used.

@bkchr
Copy link
Member

bkchr commented Dec 14, 2021

Given the nature of Metadata potentially changing block to block

This is something we should prevent. I mean I have removed some of the changing things recently and we should also create a test for this to ensure that metadata doesn't change.

@shawntabrizi
Copy link
Member

I mean changing because of runtime upgrades, which could happen all the time depending on the chain.

@bkchr
Copy link
Member

bkchr commented Jan 5, 2022

I thought about this again and I think this use case should just be done with: https://github.com/chevdor/subwasm

Then you can just point it to a wasm file (instead of a running node) and let it extract metadata from there.

@jsdw
Copy link
Contributor

jsdw commented Jan 6, 2022

I thought about this again and I think this use case should just be done with: https://github.com/chevdor/subwasm

Then you can just point it to a wasm file (instead of a running node) and let it extract metadata from there.

A use case for this feature request was that, given some substrate (or compatible) binary, when testing it is useful to be able to easily acquire the metadata currently in use to generate an interface (in this case via subxt), and then run tests against that interface. This replaced a previous approach whereby we hardcoded the metadata used in the tests, which of course could cause test failures against incompatible nodes, and needed manually keeping up to date.

The way we do this currently is to start a substrate node, make the RPC call to acquire metadata and then kill it. This command would simplify that process for us, basically :)

@bkchr
Copy link
Member

bkchr commented Jan 6, 2022

Yeah I get it, but people would also need to add this cli command to their cli. This stuff isn't coming automatically. But for testing you could also just give it the wasm binary? There isn't that much difference if you provide a node or the runtime directly?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants