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

zcash_note_encryption generalization #746

Closed
wants to merge 36 commits into from

Conversation

PaulLaux
Copy link

@PaulLaux PaulLaux commented Dec 28, 2022

In order to support note encryption for zsa, we suggest extending the current zcash_note_encryption implementation. Currently, the COMPACT_NOTE_SIZE is a constant, however we need to support variable note sizes to include the AssetId field for zsa notes.

Currently, in zcash_note_encryption:

/// The size of a compact note.
pub const COMPACT_NOTE_SIZE: usize = 1 + // version
    11 + // diversifier
    8  + // value
    32; // rseed (or rcm prior to ZIP 212)
/// The size of [`NotePlaintextBytes`].
pub const NOTE_PLAINTEXT_SIZE: usize = COMPACT_NOTE_SIZE + 512;

and

pub const ENC_CIPHERTEXT_SIZE: usize = NOTE_PLAINTEXT_SIZE + AEAD_TAG_SIZE;

We suggest moving the constants into the specific implementation (impl Domain for OrchardDomain and Sapling) of the Domain trait by adding abstract types to NotePlaintextBytes, NoteCiphertextBytes, CompactNotePlaintextBytes, CompactNoteCiphertextBytes.

We get

pub trait Domain {
    type EphemeralSecretKey: ConstantTimeEq;
    type EphemeralPublicKey;
    type PreparedEphemeralPublicKey;
    type SharedSecret;
    type SymmetricKey: AsRef<[u8]>;
    type Note;
    type Recipient;
    type DiversifiedTransmissionKey;
    type IncomingViewingKey;
    type OutgoingViewingKey;
    type ValueCommitment;
    type ExtractedCommitment;
    type ExtractedCommitmentBytes: Eq + for<'a> From<&'a Self::ExtractedCommitment>;
    type Memo;

    // Types for variable note size handling:
    type NotePlaintextBytes: AsMut<[u8]> + for<'a> From<&'a [u8]>;
    type NoteCiphertextBytes: AsRef<[u8]> + for<'a> From<&'a [u8]>;
    type CompactNotePlaintextBytes: AsMut<[u8]> + for<'a> From<&'a [u8]>;
    type CompactNoteCiphertextBytes: AsRef<[u8]>;

Also, the constant will be removed from functions' signatures since they are unknown at compilation time. For example:

pub fn try_note_decryption<D: Domain, Output: ShieldedOutput<D, ENC_CIPHERTEXT_SIZE>>(

Will be replaced with simply

pub fn try_note_decryption<D: Domain, Output: ShieldedOutput<D>>(

We provided our initial implementation to be complemented by the appropriate changes in Orchard::note_encryption.rs. Currently can be seen here for v2 notes https://github.com/QED-it/orchard/pull/7/files#diff-73b5b84dde3fc78a9942d6c827ed46e4ab9e027ca508e6279b9262fe985c3bffL2 (see note_encryption.rs diff)

The changes will allow us to implement an Orchard::Domain for V3 notes while keeping compatibility with the existing Orchard Domain ( for V2 notes ) and Sapling.

@PaulLaux
Copy link
Author

@str4d @daira @nuttycom, we will appreciate some early feedback about this proposal.

PaulLaux and others added 3 commits January 31, 2023 14:43
@str4d str4d self-requested a review March 14, 2023 13:23
@nuttycom nuttycom added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 15, 2023
@str4d
Copy link
Contributor

str4d commented Jun 21, 2023

We discussed this PR in ZIP Sync today. Current thoughts:

  • We operate in a closed-world environment, so rather than runtime-checked plaintext and ciphertext lengths, we would prefer that the Domain trait have those lengths as constants, and we continue to use arrays like e.g. [u8; D::NOTE_PLAINTEXT_LEN].
  • The kind of note ciphertext is structurally bound to the version of the transaction, so we will want to encapsulate the runtime checks entirely inside Transaction (by replacing the Option<orchard::Bundle> with a three-state enum that can represent pre-v5 (none), v5, and v6 Orchard bundle kinds). Matching on that enum then enables us to ensure we use the decryption logic that structurally matches the length constants above.
  • The changes here are likely to conflict with the also-being-considered-for-NU decoupled memos ([ZIP 231] Memo Bundles (decouple memos from transaction outputs) zips#627). Those changes would result in the addition of another type for the decoupled memo ciphertexts; we should consider what an API would look like that can serve both those and ZSAs, and maybe aim to get there directly.
  • This will be much simpler to work on (incrementally!) and review after Extract "component" crates into a separate repository #768 is completed (and the cyclic dependency between this repository and zcash/orchard is broken). I'm going to prioritise this refactor for after we've finished the fast spendability work.

@str4d
Copy link
Contributor

str4d commented Nov 18, 2023

The zcash_note_encryption crate has now been extracted into its own repository: https://github.com/zcash/zcash_note_encryption

@PaulLaux
Copy link
Author

moved to zcash/zcash_note_encryption#2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants