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

Initial setup #1

Merged
merged 6 commits into from
Mar 22, 2023
Merged

Initial setup #1

merged 6 commits into from
Mar 22, 2023

Conversation

pmikolajczyk41
Copy link
Collaborator

@pmikolajczyk41 pmikolajczyk41 commented Feb 28, 2023

This PR introduces:

  1. Rust env configuration (rustfmt and rust-toolchain files)
  2. try-runtime-core library crate
  3. try-runtime-cli binary crate

try-runtime-core library crate

We expose a library that gathers all available try-runtime actions. They can be used either directly in some CLI tool or composed into another library. There is a feature "cli", which exports some clap-based command structs that can be conveniently injected to other (broader) tools or used as standalone tools.

try-runtime-cli binary crate

We expose a simple CLI tool that is suited for Polkadot.

Notes

  • There are two kinds of actions: universal (like on-runtime-upgrade or follow-chain) and chain-specific (like fast-forward). Therefore, we must enable end-users for easy and flexible tool composition - this is why we expose actions both as plain functions and as granular commands.
  • For now, only on_runtime_upgrade was migrated, so that reviewing / editing is easier and faster. The rest of commands will be migrated after this PR is merged.
  • subxt is now set for 1c5faf3 - the latest release doesn't work with recent Substrate libraries. It should be set to 0.27.2 once it is released.
  • We should return something else than sc_cli::Result, probably try-runtime-specific error enum - to be done.
  • We are still missing CI - to be done.

rust-toolchain Outdated
@@ -0,0 +1 @@
nightly-2022-11-28
Copy link
Collaborator

Choose a reason for hiding this comment

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

needed? or else too opinionated?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd avoid nightly personally if at all possible (but perhaps it's not in this case; I haven't dug into the code yet). Assuming it's needed, I think pinning to a specific one is a good approach to avoid random breakages and such (like any version it should periodically be updated though, so perhaps a more recent one is possible?)

@kianenigma
Copy link
Collaborator

Would love to have a look a this soon! thanks for the initiative :)

@kianenigma
Copy link
Collaborator

Should we make this a workspace?

@kianenigma
Copy link
Collaborator

related: paritytech/substrate#12975

use subxt::PolkadotConfig;
use try_runtime_core::commands::TryRuntime;

#[subxt::subxt(
Copy link
Collaborator

Choose a reason for hiding this comment

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

any thoughts about how this would work against arbitrary substrate-based chains? I am sure I was told subxt supports this, but don't have it off the top of my head.

Copy link
Contributor

Choose a reason for hiding this comment

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

This macro generates all of the static interface to talk to the node that the metadata comes from. Currently, as long as other nodes support the exact same interface that the metadata was generated against, it'll mostly Just Work. Subxt will take care of things like call indexes and pallet indexes being different across nodes.

subxt has validation logic which will do a runtime check when you use this interface to verify that the calls etc you use line up with the node you're trying to talk to (this validation can be switched off if you want to yolo it)

An in flight PR moves the static interface to use new encode and decode machinery (see scale-encode and scale-decode crates) which loosens the requirements on the static code the macro generates being exactly identical to the metadata, basically giving more leeway for things to keep working if the interface does change a little in certain ways (though the validation logic still will warn if things change and needs opting out of if interfaces do change in a breaking way)

So yup, it'll work cross chain as long as the pallet interfaces you use line up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will make it more universal with build.rs module that will allow for custom node address for scraping runtime configuration. In a next PR :)

Copy link
Collaborator

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Great to have a kickstart here. Happy to iterate fast, and bring over more functionality. Once feature-complete, we should delete this from substrate.

@paritytech/tools-team/@jsdw would you help with the ownership of this line of work? it is sub-xt user, and if the plans we have with @pmikolajczyk41 work out, it will be a single tool that can be used across the entire ecosystem.

use try_runtime_core::commands::TryRuntime;

#[subxt::subxt(
runtime_metadata_url = "wss://rpc.polkadot.io:443",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend bundling the metadata with the tool, so that:

  • Compiling this code doesn't require network access to download the metadata
  • Changes to the metadata don't lead to this suddenly being unable to compile (it's good to be aware of such changes though, but for that a CI step can be used; eg we could put a version of this macro that calls the URl behyind a feature flag to make it easy to run a daily check that the code compiles against latest still)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the suggestions. I will take care of CI pipelines later on. Combining this comment with the other, I think the best solution will be to have:

  • generated polkadot runtime file for default, offline compilation
  • build.rs module for customizing runtime (however it will require network access or a custom file)

cli/main.rs Outdated

type UncheckedExtrinsic = generic::UncheckedExtrinsic<
<PolkadotConfig as subxt::Config>::Address,
polkadot::runtime_types::polkadot_runtime::RuntimeCall,
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this RuntimeCall enum expect to be scale encoded (via .encode()) to send valid transactions to a node? If so, note that it will not be portable across runtimes, as the pallet indexes etc are very likely to be different in different runtimes.

(Subxt doesn't use this enum directly in its APIs and manually lines up indexes when encoding calls, and the new scale-encode (and updated scale-decode) crates help to encode things based on metadata (which in the case of variants means they will be lined up by name, not declaration order). Maybe these can help in some way, but I'm not sure how it's used yet :))

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as discussed off-GH, currently, RuntimeCall is needed for building correct Block type, which will then be needed for just processing blocks and transactions within externalities, so we should be ok with this code

@pmikolajczyk41 pmikolajczyk41 merged commit f194191 into main Mar 22, 2023
@pmikolajczyk41 pmikolajczyk41 deleted the setup branch March 22, 2023 13:28
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.

3 participants