feat: return note_id alongside nullifier for consumed public notes in SyncTransactions#2304
Conversation
JereSalo
force-pushed
the
jere-consumed-note-refs-next
branch
from
1e5cdcb to
c232e3e
Compare
… SyncTransactions Add a consumed_note_refs side-channel to each SyncTransactions transaction record, mapping a consumed input note's nullifier to its note id for public notes the node can resolve. Lets a follower that watches an account without registering the note's tag recover the consumed note via GetNotesById. The transaction header and its input commitments are left untouched; private notes have no stored nullifier and are absent.
JereSalo
force-pushed
the
jere-consumed-note-refs-next
branch
from
c232e3e to
214cbda
Compare
Mirko-von-Leipzig
left a comment
Mirko-von-Leipzig
left a comment
There was a problem hiding this comment.
I wonder if we wouldn't be better served by adding this information to the notes table i.e. consumed by tx.id.
But I suppose we can always do that later.
| // Maps a consumed input note's nullifier to its note ID, for the public notes the node can | ||
| // resolve. Authenticated inputs in `header.input_notes` carry only a nullifier, so this lets a | ||
| // follower recover the consumed note via `GetNotesById`. Private notes are absent. | ||
| repeated ConsumedNoteRef consumed_note_refs = 4; |
There was a problem hiding this comment.
I think we don't need the stuttering
| repeated ConsumedNoteRef consumed_note_refs = 4; | |
| repeated ConsumedNoteRef consumed_notes = 4; |
| } | ||
|
|
||
| // Maps a consumed input note's nullifier to its note ID. | ||
| message ConsumedNoteRef { |
There was a problem hiding this comment.
I don't like the name since it carries what we're using it for, instead of describing what it is.
I don't really have good ideas though.
NoteBinding
NoteReference
Ideally we'd also leave it vague enough that we can add fields to it later?
| pub(crate) fn select_note_ids_by_nullifier( | ||
| conn: &mut SqliteConnection, | ||
| nullifiers: &[Nullifier], | ||
| ) -> Result<BTreeMap<Nullifier, NoteId>, DatabaseError> { |
There was a problem hiding this comment.
Could perhaps be a HashMap? Or do we need ordering?
There was a problem hiding this comment.
We don't need ordering but in this case I went for a BTreeMap because of consistency with the rest of the usages in the file. It can be changed, though there doesn't seem to be a benefit at these low sizes. It makes sense if we want to communicate that we don't need ordering, and use BTreeMap only in the places where it's necessary.
Mirko-von-Leipzig
left a comment
Mirko-von-Leipzig
left a comment
There was a problem hiding this comment.
Approving; we can figure naming etc out for next major release since we have to figure out the wire format as well.
|
An issue has been opened to address follow up comments: #2311 |
2 participants
Summary
Enables a client that tracks a public account to recover the notes it consumed.
SyncTransactionsreturns authenticated consumed inputs as a bare nullifier, which a follower cannot resolve to a note. This adds aconsumed_note_refsside-channel mapping each resolvable public note's nullifier to its note ID, so the client can fetch the note viaGetNotesById.Closes #2296
Changelog