fix: Initializer checks across txs (#4842)

The approach in #4807, where I pushed a note-hash for signalling
initialization and then pushed a read-request to check initialization
later, didn't work. It worked only when initialization is checked in the
same tx as the initialization happens. If it happens on a different tx,
the note-hash gets siloed and hashed with a nonce before getting added
to the note hash tree. While we can silo from the contract (since the
contract knows its own address), we cannot hash with the nonce since we
don't know it. This nonce is typically stored as part of the associated
note in the PXE db, but in this case, we are pusing a note commitment
_without_ an associated note. Not only that, but the PXE today just
doesn't support read requests not related to a note in the local db.

So this PR changes that approach to just use the nullifier, which was
the original idea, since it does not require emitting two pieces of
data. It works because nullifiers are siloed but they are not hashed
with a nonce, so it's possible to reconstruct them when checking
initialization. However, since we don't yet support reading a nullifier
in the same tx (or same block even) that it was created, we need to wait
for the initialization tx to be mined before we can call any function in
the contract.

noir-projects/aztec-nr/aztec/src/initializer.nr

@@ -1,29 +1,23 @@
 use dep::protocol_types::hash::silo_nullifier;
 use crate::context::PrivateContext;
+use crate::history::nullifier_inclusion::prove_nullifier_inclusion;
 
 pub fn mark_as_initialized(context: &mut PrivateContext) {
-  let init_nullifier = compute_unsiloed_contract_initialization_nullifier(context);
+  let init_nullifier = compute_unsiloed_contract_initialization_nullifier(*context);
   context.push_new_nullifier(init_nullifier, 0);
-
-  // We push a commitment as well and use this value to check initialization,
-  // since we cannot yet read a nullifier from the same tx in which it was emitted.
-  // Eventually, when that's supported, we should delete this note_hash and
-  // have all checks rely on reading the nullifier directly.
-  // TODO(@spalladino) Remove when possible.
-  context.push_new_note_hash(init_nullifier);
 }
 
 // TODO(@spalladino): Add a variant using PublicContext once we can read nullifiers or note hashes from public-land.
 pub fn assert_is_initialized(context: &mut PrivateContext) {
-  let init_nullifier = compute_contract_initialization_nullifier(context);
-  context.push_read_request(init_nullifier);
+  let init_nullifier = compute_contract_initialization_nullifier(*context);
+  prove_nullifier_inclusion(init_nullifier, *context);
 }
 
-pub fn compute_contract_initialization_nullifier(context: &mut PrivateContext) -> Field {
+pub fn compute_contract_initialization_nullifier(context: PrivateContext) -> Field {
   let address = context.this_address();
   silo_nullifier(address, compute_unsiloed_contract_initialization_nullifier(context))
 }
 
-pub fn compute_unsiloed_contract_initialization_nullifier(context: &mut PrivateContext) -> Field {
+pub fn compute_unsiloed_contract_initialization_nullifier(context: PrivateContext) -> Field {
   context.this_address().to_field()
 }

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

@@ -19,7 +19,7 @@ contract StatefulTest {
 
     #[aztec(private)]
     fn constructor(owner: AztecAddress, value: Field) {
-        let selector = FunctionSelector::from_signature("internal_create_note((Field),Field)");
+        let selector = FunctionSelector::from_signature("create_note_no_init_check((Field),Field)");
         let _res = context.call_private_function(context.this_address(), selector, [owner.to_field(), value]);
         mark_as_initialized(&mut context);
     }
@@ -34,7 +34,7 @@ contract StatefulTest {
     }
 
     #[aztec(private)]
-    internal fn internal_create_note(owner: AztecAddress, value: Field) {
+    fn create_note_no_init_check(owner: AztecAddress, value: Field) {
         if (value != 0) {
             let loc = storage.notes.at(owner);
             increment(loc, value, owner);
@@ -53,6 +53,18 @@ contract StatefulTest {
         increment(recipient_notes, amount, recipient);
     }
 
+    #[aztec(private)]
+    fn destroy_and_create_no_init_check(recipient: AztecAddress, amount: Field) {
+        let sender = context.msg_sender();
+
+        let sender_notes = storage.notes.at(sender);
+        decrement(sender_notes, amount, sender);
+
+        let recipient_notes = storage.notes.at(recipient);
+        increment(recipient_notes, amount, recipient);
+    }
+
+
     #[aztec(public)]
     fn increment_public_value(owner: AztecAddress, value: Field) {
         let loc = storage.public_values.at(owner);
@@ -69,10 +81,4 @@ contract StatefulTest {
     unconstrained fn get_public_value(owner: AztecAddress) -> pub Field {
         storage.public_values.at(owner).read()
     }
-
-    // This method is here because we've hit the 32 function limit for the TestContract
-    #[aztec(private)]
-    fn deploy_contract(target: AztecAddress) {
-        aztec_deploy_contract(&mut context, target);
-    }
-}
+}

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

@@ -14,10 +14,11 @@ contract Test {
         context::{Context, inputs::private_context_inputs::PrivateContextInputs}, hash::pedersen_hash,
         context::PrivateContext,
         note::{
-        note_header::NoteHeader, utils as note_utils, lifecycle::{create_note, destroy_note},
-        note_getter::{get_notes, view_notes}, note_getter_options::{NoteGetterOptions, NoteStatus},
-        note_viewer_options::NoteViewerOptions
-    },
+            note_header::NoteHeader, utils as note_utils, lifecycle::{create_note, destroy_note},
+            note_getter::{get_notes, view_notes}, note_getter_options::{NoteGetterOptions, NoteStatus},
+            note_viewer_options::NoteViewerOptions
+        },
+        deploy::{deploy_contract as aztec_deploy_contract},
         oracle::{get_public_key::get_public_key as get_public_key_oracle, context::get_portal_address, rand::rand},
         state_vars::PrivateImmutable, log::emit_unencrypted_log_from_private
     };
@@ -349,6 +350,11 @@ contract Test {
         assert(context.historical_header.hash() == header_hash, "Invalid header hash");
     }
 
+    #[aztec(private)]
+    fn deploy_contract(target: AztecAddress) {
+        aztec_deploy_contract(&mut context, target);
+    }
+
     unconstrained fn get_constant() -> pub Field {
         let constant = storage.example_constant.view_note();
         constant.value

yarn-project/end-to-end/src/e2e_deploy_contract.test.ts

@@ -194,11 +194,16 @@ describe('e2e_deploy_contract', () => {
         const owner = await registerRandomAccount(pxe);
         const initArgs: StatefulContractCtorArgs = [owner, 42];
         const contract = await registerContract(testWallet, StatefulTestContract, initArgs);
+        logger.info(`Calling the constructor for ${contract.address}`);
         await contract.methods
           .constructor(...initArgs)
           .send()
           .wait();
+        logger.info(`Checking if the constructor was run for ${contract.address}`);
         expect(await contract.methods.summed_values(owner).view()).toEqual(42n);
+        logger.info(`Calling a function that requires initialization on ${contract.address}`);
+        await contract.methods.create_note(owner, 10).send().wait();
+        expect(await contract.methods.summed_values(owner).view()).toEqual(52n);
       },
       30_000,
     );
@@ -214,6 +219,20 @@ describe('e2e_deploy_contract', () => {
       expect(await contracts[1].methods.summed_values(owner).view()).toEqual(52n);
     }, 30_000);
 
+    // TODO(@spalladino): This won't work until we can read a nullifier in the same tx in which it was emitted.
+    it.skip('initializes and calls a private function in a single tx', async () => {
+      const owner = await registerRandomAccount(pxe);
+      const initArgs: StatefulContractCtorArgs = [owner, 42];
+      const contract = await registerContract(wallet, StatefulTestContract, initArgs);
+      const batch = new BatchCall(wallet, [
+        contract.methods.constructor(...initArgs).request(),
+        contract.methods.create_note(owner, 10).request(),
+      ]);
+      logger.info(`Executing constructor and private function in batch at ${contract.address}`);
+      await batch.send().wait();
+      expect(await contract.methods.summed_values(owner).view()).toEqual(52n);
+    });
+
     it('refuses to initialize a contract twice', async () => {
       const owner = await registerRandomAccount(pxe);
       const initArgs: StatefulContractCtorArgs = [owner, 42];
@@ -234,12 +253,9 @@ describe('e2e_deploy_contract', () => {
       const owner = await registerRandomAccount(pxe);
       const initArgs: StatefulContractCtorArgs = [owner, 42];
       const contract = await registerContract(wallet, StatefulTestContract, initArgs);
-      // TODO(@spalladino): It'd be nicer to be able to fail the assert with a more descriptive message,
-      // but the best we can do for now is pushing a read request to the kernel and wait for it to fail.
-      // Maybe we need an unconstrained check for the read request that runs within the app circuit simulation
-      // so we can bail earlier with a more descriptive error? I should create an issue for this.
+      // TODO(@spalladino): It'd be nicer to be able to fail the assert with a more descriptive message.
       await expect(contract.methods.create_note(owner, 10).send().wait()).rejects.toThrow(
-        /The read request.*does not match/,
+        /nullifier witness not found/i,
       );
     });
 
@@ -350,7 +366,7 @@ describe('e2e_deploy_contract', () => {
       // Register the instance to be deployed in the pxe
       await wallet.addContracts([{ artifact, instance }]);
       // Set up the contract that calls the deployer (which happens to be the StatefulTestContract) and call it
-      const deployer = await registerContract(wallet, StatefulTestContract, [accounts[0].address, 48]);
+      const deployer = await registerContract(wallet, TestContract, [accounts[0].address, 48]);
       await deployer.methods.deploy_contract(instance.address).send().wait();
     });
   });

yarn-project/end-to-end/src/e2e_inclusion_proofs_contract.test.ts

@@ -237,10 +237,10 @@ describe('e2e_inclusion_proofs_contract', () => {
 
       await expect(
         contract.methods.test_nullifier_inclusion(randomNullifier, true, blockNumber).send().wait(),
-      ).rejects.toThrow(`Low nullifier witness not found for nullifier ${randomNullifier.toString()} at block`);
+      ).rejects.toThrow(`Nullifier witness not found for nullifier ${randomNullifier.toString()} at block`);
 
       await expect(contract.methods.test_nullifier_inclusion(randomNullifier, false, 0n).send().wait()).rejects.toThrow(
-        `Low nullifier witness not found for nullifier ${randomNullifier.toString()} at block`,
+        `Nullifier witness not found for nullifier ${randomNullifier.toString()} at block`,
       );
     });
   });

yarn-project/simulator/src/acvm/oracle/oracle.ts

@@ -99,9 +99,7 @@ export class Oracle {
 
     const witness = await this.typedOracle.getNullifierMembershipWitness(parsedBlockNumber, parsedNullifier);
     if (!witness) {
-      throw new Error(
-        `Low nullifier witness not found for nullifier ${parsedNullifier} at block ${parsedBlockNumber}.`,
-      );
+      throw new Error(`Nullifier witness not found for nullifier ${parsedNullifier} at block ${parsedBlockNumber}.`);
     }
     return witness.toFields().map(toACVMField);
   }

yarn-project/simulator/src/client/private_execution.test.ts

@@ -337,7 +337,7 @@ describe('Private Execution test suite', () => {
     });
 
     it('should run the create_note function', async () => {
-      const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'create_note');
+      const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'create_note_no_init_check');
 
       const result = await runSimulator({ args: [owner, 140], artifact });
 
@@ -364,7 +364,7 @@ describe('Private Execution test suite', () => {
 
     it('should run the destroy_and_create function', async () => {
       const amountToTransfer = 100n;
-      const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create');
+      const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create_no_init_check');
 
       const storageSlot = computeSlotForMapping(new Fr(1n), owner);
       const recipientStorageSlot = computeSlotForMapping(new Fr(1n), recipient);
@@ -411,8 +411,7 @@ describe('Private Execution test suite', () => {
       expect(changeNote.note.items[0]).toEqual(new Fr(40n));
 
       const readRequests = sideEffectArrayToValueArray(
-        // We remove the first element which is the read request for the initialization commitment
-        nonEmptySideEffects(result.callStackItem.publicInputs.readRequests.slice(1)),
+        nonEmptySideEffects(result.callStackItem.publicInputs.readRequests),
       );
 
       expect(readRequests).toHaveLength(consumedNotes.length);
@@ -422,7 +421,7 @@ describe('Private Execution test suite', () => {
     it('should be able to destroy_and_create with dummy notes', async () => {
       const amountToTransfer = 100n;
       const balance = 160n;
-      const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create');
+      const artifact = getFunctionArtifact(StatefulTestContractArtifact, 'destroy_and_create_no_init_check');
 
       const storageSlot = computeSlotForMapping(new Fr(1n), owner);
       const noteTypeId = new Fr(869710811710178111116101n); // ValueNote