You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The #1404 masking family was fixed in four increments: PR #1405 healed the severed source() link on UnexpectedErrorResponse, PR #1409 healed RpcRequestError::Method and the three StatusError wrappers (#1408), PR #1410 made the UnexpectedErrorResponse payload a statically typed RpcError and added per-passthrough-type contract tests (#1406), and PR #1411 gave all 28 JSON-RPC methods the recovery walk (#1407).
All of that work, however, preserves the existing architecture: rejection attribution happens by walking source() chains at runtime and downcasting each link to RpcError. Error::source() is a type-erasing API, so whether a rejection is reachable is a runtime property the compiler cannot check — which is exactly why the #1406 contract tests had to exist, and why the #1404 bug class (a wrapper silently severing the chain) was representable in the first place.
Remaining type erasure
Two wrappers still erase the concrete cause on the path from a node rejection to the serving surfaces:
ChainIndexError { kind, message, source: Option<Box<dyn Error + Send + Sync>> } in packages/zaino-state/src/error.rs — a stringly kind + message struct rather than a typed enum.
Correspondingly, both serving surfaces recover by downcast: rpc_error_in_source_chain feeding From<NodeBackedIndexerServiceError> for tonic::Status (gRPC), and rpc_error_from_error_source inside error_object_from_source_chain (JSON-RPC).
Proposal: typed variants and exhaustive matching, no walks
Preserve the concrete error type through each layer as enum variants, and replace the recovery walks with pattern matches:
BlockchainSourceError gains typed variants — for example NodeRejection(RpcError) and Transport(TransportError) — populated by a From<RpcRequestError<M>> impl that classifies at the boundary. PR Statically typed UnexpectedErrorResponse payload + per-passthrough-type contract tests #1410's Tier 1 change makes this tractable: an unmapped rejection is already a statically typed RpcError when it leaves zaino-fetch.
ChainIndexError becomes an enum with typed variants, for example BackingValidator(BlockchainSourceError), in place of the kind + message + boxed source struct.
The conversions — From<NodeBackedIndexerServiceError> for tonic::Status and the per-method JSON-RPC translators — become exhaustive matches on those variants. The compiler then proves a node rejection reaches the wire; a new variant that swallows a rejection fails to compile rather than failing a contract test.
Open design question
The generic RpcRequestError<MethodError> payload (Method(MethodError)) is typed per RPC method. Keeping it statically typed through the wrappers means either parameterizing BlockchainSourceError (a type parameter that infects everything above it) or classifying MethodError into a common typed representation at the boundary (every method error is born from an RpcError, so a lossless common form is plausible). This choice shapes the whole refactor and deserves the ADR's attention first.
Payoff and cost
Payoff: the #1404 bug class becomes unrepresentable; the #1406 contract tests, the source()-reachability doc contracts, and both downcast walks become deletable; error messages stop depending on Display-formatting conventions of erased types.
Cost: ChainIndexError construction sites are spread across all of zaino-state, so this is a large mechanical refactor on top of one real design decision. The regression tests landed in the #1404–#1411 series pin the wallet-visible wire behavior on both surfaces and serve as the safety net.
Given the scope, this should start as an ADR (docs/adr) settling the MethodError question and the variant taxonomy before any code moves.
Context
The #1404 masking family was fixed in four increments: PR #1405 healed the severed
source()link onUnexpectedErrorResponse, PR #1409 healedRpcRequestError::Methodand the threeStatusErrorwrappers (#1408), PR #1410 made theUnexpectedErrorResponsepayload a statically typedRpcErrorand added per-passthrough-type contract tests (#1406), and PR #1411 gave all 28 JSON-RPC methods the recovery walk (#1407).All of that work, however, preserves the existing architecture: rejection attribution happens by walking
source()chains at runtime and downcasting each link toRpcError.Error::source()is a type-erasing API, so whether a rejection is reachable is a runtime property the compiler cannot check — which is exactly why the #1406 contract tests had to exist, and why the #1404 bug class (a wrapper silently severing the chain) was representable in the first place.Remaining type erasure
Two wrappers still erase the concrete cause on the path from a node rejection to the serving surfaces:
BlockchainSourceError::UnrecoverableWithSource { source: Box<dyn Error + Send + Sync> }inpackages/zaino-state/src/chain_index/source.rs.ChainIndexError { kind, message, source: Option<Box<dyn Error + Send + Sync>> }inpackages/zaino-state/src/error.rs— a stringlykind + messagestruct rather than a typed enum.Correspondingly, both serving surfaces recover by downcast:
rpc_error_in_source_chainfeedingFrom<NodeBackedIndexerServiceError> for tonic::Status(gRPC), andrpc_error_from_error_sourceinsideerror_object_from_source_chain(JSON-RPC).Proposal: typed variants and exhaustive matching, no walks
Preserve the concrete error type through each layer as enum variants, and replace the recovery walks with pattern matches:
BlockchainSourceErrorgains typed variants — for exampleNodeRejection(RpcError)andTransport(TransportError)— populated by aFrom<RpcRequestError<M>>impl that classifies at the boundary. PR Statically typed UnexpectedErrorResponse payload + per-passthrough-type contract tests #1410's Tier 1 change makes this tractable: an unmapped rejection is already a statically typedRpcErrorwhen it leaves zaino-fetch.ChainIndexErrorbecomes an enum with typed variants, for exampleBackingValidator(BlockchainSourceError), in place of thekind + message + boxed sourcestruct.From<NodeBackedIndexerServiceError> for tonic::Statusand the per-method JSON-RPC translators — become exhaustive matches on those variants. The compiler then proves a node rejection reaches the wire; a new variant that swallows a rejection fails to compile rather than failing a contract test.Open design question
The generic
RpcRequestError<MethodError>payload (Method(MethodError)) is typed per RPC method. Keeping it statically typed through the wrappers means either parameterizingBlockchainSourceError(a type parameter that infects everything above it) or classifyingMethodErrorinto a common typed representation at the boundary (every method error is born from anRpcError, so a lossless common form is plausible). This choice shapes the whole refactor and deserves the ADR's attention first.Payoff and cost
Payoff: the #1404 bug class becomes unrepresentable; the #1406 contract tests, the
source()-reachability doc contracts, and both downcast walks become deletable; error messages stop depending onDisplay-formatting conventions of erased types.Cost:
ChainIndexErrorconstruction sites are spread across all of zaino-state, so this is a large mechanical refactor on top of one real design decision. The regression tests landed in the #1404–#1411 series pin the wallet-visible wire behavior on both surfaces and serve as the safety net.Given the scope, this should start as an ADR (docs/adr) settling the
MethodErrorquestion and the variant taxonomy before any code moves.Follow-up to #1404; builds on #1405, #1408/#1409, #1406/#1410, #1407/#1411.
🤖 Generated with Claude Code