feat: implement setting multiple note attachments#2795
Open
PhilippGackstatter wants to merge 24 commits intonextfrom
Open
feat: implement setting multiple note attachments#2795PhilippGackstatter wants to merge 24 commits intonextfrom
PhilippGackstatter wants to merge 24 commits intonextfrom
Conversation
Also optimizes metadata header serialization.
5625e17 to
e1fdeb5
Compare
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
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.
Overview
This branch extends the note attachment model from supporting a single attachment per note to supporting up to 4 attachments per note. Previously, a note could carry at most one attachment (None, Word, or Array); now notes have an ordered collection of attachments that are appended incrementally.
Key Conceptual Changes
Single attachment → Collection of attachments
NoteMetadatano longer owns an attachment. It is now a pure user-facing type containing only sender, note type, and tag.NoteAttachmentscollection type holds 0–4NoteAttachmentvalues per note.Notestruct gains anattachments: NoteAttachmentsfield.Attachment commitment is now over individual attachment commitments
NoteAttachmentscommitment is computed over the individual attachment commitment words (i.e.,hash(attachment_0_commitment || ... || attachment_N_commitment)), rather than directly over raw attachment data.New
NoteMetadataHeaderas the protocol-level encodingNoteMetadataHeaderwrapsNoteMetadatatogether with attachment headers and an attachments commitment.num_wordsvalues (8 bits each) alongside the note tag (32 bits).hash(NOTE_ID || METADATA_HEADER_COMMITMENT)where the header commitment includes attachments.TryFrom<Word> for NoteMetadataHeaderbecause the code was unused and it no longer works: We cannot recover the attachments commitment from a single word.NoteAttachmentHeaderreplacesNoteAttachmentKindNoteAttachmentKindenum (None/Word/Array) is removed.NoteAttachmentHeaderstruct carriesscheme: NoteAttachmentSchemeandnum_words: u8, which together describe the attachment's type and size.NoteAttachmentScheme::noneis kept at 0 since we can check presence of an attachment withword_size != 0.num_words = 1indicates a Word attachment;num_words > 1indicates an Array attachment. There is no longer a "None" variant - absence of an attachment is an empty list of attachments.NoteAttachmentSchemenarrowed tou16u32; it is now au16(max 65534) to fit four schemes into a single felt in the metadata encoding.NoteAttachmentArraynow storesVec<Word>instead ofVec<Felt>Vec<Word>rather than a flat vector of field elements.NoteAttachment::MAX_NUM_WORDS(254) constrains the size of any single attachment.NoteAttachments::MAX_NUM_WORDS(512) constrains the size of all attachments of a note - at most 16 KiB total like before.Word vs Array
NoteAttachmentContent::Word, otherwiseArray. Other than the size difference, these two are the same.Wordvariant is a bit nicer - no length checking.NoteAttachmentContentstructure.Kernel procedure:
set_attachment→add_attachmentoutput_note_set_attachmentis replaced byoutput_note_add_attachment, which appends an attachment to the next available slot and increments a counter.Memory layout changes
num_attachmentscounter. It is technically redundant since the number of attachmens can be derived from the metadata. This is for ease of access in the kernel.ATTACHMENTS_COMMITMENT(renamed fromATTACHMENT) representing the commitment over all attachments. All individual attachment commitments or their raw data is expected to be accessed through the advice provider.TransactionHeader does not contain attachment data
TransactionHeaderpreviously contained the full attachment data as part of theNoteMetadatawithinNoteHeader, and this is no longer the case. But this now seems correct, because attachments are always publicly available in blocks because aBlockBodycontains all fullOutputNoteobjects and:PublicOutputeNote(Note), where theNoteholds the full attachments data,PrivateOutputNote(previouslyPrivateNoteHeader), which also hols the full attachments data in addition to the note header.Alternatives: Hash Single Word Attachments
As discussed in #2555 (comment), there are some alternatives.
Choices:
I ended up implementing the second option, "hash all attachments". The main benefit is that attachments appear more uniformly (``), an attachment is conceptually simply the hash of some number of words. The main downside is that accessing a word-sized attachment requires hashing.
Follow-up
In subsequent PRs:
input_note_get_metadataand possibly a newinput_note_get_attachmentsprocedure.metadata_into_tagfor note tag extraction which now becomes necessary.Note.