Validate miner transaction fees (#3067)

* validate consensus rule: negative fee not allowed

* fix a test TODO

* fix imports

* move import back

* fix panic text

* join consensus rule check code

* match assertion better in tests

* fix test

* fix consensus rule validation

* remove panics

* Delete a TODO

Co-authored-by: teor <teor@riseup.net>

zebra-chain/src/transaction.rs

@@ -1157,7 +1157,7 @@ impl Transaction {
     /// and added to sapling pool.
     ///
     /// https://zebra.zfnd.org/dev/rfcs/0012-value-pools.html#definitions
-    fn sapling_value_balance(&self) -> ValueBalance<NegativeAllowed> {
+    pub fn sapling_value_balance(&self) -> ValueBalance<NegativeAllowed> {
         let sapling_value_balance = match self {
             Transaction::V4 {
                 sapling_shielded_data: Some(sapling_shielded_data),
@@ -1224,7 +1224,7 @@ impl Transaction {
     /// and added to orchard pool.
     ///
     /// https://zebra.zfnd.org/dev/rfcs/0012-value-pools.html#definitions
-    fn orchard_value_balance(&self) -> ValueBalance<NegativeAllowed> {
+    pub fn orchard_value_balance(&self) -> ValueBalance<NegativeAllowed> {
         let orchard_value_balance = self
             .orchard_shielded_data()
             .map(|shielded_data| shielded_data.value_balance)

zebra-consensus/src/block.rs

@@ -259,13 +259,13 @@ where
                 })?;
             }
 
-            // TODO: check miner subsidy and miner fees (#1162)
+            let block_miner_fees =
-            let _block_miner_fees =
                 block_miner_fees.map_err(|amount_error| BlockError::SummingMinerFees {
                     height,
                     hash,
                     source: amount_error,
                 })?;
+            check::miner_fees_are_valid(&block, network, block_miner_fees)?;
 
             // Finally, submit the block for contextual verification.
             let new_outputs = Arc::try_unwrap(known_utxos)

zebra-consensus/src/block/check.rs

@@ -4,6 +4,7 @@ use chrono::{DateTime, Utc};
 use std::collections::HashSet;
 
 use zebra_chain::{
+    amount::{Amount, Error as AmountError, NonNegative},
     block::{Block, Hash, Header, Height},
     parameters::{Network, NetworkUpgrade},
     transaction,
@@ -94,21 +95,19 @@ pub fn equihash_solution_is_valid(header: &Header) -> Result<(), equihash::Error
     header.solution.check(header)
 }
 
-/// Returns `Ok(())` if the block subsidy and miner fees in `block` are valid for `network`
+/// Returns `Ok(())` if the block subsidy in `block` is valid for `network`
 ///
 /// [3.9]: https://zips.z.cash/protocol/protocol.pdf#subsidyconcepts
 pub fn subsidy_is_valid(block: &Block, network: Network) -> Result<(), BlockError> {
     let height = block.coinbase_height().ok_or(SubsidyError::NoCoinbase)?;
     let coinbase = block.transactions.get(0).ok_or(SubsidyError::NoCoinbase)?;
 
+    // Validate founders reward and funding streams
     let halving_div = subsidy::general::halving_divisor(height, network);
     let canopy_activation_height = NetworkUpgrade::Canopy
         .activation_height(network)
         .expect("Canopy activation height is known");
 
-    // TODO: the sum of the coinbase transaction outputs must be less than or equal to the block subsidy plus transaction fees
-
-    // Check founders reward and funding streams
     if height < SLOW_START_INTERVAL {
         unreachable!(
             "unsupported block height: callers should handle blocks below {:?}",
@@ -161,6 +160,45 @@ pub fn subsidy_is_valid(block: &Block, network: Network) -> Result<(), BlockErro
     }
 }
 
+/// Returns `Ok(())` if the miner fees consensus rule is valid.
+///
+/// [7.1.2]: https://zips.z.cash/protocol/protocol.pdf#txnconsensus
+pub fn miner_fees_are_valid(
+    block: &Block,
+    network: Network,
+    block_miner_fees: Amount<NonNegative>,
+) -> Result<(), BlockError> {
+    let height = block.coinbase_height().ok_or(SubsidyError::NoCoinbase)?;
+    let coinbase = block.transactions.get(0).ok_or(SubsidyError::NoCoinbase)?;
+
+    let transparent_value_balance: Amount = subsidy::general::output_amounts(coinbase)
+        .iter()
+        .sum::<Result<Amount<NonNegative>, AmountError>>()
+        .map_err(|_| SubsidyError::SumOverflow)?
+        .constrain()
+        .expect("positive value always fit in `NegativeAllowed`");
+    let sapling_value_balance = coinbase.sapling_value_balance().sapling_amount();
+    let orchard_value_balance = coinbase.orchard_value_balance().orchard_amount();
+
+    let block_subsidy = subsidy::general::block_subsidy(height, network)
+        .expect("a valid block subsidy for this height and network");
+
+    // Consensus rule: The total value in zatoshi of transparent outputs from a
+    // coinbase transaction, minus vbalanceSapling, minus vbalanceOrchard, MUST NOT
+    // be greater than the value in zatoshi of block subsidy plus the transaction fees
+    // paid by transactions in this block.
+    // https://zips.z.cash/protocol/protocol.pdf#txnconsensus
+    let left = (transparent_value_balance - sapling_value_balance - orchard_value_balance)
+        .map_err(|_| SubsidyError::SumOverflow)?;
+    let right = (block_subsidy + block_miner_fees).map_err(|_| SubsidyError::SumOverflow)?;
+
+    if left > right {
+        return Err(SubsidyError::InvalidMinerFees)?;
+    }
+
+    Ok(())
+}
+
 /// Returns `Ok(())` if `header.time` is less than or equal to
 /// 2 hours in the future, according to the node's local clock (`now`).
 ///

zebra-consensus/src/block/subsidy/funding_streams.rs

@@ -53,7 +53,7 @@ pub fn funding_stream_values(
 /// as described in [protocol specification §7.10][7.10]
 ///
 /// [7.10]: https://zips.z.cash/protocol/protocol.pdf#fundingstreams
-fn height_for_first_halving(network: Network) -> Height {
+pub fn height_for_first_halving(network: Network) -> Height {
     // First halving on Mainnet is at Canopy
     // while in Testnet is at block constant height of `1_116_000`
     // https://zips.z.cash/protocol/protocol.pdf#zip214fundingstreams

zebra-consensus/src/block/subsidy/general.rs

@@ -118,6 +118,11 @@ mod test {
     use super::*;
     use color_eyre::Report;
 
+    use crate::block::subsidy::{
+        founders_reward::founders_reward,
+        funding_streams::{funding_stream_values, height_for_first_halving},
+    };
+
     #[test]
     fn halving_test() -> Result<(), Report> {
         zebra_test::init();
@@ -307,8 +312,8 @@ mod test {
     }
 
     fn miner_subsidy_for_network(network: Network) -> Result<(), Report> {
-        use crate::block::subsidy::founders_reward::founders_reward;
         let blossom_height = Blossom.activation_height(network).unwrap();
+        let first_halving_height = height_for_first_halving(network);
 
         // Miner reward before Blossom is 80% of the total block reward
         // 80*12.5/100 = 10 ZEC
@@ -330,8 +335,17 @@ mod test {
             miner_subsidy(blossom_height, network, Some(founders_amount))
         );
 
-        // TODO: After first halving, miner will get 2.5 ZEC per mined block
+        // After first halving, miner will get 2.5 ZEC per mined block (not counting fees)
-        // but we need funding streams code to get this number
+        let funding_stream_values = funding_stream_values(first_halving_height, network)?
+            .iter()
+            .map(|row| row.1)
+            .sum::<Result<Amount<NonNegative>, Error>>()
+            .unwrap();
+
+        assert_eq!(
+            Amount::try_from(250_000_000),
+            miner_subsidy(first_halving_height, network, Some(funding_stream_values))
+        );
 
         // TODO: After second halving, there will be no funding streams, and
         // miners will get all the block reward

zebra-consensus/src/block/tests.rs

@@ -1,6 +1,6 @@
 //! Tests for block verification
 
-use std::sync::Arc;
+use std::{convert::TryFrom, sync::Arc};
 
 use chrono::Utc;
 use color_eyre::eyre::{eyre, Report};
@@ -8,6 +8,7 @@ use once_cell::sync::Lazy;
 use tower::{buffer::Buffer, util::BoxService};
 
 use zebra_chain::{
+    amount::{Amount, MAX_MONEY},
     block::{
         self,
         tests::generate::{large_multi_transaction_block, large_single_transaction_block},
@@ -196,7 +197,6 @@ fn difficulty_is_valid_for_network(network: Network) -> Result<(), Report> {
 #[test]
 fn difficulty_validation_failure() -> Result<(), Report> {
     zebra_test::init();
-    use crate::error::*;
 
     // Get a block in the mainnet, and mangle its difficulty field
     let block =
@@ -306,8 +306,6 @@ fn subsidy_is_valid_for_network(network: Network) -> Result<(), Report> {
 #[test]
 fn coinbase_validation_failure() -> Result<(), Report> {
     zebra_test::init();
-    use crate::error::*;
-
     let network = Network::Mainnet;
 
     // Get a block in the mainnet that is inside the founders reward period,
@@ -379,9 +377,6 @@ fn coinbase_validation_failure() -> Result<(), Report> {
 #[test]
 fn founders_reward_validation_failure() -> Result<(), Report> {
     zebra_test::init();
-    use crate::error::*;
-    use zebra_chain::transaction::Transaction;
-
     let network = Network::Mainnet;
 
     // Get a block in the mainnet that is inside the founders reward period.
@@ -393,12 +388,16 @@ fn founders_reward_validation_failure() -> Result<(), Report> {
     let tx = block
         .transactions
         .get(0)
-        .map(|transaction| Transaction::V3 {
+        .map(|transaction| {
-            inputs: transaction.inputs().to_vec(),
+            let mut output = transaction.outputs()[0].clone();
-            outputs: vec![transaction.outputs()[0].clone()],
+            output.value = Amount::try_from(i32::MAX).unwrap();
-            lock_time: transaction.lock_time().unwrap_or_else(LockTime::unlocked),
+            Transaction::V3 {
-            expiry_height: Height(0),
+                inputs: transaction.inputs().to_vec(),
-            joinsplit_data: None,
+                outputs: vec![output],
+                lock_time: transaction.lock_time().unwrap_or_else(LockTime::unlocked),
+                expiry_height: Height(0),
+                joinsplit_data: None,
+            }
         })
         .unwrap();
 
@@ -410,10 +409,11 @@ fn founders_reward_validation_failure() -> Result<(), Report> {
     };
 
     // Validate it
-    let result = check::subsidy_is_valid(&block, network).unwrap_err();
+    let result = check::subsidy_is_valid(&block, network);
-    let expected = BlockError::Transaction(TransactionError::Subsidy(
+    let expected = Err(BlockError::Transaction(TransactionError::Subsidy(
         SubsidyError::FoundersRewardNotFound,
-    ));
+    )));
+
     assert_eq!(expected, result);
 
     Ok(())
@@ -451,9 +451,6 @@ fn funding_stream_validation_for_network(network: Network) -> Result<(), Report>
 #[test]
 fn funding_stream_validation_failure() -> Result<(), Report> {
     zebra_test::init();
-    use crate::error::*;
-    use zebra_chain::transaction::Transaction;
-
     let network = Network::Mainnet;
 
     // Get a block in the mainnet that is inside the funding stream period.
@@ -465,13 +462,17 @@ fn funding_stream_validation_failure() -> Result<(), Report> {
     let tx = block
         .transactions
         .get(0)
-        .map(|transaction| Transaction::V4 {
+        .map(|transaction| {
-            inputs: transaction.inputs().to_vec(),
+            let mut output = transaction.outputs()[0].clone();
-            outputs: vec![transaction.outputs()[0].clone()],
+            output.value = Amount::try_from(i32::MAX).unwrap();
-            lock_time: transaction.lock_time().unwrap_or_else(LockTime::unlocked),
+            Transaction::V4 {
-            expiry_height: Height(0),
+                inputs: transaction.inputs().to_vec(),
-            joinsplit_data: None,
+                outputs: vec![output],
-            sapling_shielded_data: None,
+                lock_time: transaction.lock_time().unwrap_or_else(LockTime::unlocked),
+                expiry_height: Height(0),
+                joinsplit_data: None,
+                sapling_shielded_data: None,
+            }
         })
         .unwrap();
 
@@ -483,10 +484,65 @@ fn funding_stream_validation_failure() -> Result<(), Report> {
     };
 
     // Validate it
-    let result = check::subsidy_is_valid(&block, network).unwrap_err();
+    let result = check::subsidy_is_valid(&block, network);
-    let expected = BlockError::Transaction(TransactionError::Subsidy(
+    let expected = Err(BlockError::Transaction(TransactionError::Subsidy(
         SubsidyError::FundingStreamNotFound,
-    ));
+    )));
+    assert_eq!(expected, result);
+
+    Ok(())
+}
+
+#[test]
+fn miner_fees_validation_success() -> Result<(), Report> {
+    zebra_test::init();
+
+    miner_fees_validation_for_network(Network::Mainnet)?;
+    miner_fees_validation_for_network(Network::Testnet)?;
+
+    Ok(())
+}
+
+fn miner_fees_validation_for_network(network: Network) -> Result<(), Report> {
+    let block_iter = match network {
+        Network::Mainnet => zebra_test::vectors::MAINNET_BLOCKS.iter(),
+        Network::Testnet => zebra_test::vectors::TESTNET_BLOCKS.iter(),
+    };
+
+    for (&height, block) in block_iter {
+        if Height(height) > SLOW_START_SHIFT {
+            let block = Block::zcash_deserialize(&block[..]).expect("block should deserialize");
+
+            // fake the miner fee to a big amount
+            let miner_fees = Amount::try_from(MAX_MONEY / 2).unwrap();
+
+            // Validate
+            let result = check::miner_fees_are_valid(&block, network, miner_fees);
+            assert!(result.is_ok());
+        }
+    }
+
+    Ok(())
+}
+
+#[test]
+fn miner_fees_validation_failure() -> Result<(), Report> {
+    zebra_test::init();
+    let network = Network::Mainnet;
+
+    let block =
+        Arc::<Block>::zcash_deserialize(&zebra_test::vectors::BLOCK_MAINNET_347499_BYTES[..])
+            .expect("block should deserialize");
+
+    // fake the miner fee to a small amount
+    let miner_fees = Amount::zero();
+
+    // Validate
+    let result = check::miner_fees_are_valid(&block, network, miner_fees);
+
+    let expected = Err(BlockError::Transaction(TransactionError::Subsidy(
+        SubsidyError::InvalidMinerFees,
+    )));
     assert_eq!(expected, result);
 
     Ok(())

zebra-consensus/src/error.rs

@@ -25,6 +25,12 @@ pub enum SubsidyError {
 
     #[error("funding stream expected output not found")]
     FundingStreamNotFound,
+
+    #[error("miner fees are invalid")]
+    InvalidMinerFees,
+
+    #[error("a sum of amounts overflowed")]
+    SumOverflow,
 }
 
 #[derive(Error, Clone, Debug, PartialEq, Eq)]