feat: assert initialization when reading PublicImmutable from private and utility context (#19957)

Fixes #15703

 Summary

- Adds initialization check when reading `PublicImmutable` from private
and utility contexts by asserting the initialization nullifier exists
- Updates documentation to explain how initialization verification works
differently in private vs public contexts
  - Fixes test imports and expected error messages

Author: AztecBot

noir-projects/aztec-nr/aztec/src/state_vars/public_immutable.nr

@@ -1,8 +1,16 @@
-use crate::{context::{PrivateContext, PublicContext, UtilityContext}, state_vars::StateVariable, utils::WithHash};
+use crate::{
+    context::{PrivateContext, PublicContext, UtilityContext},
+    nullifier::utils::compute_nullifier_existence_request,
+    oracle::nullifiers::check_nullifier_exists,
+    state_vars::StateVariable,
+    utils::WithHash,
+};
 use crate::protocol::{
     constants::DOM_SEP__INITIALIZATION_NULLIFIER, hash::poseidon2_hash_with_separator, traits::Packable,
 };
 
+mod test;
+
 /// Immutable public values.
 ///
 /// This is one of the most basic public state variables. It is similar to an `immutable` or `constant` Solidity state
@@ -249,9 +257,15 @@ impl<T> PublicImmutable<T, UtilityContext> {
     where
         T: Packable + Eq,
     {
-        // TODO(#15703): this fn should fail if the variable is not initialized
+        assert(self.is_initialized(), "Trying to read from uninitialized PublicImmutable");
         WithHash::utility_public_storage_read(self.context, self.storage_slot)
     }
+
+    /// Returns true if the `PublicImmutable` has been initialized.
+    pub unconstrained fn is_initialized(self) -> bool {
+        let nullifier = self.compute_initialization_inner_nullifier();
+        check_nullifier_exists(nullifier)
+    }
 }
 
 impl<T> PublicImmutable<T, &mut PrivateContext> {
@@ -270,10 +284,11 @@ impl<T> PublicImmutable<T, &mut PrivateContext> {
     ///
     /// ## Cost
     ///
-    /// A historical public storage read at the anchor block is performed for a single storage slot, **regardless of
-    /// `T`'s packed length**. This is because [PublicImmutable::initialize] stores not just the value but also its
-    /// hash: this function obtains the preimage from an oracle and proves that it matches the hash from public
-    /// storage.
+    /// A nullifier existence request is pushed to the context, which will be verified by the kernel circuit.
+    /// Additionally, a historical public storage read at the anchor block is performed for a single storage slot,
+    /// **regardless of `T`'s packed length**. This is because [PublicImmutable::initialize] stores not just the value
+    /// but also its hash: this function obtains the preimage from an oracle and proves that it matches the hash from
+    /// public storage.
     ///
     /// Because of this reason it is convenient to group together all of a contract's public immutable values that are
     /// read privately in a single type `T`:
@@ -302,7 +317,17 @@ impl<T> PublicImmutable<T, &mut PrivateContext> {
     where
         T: Packable + Eq,
     {
-        // TODO(#15703): this fn should fail if the variable is not initialized
+        let nullifier = self.compute_initialization_inner_nullifier();
+
+        // Safety: We use this check to be able to test this function works properly on synthetic envs
+        // like TXE. We assert the returned value only to provide a clear error message. The actual
+        // constrained check that the nullifier exists is done below with `assert_nullifier_exists`
+        // We should improve our synthetic envs because this check forces an unnecesary roundtrip
+        let nullifier_exists = unsafe { check_nullifier_exists(nullifier) };
+        assert(nullifier_exists, "Trying to read from uninitialized PublicImmutable");
+
+        let nullifier_existence_request = compute_nullifier_existence_request(nullifier, self.context.this_address());
+        self.context.assert_nullifier_exists(nullifier_existence_request);
         WithHash::historical_public_storage_read(
             self.context.get_anchor_block_header(),
             self.context.this_address(),

noir-projects/aztec-nr/aztec/src/state_vars/public_immutable/test.nr

@@ -1,6 +1,8 @@
-use crate::{context::{PrivateContext, PublicContext, UtilityContext}, state_vars::public_immutable::PublicImmutable};
+use crate::{
+    context::{PrivateContext, PublicContext, UtilityContext},
+    state_vars::{public_immutable::PublicImmutable, StateVariable},
+};
 use crate::test::{helpers::test_environment::TestEnvironment, mocks::MockStruct};
-use std::mem::zeroed;
 
 global storage_slot: Field = 7;
 
@@ -99,25 +101,23 @@ unconstrained fn read_uninitialized_public_fails() {
     });
 }
 
-// TODO(#15703): this test should be made to fail
-#[test]
-unconstrained fn read_uninitialized_private_returns_default() {
+#[test(should_fail_with = "Trying to read from uninitialized PublicImmutable")]
+unconstrained fn read_uninitialized_private_fails() {
     let env = TestEnvironment::new();
 
     env.private_context(|context| {
         let state_var = in_private(context);
-        assert_eq(state_var.read(), zeroed());
+        let _ = state_var.read();
     });
 }
 
-// TODO(#15703): this test should be made to fail
-#[test]
-unconstrained fn read_uninitialized_utility_returns_default() {
+#[test(should_fail_with = "Trying to read from uninitialized PublicImmutable")]
+unconstrained fn read_uninitialized_utility_fails() {
     let env = TestEnvironment::new();
 
     env.utility_context(|context| {
         let state_var = in_utility(context);
-        assert_eq(state_var.read(), zeroed());
+        let _ = state_var.read();
     });
 }
 

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

@@ -39,11 +39,10 @@ describe('e2e_state_vars', () => {
   afterAll(() => teardown());
 
   describe('PublicImmutable', () => {
-    it('private read of uninitialized PublicImmutable', async () => {
-      const s = await contract.methods.get_public_immutable().simulate({ from: defaultAccountAddress });
-
-      // Send the transaction and wait for it to be mined (wait function throws if the tx is not mined)
-      await contract.methods.match_public_immutable(s.account, s.value).send({ from: defaultAccountAddress });
+    it('private read of uninitialized PublicImmutable should fail', async () => {
+      await expect(
+        contract.methods.get_public_immutable_constrained_private().simulate({ from: defaultAccountAddress }),
+      ).rejects.toThrow('Trying to read from uninitialized PublicImmutable');
     });
 
     it('initialize and read PublicImmutable', async () => {
@@ -57,7 +56,7 @@ describe('e2e_state_vars', () => {
       expect(read).toEqual({ account: defaultAccountAddress, value: read.value });
     });
 
-    it('private read of PublicImmutable', async () => {
+    it('private read of initialized PublicImmutable', async () => {
       // Reads the value using a utility function checking the return values with:
       // 1. A constrained private function that reads it directly
       // 2. A constrained private function that calls another private function that reads.