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

refactor(AztecMacro): Error out when computing hash for unknown notes #4520

Closed
Tracked by #5077
nventuro opened this issue Feb 8, 2024 · 0 comments · Fixed by #5086
Closed
Tracked by #5077

refactor(AztecMacro): Error out when computing hash for unknown notes #4520

nventuro opened this issue Feb 8, 2024 · 0 comments · Fixed by #5086
Assignees

Comments

@nventuro
Copy link
Contributor

nventuro commented Feb 8, 2024

compute_hash_and_nullifier typically always attempts to produce a result, regardless of the validity of the inputs. While it's not possible to check if a given storage slot is valid due to how Maps dynamically allocate them, we can instead check the validity of note type ids (introduced in #4500) and error out on uknown ones, simplfying the debugging process.

An even more sophisticated version of this might try to check that the note type id and storage slot match, though this won't be doable for state variables such as maps.

@LHerskind LHerskind changed the title Error out when computing hash for unknown notes refactor(AztecMacro): Error out when computing hash for unknown notes Mar 8, 2024
nventuro added a commit that referenced this issue Mar 8, 2024
Closes #4520.

I did some work adding tests for this, but ultimately decided to go back
on it. We don't really have any tests for other parts of the `addNotes`
flow (such as the checks for note existence, inclusion in the provided
tx, non-nullification, etc.), and this didn't feel like the right place
to work on those.

We definitely should however. Part of the problem is that writing a
contract that allows for manual note creation, deletion and retrieval
from a test is quite annoying - I did some of this in `TestContract` for
#4238. For this we'd also need to have different functions for different
note types, and even then some of these notes can only be added
automatically via broadcast due to the random values in the note.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant