Conversation
PaulLaux
left a comment
PaulLaux
left a comment
There was a problem hiding this comment.
added minor comments and a question
|
|
||
| fn enc_ciphertext_compact(&self) -> D::CompactNoteCiphertextBytes { | ||
| self.encrypted_note().enc_ciphertext.as_ref()[..D::COMPACT_NOTE_SIZE].into() | ||
| // FIXME: can use unwrap here? |
| // FIXME: shouldn't it return None like enc_ciphertext does? | ||
| fn enc_ciphertext_compact(&self) -> D::CompactNoteCiphertextBytes { | ||
| self.enc_ciphertext | ||
| // FIXME: can use unwrap here? |
| None | ||
| } | ||
|
|
||
| // FIXME: shouldn't it return None like enc_ciphertext does? |
There was a problem hiding this comment.
no, enc_ciphertext should not be used for Orchard. It was used only by sapling.
src/note_encryption/domain.rs
Outdated
| fn parse_note_plaintext_bytes(plaintext: &[u8]) -> Option<Self::NotePlaintextBytes> { | ||
| Self::NotePlaintextBytes::from_slice(plaintext) | ||
| } | ||
|
|
||
| fn parse_note_ciphertext_bytes( | ||
| output: &[u8], | ||
| tag: [u8; AEAD_TAG_SIZE], | ||
| ) -> Option<Self::NoteCiphertextBytes> { | ||
| Self::NoteCiphertextBytes::from_slice_with_tag(output, tag) | ||
| } | ||
|
|
||
| fn parse_compact_note_plaintext_bytes( | ||
| plaintext: &[u8], | ||
| ) -> Option<Self::CompactNotePlaintextBytes> { | ||
| Self::CompactNotePlaintextBytes::from_slice(plaintext) | ||
| } |
There was a problem hiding this comment.
What is the point in adding this if not being used?
There was a problem hiding this comment.
The idea of introducing these functions came from str4d's review comment. Although these functions are not used in the orchard or sapling-crypto crates directly, they are used in the zcash_note_encryption crate: zcash_note_encryption delegates the implementation of these functions to upper-level crates, and then use them.
Therefore, I added them to the Domain trait as methods and then implemented them in both orchard and sapling-crypto crates.
However, as it turns out, the implementation is the same for both crates and relies solely on the NoteBytes trait methods. Given this, we can simplify the code by moving these function implementations directly to the Domain trait in the zcash_note_encryption crate.
I have tested this change, and it works correctly. I can proceed with moving the implementation (here and in sapling-crypto) if everyone agrees.
… to reduce changes in PR
2 participants
Orchard has been synced with the changes from PR #10 in the
zcash_note_encryptionrepository. This update includes the following changes:parse_note_plaintext_bytes,parse_note_ciphertext_bytes, andparse_compact_note_plaintext_bytesmethods of theDomaintrait fromzcash_note_encryption.NoteBytestrait andNoteBytesDatastructure fromzcash_note_encryptioninstead of having local definitions and implementations.Note
This PR uses the
resolve_zcash_pr2_issuesbranch ofzcash_note_encryptioninCargo.toml. Before merging this PR, PR #10 needs to be merged into thezsa1branch ofzcash_note_encryption. Then, this Orchard PR branch should be updated to use thezsa1branch ofzcash_note_encryptionbefor merging this PR.