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

Failed to build some contracts with rustc 1.70 #1139

Closed
kvinwang opened this issue Jun 2, 2023 · 13 comments · Fixed by #1189
Closed

Failed to build some contracts with rustc 1.70 #1139

kvinwang opened this issue Jun 2, 2023 · 13 comments · Fixed by #1189

Comments

@kvinwang
Copy link
Contributor

kvinwang commented Jun 2, 2023

 [2/4] Post processing code
ERROR: Loading of original wasm failed

Caused by:
    0: Loading of wasm module at '[...]/target/ink/wasm32-unknown-unknown/release/system.wasm' failed
    1: Unknown opcode 192

The failed contract is here: https://github.com/Phala-Network/phala-blockchain/tree/master/crates/pink-drivers/system

@kvinwang
Copy link
Contributor Author

kvinwang commented Jun 2, 2023

There is an upstream discussion: rust-lang/rust#109807

@ascjones
Copy link
Collaborator

ascjones commented Jun 2, 2023

@kvinwang thanks for reporting this we will look into it. Regarding the linked issue,cargo-contract does not explicitly pass target-feature=-sign-ext, so is the issue linked relevant (although I haven't read it properly all the way through)? I'm not an expert in these Wasm features so need to look at this to understand it better.

@kvinwang
Copy link
Contributor Author

kvinwang commented Jun 3, 2023

so is the issue linked relevant (although I haven't read it properly all the way through)?

Yep, it is relevant. I can also reproduce the example in that issue with:

RUSTFLAGS="-C linker-plugin-lto -C target-cpu=mvp" cargo build --target wasm32-unknown-unknown --release -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort --no-default-features

where -C linker-plugin-lto is used by cargo-contract.

For ink contracts, a simple example to reproduce it could be: let foo: String = Decoce::decode(&mut &buf[..]).unwrap().

@ascjones
Copy link
Collaborator

ascjones commented Jun 5, 2023

Thanks @kvinwang for clarifying and for the repro that is super helpful. It is high priority for this to be resolved ASAP, though it seems we are dependent on that upstream issue being resolved?

Even if we update parity-wasm to support that opcode (it is deprecated...), pallet-contracts itself explicitly disables that feature: https://github.com/paritytech/substrate/blob/master/frame/contracts/src/wasm/mod.rs#LL223C12-L223C12

@webmaster128
Copy link

Even if we update parity-wasm to support that opcode (it is deprecated...)

A patch for parity-wasm would be much appreciated ❤️ (I know it's deprecated but still)

@ascjones
Copy link
Collaborator

@webmaster128 the idea is to replace it see #1141

@webmaster128
Copy link

Thank you @ascjones. I'll follow that ticket. We use parity-wasm as well in CosmWasm and getting a patch there short term while looking for alternatives mid-term would be great. But I also understand if there is nobody maintaining parity-wasm anymore.

@ascjones
Copy link
Collaborator

We use parity-wasm as well in CosmWasm and getting a patch there short term while looking for alternatives mid-term would be great. But I also understand if there is nobody maintaining parity-wasm anymore.

When we do this I will look at what the filecoin guys have done in https://github.com/filecoin-project/fvm-wasm-instrument, which is a fork of https://github.com/paritytech/wasm-instrument with parity-wasm replaced by was-encoder and wasmparser. Hope that helps!

@webmaster128
Copy link

webmaster128 commented Jun 22, 2023

Turns out you can tell parity-wasm to allow the sign extension opcodes via an already existing feature. This might help short term until the mogration to a different parser is completed. See CosmWasm/cosmwasm#1743.

@ascjones
Copy link
Collaborator

Turns out you can tell parity-wasm to allow the sign extension opcodes via an already existing feature. This might help short term until the mogration to a different parser is completed. See CosmWasm/cosmwasm#1743.

Fantastic, that is really useful thanks! It will make fixing it on the cargo-contract side much easier.

@webmaster128
Copy link

Not sure if this helps in your case, but we realized that a different way to mitigate the problem is running the compiled Wasm through wasm-opt with --signext-lowering. This ensures none of the operations in question are left, no matter which Rust version you use for compilation. This way you solve the issue for hosts that only support Wasm MVP.

@hieptran1812
Copy link

@kvinwang Have you fixed this issue? I have this issue as well

@kvinwang
Copy link
Contributor Author

kvinwang commented Aug 9, 2023

@kvinwang Have you fixed this issue? I have this issue as well

I use rustc-1.69 as a workaround.

rcarback added a commit to harvard-polkadot-tarot/tarot-contract that referenced this issue Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants