Skip to content

Encryption generalization#18

Merged
PaulLaux merged 29 commits intozsa1from
encryption_generalization
Jan 31, 2023
Merged

Encryption generalization#18
PaulLaux merged 29 commits intozsa1from
encryption_generalization

Conversation

@PaulLaux
Copy link
Collaborator

@PaulLaux PaulLaux commented Dec 26, 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 during the 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

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 PaulLaux changed the base branch from main to zsa1 December 26, 2022 07:58
@QED-it QED-it deleted a comment from what-the-diff bot Jan 23, 2023
Copy link
Collaborator

@alexeykoren alexeykoren left a comment

Choose a reason for hiding this comment

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

Looks good to me, although I agree with old Vivec's comments about variables naming

Cargo.toml Outdated
schemer-rusqlite = { git = "https://github.com/aschampion/schemer.git", rev = "6726b60f43f72c6e24a18d31be0ec7d42829e5e1" }
group = { git = "https://github.com/zkcrypto/group.git", rev = "f61e3e420ed1220c8f1f80988f8c6c5e202d8715" }
orchard = { version = "0.3", git = "https://github.com/QED-it/orchard", rev = "5a50fb8d11361d3f7d1f3b2f6c0a468f88c0db49" }
orchard = { version = "0.3", git = "https://github.com/QED-it/orchard", rev = "c6e048a4474f0bef5182ed56e5d2ecff6fea0389" }

Choose a reason for hiding this comment

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

We should bump this up to the commit that is the latest eventually, (currently 49a7775eccdd74ebaaa30aabc42fccf063b25ec1), but this is not functionality-critical.

This also seems like a cycle with the dependency requirement in Orchard though...

Choose a reason for hiding this comment

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

Suggested change
orchard = { version = "0.3", git = "https://github.com/QED-it/orchard", rev = "c6e048a4474f0bef5182ed56e5d2ecff6fea0389" }
orchard = { version = "0.3", git = "https://github.com/QED-it/orchard", rev = "49a7775eccdd74ebaaa30aabc42fccf063b25ec1" }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, due to the cycle, it is not possible to provide the correct rev for both repos.

&[],
&mut output[..NOTE_PLAINTEXT_SIZE],
)
.encrypt_in_place_detached([0u8; 12][..].into(), &[], output.as_mut())

Choose a reason for hiding this comment

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

output already is &mut [u8], so output.as_mut() seems unnecessary.

Suggested change
.encrypt_in_place_detached([0u8; 12][..].into(), &[], output.as_mut())
.encrypt_in_place_detached([0u8; 12][..].into(), &[], output)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch.done.

let mut tnc = TransmittedNoteCiphertext {
epk_bytes: [0u8; 32],
enc_ciphertext: [0u8; 580],
enc_ciphertext: [0u8; 612],

Choose a reason for hiding this comment

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

This is V3 specific at the moment, would need updating along with backward compatibility

PaulLaux added a commit to QED-it/zcash-test-vectors that referenced this pull request Jan 31, 2023
- Updated to support encryption_v3 as given in QED-it/orchard#38 and QED-it/librustzcash#18.

- This PR breaks compatibility with OrchardDomainV2
@PaulLaux PaulLaux merged commit 70020cf into zsa1 Jan 31, 2023
PaulLaux added a commit to QED-it/orchard that referenced this pull request Jan 31, 2023
Added `OrchardDomainV3` on top of the encryption generalization (QED-it/librustzcash#18).

not for review: note_encryption.rs, note_encryptionv2v3.rs and src/test_vectors/note_encryption.rs. These files represent two possible approaches for backward compatibility and will be finalized down the road. (the files were excluded from the build).
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.

3 participants