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

Generate only runtime types #845

Merged
merged 13 commits into from Mar 21, 2023

Conversation

pmikolajczyk41
Copy link
Contributor

Context

In some cases we are interested only in the RuntimeCall enum (or more generally, only in some runtime types). For example:

Unfortunately, we cannot expose just a single enum, because it recursively depends on pallet-specific types as well as auxiliary Substrate types (like sp_core::crypto::*). Hence, we must generate a whole bunch of items.

PR changes

This PR introduces a new flag to subxt::subxt macro: runtime_types_only. If it is set to true, only runtime_types module will be generated. Usually, it is 3x less code than the full version. We also enrich the subxt codegen CLI command with a corresponding flag.

Tests / examples

An example has been added to showcase defining Block type outside Substrate node.

Possible improvements:

  1. We could try investigating which types are really needed for RuntimeCall and drop unused ones. However, such code pruning should be done by the compiler itself, so it might be pointless to some extent.

@jsdw
Copy link
Collaborator

jsdw commented Mar 2, 2023

  1. We could try investigating which types are really needed for RuntimeCall and drop unused ones. However, such code pruning should be done by the compiler itself, so it might be pointless to some extent.

This applies for everything else that's generated too, so I wonder how useful it is to only generate the runtime call enum in general?

There's a risk that we start adding other options to only generate specific bits and pieces that people are interested in, so I'm a bit wary of heading down this route in general (and as you found, it's a bit tricky filtering down the actual types that we generate, so you end up with a bunch of cruft too that isn't really needed still).

@pmikolajczyk41
Copy link
Contributor Author

pmikolajczyk41 commented Mar 2, 2023

@jsdw Sometimes it is useful to keep generated module as part of a crate (so that e.g. your IDE can support it). When you need only RuntimeCall, we can easily reduce code 3 times at least.

@jsdw
Copy link
Collaborator

jsdw commented Mar 2, 2023

Mmm, so this basically generates all of the runtime types but none of the interfaces for calls, storage etc. I think I could get behind that as a concept, but would probably be against much other cherry picking of what's generated for fear of making the code too fiddly to work with and a prolieration of options to opt out of various bits and pieces (at least, as the code currently stands).

@pmikolajczyk41
Copy link
Contributor Author

If I'm not mistaken this is the only reasonable subset thing to extract - all other parts potentially depend on runtime types and usually on each other. However, there are valid usecases for runtime types only. But I agree, further granulation would be bad

@jsdw
Copy link
Collaborator

jsdw commented Mar 2, 2023

If I'm not mistaken this is the only reasonable subset thing to extract - all other parts potentially depend on runtime types and usually on each other. However, there are valid usecases for runtime types only. But I agree, further granulation would be bad

Another area I have thought about before is being able to generate code only for specific pallets, with the idea that People might want to write apps that only require specific pallets and don't care about the rest (the contracts CLI tool is one example). For this to be useful, I'd also want to be able to avoid generating any types which weren't actually needed.

Comment on lines 40 to 60
type Header = generic::Header<u32, BlakeTwo256>;
type Block = generic::Block<Header, UncheckedExtrinsic>;

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
tracing_subscriber::fmt::init();

let polkadot_header = Header::new(
41,
H256::default(),
H256::default(),
H256::default(),
Digest::default(),
);

let polkadot_block = Block::new(polkadot_header, vec![]);

println!("{polkadot_block:?}");

Ok(())
}
Copy link
Collaborator

@jsdw jsdw Mar 15, 2023

Choose a reason for hiding this comment

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

This part of the example doesn't really have anything to do with the "runtime types only" flag or making use of the RuntimeCall generated type?

Maybe just some code that encodes (or even just builds) some UncheckedExtrinsic which is using the generated RuntimeCall would make more sense as an example?

Perhaps I'd also rename it to runtime_types_only.rs to point to the feature it's demo-ing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this example is my very motivation for the runtime_types_only flag :) I am using it in the alias type RuntimeCall = polkadot::runtime_types::polkadot_runtime::RuntimeCall;, which is required for specifying UncheckedExtrinsic type and then Block type. Usually, these types are created in runtime, where RuntimeCall enum is available from construct_runtime! macro. However, there are some usecases, where we are outside runtime, but still need to have Block type defined explicitly.

Nevertheless, if you prefer, I can change this example.

Copy link
Collaborator

@jsdw jsdw Mar 20, 2023

Choose a reason for hiding this comment

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

I get that; it's more just that the example should highlight the feature it's showing off in the general case I think (ie that runtime types are still available but the "interface" for building tx's etc is not). It's something we can iterate on, and so I'm not too bothered either way, but I think personally I'd want the example to:

  • make it clear that we apply the runtime_tpyes_only flag (easy to miss),
  • and then that allows us to still import runtime types,
  • but would fail if you tried using the rest of the interface).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've renamed the example and added a commented-out snippet explaining what we lose when using runtime_types_only flag

@jsdw
Copy link
Collaborator

jsdw commented Mar 15, 2023

@pmikolajczyk41 Thanks for the contribution! This looks good to me once the conflicts are resolved; just a comment on the example :)

Copy link
Contributor

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for contributing!

@jsdw jsdw merged commit c9527ab into paritytech:master Mar 21, 2023
@pmikolajczyk41 pmikolajczyk41 deleted the pmikolajczyk41/runtime-call branch April 21, 2023 08:01
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

Successfully merging this pull request may close these issues.

None yet

3 participants