diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ef5f4d4f7..eae7a5205b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### Changes +- No longer pad the note inputs on insertion into advice map ([#2232](https://github.com/0xMiden/miden-base/pull/2232)). - Added proc-macro `WordWrapper` to ease implementation of `Word`-wrapping types ([#2071](https://github.com/0xMiden/miden-base/pull/2108)). - [BREAKING] Added `BlockBody` and `BlockProof` structs in preparation for validator signatures and deferred block proving ([#2012](https://github.com/0xMiden/miden-base/pull/2012)). - [BREAKING] Renamed `TransactionEvent` into `TransactionEventId` and split event handling into data extraction and handling logic ([#2071](https://github.com/0xMiden/miden-base/pull/2071)). diff --git a/crates/miden-protocol/asm/protocol/active_note.masm b/crates/miden-protocol/asm/protocol/active_note.masm index 3a32919912..9cda06a621 100644 --- a/crates/miden-protocol/asm/protocol/active_note.masm +++ b/crates/miden-protocol/asm/protocol/active_note.masm @@ -1,3 +1,4 @@ +use miden::core::crypto::hashes::rpo256 use miden::core::mem use miden::protocol::kernel_proc_offsets @@ -95,7 +96,7 @@ end #! #! Where: #! - dest_ptr is the memory address to write the note inputs. -#! - NOTE_INPUTS_COMMITMENT is the sequential hash of the padded note's inputs. +#! - NOTE_INPUTS_COMMITMENT is the commitment to the note's inputs. #! - INPUTS is the data corresponding to the note's inputs. #! #! Panics if: @@ -274,7 +275,11 @@ end #! Operand stack: [num_inputs, dest_ptr] proc write_inputs_to_memory # load the inputs from the advice map to the advice stack - adv.push_mapvaln + # we pad the number of inputs to the next multiple of 8 so that we can use the + # `pipe_double_words_to_memory` instruction. The padded zeros don't affect the commitment + # computation. + + adv.push_mapvaln.8 # OS => [NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] # AS => [advice_num_inputs, [INPUT_VALUES]] @@ -283,12 +288,6 @@ proc write_inputs_to_memory # OS => [num_inputs, advice_num_inputs, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] # AS => [[INPUT_VALUES]] - # Validate the note inputs length. Round up the number of inputs to the next multiple of 8: that - # value should be equal to the length obtained from the `adv.push_mapvaln` procedure. - u32divmod.8 neq.0 add mul.8 - # OS => [rounded_up_num_inputs, advice_num_inputs, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] - # AS => [[INPUT_VALUES]] - assert_eq.err=ERR_NOTE_INVALID_NUMBER_OF_INPUTS # OS => [NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] # AS => [[INPUT_VALUES]] @@ -303,13 +302,45 @@ proc write_inputs_to_memory # OS => [even_num_words, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] # AS => [[INPUT_VALUES]] - # prepare the stack for the `pipe_preimage_to_memory` procedure - dup.6 swap - # OS => [even_num_words, dest_ptr, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] + # compute the end pointer for writing the padded inputs (even_num_words * 4 elements) + dup.6 swap mul.4 add + # OS => [end_ptr, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] + # AS => [[INPUT_VALUES]] + + # prepare the stack for the `pipe_double_words_to_memory` procedure. + # + # To match `rpo256::hash_elements` (used for NOTE_INPUTS_COMMITMENT), we set the first capacity + # element to `num_inputs % 8`. + dup.6 dup.6 + # OS => [num_inputs, write_ptr, end_ptr, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] + # AS => [[INPUT_VALUES]] + + u32divmod.8 swap drop + # OS => [num_inputs_mod_8, write_ptr, end_ptr, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] + # AS => [[INPUT_VALUES]] + + push.0.0.0 + # OS => [A, write_ptr, end_ptr, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr], where A = [0, 0, 0, num_inputs_mod_8] + # AS => [[INPUT_VALUES]] + + padw padw + # OS => [PAD, PAD, A, write_ptr, end_ptr, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] # AS => [[INPUT_VALUES]] # write the inputs from the advice stack into memory - exec.mem::pipe_preimage_to_memory drop - # OS => [num_inputs, dest_ptr] + exec.mem::pipe_double_words_to_memory + # OS => [PERM, PERM, PERM, end_ptr', NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] # AS => [] + + # extract the computed commitment from the hasher state + exec.rpo256::squeeze_digest + # OS => [COMPUTED_COMMITMENT, end_ptr', NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] + + # drop end_ptr' + movup.4 drop + # OS => [COMPUTED_COMMITMENT, NOTE_INPUTS_COMMITMENT, num_inputs, dest_ptr] + + # validate that the inputs written to memory match the inputs commitment + assert_eqw.err=ERR_NOTE_DATA_DOES_NOT_MATCH_COMMITMENT + # => [num_inputs, dest_ptr] end diff --git a/crates/miden-protocol/asm/protocol/note.masm b/crates/miden-protocol/asm/protocol/note.masm index 284c68d4ea..85730c9528 100644 --- a/crates/miden-protocol/asm/protocol/note.masm +++ b/crates/miden-protocol/asm/protocol/note.masm @@ -3,6 +3,9 @@ use miden::core::crypto::hashes::rpo256 use miden::core::math::u64 use miden::core::mem +# Re-export the max inputs per note constant. +pub use ::miden::protocol::util::note::MAX_INPUTS_PER_NOTE + # ERRORS # ================================================================================================= @@ -16,8 +19,6 @@ const ERR_PROLOGUE_NOTE_INPUTS_LEN_EXCEEDED_LIMIT="number of note inputs exceede #! This procedure checks that the provided number of note inputs is within limits and then computes #! the commitment. #! -#! Notice that the note inputs are padded with zeros in case their number is not a multiple of 8. -#! #! If the number of note inputs is 0, procedure returns the empty word: [0, 0, 0, 0]. #! #! Inputs: [inputs_ptr, num_inputs] @@ -38,18 +39,11 @@ pub proc compute_inputs_commitment u32lte assert.err=ERR_PROLOGUE_NOTE_INPUTS_LEN_EXCEEDED_LIMIT # => [inputs_ptr, num_inputs] - # compute the inputs commitment - # - # TODO: refactor the hash computation process as part of the - # https://github.com/0xMiden/miden-base/issues/2036 and - # https://github.com/0xMiden/miden-base/issues/2129 - exec.rpo256::pad_and_hash_elements + # compute the inputs commitment (over the unpadded values) + exec.rpo256::hash_elements # => [INPUTS_COMMITMENT] end -# Re-export the max inputs per note constant. -pub use ::miden::protocol::util::note::MAX_INPUTS_PER_NOTE - #! Writes the assets data stored in the advice map to the memory specified by the provided #! destination pointer. #! diff --git a/crates/miden-protocol/src/note/inputs.rs b/crates/miden-protocol/src/note/inputs.rs index 57b3b9365d..7d7b04a0c6 100644 --- a/crates/miden-protocol/src/note/inputs.rs +++ b/crates/miden-protocol/src/note/inputs.rs @@ -8,7 +8,7 @@ use crate::utils::serde::{ DeserializationError, Serializable, }; -use crate::{Felt, Hasher, MAX_INPUTS_PER_NOTE, WORD_SIZE, Word, ZERO}; +use crate::{Felt, Hasher, MAX_INPUTS_PER_NOTE, Word}; // NOTE INPUTS // ================================================================================================ @@ -18,9 +18,8 @@ use crate::{Felt, Hasher, MAX_INPUTS_PER_NOTE, WORD_SIZE, Word, ZERO}; /// A note can be associated with up to 1024 input values. Each value is represented by a single /// field element. Thus, note input values can contain up to ~8 KB of data. /// -/// All inputs associated with a note can be reduced to a single commitment which is computed by -/// first padding the inputs with ZEROs to the next multiple of 8, and then by computing a -/// sequential hash of the resulting elements. +/// All inputs associated with a note can be reduced to a single commitment which is computed as an +/// RPO256 hash over the input elements. #[derive(Clone, Debug)] pub struct NoteInputs { values: Vec, @@ -40,7 +39,9 @@ impl NoteInputs { return Err(NoteError::TooManyInputs(values.len())); } - Ok(pad_and_build(values)) + let commitment = Hasher::hash_elements(&values); + + Ok(Self { values, commitment }) } // PUBLIC ACCESSORS @@ -69,19 +70,14 @@ impl NoteInputs { } /// Returns the note's input as a vector of field elements. - /// - /// The format is `INPUTS || PADDING`, where: - /// - /// - INPUTS is the variable inputs for the note - /// - PADDING is the optional padding to align the data with a 2WORD boundary pub fn to_elements(&self) -> Vec { - pad_inputs(&self.values) + self.values.to_vec() } } impl Default for NoteInputs { fn default() -> Self { - pad_and_build(vec![]) + Self::new(vec![]).expect("empty values should be valid") } } @@ -111,31 +107,6 @@ impl TryFrom> for NoteInputs { } } -// HELPER FUNCTIONS -// ================================================================================================ - -/// Returns a vector with built from the provided inputs and padded to the next multiple of 8. -fn pad_inputs(inputs: &[Felt]) -> Vec { - const BLOCK_SIZE: usize = WORD_SIZE * 2; - - let padded_len = inputs.len().next_multiple_of(BLOCK_SIZE); - let mut padded_inputs = Vec::with_capacity(padded_len); - padded_inputs.extend(inputs.iter()); - padded_inputs.resize(padded_len, ZERO); - - padded_inputs -} - -/// Pad `values` and returns a new `NoteInputs`. -fn pad_and_build(values: Vec) -> NoteInputs { - let commitment = { - let padded_values = pad_inputs(&values); - Hasher::hash_elements(&padded_values) - }; - - NoteInputs { values, commitment } -} - // SERIALIZATION // ================================================================================================ @@ -168,7 +139,7 @@ mod tests { fn test_input_ordering() { // inputs are provided in reverse stack order let inputs = vec![Felt::new(1), Felt::new(2), Felt::new(3)]; - // we expect the inputs to be padded to length 16 and to remain in reverse stack order. + // we expect the inputs to remain in reverse stack order. let expected_ordering = vec![Felt::new(1), Felt::new(2), Felt::new(3)]; let note_inputs = NoteInputs::new(inputs).expect("note created should succeed"); diff --git a/crates/miden-protocol/src/transaction/kernel/memory.rs b/crates/miden-protocol/src/transaction/kernel/memory.rs index 9cbfcd5754..f5db5d4e1e 100644 --- a/crates/miden-protocol/src/transaction/kernel/memory.rs +++ b/crates/miden-protocol/src/transaction/kernel/memory.rs @@ -358,7 +358,7 @@ pub const NOTE_MEM_SIZE: MemoryAddress = 2048; // // Notice that note input values are not loaded to the memory, only their length. In order to obtain // the input values the advice map should be used: they are stored there as -// `INPUTS_COMMITMENT -> INPUTS || PADDING`. +// `INPUTS_COMMITMENT -> INPUTS`. // // As opposed to the asset values, input values are never used in kernel memory, so their presence // there is unnecessary. diff --git a/crates/miden-testing/src/kernel_tests/tx/test_note.rs b/crates/miden-testing/src/kernel_tests/tx/test_note.rs index 5d761de2d1..c8dec803f5 100644 --- a/crates/miden-testing/src/kernel_tests/tx/test_note.rs +++ b/crates/miden-testing/src/kernel_tests/tx/test_note.rs @@ -29,7 +29,7 @@ use miden_protocol::testing::account_id::{ }; use miden_protocol::transaction::memory::ACTIVE_INPUT_NOTE_PTR; use miden_protocol::transaction::{OutputNote, TransactionArgs}; -use miden_protocol::{Felt, Word, ZERO}; +use miden_protocol::{Felt, Hasher, Word, ZERO}; use miden_standards::account::wallets::BasicWallet; use miden_standards::code_builder::CodeBuilder; use miden_standards::testing::note::NoteBuilder; @@ -198,43 +198,38 @@ async fn test_build_recipient() -> anyhow::Result<()> { // Define test values as Words let word_1 = Word::from([1, 2, 3, 4u32]); let word_2 = Word::from([5, 6, 7, 8u32]); - let word_3 = Word::from([9, 10, 11, 12u32]); - let word_4 = Word::from([13, 14, 15, 16u32]); const BASE_ADDR: u32 = 4000; let code = format!( " use miden::core::sys - use miden::protocol::note begin # put the values that will be hashed into the memory push.{word_1} push.{base_addr} mem_storew_be dropw push.{word_2} push.{addr_1} mem_storew_be dropw - push.{word_3} push.{addr_2} mem_storew_be dropw - push.{word_4} push.{addr_3} mem_storew_be dropw - # Test with 4 values + # Test with 4 values (needs padding to 8) push.{script_root} # SCRIPT_ROOT push.{serial_num} # SERIAL_NUM push.4.4000 # num_inputs, inputs_ptr exec.note::build_recipient # => [RECIPIENT_4] - # Test with 5 values + # Test with 5 values (needs padding to 8) push.{script_root} # SCRIPT_ROOT push.{serial_num} # SERIAL_NUM push.5.4000 # num_inputs, inputs_ptr exec.note::build_recipient # => [RECIPIENT_5, RECIPIENT_4] - # Test with 13 values + # Test with 8 values (no padding needed - exactly one rate block) push.{script_root} # SCRIPT_ROOT push.{serial_num} # SERIAL_NUM - push.13.4000 # num_inputs, inputs_ptr + push.8.4000 # num_inputs, inputs_ptr exec.note::build_recipient - # => [RECIPIENT_13, RECIPIENT_5, RECIPIENT_4] + # => [RECIPIENT_8, RECIPIENT_5, RECIPIENT_4] # truncate the stack exec.sys::truncate_stack @@ -242,38 +237,56 @@ async fn test_build_recipient() -> anyhow::Result<()> { ", word_1 = word_1, word_2 = word_2, - word_3 = word_3, - word_4 = word_4, base_addr = BASE_ADDR, addr_1 = BASE_ADDR + 4, - addr_2 = BASE_ADDR + 8, - addr_3 = BASE_ADDR + 12, script_root = note_script.root(), serial_num = serial_num, ); let exec_output = &tx_context.execute_code(&code).await?; - // Create expected recipients and get their digests - let note_inputs_4 = NoteInputs::new(word_1.to_vec())?; - let recipient_4 = NoteRecipient::new(serial_num, note_script.clone(), note_inputs_4); + // Create expected NoteInputs for each test case + let inputs_4 = word_1.to_vec(); + let note_inputs_4 = NoteInputs::new(inputs_4.clone())?; let mut inputs_5 = word_1.to_vec(); inputs_5.push(word_2[0]); - let note_inputs_5 = NoteInputs::new(inputs_5)?; - let recipient_5 = NoteRecipient::new(serial_num, note_script.clone(), note_inputs_5); + let note_inputs_5 = NoteInputs::new(inputs_5.clone())?; - let mut inputs_13 = word_1.to_vec(); - inputs_13.extend_from_slice(&word_2.to_vec()); - inputs_13.extend_from_slice(&word_3.to_vec()); - inputs_13.push(word_4[0]); - let note_inputs_13 = NoteInputs::new(inputs_13)?; - let recipient_13 = NoteRecipient::new(serial_num, note_script, note_inputs_13); + let mut inputs_8 = word_1.to_vec(); + inputs_8.extend_from_slice(&word_2.to_vec()); + let note_inputs_8 = NoteInputs::new(inputs_8.clone())?; + + // Create expected recipients and get their digests + let recipient_4 = NoteRecipient::new(serial_num, note_script.clone(), note_inputs_4.clone()); + let recipient_5 = NoteRecipient::new(serial_num, note_script.clone(), note_inputs_5.clone()); + let recipient_8 = NoteRecipient::new(serial_num, note_script.clone(), note_inputs_8.clone()); + + for note_inputs in [ + (note_inputs_4, inputs_4.clone()), + (note_inputs_5, inputs_5.clone()), + (note_inputs_8, inputs_8.clone()), + ] { + let inputs_advice_map_key = note_inputs.0.commitment(); + assert_eq!( + exec_output.advice.get_mapped_values(&inputs_advice_map_key).unwrap(), + note_inputs.1, + "advice entry with note inputs should contain the unpadded values" + ); + + let num_inputs_advice_map_key = + Hasher::hash_elements(note_inputs.0.commitment().as_elements()); + assert_eq!( + exec_output.advice.get_mapped_values(&num_inputs_advice_map_key).unwrap(), + &[Felt::from(note_inputs.0.num_values())], + "advice entry with num note inputs should contain the original number of values" + ); + } let mut expected_stack = alloc::vec::Vec::new(); expected_stack.extend_from_slice(recipient_4.digest().as_elements()); expected_stack.extend_from_slice(recipient_5.digest().as_elements()); - expected_stack.extend_from_slice(recipient_13.digest().as_elements()); + expected_stack.extend_from_slice(recipient_8.digest().as_elements()); expected_stack.reverse(); assert_eq!(exec_output.stack[0..12], expected_stack); diff --git a/docs/src/protocol_library.md b/docs/src/protocol_library.md index b67662d744..01bc92dee6 100644 --- a/docs/src/protocol_library.md +++ b/docs/src/protocol_library.md @@ -117,7 +117,6 @@ Note utility procedures can be used to compute the required utility data or writ | Procedure | Description | Context | | --------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- | | `compute_inputs_commitment` | Computes the commitment to the output note inputs starting at the specified memory address.

**Inputs:** `[inputs_ptr, num_inputs]`
**Outputs:** `[INPUTS_COMMITMENT]` | Any | -| `get_max_inputs_per_note` | Returns the max allowed number of input values per note.

**Inputs:** `[]`
**Outputs:** `[max_inputs_per_note]` | Any | | `write_assets_to_memory` | Writes the assets data stored in the advice map to the memory specified by the provided destination pointer.

**Inputs:** `[ASSETS_COMMITMENT, num_assets, dest_ptr]`
**Outputs:** `[num_assets, dest_ptr]` | Any | | `build_recipient_hash` | Returns the `RECIPIENT` for a specified `SERIAL_NUM`, `SCRIPT_ROOT`, and inputs commitment.

**Inputs:** `[SERIAL_NUM, SCRIPT_ROOT, INPUT_COMMITMENT]`
**Outputs:** `[RECIPIENT]` | Any | | `build_recipient` | Builds the recipient hash from note inputs, script root, and serial number.

**Inputs:** `[inputs_ptr, num_inputs, SERIAL_NUM, SCRIPT_ROOT]`
**Outputs:** `[RECIPIENT]` | Any |