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.

Sign up for GitHub

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

Jump to bottom

Spike: replace getCommitments oracle with checkCommitmentExists? #1142

Closed
dbanks12 opened this issue Jul 21, 2023 · 0 comments · Fixed by #2256
Closed

Spike: replace getCommitments oracle with checkCommitmentExists? #1142

dbanks12 opened this issue Jul 21, 2023 · 0 comments · Fixed by #2256
Assignees
@LeilaWang

Comments

@dbanks12
Copy link
Collaborator

Since the kernel handles read membership checks, we don't need to check the root/leaf-index here in the app circuit. App can just push commitment to read requests and trigger an oracle call that errors if the commitment doesn't exist.
@dbanks12 dbanks12 added the spike label Jul 21, 2023
@dbanks12 dbanks12 self-assigned this Jul 21, 2023
@dbanks12 dbanks12 added this to A3 Jul 21, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Jul 21, 2023
@dbanks12 dbanks12 moved this from Todo to In Progress in A3 Jul 21, 2023
@dbanks12 dbanks12 mentioned this issue Jul 21, 2023
Closed
6 tasks
@dbanks12 dbanks12 moved this from In Progress to Todo in A3 Aug 7, 2023
@dbanks12 dbanks12 assigned LeilaWang and unassigned dbanks12 Sep 12, 2023
@LeilaWang LeilaWang mentioned this issue Sep 12, 2023
Merged
4 tasks
LeilaWang added a commit that referenced this issue Sep 13, 2023
@LeilaWang @dbanks12
fix: ensure_note_hash_exists (#2256)
271b060 
Closes #1142 #1029 

- Rename `oracle.getCommitment` to `oracle.checkNoteHashExists`.
- Change `Set.assert_contains_and_remove(note, nonce)` to take a nonce
in addition to a note. We calculate the inner note hash from the
provided note. And although the nonce can be set to the header of the
note, by making it a required parameter of this method makes it clearer
that nonce is needed to check the existence of a note hash. (An example
will be created in a later PR to show how a recipient can learn about
the nonce if they don't have the encrypted data.)
- Change the api on CommitmentDb to only return an index of a note hash:
the index is all we need to know if a note hash exists, and to use it to
get `readRequestMembershipWitnesses` later in the kernel prover.
- (A tiny change that is not really related to this PR): We don't have
to emit storage slot when notifying the simulator about a new nullifier.

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [ ] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [ ] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [ ] Every change is related to the PR description.
- [ ] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).

---------

Co-authored-by: David Banks <[email protected]>
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@LeilaWang LeilaWang

Labels
None yet
Projects
A3
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: ensure_note_hash_exists AztecProtocol/aztec-packages
2 participants
@LeilaWang @dbanks12