ZIP-211: Validate Disabling Addition of New Value to the Sprout Value…

… Pool (#2399)

* add disabled sprout pool check

* change method name

* change error name

* fix typo

* make the success test case in other tx than the coinbase

* use new `height` method instead of deriving `PartialOrd` in `NetworkUpgrade`

* move check of network upgrade into function, rename, docs

* increase test coverage

* fix comment

zebra-chain/src/transaction.rs

@@ -22,7 +22,7 @@ pub use sapling::FieldNotPresent;
 pub use sighash::HashType;
 
 use crate::{
-    block, orchard,
+    amount, block, orchard,
     parameters::NetworkUpgrade,
     primitives::{Bctv14Proof, Groth16Proof},
     sapling, sprout, transparent,
@@ -294,6 +294,54 @@ impl Transaction {
         }
     }
 
+    /// Returns the `vpub_old` fields from `JoinSplit`s in this transaction, regardless of version.
+    ///
+    /// This value is removed from the transparent value pool of this transaction, and added to the
+    /// sprout value pool.
+    pub fn sprout_pool_added_values(
+        &self,
+    ) -> Box<dyn Iterator<Item = &amount::Amount<amount::NonNegative>> + '_> {
+        match self {
+            // JoinSplits with Bctv14 Proofs
+            Transaction::V2 {
+                joinsplit_data: Some(joinsplit_data),
+                ..
+            }
+            | Transaction::V3 {
+                joinsplit_data: Some(joinsplit_data),
+                ..
+            } => Box::new(
+                joinsplit_data
+                    .joinsplits()
+                    .map(|joinsplit| &joinsplit.vpub_old),
+            ),
+            // JoinSplits with Groth Proofs
+            Transaction::V4 {
+                joinsplit_data: Some(joinsplit_data),
+                ..
+            } => Box::new(
+                joinsplit_data
+                    .joinsplits()
+                    .map(|joinsplit| &joinsplit.vpub_old),
+            ),
+            // No JoinSplits
+            Transaction::V1 { .. }
+            | Transaction::V2 {
+                joinsplit_data: None,
+                ..
+            }
+            | Transaction::V3 {
+                joinsplit_data: None,
+                ..
+            }
+            | Transaction::V4 {
+                joinsplit_data: None,
+                ..
+            }
+            | Transaction::V5 { .. } => Box::new(std::iter::empty()),
+        }
+    }
+
     /// Access the sprout::Nullifiers in this transaction, regardless of version.
     pub fn sprout_nullifiers(&self) -> Box<dyn Iterator<Item = &sprout::Nullifier> + '_> {
         // This function returns a boxed iterator because the different

zebra-consensus/src/error.rs

@@ -81,6 +81,9 @@ pub enum TransactionError {
     // temporary error type until #1186 is fixed
     #[error("Downcast from BoxError to redjubjub::Error failed")]
     InternalDowncastError(String),
+
+    #[error("adding to the sprout pool is disabled after Canopy")]
+    DisabledAddToSproutPool,
 }
 
 impl From<BoxError> for TransactionError {

zebra-consensus/src/transaction.rs

@@ -174,6 +174,10 @@ where
                 check::coinbase_tx_no_prevout_joinsplit_spend(&tx)?;
             }
 
+            // [Canopy onward]: `vpub_old` MUST be zero.
+            // https://zips.z.cash/protocol/protocol.pdf#joinsplitdesc
+            check::disabled_add_to_sprout_pool(&tx, req.height(), network)?;
+
             // "The consensus rules applied to valueBalance, vShieldedOutput, and bindingSig
             // in non-coinbase transactions MUST also be applied to coinbase transactions."
             //

zebra-consensus/src/transaction/check.rs

@@ -3,13 +3,18 @@
 //! Code in this file can freely assume that no pre-V4 transactions are present.
 
 use zebra_chain::{
+    amount::{Amount, NonNegative},
+    block::Height,
     orchard::Flags,
+    parameters::{Network, NetworkUpgrade},
     sapling::{Output, PerSpendAnchor, Spend},
     transaction::Transaction,
 };
 
 use crate::error::TransactionError;
 
+use std::convert::TryFrom;
+
 /// Checks that the transaction has inputs and outputs.
 ///
 /// For `Transaction::V4`:
@@ -119,3 +124,33 @@ pub fn output_cv_epk_not_small_order(output: &Output) -> Result<(), TransactionE
         Ok(())
     }
 }
+
+/// Check if a transaction is adding to the sprout pool after Canopy
+/// network upgrade given a block height and a network.
+///
+/// https://zips.z.cash/zip-0211
+/// https://zips.z.cash/protocol/protocol.pdf#joinsplitdesc
+pub fn disabled_add_to_sprout_pool(
+    tx: &Transaction,
+    height: Height,
+    network: Network,
+) -> Result<(), TransactionError> {
+    let canopy_activation_height = NetworkUpgrade::Canopy
+        .activation_height(network)
+        .expect("Canopy activation height must be present for both networks");
+
+    // [Canopy onward]: `vpub_old` MUST be zero.
+    // https://zips.z.cash/protocol/protocol.pdf#joinsplitdesc
+    if height >= canopy_activation_height {
+        let zero = Amount::<NonNegative>::try_from(0).expect("an amount of 0 is always valid");
+
+        let tx_sprout_pool = tx.sprout_pool_added_values();
+        for vpub_old in tx_sprout_pool {
+            if *vpub_old != zero {
+                return Err(TransactionError::DisabledAddToSproutPool);
+            }
+        }
+    }
+
+    Ok(())
+}

zebra-consensus/src/transaction/tests.rs

@@ -3,7 +3,7 @@ use std::{collections::HashMap, convert::TryFrom, convert::TryInto, sync::Arc};
 use tower::{service_fn, ServiceExt};
 
 use zebra_chain::{
-    amount::Amount,
+    amount::{Amount, NonNegative},
     block, orchard,
     parameters::{Network, NetworkUpgrade},
     primitives::{ed25519, x25519, Groth16Proof},
@@ -929,3 +929,61 @@ fn mock_sprout_join_split_data() -> (JoinSplitData<Groth16Proof>, ed25519::Signi
 
     (joinsplit_data, signing_key)
 }
+
+#[test]
+fn add_to_sprout_pool_after_nu() {
+    zebra_test::init();
+
+    // get a block that we know it haves a transaction with `vpub_old` field greater than 0.
+    let block: Arc<_> = zebra_chain::block::Block::zcash_deserialize(
+        &zebra_test::vectors::BLOCK_MAINNET_419199_BYTES[..],
+    )
+    .unwrap()
+    .into();
+
+    // create a block height at canopy activation.
+    let network = Network::Mainnet;
+    let block_height = NetworkUpgrade::Canopy.activation_height(network).unwrap();
+
+    // create a zero amount.
+    let zero = Amount::<NonNegative>::try_from(0).expect("an amount of 0 is always valid");
+
+    // the coinbase transaction should pass the check.
+    assert_eq!(
+        check::disabled_add_to_sprout_pool(&block.transactions[0], block_height, network),
+        Ok(())
+    );
+
+    // the 2nd transaction has no joinsplits, should pass the check.
+    assert_eq!(block.transactions[1].joinsplit_count(), 0);
+    assert_eq!(
+        check::disabled_add_to_sprout_pool(&block.transactions[1], block_height, network),
+        Ok(())
+    );
+
+    // the 5th transaction has joinsplits and the `vpub_old` cumulative is greater than 0,
+    // should fail the check.
+    assert!(block.transactions[4].joinsplit_count() > 0);
+    let vpub_old: Amount<NonNegative> = block.transactions[4]
+        .sprout_pool_added_values()
+        .fold(zero, |acc, &x| (acc + x).unwrap());
+    assert!(vpub_old > zero);
+
+    assert_eq!(
+        check::disabled_add_to_sprout_pool(&block.transactions[3], block_height, network),
+        Err(TransactionError::DisabledAddToSproutPool)
+    );
+
+    // the 8th transaction has joinsplits and the `vpub_old` cumulative is 0,
+    // should pass the check.
+    assert!(block.transactions[7].joinsplit_count() > 0);
+    let vpub_old: Amount<NonNegative> = block.transactions[7]
+        .sprout_pool_added_values()
+        .fold(zero, |acc, &x| (acc + x).unwrap());
+    assert_eq!(vpub_old, zero);
+
+    assert_eq!(
+        check::disabled_add_to_sprout_pool(&block.transactions[7], block_height, network),
+        Ok(())
+    );
+}