Skip to content

Commit

Permalink
feat!: make note_getter return BoundedVec instead of an Option array (#…
Browse files Browse the repository at this point in the history
…7050)

`note_getter` was collapsing all returned notes at the beginning of the
options array, which allowed us to iterate over `options.limit` instead
of `len()`, but this was quite tricky to realize and harder to explain
and realize was correct. This new interface avoids this issue entirely,
though it does require for the user to be familiar with usage of
`BoundedVec` and the fact that Noir loops must be compile-time bounded.

This slightly reduces gate counts since we no longer have all of the
`_is_some` booleans, but the big win will be once we begin setting
`limit` properly. Also, once this is merged, we can use the
`utils::colapse` function from #7016 inside `note_getter` for further
gate count reductions.
  • Loading branch information
nventuro authored Jun 19, 2024
1 parent c30dc38 commit f9ac0fc
Show file tree
Hide file tree
Showing 28 changed files with 201 additions and 150 deletions.
28 changes: 28 additions & 0 deletions docs/docs/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,34 @@ Aztec is in full-speed development. Literally every version breaks compatibility
Earlier we had just one function - `transfer()` which used authwits to handle the case where a contract/user wants to transfer funds on behalf of another user.
To reduce circuit sizes and proof times, we are breaking up `transfer` and introducing a dedicated `transferFrom()` function like in the ERC20 standard.

### [Aztec.nr] `note_getter` returns `BoundedVec`

The `get_notes` and `view_notes` function no longer return an array of options (i.e. `[Option<Note>, N_NOTES]`) but instead a `BoundedVec<Note, N_NOTES>`. This better conveys the useful property the old array had of having all notes collapsed at the beginning of the array, which allows for powerful optimizations and gate count reduction when setting the `options.limit` value.

A `BoundedVec` has a `max_len()`, which equals the number of elements it can hold, and a `len()`, which equals the number of elements it currently holds. Since `len()` is typically not knwon at compile time, iterating over a `BoundedVec` looks slightly different than iterating over an array of options:

```diff
- let option_notes = get_notes(options);
- for i in 0..option_notes.len() {
- if option_notes[i].is_some() {
- let note = option_notes[i].unwrap_unchecked();
- }
- }
+ let notes = get_notes(options);
+ for i in 0..notes.max_len() {
+ if i < notes.len() {
+ let note = notes.get_unchecked(i);
+ }
+ }
```

To further reduce gate count, you can iterate over `options.limit` instead of `max_len()`, since `options.limit` is guaranteed to be larger or equal to `len()`, and smaller or equal to `max_len()`:

```diff
- for i in 0..notes.max_len() {
+ for i in 0..options.limit {
```

### [Aztec.nr] `options.limit` has to be constant

The `limit` parameter in `NoteGetterOptions` and `NoteViewerOptions` is now required to be a compile-time constant. This allows performing loops over this value, which leads to reduced circuit gate counts when setting a `limit` value.
Expand Down
31 changes: 19 additions & 12 deletions noir-projects/aztec-nr/aztec/src/note/note_getter.nr
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub fn get_notes<Note, N, M, FILTER_ARGS>(
context: &mut PrivateContext,
storage_slot: Field,
options: NoteGetterOptions<Note, N, M, FILTER_ARGS>
) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N, M> {
) -> BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL> where Note: NoteInterface<N, M> {
let opt_notes = get_notes_internal(storage_slot, options);

constrain_get_notes_internal(context, storage_slot, opt_notes, options)
Expand All @@ -115,8 +115,8 @@ fn constrain_get_notes_internal<Note, N, M, FILTER_ARGS>(
storage_slot: Field,
opt_notes: [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL],
options: NoteGetterOptions<Note, N, M, FILTER_ARGS>
) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N, M> {
let mut returned_notes = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
) -> BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL> where Note: NoteInterface<N, M> {
let mut returned_notes = BoundedVec::new();

// The filter is applied first to avoid pushing note read requests for notes we're not interested in. Note that
// while the filter function can technically mutate the contents of the notes (as opposed to simply removing some),
Expand All @@ -126,7 +126,6 @@ fn constrain_get_notes_internal<Note, N, M, FILTER_ARGS>(
let filter_args = options.filter_args;
let filtered_notes = filter_fn(opt_notes, filter_args);

let mut num_notes = 0;
let mut prev_fields = [0; N];
for i in 0..filtered_notes.len() {
let opt_note = filtered_notes[i];
Expand All @@ -151,14 +150,12 @@ fn constrain_get_notes_internal<Note, N, M, FILTER_ARGS>(
// resulting in a smaller circuit and faster proving times.
// We write at returned_notes[num_notes] because num_notes is only advanced when we have a value in
// filtered_notes.
returned_notes[num_notes] = Option::some(note);
num_notes += 1;
returned_notes.push(note);
};
}

assert(num_notes <= options.limit, "Got more notes than limit.");

assert(num_notes != 0, "Cannot return zero notes");
assert(returned_notes.len() <= options.limit, "Got more notes than limit.");
assert(returned_notes.len() != 0, "Cannot return zero notes");

returned_notes
}
Expand Down Expand Up @@ -223,12 +220,13 @@ unconstrained fn get_notes_internal<Note, N, M, FILTER_ARGS>(
unconstrained pub fn view_notes<Note, N, M>(
storage_slot: Field,
options: NoteViewerOptions<Note, N, M>
) -> [Option<Note>; MAX_NOTES_PER_PAGE] where Note: NoteInterface<N, M> {
) -> BoundedVec<Note, MAX_NOTES_PER_PAGE> where Note: NoteInterface<N, M> {
let (num_selects, select_by_indexes, select_by_offsets, select_by_lengths, select_values, select_comparators, sort_by_indexes, sort_by_offsets, sort_by_lengths, sort_order) = flatten_options(options.selects, options.sorts);
let placeholder_opt_notes = [Option::none(); MAX_NOTES_PER_PAGE];
let placeholder_fields = [0; VIEW_NOTE_ORACLE_RETURN_LENGTH];
let placeholder_note_length = [0; N];
oracle::notes::get_notes(

let notes_array = oracle::notes::get_notes(
storage_slot,
num_selects,
select_by_indexes,
Expand All @@ -246,7 +244,16 @@ unconstrained pub fn view_notes<Note, N, M>(
placeholder_opt_notes,
placeholder_fields,
placeholder_note_length
)
);

let mut notes = BoundedVec::new();
for i in 0..notes_array.len() {
if notes_array[i].is_some() {
notes.push(notes_array[i].unwrap_unchecked());
}
}

notes
}

unconstrained fn flatten_options<Note, N>(
Expand Down
21 changes: 17 additions & 4 deletions noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,19 @@ fn build_valid_note(value: Field) -> MockNote {
MockNote::new(value).contract_address(cheatcodes::get_contract_address()).storage_slot(storage_slot).build()
}

fn assert_equivalent_vec_and_array<T, N>(vec: BoundedVec<T, N>, arr: [Option<T>; N]) where T: Eq {
let mut count = 0;

for i in 0..N {
if arr[i].is_some() {
assert_eq(arr[i].unwrap(), vec.get(count));
count += 1;
}
}

assert_eq(count, vec.len());
}

#[test]
fn processes_single_note() {
let mut env = setup();
Expand All @@ -32,7 +45,7 @@ fn processes_single_note() {
let options = NoteGetterOptions::new();
let returned = constrain_get_notes_internal(&mut context, storage_slot, notes_to_constrain, options);

assert_eq(returned, notes_to_constrain);
assert_equivalent_vec_and_array(returned, notes_to_constrain);
assert_eq(context.note_hash_read_requests.len(), 1);
}

Expand All @@ -48,7 +61,7 @@ fn processes_many_notes() {
let options = NoteGetterOptions::new();
let returned = constrain_get_notes_internal(&mut context, storage_slot, notes_to_constrain, options);

assert_eq(returned, notes_to_constrain);
assert_equivalent_vec_and_array(returned, notes_to_constrain);
assert_eq(context.note_hash_read_requests.len(), 2);
}

Expand Down Expand Up @@ -78,7 +91,7 @@ fn collapses_notes_at_the_beginning_of_the_array() {
expected[5] = Option::some(build_valid_note(5));
expected[6] = Option::some(build_valid_note(6));

assert_eq(returned, expected);
assert_equivalent_vec_and_array(returned, expected);
}

#[test(should_fail_with="Cannot return zero notes")]
Expand Down Expand Up @@ -137,7 +150,7 @@ fn applies_filter_before_constraining() {
let mut expected = [Option::none(); MAX_NOTE_HASH_READ_REQUESTS_PER_CALL];
expected[0] = Option::some(build_valid_note(42));

assert_eq(returned, expected);
assert_equivalent_vec_and_array(returned, expected);
assert_eq(context.note_hash_read_requests.len(), 1);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl<Note> PrivateImmutable<Note, UnconstrainedContext> {
// docs:start:view_note
unconstrained pub fn view_note<N, M>(self) -> Note where Note: NoteInterface<N, M> {
let mut options = NoteViewerOptions::new();
view_notes(self.storage_slot, options.set_limit(1))[0].unwrap()
view_notes(self.storage_slot, options.set_limit(1)).get(0)
}
// docs:end:view_note
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<Note> PrivateMutable<Note, UnconstrainedContext> {
// docs:start:view_note
unconstrained pub fn view_note<N, M>(self) -> Note where Note: NoteInterface<N, M> {
let mut options = NoteViewerOptions::new();
view_notes(self.storage_slot, options.set_limit(1))[0].unwrap()
view_notes(self.storage_slot, options.set_limit(1)).get(0)
}
// docs:end:view_note
}
8 changes: 3 additions & 5 deletions noir-projects/aztec-nr/aztec/src/state_vars/private_set.nr
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,8 @@ impl<Note> PrivateSet<Note, &mut PrivateContext> {
pub fn get_notes<N, M, FILTER_ARGS>(
self,
options: NoteGetterOptions<Note, N, M, FILTER_ARGS>
) -> [Option<Note>; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] where Note: NoteInterface<N, M> {
let storage_slot = self.storage_slot;
let opt_notes = get_notes(self.context, storage_slot, options);
opt_notes
) -> BoundedVec<Note, MAX_NOTE_HASH_READ_REQUESTS_PER_CALL> where Note: NoteInterface<N, M> {
get_notes(self.context, self.storage_slot, options)
}
// docs:end:get_notes
}
Expand All @@ -71,7 +69,7 @@ impl<Note> PrivateSet<Note, UnconstrainedContext> {
unconstrained pub fn view_notes<N, M>(
self,
options: NoteViewerOptions<Note, N, M>
) -> [Option<Note>; MAX_NOTES_PER_PAGE] where Note: NoteInterface<N, M> {
) -> BoundedVec<Note, MAX_NOTES_PER_PAGE> where Note: NoteInterface<N, M> {
view_notes(self.storage_slot, options)
}
// docs:end:view_notes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ impl<Context> EasyPrivateUint<&mut PrivateContext> {

// docs:start:get_notes
let options = NoteGetterOptions::with_filter(filter_notes_min_sum, subtrahend as Field);
let maybe_notes = self.set.get_notes(options);
let notes = self.set.get_notes(options);
// docs:end:get_notes

let mut minuend: u64 = 0;
for i in 0..options.limit {
if maybe_notes[i].is_some() {
let note = maybe_notes[i].unwrap_unchecked();
if i < notes.len() {
let note = notes.get_unchecked(i);

// Removes the note from the owner's set of notes.
// docs:start:remove
Expand Down
14 changes: 7 additions & 7 deletions noir-projects/aztec-nr/value-note/src/balance_utils.nr
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ unconstrained pub fn get_balance_with_offset(set: PrivateSet<ValueNote, Unconstr
let mut balance = 0;
// docs:start:view_notes
let mut options = NoteViewerOptions::new();
let opt_notes = set.view_notes(options.set_offset(offset));
let notes = set.view_notes(options.set_offset(offset));
// docs:end:view_notes
let len = opt_notes.len();
for i in 0..len {
if opt_notes[i].is_some() {
balance += opt_notes[i].unwrap_unchecked().value;
for i in 0..options.limit {
if i < notes.len() {
balance += notes.get_unchecked(i).value;
}
}
if (opt_notes[len - 1].is_some()) {
balance += get_balance_with_offset(set, offset + opt_notes.len() as u32);

if (notes.len() == options.limit) {
balance += get_balance_with_offset(set, offset + options.limit);
}

balance
Expand Down
6 changes: 3 additions & 3 deletions noir-projects/aztec-nr/value-note/src/utils.nr
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ pub fn decrement_by_at_most(
outgoing_viewer: AztecAddress
) -> Field {
let options = create_note_getter_options_for_decreasing_balance(max_amount);
let opt_notes = balance.get_notes(options);
let notes = balance.get_notes(options);

let mut decremented = 0;
for i in 0..options.limit {
if opt_notes[i].is_some() {
let note = opt_notes[i].unwrap_unchecked();
if i < notes.len() {
let note = notes.get_unchecked(i);

decremented += destroy_note(balance, note);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ contract Benchmarking {
let owner_notes = storage.notes.at(owner);
let mut getter_options = NoteGetterOptions::new();
let notes = owner_notes.get_notes(getter_options.set_limit(1).set_offset(index));
let note = notes[0].unwrap_unchecked();
let note = notes.get_unchecked(0);
owner_notes.remove(note);
increment(owner_notes, note.value, owner, outgoing_viewer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,17 @@ impl Deck<&mut PrivateContext> {

pub fn get_cards<N>(&mut self, cards: [Card; N]) -> [CardNote; N] {
let options = NoteGetterOptions::with_filter(filter_cards, cards);
let maybe_notes = self.set.get_notes(options);
let notes = self.set.get_notes(options);

// This array will hold the notes that correspond to each of the requested cards. It begins empty (with all the
// options being none) and we gradually fill it up as we find the matching notes.
let mut found_cards = [Option::none(); N];

for i in 0..options.limit {
if maybe_notes[i].is_some() {
let card_note = CardNote::from_note(maybe_notes[i].unwrap_unchecked());
if i < notes.len() {
let card_note = CardNote::from_note(notes.get_unchecked(i));

// For each note that we read, we search for a matching card that we have not already marked as found.
for j in 0..cards.len() {
if found_cards[j].is_none()
& (cards[j].strength == card_note.card.strength)
Expand All @@ -139,6 +144,7 @@ impl Deck<&mut PrivateContext> {
}
}

// And then we assert that we did indeed find all cards, since found_cards and cards have the same length.
found_cards.map(
|card_note: Option<CardNote>| {
assert(card_note.is_some(), "Card not found");
Expand All @@ -156,16 +162,19 @@ impl Deck<&mut PrivateContext> {
}

impl Deck<UnconstrainedContext> {
unconstrained pub fn view_cards(self, offset: u32) -> [Option<Card>; MAX_NOTES_PER_PAGE] {
unconstrained pub fn view_cards(self, offset: u32) -> BoundedVec<Card, MAX_NOTES_PER_PAGE> {
let mut options = NoteViewerOptions::new();
let opt_notes = self.set.view_notes(options.set_offset(offset));
let mut opt_cards = [Option::none(); MAX_NOTES_PER_PAGE];

for i in 0..opt_notes.len() {
opt_cards[i] = opt_notes[i].map(|note: ValueNote| Card::from_field(note.value));
let notes = self.set.view_notes(options.set_offset(offset));

// TODO: ideally we'd do let cards = notes.map(|note| Cards::from_field(note.value));
// see https://github.com/noir-lang/noir/pull/5250
let mut cards = BoundedVec::new();
cards.len = notes.len();
for i in 0..notes.len() {
cards.storage[i] = Card::from_field(notes.get_unchecked(i).value);
}

opt_cards
cards
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,12 @@ contract CardGame {
game_storage.write(game_data);
}

unconstrained fn view_collection_cards(owner: AztecAddress, offset: u32) -> pub [Option<Card>; MAX_NOTES_PER_PAGE] {
unconstrained fn view_collection_cards(owner: AztecAddress, offset: u32) -> pub BoundedVec<Card, MAX_NOTES_PER_PAGE> {
let collection = storage.collections.at(owner);

collection.view_cards(offset)
}

unconstrained fn view_game_cards(game: u32, player: AztecAddress, offset: u32) -> pub [Option<Card>; MAX_NOTES_PER_PAGE] {
unconstrained fn view_game_cards(game: u32, player: AztecAddress, offset: u32) -> pub BoundedVec<Card, MAX_NOTES_PER_PAGE> {
let game_deck = storage.game_decks.at(game as Field).at(player);

game_deck.view_cards(offset)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ contract Child {
let mut options = NoteGetterOptions::new();
options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1);
let notes = storage.a_map_with_private_values.at(owner).get_notes(options);
notes[0].unwrap_unchecked().value
notes.get_unchecked(0).value
}

// Increments `current_value` by `new_value`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ contract Counter {
let counter_slot = Counter::storage().counters.slot;
let owner_slot = derive_storage_slot_in_map(counter_slot, owner);
let mut options = NoteViewerOptions::new();
let opt_notes: [Option<ValueNote>; MAX_NOTES_PER_PAGE] = view_notes(owner_slot, options);
assert(opt_notes[0].unwrap().value == 5);
let notes: BoundedVec<ValueNote, MAX_NOTES_PER_PAGE> = view_notes(owner_slot, options);
assert(notes.get(0).value == 5);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ contract DelegatedOn {
let mut options = NoteGetterOptions::new();
options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1);
let notes = storage.a_map_with_private_values.at(owner).get_notes(options);
notes[0].unwrap_unchecked().value
notes.get_unchecked(0).value
}

unconstrained fn view_public_value() -> pub Field {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ contract Delegator {
let mut options = NoteGetterOptions::new();
options = options.select(ValueNote::properties().value, amount, Option::none()).set_limit(1);
let notes = storage.a_map_with_private_values.at(owner).get_notes(options);
notes[0].unwrap_unchecked().value
notes.get_unchecked(0).value
}

unconstrained fn view_public_value() -> pub Field {
Expand Down
Loading

0 comments on commit f9ac0fc

Please sign in to comment.