Skip to content

Commit

Permalink
updates based on review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
vivek-arte committed Jan 26, 2024
1 parent a76a074 commit f9023ed
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 23 deletions.
13 changes: 8 additions & 5 deletions src/issuance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,12 +410,15 @@ impl IssueBundle<Prepared> {
Ok(())
})?;

// Make sure the signature can be generated.
let signature = isk
.try_sign(&self.authorization.sighash)
.map_err(|_| IssueBundleInvalidSignature)?;

Ok(IssueBundle {
ik: self.ik,
actions: self.actions,
authorization: Signed {
signature: isk.sign(&self.authorization.sighash),
},
authorization: Signed { signature },
})
}
}
Expand Down Expand Up @@ -1166,7 +1169,7 @@ mod tests {
let mut signed = bundle.prepare(sighash).sign(&isk).unwrap();

signed.set_authorization(Signed {
signature: wrong_isk.sign(&sighash),
signature: wrong_isk.try_sign(&sighash).unwrap(),
});

let prev_finalized = &HashSet::new();
Expand Down Expand Up @@ -1341,7 +1344,7 @@ mod tests {
ik: bundle.ik,
actions: bundle.actions,
authorization: Signed {
signature: isk.sign(&sighash),
signature: isk.try_sign(&sighash).unwrap(),
},
};

Expand Down
39 changes: 21 additions & 18 deletions src/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,14 @@ use group::{
prime::PrimeCurveAffine,
Curve, GroupEncoding,
};
use k256::schnorr::signature::{Signer, Verifier};
use k256::schnorr::{Signature, VerifyingKey};
use k256::{schnorr, NonZeroScalar, PublicKey};
use k256::{
schnorr,
schnorr::{
signature::{Signer, Verifier},
Signature, VerifyingKey,
},
NonZeroScalar, PublicKey,
};
use pasta_curves::{pallas, pallas::Scalar};
use rand::rngs::OsRng;
use rand::RngCore;
Expand Down Expand Up @@ -245,8 +250,7 @@ impl IssuanceAuthorizingKey {
///
/// [ZIP 32]: https://zips.z.cash/zip-0032
pub(crate) fn random() -> Self {
let mut rng = OsRng;
IssuanceAuthorizingKey(NonZeroScalar::random(&mut rng))
IssuanceAuthorizingKey(NonZeroScalar::random(&mut OsRng))
}

/// Constructs an Orchard issuance key from uniformly-random bytes.
Expand All @@ -260,7 +264,7 @@ impl IssuanceAuthorizingKey {

/// Returns the raw bytes of the issuance key.
pub fn to_bytes(&self) -> [u8; 32] {
self.0.to_bytes().as_slice().try_into().unwrap()
self.0.to_bytes().try_into().unwrap() // The unwrap call never fails since ??

Check warning on line 267 in src/keys.rs

View workflow job for this annotation

GitHub Actions / Clippy (beta)

use of a fallible conversion when an infallible one could be used

warning: use of a fallible conversion when an infallible one could be used --> src/keys.rs:267:27 | 267 | self.0.to_bytes().try_into().unwrap() // The unwrap call never fails since ?? | ^^^^^^^^^^^^^^^^^^^ help: use: `into()` | = note: converting `GenericArray<u8, UInt<UInt<UInt<UInt<UInt<UInt<UTerm, B1>, B0>, B0>, B0>, B0>, B0>>` to `[u8; 32]` cannot fail = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions = note: `#[warn(clippy::unnecessary_fallible_conversions)]` on by default

Check warning on line 267 in src/keys.rs

View workflow job for this annotation

GitHub Actions / Clippy (beta)

use of a fallible conversion when an infallible one could be used

warning: use of a fallible conversion when an infallible one could be used --> src/keys.rs:267:27 | 267 | self.0.to_bytes().try_into().unwrap() // The unwrap call never fails since ?? | ^^^^^^^^^^^^^^^^^^^ help: use: `into()` | = note: converting `GenericArray<u8, UInt<UInt<UInt<UInt<UInt<UInt<UTerm, B1>, B0>, B0>, B0>, B0>, B0>>` to `[u8; 32]` cannot fail = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions = note: `#[warn(clippy::unnecessary_fallible_conversions)]` on by default
}

/// Derives the Orchard-ZSA issuance key for the given seed, coin type, and account.
Expand All @@ -280,18 +284,20 @@ impl IssuanceAuthorizingKey {
}

/// Sign the provided message using the `IssuanceAuthorizingKey`.
/// Only supports signing of messages longer than 32 bytes, using prehashing, as described in [BIP 340][bip340]
/// Only supports signing of messages longer than 32 bytes, as described in [BIP 340][bip340]
///
/// [bip340]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki#messages-of-arbitrary-size
pub fn sign(&self, msg: &[u8]) -> Signature {
pub fn try_sign(&self, msg: &[u8]) -> Result<Signature, schnorr::Error> {
assert!(msg.len() >= 32);
schnorr::SigningKey::from(self.0).sign(msg)
schnorr::SigningKey::from(self.0).try_sign(msg)
}
}

impl Debug for IssuanceAuthorizingKey {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
f.debug_tuple("").field(&self.0.to_bytes()).finish()
f.debug_tuple("IssuanceAuthorizingKey")
.field(&self.0.to_bytes())
.finish()
}
}

Expand Down Expand Up @@ -321,13 +327,11 @@ impl IssuanceValidatingKey {
/// Converts this issuance validating key to its serialized form,
/// in big-endian order as defined in BIP 340.
pub fn to_bytes(&self) -> [u8; 32] {
// This provides the big-endian encoding, via the FieldBytesEncoding trait in the k256 crate.
VerifyingKey::try_from(self.0)
.unwrap()
.to_bytes()
.as_slice()
.try_into()
.unwrap()
.unwrap() // This cannot fail

Check warning on line 334 in src/keys.rs

View workflow job for this annotation

GitHub Actions / Clippy (beta)

use of a fallible conversion when an infallible one could be used

warning: use of a fallible conversion when an infallible one could be used --> src/keys.rs:333:14 | 333 | .try_into() | ______________^ 334 | | .unwrap() // This cannot fail | |_____________________^ help: use: `into()` | = note: converting `GenericArray<u8, UInt<UInt<UInt<UInt<UInt<UInt<UTerm, B1>, B0>, B0>, B0>, B0>, B0>>` to `[u8; 32]` cannot fail = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions

Check warning on line 334 in src/keys.rs

View workflow job for this annotation

GitHub Actions / Clippy (beta)

use of a fallible conversion when an infallible one could be used

warning: use of a fallible conversion when an infallible one could be used --> src/keys.rs:333:14 | 333 | .try_into() | ______________^ 334 | | .unwrap() // This cannot fail | |_____________________^ help: use: `into()` | = note: converting `GenericArray<u8, UInt<UInt<UInt<UInt<UInt<UInt<UTerm, B1>, B0>, B0>, B0>, B0>, B0>>` to `[u8; 32]` cannot fail = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions
}

/// Constructs an Orchard issuance validating key from the provided bytes.
Expand Down Expand Up @@ -1205,16 +1209,15 @@ mod tests {
fn cannot_generate_issuance_authorization_signature_for_short_messages() {
let isk = IssuanceAuthorizingKey::random();
let msg = [0u8; 16];
let _ = isk.sign(&msg);
let _ = isk.try_sign(&msg);
}

#[test]
fn issuance_authorizing_key_from_bytes_to_bytes_roundtrip() {
let isk = IssuanceAuthorizingKey::random();
let isk_bytes = isk.to_bytes();
let isk_roundtrip = IssuanceAuthorizingKey::from_bytes(isk_bytes);
assert!(isk_roundtrip.is_some());
assert_eq!(isk.to_bytes(), isk_roundtrip.unwrap().to_bytes());
let isk_roundtrip = IssuanceAuthorizingKey::from_bytes(isk_bytes).unwrap();
assert_eq!(isk_bytes, isk_roundtrip.to_bytes());
}

proptest! {
Expand Down Expand Up @@ -1342,7 +1345,7 @@ mod tests {

let message = tv.msg;

let signature = isk.sign(&message);
let signature = isk.try_sign(&message).unwrap();
let sig_bytes: [u8; 64] = signature.to_bytes();
assert_eq!(sig_bytes, tv.sig);

Expand Down

0 comments on commit f9023ed

Please sign in to comment.