feat: optimizing SharedMutable storage slots

barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp

@@ -207,10 +207,12 @@ void AvmTraceBuilder::validate_contract_instance_current_class_id(uint32_t clk,
 
     // update_preimage is validated, now validate the contract class id
     FF expected_current_class_id;
-    const FF prev_value = instance.update_preimage[SCHEDULED_DELAY_CHANGE_PCKD_LEN + 0];
-    const FF next_value = instance.update_preimage[SCHEDULED_DELAY_CHANGE_PCKD_LEN + 1];
-    const uint32_t block_of_change =
-        static_cast<uint32_t>(instance.update_preimage[SCHEDULED_DELAY_CHANGE_PCKD_LEN + 2]);
+    const FF prev_value = instance.update_preimage[1];
+    const FF next_value = instance.update_preimage[2];
+    // block_of_change is packed into the first field along with ScheduledDelayChange as [ sdc.pre_inner: u32 |
+    // sdc.pre_is_some: u8 | sdc.post_inner: u32 | sdc.post_is_some: u8 | sdc.block_of_change: u32 |
+    // svc.block_of_change: u32 ]. Extract just the lowest 32 bits which contain svc.block_of_change by casting to u32
+    const uint32_t block_of_change = static_cast<uint32_t>(instance.update_preimage[0]);
 
     // Fourth item is related to update delays which we don't care.
     if (static_cast<uint32_t>(public_inputs.global_variables.block_number) < block_of_change) {

barretenberg/cpp/src/barretenberg/vm/aztec_constants.hpp

@@ -147,8 +147,7 @@
 #define AVM_EMITNULLIFIER_BASE_DA_GAS 512
 #define AVM_SENDL2TOL1MSG_BASE_DA_GAS 512
 #define AVM_EMITUNENCRYPTEDLOG_DYN_DA_GAS 512
-#define SCHEDULED_DELAY_CHANGE_PCKD_LEN 1
-#define UPDATES_SHARED_MUTABLE_VALUES_LEN 4
+#define UPDATES_SHARED_MUTABLE_VALUES_LEN 3
 #define GENERATOR_INDEX__NOTE_HASH_NONCE 2
 #define GENERATOR_INDEX__UNIQUE_NOTE_HASH 3
 #define GENERATOR_INDEX__SILOED_NOTE_HASH 4

l1-contracts/src/core/libraries/ConstantsGen.sol

@@ -312,7 +312,5 @@ library Constants {
   uint256 internal constant DEFAULT_UPDATE_DELAY = 3600;
   uint256 internal constant MINIMUM_UPDATE_DELAY = 25;
   uint256 internal constant UPDATES_VALUE_SIZE = 1;
-  uint256 internal constant UPDATES_SCHEDULED_VALUE_CHANGE_LEN = 3;
-  uint256 internal constant SCHEDULED_DELAY_CHANGE_PCKD_LEN = 1;
-  uint256 internal constant UPDATES_SHARED_MUTABLE_VALUES_LEN = 4;
+  uint256 internal constant UPDATES_SHARED_MUTABLE_VALUES_LEN = 3;
 }

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

@@ -46,14 +46,6 @@ where
         assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
         Self { context, storage_slot }
     }
-
-    fn get_value_change_storage_slot(self) -> Field {
-        SharedMutableValues::<T, INITIAL_DELAY>::get_value_change_storage_slot(self.storage_slot)
-    }
-
-    fn get_delay_change_storage_slot(self) -> Field {
-        SharedMutableValues::<T, INITIAL_DELAY>::get_delay_change_storage_slot(self.storage_slot)
-    }
 }
 
 impl<T, let INITIAL_DELAY: u32, let N: u32> SharedMutable<T, INITIAL_DELAY, &mut PublicContext>
@@ -111,11 +103,22 @@ where
     }
 
     fn read_value_change(self) -> ScheduledValueChange<T> {
-        self.context.storage_read(self.get_value_change_storage_slot())
+        // We don't read ScheduledValueChange directly by having it implement Packable because ScheduledValueChange
+        // and ScheduledDelayChange are packed together (sdc and svc.block_of_change are stored in the same slot).
+        let packed = self.context.storage_read(self.storage_slot);
+        SharedMutableValues::unpack_value_change(packed)
     }
 
     fn read_delay_change(self) -> ScheduledDelayChange<INITIAL_DELAY> {
-        self.context.storage_read(self.get_delay_change_storage_slot())
+        // Since all ScheduledDelayChange member are packed into a single field, we can read a single storage slot
+        // here and skip the ones that correspond to ScheduledValueChange members. We are abusing the fact that
+        // the field containing the ScheduledDelayChange data is the first one in the storage layout - otherwise we'd
+        // need to offset the storage slot to get the position where it'd land.
+        // We don't read ScheduledDelayChange directly by having it implement Packable because
+        // ScheduledValueChange and ScheduledDelayChange are packed together (sdc and svc.block_of_change are
+        // stored in the same slot).
+        let packed = self.context.storage_read(self.storage_slot);
+        SharedMutableValues::<T, INITIAL_DELAY>::unpack_delay_change(packed)
     }
 
     fn write(
@@ -186,12 +189,10 @@ where
     T: Packable<N> + Eq,
 {
     pub unconstrained fn get_current_value(self) -> T {
-        let value_change: ScheduledValueChange<T> = WithHash::unconstrained_public_storage_read(
-            self.context,
-            self.get_value_change_storage_slot(),
-        );
+        let smv: SharedMutableValues<T, INITIAL_DELAY> =
+            WithHash::unconstrained_public_storage_read(self.context, self.storage_slot);
 
         let block_number = self.context.block_number() as u32;
-        value_change.get_current_at(block_number)
+        smv.svc.get_current_at(block_number)
     }
 }

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

@@ -61,11 +61,18 @@ unconstrained fn test_storage_slot_allocation() {
 }
 
 fn get_shared_mutable_num_slots() -> Field {
-    let scheduled_delay_packed_len = 1; // There is 1 ScheduledDelayChange stored in storage and it fits into 1 slot.
-    let example_struct_packed_len = 5; // ExampleStruct packs to 5 slots.
+    // ExampleStruct packs to 5 slots.
+    let example_struct_packed_len = 5;
+
+    // ScheduledDelayChange and block_of_change of ScheduledValueChange are stored in a single slot.
+    let sdc_and_block_of_change_of_svc_packed_len = 1;
+
     // There are 2 example structs stored in the packed representation of ScheduledValueChange<ExampleStruct>.
-    let scheduled_value_change_packed_len = (example_struct_packed_len * 2 + 1);
-    let with_hash_additional_slots = 1; // Wrapping the values in WithHash adds 1 slot.
+    let value_change_and_delay_change_packed_len =
+        example_struct_packed_len * 2 + sdc_and_block_of_change_of_svc_packed_len;
+
+    // Wrapping the values in WithHash adds 1 slot.
+    let with_hash_additional_slots = 1;
 
-    scheduled_delay_packed_len + scheduled_value_change_packed_len + with_hash_additional_slots
+    value_change_and_delay_change_packed_len + with_hash_additional_slots
 }