diff --git a/cspell.json b/cspell.json index 14cdde7586bc..230fb66f8e92 100644 --- a/cspell.json +++ b/cspell.json @@ -275,6 +275,7 @@ "structs", "subarray", "subarrays", + "subbvec", "subdir", "sublabel", "sublabels", @@ -307,9 +308,9 @@ "unnullify", "unpadded", "unprefixed", - "unsiloed", "unshift", "unshifted", + "unsiloed", "unsynched", "unzipit", "updateable", diff --git a/noir-projects/aztec-nr/aztec/src/discovery/private_logs.nr b/noir-projects/aztec-nr/aztec/src/discovery/private_logs.nr index aa919e233966..f19af033700a 100644 --- a/noir-projects/aztec-nr/aztec/src/discovery/private_logs.nr +++ b/noir-projects/aztec-nr/aztec/src/discovery/private_logs.nr @@ -5,7 +5,10 @@ use crate::{ private_notes::process_private_note_log, }, encrypted_logs::{ - log_assembly_strategies::default_aes128::aes128::AES128, log_encryption::LogEncryption, + log_assembly_strategies::default_aes128::aes128::AES128, + log_encryption::LogEncryption, + log_type::{PARTIAL_NOTE_PRIVATE_LOG_TYPE_ID, PRIVATE_NOTE_LOG_TYPE_ID}, + metadata_packing::from_expanded_metadata, }, oracle::message_discovery::sync_notes, utils::array, @@ -60,12 +63,11 @@ pub unconstrained fn do_process_log( let log_plaintext = AES128::decrypt_log(ciphertext, recipient); // The first thing to do after decrypting the log is to determine what type of private log we're processing. We - // currently just have two log types: 0 for private notes and 1 for partial notes. This will likely be expanded and - // improved upon in the future to also handle events, etc. + // have 3 log types: private note logs, partial note logs and event logs. let (log_type_id, log_metadata, log_content) = decode_log_plaintext(log_plaintext); - if log_type_id == 0 { + if log_type_id == PRIVATE_NOTE_LOG_TYPE_ID { debug_log("Processing private note log"); process_private_note_log( @@ -78,7 +80,7 @@ pub unconstrained fn do_process_log( log_metadata, log_content, ); - } else if log_type_id == 1 { + } else if log_type_id == PARTIAL_NOTE_PRIVATE_LOG_TYPE_ID { debug_log("Processing partial note private log"); process_partial_note_private_log(contract_address, recipient, log_metadata, log_content); @@ -101,13 +103,13 @@ pub unconstrained fn do_process_log( /// log_plaintext: [ log_expanded_metadata, ...log_content ] /// ``` /// -/// The expanded metadata itself is (currently) interpreted as a u64, of which: -/// - the upper 57 bits are the log type id -/// - the remaining 7 bits are called the 'log metadata' +/// The expanded metadata itself is (currently) interpreted as a u128, of which: +/// - the upper 64 bits are the log type id +/// - the lower 64 bits are called the 'log metadata' /// /// ``` -/// log_expanded_metadata: [ log_type_id | log_metadata ] -/// <--- 57 bits --->|<--- 7 bits ---> +/// log_expanded_metadata: [ log_type_id | log_metadata ] +/// <--- 64 bits --->|<--- 64 bits ---> /// ``` /// /// The meaning of the log metadata and log content depend on the value of the log type id. Note that there is @@ -130,10 +132,7 @@ unconstrained fn decode_log_plaintext( // See the documentation of this function for a description of the log layout let expanded_log_metadata = log_plaintext.get(0); - - let log_type_id = ((expanded_log_metadata as u64) / 128); - let log_metadata = ((expanded_log_metadata as u64) % 128); - + let (log_metadata, log_type_id) = from_expanded_metadata(expanded_log_metadata); let log_content = array::subbvec(log_plaintext, PRIVATE_LOG_EXPANDED_METADATA_LEN); (log_type_id, log_metadata, log_content) diff --git a/noir-projects/aztec-nr/aztec/src/encrypted_logs/log_assembly_strategies/default_aes128/note.nr b/noir-projects/aztec-nr/aztec/src/encrypted_logs/log_assembly_strategies/default_aes128/note.nr index eacfca5f0f36..5a4bca07a58e 100644 --- a/noir-projects/aztec-nr/aztec/src/encrypted_logs/log_assembly_strategies/default_aes128/note.nr +++ b/noir-projects/aztec-nr/aztec/src/encrypted_logs/log_assembly_strategies/default_aes128/note.nr @@ -1,7 +1,10 @@ use crate::{ context::PrivateContext, encrypted_logs::{ - log_assembly_strategies::default_aes128::aes128::AES128, log_encryption::LogEncryption, + log_assembly_strategies::default_aes128::aes128::AES128, + log_encryption::LogEncryption, + log_type::{PARTIAL_NOTE_PRIVATE_LOG_TYPE_ID, PRIVATE_NOTE_LOG_TYPE_ID}, + metadata_packing::to_expanded_metadata, }, note::{note_emission::NoteEmission, note_interface::NoteType}, oracle::notes::{get_app_tag_as_sender, increment_app_tagging_secret_index_as_sender}, @@ -26,6 +29,7 @@ fn assert_note_exists(context: PrivateContext, note_hash_counter: u3 fn compute_note_plaintext_for_this_strategy( note: Note, storage_slot: Field, + log_type_id: u64, ) -> [Field; (N + 2)] where Note: NoteType + Packable, @@ -34,14 +38,9 @@ where let mut fields = [0; N + 2]; - // Note that we're almost accidentally following the standard log encoding here: because the note type id only uses - // 7 bits, it just barely fits in the log metadata, and the log type id is implicitly 0 (i.e. a private note log). - // Partial notes modify `get_id` to have it set the 8th bit, resulting in a log type id of 1 (i.e. a partial note - // private log). Fields 1 to len are the note content, which we here hardcode to be the storage slot in the first - // field, and the packed note following after. - // Ideally we'd know if this is a private note or a partial note, and call a function that'd be the opposite of - // discovery::private_notes::decode_private_note_log (or partial accordingly). - fields[0] = Note::get_id(); + // We pack log type id and log metadata into the first field. Search for `decode_log_plaintext` function to see + // where the value gets decoded. + fields[0] = to_expanded_metadata(Note::get_id() as u64, log_type_id); fields[1] = storage_slot; for i in 0..packed_note.len() { fields[i + 2] = packed_note[i]; @@ -50,7 +49,7 @@ where fields } -pub fn compute_log( +pub fn compute_note_log( note: Note, storage_slot: Field, recipient: AztecAddress, @@ -59,7 +58,44 @@ pub fn compute_log( where Note: NoteType + Packable, { - let plaintext = compute_note_plaintext_for_this_strategy(note, storage_slot); + compute_log( + note, + storage_slot, + recipient, + sender, + PRIVATE_NOTE_LOG_TYPE_ID, + ) +} + +pub fn compute_partial_note_log( + note: Note, + storage_slot: Field, + recipient: AztecAddress, + sender: AztecAddress, +) -> [Field; PRIVATE_LOG_SIZE_IN_FIELDS] +where + Note: NoteType + Packable, +{ + compute_log( + note, + storage_slot, + recipient, + sender, + PARTIAL_NOTE_PRIVATE_LOG_TYPE_ID, + ) +} + +fn compute_log( + note: Note, + storage_slot: Field, + recipient: AztecAddress, + sender: AztecAddress, + log_type_id: u64, +) -> [Field; PRIVATE_LOG_SIZE_IN_FIELDS] +where + Note: NoteType + Packable, +{ + let plaintext = compute_note_plaintext_for_this_strategy(note, storage_slot, log_type_id); let ciphertext = AES128::encrypt_log(plaintext, recipient); @@ -89,7 +125,7 @@ fn prefix_with_tag( log_with_tag } -pub unconstrained fn compute_log_unconstrained( +pub unconstrained fn compute_note_log_unconstrained( note: Note, storage_slot: Field, recipient: AztecAddress, @@ -98,7 +134,7 @@ pub unconstrained fn compute_log_unconstrained( where Note: NoteType + Packable, { - compute_log(note, storage_slot, recipient, sender) + compute_note_log(note, storage_slot, recipient, sender) } /// Sends an encrypted message to `recipient` with the content of the note, which they will discover when processing @@ -118,7 +154,7 @@ where let note_hash_counter = e.note_hash_counter; assert_note_exists(*context, note_hash_counter); - let encrypted_log = compute_log(note, storage_slot, recipient, sender); + let encrypted_log = compute_note_log(note, storage_slot, recipient, sender); context.emit_raw_note_log(encrypted_log, note_hash_counter); } } @@ -146,7 +182,7 @@ where // Safety: this function does not constrain the encryption of the log, as explained on its description. let encrypted_log = - unsafe { compute_log_unconstrained(note, storage_slot, recipient, sender) }; + unsafe { compute_note_log_unconstrained(note, storage_slot, recipient, sender) }; context.emit_raw_note_log(encrypted_log, note_hash_counter); } } diff --git a/noir-projects/aztec-nr/aztec/src/encrypted_logs/log_type.nr b/noir-projects/aztec-nr/aztec/src/encrypted_logs/log_type.nr new file mode 100644 index 000000000000..951bbac8d24b --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/encrypted_logs/log_type.nr @@ -0,0 +1,3 @@ +pub global PRIVATE_NOTE_LOG_TYPE_ID: u64 = 0; +pub global PARTIAL_NOTE_PRIVATE_LOG_TYPE_ID: u64 = 1; +pub global PRIVATE_EVENT_LOG_TYPE_ID: u64 = 2; diff --git a/noir-projects/aztec-nr/aztec/src/encrypted_logs/metadata_packing.nr b/noir-projects/aztec-nr/aztec/src/encrypted_logs/metadata_packing.nr new file mode 100644 index 000000000000..14026085d54a --- /dev/null +++ b/noir-projects/aztec-nr/aztec/src/encrypted_logs/metadata_packing.nr @@ -0,0 +1,89 @@ +global U64_SHIFT_MULTIPLIER: Field = 2.pow_32(64); + +pub fn to_expanded_metadata(log_metadata: u64, log_type: u64) -> Field { + let metadata_field = log_metadata as Field; + // We use multiplication instead of bit shifting operations to shift the type bits as bit shift operations are + // expensive in circuits. + let type_field: Field = (log_type as Field) * U64_SHIFT_MULTIPLIER; + type_field + metadata_field +} + +pub fn from_expanded_metadata(input: Field) -> (u64, u64) { + input.assert_max_bit_size::<128>(); + let metadata = (input as u64); + // Use division instead of bit shift since bit shifts are expensive in circuits + let log_type = ((input - (metadata as Field)) / U64_SHIFT_MULTIPLIER) as u64; + (metadata, log_type) +} + +mod tests { + use super::{from_expanded_metadata, to_expanded_metadata}; + + global U64_MAX: Field = 2.pow_32(64) - 1; + global U128_MAX: Field = 2.pow_32(128) - 1; + + #[test] + fn packing_metadata() { + // Test case 1: All bits set + let packed = to_expanded_metadata(U64_MAX as u64, U64_MAX as u64); + let (metadata, log_type) = from_expanded_metadata(packed); + assert(metadata == U64_MAX as u64, "Metadata bits should be all 1s"); + assert(log_type == U64_MAX as u64, "Log type bits should be all 1s"); + + // Test case 2: Only log type bits set + let packed = to_expanded_metadata(0, U64_MAX as u64); + let (metadata, log_type) = from_expanded_metadata(packed); + assert(metadata == 0, "Metadata bits should be 0"); + assert(log_type == U64_MAX as u64, "Log type bits should be all 1s"); + + // Test case 3: Only metadata bits set + let packed = to_expanded_metadata(U64_MAX as u64, 0); + let (metadata, log_type) = from_expanded_metadata(packed); + assert(metadata == U64_MAX as u64, "Metadata bits should be all 1s"); + assert(log_type == 0, "Log type bits should be 0"); + + // Test case 4: Zero + let packed = to_expanded_metadata(0, 0); + let (metadata, log_type) = from_expanded_metadata(packed); + assert(metadata == 0, "Metadata bits should be 0"); + assert(log_type == 0, "Log type bits should be 0"); + } + + #[test] + fn unpacking_metadata() { + // Test case 1: All bits set + let input = U128_MAX; + let (metadata, log_type) = from_expanded_metadata(input); + assert(metadata == U64_MAX as u64, "Metadata bits should be all 1s"); + assert(log_type == U64_MAX as u64, "Log type bits should be all 1s"); + + // Test case 2: Only log type bits set + let input = U128_MAX - U64_MAX; + let (metadata, log_type) = from_expanded_metadata(input); + assert(metadata == 0, "Metadata bits should be 0"); + assert(log_type == U64_MAX as u64, "Log type bits should be all 1s"); + + // Test case 3: Only metadata bits set + let input = U64_MAX; + let (metadata, log_type) = from_expanded_metadata(input); + assert(metadata == U64_MAX as u64, "Metadata bits should be all 1s"); + assert(log_type == 0, "Log type bits should be 0"); + + // Test case 4: Zero + let input = 0; + let (metadata, log_type) = from_expanded_metadata(input); + assert(metadata == 0, "Metadata bits should be 0"); + assert(log_type == 0, "Log type bits should be 0"); + } + + #[test] + fn roundtrip_metadata(original_metadata: u64, original_type: u64) { + let packed = to_expanded_metadata(original_metadata, original_type); + let (unpacked_metadata, unpacked_type) = from_expanded_metadata(packed); + assert(original_type == unpacked_type, "Log type bits should match after roundtrip"); + assert( + original_metadata == unpacked_metadata, + "Metadata bits should match after roundtrip", + ); + } +} diff --git a/noir-projects/aztec-nr/aztec/src/encrypted_logs/mod.nr b/noir-projects/aztec-nr/aztec/src/encrypted_logs/mod.nr index c84b9b14f6a2..9ef1ffa02dce 100644 --- a/noir-projects/aztec-nr/aztec/src/encrypted_logs/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/encrypted_logs/mod.nr @@ -1,3 +1,5 @@ pub mod encrypt; pub mod log_assembly_strategies; pub mod log_encryption; +pub mod log_type; +pub mod metadata_packing; diff --git a/noir-projects/aztec-nr/uint-note/src/uint_note.nr b/noir-projects/aztec-nr/uint-note/src/uint_note.nr index b0b761e20a47..450177208e35 100644 --- a/noir-projects/aztec-nr/uint-note/src/uint_note.nr +++ b/noir-projects/aztec-nr/uint-note/src/uint_note.nr @@ -143,12 +143,12 @@ impl UintNote { public_log_tag: commitment, }; - // TODO: we're abusing the note encoding scheme by computing the log for a fake note type with such a note type - // id that the recipient will realize that these are the private fields of a partial note. Ideally we'd not rely - // on this crude mechanism and we'd instead compute it as a proper event log. However, given the current state - // of the log library it's far easier to do it this way. - let encrypted_log = - default_aes128::note::compute_log(private_log_content, storage_slot, recipient, sender); + let encrypted_log = default_aes128::note::compute_partial_note_log( + private_log_content, + storage_slot, + recipient, + sender, + ); context.emit_private_log(encrypted_log); PartialUintNote { commitment } @@ -188,9 +188,7 @@ struct PrivateUintPartialNotePrivateLogContent { impl NoteType for PrivateUintPartialNotePrivateLogContent { fn get_id() -> Field { - // We abuse the fact that note type ids are 7 bits long to use the 8th bit indicate the log corresponds to a - // partial note. Ideally we'd use proper events with selectors, but those are not handled well at the moment. - UintNote::get_id() + 128 + UintNote::get_id() } } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/types/nft_note.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/types/nft_note.nr index 44cbe8d5a881..12d10ced4e40 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/types/nft_note.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/types/nft_note.nr @@ -1,6 +1,8 @@ use dep::aztec::{ context::{PrivateContext, PublicContext}, - encrypted_logs::log_assembly_strategies::default_aes128, + encrypted_logs::{ + log_assembly_strategies::default_aes128, log_type::PARTIAL_NOTE_PRIVATE_LOG_TYPE_ID, + }, keys::getters::{get_nsk_app, get_public_keys}, macros::notes::custom_note, note::note_interface::{NoteHash, NoteType}, @@ -113,7 +115,7 @@ impl NFTNote { /// Each partial note should only be used once, since otherwise multiple notes would be linked together and known to /// belong to the same owner. /// - /// As part of the partial note cration process, a log will be sent to `recipient` from `sender` so that they can + /// As part of the partial note creation process, a log will be sent to `recipient` from `sender` so that they can /// discover the note. `recipient` will typically be the same as `owner`. pub fn partial( owner: AztecAddress, @@ -144,12 +146,12 @@ impl NFTNote { public_log_tag: commitment, }; - // TODO: we're abusing the note encoding scheme by computing the log for a fake note type with such a note type - // id that the recipient will realize that these are the private fields of a partial note. Ideally we'd not rely - // on this crude mechanism and we'd instead compute it as a proper event log. However, given the current state - // of the log library it's far easier to do it this way. - let encrypted_log = - default_aes128::note::compute_log(private_log_content, storage_slot, recipient, sender); + let encrypted_log = default_aes128::note::compute_partial_note_log( + private_log_content, + storage_slot, + recipient, + sender, + ); context.emit_private_log(encrypted_log); PartialNFTNote { commitment } @@ -189,15 +191,13 @@ struct PrivateNFTPartialNotePrivateLogContent { impl NoteType for PrivateNFTPartialNotePrivateLogContent { fn get_id() -> Field { - // We abuse the fact that note type ids are 7 bits long to use the 8th bit indicate the log corresponds to a - // partial note. Ideally we'd use proper events with selectors, but those are not handled well at the moment. - NFTNote::get_id() + 128 + NFTNote::get_id() } } /// A partial instance of a NFTNote. This value represents a private commitment to the owner, randomness and storage /// slot, but the token id field has not yet been set. A partial note can be completed in public with the `complete` -/// function (revealing the tolken id to the public), resulting in a NFTNote that can be used like any other one (except +/// function (revealing the token id to the public), resulting in a NFTNote that can be used like any other one (except /// of course that its token id is known). #[derive(Packable, Serialize, Deserialize)] pub struct PartialNFTNote { @@ -307,7 +307,7 @@ mod test { partial_note.compute_note_completion_log(token_id)[0], ); - // Then we exctract all fields except the first of both logs (i.e. the public log tag), and combine them to + // Then we extract all fields except the first of both logs (i.e. the public log tag), and combine them to // produce the note's packed representation. This requires that the members of the intermediate structs are in // the same order as in NFTNote. let private_log_without_public_tag: [_; 2] = subarray(private_log_content.pack(), 1);