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

Add nargo command to both prove and verify #719

Closed
jfecher opened this issue Jan 31, 2023 · 6 comments · Fixed by #737
Closed

Add nargo command to both prove and verify #719

jfecher opened this issue Jan 31, 2023 · 6 comments · Fixed by #737
Labels
enhancement New feature or request

Comments

@jfecher
Copy link
Contributor

jfecher commented Jan 31, 2023

Problem

Proving and immediately verifying afterward is a common usecase but has no single command to do both. It would also be nice if there were an option to do so without writing to the file system between the two steps.

Solution

Add a nargo run command to prove and verify.

Alternatives considered

Additional context

@jfecher jfecher added the enhancement New feature or request label Jan 31, 2023
@kevaundray
Copy link
Contributor

@TomAFrench what are your thoughts on this?

@TomAFrench
Copy link
Member

TomAFrench commented Jan 31, 2023

I'd question whether this is/should be a common action for users other than us. Noir devs trust that Nargo/backends are implemented correctly and so will properly create a valid proof as long as that the witness for their circuit can be solved, after that point it's out of their hands. Anything after solving the witness is wasted CPU time then surely unless you're actually creating a proof which will be used for anything other than testing? (#626)

It feels like we'd be the main users of this action for e2e testing (as we'll be mucking around in the guts of nargo and so could break how we interact with the backend, etc.) so this might be better to have as a flag on nargo prove rather than a full-fledged command i.e. nargo prove --checked.

Happy to be corrected if I'm missing something though.

@phated
Copy link
Contributor

phated commented Jan 31, 2023

I might be mistaken, but I believe snarkjs has fullProve which is a prove and verify and everyone I've seen uses this (Dark Forest, etc) so I think it is pretty common 🤔

@TomAFrench
Copy link
Member

TomAFrench commented Jan 31, 2023

I might be mistaken, but I believe snarkjs has fullProve which is a prove and verify and everyone I've seen uses this (Dark Forest, etc) so I think it is pretty common thinking

https://github.com/iden3/snarkjs/blob/b478e44e9ffb6270b51e24c38158730a39b27b02/cli.js#L1059-L1077

Looks like their fullProve is roughly equivalent to our nargo prove (i.e. solve witness and prove) and doesn't verify. Their prove assumes that you already have a witness for the circuit.

@phated
Copy link
Contributor

phated commented Jan 31, 2023

👍 That's what I get for not checking before I opened my mouth. Thanks for looking.

@kevaundray
Copy link
Contributor

I'd question whether this is/should be a common action for users other than us. Noir devs trust that Nargo/backends are implemented correctly and so will properly create a valid proof as long as that the witness for their circuit can be solved, after that point it's out of their hands. Anything after solving the witness is wasted CPU time then surely unless you're actually creating a proof which will be used for anything other than testing? (#626)

It feels like we'd be the main users of this action for e2e testing (as we'll be mucking around in the guts of nargo and so could break how we interact with the backend, etc.) so this might be better to have as a flag on nargo prove rather than a full-fledged command i.e. nargo prove --checked.

Happy to be corrected if I'm missing something though.

The --checked flag seems like a good trade-off, I'd imagine that we move towards using @jfecher 's testing framework as time goes by.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants