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

Refactor enc_ciphertext to return reference instead of copy #13

Merged
merged 8 commits into from
Aug 12, 2024
Merged
18 changes: 16 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,22 @@ and this library adheres to Rust's notion of
abstract types `NotePlaintextBytes`, `NoteCiphertextBytes`,
`CompactNotePlaintextBytes`, and `CompactNoteCiphertextBytes` to the `Domain`
trait.
- Moved the specific constants into the `Domain` trait implementations.

- Removed the separate `NotePlaintextBytes` type definition (as it is now an
associated type).
- Added new `parse_note_plaintext_bytes`, `parse_note_ciphertext_bytes`, and
`parse_compact_note_plaintext_bytes` methods to the `Domain` trait.
- Updated the `note_plaintext_bytes` method of the `Domain` trait to return the
`NotePlaintextBytes` associated type.
- Updated the `encrypt_note_plaintext` method of `NoteEncryption` to return the
`NoteCiphertextBytes` associated type of the `Domain` instead of the explicit
array.
- Updated the `enc_ciphertext` method of the `ShieldedOutput` trait to return an
`Option` of a reference instead of a copy.
- Moved the specific constants into the `Domain` trait implementations, while
keeping the original constants for backward compatibility.

Choose a reason for hiding this comment

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

I don't think we should keep the original constants. this might create confusion.

Copy link
Author

Choose a reason for hiding this comment

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

Done (removed the constants and updated CHANGELOG accordingly).

- Added new `note_bytes` module with helper trait and struct to deal with note
bytes data with abstracted underlying array size.

## [0.4.0] - 2023-06-06
### Changed
- The `esk` and `ephemeral_key` arguments have been removed from
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[toolchain]
channel = "1.56.1"
components = [ "clippy", "rustfmt" ]
components = ["clippy", "rustfmt"]
24 changes: 9 additions & 15 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,22 @@ pub mod note_bytes;

use note_bytes::NoteBytes;

/// The size of a compact note for Sapling and Orchard Vanilla.
/// The size of a compact note for Sapling and pre-ZSA Orchard.

Choose a reason for hiding this comment

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

we don't want new term pre-ZSA Orchard.
... for Sapling and Orchard is fine (x2)

Copy link
Author

Choose a reason for hiding this comment

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

Done (removed unused constants COMPACT_NOTE_SIZE, NOTE_PLAINTEXT_SIZE, ENC_CIPHERTEXT_SIZE with those comments).

pub const COMPACT_NOTE_SIZE: usize = 1 + // version
11 + // diversifier
8 + // value
32; // rseed (or rcm prior to ZIP 212)
/// The size of `NotePlaintextBytes` for Sapling and Orchard Vanilla.
/// The size of [`Domain::NotePlaintextBytes`] for Sapling and pre-ZSA Orchard.
pub const NOTE_PLAINTEXT_SIZE: usize = COMPACT_NOTE_SIZE + 512;

/// The size of the memo.
pub const MEMO_SIZE: usize = 512;
/// The size of the authentication tag used for note encryption.
pub const AEAD_TAG_SIZE: usize = 16;

/// The size of [`OutPlaintextBytes`].
pub const OUT_PLAINTEXT_SIZE: usize = 32 + // pk_d
32; // esk
const AEAD_TAG_SIZE: usize = 16;
/// The size of an encrypted note plaintext for Sapling and pre-ZSA Orchard.
pub const ENC_CIPHERTEXT_SIZE: usize = NOTE_PLAINTEXT_SIZE + AEAD_TAG_SIZE;

Choose a reason for hiding this comment

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

remove COMPACT_NOTE_SIZE, NOTE_PLAINTEXT_SIZE, ENC_CIPHERTEXT_SIZE from this file.
These are domain specific values and should be part of the implementation.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

/// The size of an encrypted outgoing plaintext.
pub const OUT_CIPHERTEXT_SIZE: usize = OUT_PLAINTEXT_SIZE + AEAD_TAG_SIZE;

/// The size of an encrypted note plaintext for Sapling and Orchard Vanilla.
pub const ENC_CIPHERTEXT_SIZE: usize = NOTE_PLAINTEXT_SIZE + AEAD_TAG_SIZE;

/// A symmetric key that can be used to recover a single Sapling or Orchard output.
pub struct OutgoingCipherKey(pub [u8; 32]);

Expand Down Expand Up @@ -270,7 +264,8 @@ pub trait Domain {
plaintext: &Self::CompactNotePlaintextBytes,
) -> Option<(Self::Note, Self::Recipient)>;

/// Splits the memo field from the given note plaintext.
/// Splits the given note plaintext into the compact part (containing the note) and
/// the memo field.
///
/// # Compatibility
///
Expand Down Expand Up @@ -374,7 +369,7 @@ pub trait ShieldedOutput<D: Domain> {
fn cmstar_bytes(&self) -> D::ExtractedCommitmentBytes;

/// Exposes the note ciphertext of the output. Returns `None` if the output is compact.
fn enc_ciphertext(&self) -> Option<D::NoteCiphertextBytes>;
fn enc_ciphertext(&self) -> Option<&D::NoteCiphertextBytes>;

// FIXME: Should we return `Option<D::CompactNoteCiphertextBytes>` or
// `&D::CompactNoteCiphertextBytes` instead? (complexity)?
Expand All @@ -383,8 +378,7 @@ pub trait ShieldedOutput<D: Domain> {

//// Splits the AEAD tag from the ciphertext.
fn split_ciphertext_at_tag(&self) -> Option<(D::NotePlaintextBytes, [u8; AEAD_TAG_SIZE])> {
let enc_ciphertext = self.enc_ciphertext()?;
let enc_ciphertext_bytes = enc_ciphertext.as_ref();
let enc_ciphertext_bytes = self.enc_ciphertext()?.as_ref();

let (plaintext, tail) = enc_ciphertext_bytes
.len()
Expand Down