From 5548dffd3b3c86e9c81249cfea9eb5c0d052136a Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Wed, 15 Jul 2020 13:21:06 +1000
Subject: [PATCH] refactor: Move the coinbase first check to a Block method

---
 zebra-chain/src/block.rs     | 31 ++++++++++++++++++++++++++++++-
 zebra-chain/src/lib.rs       |  1 +
 zebra-consensus/src/block.rs | 27 +--------------------------
 3 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs
index 3ea0545cd..ca52157ba 100644
--- a/zebra-chain/src/block.rs
+++ b/zebra-chain/src/block.rs
@@ -9,7 +9,7 @@ mod serialize;
 mod tests;
 
 use serde::{Deserialize, Serialize};
-use std::sync::Arc;
+use std::{error, sync::Arc};
 
 #[cfg(test)]
 use proptest_derive::Arbitrary;
@@ -43,6 +43,10 @@ pub struct Block {
 /// transaction in the chain is approximately 1.5 kB smaller.)
 pub const MAX_BLOCK_BYTES: u64 = 2_000_000;
 
+/// The error type for Block checks.
+// TODO(jlusby): Error = Report ?
+type Error = Box<dyn error::Error + Send + Sync + 'static>;
+
 impl Block {
     /// Return the block height reported in the coinbase transaction, if any.
     pub fn coinbase_height(&self) -> Option<BlockHeight> {
@@ -55,6 +59,31 @@ impl Block {
                 _ => None,
             })
     }
+
+    /// Check that there is exactly one coinbase transaction in `Block`, and that
+    /// the coinbase transaction is the first transaction in the block.
+    ///
+    /// "The first (and only the first) transaction in a block is a coinbase
+    /// transaction, which collects and spends any miner subsidy and transaction
+    /// fees paid by transactions included in this block."[S 3.10][3.10]
+    ///
+    /// [3.10]: https://zips.z.cash/protocol/protocol.pdf#coinbasetransactions
+    pub fn is_coinbase_first(&self) -> Result<(), Error> {
+        if self.coinbase_height().is_some() {
+            // No coinbase inputs in additional transactions allowed
+            if self
+                .transactions
+                .iter()
+                .skip(1)
+                .any(|tx| tx.contains_coinbase_input())
+            {
+                Err("coinbase input found in additional transaction")?
+            }
+            Ok(())
+        } else {
+            Err("no coinbase transaction in block")?
+        }
+    }
 }
 
 impl<'a> From<&'a Block> for BlockHeaderHash {
diff --git a/zebra-chain/src/lib.rs b/zebra-chain/src/lib.rs
index 3e8dcad83..cd0c3c954 100644
--- a/zebra-chain/src/lib.rs
+++ b/zebra-chain/src/lib.rs
@@ -3,6 +3,7 @@
 #![doc(html_logo_url = "https://www.zfnd.org/images/zebra-icon.png")]
 #![doc(html_root_url = "https://doc.zebra.zfnd.org/zebra_chain")]
 #![deny(missing_docs)]
+#![allow(clippy::try_err)]
 
 #[macro_use]
 extern crate serde;
diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs
index acdbdbcef..b2c15b90c 100644
--- a/zebra-consensus/src/block.rs
+++ b/zebra-consensus/src/block.rs
@@ -53,31 +53,6 @@ pub(crate) fn node_time_check(
     }
 }
 
-/// Check that there is exactly one coinbase transaction in `Block`, and that
-/// the coinbase transaction is the first transaction in the block.
-///
-/// "The first (and only the first) transaction in a block is a coinbase
-/// transaction, which collects and spends any miner subsidy and transaction
-/// fees paid by transactions included in this block."[S 3.10][3.10]
-///
-/// [3.10]: https://zips.z.cash/protocol/protocol.pdf#coinbasetransactions
-pub(crate) fn coinbase_is_first_check(block: &Block) -> Result<(), Error> {
-    if block.coinbase_height().is_some() {
-        // No coinbase inputs in additional transactions allowed
-        if block
-            .transactions
-            .iter()
-            .skip(1)
-            .any(|tx| tx.contains_coinbase_input())
-        {
-            Err("coinbase input found in additional transaction")?
-        }
-        Ok(())
-    } else {
-        Err("no coinbase transaction in block")?
-    }
-}
-
 struct BlockVerifier<S> {
     /// The underlying `ZebraState`, possibly wrapped in other services.
     state_service: S,
@@ -123,7 +98,7 @@ where
             let now = Utc::now();
             node_time_check(block.header.time, now)?;
             block.header.is_equihash_solution_valid()?;
-            coinbase_is_first_check(block.as_ref())?;
+            block.is_coinbase_first()?;
 
             // `Tower::Buffer` requires a 1:1 relationship between `poll()`s
             // and `call()`s, because it reserves a buffer slot in each