From 6a27482c953e8e9289e2e41ae8c7b04c18aae803 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 5 Sep 2024 13:07:43 +0200
Subject: [PATCH 01/45] clean-up: simplify the def of `MapServerError`

---
 zebra-rpc/src/methods/errors.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/zebra-rpc/src/methods/errors.rs b/zebra-rpc/src/methods/errors.rs
index be9231d058d..98139f6fa2e 100644
--- a/zebra-rpc/src/methods/errors.rs
+++ b/zebra-rpc/src/methods/errors.rs
@@ -2,7 +2,7 @@
 
 use jsonrpc_core::ErrorCode;
 
-pub(crate) trait MapServerError<T, E> {
+pub(crate) trait MapServerError<T> {
     fn map_server_error(self) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
@@ -13,7 +13,7 @@ pub(crate) trait OkOrServerError<T> {
     ) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
-impl<T, E> MapServerError<T, E> for Result<T, E>
+impl<T, E> MapServerError<T> for Result<T, E>
 where
     E: ToString,
 {

From 4827028703efcdf4a1e0ddce030e1ed586db6088 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Mon, 21 Oct 2024 21:34:51 +0200
Subject: [PATCH 02/45] Use `HexData` instead of `String` for TXIDs

---
 zebra-rpc/src/methods.rs | 66 +++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 38 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 736021c7668..b802712be7b 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -11,6 +11,7 @@ use std::{collections::HashSet, fmt::Debug, sync::Arc};
 use chrono::Utc;
 use futures::{stream::FuturesOrdered, FutureExt, StreamExt, TryFutureExt};
 use hex::{FromHex, ToHex};
+use hex_data::HexData;
 use indexmap::IndexMap;
 use jsonrpc_core::{self, BoxFuture, Error, ErrorCode, Result};
 use jsonrpc_derive::rpc;
@@ -257,7 +258,7 @@ pub trait Rpc {
     #[rpc(name = "getrawtransaction")]
     fn get_raw_transaction(
         &self,
-        txid_hex: String,
+        txid: HexData,
         verbose: Option<u8>,
     ) -> BoxFuture<Result<GetRawTransaction>>;
 
@@ -1005,64 +1006,53 @@ where
         .boxed()
     }
 
-    // TODO: use HexData or SentTransactionHash to handle the transaction ID
     fn get_raw_transaction(
         &self,
-        txid_hex: String,
+        HexData(txid): HexData,
         verbose: Option<u8>,
     ) -> BoxFuture<Result<GetRawTransaction>> {
         let mut state = self.state.clone();
         let mut mempool = self.mempool.clone();
-        let verbose = verbose.unwrap_or(0);
-        let verbose = verbose != 0;
+        let verbose = verbose.unwrap_or(0) != 0;
 
         async move {
-            let txid = transaction::Hash::from_hex(txid_hex).map_err(|_| {
-                Error::invalid_params("transaction ID is not specified as a hex string")
-            })?;
+            let txid = transaction::Hash::from_bytes_in_display_order(
+                &txid
+                    .try_into()
+                    .map_err(|_| Error::invalid_params("invalid TXID length"))?,
+            );
 
             // Check the mempool first.
-            //
-            // # Correctness
-            //
-            // Transactions are removed from the mempool after they are mined into blocks,
-            // so the transaction could be just in the mempool, just in the state, or in both.
-            // (And the mempool and state transactions could have different authorising data.)
-            // But it doesn't matter which transaction we choose, because the effects are the same.
-            let mut txid_set = HashSet::new();
-            txid_set.insert(txid);
-            let request = mempool::Request::TransactionsByMinedId(txid_set);
-
-            let response = mempool
+            match mempool
                 .ready()
-                .and_then(|service| service.call(request))
+                .and_then(|service| {
+                    service.call(mempool::Request::TransactionsByMinedId(HashSet::from([
+                        txid,
+                    ])))
+                })
                 .await
-                .map_server_error()?;
-
-            match response {
-                mempool::Response::Transactions(unmined_transactions) => {
-                    if !unmined_transactions.is_empty() {
-                        let tx = unmined_transactions[0].transaction.clone();
-                        return Ok(GetRawTransaction::from_transaction(tx, None, 0, verbose));
+                .map_server_error()?
+            {
+                mempool::Response::Transactions(txns) => {
+                    if let Some(tx) = txns.first() {
+                        return Ok(GetRawTransaction::Raw(tx.transaction.clone().into()));
                     }
                 }
-                _ => unreachable!("unmatched response to a transactionids request"),
+                _ => unreachable!("unmatched response to a `TransactionsByMinedId` request"),
             };
 
-            // Now check the state
-            let request = zebra_state::ReadRequest::Transaction(txid);
-            let response = state
+            // If the tx wasn't in the mempool, check the state.
+            match state
                 .ready()
-                .and_then(|service| service.call(request))
+                .and_then(|service| service.call(zebra_state::ReadRequest::Transaction(txid)))
                 .await
-                .map_server_error()?;
-
-            match response {
+                .map_server_error()?
+            {
                 zebra_state::ReadResponse::Transaction(Some(MinedTx {
                     tx,
                     height,
                     confirmations,
-                })) => Ok(GetRawTransaction::from_transaction(
+                })) => Ok(GetRawTransaction::from_mined_tx(
                     tx,
                     Some(height),
                     confirmations,
@@ -1798,7 +1788,7 @@ pub struct GetAddressTxIdsRequest {
 impl GetRawTransaction {
     /// Converts `tx` and `height` into a new `GetRawTransaction` in the `verbose` format.
     #[allow(clippy::unwrap_in_result)]
-    fn from_transaction(
+    fn from_mined_tx(
         tx: Arc<Transaction>,
         height: Option<block::Height>,
         confirmations: u32,

From 1dcd49a0900af7e62365bc102c40e2504c16f20f Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Mon, 21 Oct 2024 21:35:32 +0200
Subject: [PATCH 03/45] Remove a redundant test

We don't need such a test anymore because the deserialization is handled
by Serde now.
---
 zebra-rpc/src/methods/tests/prop.rs | 64 +++--------------------------
 1 file changed, 5 insertions(+), 59 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs
index 409a6aefe52..f2e5e45d12a 100644
--- a/zebra-rpc/src/methods/tests/prop.rs
+++ b/zebra-rpc/src/methods/tests/prop.rs
@@ -447,71 +447,17 @@ proptest! {
         })?;
     }
 
-    /// Test that the method rejects non-hexadecimal characters.
-    ///
-    /// Try to call `get_raw_transaction` using a string parameter that has at least one
-    /// non-hexadecimal character, and check that it fails with an expected error.
-    #[test]
-    fn get_raw_transaction_non_hexadecimal_string_results_in_an_error(
-        non_hex_string in ".*[^0-9A-Fa-f].*",
-    ) {
-        let (runtime, _init_guard) = zebra_test::init_async();
-        let _guard = runtime.enter();
-
-        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
-        tokio::time::pause();
-
-        runtime.block_on(async move {
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
-
-            let send_task = tokio::spawn(rpc.get_raw_transaction(non_hex_string, Some(0)));
-
-            mempool.expect_no_requests().await?;
-            state.expect_no_requests().await?;
-
-            let result = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
-
-            prop_assert!(
-                matches!(
-                    result,
-                    Err(Error {
-                        code: ErrorCode::InvalidParams,
-                        ..
-                    })
-                ),
-                "Result is not an invalid parameters error: {result:?}"
-            );
-
-            // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
-
-            Ok::<_, TestCaseError>(())
-        })?;
-    }
-
     /// Test that the method rejects an input that's not a transaction.
     ///
-    /// Try to call `get_raw_transaction` using random bytes that fail to deserialize as a
-    /// transaction, and check that it fails with an expected error.
+    /// Try to call `get_raw_transaction` using random bytes that fail to deserialize as a txid, and
+    /// check that it fails with an expected error.
     #[test]
     fn get_raw_transaction_invalid_transaction_results_in_an_error(
         random_bytes in any::<Vec<u8>>(),
     ) {
+        if random_bytes.len() == 32 {
+            return Ok::<_, TestCaseError>(());
+        }
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
 

From 1cfe1cc8b470e13b45add4d496a629e6b702a65b Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Mon, 21 Oct 2024 21:36:45 +0200
Subject: [PATCH 04/45] Adjust tests for using `HexData`

---
 zebra-rpc/src/methods/tests/prop.rs     |  4 +++-
 zebra-rpc/src/methods/tests/snapshot.rs | 23 +++++++++++++----------
 zebra-rpc/src/methods/tests/vectors.rs  | 17 ++++++++++-------
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs
index f2e5e45d12a..25c841f650f 100644
--- a/zebra-rpc/src/methods/tests/prop.rs
+++ b/zebra-rpc/src/methods/tests/prop.rs
@@ -28,6 +28,8 @@ use zebra_state::BoxError;
 
 use zebra_test::mock_service::MockService;
 
+use crate::methods::hex_data::HexData;
+
 use super::super::{
     AddressBalance, AddressStrings, NetworkUpgradeStatus, Rpc, RpcImpl, SentTransactionHash,
 };
@@ -481,7 +483,7 @@ proptest! {
                 NoChainTip,
             );
 
-            let send_task = tokio::spawn(rpc.get_raw_transaction(hex::encode(random_bytes), Some(0)));
+            let send_task = tokio::spawn(rpc.get_raw_transaction(HexData(random_bytes), Some(0)));
 
             mempool.expect_no_requests().await?;
             state.expect_no_requests().await?;
diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs
index f4d7804088e..f8fd9d6ac16 100644
--- a/zebra-rpc/src/methods/tests/snapshot.rs
+++ b/zebra-rpc/src/methods/tests/snapshot.rs
@@ -229,12 +229,10 @@ async fn test_rpc_response_data_for_network(network: &Network) {
     snapshot_rpc_getblockchaininfo("", get_blockchain_info, &settings);
 
     // get the first transaction of the first block which is not the genesis
-    let first_block_first_transaction = &blocks[1].transactions[0];
+    let first_block_first_tx = &blocks[1].transactions[0];
 
     // build addresses
-    let address = &first_block_first_transaction.outputs()[1]
-        .address(network)
-        .unwrap();
+    let address = &first_block_first_tx.outputs()[1].address(network).unwrap();
     let addresses = vec![address.to_string()];
 
     // `getaddressbalance`
@@ -388,10 +386,16 @@ async fn test_rpc_response_data_for_network(network: &Network) {
             responder.respond(mempool::Response::Transactions(vec![]));
         });
 
+    let txid = HexData(
+        first_block_first_tx
+            .hash()
+            .bytes_in_display_order()
+            .to_vec(),
+    );
+
     // make the api call
-    let get_raw_transaction =
-        rpc.get_raw_transaction(first_block_first_transaction.hash().encode_hex(), Some(0u8));
-    let (response, _) = futures::join!(get_raw_transaction, mempool_req);
+    let get_raw_tx = rpc.get_raw_transaction(txid.clone(), Some(0u8));
+    let (response, _) = futures::join!(get_raw_tx, mempool_req);
     let get_raw_transaction = response.expect("We should have a GetRawTransaction struct");
 
     snapshot_rpc_getrawtransaction("verbosity_0", get_raw_transaction, &settings);
@@ -406,9 +410,8 @@ async fn test_rpc_response_data_for_network(network: &Network) {
         });
 
     // make the api call
-    let get_raw_transaction =
-        rpc.get_raw_transaction(first_block_first_transaction.hash().encode_hex(), Some(1u8));
-    let (response, _) = futures::join!(get_raw_transaction, mempool_req);
+    let get_raw_tx = rpc.get_raw_transaction(txid, Some(1u8));
+    let (response, _) = futures::join!(get_raw_tx, mempool_req);
     let get_raw_transaction = response.expect("We should have a GetRawTransaction struct");
 
     snapshot_rpc_getrawtransaction("verbosity_1", get_raw_transaction, &settings);
diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
index 5b5a21e23d0..7b144e5a6ef 100644
--- a/zebra-rpc/src/methods/tests/vectors.rs
+++ b/zebra-rpc/src/methods/tests/vectors.rs
@@ -473,8 +473,9 @@ async fn rpc_getrawtransaction() {
                         conventional_fee: Amount::zero(),
                     }]));
                 });
-            let get_tx_req = rpc.get_raw_transaction(tx.hash().encode_hex(), Some(0u8));
-            let (response, _) = futures::join!(get_tx_req, mempool_req);
+            let txid = HexData(tx.hash().bytes_in_display_order().to_vec());
+            let (response, _) =
+                futures::join!(rpc.get_raw_transaction(txid, Some(0u8)), mempool_req);
             let get_tx = response.expect("We should have a GetRawTransaction struct");
             if let GetRawTransaction::Raw(raw_tx) = get_tx {
                 assert_eq!(raw_tx.as_ref(), tx.zcash_serialize_to_vec().unwrap());
@@ -503,12 +504,14 @@ async fn rpc_getrawtransaction() {
 
     let run_state_test_case = |block_idx: usize, block: Arc<Block>, tx: Arc<Transaction>| {
         let read_state = read_state.clone();
-        let tx_hash = tx.hash();
-        let get_tx_verbose_0_req = rpc.get_raw_transaction(tx_hash.encode_hex(), Some(0u8));
-        let get_tx_verbose_1_req = rpc.get_raw_transaction(tx_hash.encode_hex(), Some(1u8));
+        let txid = tx.hash();
+        let hex_txid = HexData(txid.bytes_in_display_order().to_vec());
+
+        let get_tx_verbose_0_req = rpc.get_raw_transaction(hex_txid.clone(), Some(0u8));
+        let get_tx_verbose_1_req = rpc.get_raw_transaction(hex_txid, Some(1u8));
 
         async move {
-            let (response, _) = futures::join!(get_tx_verbose_0_req, make_mempool_req(tx_hash));
+            let (response, _) = futures::join!(get_tx_verbose_0_req, make_mempool_req(txid));
             let get_tx = response.expect("We should have a GetRawTransaction struct");
             if let GetRawTransaction::Raw(raw_tx) = get_tx {
                 assert_eq!(raw_tx.as_ref(), tx.zcash_serialize_to_vec().unwrap());
@@ -516,7 +519,7 @@ async fn rpc_getrawtransaction() {
                 unreachable!("Should return a Raw enum")
             }
 
-            let (response, _) = futures::join!(get_tx_verbose_1_req, make_mempool_req(tx_hash));
+            let (response, _) = futures::join!(get_tx_verbose_1_req, make_mempool_req(txid));
             let GetRawTransaction::Object {
                 hex,
                 height,

From 9aa561d8a6900d9a6bb4a3278a0d57b0e8d38493 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Tue, 12 Nov 2024 14:47:44 +0100
Subject: [PATCH 05/45] Make `height` and `confirmations` optional

---
 zebra-rpc/src/methods.rs                | 90 +++++++++++--------------
 zebra-rpc/src/methods/tests/snapshot.rs |  2 +-
 zebra-rpc/src/methods/tests/vectors.rs  | 17 +++--
 zebra-rpc/src/tests/vectors.rs          | 27 +++++---
 4 files changed, 71 insertions(+), 65 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index b802712be7b..c6b81819349 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -6,7 +6,7 @@
 //! Some parts of the `zcashd` RPC documentation are outdated.
 //! So this implementation follows the `zcashd` server and `lightwalletd` client implementations.
 
-use std::{collections::HashSet, fmt::Debug, sync::Arc};
+use std::{collections::HashSet, fmt::Debug};
 
 use chrono::Utc;
 use futures::{stream::FuturesOrdered, FutureExt, StreamExt, TryFutureExt};
@@ -30,7 +30,7 @@ use zebra_chain::{
     transparent::{self, Address},
 };
 use zebra_node_services::mempool;
-use zebra_state::{HashOrHeight, MinedTx, OutputIndex, OutputLocation, TransactionLocation};
+use zebra_state::{HashOrHeight, OutputIndex, OutputLocation, TransactionLocation};
 
 use crate::{
     constants::{INVALID_PARAMETERS_ERROR_CODE, MISSING_BLOCK_ERROR_CODE},
@@ -1035,9 +1035,20 @@ where
             {
                 mempool::Response::Transactions(txns) => {
                     if let Some(tx) = txns.first() {
-                        return Ok(GetRawTransaction::Raw(tx.transaction.clone().into()));
+                        let hex = tx.transaction.clone().into();
+
+                        return Ok(if verbose {
+                            GetRawTransaction::Object {
+                                hex,
+                                height: None,
+                                confirmations: None,
+                            }
+                        } else {
+                            GetRawTransaction::Raw(hex)
+                        });
                     }
                 }
+
                 _ => unreachable!("unmatched response to a `TransactionsByMinedId` request"),
             };
 
@@ -1048,20 +1059,26 @@ where
                 .await
                 .map_server_error()?
             {
-                zebra_state::ReadResponse::Transaction(Some(MinedTx {
-                    tx,
-                    height,
-                    confirmations,
-                })) => Ok(GetRawTransaction::from_mined_tx(
-                    tx,
-                    Some(height),
-                    confirmations,
-                    verbose,
-                )),
+                zebra_state::ReadResponse::Transaction(Some(tx)) => {
+                    let hex = tx.tx.into();
+
+                    Ok(if verbose {
+                        GetRawTransaction::Object {
+                            hex,
+                            height: Some(tx.height.0),
+                            confirmations: Some(tx.confirmations),
+                        }
+                    } else {
+                        GetRawTransaction::Raw(hex)
+                    })
+                }
+
                 zebra_state::ReadResponse::Transaction(None) => {
+                    // TODO: Return the correct err code (-5).
                     Err("Transaction not found").map_server_error()
                 }
-                _ => unreachable!("unmatched response to a transaction request"),
+
+                _ => unreachable!("unmatched response to a `Transaction` read request"),
             }
         }
         .boxed()
@@ -1706,12 +1723,14 @@ pub enum GetRawTransaction {
         /// The raw transaction, encoded as hex bytes.
         #[serde(with = "hex")]
         hex: SerializedTransaction,
-        /// The height of the block in the best chain that contains the transaction, or -1 if
-        /// the transaction is in the mempool.
-        height: i32,
-        /// The confirmations of the block in the best chain that contains the transaction,
-        /// or 0 if the transaction is in the mempool.
-        confirmations: u32,
+        /// The height of the block in the best chain that contains the tx or `None` if the tx is in
+        /// the mempool.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        height: Option<u32>,
+        /// The height diff between the block containing the tx and the best chain tip + 1 or `None`
+        /// if the tx is in the mempool.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        confirmations: Option<u32>,
     },
 }
 
@@ -1721,8 +1740,8 @@ impl Default for GetRawTransaction {
             hex: SerializedTransaction::from(
                 [0u8; zebra_chain::transaction::MIN_TRANSPARENT_TX_SIZE as usize].to_vec(),
             ),
-            height: i32::default(),
-            confirmations: u32::default(),
+            height: Option::default(),
+            confirmations: Option::default(),
         }
     }
 }
@@ -1785,33 +1804,6 @@ pub struct GetAddressTxIdsRequest {
     end: u32,
 }
 
-impl GetRawTransaction {
-    /// Converts `tx` and `height` into a new `GetRawTransaction` in the `verbose` format.
-    #[allow(clippy::unwrap_in_result)]
-    fn from_mined_tx(
-        tx: Arc<Transaction>,
-        height: Option<block::Height>,
-        confirmations: u32,
-        verbose: bool,
-    ) -> Self {
-        if verbose {
-            GetRawTransaction::Object {
-                hex: tx.into(),
-                height: match height {
-                    Some(height) => height
-                        .0
-                        .try_into()
-                        .expect("valid block heights are limited to i32::MAX"),
-                    None => -1,
-                },
-                confirmations,
-            }
-        } else {
-            GetRawTransaction::Raw(tx.into())
-        }
-    }
-}
-
 /// Information about the sapling and orchard note commitment trees if any.
 #[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
 pub struct GetBlockTrees {
diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs
index f8fd9d6ac16..4c06c5b0c69 100644
--- a/zebra-rpc/src/methods/tests/snapshot.rs
+++ b/zebra-rpc/src/methods/tests/snapshot.rs
@@ -5,7 +5,7 @@
 //! cargo insta test --review --release -p zebra-rpc --lib -- test_rpc_response_data
 //! ```
 
-use std::collections::BTreeMap;
+use std::{collections::BTreeMap, sync::Arc};
 
 use insta::dynamic_redaction;
 use tower::buffer::Buffer;
diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
index 7b144e5a6ef..fec31676a73 100644
--- a/zebra-rpc/src/methods/tests/vectors.rs
+++ b/zebra-rpc/src/methods/tests/vectors.rs
@@ -1,15 +1,17 @@
 //! Fixed test vectors for RPC methods.
 
 use std::ops::RangeInclusive;
+use std::sync::Arc;
 
 use tower::buffer::Buffer;
 
+use zebra_chain::serialization::ZcashSerialize;
 use zebra_chain::{
     amount::Amount,
     block::Block,
     chain_tip::{mock::MockChainTip, NoChainTip},
     parameters::Network::*,
-    serialization::{ZcashDeserializeInto, ZcashSerialize},
+    serialization::ZcashDeserializeInto,
     transaction::UnminedTxId,
 };
 use zebra_node_services::BoxError;
@@ -473,10 +475,11 @@ async fn rpc_getrawtransaction() {
                         conventional_fee: Amount::zero(),
                     }]));
                 });
+
             let txid = HexData(tx.hash().bytes_in_display_order().to_vec());
-            let (response, _) =
-                futures::join!(rpc.get_raw_transaction(txid, Some(0u8)), mempool_req);
-            let get_tx = response.expect("We should have a GetRawTransaction struct");
+            let (rsp, _) = futures::join!(rpc.get_raw_transaction(txid, Some(0u8)), mempool_req);
+            let get_tx = rsp.expect("We should have a GetRawTransaction struct");
+
             if let GetRawTransaction::Raw(raw_tx) = get_tx {
                 assert_eq!(raw_tx.as_ref(), tx.zcash_serialize_to_vec().unwrap());
             } else {
@@ -520,6 +523,7 @@ async fn rpc_getrawtransaction() {
             }
 
             let (response, _) = futures::join!(get_tx_verbose_1_req, make_mempool_req(txid));
+
             let GetRawTransaction::Object {
                 hex,
                 height,
@@ -529,8 +533,11 @@ async fn rpc_getrawtransaction() {
                 unreachable!("Should return a Raw enum")
             };
 
+            let height = height.expect("state requests should have height");
+            let confirmations = confirmations.expect("state requests should have confirmations");
+
             assert_eq!(hex.as_ref(), tx.zcash_serialize_to_vec().unwrap());
-            assert_eq!(height, block_idx as i32);
+            assert_eq!(height, block_idx as u32);
 
             let depth_response = read_state
                 .oneshot(zebra_state::ReadRequest::Depth(block.hash()))
diff --git a/zebra-rpc/src/tests/vectors.rs b/zebra-rpc/src/tests/vectors.rs
index 84ed937d6ef..a93158b8b38 100644
--- a/zebra-rpc/src/tests/vectors.rs
+++ b/zebra-rpc/src/tests/vectors.rs
@@ -4,21 +4,28 @@ use crate::methods::{GetBlock, GetRawTransaction};
 
 #[test]
 pub fn test_transaction_serialization() {
-    let expected_tx = GetRawTransaction::Raw(vec![0x42].into());
-    let expected_json = r#""42""#;
-    let j = serde_json::to_string(&expected_tx).unwrap();
+    let tx = GetRawTransaction::Raw(vec![0x42].into());
 
-    assert_eq!(j, expected_json);
+    assert_eq!(serde_json::to_string(&tx).unwrap(), r#""42""#);
 
-    let expected_tx = GetRawTransaction::Object {
+    let tx = GetRawTransaction::Object {
         hex: vec![0x42].into(),
-        height: 1,
-        confirmations: 0,
+        height: Some(1),
+        confirmations: Some(0),
     };
-    let expected_json = r#"{"hex":"42","height":1,"confirmations":0}"#;
-    let j = serde_json::to_string(&expected_tx).unwrap();
 
-    assert_eq!(j, expected_json);
+    assert_eq!(
+        serde_json::to_string(&tx).unwrap(),
+        r#"{"hex":"42","height":1,"confirmations":0}"#
+    );
+
+    let tx = GetRawTransaction::Object {
+        hex: vec![0x42].into(),
+        height: None,
+        confirmations: None,
+    };
+
+    assert_eq!(serde_json::to_string(&tx).unwrap(), r#"{"hex":"42"}"#);
 }
 
 #[test]

From a973aab730c9d2b0b3ee56c4eaeb4b1f8129a0ac Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 13 Nov 2024 14:41:01 +0100
Subject: [PATCH 06/45] Use legacy error codes

---
 zebra-rpc/src/constants.rs                    | 15 ---
 zebra-rpc/src/methods.rs                      | 26 ++++--
 zebra-rpc/src/methods/errors.rs               | 37 --------
 .../src/methods/get_block_template_rpcs.rs    | 54 +++++------
 .../get_block_template.rs                     |  6 +-
 zebra-rpc/src/server.rs                       |  1 +
 zebra-rpc/src/server/error.rs                 | 91 +++++++++++++++++++
 .../src/server/rpc_call_compatibility.rs      | 18 +++-
 zebra-rpc/src/sync.rs                         |  7 +-
 zebrad/tests/common/regtest.rs                |  5 +-
 10 files changed, 156 insertions(+), 104 deletions(-)
 delete mode 100644 zebra-rpc/src/methods/errors.rs
 create mode 100644 zebra-rpc/src/server/error.rs

diff --git a/zebra-rpc/src/constants.rs b/zebra-rpc/src/constants.rs
index e8be508595f..e173563f6e8 100644
--- a/zebra-rpc/src/constants.rs
+++ b/zebra-rpc/src/constants.rs
@@ -2,21 +2,6 @@
 
 use jsonrpc_core::{Error, ErrorCode};
 
-/// The RPC error code used by `zcashd` for incorrect RPC parameters.
-///
-/// [`jsonrpc_core`] uses these codes:
-/// <https://github.com/paritytech/jsonrpc/blob/609d7a6cc160742d035510fa89fb424ccf077660/core/src/types/error.rs#L25-L36>
-///
-/// `node-stratum-pool` mining pool library expects error code `-1` to detect available RPC methods:
-/// <https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L459>
-pub const INVALID_PARAMETERS_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-1);
-
-/// The RPC error code used by `zcashd` for missing blocks.
-///
-/// `lightwalletd` expects error code `-8` when a block is not found:
-/// <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>
-pub const MISSING_BLOCK_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-8);
-
 /// The RPC error code used by `zcashd` when there are no blocks in the state.
 ///
 /// `lightwalletd` expects error code `0` when there are no blocks in the state.
diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index c6b81819349..6aa41c08621 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -33,16 +33,16 @@ use zebra_node_services::mempool;
 use zebra_state::{HashOrHeight, OutputIndex, OutputLocation, TransactionLocation};
 
 use crate::{
-    constants::{INVALID_PARAMETERS_ERROR_CODE, MISSING_BLOCK_ERROR_CODE},
     methods::trees::{GetSubtrees, GetTreestate, SubtreeRpcData},
     queue::Queue,
+    server::{
+        self,
+        error::{MapServerError, OkOrServerError},
+    },
 };
 
-mod errors;
 pub mod hex_data;
 
-use errors::{MapServerError, OkOrServerError};
-
 // We don't use a types/ module here, because it is redundant.
 pub mod trees;
 
@@ -740,7 +740,9 @@ where
                         Ok(GetBlock::Raw(block.into()))
                     }
                     zebra_state::ReadResponse::Block(None) => Err(Error {
-                        code: MISSING_BLOCK_ERROR_CODE,
+                        // `lightwalletd` expects error code `-8` when a block is not found:
+                        // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>                   zebra_state::ReadResponse::Block(None) => Err(Error {
+                        code: server::error::LegacyCode::InvalidParameter.into(),
                         message: "Block not found".to_string(),
                         data: None,
                     }),
@@ -783,10 +785,12 @@ where
                             zebra_state::ReadResponse::BlockHash(Some(hash)) => hash,
                             zebra_state::ReadResponse::BlockHash(None) => {
                                 return Err(Error {
-                                    code: MISSING_BLOCK_ERROR_CODE,
+                                    // `lightwalletd` expects error code `-8` when a block is not found:
+                                    // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>                   zebra_state::ReadResponse::Block(None) => Err(Error {
+                                    code: server::error::LegacyCode::InvalidParameter.into(),
                                     message: "block height not in best chain".to_string(),
                                     data: None,
-                                })
+                                });
                             }
                             _ => unreachable!("unmatched response to a block hash request"),
                         }
@@ -1112,10 +1116,12 @@ where
                 zebra_state::ReadResponse::Block(Some(block)) => block,
                 zebra_state::ReadResponse::Block(None) => {
                     return Err(Error {
-                        code: MISSING_BLOCK_ERROR_CODE,
+                        // `lightwalletd` expects error code `-8` when a block is not found:
+                        // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>                   zebra_state::ReadResponse::Block(None) => Err(Error {
+                        code: server::error::LegacyCode::InvalidParameter.into(),
                         message: "the requested block was not found".to_string(),
                         data: None,
-                    })
+                    });
                 }
                 _ => unreachable!("unmatched response to a block request"),
             };
@@ -1237,7 +1243,7 @@ where
                 })
             } else {
                 Err(Error {
-                    code: INVALID_PARAMETERS_ERROR_CODE,
+                    code: server::error::LegacyCode::Misc.into(),
                     message: format!("invalid pool name, must be one of: {:?}", POOL_LIST),
                     data: None,
                 })
diff --git a/zebra-rpc/src/methods/errors.rs b/zebra-rpc/src/methods/errors.rs
deleted file mode 100644
index 98139f6fa2e..00000000000
--- a/zebra-rpc/src/methods/errors.rs
+++ /dev/null
@@ -1,37 +0,0 @@
-//! Error conversions for Zebra's RPC methods.
-
-use jsonrpc_core::ErrorCode;
-
-pub(crate) trait MapServerError<T> {
-    fn map_server_error(self) -> std::result::Result<T, jsonrpc_core::Error>;
-}
-
-pub(crate) trait OkOrServerError<T> {
-    fn ok_or_server_error<S: ToString>(
-        self,
-        message: S,
-    ) -> std::result::Result<T, jsonrpc_core::Error>;
-}
-
-impl<T, E> MapServerError<T> for Result<T, E>
-where
-    E: ToString,
-{
-    fn map_server_error(self) -> Result<T, jsonrpc_core::Error> {
-        self.map_err(|error| jsonrpc_core::Error {
-            code: ErrorCode::ServerError(0),
-            message: error.to_string(),
-            data: None,
-        })
-    }
-}
-
-impl<T> OkOrServerError<T> for Option<T> {
-    fn ok_or_server_error<S: ToString>(self, message: S) -> Result<T, jsonrpc_core::Error> {
-        self.ok_or(jsonrpc_core::Error {
-            code: ErrorCode::ServerError(0),
-            message: message.to_string(),
-            data: None,
-        })
-    }
-}
diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs
index 2d50552cfec..ecf1cf84b56 100644
--- a/zebra-rpc/src/methods/get_block_template_rpcs.rs
+++ b/zebra-rpc/src/methods/get_block_template_rpcs.rs
@@ -32,35 +32,37 @@ use zebra_network::AddressBookPeers;
 use zebra_node_services::mempool;
 use zebra_state::{ReadRequest, ReadResponse};
 
-use crate::methods::{
-    best_chain_tip_height,
-    errors::MapServerError,
-    get_block_template_rpcs::{
-        constants::{
-            DEFAULT_SOLUTION_RATE_WINDOW_SIZE, GET_BLOCK_TEMPLATE_MEMPOOL_LONG_POLL_INTERVAL,
-            ZCASHD_FUNDING_STREAM_ORDER,
-        },
-        get_block_template::{
-            check_miner_address, check_synced_to_tip, fetch_mempool_transactions,
-            fetch_state_tip_and_local_time, validate_block_proposal,
-        },
-        // TODO: move the types/* modules directly under get_block_template_rpcs,
-        //       and combine any modules with the same names.
-        types::{
+use crate::{
+    methods::{
+        best_chain_tip_height,
+        get_block_template_rpcs::{
+            constants::{
+                DEFAULT_SOLUTION_RATE_WINDOW_SIZE, GET_BLOCK_TEMPLATE_MEMPOOL_LONG_POLL_INTERVAL,
+                ZCASHD_FUNDING_STREAM_ORDER,
+            },
             get_block_template::{
-                proposal::TimeSource, proposal_block_from_template, GetBlockTemplate,
+                check_miner_address, check_synced_to_tip, fetch_mempool_transactions,
+                fetch_state_tip_and_local_time, validate_block_proposal,
+            },
+            // TODO: move the types/* modules directly under get_block_template_rpcs,
+            //       and combine any modules with the same names.
+            types::{
+                get_block_template::{
+                    proposal::TimeSource, proposal_block_from_template, GetBlockTemplate,
+                },
+                get_mining_info,
+                long_poll::LongPollInput,
+                peer_info::PeerInfo,
+                submit_block,
+                subsidy::{BlockSubsidy, FundingStream},
+                unified_address, validate_address, z_validate_address,
             },
-            get_mining_info,
-            long_poll::LongPollInput,
-            peer_info::PeerInfo,
-            submit_block,
-            subsidy::{BlockSubsidy, FundingStream},
-            unified_address, validate_address, z_validate_address,
         },
+        height_from_signed_int,
+        hex_data::HexData,
+        GetBlockHash,
     },
-    height_from_signed_int,
-    hex_data::HexData,
-    GetBlockHash, MISSING_BLOCK_ERROR_CODE,
+    server::{self, error::MapServerError},
 };
 
 pub mod constants;
@@ -589,7 +591,7 @@ where
             match response {
                 zebra_state::ReadResponse::BlockHash(Some(hash)) => Ok(GetBlockHash(hash)),
                 zebra_state::ReadResponse::BlockHash(None) => Err(Error {
-                    code: MISSING_BLOCK_ERROR_CODE,
+                    code: server::error::LegacyCode::InvalidParameter.into(),
                     message: "Block not found".to_string(),
                     data: None,
                 }),
diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
index 8e9578180be..3ef3d3130da 100644
--- a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
+++ b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
@@ -25,12 +25,12 @@ use zebra_consensus::{
 use zebra_node_services::mempool;
 use zebra_state::GetBlockTemplateChainInfo;
 
-use crate::methods::{
-    errors::OkOrServerError,
-    get_block_template_rpcs::{
+use crate::{
+    methods::get_block_template_rpcs::{
         constants::{MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP, NOT_SYNCED_ERROR_CODE},
         types::{default_roots::DefaultRoots, transaction::TransactionTemplate},
     },
+    server::error::OkOrServerError,
 };
 
 pub use crate::methods::get_block_template_rpcs::types::get_block_template::*;
diff --git a/zebra-rpc/src/server.rs b/zebra-rpc/src/server.rs
index 73fcde65f6b..69ab36d8c00 100644
--- a/zebra-rpc/src/server.rs
+++ b/zebra-rpc/src/server.rs
@@ -36,6 +36,7 @@ use crate::{
 use crate::methods::{GetBlockTemplateRpc, GetBlockTemplateRpcImpl};
 
 pub mod cookie;
+pub mod error;
 pub mod http_request_compatibility;
 pub mod rpc_call_compatibility;
 
diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
new file mode 100644
index 00000000000..8f9622d563d
--- /dev/null
+++ b/zebra-rpc/src/server/error.rs
@@ -0,0 +1,91 @@
+//! RPC error codes & their handling.
+
+/// Bitcoin RPC error codes
+///
+/// Drawn from https://github.com/zcash/zcash/blob/master/src/rpc/protocol.h#L32-L80.
+///
+/// ## Notes
+///
+/// - All explicit discriminants fit within `i64`.
+pub enum LegacyCode {
+    // General application defined errors
+
+    /// `std::exception` thrown in command handling
+    Misc = -1,
+    /// Server is in safe mode, and command is not allowed in safe mode
+    ForbiddenBySafeMode = -2,
+    /// Unexpected type was passed as parameter
+    Type = -3,
+    /// Invalid address or key
+    InvalidAddressOrKey = -5,
+    /// Ran out of memory during operation
+    OutOfMemory = -7,
+    /// Invalid, missing or duplicate parameter
+    InvalidParameter = -8,
+    /// Database error
+    Database = -20,
+    /// Error parsing or validating structure in raw format
+    Deserialization = -22,
+    /// General error during transaction or block submission
+    Verify = -25,
+    /// Transaction or block was rejected by network rules
+    VerifyRejected = -26,
+    /// Transaction already in chain
+    VerifyAlreadyInChain = -27,
+    /// Client still warming up
+    InWarmup = -28,
+
+    // P2P client errors
+    /// Bitcoin is not connected
+    ClientNotConnected = -9,
+    /// Still downloading initial blocks
+    ClientInInitialDownload = -10,
+    /// Node is already added
+    ClientNodeAlreadyAdded = -23,
+    /// Node has not been added before
+    ClientNodeNotAdded = -24,
+    /// Node to disconnect not found in connected nodes
+    ClientNodeNotConnected = -29,
+    /// Invalid IP/Subnet
+    ClientInvalidIpOrSubnet = -30,
+}
+
+impl From<LegacyCode> for jsonrpc_core::ErrorCode {
+    fn from(code: LegacyCode) -> Self {
+        Self::ServerError(code as i64)
+    }
+}
+
+pub(crate) trait MapServerError<T> {
+    fn map_server_error(self) -> std::result::Result<T, jsonrpc_core::Error>;
+}
+
+pub(crate) trait OkOrServerError<T> {
+    fn ok_or_server_error<S: ToString>(
+        self,
+        message: S,
+    ) -> std::result::Result<T, jsonrpc_core::Error>;
+}
+
+impl<T, E> MapServerError<T> for Result<T, E>
+where
+    E: ToString,
+{
+    fn map_server_error(self) -> Result<T, jsonrpc_core::Error> {
+        self.map_err(|error| jsonrpc_core::Error {
+            code: jsonrpc_core::ErrorCode::ServerError(0),
+            message: error.to_string(),
+            data: None,
+        })
+    }
+}
+
+impl<T> OkOrServerError<T> for Option<T> {
+    fn ok_or_server_error<S: ToString>(self, message: S) -> Result<T, jsonrpc_core::Error> {
+        self.ok_or(jsonrpc_core::Error {
+            code: jsonrpc_core::ErrorCode::ServerError(0),
+            message: message.to_string(),
+            data: None,
+        })
+    }
+}
diff --git a/zebra-rpc/src/server/rpc_call_compatibility.rs b/zebra-rpc/src/server/rpc_call_compatibility.rs
index c3974ac3cf8..5c3039efb92 100644
--- a/zebra-rpc/src/server/rpc_call_compatibility.rs
+++ b/zebra-rpc/src/server/rpc_call_compatibility.rs
@@ -6,13 +6,14 @@
 use std::future::Future;
 
 use futures::future::{Either, FutureExt};
+
 use jsonrpc_core::{
     middleware::Middleware,
     types::{Call, Failure, Output, Response},
-    BoxFuture, ErrorCode, Metadata, MethodCall, Notification,
+    BoxFuture, Metadata, MethodCall, Notification,
 };
 
-use crate::constants::{INVALID_PARAMETERS_ERROR_CODE, MAX_PARAMS_LOG_LENGTH};
+use crate::{constants::MAX_PARAMS_LOG_LENGTH, server};
 
 /// JSON-RPC [`Middleware`] with compatibility workarounds.
 ///
@@ -57,13 +58,20 @@ impl<M: Metadata> Middleware<M> for FixRpcResponseMiddleware {
 }
 
 impl FixRpcResponseMiddleware {
-    /// Replace [`jsonrpc_core`] server error codes in `output` with the `zcashd` equivalents.
+    /// Replaces [`jsonrpc_core::ErrorCode`]s in the [`Output`] with their `zcashd` equivalents.
+    ///
+    /// ## Replaced Codes
+    ///
+    /// 1. [`jsonrpc_core::ErrorCode::InvalidParams`] -> [`server::error::LegacyCode::Misc`]
+    ///    Rationale:
+    ///    The `node-stratum-pool` mining pool library expects error code `-1` to detect available RPC methods:
+    ///    <https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L459>
     fn fix_error_codes(output: &mut Option<Output>) {
         if let Some(Output::Failure(Failure { ref mut error, .. })) = output {
-            if matches!(error.code, ErrorCode::InvalidParams) {
+            if matches!(error.code, jsonrpc_core::ErrorCode::InvalidParams) {
                 let original_code = error.code.clone();
 
-                error.code = INVALID_PARAMETERS_ERROR_CODE;
+                error.code = server::error::LegacyCode::Misc.into();
                 tracing::debug!("Replacing RPC error: {original_code:?} with {error}");
             }
         }
diff --git a/zebra-rpc/src/sync.rs b/zebra-rpc/src/sync.rs
index fd323ef64bb..ccb5c3ba33b 100644
--- a/zebra-rpc/src/sync.rs
+++ b/zebra-rpc/src/sync.rs
@@ -20,10 +20,7 @@ use zebra_state::{
 
 use zebra_chain::diagnostic::task::WaitForPanics;
 
-use crate::{
-    constants::MISSING_BLOCK_ERROR_CODE,
-    methods::{hex_data::HexData, GetBlockHeightAndHash},
-};
+use crate::{methods::{hex_data::HexData, GetBlockHeightAndHash}, server};
 
 /// How long to wait between calls to `getbestblockheightandhash` when it:
 /// - Returns an error, or
@@ -383,7 +380,7 @@ impl SyncerRpcMethods for RpcRequestClient {
             Err(err)
                 if err
                     .downcast_ref::<jsonrpc_core::Error>()
-                    .is_some_and(|err| err.code == MISSING_BLOCK_ERROR_CODE) =>
+                    .is_some_and(|err| err.code == server::error::LegacyCode::InvalidParameter.into()) =>
             {
                 Ok(None)
             }
diff --git a/zebrad/tests/common/regtest.rs b/zebrad/tests/common/regtest.rs
index bf1cba697de..ccd9bf8aab7 100644
--- a/zebrad/tests/common/regtest.rs
+++ b/zebrad/tests/common/regtest.rs
@@ -17,7 +17,6 @@ use zebra_chain::{
 };
 use zebra_node_services::rpc_client::RpcRequestClient;
 use zebra_rpc::{
-    constants::MISSING_BLOCK_ERROR_CODE,
     methods::{
         get_block_template_rpcs::{
             get_block_template::{
@@ -27,7 +26,7 @@ use zebra_rpc::{
         },
         hex_data::HexData,
     },
-    server::OPENED_RPC_ENDPOINT_MSG,
+    server::{self, OPENED_RPC_ENDPOINT_MSG},
 };
 use zebra_test::args;
 
@@ -163,7 +162,7 @@ impl MiningRpcMethods for RpcRequestClient {
             Err(err)
                 if err
                     .downcast_ref::<jsonrpc_core::Error>()
-                    .is_some_and(|err| err.code == MISSING_BLOCK_ERROR_CODE) =>
+                    .is_some_and(|err| err.code == server::error::LegacyCode::InvalidParameter.into()) =>
             {
                 Ok(None)
             }

From 7ccac980dc00d0aef7bbeb0c1b1b32eccf029b5f Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 13 Nov 2024 14:44:07 +0100
Subject: [PATCH 07/45] fmt

---
 zebra-rpc/src/server/error.rs  | 1 -
 zebra-rpc/src/sync.rs          | 9 +++++++--
 zebrad/tests/common/regtest.rs | 4 +++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index 8f9622d563d..b5dbdeb518d 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -9,7 +9,6 @@
 /// - All explicit discriminants fit within `i64`.
 pub enum LegacyCode {
     // General application defined errors
-
     /// `std::exception` thrown in command handling
     Misc = -1,
     /// Server is in safe mode, and command is not allowed in safe mode
diff --git a/zebra-rpc/src/sync.rs b/zebra-rpc/src/sync.rs
index ccb5c3ba33b..40373d0eaed 100644
--- a/zebra-rpc/src/sync.rs
+++ b/zebra-rpc/src/sync.rs
@@ -20,7 +20,10 @@ use zebra_state::{
 
 use zebra_chain::diagnostic::task::WaitForPanics;
 
-use crate::{methods::{hex_data::HexData, GetBlockHeightAndHash}, server};
+use crate::{
+    methods::{hex_data::HexData, GetBlockHeightAndHash},
+    server,
+};
 
 /// How long to wait between calls to `getbestblockheightandhash` when it:
 /// - Returns an error, or
@@ -380,7 +383,9 @@ impl SyncerRpcMethods for RpcRequestClient {
             Err(err)
                 if err
                     .downcast_ref::<jsonrpc_core::Error>()
-                    .is_some_and(|err| err.code == server::error::LegacyCode::InvalidParameter.into()) =>
+                    .is_some_and(|err| {
+                        err.code == server::error::LegacyCode::InvalidParameter.into()
+                    }) =>
             {
                 Ok(None)
             }
diff --git a/zebrad/tests/common/regtest.rs b/zebrad/tests/common/regtest.rs
index ccd9bf8aab7..acd89d89aba 100644
--- a/zebrad/tests/common/regtest.rs
+++ b/zebrad/tests/common/regtest.rs
@@ -162,7 +162,9 @@ impl MiningRpcMethods for RpcRequestClient {
             Err(err)
                 if err
                     .downcast_ref::<jsonrpc_core::Error>()
-                    .is_some_and(|err| err.code == server::error::LegacyCode::InvalidParameter.into()) =>
+                    .is_some_and(|err| {
+                        err.code == server::error::LegacyCode::InvalidParameter.into()
+                    }) =>
             {
                 Ok(None)
             }

From 08234b7e08a09013fa2d943ccc6d51675e4725a2 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 13 Nov 2024 14:44:36 +0100
Subject: [PATCH 08/45] Remove unneeded error codes

---
 zebra-rpc/src/constants.rs | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/zebra-rpc/src/constants.rs b/zebra-rpc/src/constants.rs
index e173563f6e8..cf8714c2958 100644
--- a/zebra-rpc/src/constants.rs
+++ b/zebra-rpc/src/constants.rs
@@ -1,25 +1,4 @@
 //! Constants for RPC methods and server responses.
 
-use jsonrpc_core::{Error, ErrorCode};
-
-/// The RPC error code used by `zcashd` when there are no blocks in the state.
-///
-/// `lightwalletd` expects error code `0` when there are no blocks in the state.
-//
-// TODO: find the source code that expects or generates this error
-pub const NO_BLOCKS_IN_STATE_ERROR_CODE: ErrorCode = ErrorCode::ServerError(0);
-
-/// The RPC error used by `zcashd` when there are no blocks in the state.
-//
-// TODO: find the source code that expects or generates this error text, if there is any
-//       replace literal Error { ... } with this error
-pub fn no_blocks_in_state_error() -> Error {
-    Error {
-        code: NO_BLOCKS_IN_STATE_ERROR_CODE,
-        message: "No blocks in state".to_string(),
-        data: None,
-    }
-}
-
 /// When logging parameter data, only log this much data.
 pub const MAX_PARAMS_LOG_LENGTH: usize = 100;

From 3247923bcf4908e42693bdce5412508a28c33605 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 13 Nov 2024 14:46:48 +0100
Subject: [PATCH 09/45] Remove `zebra-rpc/src/constants.rs`

---
 zebra-rpc/src/constants.rs                     | 4 ----
 zebra-rpc/src/lib.rs                           | 1 -
 zebra-rpc/src/server/rpc_call_compatibility.rs | 4 +++-
 3 files changed, 3 insertions(+), 6 deletions(-)
 delete mode 100644 zebra-rpc/src/constants.rs

diff --git a/zebra-rpc/src/constants.rs b/zebra-rpc/src/constants.rs
deleted file mode 100644
index cf8714c2958..00000000000
--- a/zebra-rpc/src/constants.rs
+++ /dev/null
@@ -1,4 +0,0 @@
-//! Constants for RPC methods and server responses.
-
-/// When logging parameter data, only log this much data.
-pub const MAX_PARAMS_LOG_LENGTH: usize = 100;
diff --git a/zebra-rpc/src/lib.rs b/zebra-rpc/src/lib.rs
index 778788c9edf..a5c2f3e5a17 100644
--- a/zebra-rpc/src/lib.rs
+++ b/zebra-rpc/src/lib.rs
@@ -5,7 +5,6 @@
 #![doc(html_root_url = "https://docs.rs/zebra_rpc")]
 
 pub mod config;
-pub mod constants;
 pub mod methods;
 pub mod queue;
 pub mod server;
diff --git a/zebra-rpc/src/server/rpc_call_compatibility.rs b/zebra-rpc/src/server/rpc_call_compatibility.rs
index 5c3039efb92..209596180c0 100644
--- a/zebra-rpc/src/server/rpc_call_compatibility.rs
+++ b/zebra-rpc/src/server/rpc_call_compatibility.rs
@@ -13,7 +13,7 @@ use jsonrpc_core::{
     BoxFuture, Metadata, MethodCall, Notification,
 };
 
-use crate::{constants::MAX_PARAMS_LOG_LENGTH, server};
+use crate::server;
 
 /// JSON-RPC [`Middleware`] with compatibility workarounds.
 ///
@@ -81,6 +81,8 @@ impl FixRpcResponseMiddleware {
     ///
     /// Prints out only the method name and the received parameters.
     fn call_description(call: &Call) -> String {
+        const MAX_PARAMS_LOG_LENGTH: usize = 100;
+
         match call {
             Call::MethodCall(MethodCall { method, params, .. }) => {
                 let mut params = format!("{params:?}");

From f6da233f3e4716f51a3ae3a8f3f2e5b8fc2c06e1 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 13 Nov 2024 14:54:31 +0100
Subject: [PATCH 10/45] Rename `MapServerError` to `MapError`

---
 zebra-rpc/src/methods.rs                      | 74 +++++++++----------
 .../src/methods/get_block_template_rpcs.rs    | 27 ++++---
 zebra-rpc/src/server/error.rs                 |  8 +-
 3 files changed, 53 insertions(+), 56 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 6aa41c08621..f98ba84dbac 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -37,7 +37,7 @@ use crate::{
     queue::Queue,
     server::{
         self,
-        error::{MapServerError, OkOrServerError},
+        error::{MapError, OkOrServerError},
     },
 };
 
@@ -531,7 +531,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_server_error()?;
+                .map_error()?;
 
             let zebra_state::ReadResponse::TipPoolValues {
                 tip_height,
@@ -547,7 +547,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_server_error()?;
+                .map_error()?;
 
             let zebra_state::ReadResponse::BlockHeader(block_header) = response else {
                 unreachable!("unmatched response to a BlockHeader request")
@@ -640,7 +640,7 @@ where
             let valid_addresses = address_strings.valid_addresses()?;
 
             let request = zebra_state::ReadRequest::AddressBalance(valid_addresses);
-            let response = state.oneshot(request).await.map_server_error()?;
+            let response = state.oneshot(request).await.map_error()?;
 
             match response {
                 zebra_state::ReadResponse::AddressBalance(balance) => Ok(AddressBalance {
@@ -676,7 +676,7 @@ where
             let transaction_parameter = mempool::Gossip::Tx(raw_transaction.into());
             let request = mempool::Request::Queue(vec![transaction_parameter]);
 
-            let response = mempool.oneshot(request).await.map_server_error()?;
+            let response = mempool.oneshot(request).await.map_error()?;
 
             let mut queue_results = match response {
                 mempool::Response::Queued(results) => results,
@@ -693,15 +693,15 @@ where
                 .pop()
                 .expect("there should be exactly one item in Vec")
                 .inspect_err(|err| tracing::debug!("sent transaction to mempool: {:?}", &err))
-                .map_server_error()?
+                .map_error()?
                 .await;
 
             tracing::debug!("sent transaction to mempool: {:?}", &queue_result);
 
             queue_result
-                .map_server_error()?
+                .map_error()?
                 .map(|_| SentTransactionHash(transaction_hash))
-                .map_server_error()
+                .map_error()
         }
         .boxed()
     }
@@ -721,7 +721,7 @@ where
         let verbosity = verbosity.unwrap_or(DEFAULT_GETBLOCK_VERBOSITY);
 
         async move {
-            let hash_or_height: HashOrHeight = hash_or_height.parse().map_server_error()?;
+            let hash_or_height: HashOrHeight = hash_or_height.parse().map_error()?;
 
             if verbosity == 0 {
                 // # Performance
@@ -733,7 +733,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_server_error()?;
+                    .map_error()?;
 
                 match response {
                     zebra_state::ReadResponse::Block(Some(block)) => {
@@ -779,7 +779,7 @@ where
                             .ready()
                             .and_then(|service| service.call(request))
                             .await
-                            .map_server_error()?;
+                            .map_error()?;
 
                         match response {
                             zebra_state::ReadResponse::BlockHash(Some(hash)) => hash,
@@ -841,7 +841,7 @@ where
                 }
 
                 let tx_ids_response = futs.next().await.expect("`futs` should not be empty");
-                let tx = match tx_ids_response.map_server_error()? {
+                let tx = match tx_ids_response.map_error()? {
                     zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids
                         .ok_or_server_error("Block not found")?
                         .iter()
@@ -851,26 +851,24 @@ where
                 };
 
                 let sapling_tree_response = futs.next().await.expect("`futs` should not be empty");
-                let sapling_note_commitment_tree_count =
-                    match sapling_tree_response.map_server_error()? {
-                        zebra_state::ReadResponse::SaplingTree(Some(nct)) => nct.count(),
-                        zebra_state::ReadResponse::SaplingTree(None) => 0,
-                        _ => unreachable!("unmatched response to a SaplingTree request"),
-                    };
+                let sapling_note_commitment_tree_count = match sapling_tree_response.map_error()? {
+                    zebra_state::ReadResponse::SaplingTree(Some(nct)) => nct.count(),
+                    zebra_state::ReadResponse::SaplingTree(None) => 0,
+                    _ => unreachable!("unmatched response to a SaplingTree request"),
+                };
 
                 let orchard_tree_response = futs.next().await.expect("`futs` should not be empty");
-                let orchard_note_commitment_tree_count =
-                    match orchard_tree_response.map_server_error()? {
-                        zebra_state::ReadResponse::OrchardTree(Some(nct)) => nct.count(),
-                        zebra_state::ReadResponse::OrchardTree(None) => 0,
-                        _ => unreachable!("unmatched response to a OrchardTree request"),
-                    };
+                let orchard_note_commitment_tree_count = match orchard_tree_response.map_error()? {
+                    zebra_state::ReadResponse::OrchardTree(Some(nct)) => nct.count(),
+                    zebra_state::ReadResponse::OrchardTree(None) => 0,
+                    _ => unreachable!("unmatched response to a OrchardTree request"),
+                };
 
                 // From <https://zcash.github.io/rpc/getblock.html>
                 const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1;
 
                 let depth_response = futs.next().await.expect("`futs` should not be empty");
-                let confirmations = match depth_response.map_server_error()? {
+                let confirmations = match depth_response.map_error()? {
                     // 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)) => i64::from(depth) + 1,
@@ -882,7 +880,7 @@ where
                     let block_header_response =
                         futs.next().await.expect("`futs` should not be empty");
 
-                    match block_header_response.map_server_error()? {
+                    match block_header_response.map_error()? {
                         zebra_state::ReadResponse::BlockHeader(header) => Some(
                             header
                                 .ok_or_server_error("Block not found")?
@@ -964,7 +962,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_server_error()?;
+                .map_error()?;
 
             match response {
                 #[cfg(feature = "getblocktemplate-rpcs")]
@@ -1035,7 +1033,7 @@ where
                     ])))
                 })
                 .await
-                .map_server_error()?
+                .map_error()?
             {
                 mempool::Response::Transactions(txns) => {
                     if let Some(tx) = txns.first() {
@@ -1061,7 +1059,7 @@ where
                 .ready()
                 .and_then(|service| service.call(zebra_state::ReadRequest::Transaction(txid)))
                 .await
-                .map_server_error()?
+                .map_error()?
             {
                 zebra_state::ReadResponse::Transaction(Some(tx)) => {
                     let hex = tx.tx.into();
@@ -1079,7 +1077,7 @@ where
 
                 zebra_state::ReadResponse::Transaction(None) => {
                     // TODO: Return the correct err code (-5).
-                    Err("Transaction not found").map_server_error()
+                    Err("Transaction not found").map_error()
                 }
 
                 _ => unreachable!("unmatched response to a `Transaction` read request"),
@@ -1097,7 +1095,7 @@ where
 
         async move {
             // Convert the [`hash_or_height`] string into an actual hash or height.
-            let hash_or_height = hash_or_height.parse().map_server_error()?;
+            let hash_or_height = hash_or_height.parse().map_error()?;
 
             // Fetch the block referenced by [`hash_or_height`] from the state.
             //
@@ -1111,7 +1109,7 @@ where
                 .ready()
                 .and_then(|service| service.call(zebra_state::ReadRequest::Block(hash_or_height)))
                 .await
-                .map_server_error()?
+                .map_error()?
             {
                 zebra_state::ReadResponse::Block(Some(block)) => block,
                 zebra_state::ReadResponse::Block(None) => {
@@ -1145,7 +1143,7 @@ where
                         service.call(zebra_state::ReadRequest::SaplingTree(hash.into()))
                     })
                     .await
-                    .map_server_error()?
+                    .map_error()?
                 {
                     zebra_state::ReadResponse::SaplingTree(tree) => tree.map(|t| t.to_rpc_bytes()),
                     _ => unreachable!("unmatched response to a Sapling tree request"),
@@ -1162,7 +1160,7 @@ where
                         service.call(zebra_state::ReadRequest::OrchardTree(hash.into()))
                     })
                     .await
-                    .map_server_error()?
+                    .map_error()?
                 {
                     zebra_state::ReadResponse::OrchardTree(tree) => tree.map(|t| t.to_rpc_bytes()),
                     _ => unreachable!("unmatched response to an Orchard tree request"),
@@ -1195,7 +1193,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_server_error()?;
+                    .map_error()?;
 
                 let subtrees = match response {
                     zebra_state::ReadResponse::SaplingSubtrees(subtrees) => subtrees,
@@ -1221,7 +1219,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_server_error()?;
+                    .map_error()?;
 
                 let subtrees = match response {
                     zebra_state::ReadResponse::OrchardSubtrees(subtrees) => subtrees,
@@ -1281,7 +1279,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_server_error()?;
+                .map_error()?;
 
             let hashes = match response {
                 zebra_state::ReadResponse::AddressesTransactionIds(hashes) => {
@@ -1328,7 +1326,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_server_error()?;
+                .map_error()?;
             let utxos = match response {
                 zebra_state::ReadResponse::AddressUtxos(utxos) => utxos,
                 _ => unreachable!("unmatched response to a UtxosByAddresses request"),
diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs
index ecf1cf84b56..e6b7d60867c 100644
--- a/zebra-rpc/src/methods/get_block_template_rpcs.rs
+++ b/zebra-rpc/src/methods/get_block_template_rpcs.rs
@@ -62,7 +62,7 @@ use crate::{
         hex_data::HexData,
         GetBlockHash,
     },
-    server::{self, error::MapServerError},
+    server::{self, error::MapError},
 };
 
 pub mod constants;
@@ -586,7 +586,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_server_error()?;
+                .map_error()?;
 
             match response {
                 zebra_state::ReadResponse::BlockHash(Some(hash)) => Ok(GetBlockHash(hash)),
@@ -844,7 +844,7 @@ where
                                     Is Zebra shutting down?"
                                 );
 
-                                return Err(recv_error).map_server_error();
+                                return Err(recv_error).map_error();
                             }
                         }
                     }
@@ -1036,7 +1036,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_server_error()?;
+                    .map_error()?;
                 current_block_size = match response {
                     zebra_state::ReadResponse::TipBlockSize(Some(block_size)) => Some(block_size),
                     _ => None,
@@ -1225,13 +1225,12 @@ where
             // Always zero for post-halving blocks
             let founders = Amount::zero();
 
-            let total_block_subsidy = block_subsidy(height, &network).map_server_error()?;
-            let miner_subsidy =
-                miner_subsidy(height, &network, total_block_subsidy).map_server_error()?;
+            let total_block_subsidy = block_subsidy(height, &network).map_error()?;
+            let miner_subsidy = miner_subsidy(height, &network, total_block_subsidy).map_error()?;
 
             let (lockbox_streams, mut funding_streams): (Vec<_>, Vec<_>) =
                 funding_stream_values(height, &network, total_block_subsidy)
-                    .map_server_error()?
+                    .map_error()?
                     .into_iter()
                     // Separate the funding streams into deferred and non-deferred streams
                     .partition(|(receiver, _)| matches!(receiver, FundingStreamReceiver::Deferred));
@@ -1268,8 +1267,8 @@ where
                 founders: founders.into(),
                 funding_streams,
                 lockbox_streams,
-                funding_streams_total: funding_streams_total.map_server_error()?.into(),
-                lockbox_total: lockbox_total.map_server_error()?.into(),
+                funding_streams_total: funding_streams_total.map_error()?.into(),
+                lockbox_total: lockbox_total.map_error()?.into(),
                 total_block_subsidy: total_block_subsidy.into(),
             })
         }
@@ -1430,7 +1429,7 @@ where
 
             let mut block_hashes = Vec::new();
             for _ in 0..num_blocks {
-                let block_template = rpc.get_block_template(None).await.map_server_error()?;
+                let block_template = rpc.get_block_template(None).await.map_error()?;
 
                 let get_block_template::Response::TemplateMode(block_template) = block_template
                 else {
@@ -1446,14 +1445,14 @@ where
                     TimeSource::CurTime,
                     NetworkUpgrade::current(&network, Height(block_template.height)),
                 )
-                .map_server_error()?;
+                .map_error()?;
                 let hex_proposal_block =
-                    HexData(proposal_block.zcash_serialize_to_vec().map_server_error()?);
+                    HexData(proposal_block.zcash_serialize_to_vec().map_error()?);
 
                 let _submit = rpc
                     .submit_block(hex_proposal_block, None)
                     .await
-                    .map_server_error()?;
+                    .map_error()?;
 
                 block_hashes.push(GetBlockHash(proposal_block.hash()));
             }
diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index b5dbdeb518d..9fb4335ea85 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -55,8 +55,8 @@ impl From<LegacyCode> for jsonrpc_core::ErrorCode {
     }
 }
 
-pub(crate) trait MapServerError<T> {
-    fn map_server_error(self) -> std::result::Result<T, jsonrpc_core::Error>;
+pub(crate) trait MapError<T> {
+    fn map_error(self) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
 pub(crate) trait OkOrServerError<T> {
@@ -66,11 +66,11 @@ pub(crate) trait OkOrServerError<T> {
     ) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
-impl<T, E> MapServerError<T> for Result<T, E>
+impl<T, E> MapError<T> for Result<T, E>
 where
     E: ToString,
 {
-    fn map_server_error(self) -> Result<T, jsonrpc_core::Error> {
+    fn map_error(self) -> Result<T, jsonrpc_core::Error> {
         self.map_err(|error| jsonrpc_core::Error {
             code: jsonrpc_core::ErrorCode::ServerError(0),
             message: error.to_string(),

From 4b8825216f6c7e45ca560818b1d3c5d954a5a3ec Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 13 Nov 2024 15:03:11 +0100
Subject: [PATCH 11/45] Rename `OkOrServerError` to `OkOrError`

---
 zebra-rpc/src/methods.rs                      | 21 ++++++++-----------
 .../get_block_template.rs                     |  4 ++--
 zebra-rpc/src/server/error.rs                 | 11 ++++------
 3 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index f98ba84dbac..d1b82f6cec6 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -37,7 +37,7 @@ use crate::{
     queue::Queue,
     server::{
         self,
-        error::{MapError, OkOrServerError},
+        error::{MapError, OkOrError},
     },
 };
 
@@ -554,7 +554,7 @@ where
             };
 
             let tip_block_time = block_header
-                .ok_or_server_error("unexpectedly could not read best chain tip block header")?
+                .ok_or_error("unexpectedly could not read best chain tip block header")?
                 .time;
 
             let now = Utc::now();
@@ -843,7 +843,7 @@ where
                 let tx_ids_response = futs.next().await.expect("`futs` should not be empty");
                 let tx = match tx_ids_response.map_error()? {
                     zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids
-                        .ok_or_server_error("Block not found")?
+                        .ok_or_error("Block not found")?
                         .iter()
                         .map(|tx_id| tx_id.encode_hex())
                         .collect(),
@@ -881,12 +881,9 @@ where
                         futs.next().await.expect("`futs` should not be empty");
 
                     match block_header_response.map_error()? {
-                        zebra_state::ReadResponse::BlockHeader(header) => Some(
-                            header
-                                .ok_or_server_error("Block not found")?
-                                .time
-                                .timestamp(),
-                        ),
+                        zebra_state::ReadResponse::BlockHeader(header) => {
+                            Some(header.ok_or_error("Block not found")?.time.timestamp())
+                        }
                         _ => unreachable!("unmatched response to a BlockHeader request"),
                     }
                 } else {
@@ -926,14 +923,14 @@ where
         self.latest_chain_tip
             .best_tip_hash()
             .map(GetBlockHash)
-            .ok_or_server_error("No blocks in state")
+            .ok_or_error("No blocks in state")
     }
 
     fn get_best_block_height_and_hash(&self) -> Result<GetBlockHeightAndHash> {
         self.latest_chain_tip
             .best_tip_height_and_hash()
             .map(|(height, hash)| GetBlockHeightAndHash { height, hash })
-            .ok_or_server_error("No blocks in state")
+            .ok_or_error("No blocks in state")
     }
 
     fn get_raw_mempool(&self) -> BoxFuture<Result<Vec<String>>> {
@@ -1404,7 +1401,7 @@ where
 {
     latest_chain_tip
         .best_tip_height()
-        .ok_or_server_error("No blocks in state")
+        .ok_or_error("No blocks in state")
 }
 
 /// Response to a `getinfo` RPC request.
diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
index 3ef3d3130da..8e0b719417f 100644
--- a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
+++ b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
@@ -30,7 +30,7 @@ use crate::{
         constants::{MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP, NOT_SYNCED_ERROR_CODE},
         types::{default_roots::DefaultRoots, transaction::TransactionTemplate},
     },
-    server::error::OkOrServerError,
+    server::error::OkOrError,
 };
 
 pub use crate::methods::get_block_template_rpcs::types::get_block_template::*;
@@ -181,7 +181,7 @@ where
     // but this is ok for an estimate
     let (estimated_distance_to_chain_tip, local_tip_height) = latest_chain_tip
         .estimate_distance_to_network_chain_tip(network)
-        .ok_or_server_error("no chain tip available yet")?;
+        .ok_or_error("no chain tip available yet")?;
 
     if !sync_status.is_close_to_tip()
         || estimated_distance_to_chain_tip > MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP
diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index 9fb4335ea85..c1befe24315 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -59,11 +59,8 @@ pub(crate) trait MapError<T> {
     fn map_error(self) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
-pub(crate) trait OkOrServerError<T> {
-    fn ok_or_server_error<S: ToString>(
-        self,
-        message: S,
-    ) -> std::result::Result<T, jsonrpc_core::Error>;
+pub(crate) trait OkOrError<T> {
+    fn ok_or_error<S: ToString>(self, message: S) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
 impl<T, E> MapError<T> for Result<T, E>
@@ -79,8 +76,8 @@ where
     }
 }
 
-impl<T> OkOrServerError<T> for Option<T> {
-    fn ok_or_server_error<S: ToString>(self, message: S) -> Result<T, jsonrpc_core::Error> {
+impl<T> OkOrError<T> for Option<T> {
+    fn ok_or_error<S: ToString>(self, message: S) -> Result<T, jsonrpc_core::Error> {
         self.ok_or(jsonrpc_core::Error {
             code: jsonrpc_core::ErrorCode::ServerError(0),
             message: message.to_string(),

From 2c7aa71ff48bb5fddc8dd284bea960123bdb2e02 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 13 Nov 2024 16:54:07 +0100
Subject: [PATCH 12/45] Allow specifying error codes when mapping errors

---
 zebra-rpc/src/methods.rs                      | 95 +++++++++++--------
 .../src/methods/get_block_template_rpcs.rs    | 38 +++++---
 zebra-rpc/src/server/error.rs                 | 15 ++-
 3 files changed, 89 insertions(+), 59 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index d1b82f6cec6..0706073d88c 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -531,7 +531,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error()?;
+                .map_error(server::error::LegacyCode::default())?;
 
             let zebra_state::ReadResponse::TipPoolValues {
                 tip_height,
@@ -547,7 +547,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error()?;
+                .map_error(server::error::LegacyCode::default())?;
 
             let zebra_state::ReadResponse::BlockHeader(block_header) = response else {
                 unreachable!("unmatched response to a BlockHeader request")
@@ -640,7 +640,10 @@ where
             let valid_addresses = address_strings.valid_addresses()?;
 
             let request = zebra_state::ReadRequest::AddressBalance(valid_addresses);
-            let response = state.oneshot(request).await.map_error()?;
+            let response = state
+                .oneshot(request)
+                .await
+                .map_error(server::error::LegacyCode::default())?;
 
             match response {
                 zebra_state::ReadResponse::AddressBalance(balance) => Ok(AddressBalance {
@@ -676,7 +679,10 @@ where
             let transaction_parameter = mempool::Gossip::Tx(raw_transaction.into());
             let request = mempool::Request::Queue(vec![transaction_parameter]);
 
-            let response = mempool.oneshot(request).await.map_error()?;
+            let response = mempool
+                .oneshot(request)
+                .await
+                .map_error(server::error::LegacyCode::default())?;
 
             let mut queue_results = match response {
                 mempool::Response::Queued(results) => results,
@@ -693,15 +699,15 @@ where
                 .pop()
                 .expect("there should be exactly one item in Vec")
                 .inspect_err(|err| tracing::debug!("sent transaction to mempool: {:?}", &err))
-                .map_error()?
+                .map_error(server::error::LegacyCode::default())?
                 .await;
 
             tracing::debug!("sent transaction to mempool: {:?}", &queue_result);
 
             queue_result
-                .map_error()?
+                .map_error(server::error::LegacyCode::default())?
                 .map(|_| SentTransactionHash(transaction_hash))
-                .map_error()
+                .map_error(server::error::LegacyCode::default())
         }
         .boxed()
     }
@@ -721,7 +727,9 @@ where
         let verbosity = verbosity.unwrap_or(DEFAULT_GETBLOCK_VERBOSITY);
 
         async move {
-            let hash_or_height: HashOrHeight = hash_or_height.parse().map_error()?;
+            let hash_or_height: HashOrHeight = hash_or_height
+                .parse()
+                .map_error(server::error::LegacyCode::default())?;
 
             if verbosity == 0 {
                 // # Performance
@@ -733,7 +741,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_error()?;
+                    .map_error(server::error::LegacyCode::default())?;
 
                 match response {
                     zebra_state::ReadResponse::Block(Some(block)) => {
@@ -779,7 +787,7 @@ where
                             .ready()
                             .and_then(|service| service.call(request))
                             .await
-                            .map_error()?;
+                            .map_error(server::error::LegacyCode::default())?;
 
                         match response {
                             zebra_state::ReadResponse::BlockHash(Some(hash)) => hash,
@@ -841,7 +849,7 @@ where
                 }
 
                 let tx_ids_response = futs.next().await.expect("`futs` should not be empty");
-                let tx = match tx_ids_response.map_error()? {
+                let tx = match tx_ids_response.map_error(server::error::LegacyCode::default())? {
                     zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids
                         .ok_or_error("Block not found")?
                         .iter()
@@ -851,36 +859,39 @@ where
                 };
 
                 let sapling_tree_response = futs.next().await.expect("`futs` should not be empty");
-                let sapling_note_commitment_tree_count = match sapling_tree_response.map_error()? {
-                    zebra_state::ReadResponse::SaplingTree(Some(nct)) => nct.count(),
-                    zebra_state::ReadResponse::SaplingTree(None) => 0,
-                    _ => unreachable!("unmatched response to a SaplingTree request"),
-                };
+                let sapling_note_commitment_tree_count =
+                    match sapling_tree_response.map_error(server::error::LegacyCode::default())? {
+                        zebra_state::ReadResponse::SaplingTree(Some(nct)) => nct.count(),
+                        zebra_state::ReadResponse::SaplingTree(None) => 0,
+                        _ => unreachable!("unmatched response to a SaplingTree request"),
+                    };
 
                 let orchard_tree_response = futs.next().await.expect("`futs` should not be empty");
-                let orchard_note_commitment_tree_count = match orchard_tree_response.map_error()? {
-                    zebra_state::ReadResponse::OrchardTree(Some(nct)) => nct.count(),
-                    zebra_state::ReadResponse::OrchardTree(None) => 0,
-                    _ => unreachable!("unmatched response to a OrchardTree request"),
-                };
+                let orchard_note_commitment_tree_count =
+                    match orchard_tree_response.map_error(server::error::LegacyCode::default())? {
+                        zebra_state::ReadResponse::OrchardTree(Some(nct)) => nct.count(),
+                        zebra_state::ReadResponse::OrchardTree(None) => 0,
+                        _ => unreachable!("unmatched response to a OrchardTree request"),
+                    };
 
                 // From <https://zcash.github.io/rpc/getblock.html>
                 const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1;
 
                 let depth_response = futs.next().await.expect("`futs` should not be empty");
-                let confirmations = match depth_response.map_error()? {
-                    // 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)) => i64::from(depth) + 1,
-                    zebra_state::ReadResponse::Depth(None) => NOT_IN_BEST_CHAIN_CONFIRMATIONS,
-                    _ => unreachable!("unmatched response to a depth request"),
-                };
+                let confirmations =
+                    match depth_response.map_error(server::error::LegacyCode::default())? {
+                        // 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)) => i64::from(depth) + 1,
+                        zebra_state::ReadResponse::Depth(None) => NOT_IN_BEST_CHAIN_CONFIRMATIONS,
+                        _ => unreachable!("unmatched response to a depth request"),
+                    };
 
                 let time = if should_read_block_header {
                     let block_header_response =
                         futs.next().await.expect("`futs` should not be empty");
 
-                    match block_header_response.map_error()? {
+                    match block_header_response.map_error(server::error::LegacyCode::default())? {
                         zebra_state::ReadResponse::BlockHeader(header) => {
                             Some(header.ok_or_error("Block not found")?.time.timestamp())
                         }
@@ -959,7 +970,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error()?;
+                .map_error(server::error::LegacyCode::default())?;
 
             match response {
                 #[cfg(feature = "getblocktemplate-rpcs")]
@@ -1030,7 +1041,7 @@ where
                     ])))
                 })
                 .await
-                .map_error()?
+                .map_error(server::error::LegacyCode::default())?
             {
                 mempool::Response::Transactions(txns) => {
                     if let Some(tx) = txns.first() {
@@ -1056,7 +1067,7 @@ where
                 .ready()
                 .and_then(|service| service.call(zebra_state::ReadRequest::Transaction(txid)))
                 .await
-                .map_error()?
+                .map_error(server::error::LegacyCode::default())?
             {
                 zebra_state::ReadResponse::Transaction(Some(tx)) => {
                     let hex = tx.tx.into();
@@ -1074,7 +1085,7 @@ where
 
                 zebra_state::ReadResponse::Transaction(None) => {
                     // TODO: Return the correct err code (-5).
-                    Err("Transaction not found").map_error()
+                    Err("Transaction not found").map_error(server::error::LegacyCode::default())
                 }
 
                 _ => unreachable!("unmatched response to a `Transaction` read request"),
@@ -1092,7 +1103,9 @@ where
 
         async move {
             // Convert the [`hash_or_height`] string into an actual hash or height.
-            let hash_or_height = hash_or_height.parse().map_error()?;
+            let hash_or_height = hash_or_height
+                .parse()
+                .map_error(server::error::LegacyCode::default())?;
 
             // Fetch the block referenced by [`hash_or_height`] from the state.
             //
@@ -1106,7 +1119,7 @@ where
                 .ready()
                 .and_then(|service| service.call(zebra_state::ReadRequest::Block(hash_or_height)))
                 .await
-                .map_error()?
+                .map_error(server::error::LegacyCode::default())?
             {
                 zebra_state::ReadResponse::Block(Some(block)) => block,
                 zebra_state::ReadResponse::Block(None) => {
@@ -1140,7 +1153,7 @@ where
                         service.call(zebra_state::ReadRequest::SaplingTree(hash.into()))
                     })
                     .await
-                    .map_error()?
+                    .map_error(server::error::LegacyCode::default())?
                 {
                     zebra_state::ReadResponse::SaplingTree(tree) => tree.map(|t| t.to_rpc_bytes()),
                     _ => unreachable!("unmatched response to a Sapling tree request"),
@@ -1157,7 +1170,7 @@ where
                         service.call(zebra_state::ReadRequest::OrchardTree(hash.into()))
                     })
                     .await
-                    .map_error()?
+                    .map_error(server::error::LegacyCode::default())?
                 {
                     zebra_state::ReadResponse::OrchardTree(tree) => tree.map(|t| t.to_rpc_bytes()),
                     _ => unreachable!("unmatched response to an Orchard tree request"),
@@ -1190,7 +1203,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_error()?;
+                    .map_error(server::error::LegacyCode::default())?;
 
                 let subtrees = match response {
                     zebra_state::ReadResponse::SaplingSubtrees(subtrees) => subtrees,
@@ -1216,7 +1229,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_error()?;
+                    .map_error(server::error::LegacyCode::default())?;
 
                 let subtrees = match response {
                     zebra_state::ReadResponse::OrchardSubtrees(subtrees) => subtrees,
@@ -1276,7 +1289,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error()?;
+                .map_error(server::error::LegacyCode::default())?;
 
             let hashes = match response {
                 zebra_state::ReadResponse::AddressesTransactionIds(hashes) => {
@@ -1323,7 +1336,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error()?;
+                .map_error(server::error::LegacyCode::default())?;
             let utxos = match response {
                 zebra_state::ReadResponse::AddressUtxos(utxos) => utxos,
                 _ => unreachable!("unmatched response to a UtxosByAddresses request"),
diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs
index e6b7d60867c..a9d9150d2ac 100644
--- a/zebra-rpc/src/methods/get_block_template_rpcs.rs
+++ b/zebra-rpc/src/methods/get_block_template_rpcs.rs
@@ -586,7 +586,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error()?;
+                .map_error(server::error::LegacyCode::default())?;
 
             match response {
                 zebra_state::ReadResponse::BlockHash(Some(hash)) => Ok(GetBlockHash(hash)),
@@ -844,7 +844,7 @@ where
                                     Is Zebra shutting down?"
                                 );
 
-                                return Err(recv_error).map_error();
+                                return Err(recv_error).map_error(server::error::LegacyCode::default());
                             }
                         }
                     }
@@ -1036,7 +1036,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_error()?;
+                    .map_error(server::error::LegacyCode::default())?;
                 current_block_size = match response {
                     zebra_state::ReadResponse::TipBlockSize(Some(block_size)) => Some(block_size),
                     _ => None,
@@ -1225,12 +1225,14 @@ where
             // Always zero for post-halving blocks
             let founders = Amount::zero();
 
-            let total_block_subsidy = block_subsidy(height, &network).map_error()?;
-            let miner_subsidy = miner_subsidy(height, &network, total_block_subsidy).map_error()?;
+            let total_block_subsidy =
+                block_subsidy(height, &network).map_error(server::error::LegacyCode::default())?;
+            let miner_subsidy = miner_subsidy(height, &network, total_block_subsidy)
+                .map_error(server::error::LegacyCode::default())?;
 
             let (lockbox_streams, mut funding_streams): (Vec<_>, Vec<_>) =
                 funding_stream_values(height, &network, total_block_subsidy)
-                    .map_error()?
+                    .map_error(server::error::LegacyCode::default())?
                     .into_iter()
                     // Separate the funding streams into deferred and non-deferred streams
                     .partition(|(receiver, _)| matches!(receiver, FundingStreamReceiver::Deferred));
@@ -1267,8 +1269,12 @@ where
                 founders: founders.into(),
                 funding_streams,
                 lockbox_streams,
-                funding_streams_total: funding_streams_total.map_error()?.into(),
-                lockbox_total: lockbox_total.map_error()?.into(),
+                funding_streams_total: funding_streams_total
+                    .map_error(server::error::LegacyCode::default())?
+                    .into(),
+                lockbox_total: lockbox_total
+                    .map_error(server::error::LegacyCode::default())?
+                    .into(),
                 total_block_subsidy: total_block_subsidy.into(),
             })
         }
@@ -1429,7 +1435,10 @@ where
 
             let mut block_hashes = Vec::new();
             for _ in 0..num_blocks {
-                let block_template = rpc.get_block_template(None).await.map_error()?;
+                let block_template = rpc
+                    .get_block_template(None)
+                    .await
+                    .map_error(server::error::LegacyCode::default())?;
 
                 let get_block_template::Response::TemplateMode(block_template) = block_template
                 else {
@@ -1445,14 +1454,17 @@ where
                     TimeSource::CurTime,
                     NetworkUpgrade::current(&network, Height(block_template.height)),
                 )
-                .map_error()?;
-                let hex_proposal_block =
-                    HexData(proposal_block.zcash_serialize_to_vec().map_error()?);
+                .map_error(server::error::LegacyCode::default())?;
+                let hex_proposal_block = HexData(
+                    proposal_block
+                        .zcash_serialize_to_vec()
+                        .map_error(server::error::LegacyCode::default())?,
+                );
 
                 let _submit = rpc
                     .submit_block(hex_proposal_block, None)
                     .await
-                    .map_error()?;
+                    .map_error(server::error::LegacyCode::default())?;
 
                 block_hashes.push(GetBlockHash(proposal_block.hash()));
             }
diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index c1befe24315..52acd5b361c 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -7,9 +7,11 @@
 /// ## Notes
 ///
 /// - All explicit discriminants fit within `i64`.
+#[derive(Default)]
 pub enum LegacyCode {
     // General application defined errors
     /// `std::exception` thrown in command handling
+    #[default]
     Misc = -1,
     /// Server is in safe mode, and command is not allowed in safe mode
     ForbiddenBySafeMode = -2,
@@ -56,20 +58,23 @@ impl From<LegacyCode> for jsonrpc_core::ErrorCode {
 }
 
 pub(crate) trait MapError<T> {
-    fn map_error(self) -> std::result::Result<T, jsonrpc_core::Error>;
+    fn map_error(
+        self,
+        code: impl Into<jsonrpc_core::ErrorCode>,
+    ) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
 pub(crate) trait OkOrError<T> {
-    fn ok_or_error<S: ToString>(self, message: S) -> std::result::Result<T, jsonrpc_core::Error>;
+    fn ok_or_error(self, message: impl ToString) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
 impl<T, E> MapError<T> for Result<T, E>
 where
     E: ToString,
 {
-    fn map_error(self) -> Result<T, jsonrpc_core::Error> {
+    fn map_error(self, code: impl Into<jsonrpc_core::ErrorCode>) -> Result<T, jsonrpc_core::Error> {
         self.map_err(|error| jsonrpc_core::Error {
-            code: jsonrpc_core::ErrorCode::ServerError(0),
+            code: code.into(),
             message: error.to_string(),
             data: None,
         })
@@ -77,7 +82,7 @@ where
 }
 
 impl<T> OkOrError<T> for Option<T> {
-    fn ok_or_error<S: ToString>(self, message: S) -> Result<T, jsonrpc_core::Error> {
+    fn ok_or_error(self, message: impl ToString) -> Result<T, jsonrpc_core::Error> {
         self.ok_or(jsonrpc_core::Error {
             code: jsonrpc_core::ErrorCode::ServerError(0),
             message: message.to_string(),

From 91ed312c98f5321beda384ca6f24099e904b8f8d Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 14 Nov 2024 00:27:57 +0100
Subject: [PATCH 13/45] Allow setting error codes when mapping options

---
 zebra-rpc/src/methods.rs                      | 25 +++++++++++++------
 .../get_block_template.rs                     |  7 ++++--
 zebra-rpc/src/server/error.rs                 | 14 ++++++++---
 3 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 0706073d88c..ef58e4e362f 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -554,7 +554,10 @@ where
             };
 
             let tip_block_time = block_header
-                .ok_or_error("unexpectedly could not read best chain tip block header")?
+                .ok_or_error(
+                    server::error::LegacyCode::default(),
+                    "unexpectedly could not read best chain tip block header",
+                )?
                 .time;
 
             let now = Utc::now();
@@ -851,7 +854,7 @@ where
                 let tx_ids_response = futs.next().await.expect("`futs` should not be empty");
                 let tx = match tx_ids_response.map_error(server::error::LegacyCode::default())? {
                     zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids
-                        .ok_or_error("Block not found")?
+                        .ok_or_error(server::error::LegacyCode::default(), "Block not found")?
                         .iter()
                         .map(|tx_id| tx_id.encode_hex())
                         .collect(),
@@ -892,9 +895,15 @@ where
                         futs.next().await.expect("`futs` should not be empty");
 
                     match block_header_response.map_error(server::error::LegacyCode::default())? {
-                        zebra_state::ReadResponse::BlockHeader(header) => {
-                            Some(header.ok_or_error("Block not found")?.time.timestamp())
-                        }
+                        zebra_state::ReadResponse::BlockHeader(header) => Some(
+                            header
+                                .ok_or_error(
+                                    server::error::LegacyCode::default(),
+                                    "Block not found",
+                                )?
+                                .time
+                                .timestamp(),
+                        ),
                         _ => unreachable!("unmatched response to a BlockHeader request"),
                     }
                 } else {
@@ -934,14 +943,14 @@ where
         self.latest_chain_tip
             .best_tip_hash()
             .map(GetBlockHash)
-            .ok_or_error("No blocks in state")
+            .ok_or_error(server::error::LegacyCode::default(), "No blocks in state")
     }
 
     fn get_best_block_height_and_hash(&self) -> Result<GetBlockHeightAndHash> {
         self.latest_chain_tip
             .best_tip_height_and_hash()
             .map(|(height, hash)| GetBlockHeightAndHash { height, hash })
-            .ok_or_error("No blocks in state")
+            .ok_or_error(server::error::LegacyCode::default(), "No blocks in state")
     }
 
     fn get_raw_mempool(&self) -> BoxFuture<Result<Vec<String>>> {
@@ -1414,7 +1423,7 @@ where
 {
     latest_chain_tip
         .best_tip_height()
-        .ok_or_error("No blocks in state")
+        .ok_or_error(server::error::LegacyCode::default(),"No blocks in state")
 }
 
 /// Response to a `getinfo` RPC request.
diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
index 8e0b719417f..e96ab57c175 100644
--- a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
+++ b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
@@ -30,7 +30,7 @@ use crate::{
         constants::{MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP, NOT_SYNCED_ERROR_CODE},
         types::{default_roots::DefaultRoots, transaction::TransactionTemplate},
     },
-    server::error::OkOrError,
+    server::{self, error::OkOrError},
 };
 
 pub use crate::methods::get_block_template_rpcs::types::get_block_template::*;
@@ -181,7 +181,10 @@ where
     // but this is ok for an estimate
     let (estimated_distance_to_chain_tip, local_tip_height) = latest_chain_tip
         .estimate_distance_to_network_chain_tip(network)
-        .ok_or_error("no chain tip available yet")?;
+        .ok_or_error(
+            server::error::LegacyCode::default(),
+            "no chain tip available yet",
+        )?;
 
     if !sync_status.is_close_to_tip()
         || estimated_distance_to_chain_tip > MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP
diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index 52acd5b361c..23d093d19e7 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -65,7 +65,11 @@ pub(crate) trait MapError<T> {
 }
 
 pub(crate) trait OkOrError<T> {
-    fn ok_or_error(self, message: impl ToString) -> std::result::Result<T, jsonrpc_core::Error>;
+    fn ok_or_error(
+        self,
+        code: impl Into<jsonrpc_core::ErrorCode>,
+        message: impl ToString,
+    ) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
 impl<T, E> MapError<T> for Result<T, E>
@@ -82,9 +86,13 @@ where
 }
 
 impl<T> OkOrError<T> for Option<T> {
-    fn ok_or_error(self, message: impl ToString) -> Result<T, jsonrpc_core::Error> {
+    fn ok_or_error(
+        self,
+        code: impl Into<jsonrpc_core::ErrorCode>,
+        message: impl ToString,
+    ) -> Result<T, jsonrpc_core::Error> {
         self.ok_or(jsonrpc_core::Error {
-            code: jsonrpc_core::ErrorCode::ServerError(0),
+            code: code.into(),
             message: message.to_string(),
             data: None,
         })

From 943a815c69b43e705d319b02a3fedab2d2da3ca9 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 14 Nov 2024 12:18:15 +0100
Subject: [PATCH 14/45] Use the right error code for `getrawtransaction`

---
 zebra-rpc/src/methods.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index ef58e4e362f..8a3bd0c5e33 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -1093,8 +1093,8 @@ where
                 }
 
                 zebra_state::ReadResponse::Transaction(None) => {
-                    // TODO: Return the correct err code (-5).
-                    Err("Transaction not found").map_error(server::error::LegacyCode::default())
+                    Err("No such mempool or main chain transaction")
+                        .map_error(server::error::LegacyCode::InvalidAddressOrKey)
                 }
 
                 _ => unreachable!("unmatched response to a `Transaction` read request"),

From c3d1e83026959ed7f4deddad879d636a0ef2ab6a Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 14 Nov 2024 12:18:55 +0100
Subject: [PATCH 15/45] fmt

---
 zebra-rpc/src/methods.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 8a3bd0c5e33..40fd7c8a215 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -1423,7 +1423,7 @@ where
 {
     latest_chain_tip
         .best_tip_height()
-        .ok_or_error(server::error::LegacyCode::default(),"No blocks in state")
+        .ok_or_error(server::error::LegacyCode::default(), "No blocks in state")
 }
 
 /// Response to a `getinfo` RPC request.

From 415b91952e84b94a39f4e2b0e58914c3dee54cda Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 14 Nov 2024 12:30:09 +0100
Subject: [PATCH 16/45] Add docs for the error conversion traits

---
 zebra-rpc/src/server/error.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index 23d093d19e7..2e38b06bb27 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -57,14 +57,19 @@ impl From<LegacyCode> for jsonrpc_core::ErrorCode {
     }
 }
 
+/// A trait for mapping errors to [`jsonrpc_core::Error`].
 pub(crate) trait MapError<T> {
+    /// Maps errors to [`jsonrpc_core::Error`] with a specific error code.
     fn map_error(
         self,
         code: impl Into<jsonrpc_core::ErrorCode>,
     ) -> std::result::Result<T, jsonrpc_core::Error>;
 }
 
+/// A trait for conditionally converting a value into a `Result<T, jsonrpc_core::Error>`.
 pub(crate) trait OkOrError<T> {
+    /// Converts the implementing type to `Result<T, jsonrpc_core::Error>`, using an error code and
+    /// message if conversion is to `Err`.
     fn ok_or_error(
         self,
         code: impl Into<jsonrpc_core::ErrorCode>,

From 91361d83283becf53042fce89943445eba95a2a1 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 14 Nov 2024 14:18:42 +0100
Subject: [PATCH 17/45] Refactor the error handling for `getblock`

---
 zebra-rpc/src/methods.rs | 337 +++++++++++++++++++--------------------
 1 file changed, 168 insertions(+), 169 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 40fd7c8a215..a611b6ae0bb 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -723,166 +723,166 @@ where
         hash_or_height: String,
         verbosity: Option<u8>,
     ) -> BoxFuture<Result<GetBlock>> {
-        // From <https://zcash.github.io/rpc/getblock.html>
-        const DEFAULT_GETBLOCK_VERBOSITY: u8 = 1;
-
         let mut state = self.state.clone();
-        let verbosity = verbosity.unwrap_or(DEFAULT_GETBLOCK_VERBOSITY);
 
         async move {
             let hash_or_height: HashOrHeight = hash_or_height
                 .parse()
                 .map_error(server::error::LegacyCode::default())?;
 
-            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()
-                    .and_then(|service| service.call(request))
-                    .await
-                    .map_error(server::error::LegacyCode::default())?;
-
-                match response {
-                    zebra_state::ReadResponse::Block(Some(block)) => {
-                        Ok(GetBlock::Raw(block.into()))
-                    }
-                    zebra_state::ReadResponse::Block(None) => Err(Error {
-                        // `lightwalletd` expects error code `-8` when a block is not found:
-                        // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>                   zebra_state::ReadResponse::Block(None) => Err(Error {
-                        code: server::error::LegacyCode::InvalidParameter.into(),
-                        message: "Block not found".to_string(),
-                        data: None,
-                    }),
-                    _ => unreachable!("unmatched response to a block request"),
-                }
-            } else if verbosity == 1 || verbosity == 2 {
-                // # 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 block hash and transaction IDs from indexes,
-                // which is much more efficient than loading all the block data,
-                // then hashing the block header and all the transactions.
-
-                // Get the block hash from the height -> hash index, if needed
-                //
-                // # Concurrency
-                //
-                // For consistency, this lookup must be performed first, then all the other
-                // lookups must be based on the hash.
-                //
-                // All possible responses are valid, even if the best chain changes. Clients
-                // must be able to handle chain forks, including a hash for a block that is
-                // later discovered to be on a side chain.
-
-                let should_read_block_header = verbosity == 2;
-
-                let hash = match hash_or_height {
-                    HashOrHeight::Hash(hash) => hash,
-                    HashOrHeight::Height(height) => {
-                        let request = zebra_state::ReadRequest::BestChainBlockHash(height);
-                        let response = state
-                            .ready()
-                            .and_then(|service| service.call(request))
-                            .await
-                            .map_error(server::error::LegacyCode::default())?;
-
-                        match response {
-                            zebra_state::ReadResponse::BlockHash(Some(hash)) => hash,
-                            zebra_state::ReadResponse::BlockHash(None) => {
-                                return Err(Error {
-                                    // `lightwalletd` expects error code `-8` when a block is not found:
-                                    // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>                   zebra_state::ReadResponse::Block(None) => Err(Error {
-                                    code: server::error::LegacyCode::InvalidParameter.into(),
-                                    message: "block height not in best chain".to_string(),
-                                    data: None,
-                                });
-                            }
-                            _ => unreachable!("unmatched response to a block hash request"),
+            match verbosity.unwrap_or(1) {
+                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()
+                        .and_then(|service| service.call(request))
+                        .await
+                        .map_error(server::error::LegacyCode::default())?;
+
+                    match response {
+                        zebra_state::ReadResponse::Block(Some(block)) => {
+                            Ok(GetBlock::Raw(block.into()))
                         }
+                        zebra_state::ReadResponse::Block(None) => Err("Block not found")
+                            // `lightwalletd` expects error code `-8` when a block is not found:
+                            // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>
+                            .map_error(server::error::LegacyCode::InvalidParameter),
+                        _ => unreachable!("unmatched response to a block request"),
                     }
-                };
-
-                // TODO:  look up the height if we only have a hash,
-                //        this needs a new state request for the height -> hash index
-                let height = hash_or_height.height();
+                }
+                verbosity @ 1 | verbosity @ 2 => {
+                    // # 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 block hash and transaction IDs from indexes,
+                    // which is much more efficient than loading all the block data,
+                    // then hashing the block header and all the transactions.
 
-                // # Concurrency
-                //
-                // We look up by block hash so the hash, transaction IDs, and confirmations
-                // are consistent.
-                let mut requests = vec![
-                    // Get transaction IDs from the transaction index by block hash
+                    // Get the block hash from the height -> hash index, if needed
                     //
                     // # 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.
-                    zebra_state::ReadRequest::TransactionIdsForBlock(hash.into()),
-                    // Sapling trees
-                    zebra_state::ReadRequest::SaplingTree(hash.into()),
-                    // Orchard trees
-                    zebra_state::ReadRequest::OrchardTree(hash.into()),
-                    // Get block confirmations from the block height index
+                    // For consistency, this lookup must be performed first, then all the other
+                    // lookups must be based on the hash.
                     //
+                    // All possible responses are valid, even if the best chain changes. Clients
+                    // must be able to handle chain forks, including a hash for a block that is
+                    // later discovered to be on a side chain.
+
+                    let should_read_block_header = verbosity == 2;
+
+                    let hash = match hash_or_height {
+                        HashOrHeight::Hash(hash) => hash,
+                        HashOrHeight::Height(height) => {
+                            let request = zebra_state::ReadRequest::BestChainBlockHash(height);
+                            let response = state
+                                .ready()
+                                .and_then(|service| service.call(request))
+                                .await
+                                .map_error(server::error::LegacyCode::default())?;
+
+                            match response {
+                                zebra_state::ReadResponse::BlockHash(Some(hash)) => hash,
+                                zebra_state::ReadResponse::BlockHash(None) => {
+                                    return Err("block height not in best chain")
+                                        // `lightwalletd` expects error code `-8` when a block is not found:
+                                        // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>
+                                        .map_error(server::error::LegacyCode::InvalidParameter);
+                                }
+                                _ => unreachable!("unmatched response to a block hash request"),
+                            }
+                        }
+                    };
+
+                    // TODO:  look up the height if we only have a hash,
+                    //        this needs a new state request for the height -> hash index
+                    let height = hash_or_height.height();
+
                     // # 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.
-                    zebra_state::ReadRequest::Depth(hash),
-                ];
-
-                if should_read_block_header {
-                    // Block header
-                    requests.push(zebra_state::ReadRequest::BlockHeader(hash.into()))
-                }
+                    // We look up by block hash so the hash, transaction IDs, and confirmations
+                    // are consistent.
+                    let mut requests = vec![
+                        // Get transaction IDs from the transaction index by block hash
+                        //
+                        // # 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.
+                        zebra_state::ReadRequest::TransactionIdsForBlock(hash.into()),
+                        // Sapling trees
+                        zebra_state::ReadRequest::SaplingTree(hash.into()),
+                        // Orchard trees
+                        zebra_state::ReadRequest::OrchardTree(hash.into()),
+                        // 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.
+                        zebra_state::ReadRequest::Depth(hash),
+                    ];
+
+                    if should_read_block_header {
+                        // Block header
+                        requests.push(zebra_state::ReadRequest::BlockHeader(hash.into()))
+                    }
 
-                let mut futs = FuturesOrdered::new();
+                    let mut futs = FuturesOrdered::new();
 
-                for request in requests {
-                    futs.push_back(state.clone().oneshot(request));
-                }
+                    for request in requests {
+                        futs.push_back(state.clone().oneshot(request));
+                    }
 
-                let tx_ids_response = futs.next().await.expect("`futs` should not be empty");
-                let tx = match tx_ids_response.map_error(server::error::LegacyCode::default())? {
-                    zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids
-                        .ok_or_error(server::error::LegacyCode::default(), "Block not found")?
-                        .iter()
-                        .map(|tx_id| tx_id.encode_hex())
-                        .collect(),
-                    _ => unreachable!("unmatched response to a transaction_ids_for_block request"),
-                };
+                    let tx_ids_response = futs.next().await.expect("`futs` should not be empty");
+                    let tx = match tx_ids_response.map_error(server::error::LegacyCode::default())?
+                    {
+                        zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids
+                            .ok_or_error(server::error::LegacyCode::default(), "Block not found")?
+                            .iter()
+                            .map(|tx_id| tx_id.encode_hex())
+                            .collect(),
+                        _ => unreachable!(
+                            "unmatched response to a transaction_ids_for_block request"
+                        ),
+                    };
 
-                let sapling_tree_response = futs.next().await.expect("`futs` should not be empty");
-                let sapling_note_commitment_tree_count =
-                    match sapling_tree_response.map_error(server::error::LegacyCode::default())? {
+                    let sapling_tree_response =
+                        futs.next().await.expect("`futs` should not be empty");
+                    let sapling_note_commitment_tree_count = match sapling_tree_response
+                        .map_error(server::error::LegacyCode::default())?
+                    {
                         zebra_state::ReadResponse::SaplingTree(Some(nct)) => nct.count(),
                         zebra_state::ReadResponse::SaplingTree(None) => 0,
                         _ => unreachable!("unmatched response to a SaplingTree request"),
                     };
 
-                let orchard_tree_response = futs.next().await.expect("`futs` should not be empty");
-                let orchard_note_commitment_tree_count =
-                    match orchard_tree_response.map_error(server::error::LegacyCode::default())? {
+                    let orchard_tree_response =
+                        futs.next().await.expect("`futs` should not be empty");
+                    let orchard_note_commitment_tree_count = match orchard_tree_response
+                        .map_error(server::error::LegacyCode::default())?
+                    {
                         zebra_state::ReadResponse::OrchardTree(Some(nct)) => nct.count(),
                         zebra_state::ReadResponse::OrchardTree(None) => 0,
                         _ => unreachable!("unmatched response to a OrchardTree request"),
                     };
 
-                // From <https://zcash.github.io/rpc/getblock.html>
-                const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1;
+                    // From <https://zcash.github.io/rpc/getblock.html>
+                    const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1;
 
-                let depth_response = futs.next().await.expect("`futs` should not be empty");
-                let confirmations =
-                    match depth_response.map_error(server::error::LegacyCode::default())? {
+                    let depth_response = futs.next().await.expect("`futs` should not be empty");
+                    let confirmations = match depth_response
+                        .map_error(server::error::LegacyCode::default())?
+                    {
                         // 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)) => i64::from(depth) + 1,
@@ -890,50 +890,49 @@ where
                         _ => unreachable!("unmatched response to a depth request"),
                     };
 
-                let time = if should_read_block_header {
-                    let block_header_response =
-                        futs.next().await.expect("`futs` should not be empty");
-
-                    match block_header_response.map_error(server::error::LegacyCode::default())? {
-                        zebra_state::ReadResponse::BlockHeader(header) => Some(
-                            header
-                                .ok_or_error(
-                                    server::error::LegacyCode::default(),
-                                    "Block not found",
-                                )?
-                                .time
-                                .timestamp(),
-                        ),
-                        _ => unreachable!("unmatched response to a BlockHeader request"),
-                    }
-                } else {
-                    None
-                };
+                    let time = if should_read_block_header {
+                        let block_header_response =
+                            futs.next().await.expect("`futs` should not be empty");
+
+                        match block_header_response
+                            .map_error(server::error::LegacyCode::default())?
+                        {
+                            zebra_state::ReadResponse::BlockHeader(header) => Some(
+                                header
+                                    .ok_or_error(
+                                        server::error::LegacyCode::default(),
+                                        "Block not found",
+                                    )?
+                                    .time
+                                    .timestamp(),
+                            ),
+                            _ => unreachable!("unmatched response to a BlockHeader request"),
+                        }
+                    } else {
+                        None
+                    };
 
-                let sapling = SaplingTrees {
-                    size: sapling_note_commitment_tree_count,
-                };
+                    let sapling = SaplingTrees {
+                        size: sapling_note_commitment_tree_count,
+                    };
 
-                let orchard = OrchardTrees {
-                    size: orchard_note_commitment_tree_count,
-                };
+                    let orchard = OrchardTrees {
+                        size: orchard_note_commitment_tree_count,
+                    };
 
-                let trees = GetBlockTrees { sapling, orchard };
+                    let trees = GetBlockTrees { sapling, orchard };
 
-                Ok(GetBlock::Object {
-                    hash: GetBlockHash(hash),
-                    confirmations,
-                    height,
-                    time,
-                    tx,
-                    trees,
-                })
-            } else {
-                Err(Error {
-                    code: ErrorCode::InvalidParams,
-                    message: "Invalid verbosity value".to_string(),
-                    data: None,
-                })
+                    Ok(GetBlock::Object {
+                        hash: GetBlockHash(hash),
+                        confirmations,
+                        height,
+                        time,
+                        tx,
+                        trees,
+                    })
+                }
+                _ => Err("Verbosity must be in range from 0 to 2".to_string())
+                    .map_error(server::error::LegacyCode::InvalidParameter),
             }
         }
         .boxed()

From fb40ae033b17c31ba6cafbdf6f645081ac85ce30 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Fri, 15 Nov 2024 15:53:56 +0100
Subject: [PATCH 18/45] Refactor error handling in `sendrawtransaction`

---
 zebra-rpc/src/methods.rs | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index a611b6ae0bb..908fabda236 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -667,11 +667,12 @@ where
         let queue_sender = self.queue_sender.clone();
 
         async move {
-            let raw_transaction_bytes = Vec::from_hex(raw_transaction_hex).map_err(|_| {
-                Error::invalid_params("raw transaction is not specified as a hex string")
-            })?;
+            // Reference for the legacy error code:
+            // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/rawtransaction.cpp#L1259-L1260>
+            let raw_transaction_bytes = Vec::from_hex(raw_transaction_hex)
+                .map_error(server::error::LegacyCode::Deserialization)?;
             let raw_transaction = Transaction::zcash_deserialize(&*raw_transaction_bytes)
-                .map_err(|_| Error::invalid_params("raw transaction is structurally invalid"))?;
+                .map_error(server::error::LegacyCode::Deserialization)?;
 
             let transaction_hash = raw_transaction.hash();
 
@@ -703,14 +704,20 @@ where
                 .expect("there should be exactly one item in Vec")
                 .inspect_err(|err| tracing::debug!("sent transaction to mempool: {:?}", &err))
                 .map_error(server::error::LegacyCode::default())?
-                .await;
+                .await
+                .map_error(server::error::LegacyCode::default())?;
 
             tracing::debug!("sent transaction to mempool: {:?}", &queue_result);
 
             queue_result
-                .map_error(server::error::LegacyCode::default())?
                 .map(|_| SentTransactionHash(transaction_hash))
-                .map_error(server::error::LegacyCode::default())
+                // Reference for the legacy error code:
+                // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/rawtransaction.cpp#L1290-L1301>
+                // Note that this error code might not exactly match the one returned by zcashd
+                // since zcashd's error code selection logic is more granular. We'd need to
+                // propagate the error coming from the verifier to be able to return more specific
+                // error codes.
+                .map_error(server::error::LegacyCode::Verify)
         }
         .boxed()
     }

From c23501af5aa8fa7629d92bab2fa80396ea1130c9 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Fri, 15 Nov 2024 19:14:57 +0100
Subject: [PATCH 19/45] Refactor the error handling for `getblock`

---
 zebra-rpc/src/methods.rs | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 908fabda236..3c0f729081f 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -735,7 +735,9 @@ where
         async move {
             let hash_or_height: HashOrHeight = hash_or_height
                 .parse()
-                .map_error(server::error::LegacyCode::default())?;
+                // Reference for the legacy error code:
+                // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/blockchain.cpp#L629>
+                .map_error(server::error::LegacyCode::InvalidParameter)?;
 
             match verbosity.unwrap_or(1) {
                 0 => {
@@ -757,6 +759,9 @@ where
                         zebra_state::ReadResponse::Block(None) => Err("Block not found")
                             // `lightwalletd` expects error code `-8` when a block is not found:
                             // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>
+                            // This is because `lightwalletd` requests blocks by height, and
+                            // `zcashd` returns `-8` for invalid heights:
+                            // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/blockchain.cpp#L629>
                             .map_error(server::error::LegacyCode::InvalidParameter),
                         _ => unreachable!("unmatched response to a block request"),
                     }
@@ -800,6 +805,9 @@ where
                                     return Err("block height not in best chain")
                                         // `lightwalletd` expects error code `-8` when a block is not found:
                                         // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>
+                                        // This is because `lightwalletd` requests blocks by height, and
+                                        // `zcashd` returns `-8` for invalid heights:
+                                        // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/blockchain.cpp#L629>
                                         .map_error(server::error::LegacyCode::InvalidParameter);
                                 }
                                 _ => unreachable!("unmatched response to a block hash request"),

From 03e9b25c54e0face1f82a515628f2e25f603e3a4 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Fri, 15 Nov 2024 19:43:32 +0100
Subject: [PATCH 20/45] Update the error handling for `getrawtransaction`

---
 zebra-rpc/src/methods.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 3c0f729081f..af5d8daacdc 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -1049,10 +1049,13 @@ where
         let verbose = verbose.unwrap_or(0) != 0;
 
         async move {
+            // Reference for the legacy error code:
+            // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/server.cpp#L137-L138>
             let txid = transaction::Hash::from_bytes_in_display_order(
                 &txid
                     .try_into()
-                    .map_err(|_| Error::invalid_params("invalid TXID length"))?,
+                    .map_err(|_| "invalid TXID length")
+                    .map_error(server::error::LegacyCode::InvalidParameter)?,
             );
 
             // Check the mempool first.

From 0d1d0c85d5d8ee0d50313728a3a7db53dd2a4148 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Fri, 15 Nov 2024 23:39:13 +0100
Subject: [PATCH 21/45] Refactor error handling for `z_gettreestate`

---
 zebra-rpc/src/methods.rs | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index af5d8daacdc..3a3875ec325 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -1128,10 +1128,11 @@ where
         let network = self.network.clone();
 
         async move {
-            // Convert the [`hash_or_height`] string into an actual hash or height.
+            // Reference for the legacy error code:
+            // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/blockchain.cpp#L629>
             let hash_or_height = hash_or_height
                 .parse()
-                .map_error(server::error::LegacyCode::default())?;
+                .map_error(server::error::LegacyCode::InvalidParameter)?;
 
             // Fetch the block referenced by [`hash_or_height`] from the state.
             //
@@ -1149,13 +1150,9 @@ where
             {
                 zebra_state::ReadResponse::Block(Some(block)) => block,
                 zebra_state::ReadResponse::Block(None) => {
-                    return Err(Error {
-                        // `lightwalletd` expects error code `-8` when a block is not found:
-                        // <https://github.com/zcash/lightwalletd/blob/v0.4.16/common/common.go#L287-L290>                   zebra_state::ReadResponse::Block(None) => Err(Error {
-                        code: server::error::LegacyCode::InvalidParameter.into(),
-                        message: "the requested block was not found".to_string(),
-                        data: None,
-                    });
+                    // Reference for the legacy error code:
+                    // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/blockchain.cpp#L629>
+                    return Err("the requested block is not in the main chain").map_error(server::error::LegacyCode::InvalidParameter);
                 }
                 _ => unreachable!("unmatched response to a block request"),
             };

From 57325f057b97aaabb7dd018c5135bce1b2a5c2c6 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Fri, 15 Nov 2024 23:46:32 +0100
Subject: [PATCH 22/45] Refactor the error handling for address parsing

---
 zebra-rpc/src/methods.rs | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 3a3875ec325..8842d47ed5d 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -1152,7 +1152,8 @@ where
                 zebra_state::ReadResponse::Block(None) => {
                     // Reference for the legacy error code:
                     // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/blockchain.cpp#L629>
-                    return Err("the requested block is not in the main chain").map_error(server::error::LegacyCode::InvalidParameter);
+                    return Err("the requested block is not in the main chain")
+                        .map_error(server::error::LegacyCode::InvalidParameter);
                 }
                 _ => unreachable!("unmatched response to a block request"),
             };
@@ -1531,13 +1532,15 @@ impl AddressStrings {
     /// - check if provided list have all valid transparent addresses.
     /// - return valid addresses as a set of `Address`.
     pub fn valid_addresses(self) -> Result<HashSet<Address>> {
+        // Reference for the legacy error code:
+        // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/misc.cpp#L783-L784>
         let valid_addresses: HashSet<Address> = self
             .addresses
             .into_iter()
             .map(|address| {
-                address.parse().map_err(|error| {
-                    Error::invalid_params(format!("invalid address {address:?}: {error}"))
-                })
+                address
+                    .parse()
+                    .map_error(server::error::LegacyCode::InvalidAddressOrKey)
             })
             .collect::<Result<_>>()?;
 

From c29e41069839602cb2cb7ba5fda6fab60323154d Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Sat, 16 Nov 2024 17:37:02 +0100
Subject: [PATCH 23/45] Refactor the error handling for getrawtransaction

---
 zebra-rpc/src/methods.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 8842d47ed5d..afb51cb56cb 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -1050,12 +1050,12 @@ where
 
         async move {
             // Reference for the legacy error code:
-            // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/server.cpp#L137-L138>
+            // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/rawtransaction.cpp#L544>
             let txid = transaction::Hash::from_bytes_in_display_order(
                 &txid
                     .try_into()
                     .map_err(|_| "invalid TXID length")
-                    .map_error(server::error::LegacyCode::InvalidParameter)?,
+                    .map_error(server::error::LegacyCode::InvalidAddressOrKey)?,
             );
 
             // Check the mempool first.

From 7777f3d2507f864ac733c4b490c31be194bdb9c4 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Mon, 18 Nov 2024 13:20:39 +0100
Subject: [PATCH 24/45] Update `z_gettreestate` snapshots

---
 .../z_get_treestate_by_non_existent_hash@custom_testnet.snap   | 3 ++-
 .../z_get_treestate_excessive_block_height@custom_testnet.snap | 3 ++-
 ...get_treestate_unparsable_hash_or_height@custom_testnet.snap | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap
index d0013994ab0..7801c859a27 100644
--- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap
+++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_by_non_existent_hash@custom_testnet.snap
@@ -1,10 +1,11 @@
 ---
 source: zebra-rpc/src/methods/tests/snapshot.rs
 expression: treestate
+snapshot_kind: text
 ---
 {
   "Err": {
     "code": -8,
-    "message": "the requested block was not found"
+    "message": "the requested block is not in the main chain"
   }
 }
diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap
index d0013994ab0..7801c859a27 100644
--- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap
+++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_excessive_block_height@custom_testnet.snap
@@ -1,10 +1,11 @@
 ---
 source: zebra-rpc/src/methods/tests/snapshot.rs
 expression: treestate
+snapshot_kind: text
 ---
 {
   "Err": {
     "code": -8,
-    "message": "the requested block was not found"
+    "message": "the requested block is not in the main chain"
   }
 }
diff --git a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap
index a45d7e298dc..d7b3c2b1ff0 100644
--- a/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap
+++ b/zebra-rpc/src/methods/tests/snapshots/z_get_treestate_unparsable_hash_or_height@custom_testnet.snap
@@ -1,10 +1,11 @@
 ---
 source: zebra-rpc/src/methods/tests/snapshot.rs
 expression: treestate
+snapshot_kind: text
 ---
 {
   "Err": {
-    "code": 0,
+    "code": -8,
     "message": "parse error: could not convert the input string to a hash or height"
   }
 }

From a20b5f1d888798c02ee90d13a16efb131ae92410 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Mon, 18 Nov 2024 23:09:37 +0100
Subject: [PATCH 25/45] Cosmetics

---
 zebra-rpc/src/methods/tests/snapshot.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs
index 4c06c5b0c69..44cd2826001 100644
--- a/zebra-rpc/src/methods/tests/snapshot.rs
+++ b/zebra-rpc/src/methods/tests/snapshot.rs
@@ -376,8 +376,9 @@ async fn test_rpc_response_data_for_network(network: &Network) {
 
     // `getrawtransaction` verbosity=0
     //
-    // - similar to `getrawmempool` described above, a mempool request will be made to get the requested
-    // transaction from the mempool, response will be empty as we have this transaction in state
+    // - Similarly to `getrawmempool` described above, a mempool request will be made to get the
+    // requested transaction from the mempool. Response will be empty as we have this transaction in
+    // state.
     let mempool_req = mempool
         .expect_request_that(|request| {
             matches!(request, mempool::Request::TransactionsByMinedId(_))

From 5696e0bc794406358aa6e0b27589ec5cafe399b4 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Tue, 19 Nov 2024 10:19:14 +0100
Subject: [PATCH 26/45] Refactor error handling in `getblock`

---
 zebra-rpc/src/methods.rs | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 7021417a2b2..1566b6a0e59 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -917,19 +917,19 @@ where
                         _ => unreachable!("unmatched response to a depth request"),
                     };
 
-                let (time, height) = if should_read_block_header {
-                    let block_header_response =
-                        futs.next().await.expect("`futs` should not be empty");
+                    let (time, height) = if should_read_block_header {
+                        let block_header_response =
+                            futs.next().await.expect("`futs` should not be empty");
 
-                    match block_header_response.map_server_error()? {
-                        zebra_state::ReadResponse::BlockHeader { header, height, .. } => {
-                            (Some(header.time.timestamp()), Some(height))
+                        match block_header_response.map_error(server::error::LegacyCode::Misc)? {
+                            zebra_state::ReadResponse::BlockHeader { header, height, .. } => {
+                                (Some(header.time.timestamp()), Some(height))
+                            }
+                            _ => unreachable!("unmatched response to a BlockHeader request"),
                         }
-                        _ => unreachable!("unmatched response to a BlockHeader request"),
-                    }
-                } else {
-                    (None, hash_or_height.height())
-                };
+                    } else {
+                        (None, hash_or_height.height())
+                    };
 
                     let sapling = SaplingTrees {
                         size: sapling_note_commitment_tree_count,

From 71186b57401d08e996b963d525562c2ac8a9b0ad Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Tue, 19 Nov 2024 10:19:33 +0100
Subject: [PATCH 27/45] Refactor error handling in `getblockheader`

---
 zebra-rpc/src/methods.rs | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 1566b6a0e59..8c83b3336da 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -967,7 +967,9 @@ where
         let network = self.network.clone();
 
         async move {
-            let hash_or_height: HashOrHeight = hash_or_height.parse().map_server_error()?;
+            let hash_or_height: HashOrHeight = hash_or_height
+                .parse()
+                .map_error(server::error::LegacyCode::InvalidAddressOrKey)?;
             let zebra_state::ReadResponse::BlockHeader {
                 header,
                 hash,
@@ -977,32 +979,38 @@ where
                 .clone()
                 .oneshot(zebra_state::ReadRequest::BlockHeader(hash_or_height))
                 .await
-                .map_server_error()?
+                .map_error(server::error::LegacyCode::Misc)?
             else {
                 panic!("unexpected response to BlockHeader request")
             };
 
             let response = if !verbose {
-                GetBlockHeader::Raw(HexData(header.zcash_serialize_to_vec().map_server_error()?))
+                GetBlockHeader::Raw(HexData(
+                    header
+                        .zcash_serialize_to_vec()
+                        .map_error(server::error::LegacyCode::Misc)?,
+                ))
             } else {
                 let zebra_state::ReadResponse::SaplingTree(sapling_tree) = state
                     .clone()
                     .oneshot(zebra_state::ReadRequest::SaplingTree(hash_or_height))
                     .await
-                    .map_server_error()?
+                    .map_error(server::error::LegacyCode::Misc)?
                 else {
                     panic!("unexpected response to SaplingTree request")
                 };
 
                 // This could be `None` if there's a chain reorg between state queries.
-                let sapling_tree =
-                    sapling_tree.ok_or_server_error("missing sapling tree for block")?;
+                let sapling_tree = sapling_tree.ok_or_error(
+                    server::error::LegacyCode::Misc,
+                    "missing sapling tree for block",
+                )?;
 
                 let zebra_state::ReadResponse::Depth(depth) = state
                     .clone()
                     .oneshot(zebra_state::ReadRequest::Depth(hash))
                     .await
-                    .map_server_error()?
+                    .map_error(server::error::LegacyCode::Misc)?
                 else {
                     panic!("unexpected response to SaplingTree request")
                 };

From 7fef8bf00ab2a1bd2afaf5205658825bb40eee17 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 20 Nov 2024 18:06:14 +0100
Subject: [PATCH 28/45] Simplify `getrawtransaction`

---
 zebra-rpc/src/methods.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 8c83b3336da..560fabc8ed1 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -1172,9 +1172,9 @@ where
             match mempool
                 .ready()
                 .and_then(|service| {
-                    service.call(mempool::Request::TransactionsByMinedId(HashSet::from([
+                    service.call(mempool::Request::TransactionsByMinedId([
                         txid,
-                    ])))
+                    ].into()))
                 })
                 .await
                 .map_error(server::error::LegacyCode::default())?

From 821b8b9f9d52a4a0a03a958c339158dec1d32ba5 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 20 Nov 2024 18:07:57 +0100
Subject: [PATCH 29/45] Check errors for `getrawtransaction`

---
 zebra-rpc/src/methods/tests/prop.rs | 67 +++++++++++++++++------------
 1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs
index 6ed0978042f..1c8a4730ce2 100644
--- a/zebra-rpc/src/methods/tests/prop.rs
+++ b/zebra-rpc/src/methods/tests/prop.rs
@@ -1,6 +1,6 @@
 //! Randomised property tests for RPC methods.
 
-use std::{collections::HashSet, sync::Arc};
+use std::{collections::HashSet, fmt::Debug, sync::Arc};
 
 use futures::{join, FutureExt, TryFutureExt};
 use hex::ToHex;
@@ -450,25 +450,23 @@ proptest! {
         })?;
     }
 
-    /// Test that the method rejects an input that's not a transaction.
+    /// Calls `get_raw_transaction` with:
+    ///
+    /// 1. an invalid TXID that won't deserialize due to wrong length;
+    /// 2. a valid TXID that is not in the mempool nor in the state;
     ///
-    /// Try to call `get_raw_transaction` using random bytes that fail to deserialize as a txid, and
-    /// check that it fails with an expected error.
+    /// and checks that the RPC returns the right error code.
     #[test]
-    fn get_raw_transaction_invalid_transaction_results_in_an_error(
-        random_bytes in any::<Vec<u8>>(),
+    fn check_err_for_get_raw_transaction(
+        invalid_txid in vec(any::<u8>(), 0..31),
+        unknown_txid: [u8; 32]
     ) {
-        if random_bytes.len() == 32 {
-            return Ok::<_, TestCaseError>(());
-        }
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
 
         // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
         tokio::time::pause();
 
-        prop_assume!(transaction::Hash::zcash_deserialize(&*random_bytes).is_err());
-
         runtime.block_on(async move {
             let mut mempool = MockService::build().for_prop_tests();
             let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
@@ -484,25 +482,30 @@ proptest! {
                 NoChainTip,
             );
 
-            let send_task = tokio::spawn(rpc.get_raw_transaction(HexData(random_bytes), Some(0)));
+            let mempool_query = mempool.expect_no_requests();
+            let state_query = state.expect_no_requests();
 
-            mempool.expect_no_requests().await?;
-            state.expect_no_requests().await?;
+            let rpc_query = rpc.get_raw_transaction(HexData(invalid_txid), Some(1));
 
-            let result = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
+            let (rsp, _, _) = tokio::join!(rpc_query, mempool_query, state_query);
 
-            prop_assert!(
-                matches!(
-                    result,
-                    Err(Error {
-                        code: ErrorCode::InvalidParams,
-                        ..
-                    })
-                ),
-                "Result is not an invalid parameters error: {result:?}"
-            );
+            check_err_code(rsp, ErrorCode::ServerError(-5))?;
+
+            let txid = transaction::Hash::from_bytes_in_display_order(&unknown_txid);
+
+            let mempool_query = mempool
+                .expect_request(mempool::Request::TransactionsByMinedId([txid].into()))
+                .map_ok(|r| r.respond(mempool::Response::Transactions(vec![])));
+
+            let state_query = state
+                .expect_request(zebra_state::ReadRequest::Transaction(txid))
+                .map_ok(|r| r.respond(zebra_state::ReadResponse::Transaction(None)));
+
+            let rpc_query = rpc.get_raw_transaction(HexData(unknown_txid.to_vec()), Some(1));
+
+            let (rsp, _, _) =  tokio::join!(rpc_query, mempool_query, state_query);
+
+            check_err_code(rsp, ErrorCode::ServerError(-5))?;
 
             // The queue task should continue without errors or panics
             let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
@@ -1008,3 +1011,13 @@ proptest! {
 #[derive(Clone, Copy, Debug, Error)]
 #[error("a dummy error type")]
 pub struct DummyError;
+
+/// Checks that the given RPC response contains the given error code.
+fn check_err_code<T>(rsp: Result<T, Error>, error_code: ErrorCode) -> Result<(), TestCaseError> {
+    prop_assert!(
+        matches!(&rsp, Err(Error { code, .. }) if *code == error_code),
+        "the RPC response must match the error code: {error_code:?}"
+    );
+
+    Ok(())
+}

From 7c3890324a2474877e86243dcd9fb94e3bd96e36 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 20 Nov 2024 22:22:50 +0100
Subject: [PATCH 30/45] fmt

---
 zebra-rpc/src/methods.rs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 560fabc8ed1..f77e2d00e32 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -1172,9 +1172,7 @@ where
             match mempool
                 .ready()
                 .and_then(|service| {
-                    service.call(mempool::Request::TransactionsByMinedId([
-                        txid,
-                    ].into()))
+                    service.call(mempool::Request::TransactionsByMinedId([txid].into()))
                 })
                 .await
                 .map_error(server::error::LegacyCode::default())?

From a654e3942786ffee0a5a08ead814ccc04baac3f5 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Wed, 20 Nov 2024 22:23:02 +0100
Subject: [PATCH 31/45] Simplify proptests

---
 zebra-rpc/src/methods/tests/prop.rs | 589 ++++++++++------------------
 1 file changed, 197 insertions(+), 392 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs
index 1c8a4730ce2..6be04612d28 100644
--- a/zebra-rpc/src/methods/tests/prop.rs
+++ b/zebra-rpc/src/methods/tests/prop.rs
@@ -13,11 +13,8 @@ use tower::buffer::Buffer;
 use zebra_chain::{
     amount::{Amount, NonNegative},
     block::{self, Block, Height},
-    chain_tip::{mock::MockChainTip, NoChainTip},
-    parameters::{
-        Network::{self, *},
-        NetworkUpgrade,
-    },
+    chain_tip::{mock::MockChainTip, ChainTip, NoChainTip},
+    parameters::{Network, NetworkUpgrade},
     serialization::{ZcashDeserialize, ZcashSerialize},
     transaction::{self, Transaction, UnminedTx, VerifiedUnminedTx},
     transparent,
@@ -28,7 +25,7 @@ use zebra_state::BoxError;
 
 use zebra_test::mock_service::MockService;
 
-use crate::methods::hex_data::HexData;
+use crate::methods::{self, hex_data::HexData};
 
 use super::super::{
     AddressBalance, AddressStrings, NetworkUpgradeStatus, Rpc, RpcImpl, SentTransactionHash,
@@ -37,27 +34,19 @@ use super::super::{
 proptest! {
     /// Test that when sending a raw transaction, it is received by the mempool service.
     #[test]
-    fn mempool_receives_raw_transaction(transaction in any::<Transaction>()) {
+    fn mempool_receives_raw_tx(transaction in any::<Transaction>(), network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
+        let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
+
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
         runtime.block_on(async move {
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
             let hash = SentTransactionHash(transaction.hash());
 
-            let transaction_bytes = transaction
-                .zcash_serialize_to_vec()
-                .expect("Transaction serializes successfully");
+            let transaction_bytes = transaction.zcash_serialize_to_vec()?;
+
             let transaction_hex = hex::encode(&transaction_bytes);
 
             let send_task = tokio::spawn(rpc.send_raw_transaction(transaction_hex));
@@ -75,17 +64,14 @@ proptest! {
 
             state.expect_no_requests().await?;
 
-            let result = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
+            let result = send_task.await?;
 
             prop_assert_eq!(result, Ok(hash));
 
             // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 
@@ -93,27 +79,16 @@ proptest! {
     ///
     /// Mempool service errors should become server errors.
     #[test]
-    fn mempool_errors_are_forwarded(transaction in any::<Transaction>()) {
+    fn mempool_errors_are_forwarded(transaction in any::<Transaction>(), network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
+        let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
-        runtime.block_on(async move {
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
-            let transaction_bytes = transaction
-                .zcash_serialize_to_vec()
-                .expect("Transaction serializes successfully");
+        runtime.block_on(async move {
+            let transaction_bytes = transaction.zcash_serialize_to_vec()?;
             let transaction_hex = hex::encode(&transaction_bytes);
 
             let send_task = tokio::spawn(rpc.send_raw_transaction(transaction_hex.clone()));
@@ -128,20 +103,9 @@ proptest! {
 
             state.expect_no_requests().await?;
 
-            let result = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
-
-            prop_assert!(
-                matches!(
-                    result,
-                    Err(Error {
-                        code: ErrorCode::ServerError(_),
-                        ..
-                    })
-                ),
-                "Result is not a server error: {result:?}"
-            );
+            let result = send_task.await?;
+
+            check_err_code(result, ErrorCode::ServerError(-1))?;
 
             let send_task = tokio::spawn(rpc.send_raw_transaction(transaction_hex));
 
@@ -154,87 +118,44 @@ proptest! {
                 .await?
                 .respond(Ok::<_, BoxError>(mempool::Response::Queued(vec![Ok(rsp_rx)])));
 
-            let result = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
-
-            prop_assert!(
-                matches!(
-                    result,
-                    Err(Error {
-                        code: ErrorCode::ServerError(_),
-                        ..
-                    })
-                ),
-                "Result is not a server error: {result:?}"
-            );
+            let result = send_task.await?;
+
+            check_err_code(result, ErrorCode::ServerError(-25))?;
 
             // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 
     /// Test that when the mempool rejects a transaction the caller receives an error.
     #[test]
-    fn rejected_transactions_are_reported(transaction in any::<Transaction>()) {
+    fn rejected_txs_are_reported(transaction in any::<Transaction>(), network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
+        let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
-        runtime.block_on(async move {
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
-
-            let transaction_bytes = transaction
-                .zcash_serialize_to_vec()
-                .expect("Transaction serializes successfully");
-            let transaction_hex = hex::encode(&transaction_bytes);
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
-            let send_task = tokio::spawn(rpc.send_raw_transaction(transaction_hex));
+        runtime.block_on(async move {
+            let tx = hex::encode(&transaction.zcash_serialize_to_vec()?);
+            let req = mempool::Request::Queue(vec![UnminedTx::from(transaction).into()]);
+            let rsp = mempool::Response::Queued(vec![Err(DummyError.into())]);
+            let mempool_query = mempool.expect_request(req).map_ok(|r| r.respond(rsp));
 
-            let unmined_transaction = UnminedTx::from(transaction);
-            let expected_request = mempool::Request::Queue(vec![unmined_transaction.into()]);
-            let response = mempool::Response::Queued(vec![Err(DummyError.into())]);
+            let (rpc_rsp, _) = tokio::join!(rpc.send_raw_transaction(tx), mempool_query);
 
-            mempool
-                .expect_request(expected_request)
-                .await?
-                .respond(response);
+            check_err_code(rpc_rsp, ErrorCode::ServerError(-1))?;
 
+            // Check that no state request was made.
             state.expect_no_requests().await?;
 
-            let result = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
-
-            prop_assert!(
-                matches!(
-                    result,
-                    Err(Error {
-                        code: ErrorCode::ServerError(_),
-                        ..
-                    })
-                ),
-                "Result is not a server error: {result:?}"
-            );
-
             // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 
@@ -243,53 +164,27 @@ proptest! {
     /// Try to call `send_raw_transaction` using a string parameter that has at least one
     /// non-hexadecimal character, and check that it fails with an expected error.
     #[test]
-    fn non_hexadecimal_string_results_in_an_error(non_hex_string in ".*[^0-9A-Fa-f].*") {
+    fn non_hex_string_is_error(non_hex_string in ".*[^0-9A-Fa-f].*", network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
         // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
         tokio::time::pause();
 
         runtime.block_on(async move {
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
-
             let send_task = tokio::spawn(rpc.send_raw_transaction(non_hex_string));
 
+            // Check that there are no further requests.
             mempool.expect_no_requests().await?;
             state.expect_no_requests().await?;
 
-            let result = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
-
-            prop_assert!(
-                matches!(
-                    result,
-                    Err(Error {
-                        code: ErrorCode::InvalidParams,
-                        ..
-                    })
-                ),
-                "Result is not an invalid parameters error: {result:?}"
-            );
+            check_err_code(send_task.await?, ErrorCode::ServerError(-22))?;
 
             // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 
@@ -298,9 +193,10 @@ proptest! {
     /// Try to call `send_raw_transaction` using random bytes that fail to deserialize as a
     /// transaction, and check that it fails with an expected error.
     #[test]
-    fn invalid_transaction_results_in_an_error(random_bytes in any::<Vec<u8>>()) {
+    fn invalid_tx_results_in_an_error(random_bytes in any::<Vec<u8>>(), network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
         // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
         tokio::time::pause();
@@ -308,45 +204,17 @@ proptest! {
         prop_assume!(Transaction::zcash_deserialize(&*random_bytes).is_err());
 
         runtime.block_on(async move {
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
-
             let send_task = tokio::spawn(rpc.send_raw_transaction(hex::encode(random_bytes)));
 
             mempool.expect_no_requests().await?;
             state.expect_no_requests().await?;
 
-            let result = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
-
-            prop_assert!(
-                matches!(
-                    result,
-                    Err(Error {
-                        code: ErrorCode::InvalidParams,
-                        ..
-                    })
-                ),
-                "Result is not an invalid parameters error: {result:?}"
-            );
+            check_err_code(send_task.await?, ErrorCode::ServerError(-22))?;
 
             // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 
@@ -355,33 +223,18 @@ proptest! {
     /// Make the mock mempool service return a list of transaction IDs, and check that the RPC call
     /// returns those IDs as hexadecimal strings.
     #[test]
-    fn mempool_transactions_are_sent_to_caller(transactions in any::<Vec<VerifiedUnminedTx>>()) {
+    fn mempool_transactions_are_sent_to_caller(transactions in any::<Vec<VerifiedUnminedTx>>(),
+                                               network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
         // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
         tokio::time::pause();
 
         runtime.block_on(async move {
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
-
-            let call_task = tokio::spawn(rpc.get_raw_mempool());
-
-
             #[cfg(not(feature = "getblocktemplate-rpcs"))]
-            let expected_response = {
+            let (expected_response, mempool_query) = {
                 let transaction_ids: HashSet<_> = transactions
                     .iter()
                     .map(|tx| tx.transaction.id)
@@ -393,18 +246,18 @@ proptest! {
                     .collect();
                 expected_response.sort();
 
-                mempool
+                let mempool_query = mempool
                     .expect_request(mempool::Request::TransactionIds)
-                    .await?
-                    .respond(mempool::Response::TransactionIds(transaction_ids));
+                    .map_ok(|r|r.respond(mempool::Response::TransactionIds(transaction_ids)));
 
-                expected_response
+                (expected_response, mempool_query)
             };
 
             // Note: this depends on `SHOULD_USE_ZCASHD_ORDER` being true.
             #[cfg(feature = "getblocktemplate-rpcs")]
-            let expected_response = {
+            let (expected_response, mempool_query) = {
                 let mut expected_response = transactions.clone();
+
                 expected_response.sort_by_cached_key(|tx| {
                     // zcashd uses modified fee here but Zebra doesn't currently
                     // support prioritizing transactions
@@ -418,35 +271,31 @@ proptest! {
 
                 let expected_response = expected_response
                     .iter()
-                    .map(|tx| tx.transaction.id.mined_id().encode_hex())
-                    .collect();
+                    .map(|tx| tx.transaction.id.mined_id().encode_hex::<String>())
+                    .collect::<Vec<_>>();
 
-                mempool
+                let mempool_query = mempool
                     .expect_request(mempool::Request::FullTransactions)
-                    .await?
-                    .respond(mempool::Response::FullTransactions {
+                    .map_ok(|r| r.respond(mempool::Response::FullTransactions {
                         transactions,
                         transaction_dependencies: Default::default(),
                         last_seen_tip_hash: [0; 32].into(),
-                    });
+                    }));
 
-                expected_response
+                (expected_response, mempool_query)
             };
 
-            mempool.expect_no_requests().await?;
-            state.expect_no_requests().await?;
+            let (rpc_rsp, _) = tokio::join!(rpc.get_raw_mempool(), mempool_query);
 
-            let result = call_task
-                .await
-                .expect("Sending raw transactions should not panic");
+            prop_assert_eq!(rpc_rsp?, expected_response);
 
-            prop_assert_eq!(result, Ok(expected_response));
+            mempool.expect_no_requests().await?;
+            state.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();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 
@@ -459,37 +308,24 @@ proptest! {
     #[test]
     fn check_err_for_get_raw_transaction(
         invalid_txid in vec(any::<u8>(), 0..31),
-        unknown_txid: [u8; 32]
+        unknown_txid: [u8; 32],
+        network in any::<Network>()
     ) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
         // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
         tokio::time::pause();
 
         runtime.block_on(async move {
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
-
-            let mempool_query = mempool.expect_no_requests();
-            let state_query = state.expect_no_requests();
-
             let rpc_query = rpc.get_raw_transaction(HexData(invalid_txid), Some(1));
 
-            let (rsp, _, _) = tokio::join!(rpc_query, mempool_query, state_query);
+            check_err_code(rpc_query.await, ErrorCode::ServerError(-5))?;
 
-            check_err_code(rsp, ErrorCode::ServerError(-5))?;
+            // Check that no further requests were made.
+            mempool.expect_no_requests().await?;
+            state.expect_no_requests().await?;
 
             let txid = transaction::Hash::from_bytes_in_display_order(&unknown_txid);
 
@@ -508,10 +344,9 @@ proptest! {
             check_err_code(rsp, ErrorCode::ServerError(-5))?;
 
             // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 
@@ -520,21 +355,10 @@ proptest! {
     fn get_blockchain_info_response_without_a_chain_tip(network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
-        let mut mempool = MockService::build().for_prop_tests();
-        let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-        // look for an error with a `NoChainTip`
-        let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-            "RPC test",
-            "RPC test",
-            network,
-            false,
-            true,
-            mempool.clone(),
-            Buffer::new(state.clone(), 1),
-            NoChainTip,
-        );
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
         runtime.block_on(async move {
             let response_fut = rpc.get_blockchain_info();
@@ -556,14 +380,13 @@ proptest! {
                 "no chain tip available yet"
             );
 
-            // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
-
             mempool.expect_no_requests().await?;
             state.expect_no_requests().await?;
 
-            Ok::<_, TestCaseError>(())
+            // The queue task should continue without errors or panics
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
+
+            Ok(())
         })?;
     }
 
@@ -575,26 +398,17 @@ proptest! {
     ) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
-        let mut mempool = MockService::build().for_prop_tests();
-        let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) =
+            mock_services(network.clone(), NoChainTip);
+
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
         // get arbitrary chain tip data
         let block_height = block.coinbase_height().unwrap();
         let block_hash = block.hash();
         let block_time = block.header.time;
 
-        // Start RPC with the mocked `ChainTip`
-        let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-            "RPC test",
-            "RPC test",
-            network.clone(),
-            false,
-            true,
-            mempool.clone(),
-            Buffer::new(state.clone(), 1),
-            NoChainTip,
-        );
-
         // check no requests were made during this test
         runtime.block_on(async move {
             let response_fut = rpc.get_blockchain_info();
@@ -669,14 +483,13 @@ proptest! {
                 }
             };
 
-            // The queue task should continue without errors or panics
-            let rpc_tx_queue_task_result = rpc_tx_queue_task_handle.now_or_never();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
-
             mempool.expect_no_requests().await?;
             state.expect_no_requests().await?;
 
-            Ok::<_, TestCaseError>(())
+            // The queue task should continue without errors or panics
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
+
+            Ok(())
         })?;
     }
 
@@ -689,12 +502,11 @@ proptest! {
     ) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
-
-        let mut mempool = MockService::build().for_prop_tests();
-        let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-        // Create a mocked `ChainTip`
         let (chain_tip, _mock_chain_tip_sender) = MockChainTip::new();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, chain_tip);
+
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
         // Prepare the list of addresses.
         let address_strings = AddressStrings {
@@ -704,21 +516,8 @@ proptest! {
                 .collect(),
         };
 
-        tokio::time::pause();
-
         // Start RPC with the mocked `ChainTip`
         runtime.block_on(async move {
-            let (rpc, _rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                network,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                chain_tip,
-            );
-
             // Build the future to call the RPC
             let call = rpc.get_address_balance(address_strings);
 
@@ -743,7 +542,10 @@ proptest! {
             mempool.expect_no_requests().await?;
             state.expect_no_requests().await?;
 
-            Ok::<_, TestCaseError>(())
+            // The queue task should continue without errors or panics
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
+
+            Ok(())
         })?;
     }
 
@@ -757,31 +559,17 @@ proptest! {
     ) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
+        let (chain_tip, _mock_chain_tip_sender) = MockChainTip::new();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, chain_tip);
+
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
         prop_assume!(at_least_one_invalid_address
             .iter()
             .any(|string| string.parse::<transparent::Address>().is_err()));
 
-        let mut mempool = MockService::build().for_prop_tests();
-        let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-        // Create a mocked `ChainTip`
-        let (chain_tip, _mock_chain_tip_sender) = MockChainTip::new();
-
-        tokio::time::pause();
-
-        // Start RPC with the mocked `ChainTip`
         runtime.block_on(async move {
-            let (rpc, _rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                network,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                chain_tip,
-            );
 
             let address_strings = AddressStrings {
                 addresses: at_least_one_invalid_address,
@@ -790,55 +578,32 @@ proptest! {
             // Build the future to call the RPC
             let result = rpc.get_address_balance(address_strings).await;
 
-            // Check that the invalid addresses lead to an error
-            prop_assert!(
-                matches!(
-                    result,
-                    Err(Error {
-                        code: ErrorCode::InvalidParams,
-                        ..
-                    })
-                ),
-                "Result is not a server error: {result:?}"
-            );
+            check_err_code(result, ErrorCode::ServerError(-5))?;
 
             // Check no requests were made during this test
             mempool.expect_no_requests().await?;
             state.expect_no_requests().await?;
 
-            Ok::<_, TestCaseError>(())
+            // The queue task should continue without errors or panics
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
+
+            Ok(())
         })?;
     }
 
     /// Test the queue functionality using `send_raw_transaction`
     #[test]
-    fn rpc_queue_main_loop(tx in any::<Transaction>()) {
+    fn rpc_queue_main_loop(tx in any::<Transaction>(), network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
-        let transaction_hash = tx.hash();
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
         runtime.block_on(async move {
-            tokio::time::pause();
-
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
-
-            // send a transaction
-            let tx_bytes = tx
-                .zcash_serialize_to_vec()
-                .expect("Transaction serializes successfully");
+            let transaction_hash = tx.hash();
+            let tx_bytes = tx.zcash_serialize_to_vec()?;
             let tx_hex = hex::encode(&tx_bytes);
             let send_task = tokio::spawn(rpc.send_raw_transaction(tx_hex));
 
@@ -852,9 +617,7 @@ proptest! {
                 .unwrap()
                 .respond(Err(DummyError));
 
-            let _ = send_task
-                .await
-                .expect("Sending raw transactions should not panic");
+            let _ = send_task.await?;
 
             // advance enough time to have a new runner iteration
             let spacing = chrono::Duration::seconds(150);
@@ -897,42 +660,28 @@ proptest! {
             state.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();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 
     /// Test we receive all transactions that are sent in a channel
     #[test]
-    fn rpc_queue_receives_all_transactions_from_channel(txs in any::<[Transaction; 2]>()) {
+    fn rpc_queue_receives_all_txs_from_channel(txs in any::<[Transaction; 2]>(),
+                                               network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
+        let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
 
-        runtime.block_on(async move {
-            tokio::time::pause();
-
-            let mut mempool = MockService::build().for_prop_tests();
-            let mut state: MockService<_, _, _, BoxError> = MockService::build().for_prop_tests();
-
-            let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
-                "RPC test",
-                "RPC test",
-                Mainnet,
-                false,
-                true,
-                mempool.clone(),
-                Buffer::new(state.clone(), 1),
-                NoChainTip,
-            );
+        // CORRECTNESS: Nothing in this test depends on real time, so we can speed it up.
+        tokio::time::pause();
 
+        runtime.block_on(async move {
             let mut transactions_hash_set = HashSet::new();
             for tx in txs.clone() {
                 // send a transaction
-                let tx_bytes = tx
-                    .zcash_serialize_to_vec()
-                    .expect("Transaction serializes successfully");
+                let tx_bytes = tx.zcash_serialize_to_vec()?;
                 let tx_hex = hex::encode(&tx_bytes);
                 let send_task = tokio::spawn(rpc.send_raw_transaction(tx_hex));
 
@@ -950,9 +699,7 @@ proptest! {
                     .unwrap()
                     .respond(Err(DummyError));
 
-                let _ = send_task
-                    .await
-                    .expect("Sending raw transactions should not panic");
+                let _ = send_task.await?;
             }
 
             // advance enough time to have a new runner iteration
@@ -1000,10 +747,9 @@ proptest! {
             state.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();
-            prop_assert!(rpc_tx_queue_task_result.is_none());
+            prop_assert!(mempool_tx_queue.now_or_never().is_none());
 
-            Ok::<_, TestCaseError>(())
+            Ok(())
         })?;
     }
 }
@@ -1021,3 +767,62 @@ fn check_err_code<T>(rsp: Result<T, Error>, error_code: ErrorCode) -> Result<(),
 
     Ok(())
 }
+
+/// Creates mocked:
+///
+/// 1. mempool service,
+/// 2. state service,
+/// 3. rpc service,
+///
+/// and a handle to the mempool tx queue.
+fn mock_services<Tip>(
+    network: Network,
+    chain_tip: Tip,
+) -> (
+    zebra_test::mock_service::MockService<
+        zebra_node_services::mempool::Request,
+        zebra_node_services::mempool::Response,
+        zebra_test::mock_service::PropTestAssertion,
+    >,
+    zebra_test::mock_service::MockService<
+        zebra_state::ReadRequest,
+        zebra_state::ReadResponse,
+        zebra_test::mock_service::PropTestAssertion,
+    >,
+    methods::RpcImpl<
+        zebra_test::mock_service::MockService<
+            zebra_node_services::mempool::Request,
+            zebra_node_services::mempool::Response,
+            zebra_test::mock_service::PropTestAssertion,
+        >,
+        tower::buffer::Buffer<
+            zebra_test::mock_service::MockService<
+                zebra_state::ReadRequest,
+                zebra_state::ReadResponse,
+                zebra_test::mock_service::PropTestAssertion,
+            >,
+            zebra_state::ReadRequest,
+        >,
+        Tip,
+    >,
+    tokio::task::JoinHandle<()>,
+)
+where
+    Tip: ChainTip + Clone + Send + Sync + 'static,
+{
+    let mempool = MockService::build().for_prop_tests();
+    let state = MockService::build().for_prop_tests();
+
+    let (rpc, mempool_tx_queue) = RpcImpl::new(
+        "RPC test",
+        "RPC test",
+        network,
+        false,
+        true,
+        mempool.clone(),
+        Buffer::new(state.clone(), 1),
+        chain_tip,
+    );
+
+    (mempool, state, rpc, mempool_tx_queue)
+}

From f1fb94fac2f7d3d2b6ea16d162c60e2a55d69682 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 21 Nov 2024 13:03:51 +0100
Subject: [PATCH 32/45] Fix unit tests for `getaddresstxids`

---
 zebra-rpc/src/methods/tests/vectors.rs | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
index 0e315043e67..13561dc5ea4 100644
--- a/zebra-rpc/src/methods/tests/vectors.rs
+++ b/zebra-rpc/src/methods/tests/vectors.rs
@@ -734,25 +734,18 @@ async fn rpc_getaddresstxids_invalid_arguments() {
     );
 
     // call the method with an invalid address string
-    let address = "11111111".to_string();
-    let addresses = vec![address.clone()];
-    let start: u32 = 1;
-    let end: u32 = 2;
-    let error = rpc
+    let rpc_rsp = rpc
         .get_address_tx_ids(GetAddressTxIdsRequest {
-            addresses: addresses.clone(),
-            start,
-            end,
+            addresses: vec!["t1invalidaddress".to_owned()],
+            start: 1,
+            end: 2,
         })
         .await
         .unwrap_err();
-    assert_eq!(
-        error.message,
-        format!(
-            "invalid address \"{}\": parse error: t-addr decoding error",
-            address.clone()
-        )
-    );
+
+    assert_eq!(rpc_rsp.code, ErrorCode::ServerError(-5));
+
+    mempool.expect_no_requests().await;
 
     // create a valid address
     let address = "t3Vz22vK5z2LcKEdg16Yv4FFneEL1zg9ojd".to_string();

From 088ba625c45d3d8f89e66e05f5faf2997dd29749 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 21 Nov 2024 13:04:25 +0100
Subject: [PATCH 33/45] Fix unit tests for `getaddressutxos`

---
 zebra-rpc/src/methods/tests/vectors.rs | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
index 13561dc5ea4..7338eba82ef 100644
--- a/zebra-rpc/src/methods/tests/vectors.rs
+++ b/zebra-rpc/src/methods/tests/vectors.rs
@@ -935,17 +935,13 @@ async fn rpc_getaddressutxos_invalid_arguments() {
     );
 
     // call the method with an invalid address string
-    let address = "11111111".to_string();
-    let addresses = vec![address.clone()];
     let error = rpc
         .0
-        .get_address_utxos(AddressStrings::new(addresses))
+        .get_address_utxos(AddressStrings::new(vec!["t1invalidaddress".to_owned()]))
         .await
         .unwrap_err();
-    assert_eq!(
-        error.message,
-        format!("invalid address \"{address}\": parse error: t-addr decoding error")
-    );
+
+    assert_eq!(error.code, ErrorCode::ServerError(-5));
 
     mempool.expect_no_requests().await;
     state.expect_no_requests().await;

From 110f4b561cbdcbed3489cddd405343d2185333eb Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 21 Nov 2024 13:19:47 +0100
Subject: [PATCH 34/45] fix docs

---
 zebra-rpc/src/server/error.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index 2e38b06bb27..2251fa46f86 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -2,7 +2,7 @@
 
 /// Bitcoin RPC error codes
 ///
-/// Drawn from https://github.com/zcash/zcash/blob/master/src/rpc/protocol.h#L32-L80.
+/// Drawn from <https://github.com/zcash/zcash/blob/master/src/rpc/protocol.h#L32-L80>.
 ///
 /// ## Notes
 ///

From cdc21da2e91e11d2e059bd701d3f87ad5a66d30d Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 21 Nov 2024 14:52:27 +0100
Subject: [PATCH 35/45] Update snapshots for `getrawtransaction`

---
 zebra-rpc/src/methods/tests/snapshot.rs       | 52 ++++++++++---------
 ...aw_transaction_verbosity_0@mainnet_10.snap |  5 --
 ...aw_transaction_verbosity_0@testnet_10.snap |  5 --
 ...aw_transaction_verbosity_1@mainnet_10.snap |  9 ----
 ...aw_transaction_verbosity_1@testnet_10.snap |  9 ----
 ...awtransaction_invalid_txid@mainnet_10.snap | 11 ++++
 ...awtransaction_invalid_txid@testnet_10.snap | 11 ++++
 ...awtransaction_unknown_txid@mainnet_10.snap | 11 ++++
 ...awtransaction_unknown_txid@testnet_10.snap | 11 ++++
 ...rawtransaction_verbosity=0@mainnet_10.snap |  8 +++
 ...rawtransaction_verbosity=0@testnet_10.snap |  8 +++
 ...rawtransaction_verbosity=1@mainnet_10.snap | 12 +++++
 ...rawtransaction_verbosity=1@testnet_10.snap | 12 +++++
 13 files changed, 112 insertions(+), 52 deletions(-)
 delete mode 100644 zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_0@mainnet_10.snap
 delete mode 100644 zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_0@testnet_10.snap
 delete mode 100644 zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_1@mainnet_10.snap
 delete mode 100644 zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_1@testnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_unknown_txid@mainnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_unknown_txid@testnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=0@mainnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=0@testnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=1@mainnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=1@testnet_10.snap

diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs
index 99d4ce56c2a..4428bad7d67 100644
--- a/zebra-rpc/src/methods/tests/snapshot.rs
+++ b/zebra-rpc/src/methods/tests/snapshot.rs
@@ -406,8 +406,8 @@ async fn test_rpc_response_data_for_network(network: &Network) {
     // `getrawtransaction` verbosity=0
     //
     // - Similarly to `getrawmempool` described above, a mempool request will be made to get the
-    // requested transaction from the mempool. Response will be empty as we have this transaction in
-    // state.
+    //   requested transaction from the mempool. Response will be empty as we have this transaction
+    //   in the state.
     let mempool_req = mempool
         .expect_request_that(|request| {
             matches!(request, mempool::Request::TransactionsByMinedId(_))
@@ -423,12 +423,10 @@ async fn test_rpc_response_data_for_network(network: &Network) {
             .to_vec(),
     );
 
-    // make the api call
-    let get_raw_tx = rpc.get_raw_transaction(txid.clone(), Some(0u8));
-    let (response, _) = futures::join!(get_raw_tx, mempool_req);
-    let get_raw_transaction = response.expect("We should have a GetRawTransaction struct");
-
-    snapshot_rpc_getrawtransaction("verbosity_0", get_raw_transaction, &settings);
+    let rpc_req = rpc.get_raw_transaction(txid.clone(), Some(0u8));
+    let (rsp, _) = futures::join!(rpc_req, mempool_req);
+    settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_verbosity=0"), rsp));
+    mempool.expect_no_requests().await;
 
     // `getrawtransaction` verbosity=1
     let mempool_req = mempool
@@ -439,12 +437,29 @@ async fn test_rpc_response_data_for_network(network: &Network) {
             responder.respond(mempool::Response::Transactions(vec![]));
         });
 
-    // make the api call
-    let get_raw_tx = rpc.get_raw_transaction(txid, Some(1u8));
-    let (response, _) = futures::join!(get_raw_tx, mempool_req);
-    let get_raw_transaction = response.expect("We should have a GetRawTransaction struct");
+    let rpc_req = rpc.get_raw_transaction(txid, Some(1u8));
+    let (rsp, _) = futures::join!(rpc_req, mempool_req);
+    settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_verbosity=1"), rsp));
+    mempool.expect_no_requests().await;
 
-    snapshot_rpc_getrawtransaction("verbosity_1", get_raw_transaction, &settings);
+    // `getrawtransaction` with invalid txid
+    let rsp = rpc.get_raw_transaction(HexData(vec![0; 31]), Some(1)).await;
+    settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_invalid_txid"), rsp));
+    mempool.expect_no_requests().await;
+
+    // `getrawtransaction` with unknown txid
+    let mempool_req = mempool
+        .expect_request_that(|request| {
+            matches!(request, mempool::Request::TransactionsByMinedId(_))
+        })
+        .map(|responder| {
+            responder.respond(mempool::Response::Transactions(vec![]));
+        });
+
+    let rpc_req = rpc.get_raw_transaction(HexData(vec![0; 32]), Some(1));
+    let (rsp, _) = futures::join!(rpc_req, mempool_req);
+    settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_unknown_txid"), rsp));
+    mempool.expect_no_requests().await;
 
     // `getaddresstxids`
     let get_address_tx_ids = rpc
@@ -670,17 +685,6 @@ fn snapshot_rpc_getrawmempool(raw_mempool: Vec<String>, settings: &insta::Settin
     settings.bind(|| insta::assert_json_snapshot!("get_raw_mempool", raw_mempool));
 }
 
-/// Snapshot `getrawtransaction` response, using `cargo insta` and JSON serialization.
-fn snapshot_rpc_getrawtransaction(
-    variant: &'static str,
-    raw_transaction: GetRawTransaction,
-    settings: &insta::Settings,
-) {
-    settings.bind(|| {
-        insta::assert_json_snapshot!(format!("get_raw_transaction_{variant}"), raw_transaction)
-    });
-}
-
 /// Snapshot valid `getaddressbalance` response, using `cargo insta` and JSON serialization.
 fn snapshot_rpc_getaddresstxids_valid(
     variant: &'static str,
diff --git a/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_0@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_0@mainnet_10.snap
deleted file mode 100644
index fe57f682126..00000000000
--- a/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_0@mainnet_10.snap
+++ /dev/null
@@ -1,5 +0,0 @@
----
-source: zebra-rpc/src/methods/tests/snapshot.rs
-expression: raw_transaction
----
-"01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000"
diff --git a/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_0@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_0@testnet_10.snap
deleted file mode 100644
index 6f7145404de..00000000000
--- a/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_0@testnet_10.snap
+++ /dev/null
@@ -1,5 +0,0 @@
----
-source: zebra-rpc/src/methods/tests/snapshot.rs
-expression: raw_transaction
----
-"01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0250c30000000000002321025229e1240a21004cf8338db05679fa34753706e84f6aebba086ba04317fd8f99acd43000000000000017a914ef775f1f997f122a062fff1a2d7443abd1f9c6428700000000"
diff --git a/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_1@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_1@mainnet_10.snap
deleted file mode 100644
index 25091fe3fb5..00000000000
--- a/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_1@mainnet_10.snap
+++ /dev/null
@@ -1,9 +0,0 @@
----
-source: zebra-rpc/src/methods/tests/snapshot.rs
-expression: raw_transaction
----
-{
-  "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000",
-  "height": 1,
-  "confirmations": 10
-}
diff --git a/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_1@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_1@testnet_10.snap
deleted file mode 100644
index 61499b2e880..00000000000
--- a/zebra-rpc/src/methods/tests/snapshots/get_raw_transaction_verbosity_1@testnet_10.snap
+++ /dev/null
@@ -1,9 +0,0 @@
----
-source: zebra-rpc/src/methods/tests/snapshot.rs
-expression: raw_transaction
----
-{
-  "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0250c30000000000002321025229e1240a21004cf8338db05679fa34753706e84f6aebba086ba04317fd8f99acd43000000000000017a914ef775f1f997f122a062fff1a2d7443abd1f9c6428700000000",
-  "height": 1,
-  "confirmations": 10
-}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
new file mode 100644
index 00000000000..1aa88f2ff4a
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Err": {
+    "code": -5,
+    "message": "invalid TXID length"
+  }
+}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap
new file mode 100644
index 00000000000..1aa88f2ff4a
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap
@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Err": {
+    "code": -5,
+    "message": "invalid TXID length"
+  }
+}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_unknown_txid@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_unknown_txid@mainnet_10.snap
new file mode 100644
index 00000000000..878c8505a19
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_unknown_txid@mainnet_10.snap
@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Err": {
+    "code": -5,
+    "message": "No such mempool or main chain transaction"
+  }
+}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_unknown_txid@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_unknown_txid@testnet_10.snap
new file mode 100644
index 00000000000..878c8505a19
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_unknown_txid@testnet_10.snap
@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Err": {
+    "code": -5,
+    "message": "No such mempool or main chain transaction"
+  }
+}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=0@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=0@mainnet_10.snap
new file mode 100644
index 00000000000..90fa5021b56
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=0@mainnet_10.snap
@@ -0,0 +1,8 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Ok": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000"
+}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=0@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=0@testnet_10.snap
new file mode 100644
index 00000000000..673c9f7ce89
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=0@testnet_10.snap
@@ -0,0 +1,8 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Ok": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0250c30000000000002321025229e1240a21004cf8338db05679fa34753706e84f6aebba086ba04317fd8f99acd43000000000000017a914ef775f1f997f122a062fff1a2d7443abd1f9c6428700000000"
+}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=1@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=1@mainnet_10.snap
new file mode 100644
index 00000000000..b78a6686336
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=1@mainnet_10.snap
@@ -0,0 +1,12 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Ok": {
+    "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000",
+    "height": 1,
+    "confirmations": 10
+  }
+}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=1@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=1@testnet_10.snap
new file mode 100644
index 00000000000..ab133db9b1a
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_verbosity=1@testnet_10.snap
@@ -0,0 +1,12 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Ok": {
+    "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0250c30000000000002321025229e1240a21004cf8338db05679fa34753706e84f6aebba086ba04317fd8f99acd43000000000000017a914ef775f1f997f122a062fff1a2d7443abd1f9c6428700000000",
+    "height": 1,
+    "confirmations": 10
+  }
+}

From 475dbc9df3f041d5b6722539d136f8d2b3e56dfe Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Mon, 9 Dec 2024 15:30:58 +0100
Subject: [PATCH 36/45] Update zebra-rpc/src/server/error.rs

Co-authored-by: Arya <aryasolhi@gmail.com>
---
 zebra-rpc/src/server/error.rs | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index 2251fa46f86..42a3f593f4b 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -58,16 +58,21 @@ impl From<LegacyCode> for jsonrpc_core::ErrorCode {
 }
 
 /// A trait for mapping errors to [`jsonrpc_core::Error`].
-pub(crate) trait MapError<T> {
+pub(crate) trait MapError<T>: Sized {
     /// Maps errors to [`jsonrpc_core::Error`] with a specific error code.
     fn map_error(
         self,
         code: impl Into<jsonrpc_core::ErrorCode>,
     ) -> std::result::Result<T, jsonrpc_core::Error>;
+
+    /// Maps errors to [`jsonrpc_core::Error`] with a [`LegacyCode::Misc`] error code.
+    fn map_misc_error(self) -> std::result::Result<T, jsonrpc_core::Error> {
+        self.map_error(LegacyCode::Misc)
+    }
 }
 
 /// A trait for conditionally converting a value into a `Result<T, jsonrpc_core::Error>`.
-pub(crate) trait OkOrError<T> {
+pub(crate) trait OkOrError<T>: Sized {
     /// Converts the implementing type to `Result<T, jsonrpc_core::Error>`, using an error code and
     /// message if conversion is to `Err`.
     fn ok_or_error(
@@ -75,6 +80,14 @@ pub(crate) trait OkOrError<T> {
         code: impl Into<jsonrpc_core::ErrorCode>,
         message: impl ToString,
     ) -> std::result::Result<T, jsonrpc_core::Error>;
+
+    /// Converts the implementing type to `Result<T, jsonrpc_core::Error>`, using a [`LegacyCode::Misc`] error code.
+    fn ok_or_misc_error(
+        self,
+        message: impl ToString,
+    ) -> std::result::Result<T, jsonrpc_core::Error> {
+        self.ok_or_error(LegacyCode::Misc, message)
+    }
 }
 
 impl<T, E> MapError<T> for Result<T, E>

From bdb837284d7fe8089b25e19d180b68d82a2b3ef6 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Tue, 10 Dec 2024 18:20:41 +0100
Subject: [PATCH 37/45] Use `transaction::Hash` instead of `HexData`

---
 zebra-rpc/src/methods.rs                      | 15 +++--------
 zebra-rpc/src/methods/tests/prop.rs           | 26 +++----------------
 zebra-rpc/src/methods/tests/snapshot.rs       | 16 +++---------
 ...awtransaction_invalid_txid@mainnet_10.snap | 11 --------
 ...awtransaction_invalid_txid@testnet_10.snap | 11 --------
 zebra-rpc/src/methods/tests/vectors.rs        | 12 ++++-----
 6 files changed, 16 insertions(+), 75 deletions(-)
 delete mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
 delete mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 3e81d64d28c..a6dd144ffad 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -289,7 +289,7 @@ pub trait Rpc {
     #[rpc(name = "getrawtransaction")]
     fn get_raw_transaction(
         &self,
-        txid: HexData,
+        txid: transaction::Hash,
         verbose: Option<u8>,
     ) -> BoxFuture<Result<GetRawTransaction>>;
 
@@ -791,7 +791,7 @@ where
                     zebra_state::ReadResponse::Block(Some(block)) => {
                         Ok(GetBlock::Raw(block.into()))
                     }
-                    zebra_state::ReadResponse::Block(None) => Err("block not found")
+                    zebra_state::ReadResponse::Block(None) => Err("Block not found")
                         .map_error(server::error::LegacyCode::InvalidParameter),
                     _ => unreachable!("unmatched response to a block request"),
                 }
@@ -1121,7 +1121,7 @@ where
 
     fn get_raw_transaction(
         &self,
-        HexData(txid): HexData,
+        txid: transaction::Hash,
         verbose: Option<u8>,
     ) -> BoxFuture<Result<GetRawTransaction>> {
         let mut state = self.state.clone();
@@ -1129,15 +1129,6 @@ where
         let verbose = verbose.unwrap_or(0) != 0;
 
         async move {
-            // Reference for the legacy error code:
-            // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/rawtransaction.cpp#L544>
-            let txid = transaction::Hash::from_bytes_in_display_order(
-                &txid
-                    .try_into()
-                    .map_err(|_| "invalid TXID length")
-                    .map_error(server::error::LegacyCode::InvalidAddressOrKey)?,
-            );
-
             // Check the mempool first.
             match mempool
                 .ready()
diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs
index 6be04612d28..9d117b98c18 100644
--- a/zebra-rpc/src/methods/tests/prop.rs
+++ b/zebra-rpc/src/methods/tests/prop.rs
@@ -25,7 +25,7 @@ use zebra_state::BoxError;
 
 use zebra_test::mock_service::MockService;
 
-use crate::methods::{self, hex_data::HexData};
+use crate::methods;
 
 use super::super::{
     AddressBalance, AddressStrings, NetworkUpgradeStatus, Rpc, RpcImpl, SentTransactionHash,
@@ -299,18 +299,10 @@ proptest! {
         })?;
     }
 
-    /// Calls `get_raw_transaction` with:
-    ///
-    /// 1. an invalid TXID that won't deserialize due to wrong length;
-    /// 2. a valid TXID that is not in the mempool nor in the state;
-    ///
+    /// Calls `get_raw_transaction` with a valid TXID that is not in the mempool nor in the state,
     /// and checks that the RPC returns the right error code.
     #[test]
-    fn check_err_for_get_raw_transaction(
-        invalid_txid in vec(any::<u8>(), 0..31),
-        unknown_txid: [u8; 32],
-        network in any::<Network>()
-    ) {
+    fn check_err_for_get_raw_transaction(txid: transaction::Hash, network in any::<Network>()) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
         let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
@@ -319,16 +311,6 @@ proptest! {
         tokio::time::pause();
 
         runtime.block_on(async move {
-            let rpc_query = rpc.get_raw_transaction(HexData(invalid_txid), Some(1));
-
-            check_err_code(rpc_query.await, ErrorCode::ServerError(-5))?;
-
-            // Check that no further requests were made.
-            mempool.expect_no_requests().await?;
-            state.expect_no_requests().await?;
-
-            let txid = transaction::Hash::from_bytes_in_display_order(&unknown_txid);
-
             let mempool_query = mempool
                 .expect_request(mempool::Request::TransactionsByMinedId([txid].into()))
                 .map_ok(|r| r.respond(mempool::Response::Transactions(vec![])));
@@ -337,7 +319,7 @@ proptest! {
                 .expect_request(zebra_state::ReadRequest::Transaction(txid))
                 .map_ok(|r| r.respond(zebra_state::ReadResponse::Transaction(None)));
 
-            let rpc_query = rpc.get_raw_transaction(HexData(unknown_txid.to_vec()), Some(1));
+            let rpc_query = rpc.get_raw_transaction(txid, Some(1));
 
             let (rsp, _, _) =  tokio::join!(rpc_query, mempool_query, state_query);
 
diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs
index 4428bad7d67..f7a9722d988 100644
--- a/zebra-rpc/src/methods/tests/snapshot.rs
+++ b/zebra-rpc/src/methods/tests/snapshot.rs
@@ -416,14 +416,9 @@ async fn test_rpc_response_data_for_network(network: &Network) {
             responder.respond(mempool::Response::Transactions(vec![]));
         });
 
-    let txid = HexData(
-        first_block_first_tx
-            .hash()
-            .bytes_in_display_order()
-            .to_vec(),
-    );
+    let txid = first_block_first_tx.hash();
 
-    let rpc_req = rpc.get_raw_transaction(txid.clone(), Some(0u8));
+    let rpc_req = rpc.get_raw_transaction(txid, Some(0u8));
     let (rsp, _) = futures::join!(rpc_req, mempool_req);
     settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_verbosity=0"), rsp));
     mempool.expect_no_requests().await;
@@ -442,11 +437,6 @@ async fn test_rpc_response_data_for_network(network: &Network) {
     settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_verbosity=1"), rsp));
     mempool.expect_no_requests().await;
 
-    // `getrawtransaction` with invalid txid
-    let rsp = rpc.get_raw_transaction(HexData(vec![0; 31]), Some(1)).await;
-    settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_invalid_txid"), rsp));
-    mempool.expect_no_requests().await;
-
     // `getrawtransaction` with unknown txid
     let mempool_req = mempool
         .expect_request_that(|request| {
@@ -456,7 +446,7 @@ async fn test_rpc_response_data_for_network(network: &Network) {
             responder.respond(mempool::Response::Transactions(vec![]));
         });
 
-    let rpc_req = rpc.get_raw_transaction(HexData(vec![0; 32]), Some(1));
+    let rpc_req = rpc.get_raw_transaction([0; 32].into(), Some(1));
     let (rsp, _) = futures::join!(rpc_req, mempool_req);
     settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_unknown_txid"), rsp));
     mempool.expect_no_requests().await;
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
deleted file mode 100644
index 1aa88f2ff4a..00000000000
--- a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
+++ /dev/null
@@ -1,11 +0,0 @@
----
-source: zebra-rpc/src/methods/tests/snapshot.rs
-expression: rsp
-snapshot_kind: text
----
-{
-  "Err": {
-    "code": -5,
-    "message": "invalid TXID length"
-  }
-}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap
deleted file mode 100644
index 1aa88f2ff4a..00000000000
--- a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap
+++ /dev/null
@@ -1,11 +0,0 @@
----
-source: zebra-rpc/src/methods/tests/snapshot.rs
-expression: rsp
-snapshot_kind: text
----
-{
-  "Err": {
-    "code": -5,
-    "message": "invalid TXID length"
-  }
-}
diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
index c1dafda6f06..6ff4a328d04 100644
--- a/zebra-rpc/src/methods/tests/vectors.rs
+++ b/zebra-rpc/src/methods/tests/vectors.rs
@@ -725,9 +725,10 @@ async fn rpc_getrawtransaction() {
                     }]));
                 });
 
-            let txid = HexData(tx.hash().bytes_in_display_order().to_vec());
-            let (rsp, _) = futures::join!(rpc.get_raw_transaction(txid, Some(0u8)), mempool_req);
-            let get_tx = rsp.expect("We should have a GetRawTransaction struct");
+            let rpc_req = rpc.get_raw_transaction(tx.hash(), Some(0u8));
+
+            let (rsp, _) = futures::join!(rpc_req, mempool_req);
+            let get_tx = rsp.expect("We should have a ");
 
             if let GetRawTransaction::Raw(raw_tx) = get_tx {
                 assert_eq!(raw_tx.as_ref(), tx.zcash_serialize_to_vec().unwrap());
@@ -757,10 +758,9 @@ async fn rpc_getrawtransaction() {
     let run_state_test_case = |block_idx: usize, block: Arc<Block>, tx: Arc<Transaction>| {
         let read_state = read_state.clone();
         let txid = tx.hash();
-        let hex_txid = HexData(txid.bytes_in_display_order().to_vec());
 
-        let get_tx_verbose_0_req = rpc.get_raw_transaction(hex_txid.clone(), Some(0u8));
-        let get_tx_verbose_1_req = rpc.get_raw_transaction(hex_txid, Some(1u8));
+        let get_tx_verbose_0_req = rpc.get_raw_transaction(txid, Some(0u8));
+        let get_tx_verbose_1_req = rpc.get_raw_transaction(txid, Some(1u8));
 
         async move {
             let (response, _) = futures::join!(get_tx_verbose_0_req, make_mempool_req(txid));

From ed693598c69a0ef3b1e92e982783e0992d61399b Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Tue, 10 Dec 2024 19:20:19 +0100
Subject: [PATCH 38/45] Simplify error handling

---
 zebra-rpc/src/methods.rs                      | 81 ++++++++-----------
 .../get_block_template.rs                     | 17 ++--
 2 files changed, 38 insertions(+), 60 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index a6dd144ffad..a68d236dc57 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -562,7 +562,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error(server::error::LegacyCode::default())?;
+                .map_misc_error()?;
 
             let zebra_state::ReadResponse::TipPoolValues {
                 tip_height,
@@ -578,7 +578,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error(server::error::LegacyCode::default())?;
+                .map_misc_error()?;
 
             let zebra_state::ReadResponse::BlockHeader { header, .. } = response else {
                 unreachable!("unmatched response to a BlockHeader request")
@@ -669,10 +669,7 @@ where
             let valid_addresses = address_strings.valid_addresses()?;
 
             let request = zebra_state::ReadRequest::AddressBalance(valid_addresses);
-            let response = state
-                .oneshot(request)
-                .await
-                .map_error(server::error::LegacyCode::default())?;
+            let response = state.oneshot(request).await.map_misc_error()?;
 
             match response {
                 zebra_state::ReadResponse::AddressBalance(balance) => Ok(AddressBalance {
@@ -709,10 +706,7 @@ where
             let transaction_parameter = mempool::Gossip::Tx(raw_transaction.into());
             let request = mempool::Request::Queue(vec![transaction_parameter]);
 
-            let response = mempool
-                .oneshot(request)
-                .await
-                .map_error(server::error::LegacyCode::default())?;
+            let response = mempool.oneshot(request).await.map_misc_error()?;
 
             let mut queue_results = match response {
                 mempool::Response::Queued(results) => results,
@@ -729,9 +723,9 @@ where
                 .pop()
                 .expect("there should be exactly one item in Vec")
                 .inspect_err(|err| tracing::debug!("sent transaction to mempool: {:?}", &err))
-                .map_error(server::error::LegacyCode::default())?
+                .map_misc_error()?
                 .await
-                .map_error(server::error::LegacyCode::default())?;
+                .map_misc_error()?;
 
             tracing::debug!("sent transaction to mempool: {:?}", &queue_result);
 
@@ -785,7 +779,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_error(server::error::LegacyCode::default())?;
+                    .map_misc_error()?;
 
                 match response {
                     zebra_state::ReadResponse::Block(Some(block)) => {
@@ -844,9 +838,9 @@ where
                 }
 
                 let tx_ids_response = futs.next().await.expect("`futs` should not be empty");
-                let tx = match tx_ids_response.map_error(server::error::LegacyCode::default())? {
+                let tx = match tx_ids_response.map_misc_error()? {
                     zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids
-                        .ok_or_error(server::error::LegacyCode::default(), "block not found")?
+                        .ok_or_misc_error("block not found")?
                         .iter()
                         .map(|tx_id| tx_id.encode_hex())
                         .collect(),
@@ -863,8 +857,7 @@ where
                 let nu5_activation = NetworkUpgrade::Nu5.activation_height(&network);
 
                 // This could be `None` if there's a chain reorg between state queries.
-                let orchard_tree =
-                    orchard_tree.ok_or_misc_error("missing orchard tree for block")?;
+                let orchard_tree = orchard_tree.ok_or_misc_error("missing Orchard tree")?;
 
                 let final_orchard_root = match nu5_activation {
                     Some(activation_height) if height >= activation_height => {
@@ -933,50 +926,42 @@ where
                 .clone()
                 .oneshot(zebra_state::ReadRequest::BlockHeader(hash_or_height))
                 .await
-                .map_err(|_| Error {
+                .map_err(|_| "block height not in best chain")
+                .map_error(
                     // ## Compatibility with `zcashd`.
                     //
                     // Since this function is reused by getblock(), we return the errors
                     // expected by it (they differ whether a hash or a height was passed).
-                    code: if hash_or_height.hash().is_some() {
-                        server::error::LegacyCode::InvalidAddressOrKey.into()
+                    if hash_or_height.hash().is_some() {
+                        server::error::LegacyCode::InvalidAddressOrKey
                     } else {
-                        server::error::LegacyCode::InvalidParameter.into()
+                        server::error::LegacyCode::InvalidParameter
                     },
-                    message: "block height not in best chain".to_owned(),
-                    data: None,
-                })?
+                )?
             else {
                 panic!("unexpected response to BlockHeader request")
             };
 
             let response = if !verbose {
-                GetBlockHeader::Raw(HexData(
-                    header
-                        .zcash_serialize_to_vec()
-                        .map_error(server::error::LegacyCode::Misc)?,
-                ))
+                GetBlockHeader::Raw(HexData(header.zcash_serialize_to_vec().map_misc_error()?))
             } else {
                 let zebra_state::ReadResponse::SaplingTree(sapling_tree) = state
                     .clone()
                     .oneshot(zebra_state::ReadRequest::SaplingTree(hash_or_height))
                     .await
-                    .map_error(server::error::LegacyCode::Misc)?
+                    .map_misc_error()?
                 else {
                     panic!("unexpected response to SaplingTree request")
                 };
 
                 // This could be `None` if there's a chain reorg between state queries.
-                let sapling_tree = sapling_tree.ok_or_error(
-                    server::error::LegacyCode::Misc,
-                    "missing sapling tree for block",
-                )?;
+                let sapling_tree = sapling_tree.ok_or_misc_error("missing Sapling tree")?;
 
                 let zebra_state::ReadResponse::Depth(depth) = state
                     .clone()
                     .oneshot(zebra_state::ReadRequest::Depth(hash))
                     .await
-                    .map_error(server::error::LegacyCode::Misc)?
+                    .map_misc_error()?
                 else {
                     panic!("unexpected response to SaplingTree request")
                 };
@@ -1036,14 +1021,14 @@ where
         self.latest_chain_tip
             .best_tip_hash()
             .map(GetBlockHash)
-            .ok_or_error(server::error::LegacyCode::default(), "No blocks in state")
+            .ok_or_misc_error("No blocks in state")
     }
 
     fn get_best_block_height_and_hash(&self) -> Result<GetBlockHeightAndHash> {
         self.latest_chain_tip
             .best_tip_height_and_hash()
             .map(|(height, hash)| GetBlockHeightAndHash { height, hash })
-            .ok_or_error(server::error::LegacyCode::default(), "No blocks in state")
+            .ok_or_misc_error("No blocks in state")
     }
 
     fn get_raw_mempool(&self) -> BoxFuture<Result<Vec<String>>> {
@@ -1072,7 +1057,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error(server::error::LegacyCode::default())?;
+                .map_misc_error()?;
 
             match response {
                 #[cfg(feature = "getblocktemplate-rpcs")]
@@ -1136,7 +1121,7 @@ where
                     service.call(mempool::Request::TransactionsByMinedId([txid].into()))
                 })
                 .await
-                .map_error(server::error::LegacyCode::default())?
+                .map_misc_error()?
             {
                 mempool::Response::Transactions(txns) => {
                     if let Some(tx) = txns.first() {
@@ -1162,7 +1147,7 @@ where
                 .ready()
                 .and_then(|service| service.call(zebra_state::ReadRequest::Transaction(txid)))
                 .await
-                .map_error(server::error::LegacyCode::default())?
+                .map_misc_error()?
             {
                 zebra_state::ReadResponse::Transaction(Some(tx)) => {
                     let hex = tx.tx.into();
@@ -1215,7 +1200,7 @@ where
                 .ready()
                 .and_then(|service| service.call(zebra_state::ReadRequest::Block(hash_or_height)))
                 .await
-                .map_error(server::error::LegacyCode::default())?
+                .map_misc_error()?
             {
                 zebra_state::ReadResponse::Block(Some(block)) => block,
                 zebra_state::ReadResponse::Block(None) => {
@@ -1246,7 +1231,7 @@ where
                         service.call(zebra_state::ReadRequest::SaplingTree(hash.into()))
                     })
                     .await
-                    .map_error(server::error::LegacyCode::default())?
+                    .map_misc_error()?
                 {
                     zebra_state::ReadResponse::SaplingTree(tree) => tree.map(|t| t.to_rpc_bytes()),
                     _ => unreachable!("unmatched response to a Sapling tree request"),
@@ -1263,7 +1248,7 @@ where
                         service.call(zebra_state::ReadRequest::OrchardTree(hash.into()))
                     })
                     .await
-                    .map_error(server::error::LegacyCode::default())?
+                    .map_misc_error()?
                 {
                     zebra_state::ReadResponse::OrchardTree(tree) => tree.map(|t| t.to_rpc_bytes()),
                     _ => unreachable!("unmatched response to an Orchard tree request"),
@@ -1296,7 +1281,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_error(server::error::LegacyCode::default())?;
+                    .map_misc_error()?;
 
                 let subtrees = match response {
                     zebra_state::ReadResponse::SaplingSubtrees(subtrees) => subtrees,
@@ -1322,7 +1307,7 @@ where
                     .ready()
                     .and_then(|service| service.call(request))
                     .await
-                    .map_error(server::error::LegacyCode::default())?;
+                    .map_misc_error()?;
 
                 let subtrees = match response {
                     zebra_state::ReadResponse::OrchardSubtrees(subtrees) => subtrees,
@@ -1382,7 +1367,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error(server::error::LegacyCode::default())?;
+                .map_misc_error()?;
 
             let hashes = match response {
                 zebra_state::ReadResponse::AddressesTransactionIds(hashes) => {
@@ -1429,7 +1414,7 @@ where
                 .ready()
                 .and_then(|service| service.call(request))
                 .await
-                .map_error(server::error::LegacyCode::default())?;
+                .map_misc_error()?;
             let utxos = match response {
                 zebra_state::ReadResponse::AddressUtxos(utxos) => utxos,
                 _ => unreachable!("unmatched response to a UtxosByAddresses request"),
@@ -1507,7 +1492,7 @@ where
 {
     latest_chain_tip
         .best_tip_height()
-        .ok_or_error(server::error::LegacyCode::default(), "No blocks in state")
+        .ok_or_misc_error("No blocks in state")
 }
 
 /// Response to a `getinfo` RPC request.
diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
index e113459fcf4..3a934d629ff 100644
--- a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
+++ b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs
@@ -30,7 +30,7 @@ use crate::{
         constants::{MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP, NOT_SYNCED_ERROR_CODE},
         types::{default_roots::DefaultRoots, transaction::TransactionTemplate},
     },
-    server::{self, error::OkOrError},
+    server::error::OkOrError,
 };
 
 pub use crate::methods::get_block_template_rpcs::types::get_block_template::*;
@@ -87,13 +87,9 @@ pub fn check_parameters(parameters: &Option<JsonParameters>) -> Result<()> {
 pub fn check_miner_address(
     miner_address: Option<transparent::Address>,
 ) -> Result<transparent::Address> {
-    miner_address.ok_or_else(|| Error {
-        code: ErrorCode::ServerError(0),
-        message: "configure mining.miner_address in zebrad.toml \
-                  with a transparent address"
-            .to_string(),
-        data: None,
-    })
+    miner_address.ok_or_misc_error(
+        "set `mining.miner_address` in `zebrad.toml` to a transparent address".to_string(),
+    )
 }
 
 /// Attempts to validate block proposal against all of the server's
@@ -181,10 +177,7 @@ where
     // but this is ok for an estimate
     let (estimated_distance_to_chain_tip, local_tip_height) = latest_chain_tip
         .estimate_distance_to_network_chain_tip(network)
-        .ok_or_error(
-            server::error::LegacyCode::default(),
-            "no chain tip available yet",
-        )?;
+        .ok_or_misc_error("no chain tip available yet")?;
 
     if !sync_status.is_close_to_tip()
         || estimated_distance_to_chain_tip > MAX_ESTIMATED_DISTANCE_TO_NETWORK_CHAIN_TIP

From 37d1a44588c7600fe2668ed3d1a235c5b36b32c9 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Tue, 10 Dec 2024 19:51:16 +0100
Subject: [PATCH 39/45] Update zebra-rpc/src/server/error.rs

Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
---
 zebra-rpc/src/server/error.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zebra-rpc/src/server/error.rs b/zebra-rpc/src/server/error.rs
index 42a3f593f4b..4cfc7b38571 100644
--- a/zebra-rpc/src/server/error.rs
+++ b/zebra-rpc/src/server/error.rs
@@ -2,7 +2,7 @@
 
 /// Bitcoin RPC error codes
 ///
-/// Drawn from <https://github.com/zcash/zcash/blob/master/src/rpc/protocol.h#L32-L80>.
+/// Drawn from <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/protocol.h#L32-L80>.
 ///
 /// ## Notes
 ///

From 2c342f7e1ad48e261ae6b32ebd423e83b34fe800 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Tue, 10 Dec 2024 23:19:07 +0100
Subject: [PATCH 40/45] Move a note on performance

---
 zebra-rpc/src/methods.rs | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index a68d236dc57..7cc0b46b48e 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -742,6 +742,11 @@ where
         .boxed()
     }
 
+    // # Performance
+    //
+    // `lightwalletd` calls this RPC with verosity 1 for its initial sync of 2 million blocks, the
+    // performace of this RPC with verbosity 1 significantly affects `lightwalletd`s sync time.
+    //
     // TODO:
     // - use `height_from_signed_int()` to handle negative heights
     //   (this might be better in the state request, because it needs the state height)
@@ -770,10 +775,6 @@ where
                 .map_error(server::error::LegacyCode::InvalidParameter)?;
 
             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()

From 74fe0f408827f6af69dcf30e8abea0c1791e2e84 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Tue, 10 Dec 2024 23:27:43 +0100
Subject: [PATCH 41/45] Fix a typo

---
 zebra-rpc/src/methods.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 7cc0b46b48e..96b3d65d010 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -745,7 +745,7 @@ where
     // # Performance
     //
     // `lightwalletd` calls this RPC with verosity 1 for its initial sync of 2 million blocks, the
-    // performace of this RPC with verbosity 1 significantly affects `lightwalletd`s sync time.
+    // performance of this RPC with verbosity 1 significantly affects `lightwalletd`s sync time.
     //
     // TODO:
     // - use `height_from_signed_int()` to handle negative heights

From 6e1e011e802fc251c1305ba8e7b56d9740f916e0 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 12 Dec 2024 13:17:11 +0100
Subject: [PATCH 42/45] Use `String` instead of `transaction::Hash`

---
 zebra-rpc/src/methods.rs                | 9 +++++++--
 zebra-rpc/src/methods/tests/snapshot.rs | 6 +++---
 zebra-rpc/src/methods/tests/vectors.rs  | 7 ++++---
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 96b3d65d010..0d7986f033f 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -289,7 +289,7 @@ pub trait Rpc {
     #[rpc(name = "getrawtransaction")]
     fn get_raw_transaction(
         &self,
-        txid: transaction::Hash,
+        txid: String,
         verbose: Option<u8>,
     ) -> BoxFuture<Result<GetRawTransaction>>;
 
@@ -1107,7 +1107,7 @@ where
 
     fn get_raw_transaction(
         &self,
-        txid: transaction::Hash,
+        txid: String,
         verbose: Option<u8>,
     ) -> BoxFuture<Result<GetRawTransaction>> {
         let mut state = self.state.clone();
@@ -1115,6 +1115,11 @@ where
         let verbose = verbose.unwrap_or(0) != 0;
 
         async move {
+            // Reference for the legacy error code:
+            // <https://github.com/zcash/zcash/blob/99ad6fdc3a549ab510422820eea5e5ce9f60a5fd/src/rpc/rawtransaction.cpp#L544>
+            let txid = transaction::Hash::from_hex(txid)
+                .map_error(server::error::LegacyCode::InvalidAddressOrKey)?;
+
             // Check the mempool first.
             match mempool
                 .ready()
diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs
index f7a9722d988..eff36b3f991 100644
--- a/zebra-rpc/src/methods/tests/snapshot.rs
+++ b/zebra-rpc/src/methods/tests/snapshot.rs
@@ -416,9 +416,9 @@ async fn test_rpc_response_data_for_network(network: &Network) {
             responder.respond(mempool::Response::Transactions(vec![]));
         });
 
-    let txid = first_block_first_tx.hash();
+    let txid = first_block_first_tx.hash().encode_hex::<String>();
 
-    let rpc_req = rpc.get_raw_transaction(txid, Some(0u8));
+    let rpc_req = rpc.get_raw_transaction(txid.clone(), Some(0u8));
     let (rsp, _) = futures::join!(rpc_req, mempool_req);
     settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_verbosity=0"), rsp));
     mempool.expect_no_requests().await;
@@ -446,7 +446,7 @@ async fn test_rpc_response_data_for_network(network: &Network) {
             responder.respond(mempool::Response::Transactions(vec![]));
         });
 
-    let rpc_req = rpc.get_raw_transaction([0; 32].into(), Some(1));
+    let rpc_req = rpc.get_raw_transaction(transaction::Hash::from([0; 32]).encode_hex(), Some(1));
     let (rsp, _) = futures::join!(rpc_req, mempool_req);
     settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_unknown_txid"), rsp));
     mempool.expect_no_requests().await;
diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
index 6ff4a328d04..f4ae6257be8 100644
--- a/zebra-rpc/src/methods/tests/vectors.rs
+++ b/zebra-rpc/src/methods/tests/vectors.rs
@@ -725,7 +725,7 @@ async fn rpc_getrawtransaction() {
                     }]));
                 });
 
-            let rpc_req = rpc.get_raw_transaction(tx.hash(), Some(0u8));
+            let rpc_req = rpc.get_raw_transaction(tx.hash().encode_hex(), Some(0u8));
 
             let (rsp, _) = futures::join!(rpc_req, mempool_req);
             let get_tx = rsp.expect("We should have a ");
@@ -758,9 +758,10 @@ async fn rpc_getrawtransaction() {
     let run_state_test_case = |block_idx: usize, block: Arc<Block>, tx: Arc<Transaction>| {
         let read_state = read_state.clone();
         let txid = tx.hash();
+        let hex_txid = txid.encode_hex::<String>();
 
-        let get_tx_verbose_0_req = rpc.get_raw_transaction(txid, Some(0u8));
-        let get_tx_verbose_1_req = rpc.get_raw_transaction(txid, Some(1u8));
+        let get_tx_verbose_0_req = rpc.get_raw_transaction(hex_txid.clone(), Some(0u8));
+        let get_tx_verbose_1_req = rpc.get_raw_transaction(hex_txid, Some(1u8));
 
         async move {
             let (response, _) = futures::join!(get_tx_verbose_0_req, make_mempool_req(txid));

From ae6000f9f37d8352a2a2188cad37174dcb1c80cc Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 12 Dec 2024 13:17:49 +0100
Subject: [PATCH 43/45] Adjust and add proptests

---
 zebra-rpc/src/methods/tests/prop.rs | 43 +++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs
index 9d117b98c18..0af5e03b0b9 100644
--- a/zebra-rpc/src/methods/tests/prop.rs
+++ b/zebra-rpc/src/methods/tests/prop.rs
@@ -3,7 +3,7 @@
 use std::{collections::HashSet, fmt::Debug, sync::Arc};
 
 use futures::{join, FutureExt, TryFutureExt};
-use hex::ToHex;
+use hex::{FromHex, ToHex};
 use jsonrpc_core::{Error, ErrorCode};
 use proptest::{collection::vec, prelude::*};
 use thiserror::Error;
@@ -299,10 +299,16 @@ proptest! {
         })?;
     }
 
-    /// Calls `get_raw_transaction` with a valid TXID that is not in the mempool nor in the state,
+    /// Calls `get_raw_transaction` with:
+    ///
+    /// 1. an invalid TXID that won't deserialize;
+    /// 2. a valid TXID that is not in the mempool nor in the state;
+    ///
     /// and checks that the RPC returns the right error code.
     #[test]
-    fn check_err_for_get_raw_transaction(txid: transaction::Hash, network in any::<Network>()) {
+    fn check_err_for_get_raw_transaction(unknown_txid: transaction::Hash,
+                                         invalid_txid in invalid_txid(),
+                                         network: Network) {
         let (runtime, _init_guard) = zebra_test::init_async();
         let _guard = runtime.enter();
         let (mut mempool, mut state, rpc, mempool_tx_queue) = mock_services(network, NoChainTip);
@@ -311,19 +317,29 @@ proptest! {
         tokio::time::pause();
 
         runtime.block_on(async move {
+            // Check the invalid TXID first.
+            let rpc_rsp = rpc.get_raw_transaction(invalid_txid, Some(1)).await;
+
+            check_err_code(rpc_rsp, ErrorCode::ServerError(-5))?;
+
+            // Check that no further requests were made.
+            mempool.expect_no_requests().await?;
+            state.expect_no_requests().await?;
+
+            // Now check the unknown TXID.
             let mempool_query = mempool
-                .expect_request(mempool::Request::TransactionsByMinedId([txid].into()))
+                .expect_request(mempool::Request::TransactionsByMinedId([unknown_txid].into()))
                 .map_ok(|r| r.respond(mempool::Response::Transactions(vec![])));
 
             let state_query = state
-                .expect_request(zebra_state::ReadRequest::Transaction(txid))
+                .expect_request(zebra_state::ReadRequest::Transaction(unknown_txid))
                 .map_ok(|r| r.respond(zebra_state::ReadResponse::Transaction(None)));
 
-            let rpc_query = rpc.get_raw_transaction(txid, Some(1));
+            let rpc_query = rpc.get_raw_transaction(unknown_txid.encode_hex(), Some(1));
 
-            let (rsp, _, _) =  tokio::join!(rpc_query, mempool_query, state_query);
+            let (rpc_rsp, _, _) =  tokio::join!(rpc_query, mempool_query, state_query);
 
-            check_err_code(rsp, ErrorCode::ServerError(-5))?;
+            check_err_code(rpc_rsp, ErrorCode::ServerError(-5))?;
 
             // The queue task should continue without errors or panics
             prop_assert!(mempool_tx_queue.now_or_never().is_none());
@@ -740,6 +756,17 @@ proptest! {
 #[error("a dummy error type")]
 pub struct DummyError;
 
+// Helper functions
+
+/// Creates [`String`]s that won't deserialize into [`transaction::Hash`].
+fn invalid_txid() -> BoxedStrategy<String> {
+    any::<String>()
+        .prop_filter("string must not deserialize into TXID", |s| {
+            transaction::Hash::from_hex(s).is_err()
+        })
+        .boxed()
+}
+
 /// Checks that the given RPC response contains the given error code.
 fn check_err_code<T>(rsp: Result<T, Error>, error_code: ErrorCode) -> Result<(), TestCaseError> {
     prop_assert!(

From 7db6ff6bb2501e9a5df4ec50c957c4b8ca8a83db Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 12 Dec 2024 14:19:47 +0100
Subject: [PATCH 44/45] Reintroduce snapshots for invalid TXIDs

---
 zebra-rpc/src/methods/tests/snapshot.rs               |  7 +++++++
 .../getrawtransaction_invalid_txid@mainnet_10.snap    | 11 +++++++++++
 .../getrawtransaction_invalid_txid@testnet_10.snap    | 11 +++++++++++
 zebra-rpc/src/methods/tests/vectors.rs                |  2 +-
 4 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
 create mode 100644 zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap

diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs
index eff36b3f991..2bdec5d7497 100644
--- a/zebra-rpc/src/methods/tests/snapshot.rs
+++ b/zebra-rpc/src/methods/tests/snapshot.rs
@@ -451,6 +451,13 @@ async fn test_rpc_response_data_for_network(network: &Network) {
     settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_unknown_txid"), rsp));
     mempool.expect_no_requests().await;
 
+    // `getrawtransaction` with an invalid TXID
+    let rsp = rpc
+        .get_raw_transaction("aBadC0de".to_owned(), Some(1))
+        .await;
+    settings.bind(|| insta::assert_json_snapshot!(format!("getrawtransaction_invalid_txid"), rsp));
+    mempool.expect_no_requests().await;
+
     // `getaddresstxids`
     let get_address_tx_ids = rpc
         .get_address_tx_ids(GetAddressTxIdsRequest {
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
new file mode 100644
index 00000000000..e048f7ad516
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@mainnet_10.snap
@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Err": {
+    "code": -5,
+    "message": "Invalid string length"
+  }
+}
diff --git a/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap
new file mode 100644
index 00000000000..e048f7ad516
--- /dev/null
+++ b/zebra-rpc/src/methods/tests/snapshots/getrawtransaction_invalid_txid@testnet_10.snap
@@ -0,0 +1,11 @@
+---
+source: zebra-rpc/src/methods/tests/snapshot.rs
+expression: rsp
+snapshot_kind: text
+---
+{
+  "Err": {
+    "code": -5,
+    "message": "Invalid string length"
+  }
+}
diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
index f4ae6257be8..998cdc155ee 100644
--- a/zebra-rpc/src/methods/tests/vectors.rs
+++ b/zebra-rpc/src/methods/tests/vectors.rs
@@ -728,7 +728,7 @@ async fn rpc_getrawtransaction() {
             let rpc_req = rpc.get_raw_transaction(tx.hash().encode_hex(), Some(0u8));
 
             let (rsp, _) = futures::join!(rpc_req, mempool_req);
-            let get_tx = rsp.expect("We should have a ");
+            let get_tx = rsp.expect("we should have a `GetRawTransaction` struct");
 
             if let GetRawTransaction::Raw(raw_tx) = get_tx {
                 assert_eq!(raw_tx.as_ref(), tx.zcash_serialize_to_vec().unwrap());

From c9088f8f97ea70659ae6f333f2fcb7735ac451b6 Mon Sep 17 00:00:00 2001
From: Marek <mail@marek.onl>
Date: Thu, 12 Dec 2024 17:07:41 +0100
Subject: [PATCH 45/45] Don't derive `Serialize` & `Deserialize` for txids

Deriving `serde::Serialize` & `serde::Deserialize` for
`transaction::Hash` was superfluous, and we didn't need it anywhere in
the code.
---
 zebra-chain/src/transaction/hash.rs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/zebra-chain/src/transaction/hash.rs b/zebra-chain/src/transaction/hash.rs
index a7fa60066d2..98ba39692b7 100644
--- a/zebra-chain/src/transaction/hash.rs
+++ b/zebra-chain/src/transaction/hash.rs
@@ -34,7 +34,6 @@ use std::{fmt, sync::Arc};
 use proptest_derive::Arbitrary;
 
 use hex::{FromHex, ToHex};
-use serde::{Deserialize, Serialize};
 
 use crate::serialization::{
     ReadZcashExt, SerializationError, WriteZcashExt, ZcashDeserialize, ZcashSerialize,
@@ -56,7 +55,7 @@ use super::{txid::TxIdBuilder, AuthDigest, Transaction};
 ///
 /// [ZIP-244]: https://zips.z.cash/zip-0244
 /// [Spec: Transaction Identifiers]: https://zips.z.cash/protocol/protocol.pdf#txnidentifiers
-#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize, Hash)]
+#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
 pub struct Hash(pub [u8; 32]);