feat!: split storage access oracles (#7237)

Part of #7230.
This splits the storage access oracles that transpile into opcodes, and
the ones that are resolved as actual oracle calls by PXE. I took the
liberty to introduce some slight improvements, such as removing unused
`length` params, write return values, and making the default fns call
serialize and deserialize, mirroring the actual usage they had
throughout our codebase and cleaning up the callsites.

The oracle improvements proposed in #7230 (taking arbitrary addresses
and block number) will come in a separate PR, as those require further
changes to PXE etc.

avm-transpiler/src/transpile.rs

@@ -263,8 +263,8 @@ fn handle_foreign_call(
         "avmOpcodeGetContractInstance" => {
             handle_get_contract_instance(avm_instrs, destinations, inputs);
         }
-        "storageRead" => handle_storage_read(avm_instrs, destinations, inputs),
-        "storageWrite" => handle_storage_write(avm_instrs, destinations, inputs),
+        "avmOpcodeStorageRead" => handle_storage_read(avm_instrs, destinations, inputs),
+        "avmOpcodeStorageWrite" => handle_storage_write(avm_instrs, destinations, inputs),
         "debugLog" => handle_debug_log(avm_instrs, destinations, inputs),
         // Getters.
         _ if inputs.is_empty() && destinations.len() == 1 => {
@@ -926,7 +926,7 @@ fn handle_storage_write(
     inputs: &Vec<ValueOrArray>,
 ) {
     assert!(inputs.len() == 2);
-    assert!(destinations.len() == 1);
+    assert!(destinations.len() == 0);
 
     let slot_offset_maybe = inputs[0];
     let slot_offset = match slot_offset_maybe {
@@ -992,8 +992,8 @@ fn handle_storage_read(
     inputs: &Vec<ValueOrArray>,
 ) {
     // For the foreign calls we want to handle, we do not want inputs, as they are getters
-    assert!(inputs.len() == 2); // output, len - but we dont use this len - its for the oracle
-    assert!(destinations.len() == 1);
+    assert!(inputs.len() == 1); // storage_slot
+    assert!(destinations.len() == 1); // return values
 
     let slot_offset_maybe = inputs[0];
     let slot_offset = match slot_offset_maybe {

noir-projects/aztec-nr/aztec/src/context/public_context.nr

@@ -166,6 +166,22 @@ impl PublicContext {
     fn da_gas_left(self) -> Field {
         da_gas_left()
     }
+
+    fn raw_storage_read<N>(_self: Self, storage_slot: Field) -> [Field; N] {
+        storage_read(storage_slot)
+    }
+
+    fn storage_read<T, N>(self, storage_slot: Field) -> T where T: Deserialize<N> {
+        T::deserialize(self.raw_storage_read(storage_slot))
+    }
+
+    fn raw_storage_write<N>(_self: Self, storage_slot: Field, values: [Field; N]) {
+        storage_write(storage_slot, values);
+    }
+
+    fn storage_write<T, N>(self, storage_slot: Field, value: T) where T: Serialize<N> {
+        self.raw_storage_write(storage_slot, value.serialize());
+    }
 }
 
 // Helper functions
@@ -258,6 +274,14 @@ unconstrained fn call_static<RET_SIZE>(
     call_static_opcode(gas, address, args, function_selector)
 }
 
+unconstrained fn storage_read<N>(storage_slot: Field) -> [Field; N] {
+    storage_read_opcode(storage_slot)
+}
+
+unconstrained fn storage_write<N>(storage_slot: Field, values: [Field; N]) {
+    storage_write_opcode(storage_slot, values);
+}
+
 impl Empty for PublicContext {
     fn empty() -> Self {
         PublicContext::new(PublicContextInputs::empty())
@@ -345,6 +369,12 @@ unconstrained fn call_static_opcode<RET_SIZE>(
 ) -> ([Field; RET_SIZE], u8) {}
 //    ^ return data      ^ success
 
+#[oracle(avmOpcodeStorageRead)]
+unconstrained fn storage_read_opcode<N>(storage_slot: Field) -> [Field; N] {}
+
+#[oracle(avmOpcodeStorageWrite)]
+unconstrained fn storage_write_opcode<N>(storage_slot: Field, values: [Field; N]) {}
+
 struct FunctionReturns<N> {
     values: [Field; N]
 }

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

@@ -10,7 +10,6 @@ mod event;
 mod oracle;
 mod state_vars;
 mod prelude;
-mod public_storage;
 mod encrypted_logs;
 mod unencrypted_logs;
 use dep::protocol_types;

noir-projects/aztec-nr/aztec/src/oracle/storage.nr

@@ -1,19 +1,43 @@
-use dep::protocol_types::traits::{Deserialize, Serialize};
+use dep::protocol_types::traits::Deserialize;
 
 #[oracle(storageRead)]
-unconstrained fn storage_read_oracle<N>(_storage_slot: Field, _number_of_elements: Field) -> [Field; N] {}
+unconstrained fn storage_read_oracle<N>(storage_slot: Field, length: Field) -> [Field; N] {}
 
-unconstrained fn storage_read_oracle_wrapper<N>(_storage_slot: Field) -> [Field; N] {
-    storage_read_oracle(_storage_slot, N)
+unconstrained pub fn raw_storage_read<N>(storage_slot: Field) -> [Field; N] {
+    storage_read_oracle(storage_slot, N)
 }
 
-pub fn storage_read<N>(storage_slot: Field) -> [Field; N] {
-    storage_read_oracle_wrapper(storage_slot)
+unconstrained pub fn storage_read<T, N>(storage_slot: Field) -> T where T: Deserialize<N> {
+    T::deserialize(raw_storage_read(storage_slot))
 }
 
-#[oracle(storageWrite)]
-unconstrained fn storage_write_oracle<N>(_storage_slot: Field, _values: [Field; N]) -> [Field; N] {}
+mod tests {
+    use crate::oracle::storage::{raw_storage_read, storage_read};
 
-unconstrained pub fn storage_write<N>(storage_slot: Field, fields: [Field; N]) {
-    let _hash = storage_write_oracle(storage_slot, fields);
+    use std::test::OracleMock;
+    use crate::test::mocks::mock_struct::MockStruct;
+
+    #[test]
+    fn test_raw_storage_read() {
+        let slot = 7;
+        let written = MockStruct { a: 13, b: 42 };
+
+        let _ = OracleMock::mock("storageRead").with_params((slot, 2)).returns(written.serialize());
+
+        let read: [Field; 2] = raw_storage_read(slot);
+        assert_eq(read[0], 13);
+        assert_eq(read[1], 42);
+    }
+
+    #[test]
+    fn test_storage_read() {
+        let slot = 7;
+        let written = MockStruct { a: 13, b: 42 };
+
+        let _ = OracleMock::mock("storageRead").with_params((slot, 2)).returns(written.serialize());
+
+        let read: MockStruct = storage_read(slot);
+        assert_eq(read.a, 13);
+        assert_eq(read.b, 42);
+    }
 }

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

@@ -1,5 +1,5 @@
 use crate::{
-    context::{PublicContext, UnconstrainedContext}, oracle::{storage::{storage_read, storage_write}},
+    context::{PublicContext, UnconstrainedContext}, oracle::storage::storage_read,
     state_vars::storage::Storage
 };
 use dep::protocol_types::{constants::INITIALIZATION_SLOT_SEPARATOR, traits::{Deserialize, Serialize}};
@@ -37,32 +37,25 @@ impl <T> PublicImmutable<T, &mut PublicContext> {
 
         // We check that the struct is not yet initialized by checking if the initialization slot is 0
         let initialization_slot = INITIALIZATION_SLOT_SEPARATOR + self.storage_slot;
-        let fields_read: [Field; 1] = storage_read(initialization_slot);
-        assert(fields_read[0] == 0, "PublicImmutable already initialized");
+        let init_field: Field = self.context.storage_read(initialization_slot);
+        assert(init_field == 0, "PublicImmutable already initialized");
 
         // We populate the initialization slot with a non-zero value to indicate that the struct is initialized
-        storage_write(initialization_slot, [0xdead]);
-
-        let fields_write = T::serialize(value);
-        storage_write(self.storage_slot, fields_write);
+        self.context.storage_write(initialization_slot, 0xdead);
+        self.context.storage_write(self.storage_slot, value);
     }
     // docs:end:public_immutable_struct_write
 
     // Note that we don't access the context, but we do call oracles that are only available in public
     // docs:start:public_immutable_struct_read
     pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
-        let fields = storage_read(self.storage_slot);
-        T::deserialize(fields)
+        self.context.storage_read(self.storage_slot)
     }
     // docs:end:public_immutable_struct_read
 }
 
 impl<T> PublicImmutable<T, UnconstrainedContext> {
-    pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
-        // This looks the same as the &mut PublicContext impl, but is actually very different. In public execution the
-        // storage read oracle gets transpiled to SLOAD opcodes, whereas in unconstrained execution the PXE returns
-        // historical data.
-        let fields = storage_read(self.storage_slot);
-        T::deserialize(fields)
+    unconstrained pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
+        storage_read(self.storage_slot)
     }
 }

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

@@ -1,6 +1,5 @@
 use crate::context::{PublicContext, UnconstrainedContext};
 use crate::oracle::storage::storage_read;
-use crate::oracle::storage::storage_write;
 use dep::protocol_types::traits::{Deserialize, Serialize};
 use crate::state_vars::storage::Storage;
 
@@ -29,25 +28,19 @@ impl<T, Context> PublicMutable<T, Context> {
 impl<T> PublicMutable<T, &mut PublicContext> {
     // docs:start:public_mutable_struct_read
     pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
-        let fields = storage_read(self.storage_slot);
-        T::deserialize(fields)
+        self.context.storage_read(self.storage_slot)
     }
     // docs:end:public_mutable_struct_read
 
     // docs:start:public_mutable_struct_write
     pub fn write<T_SERIALIZED_LEN>(self, value: T) where T: Serialize<T_SERIALIZED_LEN> {
-        let fields = T::serialize(value);
-        storage_write(self.storage_slot, fields);
+        self.context.storage_write(self.storage_slot, value);
     }
     // docs:end:public_mutable_struct_write
 }
 
 impl<T> PublicMutable<T, UnconstrainedContext> {
-    pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
-        // This looks the same as the &mut PublicContext impl, but is actually very different. In public execution the
-        // storage read oracle gets transpiled to SLOAD opcodes, whereas in unconstrained execution the PXE returns
-        // historical data.
-        let fields = storage_read(self.storage_slot);
-        T::deserialize(fields)
+    unconstrained pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
+        storage_read(self.storage_slot)
     }
 }

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

@@ -1,6 +1,6 @@
 use crate::{
-    context::{PrivateContext, PublicContext, UnconstrainedContext},
-    oracle::{storage::{storage_read, storage_write}}, state_vars::storage::Storage
+    context::{PrivateContext, PublicContext, UnconstrainedContext}, oracle::storage::storage_read,
+    state_vars::storage::Storage
 };
 use dep::protocol_types::{constants::INITIALIZATION_SLOT_SEPARATOR, traits::{Deserialize, Serialize}};
 
@@ -33,26 +33,22 @@ impl<T> SharedImmutable<T, &mut PublicContext> {
 
         // We check that the struct is not yet initialized by checking if the initialization slot is 0
         let initialization_slot = INITIALIZATION_SLOT_SEPARATOR + self.storage_slot;
-        let fields_read: [Field; 1] = storage_read(initialization_slot);
-        assert(fields_read[0] == 0, "SharedImmutable already initialized");
+        let init_field: Field = self.context.storage_read(initialization_slot);
+        assert(init_field == 0, "SharedImmutable already initialized");
 
         // We populate the initialization slot with a non-zero value to indicate that the struct is initialized
-        storage_write(initialization_slot, [0xdead]);
-
-        let fields_write = T::serialize(value);
-        storage_write(self.storage_slot, fields_write);
+        self.context.storage_write(initialization_slot, 0xdead);
+        self.context.storage_write(self.storage_slot, value);
     }
 
     pub fn read_public<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
-        let fields = storage_read(self.storage_slot);
-        T::deserialize(fields)
+        self.context.storage_read(self.storage_slot)
     }
 }
 
 impl<T> SharedImmutable<T, UnconstrainedContext> {
-    pub fn read_public<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
-        let fields = storage_read(self.storage_slot);
-        T::deserialize(fields)
+    unconstrained pub fn read_public<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
+        storage_read(self.storage_slot)
     }
 }
 

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

@@ -1,7 +1,6 @@
 use dep::protocol_types::{hash::pedersen_hash, traits::FromField};
 
 use crate::context::{PrivateContext, PublicContext};
-use crate::public_storage;
 use crate::state_vars::{
     storage::Storage,
     shared_mutable::{scheduled_value_change::ScheduledValueChange, scheduled_delay_change::ScheduledDelayChange}
@@ -98,19 +97,19 @@ impl<T, INITIAL_DELAY> SharedMutable<T, INITIAL_DELAY, &mut PublicContext> {
     }
 
     fn read_value_change(self) -> ScheduledValueChange<T> {
-        public_storage::read(self.get_value_change_storage_slot())
+        self.context.storage_read(self.get_value_change_storage_slot())
     }
 
     fn read_delay_change(self) -> ScheduledDelayChange<INITIAL_DELAY> {
-        public_storage::read(self.get_delay_change_storage_slot())
+        self.context.storage_read(self.get_delay_change_storage_slot())
     }
 
     fn write_value_change(self, value_change: ScheduledValueChange<T>) {
-        public_storage::write(self.get_value_change_storage_slot(), value_change);
+        self.context.storage_write(self.get_value_change_storage_slot(), value_change);
     }
 
     fn write_delay_change(self, delay_change: ScheduledDelayChange<INITIAL_DELAY>) {
-        public_storage::write(self.get_delay_change_storage_slot(), delay_change);
+        self.context.storage_write(self.get_delay_change_storage_slot(), delay_change);
     }
 }
 

noir-projects/aztec-nr/aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr

@@ -1,7 +1,6 @@
 use dep::protocol_types::{hash::pedersen_hash, traits::FromField, address::AztecAddress, header::Header};
 
 use crate::context::PrivateContext;
-use crate::public_storage;
 use crate::state_vars::{
     storage::Storage,
     shared_mutable::{scheduled_delay_change::ScheduledDelayChange, scheduled_value_change::ScheduledValueChange}

noir-projects/noir-contracts/contracts/token_contract/src/test/utils.nr

@@ -75,8 +75,8 @@ pub fn check_public_balance(token_contract_address: AztecAddress, address: Aztec
 
     let balances_slot = Token::storage().public_balances.slot;
     let address_slot = derive_storage_slot_in_map(balances_slot, address);
-    let fields = storage_read(address_slot);
-    assert(U128::deserialize(fields).to_field() == address_amount, "Public balance is not correct");
+    let amount: U128 = storage_read(address_slot);
+    assert(amount.to_field() == address_amount, "Public balance is not correct");
     cheatcodes::set_contract_address(current_contract_address);
 }
 

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

@@ -94,7 +94,7 @@ describe('e2e_state_vars', () => {
       // Jest executes the tests sequentially and the first call to initialize_shared_immutable was executed
       // in the previous test, so the call below should fail.
       await expect(contract.methods.initialize_shared_immutable(1).prove()).rejects.toThrow(
-        "Assertion failed: SharedImmutable already initialized 'fields_read[0] == 0'",
+        'Assertion failed: SharedImmutable already initialized',
       );
     });
   });
@@ -114,7 +114,7 @@ describe('e2e_state_vars', () => {
       // Jest executes the tests sequentially and the first call to initialize_public_immutable was executed
       // in the previous test, so the call below should fail.
       await expect(contract.methods.initialize_public_immutable(1).prove()).rejects.toThrow(
-        "Assertion failed: PublicImmutable already initialized 'fields_read[0] == 0'",
+        'Assertion failed: PublicImmutable already initialized',
       );
     });
   });