-
Notifications
You must be signed in to change notification settings - Fork 2.7k
try-runtime - specifying runtime & cutting off unnecessary configuration burden #12156
Description
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:
- we add a new shared parameter for all
try-runtimesubcommands:enum Runtime, which has three variants:Remote | Native | Local(Pathbuf)with natural meaning:
-Remote: we useWasmexecution with the runtime included in the scraped storage (and we do not have to overwrite anything)
-Native: we useNativeexecution with the code from the binary that is actually running (in case of spec-* mismatch we can panic)
-Local(Pathbuf): we useWasmexecution with the runtime read from local file atPathbuf(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.
- we completely get rid of using
sc_service::Configurationandsc_cli::SharedParamsintry-runtime- if I understand correctly, the only essential information that we use from it are:default_heap_pages,max_runtime_instancesandruntime_cache_size. We could 'move' these three arguments toSharedParamsand stop using both concepts at all. However, the problem is not gone:TryRuntimeCmdwill be still launched usingsc::cli::Runner, which requiressc_service::Configurationobject. If I'm not mistaken,try-runtimeis a command that needs nothing more than an async environment (with some logging configured etc), and the whole overhead withsc_service::Configurationis 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
Labels
Type
Projects
Status