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

Noir contracts shouldn't get notes that were nullified earlier in TX #940

Closed
Tracked by #512
rahul-kothari opened this issue Jun 30, 2023 · 3 comments · Fixed by #1186
Closed
Tracked by #512

Noir contracts shouldn't get notes that were nullified earlier in TX #940

rahul-kothari opened this issue Jun 30, 2023 · 3 comments · Fixed by #1186
Assignees
@jeanmon

Comments

@rahul-kothari
Copy link
Contributor

Ref -> storage_slot.remove(note) doesn't remove until the base rollup. So for the same transaction, you always get the same notes and can't spend other notes. This prevents how many transactions can happen at the same time.
https://aztecprotocol.slack.com/archives/C03P17YHVK8/p1688120000664279
@github-project-automation github-project-automation bot moved this to Todo in A3 Jun 30, 2023
@dbanks12
Copy link
Collaborator

dbanks12 commented Jun 30, 2023 •
edited
Loading

Related to (probably blocked by)

@dbanks12 dbanks12 mentioned this issue Jun 30, 2023
Merged
6 tasks
@dbanks12 dbanks12 changed the title Pending nullifiers in ACIR simulaltor Pending nullifiers in ACIR simulator Jul 14, 2023
@dbanks12 dbanks12 changed the title Pending nullifiers in ACIR simulator Noir contracts shouldn't get notes that were nullified earlier in TX Jul 14, 2023
@dbanks12
Copy link
Collaborator

Renamed the ticket from previous title. We'll need to track pending nullifiers in the ACIR simulator and check against them whenever doing getNotes.

I'd be glad to pick this ticket up if it's okay with you @sirasistant!

@dbanks12 dbanks12 mentioned this issue Jul 14, 2023
Closed
@dbanks12 dbanks12 assigned suyash67 and jeanmon and unassigned sirasistant and suyash67 Jul 17, 2023
@jeanmon jeanmon moved this from Todo to In Progress in A3 Jul 24, 2023
@jeanmon
Copy link
Contributor

jeanmon commented Jul 24, 2023 •
edited
Loading

IMPORTANT:
We need to handle BOTH following cases:

  1. The nullifier nullifies a pending commitment (namely on created within this transaction)
  2. The nullifier nullifies a settled commitment pertaining to a previous transaction.

In both cases, getNotes should filter notes which were already nullified during this transaction.

This was referenced Jul 25, 2023
Merged
Closed
jeanmon added a commit that referenced this issue Jul 27, 2023
@jeanmon
940 - getNotes filter nullified notes (#1186)
12830b0 
# Description

Resolves #940

# Checklist:

- [x] I have reviewed my diff in github, line by line.
- [x] Every change is related to the PR description.
- [x] 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 the issue(s) that it resolves.
- [x] There are no unexpected formatting changes, superfluous debug
logs, or commented-out code.
- [x] The branch has been merged or rebased against the head of its
merge target.
- [x] I'm happy for the PR to be merged at the reviewer's next
convenience.
@github-project-automation github-project-automation bot moved this from In Progress to Done in A3 Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jeanmon jeanmon

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

Successfully merging a pull request may close this issue.

940 - getNotes filter nullified notes AztecProtocol/aztec-packages
6 participants
@sirasistant @suyash67 @rahul-kothari @iAmMichaelConnor @dbanks12 @jeanmon