-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Return ref from enc ciphertext #3
Conversation
…iphertext These changes were discussed and suggested in PR zcash_note_encryption#2
…constant definitions from zcash_note_encryption to here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comments to simplify
src/bundle.rs
Outdated
@@ -342,7 +343,7 @@ impl<Proof> OutputDescription<Proof> { | |||
} | |||
|
|||
/// Returns the encrypted note ciphertext. | |||
pub fn enc_ciphertext(&self) -> &[u8; ENC_CIPHERTEXT_SIZE] { | |||
pub fn enc_ciphertext(&self) -> &<SaplingDomain as Domain>::NoteCiphertextBytes { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's stick to simply
/// Returns the encrypted note ciphertext.
pub fn enc_ciphertext(&self) -> &NoteBytesData<{ ENC_CIPHERTEXT_SIZE }> {
&self.enc_ciphertext
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
src/bundle.rs
Outdated
@@ -414,8 +415,8 @@ impl<A> ShieldedOutput<SaplingDomain> for OutputDescription<A> { | |||
self.cmu.to_bytes() | |||
} | |||
|
|||
fn enc_ciphertext(&self) -> Option<<SaplingDomain as Domain>::NoteCiphertextBytes> { | |||
Some(NoteBytesData(self.enc_ciphertext)) | |||
fn enc_ciphertext(&self) -> Option<&<SaplingDomain as Domain>::NoteCiphertextBytes> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fn enc_ciphertext(&self) -> Option<&NoteBytesData<{ ENC_CIPHERTEXT_SIZE }>> {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same for row 442:
fn enc_ciphertext_compact(&self) -> NoteBytesData<{ COMPACT_NOTE_SIZE }> {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
src/note_encryption.rs
Outdated
@@ -351,7 +362,7 @@ impl ShieldedOutput<SaplingDomain> for CompactOutputDescription { | |||
self.cmu.to_bytes() | |||
} | |||
|
|||
fn enc_ciphertext(&self) -> Option<<SaplingDomain as Domain>::NoteCiphertextBytes> { | |||
fn enc_ciphertext(&self) -> Option<&<SaplingDomain as Domain>::NoteCiphertextBytes> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure we have the librustzcash
required changes as well
This PR updates the
ShieldedOutput
implementation andOutputDescription
struct to align with the recent changes in thezcash_note_encryption
crate. Specifically, theenc_ciphertext
method now returns a reference instead of a copy. Also, it changedenc_ciphertext
storage inOutputDescription
toNoteBytesData
.This change was discussed and suggested in PR zcash/zcash_note_encryption#2 review.