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

chore(poseidon): add verification key json files #125

Merged
merged 7 commits into from
Sep 23, 2024

Conversation

sripwoud
Copy link
Member

@sripwoud sripwoud commented Jul 22, 2024

Generated with:

  1. create options.json:
    {
       "path/to/zk-kit.circom/packages/poseidon-proof/circomkit.json": {
           "circuit": "poseidon-proof",
           "paramsList": [["1"],["2"],["3"],["4"],["5"],["6"],["7"],["8"],["9"],["10"],["11"],["12"],["13"],["14"],["15"],["16"]]
        }
     }
```commandline
pnpm add -g @zk-kit/artifacts-cli
cd $(mktemp -d)
snarkli gb path/to/options.json $(pwd)
circuit=poseidon
for i in {1..16};do mv -f "$circuit-proof-$i/$circuit-proof/groth16_vkey.json" "/path/to/snark-artifacts/packages/$circuit/circuit-$i.json";done
for i in {1..16};do mv -f "$circuit-proof-$i/$circuit-proof/groth16_pkey.zkey" "/path/to/snark-artifacts/packages/$circuit/$circuit-$i.zkey";done
for i in {1..16};do mv -f "$circuit-proof-$i/$circuit-proof/$circuit-proof_js/$circuit-proof.wasm" "/path/to/snark-artifacts/packages/$circuit/$circuit-$i.wasm";done
```

Test

See #143

git checkout test/poseidon-verifkey
pnpm --filter ./packages/poseidon t

@sripwoud sripwoud requested a review from cedoor as a code owner July 22, 2024 10:06
@sripwoud sripwoud added the artifacts New SNARK artifacts. label Jul 22, 2024
@sripwoud sripwoud self-assigned this Jul 22, 2024
Copy link

changeset-bot bot commented Jul 22, 2024

⚠️ No Changeset found

Latest commit: 4e8c557

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@@ -2,7 +2,7 @@
"name": "@zk-kit/poseidon-artifacts",
"description": "zk-kit poseidon artifacts",
"license": "MIT",
"version": "1.0.0-beta.1",
"version": "1.0.0-beta.4",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@cedoor cedoor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we test them before merging?

@sripwoud
Copy link
Member Author

Would be better
Yes
But I wasn't sure what was the easiest way.
A jest test, using snakjs CLI directly?

@cedoor
Copy link
Member

cedoor commented Jul 23, 2024

Would be better Yes But I wasn't sure what was the easiest way. A jest test, using snakjs CLI directly?

Do we need the CLI? All the artifacts are ready to be used. So snarkjs can be used to generate the proof and then to verify it right?

@sripwoud sripwoud marked this pull request as draft July 23, 2024 20:12
@sripwoud
Copy link
Member Author

sripwoud commented Jul 23, 2024

@cedoor I added tests that cover all the wasm, zkey , json artifacts... see #127
and they are all failing.

Either my test suite itself is wrong (although it is very similar to what we do here)
or all the verification keys I generated are wrong...

@sripwoud sripwoud force-pushed the chore/poseidon-verifkey branch from 0f4d6f1 to 5b56e77 Compare September 11, 2024 12:23
@sripwoud sripwoud marked this pull request as ready for review September 18, 2024 14:29
@sripwoud
Copy link
Member Author

@cedoor New artifacts generated and tests added in #143: should we merge #143 btw for reference (or even add it into the CI)?

@sripwoud sripwoud requested a review from cedoor September 18, 2024 14:30
@sripwoud
Copy link
Member Author

sripwoud commented Sep 18, 2024

The reason why my initial tests were failing (making me believe the CLI was buggy) was because I was using incompatible zkey and vkey artifacts to generate and verify the proofs.
If 2 different setups are run for a same circuit C, producing {v,z}key{A,B},
one cannot

  • generate a proof P with zkeyA
  • and then verify P with vkeyB

This will fail (which is what i was initially doing)

@cedoor
Copy link
Member

cedoor commented Sep 20, 2024

@cedoor New artifacts generated and tests added in #143: should we merge #143 btw for reference (or even add it into the CI)?

Yea, I'd add it to the CI 👍🏽 We could open another issue/PR for it.

Do we need to rebase this branch?

@sripwoud sripwoud force-pushed the chore/poseidon-verifkey branch from c873086 to 4e8c557 Compare September 21, 2024 09:23
* test(poseidon): verify proofs

* test: update
@sripwoud
Copy link
Member Author

@cedoor branch is rebased
#144 refactors the tests (the ci already picks them)

@cedoor
Copy link
Member

cedoor commented Sep 23, 2024

@sripwoud kk, then this PR looks ok to me!

@sripwoud sripwoud merged commit 33661df into main Sep 23, 2024
1 check passed
@sripwoud sripwoud deleted the chore/poseidon-verifkey branch September 23, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
artifacts New SNARK artifacts.
Projects
Status: ✔️ Done
Development

Successfully merging this pull request may close these issues.

2 participants