From 5ea7dc61191530f98d600b31bd97bc5932e036f4 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 22 Mar 2023 17:33:50 -0300 Subject: [PATCH 1/5] add error handling --- zebra-chain/src/error.rs | 38 +++++++++++++++++++++++++++ zebra-chain/src/lib.rs | 1 + zebra-chain/src/orchard/commitment.rs | 15 ++++++----- zebra-chain/src/orchard/keys.rs | 15 +++++++---- zebra-chain/src/orchard/note.rs | 29 ++++++++++++-------- zebra-chain/src/sapling/commitment.rs | 29 ++++++++++---------- zebra-chain/src/sapling/keys.rs | 12 ++++++--- 7 files changed, 99 insertions(+), 40 deletions(-) create mode 100644 zebra-chain/src/error.rs diff --git a/zebra-chain/src/error.rs b/zebra-chain/src/error.rs new file mode 100644 index 00000000000..858c345096c --- /dev/null +++ b/zebra-chain/src/error.rs @@ -0,0 +1,38 @@ +//! Errors that can occur inside any `zebra-chain` submodule. +use thiserror::Error; + +/// Number of times a `diversify_hash` will try to obtain a diversified base point. +pub const DIVERSIFY_HASH_TRIES: u8 = 5; + +/// Errors related to random bytes generation. +#[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] +pub enum RandError { + /// Error of the `try_fill_bytes` function. + #[error("failed to generate a secure stream of random bytes")] + FillBytes, + /// Error of the `diversify_hash` function. + #[error( + "failed to generate a diversified base point after {} tries", + DIVERSIFY_HASH_TRIES + )] + DiversifyHash, +} + +/// Errors related to `AffinePoint`. +#[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] +pub enum AffineError { + /// Error of `jubjub::AffinePoint::try_from`. + #[error("failed to generate a AffinePoint from a diversifier")] + FromDiversifier, +} + +/// An error type that can have `RandError` or `AffineError`. +#[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] +pub enum CryptoError { + /// Errors of type `RandError`. + #[error("Random generation failure")] + Rand(#[from] RandError), + /// Errors of type `AffineError`. + #[error("Affine point failure")] + Affine(#[from] AffineError), +} diff --git a/zebra-chain/src/lib.rs b/zebra-chain/src/lib.rs index d96e681e0be..eee58e7e9e1 100644 --- a/zebra-chain/src/lib.rs +++ b/zebra-chain/src/lib.rs @@ -23,6 +23,7 @@ pub mod block; pub mod chain_sync_status; pub mod chain_tip; pub mod diagnostic; +pub mod error; pub mod fmt; pub mod history_tree; pub mod orchard; diff --git a/zebra-chain/src/orchard/commitment.rs b/zebra-chain/src/orchard/commitment.rs index db2540b0005..c8f9023dac6 100644 --- a/zebra-chain/src/orchard/commitment.rs +++ b/zebra-chain/src/orchard/commitment.rs @@ -12,6 +12,7 @@ use rand_core::{CryptoRng, RngCore}; use crate::{ amount::Amount, + error::RandError, serialization::{ serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, }, @@ -22,14 +23,16 @@ use super::sinsemilla::*; /// Generates a random scalar from the scalar field 𝔽_{q_P}. /// /// -pub fn generate_trapdoor(csprng: &mut T) -> pallas::Scalar +pub fn generate_trapdoor(csprng: &mut T) -> Result where T: RngCore + CryptoRng, { let mut bytes = [0u8; 64]; - csprng.fill_bytes(&mut bytes); + csprng + .try_fill_bytes(&mut bytes) + .map_err(|_| RandError::FillBytes)?; // pallas::Scalar::from_bytes_wide() reduces the input modulo q_P under the hood. - pallas::Scalar::from_bytes_wide(&bytes) + Ok(pallas::Scalar::from_bytes_wide(&bytes)) } /// The randomness used in the Simsemilla hash for note commitment. @@ -223,13 +226,13 @@ impl ValueCommitment { /// Generate a new _ValueCommitment_. /// /// - pub fn randomized(csprng: &mut T, value: Amount) -> Self + pub fn randomized(csprng: &mut T, value: Amount) -> Result where T: RngCore + CryptoRng, { - let rcv = generate_trapdoor(csprng); + let rcv = generate_trapdoor(csprng)?; - Self::new(rcv, value) + Ok(Self::new(rcv, value)) } /// Generate a new `ValueCommitment` from an existing `rcv on a `value`. diff --git a/zebra-chain/src/orchard/keys.rs b/zebra-chain/src/orchard/keys.rs index 7fdbfb81045..3fd9e0ce67a 100644 --- a/zebra-chain/src/orchard/keys.rs +++ b/zebra-chain/src/orchard/keys.rs @@ -13,8 +13,11 @@ use halo2::{ }; use rand_core::{CryptoRng, RngCore}; -use crate::serialization::{ - serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, +use crate::{ + error::RandError, + serialization::{ + serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, + }, }; use super::sinsemilla::*; @@ -102,14 +105,16 @@ impl Diversifier { /// Generate a new `Diversifier`. /// /// - pub fn new(csprng: &mut T) -> Self + pub fn new(csprng: &mut T) -> Result where T: RngCore + CryptoRng, { let mut bytes = [0u8; 11]; - csprng.fill_bytes(&mut bytes); + csprng + .try_fill_bytes(&mut bytes) + .map_err(|_| RandError::FillBytes)?; - Self::from(bytes) + Ok(Self::from(bytes)) } } diff --git a/zebra-chain/src/orchard/note.rs b/zebra-chain/src/orchard/note.rs index 5eb78236b09..6ba07bd7f16 100644 --- a/zebra-chain/src/orchard/note.rs +++ b/zebra-chain/src/orchard/note.rs @@ -4,7 +4,10 @@ use group::{ff::PrimeField, GroupEncoding}; use halo2::pasta::pallas; use rand_core::{CryptoRng, RngCore}; -use crate::amount::{Amount, NonNegative}; +use crate::{ + amount::{Amount, NonNegative}, + error::RandError, +}; use super::{address::Address, sinsemilla::extract_p}; @@ -22,13 +25,15 @@ mod arbitrary; pub struct SeedRandomness(pub(crate) [u8; 32]); impl SeedRandomness { - pub fn new(csprng: &mut T) -> Self + pub fn new(csprng: &mut T) -> Result where T: RngCore + CryptoRng, { let mut bytes = [0u8; 32]; - csprng.fill_bytes(&mut bytes); - Self(bytes) + csprng + .try_fill_bytes(&mut bytes) + .map_err(|_| RandError::FillBytes)?; + Ok(Self(bytes)) } } @@ -57,14 +62,16 @@ impl From for Rho { } impl Rho { - pub fn new(csprng: &mut T) -> Self + pub fn new(csprng: &mut T) -> Result where T: RngCore + CryptoRng, { let mut bytes = [0u8; 32]; - csprng.fill_bytes(&mut bytes); + csprng + .try_fill_bytes(&mut bytes) + .map_err(|_| RandError::FillBytes)?; - Self(extract_p(pallas::Point::from_bytes(&bytes).unwrap())) + Ok(Self(extract_p(pallas::Point::from_bytes(&bytes).unwrap()))) } } @@ -108,15 +115,15 @@ impl Note { address: Address, value: Amount, nf_old: Nullifier, - ) -> Self + ) -> Result where T: RngCore + CryptoRng, { - Self { + Ok(Self { address, value, rho: nf_old.into(), - rseed: SeedRandomness::new(csprng), - } + rseed: SeedRandomness::new(csprng)?, + }) } } diff --git a/zebra-chain/src/sapling/commitment.rs b/zebra-chain/src/sapling/commitment.rs index 9e0c5983b2f..8f12ca0e8cf 100644 --- a/zebra-chain/src/sapling/commitment.rs +++ b/zebra-chain/src/sapling/commitment.rs @@ -12,6 +12,7 @@ use rand_core::{CryptoRng, RngCore}; use crate::{ amount::{Amount, NonNegative}, + error::{AffineError, CryptoError, RandError}, serialization::{ serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, }, @@ -34,14 +35,16 @@ use pedersen_hashes::*; /// trapdoor generators. /// /// -pub fn generate_trapdoor(csprng: &mut T) -> jubjub::Fr +pub fn generate_trapdoor(csprng: &mut T) -> Result where T: RngCore + CryptoRng, { let mut bytes = [0u8; 64]; - csprng.fill_bytes(&mut bytes); + csprng + .try_fill_bytes(&mut bytes) + .map_err(|_| RandError::FillBytes)?; // Fr::from_bytes_wide() reduces the input modulo r via Fr::from_u512() - jubjub::Fr::from_bytes_wide(&bytes) + Ok(jubjub::Fr::from_bytes_wide(&bytes)) } /// The randomness used in the Pedersen Hash for note commitment. @@ -104,7 +107,7 @@ impl NoteCommitment { diversifier: Diversifier, transmission_key: TransmissionKey, value: Amount, - ) -> Option<(CommitmentRandomness, Self)> + ) -> Result<(CommitmentRandomness, Self), CryptoError> where T: RngCore + CryptoRng, { @@ -120,11 +123,9 @@ impl NoteCommitment { // The `TryFrom` impls for the `jubjub::*Point`s handles // calling `DiversifyHash` implicitly. - let g_d_bytes: [u8; 32] = if let Ok(g_d) = jubjub::AffinePoint::try_from(diversifier) { - g_d.to_bytes() - } else { - return None; - }; + let g_d_bytes = jubjub::AffinePoint::try_from(diversifier) + .map_err(|_| CryptoError::Affine(AffineError::FromDiversifier))? + .to_bytes(); let pk_d_bytes = <[u8; 32]>::from(transmission_key); let v_bytes = value.to_bytes(); @@ -133,9 +134,9 @@ impl NoteCommitment { s.extend(pk_d_bytes); s.extend(v_bytes); - let rcm = CommitmentRandomness(generate_trapdoor(csprng)); + let rcm = CommitmentRandomness(generate_trapdoor(csprng)?); - Some(( + Ok(( rcm, NoteCommitment::from(windowed_pedersen_commitment(rcm.0, &s)), )) @@ -265,13 +266,13 @@ impl ValueCommitment { /// Generate a new _ValueCommitment_. /// /// - pub fn randomized(csprng: &mut T, value: Amount) -> Self + pub fn randomized(csprng: &mut T, value: Amount) -> Result where T: RngCore + CryptoRng, { - let rcv = generate_trapdoor(csprng); + let rcv = generate_trapdoor(csprng)?; - Self::new(rcv, value) + Ok(Self::new(rcv, value)) } /// Generate a new _ValueCommitment_ from an existing _rcv_ on a _value_. diff --git a/zebra-chain/src/sapling/keys.rs b/zebra-chain/src/sapling/keys.rs index fabe2e2970c..56d14d0d5ac 100644 --- a/zebra-chain/src/sapling/keys.rs +++ b/zebra-chain/src/sapling/keys.rs @@ -15,6 +15,7 @@ use std::{fmt, io}; use rand_core::{CryptoRng, RngCore}; use crate::{ + error::{RandError, DIVERSIFY_HASH_TRIES}, primitives::redjubjub::{self, SpendAuth}, serialization::{ serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, @@ -180,18 +181,21 @@ impl Diversifier { /// /// /// - pub fn new(csprng: &mut T) -> Self + pub fn new(csprng: &mut T) -> Result where T: RngCore + CryptoRng, { - loop { + for _ in 0..DIVERSIFY_HASH_TRIES { let mut bytes = [0u8; 11]; - csprng.fill_bytes(&mut bytes); + csprng + .try_fill_bytes(&mut bytes) + .map_err(|_| RandError::FillBytes)?; if diversify_hash(bytes).is_some() { - break Self(bytes); + return Ok(Self(bytes)); } } + Err(RandError::DiversifyHash) } } From 421c9b86d6cacd1de6c4a73332ab29a175998d07 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 10 Apr 2023 16:10:12 -0300 Subject: [PATCH 2/5] change error name --- zebra-chain/src/error.rs | 6 +++--- zebra-chain/src/sapling/commitment.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/zebra-chain/src/error.rs b/zebra-chain/src/error.rs index 858c345096c..2e6df882a4c 100644 --- a/zebra-chain/src/error.rs +++ b/zebra-chain/src/error.rs @@ -20,7 +20,7 @@ pub enum RandError { /// Errors related to `AffinePoint`. #[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] -pub enum AffineError { +pub enum AffinePointError { /// Error of `jubjub::AffinePoint::try_from`. #[error("failed to generate a AffinePoint from a diversifier")] FromDiversifier, @@ -32,7 +32,7 @@ pub enum CryptoError { /// Errors of type `RandError`. #[error("Random generation failure")] Rand(#[from] RandError), - /// Errors of type `AffineError`. + /// Errors of type `AffinePointError`. #[error("Affine point failure")] - Affine(#[from] AffineError), + Affine(#[from] AffinePointError), } diff --git a/zebra-chain/src/sapling/commitment.rs b/zebra-chain/src/sapling/commitment.rs index 8f12ca0e8cf..79f3528a44a 100644 --- a/zebra-chain/src/sapling/commitment.rs +++ b/zebra-chain/src/sapling/commitment.rs @@ -12,7 +12,7 @@ use rand_core::{CryptoRng, RngCore}; use crate::{ amount::{Amount, NonNegative}, - error::{AffineError, CryptoError, RandError}, + error::{AffinePointError, CryptoError, RandError}, serialization::{ serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, }, @@ -124,7 +124,7 @@ impl NoteCommitment { // calling `DiversifyHash` implicitly. let g_d_bytes = jubjub::AffinePoint::try_from(diversifier) - .map_err(|_| CryptoError::Affine(AffineError::FromDiversifier))? + .map_err(|_| CryptoError::Affine(AffinePointError::FromDiversifier))? .to_bytes(); let pk_d_bytes = <[u8; 32]>::from(transmission_key); From ed6596a1b8c591872ec8d731d343978d04a87a53 Mon Sep 17 00:00:00 2001 From: Deirdre Connolly Date: Tue, 18 Apr 2023 22:52:42 -0400 Subject: [PATCH 3/5] Error types oriented around the primary types we expose in the zebra-chain API --- zebra-chain/src/error.rs | 44 +++++++++++++++------------ zebra-chain/src/sapling/commitment.rs | 6 ++-- zebra-chain/src/sapling/keys.rs | 11 ++++--- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/zebra-chain/src/error.rs b/zebra-chain/src/error.rs index 2e6df882a4c..adcd3037e62 100644 --- a/zebra-chain/src/error.rs +++ b/zebra-chain/src/error.rs @@ -1,38 +1,42 @@ //! Errors that can occur inside any `zebra-chain` submodule. use thiserror::Error; -/// Number of times a `diversify_hash` will try to obtain a diversified base point. -pub const DIVERSIFY_HASH_TRIES: u8 = 5; - /// Errors related to random bytes generation. #[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] pub enum RandError { /// Error of the `try_fill_bytes` function. #[error("failed to generate a secure stream of random bytes")] FillBytes, - /// Error of the `diversify_hash` function. - #[error( - "failed to generate a diversified base point after {} tries", - DIVERSIFY_HASH_TRIES - )] - DiversifyHash, } -/// Errors related to `AffinePoint`. +/// An error type pertaining to note commitments. #[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] -pub enum AffinePointError { +pub enum NoteCommitmentError { + /// Errors of type `RandError`. + #[error("Randomness generation failure")] + InsufficientRandomness(#[from] RandError), /// Error of `jubjub::AffinePoint::try_from`. - #[error("failed to generate a AffinePoint from a diversifier")] - FromDiversifier, + #[error("failed to generate a sapling::NoteCommitment from a diversifier")] + InvalidDiversifier, +} + +/// An error type pertaining to key generation, parsing, modification, +/// randomization. +#[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] +pub enum KeyError { + /// Errors of type `RandError`. + #[error("Randomness generation failure")] + InsufficientRandomness(#[from] RandError), } -/// An error type that can have `RandError` or `AffineError`. +/// An error type pertaining to payment address generation, parsing, +/// modification, diversification. #[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] -pub enum CryptoError { +pub enum AddressError { /// Errors of type `RandError`. - #[error("Random generation failure")] - Rand(#[from] RandError), - /// Errors of type `AffinePointError`. - #[error("Affine point failure")] - Affine(#[from] AffinePointError), + #[error("Randomness generation failure")] + InsufficientRandomness(#[from] RandError), + /// Errors pertaining to diversifier generation. + #[error("Randomness did not hash into the Jubjub group for producing a new diversifier")] + DiversifierGenerationFailure, } diff --git a/zebra-chain/src/sapling/commitment.rs b/zebra-chain/src/sapling/commitment.rs index 79f3528a44a..65c789dc6ed 100644 --- a/zebra-chain/src/sapling/commitment.rs +++ b/zebra-chain/src/sapling/commitment.rs @@ -12,7 +12,7 @@ use rand_core::{CryptoRng, RngCore}; use crate::{ amount::{Amount, NonNegative}, - error::{AffinePointError, CryptoError, RandError}, + error::{NoteCommitmentError, RandError}, serialization::{ serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, }, @@ -107,7 +107,7 @@ impl NoteCommitment { diversifier: Diversifier, transmission_key: TransmissionKey, value: Amount, - ) -> Result<(CommitmentRandomness, Self), CryptoError> + ) -> Result<(CommitmentRandomness, Self), NoteCommitmentError> where T: RngCore + CryptoRng, { @@ -124,7 +124,7 @@ impl NoteCommitment { // calling `DiversifyHash` implicitly. let g_d_bytes = jubjub::AffinePoint::try_from(diversifier) - .map_err(|_| CryptoError::Affine(AffinePointError::FromDiversifier))? + .map_err(|_| NoteCommitmentError::InvalidDiversifier)? .to_bytes(); let pk_d_bytes = <[u8; 32]>::from(transmission_key); diff --git a/zebra-chain/src/sapling/keys.rs b/zebra-chain/src/sapling/keys.rs index 56d14d0d5ac..2037e368a12 100644 --- a/zebra-chain/src/sapling/keys.rs +++ b/zebra-chain/src/sapling/keys.rs @@ -15,7 +15,7 @@ use std::{fmt, io}; use rand_core::{CryptoRng, RngCore}; use crate::{ - error::{RandError, DIVERSIFY_HASH_TRIES}, + error::{AddressError, RandError}, primitives::redjubjub::{self, SpendAuth}, serialization::{ serde_helpers, ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize, @@ -181,21 +181,24 @@ impl Diversifier { /// /// /// - pub fn new(csprng: &mut T) -> Result + pub fn new(csprng: &mut T) -> Result where T: RngCore + CryptoRng, { + /// Number of times a `diversify_hash` will try to obtain a diversified base point. + const DIVERSIFY_HASH_TRIES: u8 = 2; + for _ in 0..DIVERSIFY_HASH_TRIES { let mut bytes = [0u8; 11]; csprng .try_fill_bytes(&mut bytes) - .map_err(|_| RandError::FillBytes)?; + .map_err(|_| AddressError::from(RandError::FillBytes))?; if diversify_hash(bytes).is_some() { return Ok(Self(bytes)); } } - Err(RandError::DiversifyHash) + Err(AddressError::DiversifierGenerationFailure) } } From 1c003f35d3c3d49bcf6e1c279a81963a0ed673ec Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 19 Apr 2023 13:06:04 +1000 Subject: [PATCH 4/5] Fix Ok spelling --- zebra-chain/src/orchard/commitment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-chain/src/orchard/commitment.rs b/zebra-chain/src/orchard/commitment.rs index b2394697f0a..4e69258c4e5 100644 --- a/zebra-chain/src/orchard/commitment.rs +++ b/zebra-chain/src/orchard/commitment.rs @@ -36,7 +36,7 @@ where .try_fill_bytes(&mut bytes) .map_err(|_| RandError::FillBytes)?; // pallas::Scalar::from_uniform_bytes() reduces the input modulo q_P under the hood. - ok(pallas::Scalar::from_uniform_bytes(&bytes)) + Ok(pallas::Scalar::from_uniform_bytes(&bytes)) } /// The randomness used in the Simsemilla hash for note commitment. From 47a3adb948b751f51fd459e9317028ee7f25512b Mon Sep 17 00:00:00 2001 From: Deirdre Connolly Date: Thu, 20 Apr 2023 01:22:54 -0400 Subject: [PATCH 5/5] orchard::note::new(): return NoteError if randomness produces invalid Pallas point --- zebra-chain/src/error.rs | 11 +++++++++++ zebra-chain/src/orchard/note.rs | 14 ++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/zebra-chain/src/error.rs b/zebra-chain/src/error.rs index adcd3037e62..a3182a21feb 100644 --- a/zebra-chain/src/error.rs +++ b/zebra-chain/src/error.rs @@ -9,6 +9,17 @@ pub enum RandError { FillBytes, } +/// An error type pertaining to shielded notes. +#[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] +pub enum NoteError { + /// Errors of type `RandError`. + #[error("Randomness generation failure")] + InsufficientRandomness(#[from] RandError), + /// Error of `pallas::Point::from_bytes()` for new rho randomness. + #[error("failed to generate an Orchard note's rho.")] + InvalidRho, +} + /// An error type pertaining to note commitments. #[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] pub enum NoteCommitmentError { diff --git a/zebra-chain/src/orchard/note.rs b/zebra-chain/src/orchard/note.rs index 6ba07bd7f16..8d46220ff86 100644 --- a/zebra-chain/src/orchard/note.rs +++ b/zebra-chain/src/orchard/note.rs @@ -6,7 +6,7 @@ use rand_core::{CryptoRng, RngCore}; use crate::{ amount::{Amount, NonNegative}, - error::RandError, + error::{NoteError, RandError}, }; use super::{address::Address, sinsemilla::extract_p}; @@ -62,16 +62,22 @@ impl From for Rho { } impl Rho { - pub fn new(csprng: &mut T) -> Result + pub fn new(csprng: &mut T) -> Result where T: RngCore + CryptoRng, { let mut bytes = [0u8; 32]; csprng .try_fill_bytes(&mut bytes) - .map_err(|_| RandError::FillBytes)?; + .map_err(|_| NoteError::from(RandError::FillBytes))?; + + let possible_point = pallas::Point::from_bytes(&bytes); - Ok(Self(extract_p(pallas::Point::from_bytes(&bytes).unwrap()))) + if possible_point.is_some().into() { + Ok(Self(extract_p(possible_point.unwrap()))) + } else { + Err(NoteError::InvalidRho) + } } }