Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Export host functions from sp-sandbox #5939

Closed
wants to merge 1 commit into from
Closed

Export host functions from sp-sandbox #5939

wants to merge 1 commit into from

Conversation

shamilsan
Copy link

We face an error call to a missing function env:ext_sandbox_... when trying to connect a parachain that uses sp-sandbox (https://github.com/paritytech/substrate/tree/master/primitives/sandbox). This PR fixes this problem by exporting correspondent host functions.

@ordian ordian requested a review from pepyakin August 29, 2022 11:24
@bkchr
Copy link
Member

bkchr commented Aug 29, 2022

These are not exported, because they are not stable and may change in the future. It is also currently not planed for the near future to enable them. Why do you need them?

@shamilsan
Copy link
Author

These are not exported, because they are not stable and may change in the future. It is also currently not planed for the near future to enable them. Why do you need them?

We use sp-sandbox in our project (https://github.com/gear-tech/gear/blob/master/core-backend/sandbox/Cargo.toml) and have the following error when trying to connect it as a parachain to the local Rococo testnet:

AbortedDueToTrap(MessageWithBacktrace { message: "call to a missing function env:ext_sandbox_memory_new_version_1", ... })

We can't avoid it therefore we can't run our node as a parachain collator.

@bkchr
Copy link
Member

bkchr commented Aug 29, 2022

As I said, this is disabled on purpose and will not be enabled in the near future AFAIK.

If you fork, please check upstream Substrate on how this is solved there. We currently use wasmi inside wasm to have the sandboxing.

@shamilsan
Copy link
Author

As I said, this is disabled on purpose and will not be enabled in the near future AFAIK.

Is it possible to enable it under v1 and provide stabilized version later under v2?

If you fork, please check upstream Substrate on how this is solved there. We currently use wasmi inside wasm to have the sandboxing.

I've not found an example of Substrate node with using Cumulus and sandbox together. substrate-parachain-template doesn't use sp-sandbox. Could you please provide such kind of example?

@bkchr
Copy link
Member

bkchr commented Aug 29, 2022

Is it possible to enable it under v1 and provide stabilized version later under v2?

No. We would need to support these interfaces until the end of the universe.

I've not found an example of Substrate node with using Cumulus and sandbox together. substrate-parachain-template doesn't use sp-sandbox. Could you please provide such kind of example?

As I said before, if you fork, you should may merge back master from time to time: paritytech/substrate#9592

@shamilsan
Copy link
Author

As I said before, if you fork, you should may merge back master from time to time: paritytech/substrate#9592

Our Substrate fork is based on the 2022-07-12 master, so this PR has been already there. But as I see it doesn't work with the wasmer-sandbox feature. This seems to be our case. Unfortunately, wasmi works a way slower than wasmer but now I see the cause of the problem, thank you for the hint.

@bkchr
Copy link
Member

bkchr commented Aug 29, 2022

Yeah wasmi is slower than wasmer, but wasmer is even more experimental 🙈

@bkchr
Copy link
Member

bkchr commented Aug 30, 2022

I'm going to close this for now.

@bkchr bkchr closed this Aug 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants