(Some) semantic transaction verification checks

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,
+
+    #[error("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,
+
+    #[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) => panic!(e),
+        }
+    }
 }
 
 impl From<SubsidyError> for BlockError {

zebra-consensus/src/transaction.rs

@@ -5,23 +5,23 @@ use std::{
     task::{Context, Poll},
 };
 
-use displaydoc::Display;
 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;
 
 /// Asynchronous transaction verification.
 #[derive(Debug, Clone)]
@@ -43,34 +43,6 @@ where
     }
 }
 
-#[non_exhaustive]
-#[derive(Debug, Display, Error)]
-pub enum VerifyTransactionError {
-    /// Only V4 and later transactions can be verified.
-    WrongVersion,
-    /// 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.
 ///
@@ -90,7 +62,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>>;
 
@@ -119,7 +91,7 @@ where
         async move {
             match &*tx {
                 Transaction::V1 { .. } | Transaction::V2 { .. } | Transaction::V3 { .. } => {
-                    Err(VerifyTransactionError::WrongVersion)
+                    Err(TransactionError::WrongVersion)
                 }
                 Transaction::V4 {
                     inputs,
@@ -141,6 +113,7 @@ where
                     #[allow(clippy::if_same_then_else)] // delete when filled in
                     if tx.is_coinbase() {
                         // do something special for coinbase transactions
+                        check::coinbase_tx_does_not_spend_shielded(&tx)?;
                     } else {
                         // otherwise, check no coinbase inputs
                         // feed all of the inputs to the script verifier
@@ -154,27 +127,28 @@ where
                         }
                     }
 
+                    check::some_money_is_spent(&tx)?;
+                    check::any_coinbase_inputs_no_transparent_outputs(&tx)?;
+
                     let sighash = tx.sighash(
                         NetworkUpgrade::Sapling, // TODO: pass this in
                         HashType::ALL,           // TODO: check these
                         None,                    // TODO: check these
                     );
 
-                    if let Some(_joinsplit_data) = joinsplit_data {
+                    if let Some(joinsplit_data) = joinsplit_data {
                         // XXX create a method on JoinSplitData
                         // that prepares groth16::Items with the correct proofs
                         // and proof inputs, handling interstitial treestates
                         // correctly.
 
                         // Then, pass those items to self.joinsplit to verify them.
 
-                        // XXX refactor this into a nicely named check function
-                        // ed25519::VerificationKey::try_from(joinsplit_data.pub_key)
-                        //     .and_then(|vk| vk.verify(&joinsplit_data.sig, sighash))
-                        //     .map_err(VerifyTransactionError::Ed25519)
+                        check::validate_joinsplit_sig(joinsplit_data, sighash.as_bytes())?;
                     }
 
                     if let Some(shielded_data) = shielded_data {
+                        check::shielded_balances_match(&shielded_data, *value_balance)?;
                         for spend in shielded_data.spends() {
                             // TODO: check that spend.cv and spend.rk are NOT of small
                             // order.

zebra-consensus/src/transaction/check.rs

@@ -0,0 +1,107 @@
+use std::convert::TryFrom;
+
+use zebra_chain::{
+    amount::Amount,
+    primitives::{ed25519, Groth16Proof},
+    transaction::{JoinSplitData, ShieldedData, Transaction},
+};
+
+use crate::error::TransactionError;
+
+/// Validate the JoinSplit binding signature.
+///
+/// https://zips.z.cash/protocol/canopy.pdf#sproutnonmalleability
+/// https://zips.z.cash/protocol/canopy.pdf#txnencodingandconsensus
+pub fn validate_joinsplit_sig(
+    joinsplit_data: &JoinSplitData<Groth16Proof>,
+    sighash: &[u8],
+) -> Result<(), TransactionError> {
+    ed25519::VerificationKey::try_from(joinsplit_data.pub_key)
+        .and_then(|vk| vk.verify(&joinsplit_data.sig, sighash))
+        .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<(), TransactionError> {
+    match tx {
+        Transaction::V4 {
+            inputs,
+            joinsplit_data: Some(joinsplit_data),
+            shielded_data: Some(shielded_data),
+            ..
+        } => {
+            if !inputs.is_empty()
+                || joinsplit_data.joinsplits().count() > 0
+                || shielded_data.spends().count() > 0
+            {
+                Ok(())
+            } else {
+                Err(TransactionError::NoTransfer)
+            }
+        }
+        _ => Err(TransactionError::WrongVersion),
+    }
+}
+
+/// Check that a transaction with one or more transparent inputs from coinbase
+/// transactions has no transparent outputs.
+///
+/// Note that inputs from coinbase transactions include Founders’ Reward
+/// outputs.
+///
+/// https://zips.z.cash/protocol/canopy.pdf#consensusfrombitcoin
+pub fn any_coinbase_inputs_no_transparent_outputs(
+    tx: &Transaction,
+) -> Result<(), TransactionError> {
+    match tx {
+        Transaction::V4 { outputs, .. } => {
+            if !tx.contains_coinbase_input() || !outputs.is_empty() {
+                Ok(())
+            } else {
+                Err(TransactionError::NoTransfer)
+            }
+        }
+        _ => Err(TransactionError::WrongVersion),
+    }
+}
+
+/// Check that if there are no Spends or Outputs, that valueBalance is also 0.
+///
+/// https://zips.z.cash/protocol/canopy.pdf#consensusfrombitcoin
+pub fn shielded_balances_match(
+    shielded_data: &ShieldedData,
+    value_balance: Amount,
+) -> Result<(), TransactionError> {
+    if (shielded_data.spends().count() + shielded_data.outputs().count() != 0)
+        || i64::from(value_balance) == 0
+    {
+        Ok(())
+    } else {
+        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<(), TransactionError> {
+    match tx {
+        Transaction::V4 {
+            joinsplit_data: Some(joinsplit_data),
+            shielded_data: Some(shielded_data),
+            ..
+        } => {
+            if !tx.is_coinbase()
+                || (joinsplit_data.joinsplits().count() == 0 && shielded_data.spends().count() == 0)
+            {
+                Ok(())
+            } else {
+                Err(TransactionError::CoinbaseHasJoinSplitOrSpend)
+            }
+        }
+        _ => Err(TransactionError::WrongVersion),
+    }
+}

zebra-script/src/lib.rs

@@ -19,7 +19,7 @@ use zebra_chain::{
     transparent,
 };
 
-#[derive(Debug, Display, Error)]
+#[derive(Debug, Display, Error, PartialEq)]
 #[non_exhaustive]
 /// An Error type representing the error codes returned from zcash_script.
 pub enum Error {