add(rpc): Add extra `getblock` RPC fields used by some mining pools (#6097)

* clippy: remove unnecessary return statement

* Add hash, height, and confirmations fields to getblock RPC

* Remove a test that is already checked by snapshots

* Document the performance requirements of the getblock RPC

* Update snapshots, use new naming scheme

* Fix off-by-one error in confirmations

* Fix spelling mistakes

zebra-rpc/src/methods.rs

@@ -117,7 +117,7 @@ pub trait Rpc {
         raw_transaction_hex: String,
     ) -> BoxFuture<Result<SentTransactionHash>>;
 
-    /// Returns the requested block by height, as a [`GetBlock`] JSON string.
+    /// Returns the requested block by hash or height, as a [`GetBlock`] JSON string.
     /// If the block is not in Zebra's state, returns
     /// [error code `-8`.](https://github.com/zcash/zcash/issues/5758)
     ///
@@ -125,7 +125,7 @@ pub trait Rpc {
     ///
     /// # Parameters
     ///
-    /// - `height`: (string, required) The height number for the block to be returned.
+    /// - `hash|height`: (string, required) The hash or height for the block to be returned.
     /// - `verbosity`: (numeric, optional, default=1) 0 for hex encoded data, 1 for a json object,
     ///     and 2 for json object with transaction data.
     ///
@@ -133,13 +133,16 @@ pub trait Rpc {
     ///
     /// With verbosity=1, [`lightwalletd` only reads the `tx` field of the
     /// result](https://github.com/zcash/lightwalletd/blob/dfac02093d85fb31fb9a8475b884dd6abca966c7/common/common.go#L152),
-    /// so we only return that for now.
+    /// and other clients only read the `hash` and `confirmations` fields,
+    /// so we only return a few fields for now.
     ///
-    /// `lightwalletd` only requests blocks by height, so we don't support
+    /// `lightwalletd` and mining clients also do not use verbosity=2, so we don't support it.
-    /// getting blocks by hash. (But we parse the height as a JSON string, not an integer).
-    /// `lightwalletd` also does not use verbosity=2, so we don't support it.
     #[rpc(name = "getblock")]
-    fn get_block(&self, height: String, verbosity: Option<u8>) -> BoxFuture<Result<GetBlock>>;
+    fn get_block(
+        &self,
+        hash_or_height: String,
+        verbosity: Option<u8>,
+    ) -> BoxFuture<Result<GetBlock>>;
 
     /// Returns the hash of the current best blockchain tip block, as a [`GetBlockHash`] JSON string.
     ///
@@ -578,6 +581,10 @@ where
                     })?;
 
             if verbosity == 0 {
+                // # Performance
+                //
+                // This RPC is used in `lightwalletd`'s initial sync of 2 million blocks,
+                // so it needs to load block data very efficiently.
                 let request = zebra_state::ReadRequest::Block(hash_or_height);
                 let response = state
                     .ready()
@@ -601,6 +608,28 @@ where
                     _ => unreachable!("unmatched response to a block request"),
                 }
             } else if verbosity == 1 {
+                // # Performance
+                //
+                // This RPC is used in `lightwalletd`'s initial sync of 2 million blocks,
+                // so it needs to load all its fields very efficiently.
+                //
+                // Currently, we get the transaction IDs from an index, which is much more
+                // efficient than loading all the block data and hashing all the transactions.
+
+                // TODO: look up the hash if we only have a height,
+                //       and look up the height if we only have a hash
+                let hash = hash_or_height.hash().map(GetBlockHash);
+                let height = hash_or_height.height();
+
+                // TODO: do these state  queries in parallel?
+
+                // Get transaction IDs from the transaction index
+                //
+                // # Concurrency
+                //
+                // A block's transaction IDs are never modified, so all possible responses are
+                // valid. Clients that query block heights must be able to handle chain forks,
+                // including getting transaction IDs from any chain fork.
                 let request = zebra_state::ReadRequest::TransactionIdsForBlock(hash_or_height);
                 let response = state
                     .ready()
@@ -611,19 +640,62 @@ where
                         message: error.to_string(),
                         data: None,
                     })?;
-
+                let tx = match response {
-                match response {
                     zebra_state::ReadResponse::TransactionIdsForBlock(Some(tx_ids)) => {
-                        let tx_ids = tx_ids.iter().map(|tx_id| tx_id.encode_hex()).collect();
+                        tx_ids.iter().map(|tx_id| tx_id.encode_hex()).collect()
-                        Ok(GetBlock::Object { tx: tx_ids })
                     }
                     zebra_state::ReadResponse::TransactionIdsForBlock(None) => Err(Error {
                         code: MISSING_BLOCK_ERROR_CODE,
                         message: "Block not found".to_string(),
                         data: None,
-                    }),
+                    })?,
                     _ => unreachable!("unmatched response to a transaction_ids_for_block request"),
-                }
+                };
+
+                // Get block confirmations from the block height index
+                //
+                // # Concurrency
+                //
+                // All possible responses are valid, even if a block is added to the chain, or
+                // the best chain changes. Clients must be able to handle chain forks, including
+                // different confirmation values before or after added blocks, and switching
+                // between -1 and multiple different confirmation values.
+
+                // From <https://zcash.github.io/rpc/getblock.html>
+                const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1;
+
+                let confirmations = if let Some(hash) = hash_or_height.hash() {
+                    let request = zebra_state::ReadRequest::Depth(hash);
+                    let response = state
+                        .ready()
+                        .and_then(|service| service.call(request))
+                        .await
+                        .map_err(|error| Error {
+                            code: ErrorCode::ServerError(0),
+                            message: error.to_string(),
+                            data: None,
+                        })?;
+
+                    match response {
+                        // Confirmations are one more than the depth.
+                        // Depth is limited by height, so it will never overflow an i64.
+                        zebra_state::ReadResponse::Depth(Some(depth)) => Some(i64::from(depth) + 1),
+                        zebra_state::ReadResponse::Depth(None) => {
+                            Some(NOT_IN_BEST_CHAIN_CONFIRMATIONS)
+                        }
+                        _ => unreachable!("unmatched response to a depth request"),
+                    }
+                } else {
+                    // TODO: make Depth support heights as well
+                    None
+                };
+
+                Ok(GetBlock::Object {
+                    hash,
+                    confirmations,
+                    height,
+                    tx,
+                })
             } else {
                 Err(Error {
                     code: ErrorCode::InvalidParams,
@@ -1221,7 +1293,22 @@ pub enum GetBlock {
     Raw(#[serde(with = "hex")] SerializedBlock),
     /// The block object.
     Object {
+        /// The hash of the requested block.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        hash: Option<GetBlockHash>,
+
+        /// The number of confirmations of this block in the best chain,
+        /// or -1 if it is not in the best chain.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        confirmations: Option<i64>,
+
+        /// The height of the requested block.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        height: Option<Height>,
+
         /// List of transaction IDs in block order, hex-encoded.
+        //
+        // TODO: use a typed Vec<transaction::Hash> here
         tx: Vec<String>,
     },
 }
@@ -1232,6 +1319,7 @@ pub enum GetBlock {
 ///
 /// Also see the notes for the [`Rpc::get_best_block_hash`] and `get_block_hash` methods.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
+#[serde(transparent)]
 pub struct GetBlockHash(#[serde(with = "hex")] pub block::Hash);
 
 /// Response to a `z_gettreestate` RPC request.

zebra-rpc/src/methods/get_block_template_rpcs.rs

@@ -831,12 +831,12 @@ where
                 return Ok(validate_address::Response::invalid());
             }
 
-            return Ok(if address.network() == network {
+            if address.network() == network {
-                validate_address::Response {
+                Ok(validate_address::Response {
                     address: Some(raw_address),
                     is_valid: true,
                     is_script: Some(address.is_script_hash()),
-                }
+                })
             } else {
                 tracing::info!(
                     ?network,
@@ -844,8 +844,8 @@ where
                     "invalid address in validateaddress RPC: Zebra's configured network must match address network"
                 );
 
-                validate_address::Response::invalid()
+                Ok(validate_address::Response::invalid())
-            });
+            }
         }
         .boxed()
     }

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

@@ -103,27 +103,61 @@ async fn test_rpc_response_data_for_network(network: Network) {
         .expect("We should have an AddressBalance struct");
     snapshot_rpc_getaddressbalance(get_address_balance, &settings);
 
-    // `getblock`, verbosity=0
+    // `getblock` variants
     const BLOCK_HEIGHT: u32 = 1;
+    let block_hash = blocks[BLOCK_HEIGHT as usize].hash();
+
+    // `getblock`, verbosity=0, height
     let get_block = rpc
         .get_block(BLOCK_HEIGHT.to_string(), Some(0u8))
         .await
         .expect("We should have a GetBlock struct");
-    snapshot_rpc_getblock(get_block, block_data.get(&BLOCK_HEIGHT).unwrap(), &settings);
+    snapshot_rpc_getblock_data(
+        "height_verbosity_0",
+        get_block,
+        block_data.get(&BLOCK_HEIGHT).unwrap(),
+        &settings,
+    );
 
-    // `getblock`, verbosity=1
+    // `getblock`, verbosity=0, hash
+    let get_block = rpc
+        .get_block(block_hash.to_string(), Some(0u8))
+        .await
+        .expect("We should have a GetBlock struct");
+    snapshot_rpc_getblock_data(
+        "hash_verbosity_0",
+        get_block,
+        block_data.get(&BLOCK_HEIGHT).unwrap(),
+        &settings,
+    );
+
+    // `getblock`, verbosity=1, height
     let get_block = rpc
         .get_block(BLOCK_HEIGHT.to_string(), Some(1u8))
         .await
         .expect("We should have a GetBlock struct");
-    snapshot_rpc_getblock_verbose("2_args", get_block, &settings);
+    snapshot_rpc_getblock_verbose("height_verbosity_1", get_block, &settings);
 
-    // `getblock`, no verbosity, defaults to 1
+    // `getblock`, verbosity=1, hash
+    let get_block = rpc
+        .get_block(block_hash.to_string(), Some(1u8))
+        .await
+        .expect("We should have a GetBlock struct");
+    snapshot_rpc_getblock_verbose("hash_verbosity_1", get_block, &settings);
+
+    // `getblock`, no verbosity - defaults to 1, height
     let get_block = rpc
         .get_block(BLOCK_HEIGHT.to_string(), None)
         .await
         .expect("We should have a GetBlock struct");
-    snapshot_rpc_getblock_verbose("1_arg", get_block, &settings);
+    snapshot_rpc_getblock_verbose("height_verbosity_default", get_block, &settings);
+
+    // `getblock`, no verbosity - defaults to 1, hash
+    let get_block = rpc
+        .get_block(block_hash.to_string(), None)
+        .await
+        .expect("We should have a GetBlock struct");
+    snapshot_rpc_getblock_verbose("hash_verbosity_default", get_block, &settings);
 
     // `getbestblockhash`
     let get_best_block_hash = rpc
@@ -242,11 +276,16 @@ fn snapshot_rpc_getaddressbalance(address_balance: AddressBalance, settings: &in
 /// Check `getblock` response, using `cargo insta`, JSON serialization, and block test vectors.
 ///
 /// The snapshot file does not contain any data, but it does enforce the response format.
-fn snapshot_rpc_getblock(block: GetBlock, block_data: &[u8], settings: &insta::Settings) {
+fn snapshot_rpc_getblock_data(
+    variant: &'static str,
+    block: GetBlock,
+    block_data: &[u8],
+    settings: &insta::Settings,
+) {
     let block_data = hex::encode(block_data);
 
     settings.bind(|| {
-        insta::assert_json_snapshot!("get_block", block, {
+        insta::assert_json_snapshot!(format!("get_block_data_{variant}"), block, {
             "." => dynamic_redaction(move |value, _path| {
                 // assert that the block data matches, without creating a 1.5 kB snapshot file
                 assert_eq!(value.as_str().unwrap(), block_data);

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

@@ -0,0 +1,5 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: block
+---
+"[BlockData]"

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

@@ -0,0 +1,5 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: block
+---
+"[BlockData]"

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

@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: block
+---
+{
+  "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283",
+  "confirmations": 10,
+  "tx": [
+    "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609"
+  ]
+}

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

@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: block
+---
+{
+  "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23",
+  "confirmations": 10,
+  "tx": [
+    "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75"
+  ]
+}

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

@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: block
+---
+{
+  "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283",
+  "confirmations": 10,
+  "tx": [
+    "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609"
+  ]
+}

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

@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: block
+---
+{
+  "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23",
+  "confirmations": 10,
+  "tx": [
+    "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75"
+  ]
+}

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

@@ -3,6 +3,7 @@ source: zebra-rpc/src/methods/tests/snapshot.rs
 expression: block
 ---
 {
+  "height": 1,
   "tx": [
     "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609"
   ]

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

@@ -3,6 +3,7 @@ source: zebra-rpc/src/methods/tests/snapshot.rs
 expression: block
 ---
 {
+  "height": 1,
   "tx": [
     "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75"
   ]

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

@@ -3,6 +3,7 @@ source: zebra-rpc/src/methods/tests/snapshot.rs
 expression: block
 ---
 {
+  "height": 1,
   "tx": [
     "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609"
   ]

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

@@ -3,6 +3,7 @@ source: zebra-rpc/src/methods/tests/snapshot.rs
 expression: block
 ---
 {
+  "height": 1,
   "tx": [
     "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75"
   ]

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

@@ -80,7 +80,7 @@ async fn rpc_getblock() {
         latest_chain_tip,
     );
 
-    // Make calls with verbosity=0 and check response
+    // Make height calls with verbosity=0 and check response
     for (i, block) in blocks.iter().enumerate() {
         let expected_result = GetBlock::Raw(block.clone().into());
 
@@ -99,7 +99,26 @@ async fn rpc_getblock() {
         assert_eq!(get_block, expected_result);
     }
 
-    // Make calls with verbosity=1 and check response
+    // Make hash calls with verbosity=0 and check response
+    for (i, block) in blocks.iter().enumerate() {
+        let expected_result = GetBlock::Raw(block.clone().into());
+
+        let get_block = rpc
+            .get_block(blocks[i].hash().to_string(), Some(0u8))
+            .await
+            .expect("We should have a GetBlock struct");
+
+        assert_eq!(get_block, expected_result);
+
+        let get_block = rpc
+            .get_block(block.hash().to_string(), Some(0u8))
+            .await
+            .expect("We should have a GetBlock struct");
+
+        assert_eq!(get_block, expected_result);
+    }
+
+    // Make height calls with verbosity=1 and check response
     for (i, block) in blocks.iter().enumerate() {
         let get_block = rpc
             .get_block(i.to_string(), Some(1u8))
@@ -109,15 +128,41 @@ async fn rpc_getblock() {
         assert_eq!(
             get_block,
             GetBlock::Object {
+                hash: None,
+                confirmations: None,
+                height: Some(Height(i.try_into().expect("valid u32"))),
                 tx: block
                     .transactions
                     .iter()
                     .map(|tx| tx.hash().encode_hex())
-                    .collect()
+                    .collect(),
             }
         );
     }
 
+    // Make hash calls with verbosity=1 and check response
+    for (i, block) in blocks.iter().enumerate() {
+        let get_block = rpc
+            .get_block(blocks[i].hash().to_string(), Some(1u8))
+            .await
+            .expect("We should have a GetBlock struct");
+
+        assert_eq!(
+            get_block,
+            GetBlock::Object {
+                hash: Some(GetBlockHash(block.hash())),
+                confirmations: Some((blocks.len() - i).try_into().expect("valid i64")),
+                height: None,
+                tx: block
+                    .transactions
+                    .iter()
+                    .map(|tx| tx.hash().encode_hex())
+                    .collect(),
+            }
+        );
+    }
+
+    // Make height calls with no verbosity (defaults to 1) and check response
     for (i, block) in blocks.iter().enumerate() {
         let get_block = rpc
             .get_block(i.to_string(), None)
@@ -127,11 +172,36 @@ async fn rpc_getblock() {
         assert_eq!(
             get_block,
             GetBlock::Object {
+                hash: None,
+                confirmations: None,
+                height: Some(Height(i.try_into().expect("valid u32"))),
                 tx: block
                     .transactions
                     .iter()
                     .map(|tx| tx.hash().encode_hex())
-                    .collect()
+                    .collect(),
+            }
+        );
+    }
+
+    // Make hash calls with no verbosity (defaults to 1) and check response
+    for (i, block) in blocks.iter().enumerate() {
+        let get_block = rpc
+            .get_block(blocks[i].hash().to_string(), None)
+            .await
+            .expect("We should have a GetBlock struct");
+
+        assert_eq!(
+            get_block,
+            GetBlock::Object {
+                hash: Some(GetBlockHash(block.hash())),
+                confirmations: Some((blocks.len() - i).try_into().expect("valid i64")),
+                height: None,
+                tx: block
+                    .transactions
+                    .iter()
+                    .map(|tx| tx.hash().encode_hex())
+                    .collect(),
             }
         );
     }

zebra-rpc/src/tests/vectors.rs

@@ -27,12 +27,4 @@ pub fn test_block_serialization() {
     let j = serde_json::to_string(&expected_tx).unwrap();
 
     assert_eq!(j, expected_json);
-
-    let expected_tx = GetBlock::Object {
-        tx: vec!["42".into()],
-    };
-    let expected_json = r#"{"tx":["42"]}"#;
-    let j = serde_json::to_string(&expected_tx).unwrap();
-
-    assert_eq!(j, expected_json);
 }

zebra-state/src/request.rs

@@ -52,6 +52,24 @@ impl HashOrHeight {
             HashOrHeight::Height(height) => Some(height),
         }
     }
+
+    /// Returns the hash if this is a [`HashOrHeight::Hash`].
+    pub fn hash(&self) -> Option<block::Hash> {
+        if let HashOrHeight::Hash(hash) = self {
+            Some(*hash)
+        } else {
+            None
+        }
+    }
+
+    /// Returns the height if this is a [`HashOrHeight::Height`].
+    pub fn height(&self) -> Option<block::Height> {
+        if let HashOrHeight::Height(height) = self {
+            Some(*height)
+        } else {
+            None
+        }
+    }
 }
 
 impl From<block::Hash> for HashOrHeight {