Closed tmpfs closed 1 year ago
Hi ! We plan to remove rust-crypto . meanwhile : see if this helps : https://github.com/ZenGo-X/multi-party-ecdsa/pull/137
rust-crypto
dependency bothers us a lot, so we're removing it: https://github.com/ZenGo-X/curv/pull/137. Though it's one of minor challenges, a lot of other things block us from being wasm-compatible right now, e.g. we likely need pure Rust implementation of secp256k1, see #137 mentioned above.
Thanks @omershlo and @survived, I am excited to see those changes land!
a lot of other things block us from being wasm-compatible right now
It would be really useful to keep track of these to get an idea when we might be able to use multi-party-ecdsa
in the browser, would you be able to list the other blocking issue(s) please?
@survived rust-secp256k1 should work on wasm, we even try to test it in the CI: https://github.com/rust-bitcoin/rust-secp256k1/blob/master/.github/workflows/rust.yml#L32 (I say try because there are dozens of different flavors of wasm interpreters )
Hey,
I noticed that https://github.com/ZenGo-X/curv/pull/137 has landed in curv@0.8
and wanted to upgrade the dependency here but it looks like centipede
and bulletproof
would need to be upgraded in lock step.
What is the recommended process to test out updating the curv
library? Anything I can do to help please let me know :pray:
Hi @tmpfs, as you correctly observed, multi-party-ecdsa
depends on several crates that depend on curv
as well, so they need to be upgraded first. Usually I update multi-party-ecdsa
and its deps in this order: paillier
, zk-paillier
, bulletproof
, centipede
, and then multi-party-ecdsa
. Updating deps is like warming up, because they have fewer curv
usage (and generally have smaller codebase) 🙂 . Any help with upgrading crates is really appreciated ❤️
@survived, heads up that I just got this repository compiling and we move past the rust-crypto
problem but it looks like we need to enable the js
feature flag for getrandom
:
error: the wasm32-unknown-unknown target is not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support
--> /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.3/src/lib.rs:219:9
|
219 | / compile_error!("the wasm32-unknown-unknown target is not supported by \
220 | | default, you may need to enable the \"js\" feature. \
221 | | For more information see: \
222 | | https://docs.rs/getrandom/#webassembly-support");
| |_________________________________________________________________________^
error[E0433]: failed to resolve: use of undeclared crate or module `imp`
--> /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/getrandom-0.2.3/src/lib.rs:246:5
|
246 | imp::getrandom_inner(dest)
| ^^^ use of undeclared crate or module `imp`
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0433`.
error: could not compile `getrandom`
I will return to this problem once https://github.com/ZenGo-X/multi-party-ecdsa/pull/144 is ready for review :+1:
@tmpfs Small note, if you want to enable a feature in a transitive dependency you can add getrandom = {version = "0.2", features = ["js"]}
in your downstream app/crate and that will enable the js
feature transitively even if you don't use getrandom
directly.
Thanks @elichai, setting the feature flag for the transitive dependency fixes that issue and I move on to the next error:
error[E0432]: unresolved imports `libc::c_char`, `libc::c_int`, `libc::c_long`, `libc::c_ulong`, `libc::c_void`, `libc::c_double`, `libc::size_t`
--> /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/rust-gmp-kzen-0.5.1/src/mpz.rs:1:12
|
1 | use libc::{c_char, c_int, c_long, c_ulong, c_void, c_double, size_t};
| ^^^^^^ ^^^^^ ^^^^^^ ^^^^^^^ ^^^^^^ ^^^^^^^^ ^^^^^^ no `size_t` in the root
| | | | | | |
| | | | | | no `c_double` in the root
| | | | | no `c_void` in the root
| | | | no `c_ulong` in the root
| | | no `c_long` in the root
| | no `c_int` in the root
| no `c_char` in the root
So I figured I needed to enable the num-bigint
feature for curv
to use the pure Rust implementation so I added:
curv-kzen = {version = "0.9", features = ["num-bigint"], default-features = false}
But that doesn't help. Any ideas?
Interestingly, when I try compiling for the wasm32-wasi
target I get this error:
error: You can choose only one bigint implementation. See crate features.
--> /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/curv-kzen-0.9.0/src/arithmetic/mod.rs:26:1
|
26 | compile_error!("You can choose only one bigint implementation. See crate features.");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error[E0252]: the name `BigInt` is defined multiple times
--> /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/curv-kzen-0.9.0/src/arithmetic/mod.rs:37:9
|
31 | pub use big_gmp::BigInt;
| --------------- previous import of the type `BigInt` here
...
37 | pub use big_native::BigInt;
| ^^^^^^^^^^^^^^^^^^ `BigInt` reimported here
|
= note: `BigInt` must be defined only once in the type namespace of this module
help: you can use `as` to change the binding name of the import
|
37 | pub use big_native::BigInt as OtherBigInt;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
As I have disabled default-features
and configured for num-bigint
I wonder why this conflict is happening, does default-features
not apply for transitive dependencies?
@tmpfs Note that if you have other dependencies that depend on curv
(eg. multi-party-ecdsa
), then you need to turn off default features for them too, ie: multi-party-ecdsa = { git = "...", tag = "...", default-features = false }
.
The reason why you see this message is that some of your dependencies enforces curv/rust-gmp-kzen
feature. You may use cargo tree
tool to find out which one
Thanks for the tip @survived, it seems like all I needed was to set default-features
for the multi-party-ecdsa
dependency to get it to compile.
Using these dependencies:
[dependencies]
multi-party-ecdsa = {path = "../../git/multi-party-ecdsa", default-features = false}
getrandom = {version = "0.2", features = ["js"]}
curv-kzen = {version = "0.9", features = ["num-bigint"], default-features = false}
Running cargo check --target wasm32-unknown-unknown
is working :+1:
@survived, having some issues getting entropy in WASM, I wonder if you have any ideas.
First I hit this error calling Keys::create()
in the WASM module:
panicked at 'could not initialize thread_rng: All entropy sources failed (permanently unavailable); cause: OsRng: support for wasm32 requires emscripten, stdweb or wasm-bindgen (permanently unavailable)', /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/rand-0.6.5/src/rngs/thread.rs:80:17
So I added the rand
dependency with the wasm-bindgen
feature (note that the wasm-bindgen
feature no longer exists in rand@0.8
):
rand = { version="0.6.5", features = ["wasm-bindgen"] }
Which yields:
panicked at 'Error: getrandom: this target is not supported', /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/rand_core-0.5.1/src/os.rs:63:13
Any pointers on how I should set up the RNG for WASM?
Full Cargo.toml
is here.
I have been looking into the rand
errors a bit and it appears to be something to do with the transitive dependency chain configuration but I am not sure exactly where yet. I did a quick test depending upon rand@0.8
with getrandom
using the js
feature and with rand@0.6.5
with the wasm-bindgen
feature and they both work fine compiling to WASM as direct dependencies.
When I look at the rand
dependency for multi-party-ecdsa
there are 4 versions:
rand:0.3.23
rand:0.4.6
rand:0.6.5
rand:0.7.3
I noticed that the 0.6.5
version is required by curv
(as rand_legacy
) because secp256k1
depends on rand ^0.6
. Which means that upgrading rand
is not going to be feasible. Is there some cargo-fu I am missing that would get us past this error?
I managed to get it working by downgrading getrandom
from:
getrandom = {version = "0.2", features = ["js"]}
To 0.1.16
with the wasm-bindgen
feature:
getrandom = {version = "0.1.16", features = ["wasm-bindgen"]}
Phew!
@survived and anyone else following this thread I found an interesting problem. For context, I am trying to port the gg18 example to WASM using a websocket server backend, the code is here.
I got to the point of verifying correctness of round 2 answers during key generation and hit this error:
panicked at 'The global thread pool has not been initialized.: ThreadPoolBuildError { kind: IOError(Error { kind: Unsupported, message: "operation not supported on this platform" }) }', /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/rayon-core-1.9.1/src/registry.rs:170:10
Using cargo tree
I noticed that the crates centipede
, kzen-paillier
and zk-paillier
use rayon
so I figured I would use wasm-bindgen-rayon to enable threads and rayon
support in webassembly.
Once I managed to integrate with wasm-bindgen-rayon
(which involved an upgrade to webpack@5 to get it to work!) I hit a new error:
TypeError: Crypto.getRandomValues: Argument 1 can't be a SharedArrayBuffer or an ArrayBufferView backed by a SharedArrayBuffer
Which leads to this issue about Crypto.getRandomValues()
not supporting SharedArrayBuffer
.
So I think it is fair to say that adding threads to webassembly leads to the call to Crypto.getRandomValues()
being backed by a SharedArrayBuffer
which makes sense as that is what would be used for communication between threads.
I think that supporting SharedArrayBuffer
in Crypto.getRandomValues()
is a non-starter as it would be possible for other threads to modify the buffer before it has been filled leading to a data race.
Would it be feasible to introduce a feature flag for those crates that disabled use of rayon
? Is this practical?
Maybe we could have a multi-threaded
feature that is enabled by default and then in webassembly builds we set default-features
to false
for those dependencies?
Happy to do the work to land this if you think it is a good approach :pray:
So this gets a bit strange, I have learnt that the multi-threaded SharedArrayBuffer
issue was fixed in getrandom
by forcing usage of an Uint8Array
for the call to Crypto.getRandomValues()
, see this PR and was backported to getrandom@0.1
here.
So I tried to reproduce this using rand@0.6.5
, rayon
and wasm-bindgen-rayon
and got the same error, the code is in the rayon-0.6.5 branch.
Which ultimately led me to find out that rand@0.6.5 does not use getrandom which is why that fix is not present. The code that is actually executing is here: https://github.com/rust-random/rand/blob/0.6.5/rand_os/src/wasm32_bindgen.rs.
And because secp256k1
depends upon rand ^0.6
we have a bit of a problem.
@survived, some guidance on the best way forward here would be useful. Should we try to get secp256k1
to upgrade to rand@0.8
so we can lose the rand_legacy
usage in curv
?
Looks like we need to wait for this PR (related issue) to land and then we should be good.
I also wanted to update pairing-plus
to use the latest version so we don't have 3 different versions of rand
in the dependency tree, hopefully this PR will get merged too.
Once they land we can update curv
to use rand@0.8
and then I think we should be able to move past this error :pray:
I'm waiting for secp256k1
library update too that will bump rand
dependency 🙂 This will happen eventually.
Regarding pairing-plus
issue, I believe we can eliminate this one by slightly modifying curv
crate. We could (and should) make every curve support optional, so users could turn off particular curve implementation if it's not relevant for them (and even causes problems). In your case, you only need secp256k1
curve. What it requires is to simply add secp256k1-curve
, seck256r1-curve
, [...] features that would enable required dependencies
(pairing-plus
library provides implementation of BLS curve and nothing more)
Regarding
pairing-plus
issue, I believe we can eliminate this one by slightly modifyingcurv
crate. We could (and should) make every curve support optional, so users could turn off particular curve implementation if it's not relevant for them (and even causes problems). In your case, you only needsecp256k1
curve. What it requires is to simply addsecp256k1-curve
,seck256r1-curve
, [...] features that would enable required dependencies
I like the sound of this, makes a lot of sense. Happy to help out if needed!
For anyone that wants to try building for wasm32-unknown-unknown
now before we get the rand
updates here is a little hack to get past this particular problem:
// Temporary hack for getRandomValues() error
const getRandomValues = crypto.getRandomValues;
crypto.getRandomValues = function(buffer) {
const array = new Uint8Array(buffer);
const value = getRandomValues.call(crypto, array);
buffer.set(value);
return buffer;
}
Hi! I was just about to go down this path, and wanted to ask. @tmpfs were you able to compile for wasm? Were those conflicts solved or did you have to keep on using the hacked you mentioned?
Thanks!
@aon, yes it compiles, the hacks are still necessary due to the use of rayon/threads in some of the dependencies.
Ideally, multi-threaded operation should be configurable, see #154.
@tmpfs great! Well I'll try to make it work following this thread. If you happen to have somewhere a repo where this is implemented that'd be awesome. Thanks for the help!
@tmpfs Thanks!
I have been investigating using multi-party ECDSA in WASM and wanted to know if there are plans to support the
wasm32-unknown-unknown
target so we could use this library from the browser?I have explored your emerald-city library and have used it to put together a very simple demo of using WASM (via
wasm-pack
) but would prefer to use something that is ready for production and has been audited, like this library or threshold-signatures.All my attempts to compile for
wasm32-unknown-unknown
have failed so far and when I tried withmulti-party-ecdsa
I get an error (that is now quite familiar!):Which is due to the deprecated
rustc-serialize
library being used byrust-crypto
. If we look at the code there is no implementation for theunknown
target OS.We can't fix
rustc-serialize
so the solution I would imagine is to remove the dependency onrust-crypto
which I believe is now deprecated in favour ofRustCrypto
.Would it be possible to merge the experimental work in
emerald-city
so we can build this library for WASM?Thanks :pray: