Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the circuit #7

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Refactor the circuit #7

merged 2 commits into from
Mar 13, 2024

Conversation

bkomuves
Copy link
Collaborator

This is a simple refactoring to separate the codex-specific parts from the more generic, reusable parts of the proof circuit (as agreed before).

Namely, I just moved the .circom files into 3 subdirectories:

  • circuit/codex for the proof circuit itself
  • circuit/poseidon2 for the hash function
  • circuit/lib for the general purpose templates and functions

I think I updated the github workflow .yaml file accordingly, but haven't tested that one (not sure how you that?)

Note that my understanding is that the github workflow in this repository should be only used for testing purposes, so you will need to update the "real" workflow for the Codex build system too (simply because the .circom files are in different directories now, nothing else should have changed).

@AuHau
Copy link
Member

AuHau commented Mar 12, 2024

I think I updated the github workflow .yaml file accordingly, but haven't tested that one (not sure how you that?)

image

;-)

Note that my understanding is that the github workflow in this repository should be only used for testing purposes, so you will need to update the "real" workflow for the Codex build system too (simply because the .circom files are in different directories now, nothing else should have changed).

Well, the Workflow is used for generating the circuit assets that is used in Codex for testing environments - unit tests, integration tests, dist. tests. Maybe even for testnet? Not sure about that, but most probably at least for the beginning.
There were talks about moving some of this part to the Codex build pipeline, although I am missing the reasoning for that. So currently this is the only place where the circuit assets are generated. Then they are distributed manually to the appropriate places.

circuit/README.md Outdated Show resolved Hide resolved
Co-authored-by: markspanbroek <mark@spanbroek.net>
@bkomuves
Copy link
Collaborator Author

Well, the Workflow is used for generating the circuit assets that is used in Codex for testing environments - unit tests, integration tests, dist. tests. Maybe even for testnet? Not sure about that, but most probably at least for the beginning.

So I agree to use this for testing purposes, but disagree for using it for the testnet. One reason is that parameters are as of now hardcoded, and for testing for you most probably want different parameters (in particular, the number of samples is currently 5 here, but in the real system we aim for something closer to 100 - but that's much slower to run).

Another thing is that this does more things than what is necessary for the build - for example it generates fake inputs, and creates a proof - does not verify it though :) also it installs Nim just because this.

Can you have several workflows for one repo? Maybe that could be an easy way out :)

@AuHau
Copy link
Member

AuHau commented Mar 12, 2024

parameters are as of now hardcoded

No, they are not ;-) All significant parameters are possible to configure when running the flow.

disagree for using it for the testnet

Yeah, somewhat agree. I think especially for when we will rollout the incentivized testnet setup, we should have proper ceremony in order for somebody not to game it with "fake" proofs. But IMHO for first (internal?) versions it should be fine, right?

Another thing is that this does more things than what is necessary for the build - for example it generates fake inputs, and creates a proof - does not verify it though :) also it installs Nim just because this.

Yeah, we need this things for the testing in contracts repo so that is why they are there...

@benbierens
Copy link
Contributor

Adding my cents:
I think because this repo is 'codex-storage-proofs-circuits' and it is in the codex-storage org, it's completely fine that there is codex-specific code and workflows here. Additionally, I think it's a good idea to generate codex's prove assets in a separate repository from the main codebase. If there are non-codex parts that can be extracted and shared with the world, great! But the name of a (new?) repository should reflect this.

I think that for our 'internal' first test-net, we should probably generate a circuit with realist (as much as we know what that means at this stage in the game!) parameters. That means it'll be slow. And we need to know how slow. This means the asset files will be larger. And we need to know how large.

@bkomuves
Copy link
Collaborator Author

@benbierens What I really meant is that there should be at least two different workflows, one for testing purposes, and one as part of building codex. These should do slightly different things with different parameter settings and result in different artifacts.

@bkomuves bkomuves merged commit 61789ca into master Mar 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants