WASM Local-blob override

[insipx]

This is the first PR to add overrides for on-chain WASM with WASM stored locally on a users machine.

rel #7035

This PR introduces two new CLI arguments,

• wasm-overwrite a flag for enabling WASM overwrites
• wasm-overwrite-path path where the WASM blobs are stored, defaults to $BASE_PATH/$chain/wasm_runtime_overwrites

Most of the logic is contained within wasm_overwrite.rs in client/service/src/client/. The LocalCallExecutor uses this module to try and replace on-chain WASM with local WASM if wasm overwrites are enabled.

polkadot companion: paritytech/polkadot#1826

Usage:

WASM overrides may be enabled with the --wasm-runtime-overrides argument. The argument
expects a path to a directory that holds custom WASM.

Any file ending in '.wasm' will be scraped and instantiated as a WASM blob. WASM can be built by
compiling the required runtime with the changes needed. For example, compiling a runtime with
tracing enabled would produce a WASM blob that can used.

A custom WASM blob will override on-chain WASM if the spec version matches. If it is
required to overrides multiple runtimes, multiple WASM blobs matching each of the spec versions
needed must be provided in the given directory.

[dvdplm]

I think this feels a bit clunky and I wonder if we can come up with a smoother API. If wasm_overwrite could take only the spec_version maybe we could get away with try_replace(spec_v, &state)? It also feels weird to me that try_replace returns the input code if no override was found; can't we move that check elsewhere, perhaps into runtime_code()?
Maybe if you expanded a bit on the motivation it'll be clearer to me why it needs to be like this.

Having something like this would be nicer imo, (pseudo-code):

let runtime_code = {
  let spec_ver = code.spec_version();
  self.runtime_override
    .and_then(|rt_override| rt_override.getspec_ver, &state) )
    .unwrap_or_else(|| state_runtime_code.runtime_code()? )
}; 

[insipx]

Changed this so that now WasmOverwrite just has a get method that returns None if it doesn't find an overwrite, where LocalCallExecutor gets the version for a blob, so it looks something like

let runtime_code = {
    let runtime_code = state_runtime_code.runtime_code();
    self.check_overwrites(runtime_code).unwrap_or(runtime_code)?
};

I think it's a bit odd that it's accepting a runtime code and then unwrapping into another runtime code, but it encapsulates calling for the version into it's own function on LocalCallExecutor now, where the actual get function only needs spec_version and heap_pages. We could go further and put the check_overwrite in WasmOverwrite too

check_overwrite accepts RuntimeCode again, for the convenience of having code.heap_pages. We could achieve a more similar API to the comment by using the runtime_version method on LocalCallExecutor and then getting heap_pages before calling get and would probably look more like

let runtime_code = {
    let code = state_runtime_code.runtime_code()?;
    let spec = self.runtime_version(id)?.spec_version;
    self.wasm_overwrite
        .as_ref()
        .map(|o| o.get(&spec, code.heap_pages))
        .flatten()
        .unwrap_or(code)
};

The reason to avoid calling self.runtime_version on LocalCallExecutor now in check_overwrites is to avoid requiring another type parameter (Block: BlockT) on all LocalCallExecutor types.

Second version is way more clear on what's happening but first requires less code wherever an overwrite is required

[pepyakin]

There are quite some spaces, trailing whitespaces and lack of newline file termination. Is there any chance you could configure your editor to fix those on save?

[insipx]

There are quite some spaces, trailing whitespaces and lack of newline file termination. Is there any chance you could configure your editor to fix those on save?

I've since installed an editor-config plugin, which was missing in my setup 😅 . Sorry for all the whitespace, I didn't even realize I had that many

[mattrutherford]

LGTM, subject to query about naming

[mattrutherford]

Just a terminology query - should we use override vs overwrite everywhere? Seems like overwrite is used predominantly but I think override is a better choice - either way good to be consistent, unless actually there is some difference in meaning - in which case perhaps choose another word, so that the two concepts are more obviously distinct in their name.

[dvdplm]

I like override better too (and may be the one to blame for the inconsistent docs here, not sure) but kept my mouth shut for fear of nitpicking, but as you opened up the can... :)

"Overwrite" is more concrete than "override" and in some sense we are writing something over what was previously there. We're "clobbering" the on-chain runtime. On the other hand "override" reflects more on what the actual outcome of the operation is: taking control over the normal flow of a process (picking the runtime) and intervene. It's very close to the dictionary definition of "override": "interrupt the action of (an automatic device), typically in order to take manual control.".

Either works. I like "override" better. Andrews call.

[bkchr]

@insipx please merge master to fix the companion check.

[dvdplm]

@insipx is C1-low the appropriate label here?

[bkchr]

@insipx is C1-low the appropriate label here?

Yes, this does not elevate any release.

[apopiak]

@insipx Could you add some usage notes to the PR description?

[insipx]

@insipx Could you add some usage notes to the PR description?

done