fix(aztec-nr): check_notes_order lexicographic multi-key sort (#21973)

Author: nchamo

noir-projects/aztec-nr/aztec/src/note/note_getter.nr

@@ -55,18 +55,46 @@ fn check_packed_note<let N: u32>(packed_note: [Field; N], selects: BoundedVec<Op
     }
 }
 
-fn check_notes_order<let N: u32>(fields_0: [Field; N], fields_1: [Field; N], sorts: BoundedVec<Option<Sort>, N>) {
-    for i in 0..sorts.max_len() {
-        if i < sorts.len() {
-            let sort = sorts.get_unchecked(i).unwrap_unchecked();
-            let field_0 = extract_property_value_from_selector(fields_0, sort.property_selector);
-            let field_1 = extract_property_value_from_selector(fields_1, sort.property_selector);
-            let eq = field_0 == field_1;
-            let lt = field_0.lt(field_1);
-            if sort.order == SortOrder.ASC {
-                assert(eq | lt, "Return notes not sorted in ascending order.");
-            } else if !eq {
-                assert(!lt, "Return notes not sorted in descending order.");
+// Asserts that two notes are correctly ordered given the sort criteria.
+//
+// Only the provided criteria are checked -- fields not covered by any criterion are never compared, and if
+// `sort_criteria` is empty no ordering is enforced at all. Criteria are evaluated in priority order: the first
+// criterion where the two notes differ determines whether the pair is correctly ordered. If all sorted fields are
+// equal, any order is accepted.
+//
+// For example, with criteria `[field_0 ASC, field_1 DESC]`:
+//   - `[1, 10], [2, 999]` -> passes: field_0 (1 < 2) satisfies ASC, field_1 is not checked.
+//   - `[1, 10], [1, 5]`  -> passes: field_0 ties, so field_1 (10 > 5) satisfies DESC.
+//   - `[1, 5],  [1, 10]` -> fails:  field_0 ties, so field_1 (5 < 10) violates DESC.
+//   - `[1, 10], [1, 10]` -> passes: all fields tie, any order criteria is valid.
+//
+// With a single criterion, `[field_0 DESC]`:
+//   - `[10, 1], [5, 20]` -> passes: field_0 (10 > 5) satisfies DESC, field_1 is never compared.
+//   - `[5, 1],  [5, 20]` -> passes: field_0 ties and there are no more criteria, so any order is accepted.
+//
+// With no criteria:
+//   - `[2, 5],  [1, 10]` -> passes: nothing to check.
+//
+// This is called by [`confirm_hinted_notes`] to validate that the oracle returned notes in the requested order.
+fn assert_notes_order<let N: u32>(note_1: [Field; N], note_2: [Field; N], sort_criteria: BoundedVec<Option<Sort>, N>) {
+    // Tracks whether a prior criterion already determined the ordering. Once true, all subsequent
+    // asserts become trivially satisfied. We fold this into the assert condition instead of
+    // predicating the loop body to avoid gating field extraction behind a branch, which would be
+    // more expensive in a circuit.
+    let mut order_decided = false;
+    for i in 0..sort_criteria.max_len() {
+        if i < sort_criteria.len() {
+            let sort = sort_criteria.get_unchecked(i).unwrap_unchecked();
+            let field_1 = extract_property_value_from_selector(note_1, sort.property_selector);
+            let field_2 = extract_property_value_from_selector(note_2, sort.property_selector);
+            if field_1 != field_2 {
+                let lt = field_1.lt(field_2);
+                if sort.order == SortOrder.ASC {
+                    assert(lt | order_decided, "Return notes not sorted in ascending order.");
+                } else {
+                    assert(!lt | order_decided, "Return notes not sorted in descending order.");
+                }
+                order_decided = true;
             }
         }
     }
@@ -219,7 +247,7 @@ where
             let packed_note = hinted_note.note.pack();
             check_packed_note(packed_note, options.selects);
             if i != 0 {
-                check_notes_order(prev_packed_note, packed_note, options.sorts);
+                assert_notes_order(prev_packed_note, packed_note, options.sorts);
             }
             prev_packed_note = packed_note;
 

noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr

@@ -2,14 +2,22 @@ use crate::{
     note::{
         ConfirmedNote,
         HintedNote,
-        note_getter::{confirm_hinted_note, confirm_hinted_notes, extract_property_value_from_selector},
-        note_getter_options::{NoteGetterOptions, PropertySelector, SortOrder},
+        note_getter::{
+            assert_notes_order, confirm_hinted_note, confirm_hinted_notes, extract_property_value_from_selector,
+        },
+        note_getter_options::{NoteGetterOptions, PropertySelector, Sort, SortOrder},
     },
     oracle::random::random,
     test::{helpers::test_environment::TestEnvironment, mocks::mock_note::MockNote},
     utils::comparison::Comparator,
 };
 
+fn sort_criterion(index: u8, order: u8) -> Option<Sort> {
+    Option::some(
+        Sort::new(PropertySelector { index, offset: 0, length: 32 }, order),
+    )
+}
+
 use crate::protocol::{address::AztecAddress, traits::FromField};
 
 global storage_slot: Field = 42;
@@ -388,3 +396,63 @@ unconstrained fn extract_property_value_fails_on_oob_index() {
     let selector = PropertySelector { index: 1, offset: 0, length: 1 };
     let _ = extract_property_value_from_selector(packed, selector);
 }
+
+global ASC_DESC_CRITERIA: BoundedVec<Option<Sort>, 2> = BoundedVec::from_array([
+    sort_criterion(0, SortOrder.ASC),
+    sort_criterion(1, SortOrder.DESC),
+]);
+
+#[test]
+unconstrained fn assert_notes_order_passes_when_first_criterion_satisfied() {
+    assert_notes_order([1, 10], [2, 999], ASC_DESC_CRITERIA);
+}
+
+#[test]
+unconstrained fn assert_notes_order_passes_when_second_criterion_satisfied_after_tie() {
+    assert_notes_order([1, 10], [1, 5], ASC_DESC_CRITERIA);
+}
+
+#[test(should_fail_with = "Return notes not sorted in descending order.")]
+unconstrained fn assert_notes_order_fails_when_second_criterion_violated_after_tie() {
+    assert_notes_order([1, 5], [1, 10], ASC_DESC_CRITERIA);
+}
+
+#[test]
+unconstrained fn assert_notes_order_passes_when_all_fields_equal() {
+    assert_notes_order([1, 10], [1, 10], ASC_DESC_CRITERIA);
+}
+
+global DESC_ONLY_CRITERIA: BoundedVec<Option<Sort>, 2> = BoundedVec::from_array([sort_criterion(0, SortOrder.DESC)]);
+
+#[test]
+unconstrained fn assert_notes_order_passes_with_single_criterion() {
+    assert_notes_order([10, 1], [5, 20], DESC_ONLY_CRITERIA);
+}
+
+#[test]
+unconstrained fn assert_notes_order_passes_with_single_criterion_when_field_ties() {
+    assert_notes_order([5, 1], [5, 20], DESC_ONLY_CRITERIA);
+}
+
+global REVERSED_CRITERIA: BoundedVec<Option<Sort>, 2> = BoundedVec::from_array([
+    sort_criterion(1, SortOrder.ASC),
+    sort_criterion(0, SortOrder.DESC),
+]);
+
+#[test]
+unconstrained fn assert_notes_order_uses_criterion_order_not_field_order() {
+    // field_0 is 10 > 5 (would fail ASC), but the first criterion targets field_1
+    assert_notes_order([10, 1], [5, 2], REVERSED_CRITERIA);
+}
+
+#[test(should_fail_with = "Return notes not sorted in ascending order.")]
+unconstrained fn assert_notes_order_fails_when_criterion_order_not_field_order() {
+    // First criterion is field_1 ASC: 5 > 2 violates ASC
+    assert_notes_order([1, 5], [2, 2], REVERSED_CRITERIA);
+}
+
+#[test]
+unconstrained fn assert_notes_order_passes_with_no_criteria() {
+    let criteria: BoundedVec<Option<Sort>, 2> = BoundedVec::new();
+    assert_notes_order([2, 5], [1, 10], criteria);
+}