Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nuttycom
Copy link
Contributor

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.

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.
@@ -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])
Copy link
Contributor

@daira daira Dec 12, 2024

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.

Copy link
Contributor Author

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.

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK with comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants