addressing more feedback

Author: sklppy88

.test_patterns.yml

@@ -78,29 +78,6 @@ tests:
     owners:
       - "U04DT239VQU" # sean
 
-  # FAIL  ./flakey_e2e_inclusion_proofs_contract.test.ts
-  # ● e2e_inclusion_proofs_contract › contract inclusion › proves public deployment of a contract
-  #
-  #   Undefined argument value of type field
-  #
-  #     41 |   private encodeArgument(abiType: AbiType, arg: any, name?: string) {
-  #     42 |     if (arg === undefined || arg == null) {
-  #   > 43 |       throw new Error(`Undefined argument ${name ?? 'unnamed'} of type ${abiType.kind}`);
-  #        |             ^
-  #     44 |     }
-  #     45 |     switch (abiType.kind) {
-  #     46 |       case 'field':
-  #
-  #     at ArgumentEncoder.encodeArgument (../../stdlib/src/abi/encoder.ts:43:13)
-  #     at ArgumentEncoder.encode (../../stdlib/src/abi/encoder.ts:137:12)
-  #     at encodeArguments (../../stdlib/src/abi/encoder.ts:150:41)
-  #     at computeInitializationHash (../../stdlib/src/contract/contract_address.ts:75:20)
-  #     at getContractInstanceFromDeployParams (../../stdlib/src/contract/contract_instance.ts:124:9)
-  #     at Object.getContractInstanceFromDeployParams (flakey_e2e_inclusion_proofs_contract.test.ts:275:24)
-  - regex: "simple flakey_e2e_inclusion_proofs_contract"
-    owners:
-      - "UKUMA5J7K" # charlie
-
   - regex: "simple e2e_fees/private_payments"
     owners:
       - "U02G4KAD57Y" # phil

docs/docs/developers/guides/smart_contracts/writing_contracts/how_to_prove_history.md

@@ -45,7 +45,7 @@ Retrieve the note from the user's PXE.
 
 #include_code get_note_from_pxe noir-projects/noir-contracts/contracts/test_contract/src/main.nr rust
 
-In this example, the user's notes are stored in a storage_slot that is specified. We look for notes located in this storage_slot and return the ones matched by the getter options.
+In this example, we fetch notes located in the storage slot that we pass in from the function parameters. The notes also must match the criteria specified by the getter options that we declare and are able to customize.
 
 ## Prove that a note was included in a specified block
 

noir-projects/aztec-nr/aztec/src/history/note_inclusion/test.nr

@@ -1,22 +1,22 @@
 use crate::history::{note_inclusion::ProveNoteInclusion, test};
 
-// In this test, we create a note and prove its inclusion in the block.
+// In this test, we create a note and prove its inclusion in the block of its creation.
 #[test]
 unconstrained fn note_inclusion() {
     let (env, retrieved_note) = test::create_note();
 
-    let context = &mut env.private();
+    let context = &mut env.private_at(test::NOTE_CREATED_AT);
 
     // docs:start:prove_note_inclusion
-    context.historical_header.prove_note_inclusion(retrieved_note, 15);
+    context.historical_header.prove_note_inclusion(retrieved_note, test::NOTE_STORAGE_SLOT);
     // docs:end:prove_note_inclusion
 }
 
-// In this test, we create a note and fail to prove its inclusion in a block before its creation.
+// In this test, we create a note and fail to prove its inclusion in the block before its creation.
 #[test(should_fail_with = "not found in NOTE_HASH_TREE at block 1")]
 unconstrained fn note_inclusion_fails() {
     let (env, retrieved_note) = test::create_note();
 
-    let context = &mut env.private_at(1);
-    context.historical_header.prove_note_inclusion(retrieved_note, 15);
+    let context = &mut env.private_at(test::NOTE_CREATED_AT - 1);
+    context.historical_header.prove_note_inclusion(retrieved_note, test::NOTE_STORAGE_SLOT);
 }

noir-projects/aztec-nr/aztec/src/history/note_validity/test.nr

@@ -4,35 +4,35 @@ use crate::history::test;
 // In these tests, we create a note in one block and nullify it in the next.
 
 // In this test, we fail to prove the note's validity in the block before its creation. It fails the validity check due to
-// non-inclusion of the note in the block specified.
+// non-inclusion of the note in the block before its creation.
 #[test(should_fail_with = "not found in NOTE_HASH_TREE at block 1")]
 unconstrained fn note_not_valid_due_to_non_inclusion() {
     let (env, retrieved_note) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private_at(1);
+    let context = &mut env.private_at(test::NOTE_CREATED_AT - 1);
 
-    context.historical_header.prove_note_validity(retrieved_note, 15, context);
+    context.historical_header.prove_note_validity(retrieved_note, test::NOTE_STORAGE_SLOT, context);
 }
 
-// In this test, we prove the note's validity in the block after its creation, and before its nullification. It succeeds
-// because it is included and not nullified at the block specified
+// In this test, we prove the note's validity in the block at its creation, and before its nullification. It succeeds
+// because it is included and not nullified at the block specified.
 #[test]
 unconstrained fn note_is_valid() {
     let (env, retrieved_note) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private_at(2);
+    let context = &mut env.private_at(test::NOTE_CREATED_AT);
 
     // docs:start:prove_note_validity
-    context.historical_header.prove_note_validity(retrieved_note, 15, context);
+    context.historical_header.prove_note_validity(retrieved_note, test::NOTE_STORAGE_SLOT, context);
     // docs:end:prove_note_validity
 }
 
-// In this test, we fail to prove the note's validity in the block after its nullification. It fails the validity check due to
-// its nullifier being included in the state at the block specified.
+// In this test, we fail to prove the note's validity in the block at its nullification. It fails the validity check due to
+// its nullifier being included in the state at one block after it was created.
 #[test(should_fail_with = "Proving nullifier non-inclusion failed")]
 unconstrained fn note_not_valid_due_to_nullification() {
     let (env, retrieved_note) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private();
-    context.historical_header.prove_note_validity(retrieved_note, 15, context);
+    let context = &mut env.private_at(test::NOTE_NULLIFIED_AT);
+    context.historical_header.prove_note_validity(retrieved_note, test::NOTE_STORAGE_SLOT, context);
 }

noir-projects/aztec-nr/aztec/src/history/nullifier_inclusion/test.nr

@@ -1,38 +1,47 @@
 use crate::history::{nullifier_inclusion::{ProveNoteIsNullified, ProveNullifierInclusion}, test};
 use crate::oracle::random::random;
+use crate::test::helpers::test_environment::FIRST_NULLIFIER_EMITTED_IN_TXE;
 
 // In these tests, we create a note in one block and nullify it in the next.
 
-// In this test, we prove the presence of the note's nullifier in state at the current block (the block after it was nullified).
+// In this test, we prove the presence of the note's nullifier in state at the block it was nullified in.
 #[test]
 unconstrained fn note_is_nullified() {
     let (env, retrieved_note) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private();
+    let context = &mut env.private_at(test::NOTE_NULLIFIED_AT);
 
-    context.historical_header.prove_note_is_nullified(retrieved_note, 15, context);
+    context.historical_header.prove_note_is_nullified(
+        retrieved_note,
+        test::NOTE_STORAGE_SLOT,
+        context,
+    );
 }
 
 // In this test, we fail to prove the presence of the note's nullifier in state at the block before it was nullified.
 #[test(should_fail_with = "Nullifier membership witness not found at block 2.")]
 unconstrained fn note_is_not_nullified() {
     let (env, retrieved_note) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private_at(2);
+    let context = &mut env.private_at(test::NOTE_NULLIFIED_AT - 1);
 
-    context.historical_header.prove_note_is_nullified(retrieved_note, 15, context);
+    context.historical_header.prove_note_is_nullified(
+        retrieved_note,
+        test::NOTE_STORAGE_SLOT,
+        context,
+    );
 }
 
-// In this test, we prove the inclusion of an existing nullifier in state. We use 6969 + 1 because it is
-// the first nullifier created (the TXe creates deterministic first nullifiers if no side-effects are emitted)
+// In this test, we prove the inclusion of an existing nullifier in state. We use know FIRST_NULLIFIER_EMITTED_IN_TXE exists
+// because the TXe creates deterministic first nullifiers if no side-effects are emitted.
 #[test]
 unconstrained fn nullifier_inclusion() {
     let (env) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private_at(2);
+    let context = &mut env.private();
 
     // docs:start:prove_nullifier_inclusion
-    context.historical_header.prove_nullifier_inclusion(6969 + 1);
+    context.historical_header.prove_nullifier_inclusion(FIRST_NULLIFIER_EMITTED_IN_TXE);
     // docs:end:prove_nullifier_inclusion
 }
 

noir-projects/aztec-nr/aztec/src/history/nullifier_non_inclusion/test.nr

@@ -2,45 +2,54 @@ use crate::history::nullifier_non_inclusion::{ProveNoteNotNullified, ProveNullif
 use crate::oracle::random::random;
 
 use crate::history::test;
+use crate::test::helpers::test_environment::FIRST_NULLIFIER_EMITTED_IN_TXE;
 
 // In these tests, we create a note in one block and nullify it in the next.
 
-// In this test, we prove the absence of the note's nullifier in state before the note was nullified.
+// In this test, we prove the absence of the note's nullifier in state at the block before it was nullified.
 #[test]
 unconstrained fn note_not_nullified() {
     let (env, retrieved_note) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private_at(2);
+    let context = &mut env.private_at(test::NOTE_NULLIFIED_AT - 1);
 
-    context.historical_header.prove_note_not_nullified(retrieved_note, 15, context);
+    context.historical_header.prove_note_not_nullified(
+        retrieved_note,
+        test::NOTE_STORAGE_SLOT,
+        context,
+    );
 }
 
-// In this test, we fail to prove the absence of the note's nullifier in state at the current block (the block after it was nullified).
+// In this test, we fail prove the absence of the note's nullifier in state at the block it was nullified in.
 #[test(should_fail_with = "Proving nullifier non-inclusion failed")]
 unconstrained fn note_not_nullified_fails() {
     let (env, retrieved_note) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private();
-    context.historical_header.prove_note_not_nullified(retrieved_note, 15, context);
+    let context = &mut env.private_at(test::NOTE_NULLIFIED_AT);
+    context.historical_header.prove_note_not_nullified(
+        retrieved_note,
+        test::NOTE_STORAGE_SLOT,
+        context,
+    );
 }
 
 // In this test, we prove the absence of an arbitrary nullifier in state.
 #[test]
 unconstrained fn nullifier_non_inclusion() {
     let (env) = test::create_note_and_nullify_it();
 
-    let context = &mut env.private_at(2);
+    let context = &mut env.private();
 
     context.historical_header.prove_nullifier_non_inclusion(random());
 }
 
-// In this test, we fail to prove the inclusion of an existing nullifier in state. We use 6969 + 1 because it is
-// the first nullifier created (the TXe creates deterministic first nullifiers if no side-effects are emitted)
+// In this test, we fail to prove the absence of an existing nullifier in state. We use know FIRST_NULLIFIER_EMITTED_IN_TXE exists
+// because the TXe creates deterministic first nullifiers if no side-effects are emitted.
 #[test(should_fail_with = "Proving nullifier non-inclusion failed")]
 unconstrained fn nullifier_non_inclusion_fails() {
     let (env) = test::create_note_and_nullify_it();
 
     let context = &mut env.private();
 
-    context.historical_header.prove_nullifier_non_inclusion(6969 + 1);
+    context.historical_header.prove_nullifier_non_inclusion(FIRST_NULLIFIER_EMITTED_IN_TXE);
 }

noir-projects/aztec-nr/aztec/src/history/public_storage/test.nr

@@ -3,11 +3,19 @@ use crate::{
     test::helpers::test_environment::TestEnvironment,
 };
 
+global STORAGE_SLOT: Field = 420;
+global VALUE: Field = 69;
+global WRITTEN_TO_STORAGE_AT: u32 = 2;
+global STORAGE_UNSET_VALUE: Field = 0;
+
 unconstrained fn setup() -> &mut TestEnvironment {
     let mut env = TestEnvironment::new();
     let context = &mut env.public();
 
-    context.storage_write(15, 69);
+    // We sanity check that we are building block 2, and thus block 2 is this storage write will occur.
+    assert_eq(env.pending_block_number(), 2);
+
+    context.storage_write(STORAGE_SLOT, VALUE);
 
     env.advance_block_by(1);
 
@@ -19,11 +27,14 @@ unconstrained fn setup() -> &mut TestEnvironment {
 unconstrained fn public_storage_read() {
     let env = setup();
 
-    let context = &mut env.private();
+    let context = &mut env.private_at(WRITTEN_TO_STORAGE_AT);
 
     assert_eq(
-        context.historical_header.public_storage_historical_read(15, get_contract_address()),
-        69,
+        context.historical_header.public_storage_historical_read(
+            STORAGE_SLOT,
+            get_contract_address(),
+        ),
+        VALUE,
     );
 }
 
@@ -32,10 +43,13 @@ unconstrained fn public_storage_read() {
 unconstrained fn public_storage_read_empty() {
     let env = setup();
 
-    let context = &mut env.private_at(1);
+    let context = &mut env.private_at(WRITTEN_TO_STORAGE_AT - 1);
 
     assert_eq(
-        context.historical_header.public_storage_historical_read(15, get_contract_address()),
-        0,
+        context.historical_header.public_storage_historical_read(
+            STORAGE_SLOT,
+            get_contract_address(),
+        ),
+        STORAGE_UNSET_VALUE,
     );
 }

noir-projects/aztec-nr/aztec/src/history/test.nr

@@ -8,11 +8,18 @@ use crate::{
 use crate::note::retrieved_note::RetrievedNote;
 use crate::test::{helpers::test_environment::TestEnvironment, mocks::mock_note::MockNote};
 
-// We create a note at block 2 and nullify it in the next block. This helps us test our historical apis.
+pub(crate) global NOTE_STORAGE_SLOT: Field = 420;
+pub(crate) global NOTE_CREATED_AT: u32 = 2;
+pub(crate) global NOTE_NULLIFIED_AT: u32 = 3;
+
+// We create a note in block 2.
 pub(crate) unconstrained fn create_note() -> (&mut TestEnvironment, RetrievedNote<MockNote>) {
     let mut env = TestEnvironment::new();
     let context = &mut env.private();
 
+    // We sanity check that we are building block 2, thus block 2 is where the note will be added.
+    assert_eq(env.pending_block_number(), 2);
+
     let contract_address = get_contract_address();
 
     let retrieved_note = MockNote::new(69)
@@ -23,22 +30,25 @@ pub(crate) unconstrained fn create_note() -> (&mut TestEnvironment, RetrievedNot
         ))
         .build_retrieved_note();
 
-    let _ = lifecycle_create_note(context, 15, retrieved_note.note);
+    let _ = lifecycle_create_note(context, NOTE_STORAGE_SLOT, retrieved_note.note);
 
-    // #TODO (12226): FIX
+    // TODO(#12226): FIX
     // context.push_note_hash(retrieved_note.note.compute_note_hash(15));
     env.advance_block_by(1);
 
     (&mut env, retrieved_note)
 }
 
-// We create a note at block 2 and nullify it in the next block. This helps us test our historical apis.
+// We create a note at block 2 and nullify it in the next block.
 pub(crate) unconstrained fn create_note_and_nullify_it() -> (&mut TestEnvironment, RetrievedNote<MockNote>) {
     let (env, retrieved_note) = create_note();
 
     let context = &mut env.private();
 
-    destroy_note(context, retrieved_note, 15);
+    // We sanity check that we are building block 3, thus block 3 is where the note will be nullified.
+    assert_eq(env.pending_block_number(), 3);
+
+    destroy_note(context, retrieved_note, NOTE_STORAGE_SLOT);
 
     env.advance_block_by(1);
 

noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr

@@ -14,6 +14,11 @@ use crate::oracle::{
 use protocol_types::constants::PUBLIC_DISPATCH_SELECTOR;
 use protocol_types::traits::Packable;
 
+// This is the first nullifier emitted from the TXe. It is an arbitrary number assigned in the TXe that is
+// larger than the first prefilled subtree of the nullifier tree. We use know FIRST_NULLIFIER_EMITTED_IN_TXE exists
+// because the TXe creates deterministic first nullifiers if no side-effects are emitted.
+pub global FIRST_NULLIFIER_EMITTED_IN_TXE: Field = 6969 + 1;
+
 pub struct TestEnvironment {}
 
 impl TestEnvironment {

noir-projects/aztec-nr/aztec/src/test/mocks/mock_note.nr

@@ -86,8 +86,8 @@ impl MockNoteBuilder {
         RetrievedNote {
             note: MockNote { value: self.value },
             contract_address: self.contract_address.unwrap_or(AztecAddress::zero()),
-            // This is invalid note metadata, which would result in the kernel failing to prove note existence. Because
-            // our tests don't interact with the kernel this is not a problem.
+            // If the option is `None` for self.note_metadata, we fill it with invalid note metadata, which would result in the kernel
+            // failing to prove note existence. Because our tests don't interact with the kernel this is not a problem.
             metadata: self.note_metadata.unwrap_or(std::mem::zeroed()),
         }
     }

noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr

@@ -88,6 +88,13 @@ pub contract DocsExample {
         }
     }
 
+    // DO NOT uncomment this and add an initializer in this contract. We specifically need this contract to not have an initializer
+    // because of tests in 'yarn-project/end-to-end/src/e2e_deploy_contract/deploy_method.test.ts'
+    // #[initializer]
+    // #[private]
+    // // We can name our initializer anything we want as long as it's marked as aztec(initializer)
+    // fn initialize() {}
+
     // docs:start:initialize_public_immutable
     #[public]
     fn initialize_public_immutable(points: u8) {

noir-projects/noir-contracts/contracts/test_contract/src/test.nr

@@ -89,7 +89,7 @@ pub unconstrained fn setup() -> (&mut TestEnvironment, AztecAddress, AztecAddres
     let owner = env.create_account(1);
     env.impersonate(owner);
 
-    // TODO: (12276) This is here because we are testing historical proofs, and we cannot get a block_header at 1 due to a bug in world state.
+    // TODO(#12276): This is here because we are testing historical proofs, and we cannot get a valid block_header at 1 due to a bug in world state.
     env.advance_block_by(1);
 
     // Deploy contract and initialize