You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Repository navigation
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Unsupported Wasm Opcode after updating nightly toolchain #13636
I'm familiar with substrate very well, so I know this error is caused by the crate parity-wasm that developed by parity to check and decode and etc to handle wasm file.
This error comes from
So the unknown Opcode is 0xc0 (192), while the function deserialize_buffer comes from crate parity-wasm.
And for current substrate (at least for the branch polkadot-0.9.39), the parity-wasm is using the version 0.45.0
In this version, the Opcode 0xc0 is under the control of feature sign_ext
I know that substrate runtime wasm should be limited in the most concise Wasm specification.
So I'm not sure whether the Opcode group sign_ext is under the required Wasm specification for the Substrate runtime.
If it's needed, fix this issue is very simple.
However if it's not needed, it's strange why rust toolchain need to add those group Opcode in normal Wasm file.
At least I can ensure that, for the toolchain nightly-1.70.0(2023-03-18) is will appear this problem, while night-1.68.0 not. Not sure for nightly-1.69.0
Although I am still curious whether those Wasm Opcodes are necessary for the substrate runtime @bkchr , I doubt this revert rust-lang/rust#109326 is just a temp fix, it will add it back in future? If rust toolchain still use those changes, maybe substrate need to update the check for wasm code.
LLVM was reverted because it broke a lot of things. As long as the new LLVM isn't back and doesn't bring back these op codes I assume that it was some kind of bug. If we see that the LLVM upgrade brings this back, we will need to investigate it!
Although I am still curious whether those Wasm Opcodes are necessary for the substrate runtime @bkchr , I doubt this revert rust-lang/rust#109326 is just a temp fix, it will add it back in future? If rust toolchain still use those changes, maybe substrate need to update the check for wasm code.
Anyway, I close this issue for now.
binaryen v111 enables sign_ext by default as well, mb there's any new consensus inside the mozilla team
binaryen v111 enables sign_ext by default as well, mb there's any new consensus inside the mozilla team
Okay thank you! I also just checked and every major browser has enabled it, maybe time to also enable it in our executor? But for Parachains we will need to be able to support this at runtime, to be able to let the relay chain runtime decide on when to enable this feature.
binaryen v111 enables sign_ext by default as well, mb there's any new consensus inside the mozilla team
Okay thank you! I also just checked and every major browser has enabled it, maybe time to also enable it in our executor? But for Parachains we will need to be able to support this at runtime, to be able to let the relay chain runtime decide on when to enable this feature.
Hmm... we could enable it, but AFAIK we'd need to add some mechanism for versioning the PVFs' execution environment so that we can enable this in a controlled fashion. (paritytech/polkadot-sdk#917) cc @s0me0ne-unkn0wn
We could also just pass -C target-cpu=mvp as a compilation flag and I think that should disable this extensions even on the most recent version of rustc/LLVM; not entirely ideal, but it would kick the can down the road for a little bit more.
Hmm... we could enable it, but AFAIK we'd need to add some mechanism for versioning the PVFs' execution environment so that we can enable this in a controlled fashion
Yeah, this is what I meant with my complicated sentence :P
parity-wasm supports sign_ext if its sign_ext feature is enabled, maybe it's time to enable it? I believe It's used during the instrumentation phase only, so shouldn't affect anything.
Enabling it for Wasmtime through parametrization would be great, but I'm afraid it would require some time... First, we must add a new execution environment parameter that controls WASM features (we do not have one yet) and wait for the supermajority to upgrade. Then, I have to push with paritytech/polkadot#6805. Otherwise, we'll have to do a runtime upgrade to enable it. Sounds doable, though, the only question is how urgent it is.
parity-wasm supports sign_ext if its sign_ext feature is enabled, maybe it's time to enable it? I believe It's used during the instrumentation phase only, so shouldn't affect anything.
No real point in enabling it if it's not going to be supported by the executor though; it'll still fail, just a little later.
Sounds doable, though, the only question is how urgent it is.
Since AFAIK we can still force rustc to emit code without this feature it's not super urgent, but time's slowly running out. I wouldn't be surprised if the mvp feature target will eventually get removed. So it'd be better to start on this sooner rather than later.
parity-wasm supports sign_ext if its sign_ext feature is enabled, maybe it's time to enable it? I believe It's used during the instrumentation phase only, so shouldn't affect anything.
No real point in enabling it if it's not going to be supported by the executor though; it'll still fail, just a little later.
Yes. For now we should pass -C target-cpu=mvp in wasm-builder. This is what we do in cargo contract. At least it will work until they remove this CPU. And then we get a clean build error. I am not sure if they will ever remove it. Other targets still support decade old CPUs.
parity-wasm supports sign_ext if its sign_ext feature is enabled, maybe it's time to enable it? I believe It's used during the instrumentation phase only, so shouldn't affect anything.
No real point in enabling it if it's not going to be supported by the executor though; it'll still fail, just a little later.
I've just started working on this, and it seems that with this very proposal, the only problem is its support in parity-wasm indeed, as according to Wasmtime docs, it has been enabled and cannot be disabled for at least 3 years now (as far as I dug into the git history): https://docs.wasmtime.dev/stability-wasm-proposals-support.html
Activity
bkchr commentedon Mar 19, 2023
Maybe related to this rust-lang/rust#109326?
Please test with the nightly from the 20.03 when it is available and report back! Ty.
btwiuse commentedon Mar 20, 2023
Yesterday compiling https://github.com/gear-dapps/app also failed with
error: Unknown opcode 192when the issue was created.Today I updated the nightly toolchain to
rustc 1.70.0-nightly (da7c50c08 2023-03-19)and the error disappeared.atenjin commentedon Mar 20, 2023
fine, when I have updated my toolchain to:
Anything is ok for now.
Although I am still curious whether those Wasm Opcodes are necessary for the substrate runtime @bkchr , I doubt this revert rust-lang/rust#109326 is just a temp fix, it will add it back in future? If rust toolchain still use those changes, maybe substrate need to update the check for wasm code.
Anyway, I close this issue for now.
bkchr commentedon Mar 20, 2023
LLVM was reverted because it broke a lot of things. As long as the new LLVM isn't back and doesn't bring back these op codes I assume that it was some kind of bug. If we see that the LLVM upgrade brings this back, we will need to investigate it!
Ty for the report @atenjin!
clearloop commentedon Mar 20, 2023
binaryen v111 enables
sign_extby default as well, mb there's any new consensus inside the mozilla teambkchr commentedon Mar 20, 2023
Okay thank you! I also just checked and every major browser has enabled it, maybe time to also enable it in our executor? But for Parachains we will need to be able to support this at runtime, to be able to let the relay chain runtime decide on when to enable this feature.
WDYT @athei @koute
koute commentedon Mar 20, 2023
Hmm... we could enable it, but AFAIK we'd need to add some mechanism for versioning the PVFs' execution environment so that we can enable this in a controlled fashion. (paritytech/polkadot-sdk#917) cc @s0me0ne-unkn0wn
We could also just pass
-C target-cpu=mvpas a compilation flag and I think that should disable this extensions even on the most recent version of rustc/LLVM; not entirely ideal, but it would kick the can down the road for a little bit more.bkchr commentedon Mar 20, 2023
Yeah, this is what I meant with my complicated sentence :P
s0me0ne-unkn0wn commentedon Mar 20, 2023
parity-wasmsupportssign_extif itssign_extfeature is enabled, maybe it's time to enable it? I believe It's used during the instrumentation phase only, so shouldn't affect anything.Enabling it for Wasmtime through parametrization would be great, but I'm afraid it would require some time... First, we must add a new execution environment parameter that controls WASM features (we do not have one yet) and wait for the supermajority to upgrade. Then, I have to push with paritytech/polkadot#6805. Otherwise, we'll have to do a runtime upgrade to enable it. Sounds doable, though, the only question is how urgent it is.
koute commentedon Mar 20, 2023
No real point in enabling it if it's not going to be supported by the executor though; it'll still fail, just a little later.
Since AFAIK we can still force rustc to emit code without this feature it's not super urgent, but time's slowly running out. I wouldn't be surprised if the
mvpfeature target will eventually get removed. So it'd be better to start on this sooner rather than later.athei commentedon Mar 20, 2023
Yes. For now we should pass
-C target-cpu=mvpinwasm-builder. This is what we do incargo contract. At least it will work until they remove this CPU. And then we get a clean build error. I am not sure if they will ever remove it. Other targets still support decade old CPUs.nazar-pc commentedon Apr 3, 2023
I'm still getting this on latest Rust Nightly with latest Substrate
masterthat includes-C target-cpu=mvpfrom #13758. Anything else I'm missing?koute commentedon Apr 3, 2023
You're right; that was still not enough. With this PR it should work now: #13804
s0me0ne-unkn0wn commentedon Apr 3, 2023
I've just started working on this, and it seems that with this very proposal, the only problem is its support in
parity-wasmindeed, as according to Wasmtime docs, it has been enabled and cannot be disabled for at least 3 years now (as far as I dug into the git history): https://docs.wasmtime.dev/stability-wasm-proposals-support.htmlchore: update Polkadot dependencies to 0.9.39 (#500)