feat(katana): default to deserialize response as JSON

[kariy]

Deserialize the JSON-RPC response object as a raw JSON to make the client compatible with other Starknet RPC spec.

Currently, we deserialize the response as concrete types from starknet-rs crate. The crate only supports a single RPC spec at a time so the types are bounded to a single RPC spec. Meaning the client is only compatible when interacting the a server of a similar spec.

Deserializing as raw JSON makes the client more flexible and can be used to interact with Starknet servers regardless of the RPC spec version.

[claude]

Claude finished @kariy's task —— View job

---

PR Review: Default to Deserialize Response as JSON

Review Checklist:

• Analyze code changes and architecture
• Check for potential bugs and issues
• Assess performance implications
• Review security considerations
• Evaluate test coverage
• Provide comprehensive feedback

Architecture Analysis

The PR implements a significant architectural change in the RPC client (bin/katana/src/cli/rpc/client.rs), transitioning from strongly-typed responses to raw JSON deserialization. This change affects most RPC methods while preserving type safety for a subset of core operations.

Key Changes:

• Methods returning Value instead of concrete types: spec_version, get_block_*, get_transaction_*, get_class*, trace_* methods
• Preserved typed returns: get_storage_at, call, block_number, chain_id, syncing, get_nonce
• Added optional dependencies: colored_json and serde_json under the client feature

Code Quality Assessment

Strengths:

• Clean implementation with consistent error handling patterns
• Good separation of concerns in the send_request method
• Proper use of anyhow for error context

Areas for Improvement:

1. API Consistency (client.rs:64-277)

   // Inconsistent return types - some return Value, others return concrete types
   pub async fn spec_version(&self) -> Result<Value> // Returns Value
   pub async fn chain_id(&self) -> Result<Felt>     // Returns concrete type

   Recommendation: Consider a more consistent approach. Either:

   • All methods return Value with helper methods for typed access
   • Add a type parameter to send_request to allow callers to choose

2. Missing Documentation

   • No documentation about which methods return raw JSON vs typed data
   • No explanation of the rationale for keeping certain methods typed

Potential Issues

Type Safety Concerns:

• Loss of compile-time type checking for most methods
• Potential runtime errors when consuming Value responses
• Makes the API less discoverable for consumers

Error Handling Impact:

• Specific deserialization errors are now lost in the Value conversion
• Could make debugging more difficult when responses don't match expected structure

Performance Considerations

Potential Performance Impact:

• serde_json::Value deserialization is typically 10-20% slower than direct struct deserialization
• Higher memory usage due to storing type information in Value
• Additional serialization step when displaying results

Recommendation: Consider adding benchmarks to measure the actual performance impact for your use case.

Security Review

Low Risk Assessment:

• No direct security vulnerabilities introduced
• Data validation is moved from compile-time to runtime
• Potential for malformed data to pass through validation

Recommendation: Ensure downstream consumers of Value responses properly validate data structure.

Test Coverage Analysis

Current State:

• No direct unit tests for the CLI client (low risk for breaking changes)
• Current CLI usage patterns are compatible with both return types
• Integration tests use different client implementations

Recommendations:

1. Add unit tests for the new Value return types
2. Test CLI commands manually to ensure correct output formatting
3. Consider adding integration tests that validate JSON structure

Architectural Feedback

Positive Aspects:

• Achieves the goal of RPC spec compatibility
• Maintains backward compatibility where type safety is critical
• Well-structured generic send_request method

Alternative Approaches to Consider:

1. Hybrid Approach: Keep current implementation but add _raw variants:

   pub async fn get_block_with_tx_hashes(&self, block_id: BlockId) -> Result<Block>
   pub async fn get_block_with_tx_hashes_raw(&self, block_id: BlockId) -> Result<Value>

2. Type Parameter Approach: Make the client generic over response type:

   pub async fn get_block_with_tx_hashes<T: DeserializeOwned>(&self, block_id: BlockId) -> Result<T>

Dependency Management

Cargo.toml Changes Look Good:

• Appropriate use of optional dependencies
• Clear feature flag (client) for new functionality

Summary

This PR successfully addresses the RPC compatibility issue but introduces a significant architectural trade-off. The change from typed to untyped responses provides flexibility at the cost of type safety. While the implementation is clean and the current usage patterns are compatible, the inconsistent API surface and loss of compile-time checking are concerns.

Recommendation: Consider the hybrid approach mentioned above to maintain both type safety and flexibility, or ensure comprehensive documentation and testing for the new untyped methods.

---

"Accept the things to which fate binds you, and love the people with whom fate brings you together." - Marcus Aurelius, Meditations

[github-actions]

Binary size report 📊

Branch	Size
main	64.08MiB
refactor/cli-rpc	64.08MiB ( -0.00% )

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 167 lines in your changes missing coverage. Please review.

Project coverage is 72.09%. Comparing base (9bde0ae) to head (edbc74c).
Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
bin/katana/src/cli/rpc/client.rs	0.00%	167 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #157      +/-   ##
==========================================
- Coverage   73.32%   72.09%   -1.24%     
==========================================
  Files         209      217       +8     
  Lines       23132    23670     +538     
==========================================
+ Hits        16961    17064     +103     
- Misses       6171     6606     +435     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Claude finished @kariy's task —— View job

---

PR Review: Default to Deserialize Response as JSON

Review Checklist:

• Analyze code changes and architecture
• Check for potential bugs and issues
• Assess performance implications
• Review security considerations
• Evaluate test coverage
• Provide comprehensive feedback

---

Architecture Analysis

This PR represents a significant architectural shift in the RPC client implementation. The changes completely replace the high-level JsonRpcClient from starknet-rs with a custom implementation using raw HttpTransport, transitioning from strongly-typed responses to raw JSON deserialization.

Key Changes:

• Client Architecture: Replaced JsonRpcClient<HttpTransport> with direct HttpTransport usage
• Return Types: Changed from concrete types (Felt, String, ContractClass, etc.) to Value for most methods
• Request Handling: Custom send_request method with manual JSON-RPC response parsing
• Dependencies: Added serde_json as optional dependency under client feature

Code Quality Assessment

Strengths:

• Clean, well-structured implementation with consistent patterns
• Proper error handling with contextual error messages using anyhow
• Good use of references (.as_ref()) for parameter passing
• Maintains the same public API surface (method signatures aside from return types)

Areas for Improvement:

1. API Consistency (client.rs:64-277)

   pub async fn spec_version(&self) -> Result<Value>           // Raw JSON
   pub async fn block_number(&self) -> Result<BlockNumber>    // Concrete type
   pub async fn syncing(&self) -> Result<SyncStatusType>      // Concrete type

   Issue: Inconsistent return types across methods without clear rationale documented.

2. Missing Documentation

   • No API documentation explaining which methods return raw JSON vs typed responses
   • No explanation of the architectural decision or migration guide for consumers

3. Error Handling Concerns (client.rs:47-53)

   JsonRpcResponse::Error { error, .. } => {
       Err(match TryInto::<StarknetError>::try_into(&error) {
           Ok(error) => StarknetProviderError::StarknetError(error),
           Err(_) => JsonRpcClientError::<<HttpTransport as JsonRpcTransport>::Error>::JsonRpcError(error).into(),
       })
   }

   Issue: Complex error handling logic could be extracted into a helper method for better maintainability.

Potential Issues

Type Safety Concerns:

• Runtime vs Compile-time: Loss of compile-time type checking for most API responses
• Consumer Impact: Downstream code must now handle Value parsing with potential runtime failures
• API Discoverability: Harder for developers to understand expected response structure

Breaking Changes:

• Return Type Changes: Multiple methods changed from concrete types to Value
• Import Requirements: Consumers now need serde_json::Value imports
• Error Handling: Different error patterns for malformed JSON vs typed deserialization

Performance Considerations

Potential Impact:

• Memory Usage: serde_json::Value has higher memory overhead than targeted deserialization
• CPU Cost: Additional parsing step when consumers need to extract specific fields
• Serialization: Double serialization for display/output (JSON → Value → String)

Benchmarking Recommendation: The performance impact depends heavily on usage patterns. Consider adding benchmarks to measure the actual cost in your specific use case.

Security Review

Low Risk Assessment:

• No direct security vulnerabilities introduced
• Proper error handling prevents information leakage
• Uses well-established serde_json for JSON processing

Considerations:

• Data Validation: Moved from compile-time to runtime, requiring downstream validation
• Malformed Data: Potential for unexpected JSON structure to pass through validation

Test Coverage Analysis

Current State:

• No Direct Tests: The CLI client lacks unit tests (understandable for a CLI tool)
• Integration Risk: Changes to core client without comprehensive test coverage
• Regression Risk: Potential for breaking changes in downstream consumers

Recommendations:

1. Add Unit Tests: Test the new send_request method with various response types
2. Integration Tests: Verify JSON responses match expected structure
3. Manual Testing: Test CLI commands that use the changed return types

Alternative Approaches

1. Hybrid Approach

// Keep both typed and raw versions
pub async fn get_block_with_tx_hashes(&self, block_id: BlockId) -> Result<Block>
pub async fn get_block_with_tx_hashes_raw(&self, block_id: BlockId) -> Result<Value>

2. Generic Response Type

pub async fn get_block_with_tx_hashes<T: DeserializeOwned>(&self, block_id: BlockId) -> Result<T>

3. Configuration-based

pub struct Client {
    transport: HttpTransport,
    raw_responses: bool,
}

Dependency Management

Cargo.toml Changes:

• ✅ Proper use of optional dependencies under client feature
• ✅ Appropriate workspace dependency usage for serde_json
• ✅ Clean feature flag implementation

Summary

This PR successfully addresses the RPC compatibility goal by enabling interaction with different Starknet RPC spec versions. However, it introduces significant trade-offs:

Pros:

• ✅ Achieves RPC spec compatibility objective
• ✅ Clean, maintainable implementation
• ✅ Consistent error handling patterns

Cons:

• ❌ Loss of compile-time type safety for most endpoints
• ❌ Inconsistent API surface (mix of typed and untyped responses)
• ❌ Potential performance overhead
• ❌ Breaking changes without migration guide

Recommendation:
The implementation is solid, but consider:

1. Documentation: Add clear documentation about which methods return raw JSON and why
2. Consistency: Either make all methods return Value or provide clear criteria for the split
3. Migration Guide: Document the breaking changes and provide guidance for consumers
4. Testing: Add unit tests for the new client implementation

The architectural decision makes sense for the stated compatibility goals, but the execution could be improved with better documentation and more consistent API design.

---

"When you wake up in the morning, tell yourself: The people I deal with today will be meddling, ungrateful, arrogant, dishonest, jealous, and surly. They are like this because they can't tell good from evil. But I have seen the beauty of good, and the ugliness of evil, and have recognized that the wrongdoer has a nature related to my own - not of the same blood or birth, but the same mind, and possessing a share of the divine. And so none of them can hurt me."

- Marcus Aurelius, Meditations

---