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

833 - Skip validation of the membership for optimistic read requests #859

Merged
merged 4 commits into from
Jun 16, 2023

Conversation

jeanmon
Copy link
Contributor

@jeanmon jeanmon commented Jun 15, 2023

Description

Resolves #833

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.

@jeanmon jeanmon force-pushed the jm/833-pending-comm-skip-check branch from 87b3bed to 22736c5 Compare June 15, 2023 14:49
@jeanmon jeanmon marked this pull request as ready for review June 15, 2023 15:49
@jeanmon jeanmon requested a review from dbanks12 June 15, 2023 15:49
Copy link
Collaborator

@dbanks12 dbanks12 left a comment

Choose a reason for hiding this comment

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

LGTM. Well done, great comments, nice tests!

Comment on lines +85 to +90
// A pending commitment is the one that is not yet added to private data tree
// An optimistic read is when we try to "read" a pending commitment
// We determine if it is an optimistic read depending on the leaf index from the membership witness
// Note that the Merkle membership proof would be null and void in case of an optimistic read
// but we use the leaf index as a placeholder to detect an optimistic read.
const auto is_optimistic_read = (witness.leaf_index == NT::fr(-1));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Excellent comment.

@jeanmon jeanmon force-pushed the jm/833-pending-comm-skip-check branch from 47bb5e0 to b778d9e Compare June 15, 2023 16:04
@jeanmon jeanmon force-pushed the jm/833-pending-comm-skip-check branch from b778d9e to 06c3b7a Compare June 16, 2023 07:39
@dbanks12 dbanks12 merged commit aa46f33 into master Jun 16, 2023
@dbanks12 dbanks12 deleted the jm/833-pending-comm-skip-check branch June 16, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants