chore: apply where statement to impls instead of fns (#7433)

Now that we can have a `where` that [affects an entire
`impl`](noir-lang/noir#4508), I thought I'd
give this a try in the state variables. Often I've felt tricked by these
implementations, since when trying to work with them the requirements
for a given generic `T` seem fairly low, but then they add up as I start
calling more and more functions, which each add their own trait bounds.
The interface ends up feeling dishonest and not really showing all you
need to do (all the traits that must be implemented) in order to be able
to use the thing.

With this change, the entire impl now requests up front all trait
bounds, though it does mean we're a bit more restrictive than strictly
needed. I don't think this is an issue - yes, you don't need to be able
to serialize in order to read a public mutable, but you can only read if
you write before, and that requires serialization. So all in all it
seems like we always end up indirectly requiring all traits.

---------

Co-authored-by: benesjan <janbenes1234@gmail.com>

aztec/src/state_vars/private_mutable.nr

@@ -43,12 +43,9 @@ impl<Note, Context> PrivateMutable<Note, Context> {
     }
 }
 
-impl<Note> PrivateMutable<Note, &mut PrivateContext> {
+impl<Note, N, M> PrivateMutable<Note, &mut PrivateContext> where Note: NoteInterface<N, M> {
     // docs:start:initialize
-    pub fn initialize<N, M>(
-        self,
-        note: &mut Note
-    ) -> NoteEmission<Note> where Note: NoteInterface<N, M> {
+    pub fn initialize(self, note: &mut Note) -> NoteEmission<Note> {
         // Nullify the storage slot.
         let nullifier = self.compute_initialization_nullifier();
         self.context.push_nullifier(nullifier, 0);
@@ -58,10 +55,7 @@ impl<Note> PrivateMutable<Note, &mut PrivateContext> {
     // docs:end:initialize
 
     // docs:start:replace
-    pub fn replace<N, M>(
-        self,
-        new_note: &mut Note
-    ) -> NoteEmission<Note>  where Note: NoteInterface<N, M> {
+    pub fn replace(self, new_note: &mut Note) -> NoteEmission<Note> {
         let prev_note: Note = get_note(self.context, self.storage_slot);
 
         // Nullify previous note.
@@ -72,10 +66,7 @@ impl<Note> PrivateMutable<Note, &mut PrivateContext> {
     }
     // docs:end:replace
 
-    pub fn initialize_or_replace<N, M>(
-        self,
-        note: &mut Note
-    ) -> NoteEmission<Note>  where Note: NoteInterface<N, M> {
+    pub fn initialize_or_replace(self, note: &mut Note) -> NoteEmission<Note> {
         let is_initialized = check_nullifier_exists(self.compute_initialization_nullifier());
 
         // check_nullifier_exists() is an unconstrained function - we can constrain a true value by providing an 
@@ -96,7 +87,7 @@ impl<Note> PrivateMutable<Note, &mut PrivateContext> {
     }
 
     // docs:start:get_note
-    pub fn get_note<N, M>(self) -> NoteEmission<Note> where Note: NoteInterface<N, M> {
+    pub fn get_note(self) -> NoteEmission<Note> {
         let mut note = get_note(self.context, self.storage_slot);
 
         // Nullify current note to make sure it's reading the latest note.
@@ -109,14 +100,14 @@ impl<Note> PrivateMutable<Note, &mut PrivateContext> {
     // docs:end:get_note
 }
 
-impl<Note> PrivateMutable<Note, UnconstrainedContext> {
+impl<Note, N, M> PrivateMutable<Note, UnconstrainedContext> where Note: NoteInterface<N, M> {
     unconstrained pub fn is_initialized(self) -> bool {
         let nullifier = self.compute_initialization_nullifier();
         check_nullifier_exists(nullifier)
     }
 
     // docs:start:view_note
-    unconstrained pub fn view_note<N, M>(self) -> Note where Note: NoteInterface<N, M> {
+    unconstrained pub fn view_note(self) -> Note {
         let mut options = NoteViewerOptions::new();
         view_notes(self.storage_slot, options.set_limit(1)).get(0)
     }

aztec/src/state_vars/private_set.nr

@@ -26,23 +26,23 @@ impl<Note, Context> PrivateSet<Note, Context> {
     // docs:end:new
 }
 
-impl<Note> PrivateSet<Note, &mut PublicContext> {
+impl<Note, N, M> PrivateSet<Note, &mut PublicContext> where Note: NoteInterface<N, M> {
     // docs:start:insert_from_public
-    pub fn insert_from_public<N, M>(self, note: &mut Note) where Note: NoteInterface<N, M> {
+    pub fn insert_from_public(self, note: &mut Note) {
         create_note_hash_from_public(self.context, self.storage_slot, note);
     }
     // docs:end:insert_from_public
 }
 
-impl<Note> PrivateSet<Note, &mut PrivateContext> {
+impl<Note, N, M> PrivateSet<Note, &mut PrivateContext> where Note: NoteInterface<N, M> + Eq {
     // docs:start:insert
-    pub fn insert<N, M>(self, note: &mut Note) -> NoteEmission<Note> where Note: NoteInterface<N, M> {
+    pub fn insert(self, note: &mut Note) -> NoteEmission<Note> {
         create_note(self.context, self.storage_slot, note)
     }
     // docs:end:insert
 
     // docs:start:remove
-    pub fn remove<N, M>(self, note: Note) where Note: NoteInterface<N, M> {
+    pub fn remove(self, note: Note) {
         let note_hash = compute_note_hash_for_read_request(note);
         let has_been_read = self.context.note_hash_read_requests.any(|r: ReadRequest| r.value == note_hash);
         assert(has_been_read, "Can only remove a note that has been read from the set.");
@@ -52,21 +52,21 @@ impl<Note> PrivateSet<Note, &mut PrivateContext> {
     // docs:end:remove
 
     // docs:start:get_notes
-    pub fn get_notes<N, M, FILTER_ARGS>(
+    pub fn get_notes<FILTER_ARGS>(
         self,
         options: NoteGetterOptions<Note, N, M, FILTER_ARGS>
-    ) -> BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL> where Note: NoteInterface<N, M> + Eq {
+    ) -> BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL> {
         get_notes(self.context, self.storage_slot, options)
     }
     // docs:end:get_notes
 }
 
-impl<Note> PrivateSet<Note, UnconstrainedContext> {
+impl<Note, N, M> PrivateSet<Note, UnconstrainedContext> where Note: NoteInterface<N, M> {
     // docs:start:view_notes
-    unconstrained pub fn view_notes<N, M>(
+    unconstrained pub fn view_notes(
         self,
         options: NoteViewerOptions<Note, N, M>
-    ) -> BoundedVec<Note, MAX_NOTES_PER_PAGE> where Note: NoteInterface<N, M> {
+    ) -> BoundedVec<Note, MAX_NOTES_PER_PAGE> {
         view_notes(self.storage_slot, options)
     }
     // docs:end:view_notes

aztec/src/state_vars/public_immutable.nr

@@ -27,9 +27,9 @@ impl<T, Context> PublicImmutable<T, Context> {
     // docs:end:public_immutable_struct_new
 }
 
-impl <T> PublicImmutable<T, &mut PublicContext> {
+impl <T, T_SERIALIZED_LEN> PublicImmutable<T, &mut PublicContext> where T: Serialize<T_SERIALIZED_LEN> + Deserialize<T_SERIALIZED_LEN> {
     // docs:start:public_immutable_struct_write
-    pub fn initialize<T_SERIALIZED_LEN>(self, value: T) where T: Serialize<T_SERIALIZED_LEN> {
+    pub fn initialize(self, value: T) {
         // 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 init_field: Field = self.context.storage_read(initialization_slot);
@@ -43,14 +43,14 @@ impl <T> PublicImmutable<T, &mut PublicContext> {
 
     // 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> {
+    pub fn read(self) -> T {
         self.context.storage_read(self.storage_slot)
     }
     // docs:end:public_immutable_struct_read
 }
 
-impl<T> PublicImmutable<T, UnconstrainedContext> {
-    unconstrained pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
+impl<T, T_SERIALIZED_LEN> PublicImmutable<T, UnconstrainedContext>where T: Deserialize<T_SERIALIZED_LEN> {
+    unconstrained pub fn read(self) -> T {
         self.context.storage_read(self.storage_slot)
     }
 }

aztec/src/state_vars/public_mutable.nr

@@ -25,22 +25,22 @@ impl<T, Context> PublicMutable<T, Context> {
     // docs:end:public_mutable_struct_new
 }
 
-impl<T> PublicMutable<T, &mut PublicContext> {
+impl<T, T_SERIALIZED_LEN> PublicMutable<T, &mut PublicContext> where T: Serialize<T_SERIALIZED_LEN> + Deserialize<T_SERIALIZED_LEN> {
     // docs:start:public_mutable_struct_read
-    pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
+    pub fn read(self) -> T {
         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> {
+    pub fn write(self, value: T) {
         self.context.storage_write(self.storage_slot, value);
     }
     // docs:end:public_mutable_struct_write
 }
 
-impl<T> PublicMutable<T, UnconstrainedContext> {
-    unconstrained pub fn read<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
+impl<T, T_SERIALIZED_LEN> PublicMutable<T, UnconstrainedContext> where T: Deserialize<T_SERIALIZED_LEN> {
+    unconstrained pub fn read(self) -> T {
         self.context.storage_read(self.storage_slot)
     }
 }

aztec/src/state_vars/shared_immutable.nr

@@ -23,9 +23,9 @@ impl<T, Context> SharedImmutable<T, Context> {
     }
 }
 
-impl<T> SharedImmutable<T, &mut PublicContext> {
+impl<T, T_SERIALIZED_LEN> SharedImmutable<T, &mut PublicContext> where T: Serialize<T_SERIALIZED_LEN> + Deserialize<T_SERIALIZED_LEN> {
     // Intended to be only called once. 
-    pub fn initialize<T_SERIALIZED_LEN>(self, value: T) where T: Serialize<T_SERIALIZED_LEN> {
+    pub fn initialize(self, value: T) {
         // 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 init_field: Field = self.context.storage_read(initialization_slot);
@@ -36,19 +36,19 @@ impl<T> SharedImmutable<T, &mut PublicContext> {
         self.context.storage_write(self.storage_slot, value);
     }
 
-    pub fn read_public<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
+    pub fn read_public(self) -> T {
         self.context.storage_read(self.storage_slot)
     }
 }
 
-impl<T> SharedImmutable<T, UnconstrainedContext> {
-    unconstrained pub fn read_public<T_SERIALIZED_LEN>(self) -> T where T: Deserialize<T_SERIALIZED_LEN> {
+impl<T, T_SERIALIZED_LEN> SharedImmutable<T, UnconstrainedContext> where T: Serialize<T_SERIALIZED_LEN> + Deserialize<T_SERIALIZED_LEN> {
+    unconstrained pub fn read_public(self) -> T {
         self.context.storage_read(self.storage_slot)
     }
 }
 
-impl<T> SharedImmutable<T, &mut PrivateContext> {
-    pub fn read_private<T_SERIALIZED_LEN>(self) -> T  where T: Deserialize<T_SERIALIZED_LEN> {
+impl<T, T_SERIALIZED_LEN> SharedImmutable<T, &mut PrivateContext> where T: Serialize<T_SERIALIZED_LEN> + Deserialize<T_SERIALIZED_LEN> {
+    pub fn read_private(self) -> T {
         let header = self.context.get_header();
         let mut fields = [0; T_SERIALIZED_LEN];
 

aztec/src/state_vars/shared_mutable/shared_mutable.nr

@@ -13,7 +13,7 @@ use dep::std::unsafe::zeroed;
 
 mod test;
 
-struct SharedMutable<T, INITIAL_DELAY, Context> {
+struct SharedMutable<T, let INITIAL_DELAY: u32, Context> {
     context: Context,
     storage_slot: Field,
 }
@@ -51,7 +51,7 @@ fn concat_arrays<N, M, O>(arr_n: [Field; N], arr_m: [Field; M]) -> [Field; O] {
 // future, so that they can guarantee the value will not have possibly changed by then (because of the delay).
 // The delay for changing a value is initially equal to INITIAL_DELAY, but can be changed by calling
 // `schedule_delay_change`.
-impl<T, INITIAL_DELAY, Context> SharedMutable<T, INITIAL_DELAY, Context> {
+impl<T, INITIAL_DELAY, Context> SharedMutable<T, INITIAL_DELAY, Context> where T: ToField + FromField + Eq {
     pub fn new(context: Context, storage_slot: Field) -> Self {
         assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
         Self { context, storage_slot }
@@ -60,7 +60,7 @@ impl<T, INITIAL_DELAY, Context> SharedMutable<T, INITIAL_DELAY, Context> {
     fn hash_scheduled_data(
         value_change: ScheduledValueChange<T>,
         delay_change: ScheduledDelayChange<INITIAL_DELAY>
-    ) -> Field where T: ToField {
+    ) -> Field {
         // TODO(#5491 and https://github.com/noir-lang/noir/issues/4784): update this so that we don't need to rely on
         // ScheduledValueChange serializing to 3 and ScheduledDelayChange serializing to 1
         let concatenated: [Field; 4] = concat_arrays(value_change.serialize(), delay_change.serialize());
@@ -93,7 +93,7 @@ impl<T, INITIAL_DELAY, Context> SharedMutable<T, INITIAL_DELAY, Context> {
         self,
         header: Header,
         address: AztecAddress
-    ) -> (ScheduledValueChange<T>, ScheduledDelayChange<INITIAL_DELAY>, u32) where T: FromField + ToField + Eq {
+    ) -> (ScheduledValueChange<T>, ScheduledDelayChange<INITIAL_DELAY>, u32) {
         let historical_block_number = header.global_variables.block_number as u32;
 
         // We could simply produce historical inclusion proofs for both the ScheduledValueChange and
@@ -127,8 +127,8 @@ impl<T, INITIAL_DELAY, Context> SharedMutable<T, INITIAL_DELAY, Context> {
     }
 }
 
-impl<T, INITIAL_DELAY> SharedMutable<T, INITIAL_DELAY, &mut PublicContext> {
-    pub fn schedule_value_change(self, new_value: T) where T: ToField {
+impl<T, INITIAL_DELAY> SharedMutable<T, INITIAL_DELAY, &mut PublicContext> where T: ToField + FromField + Eq {
+    pub fn schedule_value_change(self, new_value: T) {
         let mut value_change = self.read_value_change();
         let delay_change = self.read_delay_change();
 
@@ -143,7 +143,7 @@ impl<T, INITIAL_DELAY> SharedMutable<T, INITIAL_DELAY, &mut PublicContext> {
         self.write(value_change, delay_change);
     }
 
-    pub fn schedule_delay_change(self, new_delay: u32) where T: ToField {
+    pub fn schedule_delay_change(self, new_delay: u32) {
         let mut delay_change = self.read_delay_change();
 
         let block_number = self.context.block_number() as u32;
@@ -183,7 +183,7 @@ impl<T, INITIAL_DELAY> SharedMutable<T, INITIAL_DELAY, &mut PublicContext> {
         self,
         value_change: ScheduledValueChange<T>,
         delay_change: ScheduledDelayChange<INITIAL_DELAY>
-    ) where T: ToField {
+    ) {
         // Whenever we write to public storage, we write both the value change and delay change as well as the hash of
         // them both. This guarantees that the hash is always kept up to date.
         // While this makes for more costly writes, it also makes private proofs much simpler because they only need to
@@ -199,8 +199,8 @@ impl<T, INITIAL_DELAY> SharedMutable<T, INITIAL_DELAY, &mut PublicContext> {
     }
 }
 
-impl<T, INITIAL_DELAY> SharedMutable<T, INITIAL_DELAY, &mut PrivateContext> {
-    pub fn get_current_value_in_private(self) -> T where T: FromField + ToField + Eq {
+impl<T, INITIAL_DELAY> SharedMutable<T, INITIAL_DELAY, &mut PrivateContext> where T: ToField + FromField + Eq {
+    pub fn get_current_value_in_private(self) -> T {
         // When reading the current value in private we construct a historical state proof for the public value.
         // However, since this value might change, we must constrain the maximum transaction block number as this proof
         // will only be valid for however many blocks we can ensure the value will not change, which will depend on the
@@ -229,12 +229,12 @@ unconstrained fn get_public_storage_hints<T, INITIAL_DELAY>(
     address: AztecAddress,
     storage_slot: Field,
     block_number: u32
-) -> (ScheduledValueChange<T>, ScheduledDelayChange<INITIAL_DELAY>) {
+) -> (ScheduledValueChange<T>, ScheduledDelayChange<INITIAL_DELAY>) where T: ToField + FromField + Eq {
     // This function cannot be part of the &mut PrivateContext impl because that'd mean that by passing `self` we'd also
     // be passing a mutable reference to an unconstrained function, which is not allowed. We therefore create a dummy
     // state variable here so that we can access the methods to compute storage slots. This will all be removed in the
     // future once we do proper storage slot allocation (#5492).
-    let dummy = SharedMutable::new((), storage_slot);
+    let dummy: SharedMutable<T, INITIAL_DELAY, ()> = SharedMutable::new((), storage_slot);
 
     (
         storage_read(address, dummy.get_value_change_storage_slot(), block_number), storage_read(address, dummy.get_delay_change_storage_slot(), block_number)

aztec/src/state_vars/shared_mutable/shared_mutable_private_getter.nr

@@ -25,7 +25,7 @@ struct SharedMutablePrivateGetter<T, INITIAL_DELAY> {
 
 // We have this as a view-only interface to reading Shared Mutables in other contracts.
 // Currently the Shared Mutable does not support this. We can adapt SharedMutable at a later date
-impl<T, INITIAL_DELAY> SharedMutablePrivateGetter<T, INITIAL_DELAY> {
+impl<T, INITIAL_DELAY> SharedMutablePrivateGetter<T, INITIAL_DELAY>  where T: FromField + ToField + Eq {
     pub fn new(
         context: &mut PrivateContext,
         other_contract_address: AztecAddress,
@@ -36,7 +36,7 @@ impl<T, INITIAL_DELAY> SharedMutablePrivateGetter<T, INITIAL_DELAY> {
         Self { context, other_contract_address, storage_slot, _dummy: [0; INITIAL_DELAY] }
     }
 
-    pub fn get_value_in_private(self, header: Header) -> T where T: FromField + ToField + Eq {
+    pub fn get_value_in_private(self, header: Header) -> T {
         // We create a dummy SharedMutable state variable so that we can reuse its historical_read_from_public_storage
         // method, greatly reducing code duplication. 
         let dummy: SharedMutable<T, INITIAL_DELAY, ()> = SharedMutable::new((), self.storage_slot);