refactor: input note authentication deduced by client #1624

juan518munoz · 2025-12-18T21:10:58Z

Replace logic for adding authenticated input notes in a Transaction. User no longer provides the NoteIDs of the authenticated note he wants to use, but rather send the Note directly (being authenticated or not), and let the Client decide if the note passed is verified.

Note: most test cases have a new step when building transaction requests with authenticated notes. Instead of submitting the transaction with the noteID, they retrieve the associated note with it from the Client, and submit the tx with that info.

Closes #1112

juan518munoz · 2025-12-19T14:52:39Z

crates/testing/miden-client-tests/src/tests.rs

-    // Check that adding an authenticated note that is not tracked by the client will return an
-    // error
-    let missing_authenticated_note_tx_request = TransactionRequestBuilder::new()
-        .build_consume_notes(vec![NoteId::from_raw(EMPTY_WORD)])
-        .unwrap();
-    let error =
-        Box::pin(client.submit_new_transaction(wallet.id(), missing_authenticated_note_tx_request))
-            .await
-            .unwrap_err();
-
-    assert!(matches!(
-        error,
-        ClientError::TransactionRequestError(
-            TransactionRequestError::MissingAuthenticatedInputNote(_)
-        )
-    ));


No longer errors if the Note passed is not authenticated, It will just not get marked as such (by the Client).

SantiagoPittella · 2025-12-19T20:54:19Z

crates/rust-client/src/transaction/mod.rs

+        authenticated_note_records.retain(|note| {
+            matches!(
+                note.state(),
+                InputNoteState::Committed(_)
+                    | InputNoteState::ConsumedAuthenticatedLocal(_)
+                    | InputNoteState::ConsumedExternal(_)
+            )
+        });
+
+        let authenticated_note_ids =
+            authenticated_note_records.iter().map(InputNoteRecord::id).collect::<Vec<_>>();


this two steps can probably be done altogether

I keep them apart as both authenticated_note_records and authenticated_note_ids are used separately afterwards.

SantiagoPittella · 2025-12-19T20:55:34Z

crates/rust-client/src/transaction/mod.rs

+            .filter_map(|note| {
+                if transaction_request.input_notes().iter().any(|n| n.id() == note.id()) {


how are we excluding unauthenticated ones here?

we weren't, good catch, addressed in a9e5113

igamigo

Overall looks good, but I think we should tweak some things before merging. Specifically, unless I'm missing something, it looks like we are adding all notes as InputNote::Unauthenticated and we should probably make the distinction before passing the notes to the VM before executing. I left a comment about this, and some other nits as well.

One thing that would be pending for this PR is the last bullet point in the issue, but maybe that's something to tackle in a different issue/PR

igamigo · 2025-12-26T19:00:25Z

bin/integration-tests/src/tests/swap_transaction.rs

+    let note = client2
+        .get_input_note(expected_output_notes[0].id())
+        .await?
+        .unwrap()
+        .try_into()?;
+    let tx_request = TransactionRequestBuilder::new().build_consume_notes(vec![note])?;


As mentioned in #1112 (comment), this does make this scenario a bit more inconvenient. I guess we could add some other way of expressing notes via note IDs but maybe this defeats the original purpose. Nothing to do for now but just making a note. Would be nice to know @mmagician's opinion on this.

One possibility I can think of is wrapping the input notes the TransactionRequestBuilder receives in some sort of enum:

enum InputNote { /// InputNote to be used is assumed to be present /// on the store. Stored(NoteId) /// InputNote provided fully. May still be present /// on the store. Provided(Note) } #[derive(Clone, Debug)] pub struct TransactionRequestBuilder { /// Notes to be consumed by the transaction. input_notes: Vec<InputNote>, ... }

This way we can still take just NoteIds from the user, and fail in the case where the requested note is not present in our store.

What do you think?

Following on the proposed enum, if we keep the current procedure of passing all full Notes regardless of if they are authenticated or not, we have a possible security flaw:

NoteA with noteId 123 is on the client store as an authenticated note.

Alice passes NoteB with noteId 123 as an input note.

Client assumes that the note is authenticated, as the id is already present on the store.

The current implementation does not have this vulnerability, as we are using the notes retrieved from the store:

// Retrieve all input notes from the store. let mut authenticated_note_records = self .store .get_input_notes(NoteFilter::List(transaction_request.get_input_note_ids())) .await?; // Verify that none of the authenticated input notes are already consumed. for note in authenticated_note_records.iter() { if note.is_consumed() { return Err(ClientError::TransactionRequestError(TransactionRequestError::InputNoteAlreadyConsumed(note.id()))); } } // Only keep authenticated input notes from the store. authenticated_note_records.retain(InputNoteRecord::is_authenticated);

But a future "optimization" might use the provided notes instead.

crates/rust-client/src/transaction/request/builder.rs

igamigo · 2025-12-26T19:06:13Z

crates/rust-client/src/transaction/request/builder.rs

+    /// Optional arguments of the Notes to be consumed by the transaction. This
    /// includes both authenticated and unauthenticated notes.
-    input_notes: Vec<(NoteId, Option<NoteArgs>)>,
+    input_notes_args: Vec<(NoteId, Option<NoteArgs>)>,


Related to the comment above, should this just be input_notes_args: Vec<(Note, Option<NoteArgs>)>? Instead of having the extra level of indirection.

extra level of indirection

Thought about that, I believe the best approach is to merge input_notes and input_notes_args into a single field (input_notes). Should we go with it?

crates/rust-client/src/transaction/request/builder.rs

igamigo · 2025-12-26T19:09:45Z

crates/rust-client/src/transaction/request/mod.rs

+    /// Notes to be consumed by the transaction.
    /// includes both authenticated and unauthenticated notes.


We should clarify here (and in TransactionRequestBuilder) this new behavior about the client consuming a note as unauthenticated when it's not present in the store and as authenticated when it is present.

Addressed in 8db0e04

igamigo · 2025-12-26T19:13:25Z

crates/rust-client/src/transaction/mod.rs

-
-        let authenticated_note_records = self
+        // Retrieve all input notes from the store.
+        // But only mark as authenticated if they are committed or consumed.


We should ideally error if we know a note has been consumed before executing (and in general, if in any case we know a transaction will not be accepted by the network, we should error as early as possible)

Addressed in 8db0e04

CHANGELOG.md

igamigo · 2025-12-26T21:34:01Z

crates/rust-client/src/transaction/request/mod.rs

    /// specified authenticated notes need to be provided, otherwise an error will be returned.
    /// The transaction input notes will include both authenticated and unauthenticated notes in the
    /// order they were provided in the transaction request.
    pub(crate) fn build_input_notes(


We should update this function's doc comments

BTW, I think this can be a private function

BTW, I think this can be a private function

The function is called from impl Client<AUTH> if I'm not mistaken, so it needs to be at least pub(crate).

We should update this function's doc comments

I don't see which part of the docs needs changing, it still only takes authenticated notes as InputNoteRecords.

igamigo · 2025-12-26T21:38:35Z

crates/rust-client/src/transaction/mod.rs

+        let mut authenticated_note_records = self
            .store
-            .get_input_notes(NoteFilter::List(authenticated_input_note_ids))
+            .get_input_notes(NoteFilter::List(transaction_request.get_input_note_ids()))


There's no need to retrieve the full NoteRecord objects here, right? Let's add a TODO (or if you prefer, an issue) to see whether we should add a specific store method to return authenticated note IDs from a set of notes.

As you point out, it's quite redundant to retrieve all notes, as we already pass them as arguments. On the other hand, we need to check that the notes are not consumed and also filter as authenticated, AFAIK this can only be done through the InputNoteRecord struct.

Am I missing something?

igamigo · 2025-12-26T21:44:08Z

crates/rust-client/src/transaction/request/mod.rs

        // Add unauthenticated input notes to the input notes map.
-        for unauthenticated_input_notes in &self.unauthenticated_input_notes {
+        for unauthenticated_input_notes in &self.input_notes {
            input_notes.insert(
                unauthenticated_input_notes.id(),
                InputNote::Unauthenticated {
                    note: unauthenticated_input_notes.clone(),
                },
            );
        }


Does this not add all authenticated notes as well? If so, we should make it so that authenticated notes are consumed exclusively as that instead of added as InputNote::Unauthenticated.

Good catch, addressed in 8db0e04

refactor: transaction builder only takes one type of note

8ad3365

juan518munoz changed the title ~~refactor: input not authentication deduced by client~~ refactor: input note authentication deduced by client Dec 19, 2025

docs: run typedoc / update markdown files

1925d41

juan518munoz marked this pull request as ready for review December 19, 2025 14:27

chore: address pending TODOs

ae4110d

juan518munoz commented Dec 19, 2025

View reviewed changes

Merge branch 'next' into jmunoz-simplify-transaction-builder-note-inputs

3e3feca

SantiagoPittella reviewed Dec 19, 2025

View reviewed changes

chore: address PR comment

a9e5113

SantiagoPittella approved these changes Dec 23, 2025

View reviewed changes

Merge branch 'next' into jmunoz-simplify-transaction-builder-note-inputs

fb7b469

SantiagoPittella requested review from fkrause98 and igamigo December 23, 2025 18:51

igamigo requested changes Dec 26, 2025

View reviewed changes

juan518munoz and others added 4 commits December 27, 2025 09:36

chore: rename variable

2b9bc78

chore: address PR comments

8db0e04

Merge branch 'next' into jmunoz-simplify-transaction-builder-note-inputs

4f4a2f5

Merge branch 'next' into jmunoz-simplify-transaction-builder-note-inputs

c655226

		.filter_map(\|note\| {
		if transaction_request.input_notes().iter().any(\|n\| n.id() == note.id()) {

		/// Notes to be consumed by the transaction.
		/// includes both authenticated and unauthenticated notes.

refactor: input note authentication deduced by client #1624

Are you sure you want to change the base?

refactor: input note authentication deduced by client #1624

Uh oh!

Conversation

juan518munoz commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juan518munoz Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igamigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juan518munoz Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juan518munoz Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juan518munoz Dec 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

juan518munoz commented Dec 18, 2025 •

edited

Loading

juan518munoz Dec 19, 2025 •

edited

Loading

juan518munoz Dec 27, 2025 •

edited

Loading

juan518munoz Dec 27, 2025 •

edited

Loading

juan518munoz Dec 27, 2025 •

edited

Loading