Simplify `where` bounds on services.

Placing bounds on the service's future is less ideal, because the future is
already constrained by the `Service` trait, so the bounds can be expressed more
directly and simply by bounding the service itself.

If the verification service already has to have a generic parameter for the
future (the `ZSF`), it could instead be generic over `S`, the storage service.
This has the upside that it's no longer required for the verification service
to box the storage service, so we don't add any extra layers of indirection,
and the where bounds become more straightforward, since they're centered on the
requirements for the storage service itself, not the future it returns.
Finally, we can simplify the bounds by using the request / response types
directly rather than defining wrapper types.

zebra-consensus/src/verify.rs

@@ -13,6 +13,7 @@ use std::{
     error,
     future::Future,
     pin::Pin,
+    sync::Arc,
     task::{Context, Poll},
 };
 use tower::{buffer::Buffer, Service};
@@ -22,32 +23,10 @@ use zebra_chain::block::{Block, BlockHeaderHash};
 mod script;
 mod transaction;
 
-/// The trait constraints that we expect from `zebra_state::ZebraState` errors.
+struct BlockVerifier<S> {
-type ZSE = Box<dyn error::Error + Send + Sync + 'static>;
+    state_service: S,
-/// The trait constraints that we expect from the `zebra_state::ZebraState` service.
-/// `ZSF` is the `Future` type for `zebra_state::ZebraState`. This type is generic,
-/// because `tower::Service` function calls require a `Sized` future type.
-type ZS<ZSF> = Box<
-    dyn Service<zebra_state::Request, Response = zebra_state::Response, Error = ZSE, Future = ZSF>
-        + Send
-        + 'static,
->;
-
-/// Block verification service.
-///
-/// After verification, blocks are added to `state_service`. We use a generic
-/// future `ZSF` for that service, so that the underlying state service can be
-/// wrapped in other services as needed.
-struct BlockVerifier<ZSF>
-where
-    ZSF: Future<Output = Result<zebra_state::Response, ZSE>> + Send + 'static,
-{
-    state_service: ZS<ZSF>,
 }
 
-/// The result type for the BlockVerifier Service.
-type Response = BlockHeaderHash;
-
 /// The error type for the BlockVerifier Service.
 // TODO(jlusby): Error = Report ?
 type Error = Box<dyn error::Error + Send + Sync + 'static>;
@@ -55,11 +34,12 @@ type Error = Box<dyn error::Error + Send + Sync + 'static>;
 /// The BlockVerifier service implementation.
 ///
 /// After verification, blocks are added to the underlying state service.
-impl<ZSF> Service<Block> for BlockVerifier<ZSF>
+impl<S> Service<Arc<Block>> for BlockVerifier<S>
 where
-    ZSF: Future<Output = Result<zebra_state::Response, ZSE>> + Send + 'static,
+    S: Service<zebra_state::Request, Response = zebra_state::Response, Error = Error>,
+    S::Future: Send + 'static,
 {
-    type Response = Response;
+    type Response = BlockHeaderHash;
     type Error = Error;
     type Future =
         Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send + 'static>>;
@@ -68,8 +48,8 @@ where
         self.state_service.poll_ready(context)
     }
 
-    fn call(&mut self, block: Block) -> Self::Future {
+    fn call(&mut self, block: Arc<Block>) -> Self::Future {
-        let header_hash: BlockHeaderHash = (&block).into();
+        let header_hash: BlockHeaderHash = block.as_ref().into();
 
         // Ignore errors for now.
         // TODO(jlusby): Error = Report, handle errors from state_service.
@@ -80,9 +60,9 @@ where
 
         // `state_service.call` is OK here because we already called
         // `state_service.poll_ready` in our `poll_ready`.
-        let add_block = self.state_service.call(zebra_state::Request::AddBlock {
+        let add_block = self
-            block: block.into(),
+            .state_service
-        });
+            .call(zebra_state::Request::AddBlock { block });
 
         async move {
             add_block.await?;
@@ -92,25 +72,36 @@ where
     }
 }
 
-/// Initialise the BlockVerifier service.
+/// Return a block verification service, using the provided state service.
 ///
-/// We use a generic type `ZS<ZSF>` for `state_service`, so that the
+/// The block verifier holds a state service of type `S`, used as context for
-/// underlying state service can be wrapped in other services as needed.
+/// block validation and to which newly verified blocks will be committed. This
-/// For similar reasons, we also return a dynamic service type.
+/// state is pluggable to allow for testing or instrumentation.
-pub fn init<ZSF>(
+///
-    state_service: ZS<ZSF>,
+/// The returned type is opaque to allow instrumentation or other wrappers, but
+/// can be boxed for storage. It is also `Clone` to allow sharing of a
+/// verification service.
+///
+/// This function should be called only once for a particular state service (and
+/// the result be shared) rather than constructing multiple verification services
+/// backed by the same state layer.
+pub fn init<S>(
+    state_service: S,
 ) -> impl Service<
-    Block,
+    Arc<Block>,
-    Response = Response,
+    Response = BlockHeaderHash,
     Error = Error,
-    Future = impl Future<Output = Result<Response, Error>>,
+    Future = impl Future<Output = Result<BlockHeaderHash, Error>>,
 > + Send
        + Clone
        + 'static
 where
-    ZSF: Future<Output = Result<zebra_state::Response, ZSE>> + Send + 'static,
+    S: Service<zebra_state::Request, Response = zebra_state::Response, Error = Error>
+        + Send
+        + 'static,
+    S::Future: Send + 'static,
 {
-    Buffer::new(BlockVerifier::<ZSF> { state_service }, 1)
+    Buffer::new(BlockVerifier { state_service }, 1)
 }
 
 #[cfg(test)]
@@ -138,20 +129,12 @@ mod tests {
             .init();
     }
 
-    /// Initialise and return an unwrapped `BlockVerifier`.
-    fn init_block_verifier<ZSF>(state_service: ZS<ZSF>) -> BlockVerifier<ZSF>
-    where
-        ZSF: Future<Output = Result<zebra_state::Response, ZSE>> + Send + 'static,
-    {
-        BlockVerifier::<ZSF> { state_service }
-    }
-
     #[tokio::test]
     #[spandoc::spandoc]
     async fn verify() -> Result<(), Report> {
-        let block = Block::zcash_deserialize(&zebra_test_vectors::BLOCK_MAINNET_415000_BYTES[..])?;
+        let block =
-        // TODO(teor): why does rustc say that _hash is unused?
+            Arc::<Block>::zcash_deserialize(&zebra_test_vectors::BLOCK_MAINNET_415000_BYTES[..])?;
-        let _hash: BlockHeaderHash = (&block).into();
+        let hash: BlockHeaderHash = block.as_ref().into();
 
         let state_service = Box::new(zebra_state::in_memory::init());
         let mut block_verifier = super::init(state_service);
@@ -165,7 +148,7 @@ mod tests {
             .map_err(|e| eyre!(e))?;
 
         ensure!(
-            matches!(verify_response, _hash),
+            verify_response == hash,
             "unexpected response kind: {:?}",
             verify_response
         );
@@ -178,12 +161,12 @@ mod tests {
     async fn round_trip() -> Result<(), Report> {
         install_tracing();
 
-        let block = Block::zcash_deserialize(&zebra_test_vectors::BLOCK_MAINNET_GENESIS_BYTES[..])?;
+        let block =
-        // TODO(teor): why does rustc say that _hash is unused?
+            Arc::<Block>::zcash_deserialize(&zebra_test_vectors::BLOCK_MAINNET_415000_BYTES[..])?;
-        let _hash: BlockHeaderHash = (&block).into();
+        let hash: BlockHeaderHash = block.as_ref().into();
 
-        let state_service = Box::new(zebra_state::in_memory::init());
+        let mut state_service = zebra_state::in_memory::init();
-        let mut block_verifier = init_block_verifier(state_service);
+        let mut block_verifier = super::init(state_service.clone());
 
         let verify_response = block_verifier
             .ready_and()
@@ -194,24 +177,23 @@ mod tests {
             .map_err(|e| eyre!(e))?;
 
         ensure!(
-            matches!(verify_response, _hash),
+            verify_response == hash,
             "unexpected response kind: {:?}",
             verify_response
         );
 
-        let state_response = block_verifier
+        let state_response = state_service
-            .state_service
             .ready_and()
             .await
             .map_err(|e| eyre!(e))?
-            .call(zebra_state::Request::GetBlock { hash: _hash })
+            .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.as_ref()),
+            } => assert_eq!(block, returned_block),
             _ => bail!("unexpected response kind: {:?}", state_response),
         }