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

Use commitment in NullfierStateDerivation proof #2833

Merged
merged 5 commits into from
Jul 14, 2023

Conversation

cronokirby
Copy link
Contributor

Fixes #2764.

Previously, this proof would take in a public note, and compute the commitment in circuit. This PR changes the proof to take in the commitment directly, thus saving on constraints.

@cronokirby cronokirby temporarily deployed to smoke-test July 13, 2023 19:06 — with GitHub Actions Inactive
@cronokirby
Copy link
Contributor Author

For merging, we may not want to be modifying proof-related stuff quite at the moment, but this is a minor change.

@cronokirby cronokirby force-pushed the 2764-nullifier-derivation-state-commitment branch from 8b81e94 to 5d8015b Compare July 14, 2023 09:40
@cronokirby cronokirby temporarily deployed to smoke-test July 14, 2023 09:40 — with GitHub Actions Inactive
This fixes a few places where we hadn't made this update yet.
Taking advantage of having touched this file to clean it up a bit.
1. Make note_commitment a public member of the struct.

This seems to match other circuits, where the witnesses are private, and the public inputs are public.

2. Use position, not_commitment, nullifier_key, nullifier as the order.

Previously, the order was inconsistent between prove / verify, and the other initialization methods.
Making this consistent seemed like a good idea.
@cronokirby cronokirby temporarily deployed to smoke-test July 14, 2023 10:08 — with GitHub Actions Inactive
Copy link
Member

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

LGTM!

@redshiftzero
Copy link
Member

Looks like the CI failure here was unrelated to the diff in this PR and was resolved with #2838, so merging

@redshiftzero redshiftzero merged commit 413dbc6 into main Jul 14, 2023
7 of 8 checks passed
@redshiftzero redshiftzero deleted the 2764-nullifier-derivation-state-commitment branch July 14, 2023 16:53
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.

modify nullifier derivation circuit to take a state commitment
2 participants