filecoin-project / bellperson

zk-SNARK library
Other
190 stars 120 forks source link

fix: remove wasm feature #281

Closed vmx closed 2 years ago

vmx commented 2 years ago

The wasm feature was introduced as getrandom needs the js feature in order to compile for wasm32-unknown-unknown. A better fix is to disable the default features of rand instead. This way the wasm feature is no longer needed.

For compiling on WebAssembly also the memmap feature was introduced. Remove that feature and replace it with target architecture flags, that exclude memmap related functions when compiled for wasm.

This lowers the maintenance burden as WebAssembly is no longer a special case.

Thanks @samuelburnham for the code that made the removal of memmap feature possible.

BREAKING CHANGE: wasm and memmap features are removed

If you're targetting WASM and using bellperson as a dependency, prior to this change you'd have included it as:

bellperson = { version = "0.23", default-features = false, features = ["wasm"] }

Now you can instead just rely on the default features:

bellperson = "0.23"

BREAKING CHANGE: resolver v2 is used

In order to make things compile nicely for the wasm target, the resolver was changed to version 2.


This should be merged once we decide that we want to do a new breaking change version.

vmx commented 2 years ago

@Anderssorby you might be interested in a review as you did the original WASM support.

@samuelburnham you might be interested in a review as you recently opened a PR for WASM on a related project.

samuelburnham commented 2 years ago

I think this is a great change, however in https://github.com/lurk-lang/bellperson/tree/sb/wasm I go further to remove the memmap feature entirely and instead conditionally compile on the wasm32 target (which could be extended with other targets in the future). This would simplify downstream imports of this crate so they don't have to disable default features.

vmx commented 2 years ago

Thanks @samuelburnham, that's a great recommendation. I cherry-picked your change and incorporated it. Now it's even cleaner.

lemmih commented 2 years ago

Who needs to sign off on this? @shawnrader @cryptonemo ?

vmx commented 2 years ago

@lemmih sorry for the delay and thanks for the ping.