state: Make Response::Added return the block header hash

Move block header hashing from zebra-consensus to zebra-state.
Handle zebra-state AddBlock errors in zebra-consensus BlockVerifier.
Add unit tests for BlockVerifier state error handling.

Part of #428.

zebra-consensus/src/verify.rs

@@ -49,14 +49,10 @@ where
     }
 
     fn call(&mut self, block: Arc<Block>) -> Self::Future {
-        let header_hash: BlockHeaderHash = block.as_ref().into();
-
-        // Ignore errors for now.
         // TODO(jlusby): Error = Report, handle errors from state_service.
         // TODO(teor):
         //   - handle chain reorgs, adjust state_service "unique block height" conditions
         //   - handle block validation errors (including errors in the block's transactions)
-        //   - handle state_service AddBlock errors, and add unit tests for those errors
 
         // `state_service.call` is OK here because we already called
         // `state_service.poll_ready` in our `poll_ready`.
@@ -65,8 +61,10 @@ where
             .call(zebra_state::Request::AddBlock { block });
 
         async move {
-            add_block.await?;
+            match add_block.await? {
-            Ok(header_hash)
+                zebra_state::Response::Added { hash } => Ok(hash),
+                _ => Err("adding block to zebra-state failed".into()),
+            }
         }
         .boxed()
     }
@@ -159,8 +157,6 @@ mod tests {
     #[tokio::test]
     #[spandoc::spandoc]
     async fn round_trip() -> Result<(), Report> {
-        install_tracing();
-
         let block =
             Arc::<Block>::zcash_deserialize(&zebra_test_vectors::BLOCK_MAINNET_415000_BYTES[..])?;
         let hash: BlockHeaderHash = block.as_ref().into();
@@ -199,4 +195,84 @@ mod tests {
 
         Ok(())
     }
+
+    #[tokio::test]
+    #[spandoc::spandoc]
+    async fn verify_fail_add_block() -> Result<(), Report> {
+        install_tracing();
+
+        let block =
+            Arc::<Block>::zcash_deserialize(&zebra_test_vectors::BLOCK_MAINNET_415000_BYTES[..])?;
+        let hash: BlockHeaderHash = block.as_ref().into();
+
+        let mut state_service = zebra_state::in_memory::init();
+        let mut block_verifier = super::init(state_service.clone());
+
+        // Add the block for the first time
+        let verify_response = block_verifier
+            .ready_and()
+            .await
+            .map_err(|e| eyre!(e))?
+            .call(block.clone())
+            .await
+            .map_err(|e| eyre!(e))?;
+
+        ensure!(
+            verify_response == hash,
+            "unexpected response kind: {:?}",
+            verify_response
+        );
+
+        let state_response = state_service
+            .ready_and()
+            .await
+            .map_err(|e| eyre!(e))?
+            .call(zebra_state::Request::GetBlock { hash })
+            .await
+            .map_err(|e| eyre!(e))?;
+
+        match state_response {
+            zebra_state::Response::Block {
+                block: returned_block,
+            } => assert_eq!(block, returned_block),
+            _ => bail!("unexpected response kind: {:?}", state_response),
+        }
+
+        // Now try to add the block again, verify should fail
+        // TODO(teor): ignore duplicate block verifies?
+        let verify_result = block_verifier
+            .ready_and()
+            .await
+            .map_err(|e| eyre!(e))?
+            .call(block.clone())
+            .await;
+
+        ensure!(
+            match verify_result {
+                Ok(_) => false,
+                // TODO(teor || jlusby): check error string
+                _ => true,
+            },
+            "unexpected result kind: {:?}",
+            verify_result
+        );
+
+        // But the state should still return the original block we added
+        let state_response = state_service
+            .ready_and()
+            .await
+            .map_err(|e| eyre!(e))?
+            .call(zebra_state::Request::GetBlock { hash })
+            .await
+            .map_err(|e| eyre!(e))?;
+
+        match state_response {
+            zebra_state::Response::Block {
+                block: returned_block,
+            } => assert_eq!(block, returned_block),
+            _ => bail!("unexpected response kind: {:?}", state_response),
+        }
+
+        Ok(())
+    }
 }

zebra-state/src/in_memory.rs

@@ -30,7 +30,8 @@ impl Service<Request> for ZebraState {
     fn call(&mut self, req: Request) -> Self::Future {
         match req {
             Request::AddBlock { block } => {
-                let result = self.index.insert(block).map(|_| Response::Added);
+                let hash = block.as_ref().into();
+                let result = self.index.insert(block).map(|_| Response::Added { hash });
 
                 async { result }.boxed()
             }

zebra-state/src/lib.rs

@@ -6,7 +6,7 @@ use zebra_chain::block::{Block, BlockHeaderHash};
 
 pub mod in_memory;
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub enum Request {
     // TODO(jlusby): deprecate in the future based on our validation story
     AddBlock { block: Arc<Block> },
@@ -14,9 +14,9 @@ pub enum Request {
     GetTip,
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub enum Response {
-    Added,
+    Added { hash: BlockHeaderHash },
     Block { block: Arc<Block> },
     Tip { hash: BlockHeaderHash },
 }
@@ -62,7 +62,7 @@ mod tests {
             .map_err(|e| eyre!(e))?;
 
         ensure!(
-            matches!(response, Response::Added),
+            response == Response::Added { hash },
             "unexpected response kind: {:?}",
             response
         );
@@ -92,7 +92,9 @@ mod tests {
         let block1: Arc<_> =
             Block::zcash_deserialize(&zebra_test_vectors::BLOCK_MAINNET_1_BYTES[..])?.into();
 
-        let expected_hash: BlockHeaderHash = block1.as_ref().into();
+        let block0_hash: BlockHeaderHash = block0.as_ref().into();
+        let block1_hash: BlockHeaderHash = block1.as_ref().into();
+        let expected_hash: BlockHeaderHash = block1_hash;
 
         let mut service = in_memory::init();
 
@@ -103,7 +105,7 @@ mod tests {
             .map_err(|e| eyre!(e))?;
 
         ensure!(
-            matches!(response, Response::Added),
+            response == Response::Added { hash: block1_hash },
             "unexpected response kind: {:?}",
             response
         );
@@ -117,7 +119,7 @@ mod tests {
             .map_err(|e| eyre!(e))?;
 
         ensure!(
-            matches!(response, Response::Added),
+            response == Response::Added { hash: block0_hash },
             "unexpected response kind: {:?}",
             response
         );