Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wasmer runtime panics with metering middleware #183

Open
sanderpick opened this issue Feb 18, 2023 · 2 comments
Open

Wasmer runtime panics with metering middleware #183

sanderpick opened this issue Feb 18, 2023 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed wasmer4

Comments

@sanderpick
Copy link
Contributor

sanderpick commented Feb 18, 2023

This issue may be a consequence of how fp-bindgen is design vs. a real bug, but I figured may as well report it here.

Wasmer's metering middleware allows you to assign a point cost to different wasm operators, specify an initial point balance for an instance, and then error when the balance is exhausted. This functionality is useful for blockchains or other environments that want to limit resources spent during the execution of some exported function.

I was playing around with the functionality and noticed that instead of erroring when an instance's metering points are exhausted, fp-bindgen just panics.

Here's a fork just adds the metering middleware to the example-protocol bindings default_store method: https://github.com/sanderpick/fp-bindgen/blob/sander/metering-panic/examples/example-protocol/bindings/rust-wasmer-runtime/bindings.rs#L54 and sets a very low initial metering point value. Note that I committed the bindings directory in order to point to it here.

When you run the primitives test, you'll notice a panic from EXC_BAD_INSTRUCTION (code=1, subcode=0xd4a00000) that happens in the exported init function (specifically at function.call()?:

Screenshot 2023-02-17 at 4 42 05 PM

If you comment out the call to init in the tests (https://github.com/sanderpick/fp-bindgen/blob/sander/metering-panic/examples/example-rust-wasmer-runtime/src/test.rs#L280), and then run the fetch_async_data test (https://github.com/sanderpick/fp-bindgen/blob/sander/metering-panic/examples/example-rust-wasmer-runtime/src/test.rs#L259), you'll notice that in this case the panic happens at the get_unchecked call in fp-bindgen-support (https://github.com/sanderpick/fp-bindgen/blob/sander/metering-panic/fp-bindgen-support/src/host/runtime.rs#L37).

Screenshot 2023-02-17 at 4 45 23 PM

If you set the initial metering points to something very high (https://github.com/sanderpick/fp-bindgen/blob/sander/metering-panic/examples/example-protocol/bindings/rust-wasmer-runtime/bindings.rs#L52), say 100_000_000, everything works as expected. You can query for spent points after a call to an exported function, etc. So, it seems like the issue is related to a mishandling of or ignoring of a MiddlewareError (https://github.com/wasmerio/wasmer/blob/447c2e3a152438db67be9ef649327fabcad6f5b8/lib/compiler/src/error.rs#L55).

I've done some digging but wanted to ask here if middlewares might just not work well w/ fp-bindgen.

@arendjr arendjr added enhancement New feature or request help wanted Extra attention is needed labels Feb 20, 2023
@arendjr
Copy link
Contributor

arendjr commented Feb 20, 2023

Thanks, this is a very well-written request! Indeed, middlewares are not currently supported, but based off this report, I think we can create an improvement to support them. Just from the looks of it, it seems it would require two things on our part:

  • Expose an API in the generated runtime so that callers can specify their middlewares without the need for forking
  • More resilient error handling on our part, so that middleware errors don't cause crashes inside the bindings

I cannot promise when we'll get to this, but it seems to be a worthwhile improvement. If you want to work on this yourself, we would welcome a PR for this 👍

@sanderpick
Copy link
Contributor Author

Hey @arendjr, thanks for the reply. I did do some digging but never got to the bottom of the issue. I think it's related to my usage of async functions over the bridge.

I ended up building a bridge based on the architecture in https://github.com/CosmWasm/cosmwasm. I was able to get metering working there and gave me a bit more flexibility overall. I will definitely keep an eye on this project though. Thank you for the help!

@arendjr arendjr added the wasmer4 label Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed wasmer4
Projects
None yet
Development

No branches or pull requests

2 participants