diff --git a/packages/zaino-serve/src/rpc/jsonrpc/service.rs b/packages/zaino-serve/src/rpc/jsonrpc/service.rs index 389e5cf11..39b5a7ec9 100644 --- a/packages/zaino-serve/src/rpc/jsonrpc/service.rs +++ b/packages/zaino-serve/src/rpc/jsonrpc/service.rs @@ -476,9 +476,9 @@ pub trait ZcashIndexerRpc { ) -> Result; } -/// Maps an indexer error to the JSON-RPC error object every plain-delegation -/// method returns: `InvalidParams` (converted to the zcash legacy "misc" code -/// in RPC middleware) carrying the error's message as data. +/// The fallback wire object for indexer errors with no typed node rejection: +/// `InvalidParams` (converted to the zcash legacy "misc" code in RPC +/// middleware) carrying the error's message as data. fn invalid_params_error_object(error: impl std::fmt::Display) -> ErrorObjectOwned { ErrorObjectOwned::owned( ErrorCode::InvalidParams.code(), @@ -487,9 +487,21 @@ fn invalid_params_error_object(error: impl std::fmt::Display) -> ErrorObjectOwne ) } -// Currently all errors are hidden from downstream client, a full fix should be implemented. this is a temporary fix to -// get zaino working, propagating a 500 error code to "block not found". This is still not the full correct behaviour -// but fixes the current bugs in zaino and gets tests running. +/// The fallback wire object `sendrawtransaction` and `getblock` use for +/// errors with no typed node rejection: `InternalError` carrying the error's +/// message as data. +fn internal_error_object(error: impl std::fmt::Display) -> ErrorObjectOwned { + ErrorObjectOwned::owned( + ErrorCode::InternalError.code(), + "Internal server error", + Some(error.to_string()), + ) +} + +/// The node's typed rejection, when `error_source` is one: the payload the +/// per-method translators below forward to the wire so clients see the +/// node's rejection code and reason instead of a generic internal error +/// (zingolabs/zaino#1404, zingolabs/zaino#1407). fn rpc_error_from_error_source<'a>( error_source: &'a (dyn std::error::Error + 'static), ) -> Option<&'a zaino_fetch::jsonrpsee::connector::RpcError> { @@ -503,13 +515,14 @@ fn error_object_from_rpc_error( } /// Walks `error`'s `source` chain and returns the first error object `map` -/// produces, falling back to a generic internal-server-error object carrying -/// the original error's message. The shared skeleton of the per-method error -/// translators below, which differ only in the mapping they apply to each -/// source. +/// produces, handing the whole error to `fallback` when no source matches. +/// The shared skeleton of the per-method error translators below, which +/// differ only in the mapping they apply to each source and in the fallback +/// object they return. fn error_object_from_source_chain( error: Error, map: impl Fn(&(dyn std::error::Error + 'static)) -> Option, + fallback: impl FnOnce(Error) -> ErrorObjectOwned, ) -> ErrorObjectOwned where Error: std::error::Error + 'static, @@ -519,45 +532,61 @@ where |error_source| error_source.source(), ) .find_map(map) - .unwrap_or_else(|| { - ErrorObjectOwned::owned( - ErrorCode::InternalError.code(), - "Internal server error", - Some(error.to_string()), - ) - }) + .unwrap_or_else(|| fallback(error)) +} + +/// The translator every plain-delegation method uses: recovers the node's +/// typed rejection from the `source()` chain when one is present, and falls +/// back to the generic invalid-params object otherwise, so a node rejection +/// of any method reaches the client with its code and reason instead of a +/// masked generic error (zingolabs/zaino#1407). +fn error_object_from_indexer_error(error: Error) -> ErrorObjectOwned +where + Error: std::error::Error + 'static, +{ + error_object_from_source_chain( + error, + |error_source| rpc_error_from_error_source(error_source).map(error_object_from_rpc_error), + invalid_params_error_object, + ) } fn getblock_error_object_from_indexer_error(error: Error) -> ErrorObjectOwned where Error: std::error::Error + 'static, { - error_object_from_source_chain(error, |error_source| { - if let Some(zaino_fetch::jsonrpsee::error::TransportError::ErrorStatusCode(500)) = - error_source.downcast_ref::() - { - Some(ErrorObjectOwned::owned( - zaino_fetch::jsonrpsee::connector::RpcError::new_from_legacycode( - zebra_rpc::server::error::LegacyCode::InvalidParameter, + error_object_from_source_chain( + error, + |error_source| { + if let Some(zaino_fetch::jsonrpsee::error::TransportError::ErrorStatusCode(500)) = + error_source.downcast_ref::() + { + Some(ErrorObjectOwned::owned( + zaino_fetch::jsonrpsee::connector::RpcError::new_from_legacycode( + zebra_rpc::server::error::LegacyCode::InvalidParameter, + "block not found", + ) + .code as i32, "block not found", - ) - .code as i32, - "block not found", - None::<()>, - )) - } else { - None - } - }) + None::<()>, + )) + } else { + rpc_error_from_error_source(error_source).map(error_object_from_rpc_error) + } + }, + internal_error_object, + ) } fn sendrawtransaction_error_object_from_indexer_error(error: Error) -> ErrorObjectOwned where Error: std::error::Error + 'static, { - error_object_from_source_chain(error, |error_source| { - rpc_error_from_error_source(error_source).map(error_object_from_rpc_error) - }) + error_object_from_source_chain( + error, + |error_source| rpc_error_from_error_source(error_source).map(error_object_from_rpc_error), + internal_error_object, + ) } /// Uses ErrorCode::InvalidParams as this is converted to zcash legacy "minsc" ErrorCode in RPC middleware. @@ -568,7 +597,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_info() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_mining_info(&self) -> Result { @@ -576,7 +605,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_mining_info() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_tx_out_set_info(&self) -> Result { @@ -584,7 +613,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_tx_out_set_info() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_best_blockhash(&self) -> Result { @@ -592,7 +621,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_best_blockhash() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_blockchain_info(&self) -> Result { @@ -600,7 +629,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_blockchain_info() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_mempool_info(&self) -> Result { @@ -608,7 +637,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_mempool_info() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_difficulty(&self) -> Result { @@ -616,7 +645,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_difficulty() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_block_deltas(&self, hash: String) -> Result { @@ -624,7 +653,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_block_deltas(hash) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_peer_info(&self) -> Result { @@ -632,7 +661,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_peer_info() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_block_subsidy(&self, height: u32) -> Result { @@ -640,7 +669,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_block_subsidy(height) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_block_count(&self) -> Result { @@ -648,7 +677,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_block_count() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_chain_tips(&self) -> Result { @@ -656,7 +685,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_chain_tips() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn validate_address( @@ -667,7 +696,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .validate_address(address) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } #[allow(deprecated)] @@ -680,7 +709,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .z_validate_address(address) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn z_get_address_balance( @@ -691,7 +720,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .z_get_address_balance(address_strings) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn send_raw_transaction( @@ -726,7 +755,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_block_header(hash, verbose) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_raw_mempool(&self) -> Result, ErrorObjectOwned> { @@ -734,7 +763,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_raw_mempool() .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn z_get_treestate( @@ -745,7 +774,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .z_get_treestate(hash_or_height) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn z_get_subtrees_by_index( @@ -758,7 +787,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .z_get_subtrees_by_index(pool, start_index, limit) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_raw_transaction( @@ -770,7 +799,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_raw_transaction(txid_hex, verbose) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_tx_out( @@ -783,7 +812,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_tx_out(txid, n, include_mempool) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_spent_info( @@ -794,7 +823,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_spent_info(request) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_address_tx_ids( @@ -805,7 +834,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_address_tx_ids(request) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn z_get_address_utxos( @@ -816,7 +845,7 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .z_get_address_utxos(address_strings) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) } async fn get_network_sol_ps( @@ -828,6 +857,158 @@ impl ZcashIndexerRpcServer for JsonR .inner_ref() .get_network_sol_ps(blocks, height) .await - .map_err(invalid_params_error_object) + .map_err(error_object_from_indexer_error) + } +} + +/// Error chains standing in for indexer errors in the translator tests below: +/// the translators only consume `source()` chains, so a minimal typed wrapper +/// reproduces exactly what they see from production error types. +#[cfg(test)] +mod chain_fixtures { + use zaino_fetch::jsonrpsee::connector::RpcError; + + /// A wrapper exposing its payload via `source()`, nestable to build + /// chains of any depth. + #[derive(Debug, thiserror::Error)] + #[error("wrapper: {0}")] + pub(super) struct Wrap(#[source] pub(super) E); + + /// The node rejection used across the translator tests: legacy code `-25` + /// (`Verify`) with a recognizable message. + pub(super) fn node_rejection() -> RpcError { + RpcError::new_from_legacycode( + zebra_rpc::server::error::LegacyCode::Verify, + "failed to validate tx: transparent input not found", + ) + } +} + +#[cfg(test)] +mod error_object_from_indexer_error { + use super::chain_fixtures::{node_rejection, Wrap}; + use super::*; + + /// A node rejection of any plain-delegation method must reach the wire + /// object with its legacy code and message (zingolabs/zaino#1407). + #[test] + fn recovers_rpc_error_code_and_message() { + let error_object = error_object_from_indexer_error(Wrap(Wrap(node_rejection()))); + + assert_eq!( + error_object.code(), + zebra_rpc::server::error::LegacyCode::Verify as i32 + ); + assert_eq!( + error_object.message(), + "failed to validate tx: transparent input not found" + ); + } + + /// Pins the fallback for chains without a typed rejection: the same + /// invalid-params object the plain-delegation methods returned before + /// recovery was added, so non-rejection errors keep their wire shape. + #[test] + fn falls_back_to_invalid_params_without_rpc_error() { + let opaque = Wrap(std::io::Error::other("connection reset by peer")); + + let error_object = error_object_from_indexer_error(opaque); + + assert_eq!(error_object.code(), ErrorCode::InvalidParams.code()); + assert_eq!(error_object.message(), "Internal server error"); + let data = error_object + .data() + .expect("the fallback must carry the error's message as data") + .get() + .to_string(); + assert!( + data.contains("connection reset by peer"), + "fallback data must carry the original message, got: {data}" + ); + } +} + +#[cfg(test)] +mod sendrawtransaction_error_object_from_indexer_error { + use super::chain_fixtures::{node_rejection, Wrap}; + use super::*; + + /// Pins the recovery behavior the #1404 fix restored: the node's typed + /// rejection, however deeply nested, reaches the wire object with its + /// legacy code and message. + #[test] + fn recovers_rpc_error_code_and_message() { + let error_object = + sendrawtransaction_error_object_from_indexer_error(Wrap(Wrap(node_rejection()))); + + assert_eq!( + error_object.code(), + zebra_rpc::server::error::LegacyCode::Verify as i32 + ); + assert_eq!( + error_object.message(), + "failed to validate tx: transparent input not found" + ); + } + + /// Pins the fallback for chains without a typed rejection (e.g. a genuine + /// transport failure): a generic internal-server-error object carrying the + /// error's message as data. + #[test] + fn falls_back_to_internal_error_without_rpc_error() { + let opaque = Wrap(std::io::Error::other("connection reset by peer")); + + let error_object = sendrawtransaction_error_object_from_indexer_error(opaque); + + assert_eq!(error_object.code(), ErrorCode::InternalError.code()); + assert_eq!(error_object.message(), "Internal server error"); + let data = error_object + .data() + .expect("the fallback must carry the error's message as data") + .get() + .to_string(); + assert!( + data.contains("connection reset by peer"), + "fallback data must carry the original message, got: {data}" + ); + } +} + +#[cfg(test)] +mod getblock_error_object_from_indexer_error { + use super::chain_fixtures::{node_rejection, Wrap}; + use super::*; + use zaino_fetch::jsonrpsee::error::TransportError; + + /// Pins the method-specific arm: an HTTP 500 from the backing node maps + /// to the zcashd-compatible "block not found" object. + #[test] + fn maps_transport_500_to_block_not_found() { + let error_object = + getblock_error_object_from_indexer_error(Wrap(TransportError::ErrorStatusCode(500))); + + assert_eq!( + error_object.code(), + zebra_rpc::server::error::LegacyCode::InvalidParameter as i32 + ); + assert_eq!(error_object.message(), "block not found"); + } + + /// A node rejection of `getblock` must reach the wire object with its + /// legacy code and message, like `sendrawtransaction`'s does — the + /// method-specific "block not found" arm runs ahead of this generic + /// recovery, not instead of it (zingolabs/zaino#1407). + #[test] + fn recovers_rpc_error_code_and_message() { + let error_object = getblock_error_object_from_indexer_error(Wrap(node_rejection())); + + assert_eq!( + error_object.code(), + zebra_rpc::server::error::LegacyCode::Verify as i32 + ); + assert_eq!( + error_object.message(), + "failed to validate tx: transparent input not found" + ); } }