-
Notifications
You must be signed in to change notification settings - Fork 34
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
fails to compile to wasm #32
Comments
This sounds like a bug in |
Hi @ulrichard, are you still hitting this issue? |
Hi I'm getting this issue when compliling to wasm:
Do you know what might be the cause? @ulrichard did you manage to solve your issue? |
I still get the same error as above with the current master. |
Note that in general you cannot safely link most C code with wasm_bindegn. While we should fix this, you'll need to use a different wasm build target to make it work. |
I am getting the same error with
@TheBlueMatt Could you elaborate? Is there a way to make this library work with |
I started experimenting: |
I had a brief look and I rekon that the problem is the secp build, which requires various things like patching out functions etc. I'd say we have to port all the wasm build stuff from |
I wonder if we can patch out libsecp entirely and link against the one in rust-secp-sys. We would like to do this anyway to minimize the amount of code we compile. |
I'm confused why we think this is related to secp - the specific error (passing a C++ argument when building targeting C) sounds like either we're invoking |
By "related to secp" I meant I messed around with the build file and could not get secp to build, and that is the first step before building the C++ code. I got past the exact error in this issue. |
Ah, so somehow we shouldn't be passing the |
Adding this to C++ builds and not to C builds seems to fix this specific error.
EDIT: I am not totally confident in this statement. |
What is the reason that compiling for wasm in rust-secp256k1 is patching the Cargo.toml file just for the test? Would that modification break anything else if applied permanently? |
@ulrichard it wouldn't hurt anything but it would be wasteful. We need to build rust-secp as both a dylib and a cdylib for WASM but we don't need any special handling for non-WASM users. (I'm actually unsure if we need this at all ... the git history shows this line being introduced in April 2020 into travis.yml and we've just preserved it since then.)
Is this a philosophical claim or a technical one? Because many people do use rust-secp as a direct dependency, including WASM users. |
@apoelstra Can you point me to a project, where rust-secp is used as a dependency for building wasm? I'm fairly new to wasm, and that could help a lot seeing how it can be done. |
I'm guessing mutiny uses wasm, is that right @benthecarman? |
Yes |
Ok, I dug a bit deeper. The issue with rust-secp seems solvable. |
Sweet, nice one! This might be in the process of getting solved upstream: bitcoin-core/secp256k1#1461. For the immediate fix we can do the whole WASM dance that we do in |
Jumping to the thread here, I have the same problem when introducing |
Thanks @jbesraa, FWIW I don't personally have any ideas or spare clock cycles to do anything about this ATM. |
no worries @tcharding. I am trying to work on this, will report back. |
The text was updated successfully, but these errors were encountered: