Skip to content

Feat/implement proof gen #7

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

Merged
merged 10 commits into from
Apr 21, 2025
Merged

Feat/implement proof gen #7

merged 10 commits into from
Apr 21, 2025

Conversation

KimiWu123
Copy link
Collaborator

@KimiWu123 KimiWu123 commented Apr 9, 2025

Description

To implement Proof functionalities by integrating Circom-Prover. Will rebase main after #6 is merged.

Related Issue(s)

#5

Known Issues

@KimiWu123 KimiWu123 force-pushed the feat/implement_proof_gen branch from 8826519 to be6c4db Compare April 15, 2025 08:32
@KimiWu123 KimiWu123 force-pushed the feat/implement_proof_gen branch from be6c4db to e89b5b9 Compare April 15, 2025 08:40
@KimiWu123 KimiWu123 force-pushed the feat/implement_proof_gen branch from 0ec9687 to 7d341d4 Compare April 15, 2025 09:13
@KimiWu123 KimiWu123 force-pushed the feat/implement_proof_gen branch from 1a4701e to 59c7739 Compare April 16, 2025 05:30
@KimiWu123 KimiWu123 force-pushed the feat/implement_proof_gen branch from 3e67e98 to 36c2c21 Compare April 16, 2025 05:43
@KimiWu123 KimiWu123 requested a review from vplasencia April 16, 2025 06:47
@vplasencia
Copy link
Member

Hey @KimiWu123! Thank you very much for creating this PR. Is this PR ready for review or is it still a draft?

@KimiWu123 KimiWu123 marked this pull request as ready for review April 16, 2025 08:40
@KimiWu123
Copy link
Collaborator Author

Hey @KimiWu123! Thank you very much for creating this PR. Is this PR ready for review or is it still a draft?

It's ready for review! Sorry, forgot to mark as "ready for review"

src/proof.rs Outdated
Comment on lines 158 to 161
p.b.x[0].clone(),
p.b.x[1].clone(),
p.b.y[0].clone(),
p.b.y[1].clone(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think here should be

Suggested change
p.b.x[0].clone(),
p.b.x[1].clone(),
p.b.y[0].clone(),
p.b.y[1].clone(),
p.b.x[1].clone(),
p.b.x[0].clone(),
p.b.y[1].clone(),
p.b.y[0].clone(),

ref
https://github.com/privacy-scaling-explorations/zk-kit/blob/577fa314c5510cd347da0d7f2fcdc200326f60ef/packages/utils/src/proof-packing.ts#L22

Copy link
Collaborator Author

@KimiWu123 KimiWu123 Apr 16, 2025

Choose a reason for hiding this comment

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

After applied this change, the proof generated by semaphore-rs still can't be verified by Semaphore-js. But it's weired that our verification fails as well.
It works!

src/proof.rs Outdated
Comment on lines 174 to 175
x: [packed[2].clone(), packed[3].clone()],
y: [packed[4].clone(), packed[5].clone()],
Copy link
Collaborator

Choose a reason for hiding this comment

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

here is the same

Suggested change
x: [packed[2].clone(), packed[3].clone()],
y: [packed[4].clone(), packed[5].clone()],
x: [packed[3].clone(), packed[2].clone()],
y: [packed[5].clone(), packed[4].clone()],

https://github.com/privacy-scaling-explorations/zk-kit/blob/577fa314c5510cd347da0d7f2fcdc200326f60ef/packages/utils/src/proof-packing.ts#L40

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This one works! Now we can verify the proof from Semaphore-js

Comment on lines +209 to +210
const MEMBER1: Element = [1; 32];
const MEMBER2: Element = [2; 32];
Copy link
Collaborator

Choose a reason for hiding this comment

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

I used to_element(Fq::from(1)) and to_element(Fq::from(2)) to make the member the same as in the semaphore-js

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Everything works now! Thanks!!

Copy link
Member

@vplasencia vplasencia left a comment

Choose a reason for hiding this comment

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

Thank you very much for the great work! 🚀

@KimiWu123 KimiWu123 merged commit 8f062c4 into main Apr 21, 2025
2 checks passed
@KimiWu123 KimiWu123 mentioned this pull request Apr 22, 2025
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.

3 participants