Skip to content

Commit

Permalink
feat: detect unknown note type ids in compute_note_hash (#5086)
Browse files Browse the repository at this point in the history
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
nventuro authored Mar 8, 2024
1 parent 4b94d58 commit 6206bec
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions noir/noir-repo/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 6206bec

Please sign in to comment.