namada icon indicating copy to clipboard operation
namada copied to clipboard

Refactor `wasmer` interfacing code

Open sug0 opened this issue 1 year ago • 4 comments

In #3308, wasmer was bumped to 4.3.1. The migration from 2.x to 4.x came with significant API changes, which were addressed in a less than ideal way. Therefore, we ought to refactor/improve this code. Here are some of the associated tasks:

  • [ ] Attempt to remove the unsafe Send and Sync impls on wasm memory. We never share memory across threads, and it is not allocated in thread local storage; the only reason it is marked as Send and Sync is to abide by the requirements of wasmer host functions, which must implement Send and Sync.
  • [ ] Reduce boilerplate of wrapping host functions in wrap_tx and wrap_vp modules.
  • [ ] Decouple wasmer::Store from WasmMemory. Instead, use the host function wrappers to grab a wasmer::AsStoreMut impl from the wasmer::FunctionEnvMut.

sug0 avatar May 27 '24 07:05 sug0

I think for the 2nd point (wrap_tx and wrap_vp) we could have something similar to the trait wasmer::HostFunction that would call env.data_mut() for us

tzemanovic avatar May 28 '24 09:05 tzemanovic

on a second thought, it might be better to use wasmer::FunctionEnvMut in our host_env directly in order to address 3rd point

tzemanovic avatar May 28 '24 10:05 tzemanovic

on a second thought, it might be better to use wasmer::FunctionEnvMut in our host_env directly in order to address 3rd point

some functionality is mocked to run tests. wasmer is not used in some calls to the host functions, so, unless we mock some kind of VmStore that is used by the VmMemory implementation, it is not advisable to pass FunctionEnvMut to host functions directly

sug0 avatar May 28 '24 12:05 sug0

@cwgoes this shouldn't be consensus breaking, it doesn't change execution semantics, it's more of an internal detail (unless the vm is buggy)

sug0 avatar Aug 29 '24 22:08 sug0