Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions docs/docs-developers/docs/resources/migration_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,43 @@ Aztec is in active development. Each version may introduce breaking changes that

## TBD

### [Aztec.nr] `for_each` visits elements in order; removing during iteration no longer supported

`CapsuleArray::for_each` and `EphemeralArray::for_each` previously iterated backwards (from the last element to the first) so that the callback could safely remove the current element. They now visit elements in order, from first to last, as is usually expected in other languages. Structurally mutating the array (e.g. via `push` or `remove`) from inside the callback is no longer supported.

For `EphemeralArray`, replace remove-during-iteration with `filter`:

```diff
- array.for_each(|index, value| {
- if should_remove(value) {
- array.remove(index);
- }
- });
+ let kept = array.filter(|value| !should_remove(value));
```

`filter` collects the kept elements into a fresh array at a new slot. If the original slot matters (e.g. a `TransientArray` slot shared with other call frames), rebuild it from the filtered result:

```noir
let kept = array.filter(|value| !should_remove(value));
let _ = array.clear();
kept.for_each(|_index, value| array.push(value));
```

`EphemeralArray`'s are cheap and by nature not persistent though, so in most cases you probably can just work with the new copy instead of going through this hassle.

`CapsuleArray` has no `filter`, so iterate manually, backwards. Removing the current element is safe in a backward loop because it only shifts elements at higher indices:

```noir
let mut i = array.len();
while i > 0 {
i -= 1;
if should_remove(array.get(i)) {
array.remove(i);
}
}
```

### [Aztec.js] Prefunded local network test accounts are now initializerless

The genesis-funded test accounts in the local network (sandbox), returned by `getInitialTestAccountsData()`, are now initializerless Schnorr accounts (`schnorr_initializerless`). An initializerless account has no onchain deployment transaction: its address commits to the signing public key (through `immutables_hash`) and its contract state is materialized locally in the PXE, so these accounts are usable right away.
Expand Down
109 changes: 10 additions & 99 deletions noir-projects/aztec-nr/aztec/src/capsules/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -103,40 +103,17 @@ impl<T> CapsuleArray<T> {

/// Calls a function on each element of the array.
///
/// The function `f` is called once with each array value and its corresponding index. The order in which values
/// are processed is arbitrary.
/// The function `f` is called once with each array value and its corresponding index, in order (from the first
/// element to the last).
///
/// ## Array Mutation
///
/// It is safe to delete the current element (and only the current element) from inside the callback via `remove`:
/// ```noir
/// array.for_each(|index, value| {
/// if some_condition(value) {
/// array.remove(index); // safe only for this index
/// }
/// }
/// ```
///
/// If all elements in the array need to iterated over and then removed, then using `for_each` results in optimal
/// efficiency.
///
/// It is **not** safe to push new elements into the array from inside the callback.
/// Structurally mutating the array from inside the callback (e.g. via `push` or `remove`) is **not** supported:
/// it can cause elements to be skipped, visited more than once, or read out of bounds.
pub unconstrained fn for_each<Env>(self, f: unconstrained fn[Env](u32, T) -> ())
where
T: Deserialize,
{
// Iterating over all elements is simple, but we want to do it in such a way that a) deleting the current
// element is safe to do, and b) deleting *all* elements is optimally efficient. This is because CapsuleArrays
// are typically used to hold pending tasks, so iterating them while clearing completed tasks (sometimes
// unconditionally, resulting in a full clear) is a very common access pattern.
//
// The way we achieve this is by iterating backwards: each element can always be deleted since it won't change
// any preceding (lower) indices, and if every element is deleted then every element will (in turn) be the last
// element. This results in an optimal full clear since `remove` will be able to skip the `capsules::copy` call
// to shift any elements past the deleted one (because there will be none).
let mut i = self.len();
while i > 0 {
i -= 1;
let n = self.len();
for i in 0..n {
f(i, self.get(i));
}
}
Expand Down Expand Up @@ -271,7 +248,7 @@ mod test {
}

#[test]
unconstrained fn for_each_called_with_all_elements() {
unconstrained fn for_each_called_with_all_elements_in_order() {
let mut env = TestEnvironment::new();
let scope = env.create_light_account();
env.private_context(|context| {
Expand All @@ -282,79 +259,13 @@ mod test {
array.push(5);
array.push(6);

// We store all values that we were called with and check that all (value, index) tuples are present. Note
// that we do not care about the order in which each tuple was passed to the closure.
let called_with = &mut BoundedVec::<(u32, Field), 3>::new();
array.for_each(|index, value| { called_with.push((index, value)); });

assert_eq(called_with.len(), 3);
assert(called_with.any(|(index, value)| (index == 0) & (value == 4)));
assert(called_with.any(|(index, value)| (index == 1) & (value == 5)));
assert(called_with.any(|(index, value)| (index == 2) & (value == 6)));
});
}

#[test]
unconstrained fn for_each_remove_some() {
let mut env = TestEnvironment::new();
let scope = env.create_light_account();
env.private_context(|context| {
let contract_address = context.this_address();
let array = CapsuleArray::at(contract_address, SLOT, scope);

array.push(4);
array.push(5);
array.push(6);

array.for_each(|index, _| {
if index == 1 {
array.remove(index);
}
});

assert_eq(array.len(), 2);
assert_eq(array.get(0), 4);
assert_eq(array.get(1), 6);
});
}

#[test]
unconstrained fn for_each_remove_all() {
let mut env = TestEnvironment::new();
let scope = env.create_light_account();
env.private_context(|context| {
let contract_address = context.this_address();
let array = CapsuleArray::at(contract_address, SLOT, scope);

array.push(4);
array.push(5);
array.push(6);

array.for_each(|index, _| { array.remove(index); });

assert_eq(array.len(), 0);
});
}

#[test]
unconstrained fn for_each_remove_all_no_copy() {
let mut env = TestEnvironment::new();
let scope = env.create_light_account();
env.private_context(|context| {
let contract_address = context.this_address();
let array = CapsuleArray::at(contract_address, SLOT, scope);

array.push(4);
array.push(5);
array.push(6);

// We test that the aztec_utl_copyCapsule was never called, which is the expensive operation we want to
// avoid.
let mock = std::test::OracleMock::mock("aztec_utl_copyCapsule");

array.for_each(|index, _| { array.remove(index); });

assert_eq(mock.times_called(), 0);
assert_eq(called_with.get(0), (0, 4));
assert_eq(called_with.get(1), (1, 5));
assert_eq(called_with.get(2), (2, 6));
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,13 @@ pub(crate) unconstrained fn fetch_and_process_partial_note_completion_logs(
// index. Each inner array contains all matching LogRetrievalResponses.
assert_eq(completion_logs.len(), pending_partial_notes.len());

completion_logs.for_each(|i, logs_for_tag: EphemeralArray<LogRetrievalResponse>| {
// Note: the loop runs backwards because it removes completed entries from `pending_partial_notes`, which is
// index-aligned with `completion_logs`. Going from last to first keeps the indices of not-yet-visited entries
// stable.
let mut i = completion_logs.len();
while i > 0 {
i -= 1;
let logs_for_tag: EphemeralArray<LogRetrievalResponse> = completion_logs.get(i);
let pending_partial_note = pending_partial_notes.get(i);
let num_logs = logs_for_tag.len();

Expand Down Expand Up @@ -172,5 +178,5 @@ pub(crate) unconstrained fn fetch_and_process_partial_note_completion_logs(
// delete the pending work entry, regardless of whether it was actually completed or not.
pending_partial_notes.remove(i);
}
});
}
}
12 changes: 2 additions & 10 deletions noir-projects/aztec-nr/aztec/src/messages/processing/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -155,23 +155,15 @@ pub(crate) unconstrained fn get_pending_partial_notes_completion_logs(
) -> EphemeralArray<EphemeralArray<LogRetrievalResponse>> {
let log_retrieval_requests = EphemeralArray::at(LOG_RETRIEVAL_REQUESTS_ARRAY_BASE_SLOT);

// We create a LogRetrievalRequest for each PendingPartialNote in the EphemeralArray. Because we need the indices
// in the request array to match the indices in the partial note array, we can't use EphemeralArray::for_each, as
// that function has arbitrary iteration order. Instead, we manually iterate the array from the beginning and push
// into the requests array, which we expect to be empty.
let mut i = 0;
let pending_partial_notes_count = pending_partial_notes.len();
while i < pending_partial_notes_count {
let pending_partial_note = pending_partial_notes.get(i);
pending_partial_notes.for_each(|_i, pending_partial_note| {
// Partial note completion logs are emitted with a domain-separated tag. To find matching logs, we apply the
// same domain separation to the stored raw tag.
let log_tag = compute_log_tag(
pending_partial_note.note_completion_log_tag,
DOM_SEP__NOTE_COMPLETION_LOG_TAG,
);
log_retrieval_requests.push(LogRetrievalRequest::new(contract_address, log_tag));
i += 1;
}
});

let responses = message_processing::get_logs_by_tag(log_retrieval_requests);

Expand Down
13 changes: 6 additions & 7 deletions noir-projects/aztec-nr/aztec/src/unconstrained_array/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,17 @@ where

/// Calls a function on each element of the array.
///
/// The function `f` is called once with each array value and its corresponding index. Iteration proceeds
/// backwards so that it is safe to remove the current element (and only the current element) inside the
/// callback.
/// The function `f` is called once with each array value and its corresponding index, in order (from the first
/// element to the last).
///
/// It is **not** safe to push new elements from inside the callback.
/// Structurally mutating the array from inside the callback (e.g. via `push`, `pop`, `remove` or `clear`) is
/// **not** supported: it can cause elements to be skipped, visited more than once, or read out of bounds.
pub unconstrained fn for_each<Env>(self, f: unconstrained fn[Env](u32, T) -> ())
where
T: Deserialize,
{
let mut i = self.len();
while i > 0 {
i -= 1;
let n = self.len();
for i in 0..n {
f(i, self.get(i));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ where
});
}

pub(crate) unconstrained fn for_each_called_with_all_elements<Oracle>()
pub(crate) unconstrained fn for_each_called_with_all_elements_in_order<Oracle>()
where
Oracle: ArrayOracles,
{
Expand All @@ -179,51 +179,9 @@ where
array.for_each(|index, value| { called_with.push((index, value)); });

assert_eq(called_with.len(), 3);
assert(called_with.any(|(index, value)| (index == 0) & (value == 4)));
assert(called_with.any(|(index, value)| (index == 1) & (value == 5)));
assert(called_with.any(|(index, value)| (index == 2) & (value == 6)));
});
}

pub(crate) unconstrained fn for_each_remove_some<Oracle>()
where
Oracle: ArrayOracles,
{
let env = TestEnvironment::new();
env.utility_context(|_| {
let array: UnconstrainedArray<Field, Oracle> = UnconstrainedArray::at(SLOT);

array.push(4);
array.push(5);
array.push(6);

array.for_each(|index, _| {
if index == 1 {
array.remove(index);
}
});

assert_eq(array.len(), 2);
assert_eq(array.get(0), 4);
assert_eq(array.get(1), 6);
});
}

pub(crate) unconstrained fn for_each_remove_all<Oracle>()
where
Oracle: ArrayOracles,
{
let env = TestEnvironment::new();
env.utility_context(|_| {
let array: UnconstrainedArray<Field, Oracle> = UnconstrainedArray::at(SLOT);

array.push(4);
array.push(5);
array.push(6);

array.for_each(|index, _| { array.remove(index); });

assert_eq(array.len(), 0);
assert_eq(called_with.get(0), (0, 4));
assert_eq(called_with.get(1), (1, 5));
assert_eq(called_with.get(2), (2, 6));
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ contract StorageProofTest {
use crate::storage_proofs::types::Node;
use crate::storage_proofs::types::StorageSlot;
use crate::storage_proofs::utils::to_nibbles;
use aztec::context::calls::PrivateCall;
use aztec::macros::functions::external;
use aztec::macros::functions::internal;
use aztec::macros::functions::only_self;
Expand Down Expand Up @@ -52,12 +51,6 @@ contract StorageProofTest {
#[contract_library_method]
fn verify_storage_proof_path_recursively(num_processed_nodes: u32, num_total_nodes: u32, start_nibble_index: u32, parent_hash: [u64; 4], storage_trie_key_nibbles: [u8; 64], storage_proof_capsule_key: Field) -> ([u64; 4], u32);

#[no_predicates]
#[contract_library_method]
fn call_verify_storage_proof_path_recursively(this_address: AztecAddress, num_processed_nodes: u32, num_total_nodes: u32, start_nibble_index: u32, parent_hash: [u64; 4], storage_trie_key_nibbles: [u8; 64], storage_proof_capsule_key: Field) -> PrivateCall<37, 72, ([u64; 4], u32)> {
StorageProofTest::at(this_address).verify_storage_proof_path_recursively(num_processed_nodes, num_total_nodes, start_nibble_index, parent_hash, storage_trie_key_nibbles, storage_proof_capsule_key)
}

#[contract_library_method]
fn compute_address_capsule_key(eth_storage_root: [u64; 4], address: EthAddress) -> Field {
poseidon2_hash((ACCOUNT_CAPSULE_KEY_SEPARATOR, eth_storage_root, address).serialize())
Expand Down Expand Up @@ -133,7 +126,7 @@ contract StorageProofTest {
Self { target_contract: AztecAddress::zero()}
}

pub fn verify_storage_proof_path_recursively(self, num_processed_nodes: u32, num_total_nodes: u32, start_nibble_index: u32, parent_hash: [u64; 4], storage_trie_key_nibbles: [u8; 64], storage_proof_capsule_key: Field) -> PrivateCall<37, 72, ([u64; 4], u32)> {
pub fn verify_storage_proof_path_recursively(self, num_processed_nodes: u32, num_total_nodes: u32, start_nibble_index: u32, parent_hash: [u64; 4], storage_trie_key_nibbles: [u8; 64], storage_proof_capsule_key: Field) -> aztec::context::calls::PrivateCall<37, 72, ([u64; 4], u32)> {
let mut serialized_params: [Field; 72] = [0_Field; 72];
let mut offset: u32 = 0_u32;
let serialized_member: [Field; 1] = num_processed_nodes.serialize();
Expand Down Expand Up @@ -173,10 +166,10 @@ contract StorageProofTest {
};
offset = offset + serialized_member_len;
let selector: aztec::protocol::abis::function_selector::FunctionSelector = <aztec::protocol::abis::function_selector::FunctionSelector as aztec::protocol::traits::FromField>::from_field(2237949917_Field);
PrivateCall::<37, 72, ([u64; 4], u32)>::new(self.target_contract, selector, "verify_storage_proof_path_recursively", serialized_params)
aztec::context::calls::PrivateCall::<37, 72, ([u64; 4], u32)>::new(self.target_contract, selector, "verify_storage_proof_path_recursively", serialized_params)
}

pub fn storage_proof(self, address: EthAddress, slot_key: [u8; 32], slot_contents: StorageSlot, eth_storage_root: [u64; 4]) -> PrivateCall<13, 70, ()> {
pub fn storage_proof(self, address: EthAddress, slot_key: [u8; 32], slot_contents: StorageSlot, eth_storage_root: [u64; 4]) -> aztec::context::calls::PrivateCall<13, 70, ()> {
let mut serialized_params: [Field; 70] = [0_Field; 70];
let mut offset: u32 = 0_u32;
let serialized_member: [Field; 1] = address.serialize();
Expand Down Expand Up @@ -204,7 +197,7 @@ contract StorageProofTest {
};
offset = offset + serialized_member_len;
let selector: aztec::protocol::abis::function_selector::FunctionSelector = <aztec::protocol::abis::function_selector::FunctionSelector as aztec::protocol::traits::FromField>::from_field(2067008734_Field);
PrivateCall::<13, 70, ()>::new(self.target_contract, selector, "storage_proof", serialized_params)
aztec::context::calls::PrivateCall::<13, 70, ()>::new(self.target_contract, selector, "storage_proof", serialized_params)
}

pub fn account_proof(self, account: Account, root: [u64; 4], nodes: [Node; 15], node_length: u32) -> aztec::context::calls::PublicStaticCall<13, 1290, ()> {
Expand Down Expand Up @@ -575,7 +568,7 @@ contract StorageProofTest {
};
assert(hinted_storage_proof_length > 0_u32, "Storage proof length must be greater than 0");
let storage_trie_key: [u8; 32] = keccak256(slot_key, 32_u32);
let (slot_leaf_hash, slot_nibble_index): ([u64; 4], u32) = call_verify_storage_proof_path_recursively(self.address, 0_u32, hinted_storage_proof_length, 0_u32, hinted_account.storage_hash, to_nibbles(storage_trie_key), storage_proof_capsule_key).call(self.context);
let (slot_leaf_hash, slot_nibble_index): ([u64; 4], u32) = StorageProofTest::at(self.address).verify_storage_proof_path_recursively(0_u32, hinted_storage_proof_length, 0_u32, hinted_account.storage_hash, to_nibbles(storage_trie_key), storage_proof_capsule_key).call(self.context);
assert(slot_leaf_hash == compute_slot_hash(slot_contents, slot_nibble_index, storage_trie_key));
assert(within_revertible_phase == self.context.in_revertible_phase(), f"Phase change detected on function with phase check. If this is expected, use #[allow_phase_change]");
self.context.finish()
Expand Down Expand Up @@ -658,7 +651,7 @@ contract StorageProofTest {
let macro__returned__values: ([u64; 4], u32) = if new_num_processed_nodes == num_total_nodes {
(child_hash, end_nibble_index)
} else {
call_verify_storage_proof_path_recursively(self.address, new_num_processed_nodes, num_total_nodes, end_nibble_index, child_hash, storage_trie_key_nibbles, storage_proof_capsule_key).call(self.context)
StorageProofTest::at(self.address).verify_storage_proof_path_recursively(new_num_processed_nodes, num_total_nodes, end_nibble_index, child_hash, storage_trie_key_nibbles, storage_proof_capsule_key).call(self.context)
};
let serialized_params: [Field; (4 * 1) + 1] = macro__returned__values.serialize();
self.context.set_return_hash(serialized_params);
Expand Down
Loading
Loading