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

try-runtime - specifying runtime & cutting off unnecessary configuration burden #12156

@pmikolajczyk41

Description

@pmikolajczyk41

I'm trying to change the way how local runtime is passed. Currently, we must pass path to a chainspec, build genesis storage and retrieve :CODE: from there. Not only it is the second time we build the storage (the first time is in create_runner function in the client code), but we also do it completetly unnecessary.

I would like to propose two things:

  1. we add a new shared parameter for all try-runtime subcommands: enum Runtime, which has three variants: Remote | Native | Local(Pathbuf) with natural meaning:
    - Remote: we use Wasm execution with the runtime included in the scraped storage (and we do not have to overwrite anything)
    - Native: we use Native execution with the code from the binary that is actually running (in case of spec-* mismatch we can panic)
    - Local(Pathbuf): we use Wasm execution with the runtime read from local file at Pathbuf (which requires overwriting :CODE: in the scraped storage)

Of course, not every variant makes sense with every subcommand, but having a common way of specifying runtime + variant checks seems to be more friendly and straightforward than implicit, non-consistent approach.

  1. we completely get rid of using sc_service::Configuration and sc_cli::SharedParams in try-runtime - if I understand correctly, the only essential information that we use from it are: default_heap_pages, max_runtime_instances and runtime_cache_size. We could 'move' these three arguments to SharedParams and stop using both concepts at all. However, the problem is not gone: TryRuntimeCmd will be still launched using sc::cli::Runner, which requires sc_service::Configuration object. If I'm not mistaken, try-runtime is a command that needs nothing more than an async environment (with some logging configured etc), and the whole overhead with sc_service::Configuration is unneeded and possibly misleading.

Unfortunately, I don't see any other way than providing some additional simplified/generalized methods / structs to the client like:

fn create_simple_runner<T: ...>(&self, command: &T) -> error::Result<SimpleRunner<Self>> {
	let tokio_runtime = build_runtime()?;
	command.generalized_init(&Self::support_url(), &Self::impl_version())?;
	SimpleRunner::new(tokio_runtime)
}

so that commands like TryRuntime can be launched like:

#[cfg(feature = "try-runtime")]
Some(Subcommand::TryRuntime(cmd)) => {
	let runner = cli.create_simple_runner(cmd)?;
	runner.async_run(|| {
		let tokio_handle = ...;
		let task_manager =
			sc_service::TaskManager::new(tokio_handle, no_prometheus_registry)
				.map_err(|e| sc_cli::Error::Service(...))?;
		Ok((cmd.run::<Block, service::ExecutorDispatch>(), task_manager))
	})
}

As long as the former point seems to be quite feasible, I have many doubts with the latter one. In particular, I don't know exactly how much effort it requires.

@kianenigma please take a look

Metadata

Metadata

Assignees

No one assigned

    Labels

    J0-enhancementAn additional feature request.

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions