Skip to content

Commit

Permalink
feat: detect unknown note type ids in compute_note_hash (AztecProtoco…
Browse files Browse the repository at this point in the history
…l/aztec-packages#5086)

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.
  • Loading branch information
AztecBot committed Mar 8, 2024
2 parents aecf7cf + a340e64 commit 8657c6b
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .aztec-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
58e1ff4ecf8dbc5e4504994a9e22b04d09d0535d
6206becb094bed9163e77b1f67b78bdd30a72c38
7 changes: 4 additions & 3 deletions aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1875,8 +1875,8 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec<String>) ->
// For now we hardcode it to 20, which is the same as MAX_NOTE_FIELDS_LENGTH.

if note_types.is_empty() {
// TODO(#4520): Even if the contract does not include any notes, other parts of the stack expect for this
// function to exist, so we include a dummy version. We likely should error out here instead.
// Even if the contract does not include any notes, other parts of the stack expect for this function to exist,
// so we include a dummy version.
"
unconstrained fn compute_note_hash_and_nullifier(
contract_address: AztecAddress,
Expand All @@ -1885,6 +1885,7 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec<String>) ->
note_type_id: Field,
serialized_note: [Field; 20]
) -> pub [Field; 4] {
assert(false, \"This contract does not use private notes\");
[0, 0, 0, 0]
}"
.to_string()
Expand All @@ -1898,10 +1899,10 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec<String>) ->
}}"
, note_type)).collect();

// TODO(#4520): error out on the else instead of returning a zero array
let full_if_statement = if_statements.join(" else ")
+ "
else {
assert(false, \"Unknown note type ID\");
[0, 0, 0, 0]
}";

Expand Down

0 comments on commit 8657c6b

Please sign in to comment.