From 6206becb094bed9163e77b1f67b78bdd30a72c38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 8 Mar 2024 14:07:12 -0300 Subject: [PATCH] feat: detect unknown note type ids in compute_note_hash (#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. --- noir/noir-repo/aztec_macros/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/noir/noir-repo/aztec_macros/src/lib.rs b/noir/noir-repo/aztec_macros/src/lib.rs index 012995d6ed4..1285dab33ab 100644 --- a/noir/noir-repo/aztec_macros/src/lib.rs +++ b/noir/noir-repo/aztec_macros/src/lib.rs @@ -1875,8 +1875,8 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec) -> // 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, @@ -1885,6 +1885,7 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec) -> 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() @@ -1898,10 +1899,10 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec) -> }}" , 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] }";