From bb8e647990ebcf6a5966b62a4470403d165a80e3 Mon Sep 17 00:00:00 2001 From: zancas Date: Thu, 16 Jul 2026 11:59:11 -0700 Subject: [PATCH 1/2] refactor: statically type the UnexpectedErrorResponse payload as RpcError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RpcRequestError::UnexpectedErrorResponse` carried its payload as `Box`, but the box guarded flexibility nothing used: every `TryFrom` impl in the tree — the `impl_rpc_error_passthrough!` macro and all four hand-written impls — fixes `Error = RpcError`, so the boxed value was always concretely an `RpcError`. This commit pins that fact in the type system. The `ResponseToError` trait now bounds its associated type with `TryFrom`, and the variant becomes `UnexpectedErrorResponse(#[source] RpcError)`. The payload needs no allocation and no downcast: consumers can match it directly, while the existing `source()`-chain recovery walks keep working unchanged. Unboxing pushed `RpcRequestError` past clippy's `result_large_err` threshold. The weight was `RpcError.data: Option`, a field nothing reads (only the two `RpcError` constructors touch it), so it is now `Option>` — a concrete, statically typed box. That shrinks every `RpcError` from roughly 136 to 40 bytes, which silences the new warning, retires the pre-existing `#[allow(clippy::result_large_err)]` on `rpc_error_code_map`, and clears the three long-standing large-`Err` clippy warnings in the e2e tests that traced back to `RpcError`'s size. Part of the zingolabs/zaino#1404 follow-up series (zingolabs/zaino#1406, zingolabs/zaino#1408). Co-Authored-By: Claude Fable 5 --- .../zaino-fetch/src/jsonrpsee/connector.rs | 35 ++++++++++++------- .../zaino-fetch/src/jsonrpsee/response.rs | 4 --- .../chain_index/source/mockchain_source.rs | 4 +-- packages/zaino-state/src/error.rs | 2 +- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/packages/zaino-fetch/src/jsonrpsee/connector.rs b/packages/zaino-fetch/src/jsonrpsee/connector.rs index 5c2c4e8d5..a5e2d1512 100644 --- a/packages/zaino-fetch/src/jsonrpsee/connector.rs +++ b/packages/zaino-fetch/src/jsonrpsee/connector.rs @@ -78,7 +78,12 @@ pub struct RpcError { /// Error Message. pub message: String, /// Error Data. - pub data: Option, + /// + /// Boxed to keep `RpcError` small: it rides in the `Err` variant of every + /// RPC-call `Result` (see [`RpcRequestError::UnexpectedErrorResponse`] and + /// the `TryFrom` impls), where a large payload bloats each such + /// `Result` even on the success path. + pub data: Option>, } impl RpcError { @@ -148,8 +153,13 @@ enum AuthMethod { /// Trait to convert a JSON-RPC response to an error. pub trait ResponseToError: Sized { /// The error type. - type RpcError: std::fmt::Debug - + TryFrom; + /// + /// The conversion error is pinned to [`RpcError`] (rather than any + /// `std::error::Error`) so an unmapped server rejection stays a statically + /// typed [`RpcError`] all the way into + /// [`RpcRequestError::UnexpectedErrorResponse`] — no boxing, and consumers + /// can match the payload directly instead of downcasting. + type RpcError: std::fmt::Debug + TryFrom; /// Converts a JSON-RPC response to an error. fn to_error(self) -> Result { @@ -190,13 +200,15 @@ pub enum RpcRequestError { /// Zaino has not yet accounted for the possibilty of this error, /// or the Node returned an undocumented/malformed error response. /// - /// The payload is `#[source]` so the node's typed rejection (usually an - /// [`RpcError`]) stays reachable through `source()` chains: the serving + /// The payload is the node's rejection as a statically typed + /// [`RpcError`] (guaranteed by [`ResponseToError`]'s + /// `TryFrom` bound), and it is `#[source]` so + /// the rejection stays reachable through `source()` chains: the serving /// surfaces recover rejection codes and messages by downcast-walking those /// chains, and severing the link here masks every unmapped node rejection /// as a generic internal error (zingolabs/zaino#1404). #[error("unexpected error response from server: {0}")] - UnexpectedErrorResponse(#[source] Box), + UnexpectedErrorResponse(#[source] RpcError), } /// JsonRpSee Client config data. @@ -424,9 +436,8 @@ impl JsonRpSeeConnector { match response.error { Some(error) => Err(RpcRequestError::Method( - R::RpcError::try_from(error).map_err(|e| { - RpcRequestError::UnexpectedErrorResponse(Box::new(e)) - })?, + R::RpcError::try_from(error) + .map_err(RpcRequestError::UnexpectedErrorResponse)?, )), None => { let result: R = @@ -1087,7 +1098,7 @@ mod tests { /// `zaino-serve/src/rpc/jsonrpc/service.rs`). `UnexpectedErrorResponse` is /// the variant that carries a node's structured rejection whenever no /// method-specific mapping exists (every `impl_rpc_error_passthrough!` - /// type, including `sendrawtransaction`'s), so its boxed payload must stay + /// type, including `sendrawtransaction`'s), so its payload must stay /// reachable through `source()`. Without the link, every such rejection is /// masked as a generic internal error (zingolabs/zaino#1404). #[test] @@ -1099,10 +1110,10 @@ mod tests { "failed to validate tx: transparent input not found", ); let error: RpcRequestError = - RpcRequestError::UnexpectedErrorResponse(Box::new(rejection)); + RpcRequestError::UnexpectedErrorResponse(rejection); let source = std::error::Error::source(&error) - .expect("the boxed node rejection must be exposed via source()"); + .expect("the node rejection must be exposed via source()"); let rpc_error = source .downcast_ref::() .expect("the source must downcast to the typed RpcError"); diff --git a/packages/zaino-fetch/src/jsonrpsee/response.rs b/packages/zaino-fetch/src/jsonrpsee/response.rs index d88722ec4..fd590c642 100644 --- a/packages/zaino-fetch/src/jsonrpsee/response.rs +++ b/packages/zaino-fetch/src/jsonrpsee/response.rs @@ -88,10 +88,6 @@ impl_infallible_response_to_error!( /// Maps a JSON-RPC error carrying `code` to the RPC-specific variant `ctor` /// builds from its message, passing every other error through unchanged. -// The Err type is fixed by the `TryFrom` impls this serves: their -// `Error = RpcError` (unboxed) is the trait contract, so the large-Err lint -// cannot be satisfied here without changing every impl's signature. -#[allow(clippy::result_large_err)] pub(crate) fn rpc_error_code_map( value: RpcError, code: i64, diff --git a/packages/zaino-state/src/chain_index/source/mockchain_source.rs b/packages/zaino-state/src/chain_index/source/mockchain_source.rs index 76045b9ad..c013def2e 100644 --- a/packages/zaino-state/src/chain_index/source/mockchain_source.rs +++ b/packages/zaino-state/src/chain_index/source/mockchain_source.rs @@ -815,12 +815,12 @@ impl BlockchainSource for MockchainSource { Err(BlockchainSourceError::unrecoverable( zaino_fetch::jsonrpsee::connector::RpcRequestError::< zaino_fetch::jsonrpsee::response::SendTransactionError, - >::UnexpectedErrorResponse(Box::new( + >::UnexpectedErrorResponse( zaino_fetch::jsonrpsee::connector::RpcError::new_from_legacycode( zebra_rpc::server::error::LegacyCode::Verify, "MockchainSource rejects all transaction submissions (static chain, no mempool)", ), - )), + ), )) } diff --git a/packages/zaino-state/src/error.rs b/packages/zaino-state/src/error.rs index ce14008a6..a3d877c0c 100644 --- a/packages/zaino-state/src/error.rs +++ b/packages/zaino-state/src/error.rs @@ -724,7 +724,7 @@ mod tonic_status_from_service_error { "failed to validate tx: transparent input not found", ); let request_error: RpcRequestError = - RpcRequestError::UnexpectedErrorResponse(Box::new(rejection)); + RpcRequestError::UnexpectedErrorResponse(rejection); let source_error = BlockchainSourceError::unrecoverable(request_error); let index_error = ChainIndexError::backing_validator(source_error); From a1a2f757b6541629560dc5938c0f4cc5c81d0393 Mon Sep 17 00:00:00 2001 From: zancas Date: Thu, 16 Jul 2026 11:59:49 -0700 Subject: [PATCH 2/2] test: enforce the UnexpectedErrorResponse contract for every passthrough type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix for zingolabs/zaino#1404 restored the `source()` link on `RpcRequestError::UnexpectedErrorResponse`, healing rejection attribution for every `impl_rpc_error_passthrough!` type at once — but only `SendTransactionError` had a regression test pinning the invariant. Issue zingolabs/zaino#1406 asks for the contract to be enforced per passthrough type so a future reshaping of the variant cannot silently reintroduce the masking for the other methods. The single-type test in zaino-fetch is generalized into a plain generic helper, `assert_unexpected_error_response_exposes_rpc_error`, whose panic messages name the failing type. The new contract test invokes it for all ten passthrough types: the eight listed in `response.rs` plus `ZValidateAddressError` and `GetAddressDeltasError` from the submodule invocations (the issue's count of eight missed those two). The `impl_rpc_error_passthrough!` doc comment now directs contributors to add new passthrough types to the test. Deliberately severing the `#[source]` link was confirmed to turn the test red with a message naming the first affected type, so the test demonstrably catches the regression it guards against. Closes zingolabs/zaino#1406. Co-Authored-By: Claude Fable 5 --- .../zaino-fetch/src/jsonrpsee/connector.rs | 75 ++++++++++++++----- .../zaino-fetch/src/jsonrpsee/response.rs | 5 ++ 2 files changed, 60 insertions(+), 20 deletions(-) diff --git a/packages/zaino-fetch/src/jsonrpsee/connector.rs b/packages/zaino-fetch/src/jsonrpsee/connector.rs index a5e2d1512..5951be357 100644 --- a/packages/zaino-fetch/src/jsonrpsee/connector.rs +++ b/packages/zaino-fetch/src/jsonrpsee/connector.rs @@ -1092,37 +1092,72 @@ pub async fn test_node_and_return_url( mod tests { use super::*; - /// zaino-serve attributes node rejections by downcast-walking `source()` - /// chains for the typed [`RpcError`] (see - /// `sendrawtransaction_error_object_from_indexer_error` in - /// `zaino-serve/src/rpc/jsonrpc/service.rs`). `UnexpectedErrorResponse` is - /// the variant that carries a node's structured rejection whenever no - /// method-specific mapping exists (every `impl_rpc_error_passthrough!` - /// type, including `sendrawtransaction`'s), so its payload must stay - /// reachable through `source()`. Without the link, every such rejection is - /// masked as a generic internal error (zingolabs/zaino#1404). - #[test] - fn unexpected_error_response_exposes_rpc_error_via_source() { - use crate::jsonrpsee::response::SendTransactionError; - + /// Asserts the `UnexpectedErrorResponse` source-reachability contract for + /// one `MethodError` instantiation: the node rejection must be exposed via + /// `source()` and downcast to the typed [`RpcError`] with its legacy code + /// intact. + fn assert_unexpected_error_response_exposes_rpc_error() + where + MethodError: std::error::Error + 'static, + { + let type_name = std::any::type_name::(); let rejection = RpcError::new_from_legacycode( zebra_rpc::server::error::LegacyCode::Verify, "failed to validate tx: transparent input not found", ); - let error: RpcRequestError = + let error: RpcRequestError = RpcRequestError::UnexpectedErrorResponse(rejection); - let source = std::error::Error::source(&error) - .expect("the node rejection must be exposed via source()"); - let rpc_error = source - .downcast_ref::() - .expect("the source must downcast to the typed RpcError"); + let source = std::error::Error::source(&error).unwrap_or_else(|| { + panic!( + "RpcRequestError::<{type_name}>::UnexpectedErrorResponse \ + must expose the node rejection via source()" + ) + }); + let rpc_error = source.downcast_ref::().unwrap_or_else(|| { + panic!( + "the source for RpcRequestError::<{type_name}> must downcast to the typed RpcError" + ) + }); assert_eq!( rpc_error.code, - zebra_rpc::server::error::LegacyCode::Verify as i64 + zebra_rpc::server::error::LegacyCode::Verify as i64, + "the legacy error code must survive for RpcRequestError::<{type_name}>" ); } + /// zaino-serve attributes node rejections by downcast-walking `source()` + /// chains for the typed [`RpcError`] (see + /// `sendrawtransaction_error_object_from_indexer_error` in + /// `zaino-serve/src/rpc/jsonrpc/service.rs`). `UnexpectedErrorResponse` is + /// the variant that carries a node's structured rejection whenever no + /// method-specific mapping exists — every `impl_rpc_error_passthrough!` + /// type — so its boxed payload must stay reachable through `source()` for + /// each of them. Without the link, every such rejection is masked as a + /// generic internal error (zingolabs/zaino#1404, zingolabs/zaino#1406). + /// + /// When a type is added to an `impl_rpc_error_passthrough!` invocation, it + /// belongs in this list too. + #[test] + fn unexpected_error_response_exposes_rpc_error_for_every_passthrough_type() { + use crate::jsonrpsee::response::{ + address_deltas::GetAddressDeltasError, z_validate_address::ZValidateAddressError, + ChainWorkError, GetBalanceError, GetSubtreesError, GetTreestateError, GetUtxosError, + SendTransactionError, TxidsError, + }; + + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + assert_unexpected_error_response_exposes_rpc_error::(); + } + /// `Method` carries the typed method-specific rejection, so — like /// `UnexpectedErrorResponse` — its payload must stay reachable through /// `source()` for the serving surfaces' downcast walks to attribute the diff --git a/packages/zaino-fetch/src/jsonrpsee/response.rs b/packages/zaino-fetch/src/jsonrpsee/response.rs index fd590c642..4b2d9305d 100644 --- a/packages/zaino-fetch/src/jsonrpsee/response.rs +++ b/packages/zaino-fetch/src/jsonrpsee/response.rs @@ -36,6 +36,11 @@ use super::connector::RpcError; /// Implements the pass-through `TryFrom` for RPC-specific error /// types that map no JSON-RPC error codes to variants of their own: every /// server-reported error surfaces unchanged as the generic [`RpcError`]. +/// +/// Every type listed in an invocation also belongs in the contract test +/// `unexpected_error_response_exposes_rpc_error_for_every_passthrough_type` +/// in `connector.rs`, which enforces that its rejections stay attributable +/// via `source()` chains (zingolabs/zaino#1406). // TODO: attempt to convert RpcError into errors specific to each RPC // response, removing types from these invocations as mappings become known. macro_rules! impl_rpc_error_passthrough {