1. change(rpc): Add a mempool field to GetBlockTemplateRpcImpl, and cleanup tests (#5493)

* Add a mempool to GetBlockTemplateRpcImpl, and cleanup tests

* Update snapshot file locations

* Update snapshot instructions

zebra-rpc/src/methods.rs

@@ -29,7 +29,7 @@ use zebra_chain::{
     transparent::{self, Address},
 };
 use zebra_network::constants::USER_AGENT;
-use zebra_node_services::{mempool, BoxError};
+use zebra_node_services::mempool;
 use zebra_state::{OutputIndex, OutputLocation, TransactionLocation};
 
 use crate::queue::Queue;
@@ -241,7 +241,11 @@ pub trait Rpc {
 /// RPC method implementations.
 pub struct RpcImpl<Mempool, State, Tip>
 where
-    Mempool: Service<mempool::Request, Response = mempool::Response, Error = BoxError>,
+    Mempool: Service<
+        mempool::Request,
+        Response = mempool::Response,
+        Error = zebra_node_services::BoxError,
+    >,
     State: Service<
         zebra_state::ReadRequest,
         Response = zebra_state::ReadResponse,
@@ -280,7 +284,11 @@ where
 
 impl<Mempool, State, Tip> RpcImpl<Mempool, State, Tip>
 where
-    Mempool: Service<mempool::Request, Response = mempool::Response, Error = BoxError> + 'static,
+    Mempool: Service<
+            mempool::Request,
+            Response = mempool::Response,
+            Error = zebra_node_services::BoxError,
+        > + 'static,
     State: Service<
             zebra_state::ReadRequest,
             Response = zebra_state::ReadResponse,
@@ -337,8 +345,11 @@ where
 
 impl<Mempool, State, Tip> Rpc for RpcImpl<Mempool, State, Tip>
 where
-    Mempool:
-        tower::Service<mempool::Request, Response = mempool::Response, Error = BoxError> + 'static,
+    Mempool: tower::Service<
+            mempool::Request,
+            Response = mempool::Response,
+            Error = zebra_node_services::BoxError,
+        > + 'static,
     Mempool::Future: Send,
     State: Service<
             zebra_state::ReadRequest,

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -1,12 +1,12 @@
 //! RPC methods related to mining only available with `getblocktemplate-rpcs` rust feature.
-use zebra_chain::{block::Height, chain_tip::ChainTip};
 
 use futures::{FutureExt, TryFutureExt};
 use jsonrpc_core::{self, BoxFuture, Error, ErrorCode, Result};
 use jsonrpc_derive::rpc;
-use tower::{Service, ServiceExt};
+use tower::{buffer::Buffer, Service, ServiceExt};
 
-pub(crate) mod types;
+use zebra_chain::{block::Height, chain_tip::ChainTip};
+use zebra_node_services::mempool;
 
 use crate::methods::{
     get_block_template_rpcs::types::{
@@ -15,6 +15,8 @@ use crate::methods::{
     GetBlockHash, MISSING_BLOCK_ERROR_CODE,
 };
 
+pub(crate) mod types;
+
 /// getblocktemplate RPC method signatures.
 #[rpc(server)]
 pub trait GetBlockTemplateRpc {
@@ -58,26 +60,46 @@ pub trait GetBlockTemplateRpc {
 }
 
 /// RPC method implementations.
-pub struct GetBlockTemplateRpcImpl<Tip, State>
+pub struct GetBlockTemplateRpcImpl<Mempool, State, Tip>
 where
-    Tip: ChainTip,
+    Mempool: Service<
+        mempool::Request,
+        Response = mempool::Response,
+        Error = zebra_node_services::BoxError,
+    >,
     State: Service<
         zebra_state::ReadRequest,
         Response = zebra_state::ReadResponse,
         Error = zebra_state::BoxError,
     >,
+    Tip: ChainTip,
 {
     // TODO: Add the other fields from the [`Rpc`] struct as-needed
-    /// Allows efficient access to the best tip of the blockchain.
-    latest_chain_tip: Tip,
+
+    // Configuration
+    //
+    // TODO: add mining config for getblocktemplate RPC miner address
+
+    // Services
+    //
+    /// A handle to the mempool service.
+    #[allow(dead_code)]
+    mempool: Buffer<Mempool, mempool::Request>,
 
     /// A handle to the state service.
     state: State,
+
+    /// Allows efficient access to the best tip of the blockchain.
+    latest_chain_tip: Tip,
 }
 
-impl<Tip, State> GetBlockTemplateRpcImpl<Tip, State>
+impl<Mempool, State, Tip> GetBlockTemplateRpcImpl<Mempool, State, Tip>
 where
-    Tip: ChainTip + Clone + Send + Sync + 'static,
+    Mempool: Service<
+            mempool::Request,
+            Response = mempool::Response,
+            Error = zebra_node_services::BoxError,
+        > + 'static,
     State: Service<
             zebra_state::ReadRequest,
             Response = zebra_state::ReadResponse,
@@ -86,19 +108,30 @@ where
         + Send
         + Sync
         + 'static,
+    Tip: ChainTip + Clone + Send + Sync + 'static,
 {
-    /// Create a new instance of the RPC handler.
-    pub fn new(latest_chain_tip: Tip, state: State) -> Self {
+    /// Create a new instance of the handler for getblocktemplate RPCs.
+    pub fn new(
+        mempool: Buffer<Mempool, mempool::Request>,
+        state: State,
+        latest_chain_tip: Tip,
+    ) -> Self {
         Self {
-            latest_chain_tip,
+            mempool,
             state,
+            latest_chain_tip,
         }
     }
 }
 
-impl<Tip, State> GetBlockTemplateRpc for GetBlockTemplateRpcImpl<Tip, State>
+impl<Mempool, State, Tip> GetBlockTemplateRpc for GetBlockTemplateRpcImpl<Mempool, State, Tip>
 where
-    Tip: ChainTip + Send + Sync + 'static,
+    Mempool: Service<
+            mempool::Request,
+            Response = mempool::Response,
+            Error = zebra_node_services::BoxError,
+        > + 'static,
+    Mempool::Future: Send,
     State: Service<
             zebra_state::ReadRequest,
             Response = zebra_state::ReadResponse,
@@ -108,6 +141,7 @@ where
         + Sync
         + 'static,
     <State as Service<zebra_state::ReadRequest>>::Future: Send,
+    Tip: ChainTip + Send + Sync + 'static,
 {
     fn get_block_count(&self) -> Result<u32> {
         self.latest_chain_tip

zebra-rpc/src/methods/tests/snapshot.rs

@@ -1,4 +1,9 @@
 //! Snapshot tests for Zebra JSON-RPC responses.
+//!
+//! To update these snapshots, run:
+//! ```sh
+//! cargo insta test --review
+//! ```
 
 use std::sync::Arc;
 
@@ -10,11 +15,13 @@ use zebra_chain::{
     serialization::ZcashDeserializeInto,
 };
 use zebra_network::constants::USER_AGENT;
-use zebra_node_services::BoxError;
 use zebra_test::mock_service::MockService;
 
 use super::super::*;
 
+#[cfg(feature = "getblocktemplate-rpcs")]
+mod get_block_template_rpcs;
+
 /// Snapshot test for RPC methods responses.
 #[tokio::test(flavor = "multi_thread")]
 async fn test_rpc_response_data() {
@@ -36,15 +43,25 @@ async fn test_rpc_response_data_for_network(network: Network) {
         .map(|(_height, block_bytes)| block_bytes.zcash_deserialize_into().unwrap())
         .collect();
 
-    let mut mempool: MockService<_, _, _, BoxError> = MockService::build().for_unit_tests();
+    let mut mempool: MockService<_, _, _, zebra_node_services::BoxError> =
+        MockService::build().for_unit_tests();
     // Create a populated state service
     let (_state, read_state, latest_chain_tip, _chain_tip_change) =
         zebra_state::populated_state(blocks.clone(), network).await;
 
+    // Start snapshots of RPC responses.
+    let mut settings = insta::Settings::clone_current();
+    settings.set_snapshot_suffix(format!("{}_{}", network_string(network), blocks.len() - 1));
+
+    // Test getblocktemplate-rpcs snapshots
     #[cfg(feature = "getblocktemplate-rpcs")]
-    let latest_chain_tip_gbt_clone = latest_chain_tip.clone();
-    #[cfg(feature = "getblocktemplate-rpcs")]
-    let read_state_clone = read_state.clone();
+    get_block_template_rpcs::test_responses(
+        mempool.clone(),
+        read_state.clone(),
+        latest_chain_tip.clone(),
+        settings.clone(),
+    )
+    .await;
 
     // Init RPC
     let (rpc, _rpc_tx_queue_task_handle) = RpcImpl::new(
@@ -56,10 +73,6 @@ async fn test_rpc_response_data_for_network(network: Network) {
         latest_chain_tip,
     );
 
-    // Start snapshots of RPC responses.
-    let mut settings = insta::Settings::clone_current();
-    settings.set_snapshot_suffix(format!("{}_{}", network_string(network), blocks.len() - 1));
-
     // `getinfo`
     let get_info = rpc.get_info().expect("We should have a GetInfo struct");
     snapshot_rpc_getinfo(get_info, &settings);
@@ -171,34 +184,6 @@ async fn test_rpc_response_data_for_network(network: Network) {
         .await
         .expect("We should have a vector of strings");
     snapshot_rpc_getaddressutxos(get_address_utxos, &settings);
-
-    #[cfg(feature = "getblocktemplate-rpcs")]
-    {
-        let get_block_template_rpc =
-            GetBlockTemplateRpcImpl::new(latest_chain_tip_gbt_clone, read_state_clone);
-
-        // `getblockcount`
-        let get_block_count = get_block_template_rpc
-            .get_block_count()
-            .expect("We should have a number");
-        snapshot_rpc_getblockcount(get_block_count, &settings);
-
-        // `getblockhash`
-        const BLOCK_HEIGHT10: i32 = 10;
-        let get_block_hash = get_block_template_rpc
-            .get_block_hash(BLOCK_HEIGHT10)
-            .await
-            .expect("We should have a GetBlockHash struct");
-
-        snapshot_rpc_getblockhash(get_block_hash, &settings);
-
-        // `getblocktemplate`
-        let get_block_template = get_block_template_rpc
-            .get_block_template()
-            .await
-            .expect("We should have a GetBlockTemplate struct");
-        snapshot_rpc_getblocktemplate(get_block_template, &settings);
-    }
 }
 
 /// Snapshot `getinfo` response, using `cargo insta` and JSON serialization.
@@ -287,27 +272,6 @@ fn snapshot_rpc_getaddressutxos(utxos: Vec<GetAddressUtxos>, settings: &insta::S
     settings.bind(|| insta::assert_json_snapshot!("get_address_utxos", utxos));
 }
 
-#[cfg(feature = "getblocktemplate-rpcs")]
-/// Snapshot `getblockcount` response, using `cargo insta` and JSON serialization.
-fn snapshot_rpc_getblockcount(block_count: u32, settings: &insta::Settings) {
-    settings.bind(|| insta::assert_json_snapshot!("get_block_count", block_count));
-}
-
-#[cfg(feature = "getblocktemplate-rpcs")]
-/// Snapshot `getblockhash` response, using `cargo insta` and JSON serialization.
-fn snapshot_rpc_getblockhash(block_hash: GetBlockHash, settings: &insta::Settings) {
-    settings.bind(|| insta::assert_json_snapshot!("get_block_hash", block_hash));
-}
-
-#[cfg(feature = "getblocktemplate-rpcs")]
-/// Snapshot `getblocktemplate` response, using `cargo insta` and JSON serialization.
-fn snapshot_rpc_getblocktemplate(
-    block_template: crate::methods::get_block_template_rpcs::types::get_block_template::GetBlockTemplate,
-    settings: &insta::Settings,
-) {
-    settings.bind(|| insta::assert_json_snapshot!("get_block_template", block_template));
-}
-
 /// Utility function to convert a `Network` to a lowercase string.
 fn network_string(network: Network) -> String {
     let mut net_suffix = network.to_string();

zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs

@@ -0,0 +1,84 @@
+//! Snapshot tests for getblocktemplate RPCs.
+//!
+//! To update these snapshots, run:
+//! ```sh
+//! cargo insta test --review --features getblocktemplate-rpcs --delete-unreferenced-snapshots
+//! ```
+
+use insta::Settings;
+use tower::{buffer::Buffer, Service};
+
+use zebra_node_services::mempool;
+use zebra_state::LatestChainTip;
+
+use zebra_test::mock_service::{MockService, PanicAssertion};
+
+use crate::methods::{GetBlockHash, GetBlockTemplateRpc, GetBlockTemplateRpcImpl};
+
+pub async fn test_responses<State>(
+    mempool: MockService<
+        mempool::Request,
+        mempool::Response,
+        PanicAssertion,
+        zebra_node_services::BoxError,
+    >,
+    read_state: State,
+    latest_chain_tip: LatestChainTip,
+    settings: Settings,
+) where
+    State: Service<
+            zebra_state::ReadRequest,
+            Response = zebra_state::ReadResponse,
+            Error = zebra_state::BoxError,
+        > + Clone
+        + Send
+        + Sync
+        + 'static,
+    <State as Service<zebra_state::ReadRequest>>::Future: Send,
+{
+    let get_block_template_rpc = GetBlockTemplateRpcImpl::new(
+        Buffer::new(mempool.clone(), 1),
+        read_state,
+        latest_chain_tip,
+    );
+
+    // `getblockcount`
+    let get_block_count = get_block_template_rpc
+        .get_block_count()
+        .expect("We should have a number");
+    snapshot_rpc_getblockcount(get_block_count, &settings);
+
+    // `getblockhash`
+    const BLOCK_HEIGHT10: i32 = 10;
+    let get_block_hash = get_block_template_rpc
+        .get_block_hash(BLOCK_HEIGHT10)
+        .await
+        .expect("We should have a GetBlockHash struct");
+
+    snapshot_rpc_getblockhash(get_block_hash, &settings);
+
+    // `getblocktemplate`
+    let get_block_template = get_block_template_rpc
+        .get_block_template()
+        .await
+        .expect("We should have a GetBlockTemplate struct");
+    snapshot_rpc_getblocktemplate(get_block_template, &settings);
+}
+
+/// Snapshot `getblockcount` response, using `cargo insta` and JSON serialization.
+fn snapshot_rpc_getblockcount(block_count: u32, settings: &insta::Settings) {
+    settings.bind(|| insta::assert_json_snapshot!("get_block_count", block_count));
+}
+
+/// Snapshot `getblockhash` response, using `cargo insta` and JSON serialization.
+fn snapshot_rpc_getblockhash(block_hash: GetBlockHash, settings: &insta::Settings) {
+    settings.bind(|| insta::assert_json_snapshot!("get_block_hash", block_hash));
+}
+
+/// Snapshot `getblocktemplate` response, using `cargo insta` and JSON serialization.
+fn snapshot_rpc_getblocktemplate(
+    block_template: crate::methods::get_block_template_rpcs::types::get_block_template::GetBlockTemplate,
+    settings: &insta::Settings,
+) {
+    settings.bind(|| insta::assert_json_snapshot!("get_block_template", block_template));
+}

zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_count@mainnet_10.snap

@@ -0,0 +1,5 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs
+expression: block_count
+---
+10

zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_count@testnet_10.snap

@@ -0,0 +1,5 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs
+expression: block_count
+---
+10

zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_hash@mainnet_10.snap

@@ -1,6 +1,5 @@
 ---
-source: zebra-rpc/src/methods/tests/snapshot.rs
-assertion_line: 270
+source: zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs
 expression: block_hash
 ---
 "00074c46a4aa8172df8ae2ad1848a2e084e1b6989b7d9e6132adc938bf835b36"

zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_hash@testnet_10.snap

@@ -1,6 +1,5 @@
 ---
-source: zebra-rpc/src/methods/tests/snapshot.rs
-assertion_line: 270
+source: zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs
 expression: block_hash
 ---
 "079f4c752729be63e6341ee9bce42fbbe37236aba22e3deb82405f3c2805c112"

zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template@mainnet_10.snap

@@ -1,6 +1,5 @@
 ---
-source: zebra-rpc/src/methods/tests/snapshot.rs
-assertion_line: 308
+source: zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs
 expression: block_template
 ---
 {

zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template@testnet_10.snap

@@ -1,6 +1,5 @@
 ---
-source: zebra-rpc/src/methods/tests/snapshot.rs
-assertion_line: 308
+source: zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs
 expression: block_template
 ---
 {

zebra-rpc/src/methods/tests/snapshots/get_block_count@mainnet_10.snap

@@ -1,6 +0,0 @@
----
-source: zebra-rpc/src/methods/tests/snapshot.rs
-assertion_line: 273
-expression: block_count
----
-10

zebra-rpc/src/methods/tests/snapshots/get_block_count@testnet_10.snap

@@ -1,6 +0,0 @@
----
-source: zebra-rpc/src/methods/tests/snapshot.rs
-assertion_line: 273
-expression: block_count
----
-10

zebra-rpc/src/methods/tests/vectors.rs

@@ -636,19 +636,12 @@ async fn rpc_getblockcount() {
         zebra_state::populated_state(blocks.clone(), Mainnet).await;
 
     // Init RPC
-    let (_rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-        "RPC test",
-        Mainnet,
-        false,
+    let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
         Buffer::new(mempool.clone(), 1),
-        read_state.clone(),
+        read_state,
         latest_chain_tip.clone(),
     );
 
-    // Init RPC
-    let get_block_template_rpc =
-        get_block_template_rpcs::GetBlockTemplateRpcImpl::new(latest_chain_tip.clone(), read_state);
-
     // Get the tip height using RPC method `get_block_count`
     let get_block_count = get_block_template_rpc
         .get_block_count()
@@ -658,10 +651,6 @@ async fn rpc_getblockcount() {
     assert_eq!(get_block_count, tip_block_height.0);
 
     mempool.expect_no_requests().await;
-
-    // The queue task should continue without errors or panics
-    let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-    assert!(matches!(rpc_tx_queue_task_result, None));
 }
 
 #[cfg(feature = "getblocktemplate-rpcs")]
@@ -676,18 +665,12 @@ async fn rpc_getblockcount_empty_state() {
         zebra_state::init_test_services(Mainnet);
 
     // Init RPC
-    let (_rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-        "RPC test",
-        Mainnet,
-        false,
+    let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
         Buffer::new(mempool.clone(), 1),
-        read_state.clone(),
+        read_state,
         latest_chain_tip.clone(),
     );
 
-    let get_block_template_rpc =
-        get_block_template_rpcs::GetBlockTemplateRpcImpl::new(latest_chain_tip.clone(), read_state);
-
     // Get the tip height using RPC method `get_block_count
     let get_block_count = get_block_template_rpc.get_block_count();
 
@@ -698,10 +681,6 @@ async fn rpc_getblockcount_empty_state() {
     assert_eq!(get_block_count.err().unwrap().message, "No blocks in state");
 
     mempool.expect_no_requests().await;
-
-    // The queue task should continue without errors or panics
-    let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-    assert!(matches!(rpc_tx_queue_task_result, None));
 }
 
 #[cfg(feature = "getblocktemplate-rpcs")]
@@ -720,17 +699,12 @@ async fn rpc_getblockhash() {
     let (_state, read_state, latest_chain_tip, _chain_tip_change) =
         zebra_state::populated_state(blocks.clone(), Mainnet).await;
 
-    // Init RPCs
-    let _rpc = RpcImpl::new(
-        "RPC test",
-        Mainnet,
-        false,
+    // Init RPC
+    let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
         Buffer::new(mempool.clone(), 1),
-        Buffer::new(read_state.clone(), 1),
+        read_state,
         latest_chain_tip.clone(),
     );
-    let get_block_template_rpc =
-        get_block_template_rpcs::GetBlockTemplateRpcImpl::new(latest_chain_tip, read_state);
 
     // Query the hashes using positive indexes
     for (i, block) in blocks.iter().enumerate() {
@@ -774,17 +748,12 @@ async fn rpc_getblocktemplate() {
     let (_state, read_state, latest_chain_tip, _chain_tip_change) =
         zebra_state::populated_state(blocks.clone(), Mainnet).await;
 
-    // Init RPCs
-    let _rpc = RpcImpl::new(
-        "RPC test",
-        Mainnet,
-        false,
+    // Init RPC
+    let get_block_template_rpc = get_block_template_rpcs::GetBlockTemplateRpcImpl::new(
         Buffer::new(mempool.clone(), 1),
-        Buffer::new(read_state.clone(), 1),
+        read_state,
         latest_chain_tip.clone(),
     );
-    let get_block_template_rpc =
-        get_block_template_rpcs::GetBlockTemplateRpcImpl::new(latest_chain_tip, read_state);
 
     let get_block_template = get_block_template_rpc
         .get_block_template()

zebra-rpc/src/server.rs

@@ -73,9 +73,12 @@ impl RpcServer {
 
             #[cfg(feature = "getblocktemplate-rpcs")]
             {
-                // Initialize the getblocktemplate rpc methods
-                let get_block_template_rpc_impl =
-                    GetBlockTemplateRpcImpl::new(latest_chain_tip.clone(), state.clone());
+                // Initialize the getblocktemplate rpc method handler
+                let get_block_template_rpc_impl = GetBlockTemplateRpcImpl::new(
+                    mempool.clone(),
+                    state.clone(),
+                    latest_chain_tip.clone(),
+                );
 
                 io.extend_with(get_block_template_rpc_impl.to_delegate());
             }

zebra-state/src/service/finalized_state/disk_format/tests/snapshot.rs

@@ -11,7 +11,7 @@
 //!
 //! If this test fails, run:
 //! ```sh
-//! cargo insta test --review --delete-unreferenced-snapshots
+//! cargo insta test --review
 //! ```
 //! to update the test snapshots, then commit the `test_*.snap` files using git.
 //!

zebra-state/src/service/finalized_state/zebra_db/block/tests/snapshot.rs

@@ -26,7 +26,7 @@
 //!
 //! If this test fails, run:
 //! ```sh
-//! cargo insta test --review --delete-unreferenced-snapshots
+//! cargo insta test --review
 //! ```
 //! to update the test snapshots, then commit the `test_*.snap` files using git.