Implement correct and maintainable WASM instrumentation

[matklad]

Today, we rely on wasm instrumentation for:

• gas metering
• stack metering
• various "is this wasm valid?" checks

This code historically caused trouble for us (bugs/performance issues). The issues here are structural – the code is based on pwasm_utils crate, which, at the moment, is deprecated and replaced with wasm-instrumentation crate upstream. Additionally, there's a feeling that we should build, rather than buy, this code, as it's fine details matter for the definition of the protocol.

There's a different angle of consideration here. Historically, parity's stuff was based on parity's implementation of wasm (pwasm). Since then, bytecodealliance produced an array of useful and well maintained tools for working with WASM. The obvious idea is to use that to implement instrumentation. However , there's a certain misalignment of initiatives here, as bytecodealliance aims only at 99% backwards compatibility, and in general intends to support only the latest version of WASM.

We discussed the following design space

• Status que of using pwasm utils – not really tenable, as there are changes we want to do to instrumentation
• Fork pwasm utils – maintaining fork of a deprecated crate in a separate repo creates a lot of overhead
• Vendor relevant parts of pwasm_utils, continue using upstream parity_wasm. This feels like a pure win short-term solution. Additionally, this ensures that we use both parity_wasm and wasmparser to process wasm modules, which gives us extra assurance that we don't accept anything we don't want to
• Rewrite instrumentation on top of wasmparser/wasmencoder – this seems like a good solution, but opens us to compatability problems during upgrades
• Fork wasmparser/wasmencoder at a specific version, and base instrumentation on that – this I think is a good solution, but is relatively labor intensive.
• Implement our own tools for manipulating wasm, potentially auto-generated from the formal spec and
  • either use that directly,
    • or use it as a fuzzing oracle for checkig bytecodealliance implementations

I think there's consensus that we should start wit 3, vendoring instrumentation and using parity_wasm.

Note: here's an example of patch we might want to apply to pwasm_utils: https://github.com/near/wasm-utils/commit/b967faf1aa43c0c8125b529b30806738ae82ccd0

---

Additional related issues to check out:

• https://github.com/near/nearcore/issues/7882
• https://github.com/near/nearcore/issues/7879
• https://github.com/near/nearcore/issues/7878

[nagisa]

There are some additional concerns with the direct use of wasmparser here. For example we document that a module must not have any more than 100k imports total. Ignoring the fact that other limits such as for the overall contract size would kick in first, it is important to remember this check is gone in the most recent version of the wasmparser crate, and we'd lose this check if we did upgrade for any reason.

While I think we should continue using wasmparser, we probably should not be relying on it to imposing any specific limits (that is – we should have our own limits that are more reasonable for our contract size of 4MiB.) Doing so would also enable us to reduce the dependency on having multiple distinct versions of wasmparser as the protocol evolves.

[matklad]

Yup, that's https://github.com/near/nearcore/issues/6456

[matklad]

And related older discussion: https://github.com/near/nearcore/issues/3191

[matklad]

https://github.com/near/nearcore/pull/6774/ vendors pwasm_utils. Potential follow-ups there:

• there's a randomized test which is ignored, because the test itself is broken (see test_build_control_flow_graph_failure)
• that's more or less code dump, it might be a good idea to align the style of that code with nearcore.

[matklad]

@nagisa I think the original intention for this issue was to be a tracking issue for finit-wasm related work. Should we tag it as such, or do we have a trcking issue for that work on some other repo?

[nagisa]

I do have a tracking issue for specifically finite-wasm work (spec, analysis, tests) at https://github.com/near/finite-wasm/issues/16 (private repository for the time being.)

You will notice, however, that this tracking issue does not track the implementation work for anything past analysis (that is, the instrumentation of a given blob of WebAssembly code is not included.)

I figured that given recent performance investigation and conclusions, it might not make sense to spend time on implementing something that doesn’t suit our needs super well either way. So for overall work, yes, having this be the overall tracking issue makes sense to me.

[nagisa]

We have largely finished the implementation of finite-wasm a while ago. Completion of this is pending:

• [ ] an integration into nearcore's near-vm-runner;
• [ ] enabling this code in mainnet;
• [ ] publicizing the crate, documentation and any accompanying material (blog posts, etc.)

[jakmeier]

@nagisa since you just publicized finite-wasm, is it time to close this issue as done?