multi-party-ecdsa icon indicating copy to clipboard operation
multi-party-ecdsa copied to clipboard

Support for `wasm32-unknown-unknown` target?

Open tmpfs opened this issue 4 years ago • 20 comments

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 :pray:

tmpfs avatar Sep 07 '21 05:09 tmpfs

Hi ! We plan to remove rust-crypto . meanwhile : see if this helps : https://github.com/ZenGo-X/multi-party-ecdsa/pull/137

omershlo avatar Sep 07 '21 06:09 omershlo

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.

survived avatar Sep 07 '21 06:09 survived

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?

tmpfs avatar Sep 08 '21 00:09 tmpfs

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

elichai avatar Sep 12 '21 09:09 elichai

Hey,

I noticed that https://github.com/ZenGo-X/curv/pull/137 has landed in [email protected] 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:

tmpfs avatar Oct 04 '21 06:10 tmpfs

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 avatar Oct 04 '21 06:10 survived

@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 avatar Oct 28 '21 01:10 tmpfs

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

elichai avatar Oct 28 '21 08:10 elichai

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 avatar Oct 29 '21 04:10 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?

tmpfs avatar Oct 29 '21 04:10 tmpfs

@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

survived avatar Oct 31 '21 07:10 survived

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:

tmpfs avatar Nov 02 '21 01:11 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 [email protected]):

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 avatar Nov 26 '21 01:11 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 [email protected] with getrandom using the js feature and with [email protected] 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 avatar Nov 29 '21 04:11 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 avatar Nov 29 '21 05:11 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 :pray:

tmpfs avatar Dec 04 '21 01:12 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 [email protected] here.

So I tried to reproduce this using [email protected], 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 [email protected] 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 [email protected] so we can lose the rand_legacy usage in curv?

tmpfs avatar Dec 06 '21 06:12 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 [email protected] and then I think we should be able to move past this error :pray:

tmpfs avatar Dec 07 '21 04:12 tmpfs

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)

survived avatar Dec 07 '21 07:12 survived

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

tmpfs avatar Dec 08 '21 00:12 tmpfs

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 avatar Dec 23 '22 14:12 aon

@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 avatar Dec 24 '22 01:12 tmpfs

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

aon avatar Dec 24 '22 17:12 aon

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

tmpfs avatar Dec 25 '22 01:12 tmpfs

@tmpfs Thanks!

aon avatar Dec 26 '22 16:12 aon