contracts: Replace sp-sandbox and wasmi-validation by newest wasmi

[athei]

Fixes #11752

Instead of refactoring sp-sandbox as mentioned in the issue we get rid of it. The abstraction doesn't serve any purpose anymore since we are only using the embedded executor. If we want to use any client resident executor in the future it will be in a completely different form. sp-sandbox will be removed in a follow up PR.

Performance increase

Check out the weight changes. Instruction costs go down between 68%-97%. This is however, mostly due to the static cost of invoking a contract going down (read further down why). The costs per instruction go down more like 50%. You can see that when expanding a benchmark and comparing the coefficient to the r (repetition) variable.

This massive uplift is mostly because we no longer include the time wasmi takes to compile an instruction into account. Before, this was the case because the old API didn't allow us to compile and invoke separately. The cost to compile a contract of size n bytes is already covered by the call_with_code_per_byte benchmark. This assumes that the contract generated there is close to the worst case in terms of compilation time. According to @Robbepop compiling br_indirect might be the worst case. It is hard to setup such a benchmark and we are limiting the size of br tables anyways. So the assumption here is that this is not exploitable by crafting a weird contract that takes a long time to compile. That said, this is not really a new assumption: We always relied on the fact that call_with_code_per_byte is representative enough of code compilation so a malicious contract can't explode our compilation time. Hence we don't introduce more attack vectors here but rather make the existing instruction benchmarks more accurate. Point is that more expensive instructions wouldn't have protected us from that anyways as they are only charged during execution and not compilation.

We suffer a regression when uploading or re-instrumenting new code. This is expected because we are parsing the code one more time now. This will eventually be resolved when we replace parity-wasm and can do all of that in a single pass. However, those are rare actions while executing contracts happens often. A big win in my book. This is also without optimizations from wasm-opt from which wasmi profits a lot. Those will come later.

Changes in this PR

Passing in Memory as parameter

The biggest change is to the define_env! proc macro and its invocations: Due to API changes we need to pass in the Memory into the actual host functions instead of passing it via the Runtime struct. Hence the macro is changed to pass in these information as an additional implicit argument (in addition to the already existing ctx). The macro is not perfect and it doesn't have to be. It now implements this single trait for a type Env that is used to register all functions in the wasmi linker:

substrate/frame/contracts/src/wasm/runtime.rs

Lines 41 to 48 in cf4224b

	/// Trait implemented by the [`define_env`] macro for the emitted `Env` struct.
	pub trait Environment<HostState> {
	/// Adds all declared functions to the supplied [`Linker`] and [`Store`].
	fn define(
	store: &mut Store<HostState>,
	linker: &mut Linker<HostState>,
	) -> Result<(), LinkerError>;
	}

HostState is our Runtime type which is the ctx or context passed into our host functions. We explain in the next section why we are generic over this type.

Wasm Validation

Previously, the macro implemented the ImportSatisfyCheck trait which was used in order to check if a contract only imports functions that pallet-contracts supports. This was kind of a brittle approach because it didn't check wether a contract could actually be instantiated when running it in Wasmi (in case of an upload without instantiate). With this PR we run an actual instantiation to validate the wasm. Because we don't really have a host state type available at that point we let the macro implement two versions of the Environment trait: Environment<()> and Environment<Runtime>. The first is used for the validation and mocks all functions to be empty (they are never called).

This is run on the instrumented code. However, we need to make sure that the code is valid according to the wasm standard before instrumenting (the instrumentation depends on it). This is why we make a pre-pass with wasmparser before conducting further checks and eventually instrumenting.

Reject contracts with start functions

Having a start function in contracts give us all kinds of headaches. They were allowed but neither Rust nor solang make use of them. Strictly speaking this is a change that breaks existing contracts with start functions but that would be some very exotic contracts written in assembly and we prefer to get rid of that before a substantial amount of contracts start depending on it. We might need to roll it back when too many people start complaining.

Bump schedule version

This is a substantial change to the weights and validation. We want all existing contracts to be re-checked and re-instrumented to profit from it. No contract should get rejected because of it. Only due to bugs in the old implementation. We would rather have those contracts be rejected than keep existing and causing security issues.

[bkchr]

sp-sandbox can probably be removed completely when it isn't used anymore?

[athei]

There is no user within substrate anymore after this PR is merged. But I am not sure if there are any other users of this crate (GEAR?).

[command-bot]

@athei Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_contracts has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2061018 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2061018/artifacts/download.

[agryaznov]

Some quite small things left, otherwise looks good!

[agryaznov]

LGTM

[Robbepop]

All in all LGTM

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-each-crate
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2083001

[yjhmelody]

Hi, any task to update wasmi for sc-executor ?

[athei]

No plans. We rather remove wasmi from the executor and make it wasmtime only. If you want to discuss this further please open discussion in the polkadot forum.