Dedupe VerifyTransactionError into TransactionError

zebra-consensus/src/block.rs

@@ -28,10 +28,7 @@ use zebra_chain::{
 };
 use zebra_state as zs;
 
-use crate::{
-    error::*,
-    transaction::{self, VerifyTransactionError},
-};
+use crate::{error::*, transaction};
 use crate::{script, BoxError};
 
 mod check;
@@ -48,27 +45,33 @@ pub struct BlockVerifier<S> {
     transaction_verifier: transaction::Verifier<S>,
 }
 
+// TODO: dedupe with crate::error::BlockError
 #[non_exhaustive]
 #[derive(Debug, Error)]
 pub enum VerifyBlockError {
     #[error("unable to verify depth for block {hash} from chain state during block verification")]
     Depth { source: BoxError, hash: block::Hash },
+
     #[error(transparent)]
     Block {
         #[from]
         source: BlockError,
     },
+
     #[error(transparent)]
     Equihash {
         #[from]
         source: equihash::Error,
     },
+
     #[error(transparent)]
     Time(zebra_chain::block::BlockTimeError),
+
     #[error("unable to commit block after semantic verification")]
     Commit(#[source] BoxError),
+
     #[error("invalid transaction")]
-    Transaction(#[source] VerifyTransactionError),
+    Transaction(#[source] TransactionError),
 }
 
 impl<S> BlockVerifier<S>

zebra-consensus/src/error.rs

@@ -7,6 +7,10 @@
 
 use thiserror::Error;
 
+use zebra_chain::primitives::ed25519;
+
+use crate::BoxError;
+
 #[derive(Error, Debug, PartialEq)]
 pub enum SubsidyError {
     #[error("no coinbase transaction in block")]
@@ -24,8 +28,45 @@ pub enum TransactionError {
     #[error("coinbase input found in non-coinbase transaction")]
     CoinbaseInputFound,
 
+    #[error("coinbase transaction MUST NOT have any JoinSplit descriptions or Spend descriptions")]
+    CoinbaseHasJoinSplitOrSpend,
+
     #[error("coinbase transaction failed subsidy validation")]
     Subsidy(#[from] SubsidyError),
+
+    #[error("transaction version number MUST be >= 4")]
+    WrongVersion,
+
+    #[error("at least one of tx_in_count, nShieldedSpend, and nJoinSplit MUST be nonzero")]
+    NoTransfer,
+
+    #[error("if there are no Spends or Outputs, the value balance MUST be 0.")]
+    BadBalance,
+
+    /// Could not verify a transparent script
+    Script(#[from] zebra_script::Error),
+
+    // XXX change this when we align groth16 verifier errors with bellman
+    // and add a from annotation when the error type is more precise
+    #[error("spend proof MUST be valid given a primary input formed from the other fields except spendAuthSig")]
+    Groth16(BoxError),
+
+    #[error(
+        "joinSplitSig MUST represent a valid signature under joinSplitPubKey of dataToBeSigned"
+    )]
+    Ed25519(#[from] ed25519::Error),
+
+    #[error("bindingSig MUST represent a valid signature under the transaction binding validating key bvk of SigHash")]
+    RedJubjub(redjubjub::Error),
+}
+
+impl From<BoxError> for TransactionError {
+    fn from(err: BoxError) -> Self {
+        match err.downcast::<redjubjub::Error>() {
+            Ok(e) => TransactionError::RedJubjub(*e),
+            Err(e) => TransactionError::Internal(e),
+        }
+    }
 }
 
 impl From<SubsidyError> for BlockError {

zebra-consensus/src/transaction.rs

@@ -10,18 +10,17 @@ use futures::{
     stream::{FuturesUnordered, StreamExt},
     FutureExt,
 };
-use thiserror::Error;
+
 use tower::{Service, ServiceExt};
 
 use zebra_chain::{
     parameters::NetworkUpgrade,
-    primitives::{ed25519, redjubjub},
     transaction::{self, HashType, Transaction},
 };
 
 use zebra_state as zs;
 
-use crate::{script, BoxError};
+use crate::{error::TransactionError, script, BoxError};
 
 mod check;
 
@@ -45,40 +44,6 @@ where
     }
 }
 
-#[non_exhaustive]
-#[derive(Debug, Display, Error)]
-pub enum VerifyTransactionError {
-    /// Only V4 and later transactions can be verified.
-    WrongVersion,
-    /// A transaction MUST move money around.
-    NoTransfer,
-    /// The balance of money moving around doesn't compute.
-    BadBalance,
-    /// Violation of coinbase rules.
-    Coinbase,
-    /// Could not verify a transparent script
-    Script(#[from] zebra_script::Error),
-    /// Could not verify a Groth16 proof of a JoinSplit/Spend/Output description
-    // XXX change this when we align groth16 verifier errors with bellman
-    // and add a from annotation when the error type is more precise
-    Groth16(BoxError),
-    /// Could not verify a Ed25519 signature with JoinSplitData
-    Ed25519(#[from] ed25519::Error),
-    /// Could not verify a RedJubjub signature with ShieldedData
-    RedJubjub(redjubjub::Error),
-    /// An error that arises from implementation details of the verification service
-    Internal(BoxError),
-}
-
-impl From<BoxError> for VerifyTransactionError {
-    fn from(err: BoxError) -> Self {
-        match err.downcast::<redjubjub::Error>() {
-            Ok(e) => VerifyTransactionError::RedJubjub(*e),
-            Err(e) => VerifyTransactionError::Internal(e),
-        }
-    }
-}
-
 /// Specifies whether a transaction should be verified as part of a block or as
 /// part of the mempool.
 ///
@@ -98,7 +63,7 @@ where
     ZS::Future: Send + 'static,
 {
     type Response = transaction::Hash;
-    type Error = VerifyTransactionError;
+    type Error = TransactionError;
     type Future =
         Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static>>;
 
@@ -127,7 +92,7 @@ where
         async move {
             match &*tx {
                 Transaction::V1 { .. } | Transaction::V2 { .. } | Transaction::V3 { .. } => {
-                    Err(VerifyTransactionError::WrongVersion)
+                    Err(TransactionError::WrongVersion)
                 }
                 Transaction::V4 {
                     inputs,

zebra-consensus/src/transaction/check.rs

@@ -6,7 +6,7 @@ use zebra_chain::{
     transaction::{JoinSplitData, ShieldedData, Transaction},
 };
 
-use crate::transaction::VerifyTransactionError;
+use crate::error::TransactionError;
 
 /// Validate the JoinSplit binding signature.
 ///
@@ -15,17 +15,17 @@ use crate::transaction::VerifyTransactionError;
 pub fn validate_joinsplit_sig(
     joinsplit_data: &JoinSplitData<Groth16Proof>,
     sighash: &[u8],
-) -> Result<(), VerifyTransactionError> {
+) -> Result<(), TransactionError> {
     ed25519::VerificationKey::try_from(joinsplit_data.pub_key)
         .and_then(|vk| vk.verify(&joinsplit_data.sig, sighash))
-        .map_err(VerifyTransactionError::Ed25519)
+        .map_err(TransactionError::Ed25519)
 }
 
 /// Check that at least one of tx_in_count, nShieldedSpend, and nJoinSplit MUST
 /// be non-zero.
 ///
 /// https://zips.z.cash/protocol/canopy.pdf#txnencodingandconsensus
-pub fn some_money_is_spent(tx: &Transaction) -> Result<(), VerifyTransactionError> {
+pub fn some_money_is_spent(tx: &Transaction) -> Result<(), TransactionError> {
     match tx {
         Transaction::V4 {
             inputs,
@@ -39,10 +39,10 @@ pub fn some_money_is_spent(tx: &Transaction) -> Result<(), VerifyTransactionErro
             {
                 Ok(())
             } else {
-                Err(VerifyTransactionError::NoTransfer)
+                Err(TransactionError::NoTransfer)
             }
         }
-        _ => Err(VerifyTransactionError::WrongVersion),
+        _ => Err(TransactionError::WrongVersion),
     }
 }
 
@@ -55,16 +55,16 @@ pub fn some_money_is_spent(tx: &Transaction) -> Result<(), VerifyTransactionErro
 /// https://zips.z.cash/protocol/canopy.pdf#consensusfrombitcoin
 pub fn any_coinbase_inputs_no_transparent_outputs(
     tx: &Transaction,
-) -> Result<(), VerifyTransactionError> {
+) -> Result<(), TransactionError> {
     match tx {
         Transaction::V4 { outputs, .. } => {
             if !tx.contains_coinbase_input() || !outputs.is_empty() {
                 Ok(())
             } else {
-                Err(VerifyTransactionError::NoTransfer)
+                Err(TransactionError::NoTransfer)
             }
         }
-        _ => Err(VerifyTransactionError::WrongVersion),
+        _ => Err(TransactionError::WrongVersion),
     }
 }
 
@@ -74,20 +74,20 @@ pub fn any_coinbase_inputs_no_transparent_outputs(
 pub fn shielded_balances_match(
     shielded_data: &ShieldedData,
     value_balance: Amount,
-) -> Result<(), VerifyTransactionError> {
+) -> Result<(), TransactionError> {
     if (shielded_data.spends().count() + shielded_data.outputs().count() != 0)
         || i64::from(value_balance) == 0
     {
         Ok(())
     } else {
-        Err(VerifyTransactionError::BadBalance)
+        Err(TransactionError::BadBalance)
     }
 }
 
 /// Check that a coinbase tx does not have any JoinSplit or Spend descriptions.
 ///
 /// https://zips.z.cash/protocol/canopy.pdf#consensusfrombitcoin
-pub fn coinbase_tx_does_not_spend_shielded(tx: &Transaction) -> Result<(), VerifyTransactionError> {
+pub fn coinbase_tx_does_not_spend_shielded(tx: &Transaction) -> Result<(), TransactionError> {
     match tx {
         Transaction::V4 {
             joinsplit_data: Some(joinsplit_data),
@@ -99,9 +99,9 @@ pub fn coinbase_tx_does_not_spend_shielded(tx: &Transaction) -> Result<(), Verif
             {
                 Ok(())
             } else {
-                Err(VerifyTransactionError::Coinbase)
+                Err(TransactionError::CoinbaseHasJoinSplitOrSpend)
             }
         }
-        _ => Err(VerifyTransactionError::WrongVersion),
+        _ => Err(TransactionError::WrongVersion),
     }
 }