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

feat: read-requests generated by app circuit, checked in private kernel #619

Merged
merged 63 commits into from
Jun 12, 2023

Conversation

dbanks12
Copy link
Contributor

@dbanks12 dbanks12 commented May 18, 2023

Description

When the app circuit reads a note, it should output a read request for that note. The RPC client will then generate membership witness (leaf index & sibling path) for that read request and pass that to the private kernel. The private kernel will silo the read request by contract address and perform a membership check to ensure that the read request corresponds to an existing commitment in the private data tree. All read requests must hash to matching private data roots. App circuit no longer needs to get any membership info for notes when it makes a get oracle call.

Outlined in these discourse threads:

Blocks #512
Closes #513

Followup tasks:

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • The branch has been merged or rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

@dbanks12 dbanks12 changed the title feat: read-request-membership checked in PKC (not app circuit) feat: check read-request-membership in PKC May 18, 2023
Copy link
Member

@Maddiaa0 Maddiaa0 left a comment

Choose a reason for hiding this comment

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

looks pretty good to me! just some comments, all nits

Copy link
Contributor

@suyash67 suyash67 left a comment

Choose a reason for hiding this comment

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

Circuits/cpp changes look correct to me. Just added minor comments.

Copy link
Contributor

@jeanmon jeanmon left a comment

Choose a reason for hiding this comment

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

LGTM. My comments are very minor.

circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp Outdated Show resolved Hide resolved
circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp Outdated Show resolved Hide resolved
circuits/cpp/src/aztec3/circuits/hash.hpp Show resolved Hide resolved
circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp Outdated Show resolved Hide resolved
@dbanks12 dbanks12 merged commit 9b8ba1b into master Jun 12, 2023
@dbanks12 dbanks12 deleted the db/read-membership-in-pkc branch June 12, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-protocol-circuits Component: Protocol circuits (kernel & rollup) T-feature-request Type: Adding a brand new feature (not to be confused with improving an existing feature). T-refactor Type: this code needs refactoring T-testing Type: Testing. More tests need to be added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App circuit should generate read requests, kernel should validate them
5 participants