feat(rust-client): read erased notes via InputNoteReader#2293
Open
JereSalo wants to merge 10 commits into
Open
feat(rust-client): read erased notes via InputNoteReader#2293JereSalo wants to merge 10 commits into
JereSalo wants to merge 10 commits into
Conversation
…nputNoteReader Preserve the note header on unauthenticated input commitments so a follower that imports an account can surface notes it only learns about from the consumer's transactions, including same-batch-erased notes that never reach a block. Add the ConsumedExternalErased state for header-only records, and retain note metadata in ConsumedExternal so externally-consumed committed notes keep a recoverable note id. The Consumed note filter and InputNoteReader return both kinds in consumption order.
JereSalo
added
no changelog
The public `from_header` docs linked to the private `placeholder_details` function, which rustdoc rejects under `-D warnings` (the doc build). Reference it as inline code instead.
JereSalo
changed the title
…rde guard Address review feedback on the erased-note recovery: - Drop the backward-compat guard in `ConsumedExternalNoteState::read_from` and read the metadata unconditionally, matching the sibling fields and the established pattern (the store has no per-version forward migration and no other state guards its serde this way). - Clarify comments and docs: use "erased notes" instead of "same-batch-erased", stop naming `InputNoteReader` in general sync paths, define the erased concept once in the `ConsumedExternalErased` state doc, and describe the state as holding any header-only unauthenticated input (typically an erased note) rather than only erased notes. - Display the state as "Consumed (header only, ...)" instead of "Consumed erased".
JereSalo
removed
the
no changelog
JereSalo
marked this pull request as ready for review
June 30, 2026 19:11
TomasArrachea
self-requested a review
July 3, 2026 19:04
…otes # Conflicts: # CHANGELOG.md # crates/rust-client/src/note/note_update_tracker.rs # crates/rust-client/src/store/note_record/input_note_record/mod.rs # crates/rust-client/src/store/note_record/input_note_record/states/consumed_external.rs # crates/sqlite-store/src/note/mod.rs # crates/testing/miden-client-tests/src/tests.rs
Header-only ConsumedExternalErased records have placeholder details, so their nullifier cannot be recomputed. Store the nullifier carried by the consuming transaction's InputNoteCommitment instead.
…-details note When a note imported from bare details (metadata-less, tracked only by its details commitment) is later recorded as a consumed unauthenticated input, the guard did not recognize it and inserted a second, header-only placeholder record under a different (placeholder) details commitment. That left a duplicate row alongside the stale expected one. Extend the guard to also skip when a tracked metadata-less expected note matches the committed note. This prevents the duplicate and matches the behavior of a client without erased-note recovery. Evolving the expected record straight to a consumed state is left as a follow-up.
| consumed_tx_order: None, | ||
| }; | ||
| InputNoteRecord { | ||
| details: placeholder_details(header.id()), |
Collaborator
There was a problem hiding this comment.
Have you considered making the details field optional in the InputNoteRecord? I think it would be cleaner to avoid the placeholder data, though might involve a lot of churn on existing code.
Collaborator
Author
There was a problem hiding this comment.
Yes. I've thought about it and perhaps it would be the proper way of doing things, but at the same time it would impact heavily on the rest of the codebase, making the scope of this PR way bigger.
I made minimal changes to the code so that we don't have problems caused by the placeholder data. But in the future it'd be useful to make a bigger refactor. c4b9ec7
Comment on lines
+648
to
+653
| Self::mark_erased_notes_as_consumed(state_sync_update, transaction); | ||
|
|
||
| // Record notes consumed by this tracked account that the client only learns about from | ||
| // the consuming transaction's unauthenticated input commitments (typically erased notes | ||
| // it never created or discovered). | ||
| Self::record_consumed_unauthenticated_notes(state_sync_update, transaction); |
Collaborator
There was a problem hiding this comment.
I wonder if the naming on this two methods could be more consistent. Roughly, mark_erased_notes_as_consumed checks if any tracked note has been erased to mark it as consumed; whereas record_consumed_unauthenticated_notes creates a new record if the erased input note was not tracked.
So record_consumed_unauthenticated_notes could be named something like record_input_erased_notes.
Or perhaps we could even simplify this by merging both. Then the note update tracked would hold the logic to check if the note is already tracked to either create a new record or update the existing one.
Collaborator
Author
There was a problem hiding this comment.
Yeah the rename makes a lot of sense. I went with mark_erased_output_notes_as_consumed and record_consumed_unauthenticated_input_notes, avoiding 'erased' for the inputs since the term unauthenticated is more broad.
I'm not sure if we should merge both though, I think that we should keep them separate as they do different things.
…stop exposing placeholder data Header-only records (state ConsumedExternalErased) previously derived their identity from placeholder details and let those placeholder values escape through conversions and the CLI. The state now stores the details commitment taken from the note header, so details_commitment(), id() and commitment() report authoritative values and the store row is keyed by the real commitment. Conversions to Note and InputNote fail for header-only records instead of fabricating a note, and From<InputNoteRecord> for NoteDetails became TryFrom. The CLI gates details-derived fields behind has_details(), falling back to a matching output record when present and showing them as unavailable otherwise. Replacing the placeholder entirely with an explicit payload model (optional details and nullable store columns) is left for a follow-up.
…otes # Conflicts: # bin/integration-tests/src/tests/client.rs # bin/integration-tests/src/tests/onchain.rs # crates/sqlite-store/src/note/mod.rs # crates/testing/miden-client-tests/src/tests.rs
Rename mark_erased_notes_as_consumed to mark_erased_output_notes_as_consumed and record_consumed_unauthenticated_notes to record_consumed_unauthenticated_input_notes. The two walk different data from the transaction record (erased output notes vs header-bearing input commitments), so the names now make that axis explicit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A client tracking a public account can read, via
InputNoteReader, the notes that account consumed. This PR adds a header-only note state,ConsumedExternalErased, so that notes created and consumed within the same batch (erased notes, which never land in a block) are also surfaced, carrying the maximum information available for them (the note header that appears on the consuming transaction). The PR also retains metadata on externally-consumed notes (ConsumedExternal) so their id stays recoverable after consumption.For an erased note the header is all that exists anywhere, since the note never reaches a block and the node keeps no body for it, so currently this is the most we can get from it. A natural follow-up to this PR (that would be useful) is to recover what actually happened inside these notes through transaction logs/events (protocol discussion, Meta: Events #2034). Currently the node does not store the body or execution events of erased notes, so by having transaction logs we could know for example what event an erased note emitted without necessarily having the full note.
The tests that added basically emit full notes and erased notes and it verifies that the input note reader can read them in the right order, despite erased notes having only the header.
Related: #1971
Closes #2116