From 8e49663e3120e71634d320b7e521992ede13de08 Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Wed, 24 Nov 2021 01:31:56 +1000
Subject: [PATCH] Sum transaction miner fees in the block verifier (#3093)

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
---
 zebra-consensus/src/block.rs | 42 ++++++++++++++++++++++++++++++++----
 zebra-consensus/src/error.rs |  9 +++++++-
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs
index c9095a1b6..b912792ba 100644
--- a/zebra-consensus/src/block.rs
+++ b/zebra-consensus/src/block.rs
@@ -22,6 +22,7 @@ use tower::{Service, ServiceExt};
 use tracing::Instrument;
 
 use zebra_chain::{
+    amount::Amount,
     block::{self, Block},
     parameters::Network,
     transparent,
@@ -175,7 +176,7 @@ where
             // Since errors cause an early exit, try to do the
             // quick checks first.
 
-            // Field validity and structure checks
+            // Quick field validity and structure checks
             let now = Utc::now();
             check::time_is_valid_at(&block.header, now, &height, &hash)
                 .map_err(VerifyBlockError::Time)?;
@@ -184,13 +185,19 @@ where
                 .transactions
                 .get(0)
                 .expect("must have coinbase transaction");
-            // Check compatibility with ZIP-212 shielded Sapling and Orchard coinbase output decryption
-            tx::check::coinbase_outputs_are_decryptable(coinbase_tx, network, height)?;
             check::subsidy_is_valid(&block, network)?;
 
             // Validate `nExpiryHeight` consensus rules
+            // TODO: check non-coinbase transaction expiry against the block height (#2387)
+            //       check the maximum expiry height for non-coinbase transactions (#2387)
             check::coinbase_expiry_height(&height, coinbase_tx, network)?;
 
+            // Now do the slower checks
+
+            // Check compatibility with ZIP-212 shielded Sapling and Orchard coinbase output decryption
+            tx::check::coinbase_outputs_are_decryptable(coinbase_tx, network, height)?;
+
+            // Send transactions to the transaction verifier to be checked
             let mut async_checks = FuturesUnordered::new();
 
             let known_utxos = Arc::new(transparent::new_ordered_outputs(
@@ -212,7 +219,11 @@ where
             }
             tracing::trace!(len = async_checks.len(), "built async tx checks");
 
+            // Get the transaction results back from the transaction verifier.
+
+            // Sum up some block totals from the transaction responses.
             let mut legacy_sigop_count = 0;
+            let mut block_miner_fees = Ok(Amount::zero());
 
             use futures::StreamExt;
             while let Some(result) = async_checks.next().await {
@@ -220,11 +231,26 @@ where
                 let response = result
                     .map_err(Into::into)
                     .map_err(VerifyBlockError::Transaction)?;
+
+                assert!(
+                    matches!(response, tx::Response::Block { .. }),
+                    "unexpected response from transaction verifier: {:?}",
+                    response
+                );
+
                 legacy_sigop_count += response
                     .legacy_sigop_count()
                     .expect("block transaction responses must have a legacy sigop count");
+
+                // Coinbase transactions consume the miner fee,
+                // so they don't add any value to the block's total miner fee.
+                if let Some(miner_fee) = response.miner_fee() {
+                    block_miner_fees += miner_fee;
+                }
             }
 
+            // Check the summed block totals
+
             if legacy_sigop_count > MAX_BLOCK_SIGOPS {
                 Err(BlockError::TooManyTransparentSignatureOperations {
                     height,
@@ -233,10 +259,18 @@ where
                 })?;
             }
 
+            // TODO: check miner subsidy and miner fees (#1162)
+            let _block_miner_fees =
+                block_miner_fees.map_err(|amount_error| BlockError::SummingMinerFees {
+                    height,
+                    hash,
+                    source: amount_error,
+                })?;
+
+            // Finally, submit the block for contextual verification.
             let new_outputs = Arc::try_unwrap(known_utxos)
                 .expect("all verification tasks using known_utxos are complete");
 
-            // Finally, submit the block for contextual verification.
             let prepared_block = zs::PreparedBlock {
                 block,
                 hash,
diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs
index 3242f0e94..ec14f6475 100644
--- a/zebra-consensus/src/error.rs
+++ b/zebra-consensus/src/error.rs
@@ -8,7 +8,7 @@
 use chrono::{DateTime, Utc};
 use thiserror::Error;
 
-use zebra_chain::{block, orchard, sapling, sprout, transparent};
+use zebra_chain::{amount, block, orchard, sapling, sprout, transparent};
 
 use crate::{block::MAX_BLOCK_SIGOPS, BoxError};
 
@@ -229,4 +229,11 @@ pub enum BlockError {
         hash: zebra_chain::block::Hash,
         legacy_sigop_count: u64,
     },
+
+    #[error("summing miner fees for block {height:?} {hash:?} failed: {source:?}")]
+    SummingMinerFees {
+        height: zebra_chain::block::Height,
+        hash: zebra_chain::block::Hash,
+        source: amount::Error,
+    },
 }