-
Notifications
You must be signed in to change notification settings - Fork 0
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
OrchardZSA backward compatability 0.8.0 #104
Changes from 6 commits
8bee528
5f78e72
5221c29
e07fab9
ee0a07d
8d2d130
b177325
4900519
0a3d231
f3ae948
fc64461
c6f8936
ce46de0
e5ab5d0
a8dadc9
a8a72c2
120b090
5d5bbf1
97f55f9
90512c8
416b496
a1e6cd6
8b28a0e
fc20a5a
1d68e0b
c820dae
088494a
46a222e
367c701
dc97b7a
cc99400
e2306e3
d721463
6ba0a38
19dbd0a
b6d577e
8795ca4
58e05b1
e67c792
a9af64c
511caf6
019cbc4
8676e26
c99c043
b883997
5ea48b4
7bed5a9
fb9e09a
12f77f7
f27b3ce
e2abb42
da51ae2
5dac03e
450f814
efe7b38
56c850a
93645de
e10efb3
2273de6
6353684
4fa556d
6ffb37b
8c5274d
f7789bf
cb0d205
fe8b986
9df82ad
e9e65bc
576f4c4
f04c205
3cd8294
9a0cdab
4a23e66
8289354
b4dec8d
3bf500f
3ccee39
a3aac82
6295375
621a5b4
fdfa5dc
1be9604
1dd1a82
cfc2ddd
f4ae7f2
f677ee2
e2e200d
b797abc
1363d47
c05d06e
1de980f
4155972
060bf95
562dcf0
27c03b4
aa46d52
1bffe59
752feb1
f45db57
b7b780c
82e19da
1e54e19
3efb12c
80c7574
b72152a
0c3742a
be1fc32
f0c40f2
c4baaa7
2d7e2a3
de85554
b20ff2f
4aff14d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -584,7 +584,7 @@ impl Builder { | |
} | ||
|
||
/// Add an instruction to burn a given amount of a specific asset. | ||
// FIXME: Should we add `add_burn` into `testing` (into `arb_bundle` function) | ||
// FIXME: Should we add `add_burn` into the `into_bundle` function in this testing module | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, I opened a new task for later There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Removed the FIXME comment. |
||
// (like `add_spend` and `add_output` are used there)? | ||
pub fn add_burn(&mut self, asset: AssetBase, value: NoteValue) -> Result<(), BuildError> { | ||
use std::collections::hash_map::Entry; | ||
|
@@ -902,8 +902,7 @@ pub struct InProgress<P, S: InProgressSignatures> { | |
} | ||
|
||
impl<P, S: InProgressSignatures> InProgress<P, S> { | ||
/// Changes this authorization from one proof type to another. | ||
// FIXME: Change comment to "mutate the proof using the provided function" | ||
/// Mutate the proof using the provided function. | ||
pub fn map_proof<F, P2>(self, f: F) -> InProgress<P2, S> | ||
where | ||
F: FnOnce(P) -> P2, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ use crate::{ | |
DiversifiedTransmissionKey, Diversifier, EphemeralPublicKey, EphemeralSecretKey, | ||
OutgoingViewingKey, PreparedEphemeralPublicKey, PreparedIncomingViewingKey, SharedSecret, | ||
}, | ||
note::{AssetBase, ExtractedNoteCommitment, Note, RandomSeed, Rho}, | ||
note::{ExtractedNoteCommitment, Note, RandomSeed, Rho}, | ||
value::{NoteValue, ValueCommitment}, | ||
}; | ||
|
||
|
@@ -45,12 +45,6 @@ const ZSA_ASSET_SIZE: usize = 32; | |
/// The size of a ZSA compact note. | ||
pub(super) const COMPACT_NOTE_SIZE_ZSA: usize = COMPACT_NOTE_SIZE_VANILLA + ZSA_ASSET_SIZE; | ||
|
||
/// The version byte for Vanilla. | ||
pub(super) const NOTE_VERSION_BYTE_V2: u8 = 0x02; | ||
|
||
/// The version byte for ZSA. | ||
pub(super) const NOTE_VERSION_BYTE_V3: u8 = 0x03; | ||
|
||
pub(super) type Memo = [u8; MEMO_SIZE]; | ||
|
||
/// Defined in [Zcash Protocol Spec § 5.4.2: Pseudo Random Functions][concreteprfs]. | ||
|
@@ -78,28 +72,32 @@ pub(super) fn prf_ock_orchard( | |
) | ||
} | ||
|
||
// FIXME: consider returning enum instead of u8 | ||
/// Retrieves the version of the note plaintext. | ||
/// Returns `Some(u8)` if the version is recognized, otherwise `None`. | ||
pub(super) fn parse_note_version(plaintext: &[u8]) -> Option<u8> { | ||
plaintext.first().and_then(|version| match *version { | ||
NOTE_VERSION_BYTE_V2 | NOTE_VERSION_BYTE_V3 => Some(*version), | ||
_ => None, | ||
}) | ||
/// Checks if the version of the note plaintext matches the expected version for the domain. | ||
pub(super) fn validate_note_version<D: OrchardDomainCommon>( | ||
plaintext: &D::CompactNotePlaintextBytes, | ||
) -> bool { | ||
plaintext | ||
.as_ref() | ||
.first() | ||
.map_or(false, |version| *version == D::NOTE_VERSION_BYTE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to restore Even if now we don't support other versions today, the encryption/decryption need to be version agnostic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @PaulLaux , I still have a question. Inside the On the other hand, to extract the asset, we can ignore the version byte value and use the Orchard flavor or compact note size alone to decide how we extract the asset. This is the way I implemented it now (with additional note version validation, which has nothing to do with asset extraction). Do you mean we need to use a combination of the note version byte and the flavor/plaitext size? Like this: let version = parse_note_version(plaintext.as_ref())?;
let asset = match version {
NOTE_VERSION_BYTE_V2 if D::COMPACT_NOTE_SIZE == COMPACT_NOTE_SIZE_VANILLA => AssetBase::native(),
NOTE_VERSION_BYTE_V3 if D::COMPACT_NOTE_SIZE == COMPACT_NOTE_SIZE_ZSA => {
let bytes = plaintext.as_ref()[COMPACT_NOTE_SIZE_VANILLA..COMPACT_NOTE_SIZE_ZSA]
.try_into()
.unwrap();
AssetBase::from_bytes(bytes).into()
},
_ => return None,
} In this case, we don't need the new let version = parse_note_version(plaintext.as_ref())?;
let asset = match version {
NOTE_VERSION_BYTE_V2 if D::COMPACT_NOTE_SIZE == COMPACT_NOTE_SIZE_VANILLA => D::extract_asset(plaintext)?,
NOTE_VERSION_BYTE_V3 if D::COMPACT_NOTE_SIZE == COMPACT_NOTE_SIZE_ZSA => D::extract_asset(plaintext)?,
_ => return None,
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when activating let asset = D::extract_asset(plaintext)?; We already chosen the Except regular error handling / propagation inside the function, nothing else is needed. |
||
} | ||
|
||
/// Parses the note plaintext (excluding the memo) and extracts the note and address if valid. | ||
/// Domain-specific requirements: | ||
/// - If the note version is 3, the `plaintext` must contain a valid encoding of a ZSA asset type. | ||
pub(super) fn parse_note_plaintext_without_memo<Bytes: AsRef<[u8]>, F>( | ||
pub(super) fn parse_note_plaintext_without_memo<D: OrchardDomainCommon, F>( | ||
rho: Rho, | ||
plaintext: &Bytes, | ||
plaintext: &D::CompactNotePlaintextBytes, | ||
get_validated_pk_d: F, | ||
) -> Option<(Note, Address)> | ||
where | ||
F: FnOnce(&Diversifier) -> Option<DiversifiedTransmissionKey>, | ||
{ | ||
// The unwraps below are guaranteed to succeed by the assertion above | ||
if !validate_note_version::<D>(plaintext) { | ||
return None; | ||
} | ||
|
||
// The unwraps below are guaranteed to succeed | ||
let diversifier = Diversifier::from_bytes( | ||
plaintext.as_ref()[NOTE_DIVERSIFIER_OFFSET..NOTE_VALUE_OFFSET] | ||
.try_into() | ||
|
@@ -121,19 +119,9 @@ where | |
|
||
let pk_d = get_validated_pk_d(&diversifier)?; | ||
let recipient = Address::from_parts(diversifier, pk_d); | ||
|
||
let asset = match parse_note_version(plaintext.as_ref())? { | ||
NOTE_VERSION_BYTE_V2 => AssetBase::native(), | ||
NOTE_VERSION_BYTE_V3 => { | ||
let bytes = plaintext.as_ref()[COMPACT_NOTE_SIZE_VANILLA..COMPACT_NOTE_SIZE_ZSA] | ||
.try_into() | ||
.unwrap(); | ||
AssetBase::from_bytes(bytes).unwrap() | ||
} | ||
_ => panic!("invalid note version"), | ||
}; | ||
|
||
let asset = D::extract_asset(plaintext)?; | ||
let note = Option::from(Note::from_parts(recipient, value, asset, rho, rseed))?; | ||
|
||
Some((note, recipient)) | ||
} | ||
|
||
|
@@ -251,7 +239,7 @@ impl<D: OrchardDomainCommon> Domain for OrchardDomain<D> { | |
ivk: &Self::IncomingViewingKey, | ||
plaintext: &D::CompactNotePlaintextBytes, | ||
) -> Option<(Self::Note, Self::Recipient)> { | ||
parse_note_plaintext_without_memo(self.rho, plaintext, |diversifier| { | ||
parse_note_plaintext_without_memo::<D, _>(self.rho, plaintext, |diversifier| { | ||
Some(DiversifiedTransmissionKey::derive(ivk, diversifier)) | ||
}) | ||
} | ||
|
@@ -261,7 +249,7 @@ impl<D: OrchardDomainCommon> Domain for OrchardDomain<D> { | |
pk_d: &Self::DiversifiedTransmissionKey, | ||
plaintext: &D::CompactNotePlaintextBytes, | ||
) -> Option<(Self::Note, Self::Recipient)> { | ||
parse_note_plaintext_without_memo(self.rho, plaintext, |_| Some(*pk_d)) | ||
parse_note_plaintext_without_memo::<D, _>(self.rho, plaintext, |_| Some(*pk_d)) | ||
} | ||
|
||
fn extract_memo( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,11 @@ use core::fmt; | |
|
||
use zcash_note_encryption_zsa::{AEAD_TAG_SIZE, MEMO_SIZE}; | ||
|
||
use crate::{action::Action, note::Rho, Note}; | ||
use crate::{ | ||
action::Action, | ||
note::{AssetBase, Rho}, | ||
Note, | ||
}; | ||
|
||
use super::{compact_action::CompactAction, domain::Memo}; | ||
|
||
|
@@ -62,6 +66,9 @@ pub trait OrchardDomainCommon: fmt::Debug + Clone { | |
/// The size of an encrypted note ciphertext, accounting for additional AEAD tag space. | ||
const ENC_CIPHERTEXT_SIZE: usize = Self::NOTE_PLAINTEXT_SIZE + AEAD_TAG_SIZE; | ||
|
||
/// The note version byte. | ||
const NOTE_VERSION_BYTE: u8; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not working. we should not bake the version byte as part of the domain. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The version is a property of the note, not the encryption. |
||
/// The raw bytes of a note plaintext. | ||
type NotePlaintextBytes: NoteBytes; | ||
/// The raw bytes of an encrypted note plaintext. | ||
|
@@ -73,6 +80,9 @@ pub trait OrchardDomainCommon: fmt::Debug + Clone { | |
|
||
/// Builds NotePlaintextBytes from Note and Memo. | ||
fn build_note_plaintext_bytes(note: &Note, memo: &Memo) -> Self::NotePlaintextBytes; | ||
|
||
/// Extracts the asset from the note plaintext. | ||
fn extract_asset(plaintext: &Self::CompactNotePlaintextBytes) -> Option<AssetBase>; | ||
} | ||
|
||
/// Orchard-specific note encryption logic. | ||
|
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.
no
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.
Got it. Removed the FIXME comment.