feat: upgrade jsonrpsee

[dinhani-cw]

No description provided.

[github-actions]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	4, because the PR involves significant changes across multiple files, including modifications to core middleware structures and RPC methods. The changes are spread across various aspects of the system, such as networking, logging, and transaction handling, which requires a thorough understanding of the existing architecture and the implications of the updates.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The PR introduces changes to the RpcMiddleware and RpcResponse structures, specifically changing the generic type S to a specific type RpcService and ResponseFuture<BoxFuture<'a, MethodResponse>>. This could potentially lead to issues if there are specific traits or methods expected from the generic type that are not present or behave differently in the new fixed types.
	Performance Concern: The extensive use of logging and tracing in the RPC methods could potentially impact performance, especially under high load. It is important to ensure that these logging statements do not significantly slow down the RPC response times.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/eth/rpc/rpc_middleware.rs
suggestion	Consider implementing logging for the RpcMiddleware call method to monitor the method calls and parameters for better traceability and debugging. This is particularly useful for identifying issues in production environments. [important]
relevant line	fn call(&self, request: jsonrpsee::types::Request<'a>) -> Self::Future {

---

relevant file	src/eth/rpc/rpc_server.rs
suggestion	Ensure that the Extensions parameter in RPC methods is utilized effectively, especially if it's intended to carry important contextual information or control structures. Currently, it appears as an unused parameter which might be an oversight. [important]
relevant line	async fn debug_set_head(params: Params<'_>, ctx: Arc, _: Extensions) -> anyhow::Result {

---

relevant file	src/eth/rpc/rpc_subscriptions.rs
suggestion	It's recommended to add error handling for the subscription insertion into the HashMap. This would manage cases where the insertion fails, possibly due to an already existing key, ensuring the system's robustness. [important]
relevant line	subs.insert(sink.connection_id(), sink);

---

relevant file	Cargo.toml
suggestion	Verify compatibility of the updated jsonrpsee library version with other dependencies and ensure that all features used in the project are supported by the new version to avoid runtime issues. [important]
relevant line	jsonrpsee = { version = "=0.23.0", features = ["server", "client"] }

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add error handling in the call method to manage request processing failures Implement error handling in the call method of RpcMiddleware to manage potential failures in request processing. src/eth/rpc/rpc_middleware.rs [49-50] fn call(&self, request: jsonrpsee::types::Request<'a>) -> Self::Future { // extract signature if available + if let Err(e) = request.validate() { + return Err(e); + } Suggestion importance[1-10]: 9 Why: Implementing error handling in the call method is crucial for robust and reliable request processing. This suggestion addresses a potential issue and significantly improves the code's resilience.	9
	Add handling for cases where Extensions is not provided in RPC methods Consider handling the case where the Extensions parameter is not provided or is empty in the RPC methods that now require it. This can be done by either providing a default value or by explicitly checking for its presence and handling the absence appropriately. src/eth/rpc/rpc_server.rs [194] -async fn debug_set_head(params: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> anyhow::Result<JsonValue, RpcError> +async fn debug_set_head(params: Params<'_>, ctx: Arc<RpcContext>, ext: Option<Extensions>) -> anyhow::Result<JsonValue, RpcError> { + if let Some(ext) = ext { + // existing logic here + } else { + // handle the case where extensions are not provided + } +} Suggestion importance[1-10]: 8 Why: This suggestion addresses a potential issue where the Extensions parameter might not be provided, which could lead to runtime errors. Handling this case improves the robustness of the code.	8
	Simplify the logging statement by removing unnecessary tuple access Ensure that the connection_id() method returns a valid ConnectionId type directly, instead of a tuple, to avoid unnecessary tuple access (.0). src/eth/rpc/rpc_subscriptions.rs [210] -tracing::debug!(id = %sink.connection_id().0, "subscribing to newPendingTransactions event"); +tracing::debug!(id = %sink.connection_id(), "subscribing to newPendingTransactions event"); Suggestion importance[1-10]: 7 Why: This suggestion addresses a potential issue with unnecessary tuple access, which can simplify the code and improve readability. However, it assumes changes to the connection_id() method that may not be trivial.	7
Maintainability	Remove unused Extensions parameter from methods where it is not utilized For the newly added Extensions parameter in RPC methods, ensure that it is utilized within the method's logic if necessary, or remove it if it is not used to avoid confusion. src/eth/rpc/rpc_server.rs [253] -async fn net_version(_: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> String +async fn net_version(_: Params<'_>, ctx: Arc<RpcContext>) -> String Suggestion importance[1-10]: 9 Why: Removing unused parameters improves code readability and maintainability. This suggestion correctly identifies and addresses the unnecessary inclusion of the Extensions parameter.	9
	Refactor RpcMiddleware to use generic dependency injection for better testing and flexibility Refactor RpcMiddleware to use dependency injection for RpcService, allowing for easier testing and flexibility in service implementations. src/eth/rpc/rpc_middleware.rs [42-44] -pub struct RpcMiddleware { - service: RpcService, +pub struct RpcMiddleware<S: RpcServiceT> { + service: S, } Suggestion importance[1-10]: 8 Why: Refactoring to use dependency injection enhances testability and flexibility, making the codebase easier to maintain and extend. This is a valuable improvement for long-term maintainability.	8
	Improve variable naming for tuple elements in the logs HashMap Consider using a more descriptive variable name for the tuple elements in the logs HashMap to improve code readability and maintainability. src/eth/rpc/rpc_subscriptions.rs [204] -logs: RwLock<HashMap<ConnectionId, (SubscriptionSink, LogFilter)>> +logs: RwLock<HashMap<ConnectionId, (sink: SubscriptionSink, filter: LogFilter)>> Suggestion importance[1-10]: 5 Why: While using more descriptive names for tuple elements can improve readability, Rust does not support named tuple elements in HashMaps directly. This suggestion is not applicable as written.	5
Enhancement	Enhance flexibility of RpcResponse by using a generic future type Update the RpcResponse struct to use a generic type for future_response instead of a fixed ResponseFuture<BoxFuture<'a, MethodResponse>> to maintain flexibility and allow for different future types. src/eth/rpc/rpc_middleware.rs [102-104] -pub struct RpcResponse<'a> { +pub struct RpcResponse<'a, F> where F: Future<Output = MethodResponse> + 'a { #[pin] - future_response: ResponseFuture<BoxFuture<'a, MethodResponse>>, + future_response: F, } Suggestion importance[1-10]: 8 Why: Using a generic future type for future_response increases the flexibility and reusability of the RpcResponse struct. This is a significant improvement in terms of code maintainability and extensibility.	8
	Enhance error handling and logging in RPC methods To improve error handling, consider adding specific error messages or logging within the RPC methods when operations fail, especially when interacting with the context or external services. src/eth/rpc/rpc_server.rs [488] -async fn eth_get_balance(params: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> anyhow::Result<String, RpcError> +async fn eth_get_balance(params: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> anyhow::Result<String, RpcError> { + let (params, address) = next_rpc_param::<Address>(params.sequence())?; + let (_, block_selection) = next_rpc_param_or_default::<BlockSelection>(params)?; + match ctx.storage.get_balance(&address, block_selection).await { + Ok(balance) => Ok(balance), + Err(e) => { + log::error!("Failed to get balance for address {:?}: {}", address, e); + Err(e) + } + } +} Suggestion importance[1-10]: 7 Why: Adding specific error messages or logging improves the observability and debuggability of the code. This suggestion enhances error handling, which is beneficial but not critical.	7
Possible bug	Ensure type consistency and correct functionality by replacing SubscriptionId with ConnectionId Replace the removed SubscriptionId type with ConnectionId in the new_pending_txs, new_heads, and logs HashMaps to ensure type consistency and correct functionality. src/eth/rpc/rpc_subscriptions.rs [202-204] +new_pending_txs: RwLock<HashMap<ConnectionId, SubscriptionSink>>, +new_heads: RwLock<HashMap<ConnectionId, SubscriptionSink>>, +logs: RwLock<HashMap<ConnectionId, (SubscriptionSink, LogFilter)>> - Suggestion importance[1-10]: 8 Why: This suggestion correctly identifies the need to replace SubscriptionId with ConnectionId for type consistency and functionality, which is crucial for the correctness of the code.	8
Best practice	Add thread safety to RpcMiddleware by ensuring it implements Send and Sync Consider adding trait bounds Send + Sync to RpcMiddleware to ensure that it can be safely shared and sent between threads, similar to the previous implementation. src/eth/rpc/rpc_middleware.rs [42-44] pub struct RpcMiddleware { service: RpcService, } +impl Send for RpcMiddleware {} +impl Sync for RpcMiddleware {} Suggestion importance[1-10]: 7 Why: Adding Send and Sync trait bounds can enhance thread safety, which is generally a good practice. However, it may not be strictly necessary depending on the use case, so it's a minor improvement rather than a crucial one.	7
	Refactor to use a middleware for handling Extensions in RPC methods Refactor the repeated pattern of adding Extensions to multiple functions by creating a higher-order function or middleware that automatically handles this parameter, simplifying the function signatures and usage. src/eth/rpc/rpc_server.rs [411] -async fn eth_send_raw_transaction(params: Params<'_>, ctx: Arc<RpcContext>, _: Extensions) -> anyhow::Result<String, RpcError> +async fn eth_send_raw_transaction(params: Params<'_>, ctx: Arc<RpcContext>) -> anyhow::Result<String, RpcError> { + with_extensions(params, ctx, |params, ctx, ext| { + // existing logic here + }) +} Suggestion importance[1-10]: 6 Why: While the idea of using a middleware to handle Extensions is good for reducing repetitive code, the implementation complexity may not justify the change unless Extensions handling is extensive and complex.	6
	Increase flexibility in dependency resolution by specifying a compatible version range for jsonrpsee Consider specifying a range of compatible versions for jsonrpsee instead of pinning to a specific version to allow more flexibility in dependency resolution and future updates. Cargo.toml [71] -jsonrpsee = { version = "=0.23.0", features = ["server", "client"] } +jsonrpsee = { version = "0.23", features = ["server", "client"] } Suggestion importance[1-10]: 6 Why: Specifying a range of compatible versions can indeed provide more flexibility in dependency management. However, pinning to a specific version might be intentional to avoid compatibility issues, so this suggestion is more of a best practice than a necessity.	6