refactor!: nuking no-longer necessary ValueNote utils (#17059)

Fixes #16879

We had utils in the `value_note` package that through time and all the
API refactors just became unnecessary: they were used only in our
testing contracts and I don't think it was a good place for those in the
first place.

For this reason I nuke those in this PR.

This is admittedly a very low-priority task but I had a bit of spare
time so decided to just tackle that.

Author: benesjan

docs/docs/migration_notes.md

@@ -22,6 +22,19 @@ This naming change resulted in quite a few changes and if you've access private
 + let header = context.get_anchor_block_header();
 ```
 
+### Removed ValueNote utils
+
+The `value_note::utils` module has been removed because it was incorrect to have those in the value note package.
+
+For the increment function you can easily just insert the note:
+
+```diff
+- use value_note::utils;
+- utils::increment(storage.notes.at(owner), value, owner, sender);
++ let note = ValueNote::new(value, owner);
++ storage.notes.at(owner).insert(note).emit(&mut context, owner, MessageDelivery.CONSTRAINED_ONCHAIN);
+```
+
 ### PrivateMutable: replace / initialize_or_replace behaviour change
 
 **Motivation:**

noir-projects/aztec-nr/value-note/src/balance_utils.nr

@@ -3,10 +3,12 @@ use dep::aztec::{
     context::UtilityContext, note::note_viewer_options::NoteViewerOptions, state_vars::PrivateSet,
 };
 
+// DEPRECATED: Having this functionality in the value note package is incorrect and will be removed in the future.
 pub unconstrained fn get_balance(set: PrivateSet<ValueNote, UtilityContext>) -> Field {
     get_balance_with_offset(set, 0)
 }
 
+// DEPRECATED: Having this functionality in the value note package is incorrect and will be removed in the future.
 pub unconstrained fn get_balance_with_offset(
     set: PrivateSet<ValueNote, UtilityContext>,
     offset: u32,

noir-projects/aztec-nr/value-note/src/filter.nr

@@ -6,6 +6,7 @@ use aztec::{
     },
 };
 
+// DEPRECATED: Having this functionality in the value note package is incorrect and will be removed in the future.
 pub fn filter_notes_min_sum(
     notes: [Option<RetrievedNote<ValueNote>>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
     min_sum: Field,

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

@@ -1,4 +1,3 @@
 pub mod balance_utils;
 pub mod filter;
-pub mod utils;
 pub mod value_note;

noir-projects/aztec-nr/value-note/src/utils.nr

@@ -3,18 +3,18 @@
 // would alter the metrics we're capturing in the benchmarks, and we want to keep the
 // subject being tested as unmodified as possible so we can detect metric changes that
 
-use dep::aztec::macros::aztec;
+use aztec::macros::aztec;
 
 #[aztec]
 pub contract Benchmarking {
-    use dep::aztec::{
+    use aztec::{
+        macros::{functions::{private, public}, storage::storage},
+        messages::message_delivery::MessageDelivery,
         note::note_getter_options::NoteGetterOptions,
         protocol_types::address::AztecAddress,
         state_vars::{Map, PrivateSet, PublicMutable},
     };
-    use dep::value_note::{utils::increment, value_note::ValueNote};
-
-    use dep::aztec::macros::{functions::{private, public}, storage::storage};
+    use value_note::value_note::ValueNote;
 
     #[storage]
     struct Storage<Context> {
@@ -25,9 +25,12 @@ pub contract Benchmarking {
     // Creates a new value note for the target owner. Use this method to seed an initial set of notes.
     #[private]
     fn create_note(owner: AztecAddress, value: Field) {
-        // docs:start:increment_valuenote
-        increment(storage.notes.at(owner), value, owner);
-        // docs:end:increment_valuenote
+        let note = ValueNote::new(value, owner);
+        storage.notes.at(owner).insert(note).emit(
+            &mut context,
+            owner,
+            MessageDelivery.CONSTRAINED_ONCHAIN,
+        );
     }
     // Deletes a note at a specific index in the set and creates a new one with the same value.
     // We explicitly pass in the note index so we can ensure we consume different notes when sending
@@ -40,7 +43,7 @@ pub contract Benchmarking {
         let mut getter_options = NoteGetterOptions::new();
         let notes = owner_notes.pop_notes(getter_options.set_limit(1).set_offset(index));
         let note = notes.get(0);
-        increment(owner_notes, note.value(), owner);
+        owner_notes.insert(note).emit(&mut context, owner, MessageDelivery.CONSTRAINED_ONCHAIN);
     }
 
     // Reads and writes to public storage and enqueues a call to another public function.

noir-projects/noir-contracts/contracts/test/benchmarking_contract/src/main.nr

@@ -1,20 +1,22 @@
 // A contract used for testing a random hodgepodge of small features from simulator and end-to-end tests.
-use dep::aztec::macros::aztec;
+use aztec::macros::aztec;
 
 #[aztec]
 pub contract StatefulTest {
-    use dep::aztec::macros::{
+    use aztec::macros::{
         functions::{
             initialization_utils::assert_is_initialized_private, initializer, noinitcheck, private,
             public, utility, view,
         },
         storage::storage,
     };
-    use dep::aztec::{
+    use aztec::{
+        messages::message_delivery::MessageDelivery,
+        note::note_getter_options::NoteGetterOptions,
         protocol_types::{abis::function_selector::FunctionSelector, address::AztecAddress},
         state_vars::{Map, PrivateSet, PublicMutable},
     };
-    use dep::value_note::{balance_utils, utils::{decrement, increment}, value_note::ValueNote};
+    use value_note::{balance_utils, value_note::ValueNote};
 
     #[storage]
     struct Storage<Context> {
@@ -49,7 +51,8 @@ pub contract StatefulTest {
     fn create_note(owner: AztecAddress, value: Field) {
         if (value != 0) {
             let loc = storage.notes.at(owner);
-            increment(loc, value, owner);
+            let note = ValueNote::new(value, owner);
+            loc.insert(note).emit(&mut context, owner, MessageDelivery.CONSTRAINED_ONCHAIN);
         }
     }
 
@@ -58,32 +61,39 @@ pub contract StatefulTest {
     fn create_note_no_init_check(owner: AztecAddress, value: Field) {
         if (value != 0) {
             let loc = storage.notes.at(owner);
-            increment(loc, value, owner);
+            let note = ValueNote::new(value, owner);
+            loc.insert(note).emit(&mut context, owner, MessageDelivery.CONSTRAINED_ONCHAIN);
         }
     }
 
     #[private]
-    fn destroy_and_create(recipient: AztecAddress, amount: Field) {
+    fn destroy_and_create(recipient: AztecAddress) {
         assert_is_initialized_private(&mut context);
         let sender = context.msg_sender();
 
         let sender_notes = storage.notes.at(sender);
-        decrement(sender_notes, amount, sender);
+        let _ = sender_notes.pop_notes(NoteGetterOptions::new().set_limit(2));
 
-        let recipient_notes = storage.notes.at(recipient);
-        increment(recipient_notes, amount, recipient);
+        storage.notes.at(recipient).insert(ValueNote::new(92, recipient)).emit(
+            &mut context,
+            recipient,
+            MessageDelivery.CONSTRAINED_ONCHAIN,
+        );
     }
 
     #[private]
     #[noinitcheck]
-    fn destroy_and_create_no_init_check(recipient: AztecAddress, amount: Field) {
+    fn destroy_and_create_no_init_check(recipient: AztecAddress) {
         let sender = context.msg_sender();
 
         let sender_notes = storage.notes.at(sender);
-        decrement(sender_notes, amount, sender);
+        let _ = sender_notes.pop_notes(NoteGetterOptions::new().set_limit(2));
 
-        let recipient_notes = storage.notes.at(recipient);
-        increment(recipient_notes, amount, recipient);
+        storage.notes.at(recipient).insert(ValueNote::new(92, recipient)).emit(
+            &mut context,
+            recipient,
+            MessageDelivery.CONSTRAINED_ONCHAIN,
+        );
     }
 
     #[public]

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

@@ -442,8 +442,6 @@ describe('Private Execution test suite', () => {
     });
 
     it('should run the destroy_and_create function', async () => {
-      const amountToTransfer = 100n;
-
       const storageSlot = await deriveStorageSlotInMap(StatefulTestContractArtifact.storageLayout['notes'].slot, owner);
       const recipientStorageSlot = await deriveStorageSlotInMap(
         StatefulTestContractArtifact.storageLayout['notes'].slot,
@@ -466,7 +464,7 @@ describe('Private Execution test suite', () => {
 
       await insertLeaves(consumedNotes);
 
-      const args = [recipient, amountToTransfer];
+      const args = [recipient];
       const { entrypoint: result } = await runSimulator({
         args,
         artifact: StatefulTestContractArtifact,
@@ -480,25 +478,22 @@ describe('Private Execution test suite', () => {
       const nullifiers = result.publicInputs.nullifiers;
       expect(nullifiers.claimedLength).toBe(consumedNotes.length);
 
-      expect(result.newNotes).toHaveLength(2);
-      const [changeNote, recipientNote] = result.newNotes;
+      expect(result.newNotes).toHaveLength(1);
+      const [recipientNote] = result.newNotes;
       expect(recipientNote.storageSlot).toEqual(recipientStorageSlot);
+      expect(recipientNote.note.items[0]).toEqual(new Fr(92n));
 
       const noteHashes = result.publicInputs.noteHashes;
-      expect(noteHashes.claimedLength).toBe(2);
-
-      expect(recipientNote.note.items[0]).toEqual(new Fr(amountToTransfer));
-      expect(changeNote.note.items[0]).toEqual(new Fr(40n));
+      expect(noteHashes.claimedLength).toBe(1);
 
       const privateLogs = result.publicInputs.privateLogs;
-      expect(privateLogs.claimedLength).toBe(2);
+      expect(privateLogs.claimedLength).toBe(1);
 
       const readRequests = result.publicInputs.noteHashReadRequests;
       expect(readRequests.claimedLength).toBe(consumedNotes.length);
     });
 
     it('should be able to destroy_and_create with dummy notes', async () => {
-      const amountToTransfer = 100n;
       const balance = 160n;
 
       const storageSlot = await deriveStorageSlotInMap(new Fr(1n), owner);
@@ -516,7 +511,7 @@ describe('Private Execution test suite', () => {
 
       await insertLeaves(consumedNotes);
 
-      const args = [recipient, amountToTransfer];
+      const args = [recipient];
       const { entrypoint: result } = await runSimulator({
         args,
         artifact: StatefulTestContractArtifact,
@@ -528,13 +523,12 @@ describe('Private Execution test suite', () => {
       const nullifiers = result.publicInputs.nullifiers;
       expect(nullifiers.claimedLength).toBe(consumedNotes.length);
 
-      expect(result.newNotes).toHaveLength(2);
-      const [changeNote, recipientNote] = result.newNotes;
-      expect(recipientNote.note.items[0]).toEqual(new Fr(amountToTransfer));
-      expect(changeNote.note.items[0]).toEqual(new Fr(balance - amountToTransfer));
+      // We've inserted just one note for recipient with hardcoded value 92
+      expect(result.newNotes).toHaveLength(1);
+      expect(result.newNotes[0].note.items[0]).toEqual(new Fr(92n));
 
       const privateLogs = result.publicInputs.privateLogs;
-      expect(privateLogs.claimedLength).toBe(2);
+      expect(privateLogs.claimedLength).toBe(1);
     });
   });