refactor: remove build.rs and integrate zkey downloading into utils #11

vivianjeng · 2025-05-27T12:41:45Z

Description

Problems:

The mobile app cannot get the relative ZKEY path
set depth in building time is not flexible, and most of the use cases we just need to import the crate instead of cloning the repo

Changes:

Deleted the build.rs file, which previously handled downloading semaphore artifacts.
Introduced a new download_zkey function in utils.rs to manage zkey downloads.
Updated proof.rs to utilize the new download_zkey function for fetching zkeys dynamically based on tree depth.
Added a new witness.rs file to handle witness dispatching based on tree depth.
Updated Cargo.toml to remove rust-witness dependency and adjust circom-prover to use the latest version from the main branch.
Added multiple witness graph binary files for different depths.

Related Issue(s)

According to the benchmark result zkmopro/mopro#427
and the compare the different usage of witness calculator
I choose the circom-witnesscalc as witness calculator and arkworks as proving backend

Other information

Checklist

I have read and understand the contributor guidelines and code of conduct.
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have run yarn style without getting any errors
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Important

We do not accept minor grammatical fixes (e.g., correcting typos, rewording sentences) unless they significantly improve clarity in technical documentation. These contributions, while appreciated, are not a priority for merging. If there is a grammatical error feel free to message the team.

- Deleted the `build.rs` file, which previously handled downloading semaphore artifacts. - Introduced a new `download_zkey` function in `utils.rs` to manage zkey downloads. - Updated `proof.rs` to utilize the new `download_zkey` function for fetching zkeys dynamically based on tree depth. - Added a new `witness.rs` file to handle witness dispatching based on tree depth. - Updated `Cargo.toml` to remove `rust-witness` dependency and adjust `circom-prover` to use the latest version from the main branch. - Added multiple witness graph binary files for different depths.

- Added a step to install the `protobuf-compiler` in the GitHub Actions workflow. - Refactored import order in `identity.rs` for consistency. - Adjusted import order in `group.rs` for improved readability.

vivianjeng · 2025-05-27T14:46:25Z

Cargo.toml

+circom-prover = { git = "https://github.com/zkmopro/mopro.git", branch = "main", features = [
+    "circom-witnesscalc",
+] }


we haven't published this
and circom-witnesscalc also hasn't been published by iden3
we will ask them to publish one so that we can publish a new circom-prover

vivianjeng · 2025-05-27T15:33:31Z

src/witness.rs

+        1_u16 => {
+            graph!(semaphore1, "../witness_graph/semaphore-1.bin");
+            semaphore1_witness


I think this can be improved, but haven't figured out how
ideally we can download the .bin and use it
but just some type error

KimiWu123 · 2025-05-28T05:59:01Z

set depth in building time is not flexible, and most of the use cases we just need to import the crate instead of cloning the repo

I have a different perspective on the statement that "set depth in building time is not flexible". In our case, I believe that your design offers great flexibility. However, for an application, developers typically need to specify the depth before production. This means that they only require a set depth for their application, and may not necessarily need flexibility. Additionally, the current design restricts users from generating a proof offline for their initial use.

@vplasencia , you have more experience with Semaphore and would like to hear your opinions.

KimiWu123 · 2025-05-28T05:26:18Z

Cargo.toml

@@ -25,16 +27,10 @@ serde = { version = "1", features = ["derive"], optional = true }
 serde_json = "1"

 # circom-prover
-rust-witness = "0.1"
-circom-prover = "=0.1.1"
+circom-prover = { git = "https://github.com/zkmopro/mopro.git", branch = "main", features = [


You don't need to specify the branch here if it's the main branch.

KimiWu123 · 2025-05-28T05:28:10Z

README.md

 ## 🛠 Install

 Clone this repository:

+```sh


We should either use "sh" or "bash".

vivianjeng · 2025-05-28T07:53:37Z

I have a different perspective on the statement that "set depth in building time is not flexible". In our case, I believe that your design offers great flexibility. However, for an application, developers typically need to specify the depth before production. This means that they only require a set depth for their application, and may not necessarily need flexibility. Additionally, the current design restricts users from generating a proof offline for their initial use.

@KimiWu123
but typically the way people use a rust crate is like

cargo add semaphore-rs

(like npm install semaphore)
then it is hard to set the depth

worldcoin use feature flags to specify depth, but it only provides 3 kind of depth

and if we choose to use feature flags
it is also not possible for users to set depth in mobile dev environment
there is no feature flags in swift, kotlin, react native and flutter
if in the future we will provide mobile native SDKs, we still need to provide the flexibility to choose from different depths

so ideally the best way is not setting depth
but download the corresponding data (zkey, .bin) when using the app
and it only needs to download once
so we just ask users to be online when the first time they download the app and connect to the internet
and the future proving can be offline

KimiWu123 · 2025-05-28T09:06:26Z

worldcoin use feature flags to specify depth, but it only provides 3 kind of depth
and if we choose to use feature flags

We don't need feature flags; we used env var, SEMAPHORE_DEPTH, at build time in the original implementation.

so ideally the best way is not setting depth
but download the corresponding data (zkey, .bin) when using the app
and it only needs to download once
so we just ask users to be online when the first time they download the app and connect to the internet
and the future proving can be offline

I know we only need to download once, and usually, when users download the app, they have internet for sure.
My point is that users get the zkey only by generating a proof. That means users have to generate a proof after they download the app (to guarantee that they can get what they need), and this design lacks flexibility for app developers. I think we should decouple these two operations (download the zkey and proof generation). If we provide a init function for app developers to download what we need (downloading zkey in our case), then I think it's good enough for me.

Two minor considerations:

On a PC, files in the temporary folder may be deleted at any time, as the temp folder is not intended for storing long-lasting files. I'm assuming it's the same case in the mobile.
We should try to avoid uploading binary files to GitHub if possible. But if it's unavoidable, then I'm fine with it.

vivianjeng · 2025-05-28T10:08:44Z

We don't need feature flags; we used env var, SEMAPHORE_DEPTH, at build time in the original implementation.

but SEMAPHORE_DEPTH cannot be set when using swift, kotlin,...
or we will need to build 32 packages for different depths
e.g. one SEMAPHORE_DEPTH one swift package
for mobile devs (or rust devs), ideally they don't need to know they need to setup the SEMAPHORE_DEPTH when they are building
and for mobile devs, they don't even need to touch Rust code, they just install through like pod 'Semaphore' without cloning any Rust code and run cargo build

If we provide a init function for app developers to download what we need (downloading zkey in our case)

I am good with a init function but actually it can be abstracted within the current download_zkey

On a PC, files in the temporary folder may be deleted at any time, as the temp folder is not intended for storing long-lasting files. I'm assuming it's the same case in the mobile.

I think the mobile temp folder is related to each app
so if you don't delete the mobile app, the file won't be cleared
(I think it might be also a problem if zkeys are updated, so maybe we can set every zkey name with different version)

We should try to avoid uploading binary files to GitHub if possible. But if it's unavoidable, then I'm fine with it.

we can make sure it works then improve this 🙏🏻

vivianjeng added 4 commits May 27, 2025 20:39

chore: update CI workflow and refactor imports

7c31e2f

- Added a step to install the `protobuf-compiler` in the GitHub Actions workflow. - Refactored import order in `identity.rs` for consistency. - Adjusted import order in `group.rs` for improved readability.

chore: lint

6f524a7

fix: fix clippy, update readme

3fc1753

vivianjeng commented May 27, 2025

View reviewed changes

vivianjeng added 2 commits May 27, 2025 22:48

chore: remove debug messages

e4ba4f6

fix: fix clippy

8a91909

vivianjeng commented May 27, 2025

View reviewed changes

vivianjeng marked this pull request as ready for review May 27, 2025 15:35

KimiWu123 reviewed May 28, 2025

View reviewed changes

vivianjeng requested a review from vplasencia May 28, 2025 11:24

KimiWu123 self-requested a review May 29, 2025 07:58

KimiWu123 approved these changes May 29, 2025

View reviewed changes

vplasencia approved these changes May 29, 2025

View reviewed changes

vivianjeng merged commit 76f420e into main May 29, 2025
2 checks passed

vivianjeng deleted the depth-features branch May 29, 2025 13:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: remove build.rs and integrate zkey downloading into utils #11

refactor: remove build.rs and integrate zkey downloading into utils #11

Uh oh!

vivianjeng commented May 27, 2025 •

edited

Loading

Uh oh!

vivianjeng May 27, 2025

Uh oh!

vivianjeng May 27, 2025

Uh oh!

KimiWu123 commented May 28, 2025

Uh oh!

KimiWu123 May 28, 2025

Uh oh!

KimiWu123 May 28, 2025

Uh oh!

vivianjeng commented May 28, 2025 •

edited

Loading

Uh oh!

KimiWu123 commented May 28, 2025 •

edited

Loading

Uh oh!

vivianjeng commented May 28, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

refactor: remove build.rs and integrate zkey downloading into utils #11

refactor: remove build.rs and integrate zkey downloading into utils #11

Uh oh!

Conversation

vivianjeng commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

Other information

Checklist

Uh oh!

vivianjeng May 27, 2025

Choose a reason for hiding this comment

Uh oh!

vivianjeng May 27, 2025

Choose a reason for hiding this comment

Uh oh!

KimiWu123 commented May 28, 2025

Uh oh!

KimiWu123 May 28, 2025

Choose a reason for hiding this comment

Uh oh!

KimiWu123 May 28, 2025

Choose a reason for hiding this comment

Uh oh!

vivianjeng commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KimiWu123 commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vivianjeng commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vivianjeng commented May 27, 2025 •

edited

Loading

vivianjeng commented May 28, 2025 •

edited

Loading

KimiWu123 commented May 28, 2025 •

edited

Loading

vivianjeng commented May 28, 2025 •

edited

Loading