Support for wasm32-unknown-unknown target?

[tmpfs]

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 with multi-party-ecdsa I get an error (that is now quite familiar!):

error[E0046]: not all trait items implemented, missing: `encode`
    --> /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/rustc-serialize-0.3.24/src/serialize.rs:1358:1
     |
853  |     fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error>;
     |     ---------------------------------------------------------------- `encode` from trait
...
1358 | impl Encodable for path::Path {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `encode` in implementation

error[E0046]: not all trait items implemented, missing: `decode`
    --> /home/muji/.cargo/registry/src/github.com-1ecc6299db9ec823/rustc-serialize-0.3.24/src/serialize.rs:1382:1
     |
904  |     fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error>;
     |     ----------------------------------------------------------- `decode` from trait
...
1382 | impl Decodable for path::PathBuf {
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `decode` in implementation

Which is due to the deprecated rustc-serialize library being used by rust-crypto. If we look at the code there is no implementation for the unknown target OS.

We can't fix rustc-serialize so the solution I would imagine is to remove the dependency on rust-crypto which I believe is now deprecated in favour of RustCrypto.

Would it be possible to merge the experimental work in emerald-city so we can build this library for WASM?

Thanks 🙏

[omershlo]

Hi ! We plan to remove rust-crypto .
meanwhile : see if this helps : #137

[survived]

rust-crypto dependency bothers us a lot, so we're removing it: ZenGo-X/curv#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.

[tmpfs]

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?

[elichai]

@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 )

[tmpfs]

Hey,

I noticed that ZenGo-X/curv#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 🙏

[survived]

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 ❤️

[tmpfs]

@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 #144 is ready for review 👍

[elichai]

@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.

[tmpfs]

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?

[tmpfs]

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?

[survived]

@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

[tmpfs]

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 👍

[tmpfs]

@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.

[tmpfs]

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?

[tmpfs]

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!

[tmpfs]

@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 🙏

[tmpfs]

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?

[tmpfs]

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 🙏

[survived]

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)

[tmpfs]

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

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;
}

[aon]

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!

[tmpfs]

@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.

[aon]

@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]

@aon, see https://github.com/LavaMoat/tss-snap/tree/main/packages/wasm.

[aon]

@tmpfs Thanks!