Skip to content

Commit acd3a0c

Browse files
authored
refactor: serialization-related macros cleanup (#15834)
In this PR I - replace the last use of `generate_serialize_to_fields` and I delete the function - rename `get_note_interface` as `get_note_type_impl` (the naming was stale) - I replace use of `get_packable_len` as the function didn't work on types whose Packable implementation included numeric generics (in that case the return type length was not yet resolved and conversion to constant failed). Instead of using the func I insert a static assert into the note implementation, - instead of loading note packed length from `NOTES` hash map in `#[aztec]` macro I use `<NoteType as Packable>::N` which significantly simplifies the macros
1 parent 70019d7 commit acd3a0c

File tree

9 files changed

+91
-307
lines changed

9 files changed

+91
-307
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ mod oracle;
99
mod state_vars;
1010
mod capsules;
1111
mod event;
12-
mod messages;
12+
pub mod messages;
1313
pub use dep::protocol_types;
1414
mod utils;
1515
mod authwit;

noir-projects/aztec-nr/aztec/src/macros/aztec.nr

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,17 +119,15 @@ comptime fn generate_contract_interface(m: Module) -> Quoted {
119119
/// discovery (to create the `aztec::messages::discovery::ComputeNoteHashAndNullifier` function) and to implement the
120120
/// `compute_note_hash_and_nullifier` unconstrained contract function.
121121
comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() -> Quoted {
122-
let notes = NOTES.entries();
123-
124-
if notes.len() > 0 {
122+
if NOTES.len() > 0 {
125123
// Contracts that do define notes produce an if-else chain where `note_type_id` is matched against the
126124
// `get_note_type_id()` function of each note type that we know of, in order to identify the note type. Once we
127125
// know it we call we correct `unpack` method from the `Packable` trait to obtain the underlying note type, and
128126
// compute the note hash (non-siloed) and inner nullifier (also non-siloed).
129127

130128
let mut if_note_type_id_match_statements_list = &[];
131-
for i in 0..notes.len() {
132-
let (typ, packed_note_length) = notes[i];
129+
for i in 0..NOTES.len() {
130+
let typ = NOTES.get(i);
133131

134132
let get_note_type_id = get_trait_impl_method(
135133
typ,
@@ -166,7 +164,7 @@ comptime fn generate_contract_library_method_compute_note_hash_and_nullifier() -
166164
// As an extra safety check we make sure that the packed_note BoundedVec has the expected
167165
// length, since we're about to interpret its raw storage as a fixed-size array by calling the
168166
// unpack function on it.
169-
let expected_len = $packed_note_length;
167+
let expected_len = <$typ as $crate::protocol_types::traits::Packable>::N;
170168
let actual_len = packed_note.len();
171169
assert(
172170
actual_len == expected_len,

noir-projects/aztec-nr/aztec/src/macros/functions/utils.nr

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use crate::macros::{
1111
module_has_storage,
1212
},
1313
};
14-
use protocol_types::meta::generate_serialize_to_fields;
1514
use std::meta::{ctstring::AsCtString, type_of};
1615

1716
pub(crate) comptime fn transform_private(f: FunctionDefinition) -> Quoted {
@@ -208,17 +207,26 @@ pub(crate) comptime fn transform_public(f: FunctionDefinition) -> Quoted {
208207

209208
// Public functions undergo a lot of transformations from their Aztec.nr form.
210209
let original_params = f.parameters();
211-
let args_len = original_params
212-
.map(|(name, typ): (Quoted, Type)| {
213-
generate_serialize_to_fields(name, typ, false).0.len()
214-
})
215-
.fold(0, |acc: u32, val: u32| acc + val);
210+
211+
let args_len_quote = if original_params.len() == 0 {
212+
// If the function has no parameters, we set the args_len to 0.
213+
quote { 0 }
214+
} else {
215+
// The following will give us <type_of_struct_member_1 as Serialize>::N + <type_of_struct_member_2 as Serialize>::N + ...
216+
original_params
217+
.map(|(_, param_type): (Quoted, Type)| {
218+
quote {
219+
<$param_type as $crate::protocol_types::traits::Serialize>::N
220+
}
221+
})
222+
.join(quote {+})
223+
};
216224

217225
// Unlike in the private case, in public the `context` does not need to receive the hash of the original params.
218226
let context_creation = quote {
219227
let mut context = dep::aztec::context::public_context::PublicContext::new(|| {
220228
// We start from 1 because we skip the selector for the dispatch function.
221-
let serialized_args : [Field; $args_len] = dep::aztec::context::public_context::calldata_copy(1, $args_len);
229+
let serialized_args : [Field; $args_len_quote] = dep::aztec::context::public_context::calldata_copy(1, $args_len_quote);
222230
dep::aztec::hash::hash_args_array(serialized_args)
223231
});
224232
};

noir-projects/aztec-nr/aztec/src/macros/notes.nr

Lines changed: 33 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,20 @@
1-
use crate::{
2-
messages::discovery::private_notes::MAX_NOTE_PACKED_LEN,
3-
note::note_getter_options::PropertySelector,
4-
};
5-
use poseidon::poseidon2::Poseidon2Hasher;
6-
use protocol_types::meta::get_packable_len;
7-
use std::{collections::umap::UHashMap, hash::BuildHasherDefault, meta::type_of};
1+
use crate::note::note_getter_options::PropertySelector;
2+
use std::{collections::bounded_vec::BoundedVec, meta::type_of};
83

9-
/// A map from note type to note_packed_len.
10-
pub comptime mut global NOTES: UHashMap<Type, u32, BuildHasherDefault<Poseidon2Hasher>> =
11-
UHashMap::default();
4+
/// Maximum number of note types within 1 contract.
5+
comptime global MAX_NOTE_TYPES: u32 = 128;
6+
7+
/// A BoundedVec containing all the note types within this contract.
8+
pub comptime mut global NOTES: BoundedVec<Type, MAX_NOTE_TYPES> = BoundedVec::new();
129

1310
comptime mut global NOTE_TYPE_ID_COUNTER: u32 = 0;
1411

1512
/// The note type id is set by enumerating the note types.
1613
comptime fn get_next_note_type_id() -> Field {
1714
// We assert that the note type id fits within 7 bits
1815
assert(
19-
NOTE_TYPE_ID_COUNTER < 128 as u32,
20-
"A contract can contain at most 128 different note types",
16+
NOTE_TYPE_ID_COUNTER < MAX_NOTE_TYPES,
17+
f"A contract can contain at most {MAX_NOTE_TYPES} different note types",
2118
);
2219

2320
let note_type_id = NOTE_TYPE_ID_COUNTER as Field;
@@ -32,12 +29,21 @@ comptime fn get_next_note_type_id() -> Field {
3229
/// ...
3330
/// }
3431
/// }
35-
comptime fn generate_note_interface(s: TypeDefinition, note_type_id: Field) -> Quoted {
32+
comptime fn generate_note_type_impl(s: TypeDefinition, note_type_id: Field) -> Quoted {
3633
let name = s.name();
34+
let typ = s.as_type();
35+
36+
// TODO: Insert note type name into the static assert message. Currently blocked by:
37+
// https://github.com/noir-lang/noir/issues/9274
3738

3839
quote {
3940
impl aztec::note::note_interface::NoteType for $name {
4041
fn get_id() -> Field {
42+
// This static assertion ensures the note's packed length doesn't exceed the maximum allowed size.
43+
// While this check would ideally live in the Packable trait implementation, we place it here since
44+
// this function is always generated by our macros and the Packable trait implementation is not.
45+
let max_packed_len = $crate::messages::discovery::private_notes::MAX_NOTE_PACKED_LEN;
46+
std::static_assert(<$typ as Packable>::N <= max_packed_len, f"Note packed length exceeds the maximum of {max_packed_len} fields");
4147
$note_type_id
4248
}
4349
}
@@ -174,40 +180,30 @@ comptime fn generate_note_properties(s: TypeDefinition) -> Quoted {
174180
///
175181
/// # Registration
176182
///
177-
/// Registers the note in the global `NOTES` map to enable note processing functionality.
183+
/// Registers the note in the global `NOTES` BoundedVec to enable note processing functionality.
178184
///
179185
/// # Generated Code
180186
///
181187
/// For detailed documentation on the generated implementations, see:
182188
/// - `generate_note_properties()`
183-
/// - `generate_note_interface()`
189+
/// - `generate_note_type_impl()`
184190
/// - `generate_note_hash_trait_impl()`
185191
pub comptime fn note(s: TypeDefinition) -> Quoted {
186192
assert_has_owner(s);
187193
assert_has_packable(s);
188194

189-
let note_packed_len = get_packable_len(s.as_type());
190-
191-
// Ensure the note's packed length does not exceed the maximum allowable size to prevent overflow
192-
// when processing the note.
193-
if note_packed_len > MAX_NOTE_PACKED_LEN {
194-
panic(
195-
f"{s} packs into {note_packed_len} fields which is larger than the maximum of {MAX_NOTE_PACKED_LEN}",
196-
);
197-
}
198-
199-
// We register the note in the global `NOTES` map with the packed length because we need that information
200-
// inside the #[aztec] macro to generate note processing functionality.
201-
NOTES.insert(s.as_type(), note_packed_len);
195+
// We register the note in the global `NOTES` BoundedVec because we need that information inside the #[aztec] macro
196+
// to generate note processing functionality.
197+
NOTES.push(s.as_type());
202198

203199
let note_properties = generate_note_properties(s);
204200
let note_type_id = get_next_note_type_id();
205-
let note_interface_impl = generate_note_interface(s, note_type_id);
201+
let note_type_impl = generate_note_type_impl(s, note_type_id);
206202
let note_hash_impl = generate_note_hash_trait_impl(s);
207203

208204
quote {
209205
$note_properties
210-
$note_interface_impl
206+
$note_type_impl
211207
$note_hash_impl
212208
}
213209
}
@@ -227,9 +223,8 @@ pub comptime fn note(s: TypeDefinition) -> Quoted {
227223
/// Unlike the `#[note]` macro, there is no requirement for an `owner` field.
228224
///
229225
/// # Registration
230-
/// Registers the note in the global `NOTES` map with:
231-
/// - Note type ID
232-
/// - Packed length
226+
///
227+
/// Registers the note in the global `NOTES` BoundedVec to enable note processing functionality.
233228
///
234229
/// # Use Cases
235230
/// Use this macro when implementing a note that needs custom:
@@ -258,27 +253,17 @@ pub comptime fn note(s: TypeDefinition) -> Quoted {
258253
pub comptime fn custom_note(s: TypeDefinition) -> Quoted {
259254
assert_has_packable(s);
260255

261-
let note_packed_len = get_packable_len(s.as_type());
262-
263-
// Ensure the note's packed length does not exceed the maximum allowable size to prevent overflow
264-
// when processing the note.
265-
if note_packed_len > MAX_NOTE_PACKED_LEN {
266-
panic(
267-
f"{s} packs into {note_packed_len} fields which is larger than the maximum of {MAX_NOTE_PACKED_LEN}",
268-
);
269-
}
270-
271-
// We register the note in the global `NOTES` map with the packed length because we need that information
272-
// inside the #[aztec] macro to generate note processing functionality.
273-
NOTES.insert(s.as_type(), note_packed_len);
256+
// We register the note in the global `NOTES` BoundedVec because we need that information inside the #[aztec] macro
257+
// to generate note processing functionality.
258+
NOTES.push(s.as_type());
274259

275260
let note_type_id = get_next_note_type_id();
276261
let note_properties = generate_note_properties(s);
277-
let note_interface_impl = generate_note_interface(s, note_type_id);
262+
let note_type_impl = generate_note_type_impl(s, note_type_id);
278263

279264
quote {
280265
$note_properties
281-
$note_interface_impl
266+
$note_type_impl
282267
}
283268
}
284269

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
[package]
2+
name = "invalid_note"
3+
authors = [""]
4+
compiler_version = ">=0.25.0"
5+
type = "contract"
6+
7+
[dependencies]
8+
aztec = { path = "../../../aztec-nr/aztec" }
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Note packed length exceeds the maximum of 12 fields
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
use aztec::{
2+
macros::notes::note,
3+
messages::discovery::private_notes::MAX_NOTE_PACKED_LEN,
4+
protocol_types::{address::AztecAddress, traits::{FromField, Packable}},
5+
};
6+
7+
#[note]
8+
pub struct InvalidNote {
9+
pub owner: AztecAddress,
10+
}
11+
12+
impl Packable for InvalidNote {
13+
let N: u32 = MAX_NOTE_PACKED_LEN + 1;
14+
15+
fn pack(self) -> [Field; Self::N] {
16+
std::mem::zeroed()
17+
}
18+
19+
fn unpack(packed: [Field; Self::N]) -> Self {
20+
Self { owner: AztecAddress::from_field(packed[0]) }
21+
}
22+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
use aztec::macros::aztec;
2+
3+
mod invalid_note;
4+
5+
#[aztec]
6+
pub contract InvalidNoteContract {
7+
}

0 commit comments

Comments
 (0)