refactoring(circuits): refactor the circuit package to improve its efficiency #832
Conversation
ctrlc03
changed the title
ctrlc03
force-pushed
the
refactoring/circuits
branch
from
53e7607 to
f451d18
Compare
ctrlc03
force-pushed
the
refactoring/circuits
branch
from
19365ec to
761a630
Compare
0xmad
changed the title
|
@ctrlc03 could you please fix conflicts? |
ctrlc03
force-pushed
the
refactoring/circuits
branch
from
761a630 to
c80ebd3
Compare
It's up to date now :) |
ctrlc03
force-pushed
the
refactoring/circuits
branch
4 times, most recently
from
5d92fcb to
a527139
Compare
ctrlc03
force-pushed
the
refactoring/circuits
branch
from
a527139 to
074b97c
Compare
✅ Deploy Preview for maci-typedoc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
ctrlc03
force-pushed
the
refactoring/circuits
branch
3 times, most recently
from
558bfc5 to
f4584de
Compare
ctrlc03
force-pushed
the
refactoring/circuits
branch
2 times, most recently
from
1521b00 to
66b7c91
Compare
|
|
||
| The circuits are used by the coordinator (the prover) to prove that they have correctly processed a batch of messages and tallied the votes correctly. This happens after a Poll has completed, and the coordinator has merged the state and message trees. The coordinator then generates a proof for each batch of messages, and submits them to the contract. The contract then verifies the proofs and updates the commitments on chain. | ||
|
|
||
| > Please note that in order to function, the circuit in charge of processing the messages requires the coordinator's private key, hence a proof for this circuit can only be generated by the coordinator. The private key is needed in order to generate the ECDH shared key used to decrypt the messages. |
There was a problem hiding this comment.
| > Please note that in order to function, the circuit in charge of processing the messages requires the coordinator's private key, hence a proof for this circuit can only be generated by the coordinator. The private key is needed in order to generate the ECDH shared key used to decrypt the messages. | |
| Please note that in order to function, the circuit in charge of processing the messages requires the coordinator's private key, hence a proof for this circuit can only be generated by the coordinator. The private key is needed in order to generate the ECDH shared key used to decrypt the messages. |
Why the special quote formatting here?
There was a problem hiding this comment.
Like an important note or something
|
|
||
| MACI uses [GROTH16](https://eprint.iacr.org/2016/260.pdf) as its proving system. GROTH16 is a zk-SNARK proving system that allows for the generation of proofs that are small and fast to verify. | ||
|
|
||
| ## How are the circuits used |
There was a problem hiding this comment.
| ## How are the circuits used | |
| ## How are the circuits used? |
|
|
||
|  | ||
|
|
||
| ## How the circuits work |
There was a problem hiding this comment.
| ## How the circuits work | |
| ## How do the circuits work? |
Keeping the consistency of question format
|
|
||
| ## How do the Circuits fit in a voting round | ||
|
|
||
|  |
There was a problem hiding this comment.
These flow charts are awesome! 🤩
I have some improvement suggestions but I think that's best saved for a future PR
There was a problem hiding this comment.
Feel free to edit the file yourself! included the source in static/img/circuits which you can load into excalidraw
| state and ballot trees according to the contents of said messages. | ||
| 2. `TallyVotes.circom`, which counts votes from users' ballots, batch by batch. | ||
| 3. `Subsidy.circom`, which implements [pairwise subsidy](https://hackmd.io/@chaosma/H1_9xmT2K) | ||
| 1. [`ProcessMessages.circom`](https://github.com/privacy-scaling-explorations/maci/blob/dev/circuits/circom/processMessages.circom), which takes a batch of encrypted messages, decrypts them, and generates a proof that the coordinator's local processing was performed correctly. |
There was a problem hiding this comment.
Nitpick but I think this should be ProcessMessages (circuit name, PascalCase) or processMessages.circom (file name, camelCase)
Naming convention question: should Circom files be camelCase?
There was a problem hiding this comment.
Might just be simpler to update the filenames to be PascalCase to match the circuit names? (not in this PR)
There was a problem hiding this comment.
Not sure about conventions, here is how circomlib names them (all lower it seems)
|
|
||
| ### Message processing (processMessages) | ||
|
|
||
| This circuit allows the coordinator to prove that they have correctly processed each message in reverse order, in a consecutive batch of 5 ^ msgBatchDepth messages to the respective state leaf within the state tree. Coordinators would use this circuit to prove correct execution at the end of each Poll. The pre-requisites for this are: |
There was a problem hiding this comment.
I think worth explaining why this happens in batches (because of gas limit, correct?)
There was a problem hiding this comment.
Well AFAIU we do it in batches because of circuits' constraints. If we were to process all messages in one go, the constraints would be so many that it would take forever and the circuit's artifacts (zkeys, r1cs, etc) would be MASSIVE (tens of GB)
| ### Vote tallying | ||
| #### Inputs | ||
|
|
||
| | Input signal | Description | |
There was a problem hiding this comment.
This table is great, thanks for doing this
There was a problem hiding this comment.
oh no can't take props for that, it's a copy pasta from here: https://maci.pse.dev/docs/spec 🙂
| 7. That each message in `msgs` exists in the message tree | ||
| 8. That after decrypting and applying each message, in reverse order, to the corresponding state and ballot leaves, the new state root, new ballot root, and `newSbSalt` are the preimage to `newSbCommitment` | ||
|
|
||
| ### Tally Votes |
There was a problem hiding this comment.
Explanation would be nice here (but as I'm reviewing I'm thinking a lot of these documentation changes could be done in future PRs)
|
|
||
| ### Subsisdy (Optional) | ||
|
|
||
| The subsidy circuit is used to implement pairwise subsidy. This is a technique that can be used to detect voters collusion. It currently is not optimized for production and the team will work on a more efficient implementation in the future. |
There was a problem hiding this comment.
| The subsidy circuit is used to implement pairwise subsidy. This is a technique that can be used to detect voters collusion. It currently is not optimized for production and the team will work on a more efficient implementation in the future. | |
| This circuit is an optional feature - it's not required for MACI to function. The subsidy circuit is used to implement pairwise subsidy. This is a technique that can be used to detect voters collusion. It currently is not optimized for production and the team will work on a more efficient implementation in the future. |
There was a problem hiding this comment.
Great work! 👏
I left a bunch of suggestions, I think entirely related to the documentation.
Given the conversation today around prioritizing the package refactor work (vs. documentation), I'm ok with addressing documentation improvements in future PRs (which I'd be happy to help with).
|
|
||
| Install dependencies for and `zkey-manager`: | ||
|
|
||
| ```bash | ||
| sudo apt-get install libgmp-dev nlohmann-json3-dev nasm g++ | ||
| ``` | ||
|
|
||
| > Note that on a arm macbook you won't need the above. However, you will not be able to compile the c++ witness generator. |
There was a problem hiding this comment.
| > Note that on a arm macbook you won't need the above. However, you will not be able to compile the c++ witness generator. | |
| > Note that on an ARM MacBook you won't need the above. However, you will not be able to compile the C++ witness generator. |
…lit into utilities vs exported functions
…and fix dir removal error
…uits and related scripts
…cription to the documentation, separate snark proof generation/verification to a new file and export all from index
ctrlc03
force-pushed
the
refactoring/circuits
branch
from
386e488 to
a849486
Compare
Refactor the package: