From b0c4d19a7ccb543a47d1c5809bfa72f1ffcf9df0 Mon Sep 17 00:00:00 2001 From: Marek Date: Fri, 13 Dec 2024 15:01:53 +0100 Subject: [PATCH] fix(rpc): Refactor `getrawtransaction` & RPC error handling (#9049) * clean-up: simplify the def of `MapServerError` * Use `HexData` instead of `String` for TXIDs * Remove a redundant test We don't need such a test anymore because the deserialization is handled by Serde now. * Adjust tests for using `HexData` * Make `height` and `confirmations` optional * Use legacy error codes * fmt * Remove unneeded error codes * Remove `zebra-rpc/src/constants.rs` * Rename `MapServerError` to `MapError` * Rename `OkOrServerError` to `OkOrError` * Allow specifying error codes when mapping errors * Allow setting error codes when mapping options * Use the right error code for `getrawtransaction` * fmt * Add docs for the error conversion traits * Refactor the error handling for `getblock` * Refactor error handling in `sendrawtransaction` * Refactor the error handling for `getblock` * Update the error handling for `getrawtransaction` * Refactor error handling for `z_gettreestate` * Refactor the error handling for address parsing * Refactor the error handling for getrawtransaction * Update `z_gettreestate` snapshots * Cosmetics * Refactor error handling in `getblock` * Refactor error handling in `getblockheader` * Simplify `getrawtransaction` * Check errors for `getrawtransaction` * fmt * Simplify proptests * Fix unit tests for `getaddresstxids` * Fix unit tests for `getaddressutxos` * fix docs * Update snapshots for `getrawtransaction` * Update zebra-rpc/src/server/error.rs Co-authored-by: Arya * Use `transaction::Hash` instead of `HexData` * Simplify error handling * Update zebra-rpc/src/server/error.rs Co-authored-by: Alfredo Garcia * Move a note on performance * Fix a typo * Use `String` instead of `transaction::Hash` * Adjust and add proptests * Reintroduce snapshots for invalid TXIDs * 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. --------- Co-authored-by: Arya Co-authored-by: Alfredo Garcia Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- zebra-chain/src/transaction/hash.rs | 3 +- zebra-rpc/src/constants.rs | 44 -- zebra-rpc/src/lib.rs | 1 - zebra-rpc/src/methods.rs | 313 ++++---- zebra-rpc/src/methods/errors.rs | 37 - .../src/methods/get_block_template_rpcs.rs | 93 ++- .../get_block_template.rs | 18 +- zebra-rpc/src/methods/tests/prop.rs | 699 ++++++------------ zebra-rpc/src/methods/tests/snapshot.rs | 65 +- ...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 + ...e_by_non_existent_hash@custom_testnet.snap | 3 +- ...excessive_block_height@custom_testnet.snap | 3 +- ...arsable_hash_or_height@custom_testnet.snap | 3 +- zebra-rpc/src/methods/tests/vectors.rs | 64 +- zebra-rpc/src/server.rs | 1 + zebra-rpc/src/server/error.rs | 118 +++ .../src/server/rpc_call_compatibility.rs | 20 +- zebra-rpc/src/sync.rs | 6 +- zebra-rpc/src/tests/vectors.rs | 27 +- zebrad/tests/common/regtest.rs | 7 +- 31 files changed, 762 insertions(+), 875 deletions(-) delete mode 100644 zebra-rpc/src/constants.rs delete mode 100644 zebra-rpc/src/methods/errors.rs 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 create mode 100644 zebra-rpc/src/server/error.rs 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]); diff --git a/zebra-rpc/src/constants.rs b/zebra-rpc/src/constants.rs deleted file mode 100644 index 14f89df6618..00000000000 --- a/zebra-rpc/src/constants.rs +++ /dev/null @@ -1,44 +0,0 @@ -//! Constants for RPC methods and server responses. - -use jsonrpc_core::{Error, ErrorCode}; - -/// The RPC error code used by `zcashd` for incorrect RPC parameters. -/// -/// [`jsonrpc_core`] uses these codes: -/// -/// -/// `node-stratum-pool` mining pool library expects error code `-1` to detect available RPC methods: -/// -pub const INVALID_PARAMETERS_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-1); - -/// The RPC error code used by `zcashd` for missing blocks, when looked up -/// by hash. -pub const INVALID_ADDRESS_OR_KEY_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-5); - -/// The RPC error code used by `zcashd` for missing blocks. -/// -/// `lightwalletd` expects error code `-8` when a block is not found: -/// -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. -// -// 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; 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/methods.rs b/zebra-rpc/src/methods.rs index 52f28d606b3..0d7986f033f 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}; @@ -34,21 +34,19 @@ use zebra_chain::{ }, }; use zebra_node_services::mempool; -use zebra_state::{HashOrHeight, MinedTx, OutputIndex, OutputLocation, TransactionLocation}; +use zebra_state::{HashOrHeight, OutputIndex, OutputLocation, TransactionLocation}; use crate::{ - constants::{ - INVALID_ADDRESS_OR_KEY_ERROR_CODE, INVALID_PARAMETERS_ERROR_CODE, MISSING_BLOCK_ERROR_CODE, - }, methods::trees::{GetSubtrees, GetTreestate, SubtreeRpcData}, queue::Queue, + server::{ + self, + error::{MapError, OkOrError}, + }, }; -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; @@ -291,7 +289,7 @@ pub trait Rpc { #[rpc(name = "getrawtransaction")] fn get_raw_transaction( &self, - txid_hex: String, + txid: String, verbose: Option, ) -> BoxFuture>; @@ -564,7 +562,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_misc_error()?; let zebra_state::ReadResponse::TipPoolValues { tip_height, @@ -580,7 +578,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_misc_error()?; let zebra_state::ReadResponse::BlockHeader { header, .. } = response else { unreachable!("unmatched response to a BlockHeader request") @@ -671,7 +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_server_error()?; + let response = state.oneshot(request).await.map_misc_error()?; match response { zebra_state::ReadResponse::AddressBalance(balance) => Ok(AddressBalance { @@ -692,11 +690,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: + // + 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(); @@ -707,7 +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_server_error()?; + let response = mempool.oneshot(request).await.map_misc_error()?; let mut queue_results = match response { mempool::Response::Queued(results) => results, @@ -724,19 +723,30 @@ where .pop() .expect("there should be exactly one item in Vec") .inspect_err(|err| tracing::debug!("sent transaction to mempool: {:?}", &err)) - .map_server_error()? - .await; + .map_misc_error()? + .await + .map_misc_error()?; tracing::debug!("sent transaction to mempool: {:?}", &queue_result); queue_result - .map_server_error()? .map(|_| SentTransactionHash(transaction_hash)) - .map_server_error() + // Reference for the legacy error code: + // + // 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() } + // # Performance + // + // `lightwalletd` calls this RPC with verosity 1 for its initial sync of 2 million blocks, the + // performance 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) @@ -745,11 +755,8 @@ where hash_or_height: String, verbosity: Option, ) -> BoxFuture> { - // From - const DEFAULT_GETBLOCK_VERBOSITY: u8 = 1; - let mut state = self.state.clone(); - let verbosity = verbosity.unwrap_or(DEFAULT_GETBLOCK_VERBOSITY); + let verbosity = verbosity.unwrap_or(1); let network = self.network.clone(); let original_hash_or_height = hash_or_height.clone(); @@ -761,29 +768,26 @@ where }; async move { - let hash_or_height: HashOrHeight = hash_or_height.parse().map_server_error()?; + let hash_or_height: HashOrHeight = hash_or_height + .parse() + // Reference for the legacy error code: + // + .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() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_misc_error()?; match response { zebra_state::ReadResponse::Block(Some(block)) => { Ok(GetBlock::Raw(block.into())) } - zebra_state::ReadResponse::Block(None) => Err(Error { - code: MISSING_BLOCK_ERROR_CODE, - message: "Block not found".to_string(), - data: None, - }), + zebra_state::ReadResponse::Block(None) => Err("Block not found") + .map_error(server::error::LegacyCode::InvalidParameter), _ => unreachable!("unmatched response to a block request"), } } else if let Some(get_block_header_future) = get_block_header_future { @@ -835,9 +839,9 @@ 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_misc_error()? { zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids - .ok_or_server_error("Block not found")? + .ok_or_misc_error("block not found")? .iter() .map(|tx_id| tx_id.encode_hex()) .collect(), @@ -846,7 +850,7 @@ where let orchard_tree_response = futs.next().await.expect("`futs` should not be empty"); let zebra_state::ReadResponse::OrchardTree(orchard_tree) = - orchard_tree_response.map_server_error()? + orchard_tree_response.map_misc_error()? else { unreachable!("unmatched response to a OrchardTree request"); }; @@ -854,8 +858,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_server_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 => { @@ -895,11 +898,8 @@ where next_block_hash, }) } else { - Err(Error { - code: ErrorCode::InvalidParams, - message: "Invalid verbosity value".to_string(), - data: None, - }) + Err("invalid verbosity value") + .map_error(server::error::LegacyCode::InvalidParameter) } } .boxed() @@ -915,7 +915,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, @@ -925,43 +927,42 @@ where .clone() .oneshot(zebra_state::ReadRequest::BlockHeader(hash_or_height)) .await - .map_err(|_| Error { - // Compatibility with zcashd. Note that 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() { - INVALID_ADDRESS_OR_KEY_ERROR_CODE + .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). + if hash_or_height.hash().is_some() { + server::error::LegacyCode::InvalidAddressOrKey } else { - MISSING_BLOCK_ERROR_CODE + server::error::LegacyCode::InvalidParameter }, - message: "block height not in best chain".to_string(), - data: None, - })? + )? 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_misc_error()?)) } else { let zebra_state::ReadResponse::SaplingTree(sapling_tree) = state .clone() .oneshot(zebra_state::ReadRequest::SaplingTree(hash_or_height)) .await - .map_server_error()? + .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_server_error("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_server_error()? + .map_misc_error()? else { panic!("unexpected response to SaplingTree request") }; @@ -1021,14 +1022,14 @@ where self.latest_chain_tip .best_tip_hash() .map(GetBlockHash) - .ok_or_server_error("No blocks in state") + .ok_or_misc_error("No blocks in state") } fn get_best_block_height_and_hash(&self) -> Result { 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_misc_error("No blocks in state") } fn get_raw_mempool(&self) -> BoxFuture>> { @@ -1057,7 +1058,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_misc_error()?; match response { #[cfg(feature = "getblocktemplate-rpcs")] @@ -1104,73 +1105,76 @@ where .boxed() } - // TODO: use HexData or SentTransactionHash to handle the transaction ID fn get_raw_transaction( &self, - txid_hex: String, + txid: String, verbose: Option, ) -> BoxFuture> { 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") - })?; + // Reference for the legacy error code: + // + let txid = transaction::Hash::from_hex(txid) + .map_error(server::error::LegacyCode::InvalidAddressOrKey)?; // 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([txid].into())) + }) .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_misc_error()? + { + mempool::Response::Transactions(txns) => { + if let Some(tx) = txns.first() { + 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 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()?; + .map_misc_error()? + { + 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) + }) + } - match response { - zebra_state::ReadResponse::Transaction(Some(MinedTx { - tx, - height, - confirmations, - })) => Ok(GetRawTransaction::from_transaction( - tx, - Some(height), - confirmations, - verbose, - )), zebra_state::ReadResponse::Transaction(None) => { - Err("Transaction not found").map_server_error() + Err("No such mempool or main chain transaction") + .map_error(server::error::LegacyCode::InvalidAddressOrKey) } - _ => unreachable!("unmatched response to a transaction request"), + + _ => unreachable!("unmatched response to a `Transaction` read request"), } } .boxed() @@ -1184,8 +1188,11 @@ where let network = self.network.clone(); 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()?; + // Reference for the legacy error code: + // + let hash_or_height = hash_or_height + .parse() + .map_error(server::error::LegacyCode::InvalidParameter)?; // Fetch the block referenced by [`hash_or_height`] from the state. // @@ -1199,15 +1206,14 @@ where .ready() .and_then(|service| service.call(zebra_state::ReadRequest::Block(hash_or_height))) .await - .map_server_error()? + .map_misc_error()? { zebra_state::ReadResponse::Block(Some(block)) => block, zebra_state::ReadResponse::Block(None) => { - return Err(Error { - code: MISSING_BLOCK_ERROR_CODE, - message: "the requested block was not found".to_string(), - data: None, - }) + // Reference for the legacy error code: + // + return Err("the requested block is not in the main chain") + .map_error(server::error::LegacyCode::InvalidParameter); } _ => unreachable!("unmatched response to a block request"), }; @@ -1231,7 +1237,7 @@ where service.call(zebra_state::ReadRequest::SaplingTree(hash.into())) }) .await - .map_server_error()? + .map_misc_error()? { zebra_state::ReadResponse::SaplingTree(tree) => tree.map(|t| t.to_rpc_bytes()), _ => unreachable!("unmatched response to a Sapling tree request"), @@ -1248,7 +1254,7 @@ where service.call(zebra_state::ReadRequest::OrchardTree(hash.into())) }) .await - .map_server_error()? + .map_misc_error()? { zebra_state::ReadResponse::OrchardTree(tree) => tree.map(|t| t.to_rpc_bytes()), _ => unreachable!("unmatched response to an Orchard tree request"), @@ -1281,7 +1287,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_misc_error()?; let subtrees = match response { zebra_state::ReadResponse::SaplingSubtrees(subtrees) => subtrees, @@ -1307,7 +1313,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_misc_error()?; let subtrees = match response { zebra_state::ReadResponse::OrchardSubtrees(subtrees) => subtrees, @@ -1329,7 +1335,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, }) @@ -1367,7 +1373,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_misc_error()?; let hashes = match response { zebra_state::ReadResponse::AddressesTransactionIds(hashes) => { @@ -1414,7 +1420,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_misc_error()?; let utxos = match response { zebra_state::ReadResponse::AddressUtxos(utxos) => utxos, _ => unreachable!("unmatched response to a UtxosByAddresses request"), @@ -1492,7 +1498,7 @@ where { latest_chain_tip .best_tip_height() - .ok_or_server_error("No blocks in state") + .ok_or_misc_error("No blocks in state") } /// Response to a `getinfo` RPC request. @@ -1586,13 +1592,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> { + // Reference for the legacy error code: + // let valid_addresses: HashSet
= 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::>()?; @@ -1991,12 +1999,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, + /// 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, }, } @@ -2006,8 +2016,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(), } } } @@ -2070,33 +2080,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_transaction( - tx: Arc, - height: Option, - 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/errors.rs b/zebra-rpc/src/methods/errors.rs deleted file mode 100644 index be9231d058d..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 { - fn map_server_error(self) -> std::result::Result; -} - -pub(crate) trait OkOrServerError { - fn ok_or_server_error( - self, - message: S, - ) -> std::result::Result; -} - -impl MapServerError for Result -where - E: ToString, -{ - fn map_server_error(self) -> Result { - self.map_err(|error| jsonrpc_core::Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - }) - } -} - -impl OkOrServerError for Option { - fn ok_or_server_error(self, message: S) -> Result { - 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 aed926b3635..42c5d282bed 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::MapError}, }; pub mod constants; @@ -584,12 +586,12 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_error(server::error::LegacyCode::default())?; 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, }), @@ -850,7 +852,7 @@ where Is Zebra shutting down?" ); - return Err(recv_error).map_server_error(); + return Err(recv_error).map_error(server::error::LegacyCode::default()); } } } @@ -1042,7 +1044,7 @@ where .ready() .and_then(|service| service.call(request)) .await - .map_server_error()?; + .map_error(server::error::LegacyCode::default())?; current_block_size = match response { zebra_state::ReadResponse::TipBlockSize(Some(block_size)) => Some(block_size), _ => None, @@ -1231,13 +1233,14 @@ 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(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_server_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)); @@ -1274,8 +1277,12 @@ 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(server::error::LegacyCode::default())? + .into(), + lockbox_total: lockbox_total + .map_error(server::error::LegacyCode::default())? + .into(), total_block_subsidy: total_block_subsidy.into(), }) } @@ -1436,7 +1443,10 @@ 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(server::error::LegacyCode::default())?; let get_block_template::Response::TemplateMode(block_template) = block_template else { @@ -1452,14 +1462,17 @@ where TimeSource::CurTime, NetworkUpgrade::current(&network, Height(block_template.height)), ) - .map_server_error()?; - let hex_proposal_block = - HexData(proposal_block.zcash_serialize_to_vec().map_server_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_server_error()?; + .map_error(server::error::LegacyCode::default())?; block_hashes.push(GetBlockHash(proposal_block.hash())); } 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 7ab1a48e20a..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 @@ -25,12 +25,12 @@ use zebra_consensus::{ use zebra_node_services::mempool::{self, TransactionDependencies}; 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::OkOrError, }; pub use crate::methods::get_block_template_rpcs::types::get_block_template::*; @@ -87,13 +87,9 @@ pub fn check_parameters(parameters: &Option) -> Result<()> { pub fn check_miner_address( miner_address: Option, ) -> Result { - 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,7 +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_server_error("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 diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs index 9435a68ac7e..0af5e03b0b9 100644 --- a/zebra-rpc/src/methods/tests/prop.rs +++ b/zebra-rpc/src/methods/tests/prop.rs @@ -1,9 +1,9 @@ //! 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; +use hex::{FromHex, ToHex}; use jsonrpc_core::{Error, ErrorCode}; use proptest::{collection::vec, prelude::*}; use thiserror::Error; @@ -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,6 +25,8 @@ use zebra_state::BoxError; use zebra_test::mock_service::MockService; +use crate::methods; + use super::super::{ AddressBalance, AddressStrings, NetworkUpgradeStatus, Rpc, RpcImpl, SentTransactionHash, }; @@ -35,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::()) { + fn mempool_receives_raw_tx(transaction in any::(), network in any::()) { 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)); @@ -73,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(()) })?; } @@ -91,27 +79,16 @@ proptest! { /// /// Mempool service errors should become server errors. #[test] - fn mempool_errors_are_forwarded(transaction in any::()) { + fn mempool_errors_are_forwarded(transaction in any::(), network in any::()) { 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())); @@ -126,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)); @@ -152,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::()) { + fn rejected_txs_are_reported(transaction in any::(), network in any::()) { 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(()) })?; } @@ -241,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::()) { 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(()) })?; } @@ -296,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::>()) { + fn invalid_tx_results_in_an_error(random_bytes in any::>(), network in any::()) { 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(); @@ -306,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(()) })?; } @@ -353,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::>()) { + fn mempool_transactions_are_sent_to_caller(transactions in any::>(), + network in any::()) { 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) @@ -391,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 @@ -416,151 +271,80 @@ 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::()) + .collect::>(); - 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(()) })?; } - /// Test that the method rejects non-hexadecimal characters. + /// Calls `get_raw_transaction` with: /// - /// 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. + /// 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 get_raw_transaction_non_hexadecimal_string_results_in_an_error( - non_hex_string in ".*[^0-9A-Fa-f].*", - ) { + 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); // 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, - ); + // Check the invalid TXID first. + let rpc_rsp = rpc.get_raw_transaction(invalid_txid, Some(1)).await; - let send_task = tokio::spawn(rpc.get_raw_transaction(non_hex_string, Some(0))); + 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?; - 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()); + // Now check the unknown TXID. + let mempool_query = mempool + .expect_request(mempool::Request::TransactionsByMinedId([unknown_txid].into())) + .map_ok(|r| r.respond(mempool::Response::Transactions(vec![]))); - 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. - #[test] - fn get_raw_transaction_invalid_transaction_results_in_an_error( - random_bytes in any::>(), - ) { - 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(); - - 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 state_query = state + .expect_request(zebra_state::ReadRequest::Transaction(unknown_txid)) + .map_ok(|r| r.respond(zebra_state::ReadResponse::Transaction(None))); - let send_task = tokio::spawn(rpc.get_raw_transaction(hex::encode(random_bytes), Some(0))); + let rpc_query = rpc.get_raw_transaction(unknown_txid.encode_hex(), Some(1)); - mempool.expect_no_requests().await?; - state.expect_no_requests().await?; + let (rpc_rsp, _, _) = tokio::join!(rpc_query, mempool_query, state_query); - 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(rpc_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(()) })?; } @@ -569,21 +353,10 @@ proptest! { fn get_blockchain_info_response_without_a_chain_tip(network in any::()) { 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(); @@ -605,14 +378,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(()) })?; } @@ -624,26 +396,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(); @@ -718,14 +481,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(()) })?; } @@ -738,12 +500,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 { @@ -753,21 +514,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); @@ -792,7 +540,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(()) })?; } @@ -806,31 +557,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::().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, @@ -839,55 +576,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::()) { + fn rpc_queue_main_loop(tx in any::(), network in any::()) { 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)); @@ -901,9 +615,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); @@ -946,42 +658,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::()) { 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)); @@ -999,9 +697,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 @@ -1049,10 +745,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(()) })?; } } @@ -1060,3 +755,83 @@ proptest! { #[derive(Clone, Copy, Debug, Error)] #[error("a dummy error type")] pub struct DummyError; + +// Helper functions + +/// Creates [`String`]s that won't deserialize into [`transaction::Hash`]. +fn invalid_txid() -> BoxedStrategy { + any::() + .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(rsp: Result, 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(()) +} + +/// Creates mocked: +/// +/// 1. mempool service, +/// 2. state service, +/// 3. rpc service, +/// +/// and a handle to the mempool tx queue. +fn mock_services( + 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) +} diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs index 02c633af260..2bdec5d7497 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; @@ -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` @@ -407,8 +405,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 the state. let mempool_req = mempool .expect_request_that(|request| { matches!(request, mempool::Request::TransactionsByMinedId(_)) @@ -417,13 +416,12 @@ async fn test_rpc_response_data_for_network(network: &Network) { responder.respond(mempool::Response::Transactions(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_transaction = response.expect("We should have a GetRawTransaction struct"); + let txid = first_block_first_tx.hash().encode_hex::(); - 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 @@ -434,13 +432,31 @@ async fn test_rpc_response_data_for_network(network: &Network) { responder.respond(mempool::Response::Transactions(vec![])); }); - // 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_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; + + // `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![])); + }); - snapshot_rpc_getrawtransaction("verbosity_1", get_raw_transaction, &settings); + 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; + + // `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 @@ -666,17 +682,6 @@ fn snapshot_rpc_getrawmempool(raw_mempool: Vec, 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..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/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 + } +} 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" } } diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 007a89ee893..998cdc155ee 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; @@ -722,9 +724,12 @@ 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 get_tx = response.expect("We should have a GetRawTransaction struct"); + + 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 `GetRawTransaction` struct"); + if let GetRawTransaction::Raw(raw_tx) = get_tx { assert_eq!(raw_tx.as_ref(), tx.zcash_serialize_to_vec().unwrap()); } else { @@ -752,12 +757,14 @@ async fn rpc_getrawtransaction() { let run_state_test_case = |block_idx: usize, block: Arc, tx: Arc| { 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 = txid.encode_hex::(); + + 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()); @@ -765,7 +772,8 @@ 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, @@ -775,8 +783,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())) @@ -870,25 +881,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(); @@ -1078,17 +1082,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; 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..4cfc7b38571 --- /dev/null +++ b/zebra-rpc/src/server/error.rs @@ -0,0 +1,118 @@ +//! RPC error codes & their handling. + +/// Bitcoin RPC error codes +/// +/// Drawn from . +/// +/// ## 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, + /// 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 for jsonrpc_core::ErrorCode { + fn from(code: LegacyCode) -> Self { + Self::ServerError(code as i64) + } +} + +/// A trait for mapping errors to [`jsonrpc_core::Error`]. +pub(crate) trait MapError: Sized { + /// Maps errors to [`jsonrpc_core::Error`] with a specific error code. + fn map_error( + self, + code: impl Into, + ) -> std::result::Result; + + /// Maps errors to [`jsonrpc_core::Error`] with a [`LegacyCode::Misc`] error code. + fn map_misc_error(self) -> std::result::Result { + self.map_error(LegacyCode::Misc) + } +} + +/// A trait for conditionally converting a value into a `Result`. +pub(crate) trait OkOrError: Sized { + /// Converts the implementing type to `Result`, using an error code and + /// message if conversion is to `Err`. + fn ok_or_error( + self, + code: impl Into, + message: impl ToString, + ) -> std::result::Result; + + /// Converts the implementing type to `Result`, using a [`LegacyCode::Misc`] error code. + fn ok_or_misc_error( + self, + message: impl ToString, + ) -> std::result::Result { + self.ok_or_error(LegacyCode::Misc, message) + } +} + +impl MapError for Result +where + E: ToString, +{ + fn map_error(self, code: impl Into) -> Result { + self.map_err(|error| jsonrpc_core::Error { + code: code.into(), + message: error.to_string(), + data: None, + }) + } +} + +impl OkOrError for Option { + fn ok_or_error( + self, + code: impl Into, + message: impl ToString, + ) -> Result { + self.ok_or(jsonrpc_core::Error { + code: code.into(), + 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..209596180c0 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::server; /// JSON-RPC [`Middleware`] with compatibility workarounds. /// @@ -57,13 +58,20 @@ impl Middleware 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: + /// fn fix_error_codes(output: &mut Option) { 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}"); } } @@ -73,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:?}"); diff --git a/zebra-rpc/src/sync.rs b/zebra-rpc/src/sync.rs index fd323ef64bb..40373d0eaed 100644 --- a/zebra-rpc/src/sync.rs +++ b/zebra-rpc/src/sync.rs @@ -21,8 +21,8 @@ use zebra_state::{ use zebra_chain::diagnostic::task::WaitForPanics; use crate::{ - constants::MISSING_BLOCK_ERROR_CODE, methods::{hex_data::HexData, GetBlockHeightAndHash}, + server, }; /// How long to wait between calls to `getbestblockheightandhash` when it: @@ -383,7 +383,9 @@ impl SyncerRpcMethods for RpcRequestClient { Err(err) if err .downcast_ref::() - .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/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] diff --git a/zebrad/tests/common/regtest.rs b/zebrad/tests/common/regtest.rs index bf1cba697de..acd89d89aba 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,9 @@ impl MiningRpcMethods for RpcRequestClient { Err(err) if err .downcast_ref::() - .is_some_and(|err| err.code == MISSING_BLOCK_ERROR_CODE) => + .is_some_and(|err| { + err.code == server::error::LegacyCode::InvalidParameter.into() + }) => { Ok(None) }