Review fixes (#23)

* fixes and suggestions

* changed "issuer" to "issuance" as per https://github.com/zcash/orchard/pull/356#discussion_r967668241

* terminology fixes

* updated naming
This commit is contained in:
Paul 2022-10-26 21:11:37 +03:00 committed by Paul
parent 985d0d243e
commit f3ebe7a1ab
6 changed files with 90 additions and 86 deletions

View File

@ -370,7 +370,9 @@ impl Builder {
let mut pre_actions: Vec<_> = Vec::new();
// Pair up the spends and recipients, extending with dummy values as necessary.
for (note_type, (mut spends, mut recipients)) in partition(&self.spends, &self.recipients) {
for (note_type, (mut spends, mut recipients)) in
partition_by_asset(&self.spends, &self.recipients)
{
let num_spends = spends.len();
let num_recipients = recipients.len();
let num_actions = [num_spends, num_recipients, MIN_ACTIONS]
@ -460,20 +462,20 @@ impl Builder {
}
/// partition a list of spends and recipients by note types.
fn partition(
fn partition_by_asset(
spends: &[SpendInfo],
recipients: &[RecipientInfo],
) -> HashMap<NoteType, (Vec<SpendInfo>, Vec<RecipientInfo>)> {
let mut hm = HashMap::new();
for s in spends.iter() {
for s in spends {
hm.entry(s.note.note_type())
.or_insert((vec![], vec![]))
.0
.push(s.clone());
}
for r in recipients.iter() {
for r in recipients {
hm.entry(r.note_type)
.or_insert((vec![], vec![]))
.1

View File

@ -19,7 +19,6 @@ pub mod value_commit_v;
pub const ORCHARD_PERSONALIZATION: &str = "z.cash:Orchard";
/// SWU hash-to-curve personalization for the value commitment generator
/// TODO: should we change to "NOTE_TYPE_PERSONALIZATION"?
pub const VALUE_COMMITMENT_PERSONALIZATION: &str = "z.cash:Orchard-cv";
/// SWU hash-to-curve personalization for the note type generator

View File

@ -11,7 +11,7 @@ use crate::issuance::Error::{
IssueActionPreviouslyFinalizedNoteType, IssueBundleIkMismatchNoteType,
IssueBundleInvalidSignature, WrongAssetDescSize,
};
use crate::keys::{IssuerAuthorizingKey, IssuerValidatingKey};
use crate::keys::{IssuanceAuthorizingKey, IssuanceValidatingKey};
use crate::note::note_type::MAX_ASSET_DESCRIPTION_SIZE;
use crate::note::{NoteType, Nullifier};
use crate::value::NoteValue;
@ -24,7 +24,7 @@ use crate::{
#[derive(Debug)]
pub struct IssueBundle<T: IssueAuth> {
/// The issuer key for the note being created.
ik: IssuerValidatingKey,
ik: IssuanceValidatingKey,
/// The list of issue actions that make up this bundle.
actions: Vec<IssueAction>,
/// The authorization for this action.
@ -84,7 +84,7 @@ impl IssueAction {
/// Return the `NoteType` if the provided `ik` is used to derive the `note_type` for **all** internal notes.
fn are_note_types_derived_correctly(
&self,
ik: &IssuerValidatingKey,
ik: &IssuanceValidatingKey,
) -> Result<NoteType, Error> {
match self
.notes
@ -137,7 +137,7 @@ impl IssueAuth for Signed {}
impl<T: IssueAuth> IssueBundle<T> {
/// Returns the issuer verification key for the bundle.
pub fn ik(&self) -> &IssuerValidatingKey {
pub fn ik(&self) -> &IssuanceValidatingKey {
&self.ik
}
/// Return the actions for a given `IssueBundle`.
@ -180,7 +180,7 @@ impl<T: IssueAuth> IssueBundle<T> {
impl IssueBundle<Unauthorized> {
/// Constructs a new `IssueBundle`.
pub fn new(ik: IssuerValidatingKey) -> IssueBundle<Unauthorized> {
pub fn new(ik: IssuanceValidatingKey) -> IssueBundle<Unauthorized> {
IssueBundle {
ik,
actions: Vec::new(),
@ -283,9 +283,9 @@ impl IssueBundle<Prepared> {
pub fn sign<R: RngCore + CryptoRng>(
self,
mut rng: R,
isk: &IssuerAuthorizingKey,
isk: &IssuanceAuthorizingKey,
) -> Result<IssueBundle<Signed>, Error> {
let expected_ik: IssuerValidatingKey = (isk).into();
let expected_ik: IssuanceValidatingKey = (isk).into();
// Make sure the `expected_ik` matches the note_type for all notes.
self.actions.iter().try_for_each(|action| {
@ -454,7 +454,7 @@ mod tests {
};
use crate::issuance::{verify_issue_bundle, IssueAction, Signed};
use crate::keys::{
FullViewingKey, IssuerAuthorizingKey, IssuerValidatingKey, Scope, SpendingKey,
FullViewingKey, IssuanceAuthorizingKey, IssuanceValidatingKey, Scope, SpendingKey,
};
use crate::note::{NoteType, Nullifier};
use crate::value::NoteValue;
@ -468,15 +468,15 @@ mod tests {
fn setup_params() -> (
OsRng,
IssuerAuthorizingKey,
IssuerValidatingKey,
IssuanceAuthorizingKey,
IssuanceValidatingKey,
Address,
[u8; 32],
) {
let mut rng = OsRng;
let sk = SpendingKey::random(&mut rng);
let isk: IssuerAuthorizingKey = (&sk).into();
let ik: IssuerValidatingKey = (&isk).into();
let isk: IssuanceAuthorizingKey = (&sk).into();
let ik: IssuanceValidatingKey = (&isk).into();
let fvk = FullViewingKey::from(&sk);
let recipient = fvk.address_at(0u32, Scope::External);
@ -689,7 +689,7 @@ mod tests {
)
.unwrap();
let wrong_isk: IssuerAuthorizingKey = (&SpendingKey::random(&mut OsRng)).into();
let wrong_isk: IssuanceAuthorizingKey = (&SpendingKey::random(&mut OsRng)).into();
let err = bundle
.prepare([0; 32])
@ -845,7 +845,7 @@ mod tests {
)
.unwrap();
let wrong_isk: IssuerAuthorizingKey = (&SpendingKey::random(&mut rng)).into();
let wrong_isk: IssuanceAuthorizingKey = (&SpendingKey::random(&mut rng)).into();
let mut signed = bundle.prepare(sighash).sign(rng, &isk).unwrap();
@ -951,8 +951,8 @@ mod tests {
let mut signed = bundle.prepare(sighash).sign(rng, &isk).unwrap();
let incorrect_sk = SpendingKey::random(&mut rng);
let incorrect_isk: IssuerAuthorizingKey = (&incorrect_sk).into();
let incorrect_ik: IssuerValidatingKey = (&incorrect_isk).into();
let incorrect_isk: IssuanceAuthorizingKey = (&incorrect_sk).into();
let incorrect_ik: IssuanceValidatingKey = (&incorrect_isk).into();
// Add "bad" note
let note = Note::new(
@ -1024,7 +1024,7 @@ mod tests {
#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
pub mod testing {
use crate::issuance::{IssueAction, IssueBundle, Prepared, Signed, Unauthorized};
use crate::keys::testing::{arb_issuer_authorizing_key, arb_issuer_validating_key};
use crate::keys::testing::{arb_issuance_authorizing_key, arb_issuance_validating_key};
use crate::note::testing::arb_zsa_note;
use proptest::collection::vec;
use proptest::prelude::*;
@ -1049,7 +1049,7 @@ pub mod testing {
pub fn arb_unathorized_issue_bundle(n_actions: usize)
(
actions in vec(arb_issue_action(), n_actions),
ik in arb_issuer_validating_key()
ik in arb_issuance_validating_key()
) -> IssueBundle<Unauthorized> {
IssueBundle {
ik,
@ -1066,7 +1066,7 @@ pub mod testing {
pub fn arb_prepared_issue_bundle(n_actions: usize)
(
actions in vec(arb_issue_action(), n_actions),
ik in arb_issuer_validating_key(),
ik in arb_issuance_validating_key(),
fake_sighash in prop::array::uniform32(prop::num::u8::ANY)
) -> IssueBundle<Prepared> {
IssueBundle {
@ -1084,8 +1084,8 @@ pub mod testing {
pub fn arb_signed_issue_bundle(n_actions: usize)
(
actions in vec(arb_issue_action(), n_actions),
ik in arb_issuer_validating_key(),
isk in arb_issuer_authorizing_key(),
ik in arb_issuance_validating_key(),
isk in arb_issuance_authorizing_key(),
rng_seed in prop::array::uniform32(prop::num::u8::ANY),
fake_sighash in prop::array::uniform32(prop::num::u8::ANY)
) -> IssueBundle<Signed> {

View File

@ -178,7 +178,7 @@ impl SpendValidatingKey {
self.0.randomize(randomizer)
}
/// Converts this spend validating key to its serialized form,
/// Converts this issuance validating key to its serialized form,
/// I2LEOSP_256(ak).
pub(crate) fn to_bytes(&self) -> [u8; 32] {
// This is correct because the wrapped point must have ỹ = 0, and
@ -194,23 +194,23 @@ impl SpendValidatingKey {
}
}
/// An issuer authorizing key, used to create issuer authorization signatures.
/// An issuance authorizing key, used to create issuance authorization signatures.
/// This type enforces that the corresponding public point (ik^) has ỹ = 0.
///
/// $\mathsf{isk}$ as defined in
/// [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents].
/// [Issuance of Zcash Shielded Assets ZIP-0227 § Asset Identifier Generation (DRAFT ZIP)][IssuanceZSA].
///
/// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents
/// [IssuanceZSA]: https://qed-it.github.io/zips/draft-ZIP-0227.html#asset-identifier-generation
#[derive(Clone, Debug)]
pub struct IssuerAuthorizingKey(redpallas::SigningKey<SpendAuth>);
pub struct IssuanceAuthorizingKey(redpallas::SigningKey<SpendAuth>);
impl IssuerAuthorizingKey {
impl IssuanceAuthorizingKey {
/// Derives isk from sk. Internal use only, does not enforce all constraints.
fn derive_inner(sk: &SpendingKey) -> pallas::Scalar {
to_scalar(PrfExpand::ZsaIsk.expand(&sk.0))
}
/// Sign the provided message using the `IssuerAuthorizingKey`.
/// Sign the provided message using the `IssuanceAuthorizingKey`.
pub fn sign(
&self,
rng: &mut (impl RngCore + CryptoRng),
@ -220,51 +220,51 @@ impl IssuerAuthorizingKey {
}
}
impl From<&SpendingKey> for IssuerAuthorizingKey {
impl From<&SpendingKey> for IssuanceAuthorizingKey {
fn from(sk: &SpendingKey) -> Self {
let isk = Self::derive_inner(sk);
// IssuerSigningKey cannot be constructed such that this assertion would fail.
// IssuanceSigningKey cannot be constructed such that this assertion would fail.
assert!(!bool::from(isk.is_zero()));
let ret = IssuerAuthorizingKey(isk.to_repr().try_into().unwrap());
let ret = IssuanceAuthorizingKey(isk.to_repr().try_into().unwrap());
// If the last bit of repr_P(ik) is 1, negate isk.
if (<[u8; 32]>::from(IssuerValidatingKey::from(&ret).0)[31] >> 7) == 1 {
IssuerAuthorizingKey((-isk).to_repr().try_into().unwrap())
if (<[u8; 32]>::from(IssuanceValidatingKey::from(&ret).0)[31] >> 7) == 1 {
IssuanceAuthorizingKey((-isk).to_repr().try_into().unwrap())
} else {
ret
}
}
}
/// A key used to validate issuer authorization signatures.
/// A key used to validate issuance authorization signatures.
///
/// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents].
/// Defined in [Issuance of Zcash Shielded Assets ZIP-0227 § Asset Identifier Generation (DRAFT PR)][IssuanceZSA].
/// Note that this is $\mathsf{ik}^\mathbb{P}$, which by construction is equivalent to
/// $\mathsf{ik}$ but stored here as a RedPallas verification key.
///
/// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents
/// [IssuanceZSA]: https://qed-it.github.io/zips/draft-ZIP-0227.html#asset-identifier-generation
#[derive(Debug, Clone, PartialOrd, Ord)]
pub struct IssuerValidatingKey(redpallas::VerificationKey<SpendAuth>);
impl From<&IssuerAuthorizingKey> for IssuerValidatingKey {
fn from(isk: &IssuerAuthorizingKey) -> Self {
IssuerValidatingKey((&isk.0).into())
pub struct IssuanceValidatingKey(redpallas::VerificationKey<SpendAuth>);
impl From<&IssuanceAuthorizingKey> for IssuanceValidatingKey {
fn from(isk: &IssuanceAuthorizingKey) -> Self {
IssuanceValidatingKey((&isk.0).into())
}
}
impl From<&IssuerValidatingKey> for pallas::Point {
fn from(issuer_validating_key: &IssuerValidatingKey) -> pallas::Point {
pallas::Point::from_bytes(&(&issuer_validating_key.0).into()).unwrap()
impl From<&IssuanceValidatingKey> for pallas::Point {
fn from(issuance_validating_key: &IssuanceValidatingKey) -> pallas::Point {
pallas::Point::from_bytes(&(&issuance_validating_key.0).into()).unwrap()
}
}
impl PartialEq for IssuerValidatingKey {
impl PartialEq for IssuanceValidatingKey {
fn eq(&self, other: &Self) -> bool {
<[u8; 32]>::from(&self.0).eq(&<[u8; 32]>::from(&other.0))
}
}
impl Eq for IssuerValidatingKey {}
impl Eq for IssuanceValidatingKey {}
impl IssuerValidatingKey {
impl IssuanceValidatingKey {
/// Converts this spend validating key to its serialized form,
/// I2LEOSP_256(ik).
pub(crate) fn to_bytes(&self) -> [u8; 32] {
@ -277,7 +277,7 @@ impl IssuerValidatingKey {
<[u8; 32]>::try_from(bytes)
.ok()
.and_then(check_structural_validity)
.map(IssuerValidatingKey)
.map(IssuanceValidatingKey)
}
/// Verifies a purported `signature` over `msg` made by this verification key.
@ -292,9 +292,9 @@ impl IssuerValidatingKey {
/// A function to check structural validity of the validating keys for authorizing transfers and
/// issuing assets
/// Structural validity checks for ik_P:
/// Structural validity checks for ak_P or ik_P:
/// - The point must not be the identity (which for Pallas is canonically encoded as all-zeroes).
/// - The sign of the y-coordinate must be positive.
/// - The compressed y-coordinate bit must be 0.
fn check_structural_validity(
verification_key_bytes: [u8; 32],
) -> Option<VerificationKey<SpendAuth>> {
@ -1043,8 +1043,8 @@ impl SharedSecret {
#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
pub mod testing {
use super::{
DiversifierIndex, DiversifierKey, EphemeralSecretKey, IssuerAuthorizingKey,
IssuerValidatingKey, SpendingKey,
DiversifierIndex, DiversifierKey, EphemeralSecretKey, IssuanceAuthorizingKey,
IssuanceValidatingKey, SpendingKey,
};
use proptest::prelude::*;
use rand::{rngs::StdRng, SeedableRng};
@ -1096,17 +1096,17 @@ pub mod testing {
}
prop_compose! {
/// Generate a uniformly distributed RedDSA issuer authorizing key.
pub fn arb_issuer_authorizing_key()(rng_seed in prop::array::uniform32(prop::num::u8::ANY)) -> IssuerAuthorizingKey {
/// Generate a uniformly distributed RedDSA issuance authorizing key.
pub fn arb_issuance_authorizing_key()(rng_seed in prop::array::uniform32(prop::num::u8::ANY)) -> IssuanceAuthorizingKey {
let mut rng = StdRng::from_seed(rng_seed);
IssuerAuthorizingKey::from(&SpendingKey::random(&mut rng))
IssuanceAuthorizingKey::from(&SpendingKey::random(&mut rng))
}
}
prop_compose! {
/// Generate a uniformly distributed RedDSA issuer validating key.
pub fn arb_issuer_validating_key()(isk in arb_issuer_authorizing_key()) -> IssuerValidatingKey {
IssuerValidatingKey::from(&isk)
/// Generate a uniformly distributed RedDSA issuance validating key.
pub fn arb_issuance_validating_key()(isk in arb_issuance_authorizing_key()) -> IssuanceValidatingKey {
IssuanceValidatingKey::from(&isk)
}
}
}
@ -1180,13 +1180,13 @@ mod tests {
let ask: SpendAuthorizingKey = (&sk).into();
assert_eq!(<[u8; 32]>::from(&ask.0), tv.ask);
let isk: IssuerAuthorizingKey = (&sk).into();
let isk: IssuanceAuthorizingKey = (&sk).into();
assert_eq!(<[u8; 32]>::from(&isk.0), tv.isk);
let ak: SpendValidatingKey = (&ask).into();
assert_eq!(<[u8; 32]>::from(ak.0), tv.ak);
let ik: IssuerValidatingKey = (&isk).into();
let ik: IssuanceValidatingKey = (&isk).into();
assert_eq!(<[u8; 32]>::from(ik.0), tv.ik);
let nk: NullifierDerivingKey = (&sk).into();

View File

@ -6,7 +6,7 @@ use std::hash::{Hash, Hasher};
use subtle::{Choice, ConstantTimeEq, CtOption};
use crate::constants::fixed_bases::{VALUE_COMMITMENT_PERSONALIZATION, VALUE_COMMITMENT_V_BYTES};
use crate::keys::IssuerValidatingKey;
use crate::keys::IssuanceValidatingKey;
/// Note type identifier.
#[derive(Clone, Copy, Debug, Eq)]
@ -15,8 +15,7 @@ pub struct NoteType(pallas::Point);
pub const MAX_ASSET_DESCRIPTION_SIZE: usize = 512;
// the hasher used to derive the assetID
#[allow(non_snake_case)]
fn assetID_hasher(msg: Vec<u8>) -> pallas::Point {
fn asset_id_hasher(msg: Vec<u8>) -> pallas::Point {
// TODO(zsa) replace personalization
pallas::Point::hash_to_curve(VALUE_COMMITMENT_PERSONALIZATION)(&msg)
}
@ -32,25 +31,29 @@ impl NoteType {
self.0.to_bytes()
}
/// $DeriveNoteType$.
/// Note type derivation$.
///
/// Defined in [Zcash Protocol Spec § TBD: Note Types][notetypes].
/// Defined in [Transfer and Burn of Zcash Shielded Assets][notetypes].
///
/// [notetypes]: https://zips.z.cash/protocol/nu5.pdf#notetypes
/// [notetypes]: https://qed-it.github.io/zips/draft-ZIP-0226.html#asset-types
///
/// # Panics
///
/// Panics if `asset_desc` is empty or greater than `MAX_ASSET_DESCRIPTION_SIZE`.
#[allow(non_snake_case)]
pub fn derive(ik: &IssuerValidatingKey, asset_desc: &str) -> Self {
pub fn derive(ik: &IssuanceValidatingKey, asset_desc: &str) -> Self {
assert!(!asset_desc.is_empty() && asset_desc.len() <= MAX_ASSET_DESCRIPTION_SIZE);
let mut s = vec![];
s.extend(ik.to_bytes());
s.extend(asset_desc.as_bytes());
NoteType(assetID_hasher(s))
NoteType(asset_id_hasher(s))
}
/// Note type for the "native" currency (zec), maintains backward compatibility with Orchard untyped notes.
pub fn native() -> Self {
NoteType(assetID_hasher(VALUE_COMMITMENT_V_BYTES.to_vec()))
NoteType(asset_id_hasher(VALUE_COMMITMENT_V_BYTES.to_vec()))
}
/// The base point used in value commitments.
@ -85,7 +88,7 @@ pub mod testing {
use proptest::prelude::*;
use crate::keys::{testing::arb_spending_key, IssuerAuthorizingKey, IssuerValidatingKey};
use crate::keys::{testing::arb_spending_key, IssuanceAuthorizingKey, IssuanceValidatingKey};
prop_compose! {
/// Generate a uniformly distributed note type
@ -97,8 +100,8 @@ pub mod testing {
if is_native {
NoteType::native()
} else {
let isk = IssuerAuthorizingKey::from(&sk);
NoteType::derive(&IssuerValidatingKey::from(&isk), &str)
let isk = IssuanceAuthorizingKey::from(&sk);
NoteType::derive(&IssuanceValidatingKey::from(&isk), &str)
}
}
}
@ -117,8 +120,8 @@ pub mod testing {
sk in arb_spending_key(),
str in "[A-Za-z]{255}"
) -> NoteType {
let isk = IssuerAuthorizingKey::from(&sk);
NoteType::derive(&IssuerValidatingKey::from(&isk), &str)
let isk = IssuanceAuthorizingKey::from(&sk);
NoteType::derive(&IssuanceValidatingKey::from(&isk), &str)
}
}
}

View File

@ -5,7 +5,7 @@ use incrementalmerkletree::bridgetree::BridgeTree;
use incrementalmerkletree::{Hashable, Tree};
use orchard::bundle::Authorized;
use orchard::issuance::{verify_issue_bundle, IssueBundle, Signed, Unauthorized};
use orchard::keys::{IssuerAuthorizingKey, IssuerValidatingKey};
use orchard::keys::{IssuanceAuthorizingKey, IssuanceValidatingKey};
use orchard::note::{ExtractedNoteCommitment, NoteType};
use orchard::note_encryption::OrchardDomain;
use orchard::tree::{MerkleHashOrchard, MerklePath};
@ -27,8 +27,8 @@ struct Keychain {
vk: VerifyingKey,
sk: SpendingKey,
fvk: FullViewingKey,
isk: IssuerAuthorizingKey,
ik: IssuerValidatingKey,
isk: IssuanceAuthorizingKey,
ik: IssuanceValidatingKey,
recipient: Address,
}
@ -42,10 +42,10 @@ impl Keychain {
fn fvk(&self) -> &FullViewingKey {
&self.fvk
}
fn isk(&self) -> &IssuerAuthorizingKey {
fn isk(&self) -> &IssuanceAuthorizingKey {
&self.isk
}
fn ik(&self) -> &IssuerValidatingKey {
fn ik(&self) -> &IssuanceValidatingKey {
&self.ik
}
}
@ -58,8 +58,8 @@ fn prepare_keys() -> Keychain {
let fvk = FullViewingKey::from(&sk);
let recipient = fvk.address_at(0u32, Scope::External);
let isk = IssuerAuthorizingKey::from(&sk);
let ik = IssuerValidatingKey::from(&isk);
let isk = IssuanceAuthorizingKey::from(&sk);
let ik = IssuanceValidatingKey::from(&isk);
Keychain {
pk,
vk,
@ -74,7 +74,7 @@ fn prepare_keys() -> Keychain {
fn sign_issue_bundle(
unauthorized: IssueBundle<Unauthorized>,
mut rng: OsRng,
isk: IssuerAuthorizingKey,
isk: IssuanceAuthorizingKey,
) -> IssueBundle<Signed> {
let sighash = unauthorized.commitment().into();
let proven = unauthorized.prepare(sighash);