-
Notifications
You must be signed in to change notification settings - Fork 38
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
Remove 0xf6 empty memo convention. #441
Conversation
752986f
to
867c33d
Compare
@@ -342,7 +338,7 @@ impl OutputInfo { | |||
let fvk: FullViewingKey = (&SpendingKey::random(rng)).into(); | |||
let recipient = fvk.address_at(0u32, Scope::External); | |||
|
|||
Self::new(None, recipient, NoteValue::zero(), None) | |||
Self::new(None, recipient, NoteValue::zero(), [0u8; 512]) |
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.
This technically changes the behaviour of this function. Since the FVK/IVK that could read the note plaintext is not available to any other party, it doesn't leak that a version of librustzcash past this PR is being used by the sender. Still a bit weird for something that is supposed to be a pure refactoring, though.
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.
It's not a pure refactoring, strictly speaking; it is a behavior change. The alternative here would be to just inject random data? But since this then ends up encrypted to a random recipient, I don't think it matters.
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.
The protocol spec says that for dummies, we encrypt a real dummy note to a random viewing key. So this is technically an observable change. But in practice that viewing key or its address is never persisted (particularly as we use ovk = None
), and thus the information change here is private even to quantum adversaries.
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.
utACK with comment.
867c33d
to
a40136e
Compare
utACK; just a rebase. |
This is a standard convention for Zcash memos, but it doesn't have any relevance to the general Orchard protocol and as such doesn't belong in this crate.
a40136e
to
4500bdf
Compare
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.
utACK 4500bdf
This is a standard convention for Zcash memos, but it doesn't have any relevance to the general Orchard protocol and as such doesn't belong in this crate.