Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

enha(rpc): add configurable max response size for JSON-RPC server #2042

Merged
merged 1 commit into from
Mar 10, 2025

Conversation

gabriel-aranha-cw
Copy link
Contributor

@gabriel-aranha-cw gabriel-aranha-cw commented Mar 10, 2025

User description

Add a new configuration parameter rpc_max_response_size_bytes to control the maximum response size for JSON-RPC server connections. This allows more granular control over server resource limits and prevents potential large response-related issues.


PR Type

Enhancement


Description

  • Add configurable max response size for JSON-RPC server

  • Introduce rpc_max_response_size_bytes configuration parameter

  • Set default max response size to 10MB

  • Apply max response size limit to RPC server


Changes walkthrough 📝

Relevant files
Enhancement
rpc_config.rs
Add max response size configuration option                             

src/eth/rpc/rpc_config.rs

  • Add rpc_max_response_size_bytes field to RpcServerConfig struct
  • Set default value to 10485760 bytes (10MB)
  • Include command-line and environment variable options
  • +4/-0     
    rpc_server.rs
    Implement max response size limit in RPC server                   

    src/eth/rpc/rpc_server.rs

  • Apply max_response_body_size setting to RPC server
  • Use rpc_max_response_size_bytes from config
  • +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Add a new configuration parameter `rpc_max_response_size_bytes` to control the maximum response size for JSON-RPC server connections. This allows more granular control over server resource limits and prevents potential large response-related issues.
    @gabriel-aranha-cw gabriel-aranha-cw marked this pull request as ready for review March 10, 2025 19:01
    Copy link

    github-actions bot commented Mar 10, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 6f87210)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Configuration Logging

    Ensure that the new configuration parameter rpc_max_response_size_bytes is properly logged during the initialization of the RPC server component.

    /// JSON-RPC server max response size limit in bytes
    #[arg(long = "max-response-size-bytes", env = "MAX_RESPONSE_SIZE_BYTES", default_value = "10485760")]
    pub rpc_max_response_size_bytes: u32,

    Copy link

    Persistent review updated to latest commit 6f87210

    Copy link

    github-actions bot commented Mar 10, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 6f87210
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Use appropriate type for size

    Consider using usize instead of u32 for rpc_max_response_size_bytes to ensure
    compatibility with the max_response_body_size method, which typically expects a
    usize parameter.

    src/eth/rpc/rpc_config.rs [20-21]

     #[arg(long = "max-response-size-bytes", env = "MAX_RESPONSE_SIZE_BYTES", default_value = "10485760")]
    -pub rpc_max_response_size_bytes: u32,
    +pub rpc_max_response_size_bytes: usize,
    Suggestion importance[1-10]: 8

    __

    Why: Changing the type from u32 to usize is important for ensuring type compatibility with the max_response_body_size method, which typically expects a usize parameter. This change can prevent potential issues and improve code consistency.

    Medium
    Ensure type compatibility for method

    Ensure that the max_response_body_size method accepts u32 as a parameter. If it
    requires usize, consider converting rpc_max_response_size_bytes to usize before
    passing it.

    src/eth/rpc/rpc_server.rs [153]

    -.max_response_body_size(rpc_config.rpc_max_response_size_bytes)
    +.max_response_body_size(rpc_config.rpc_max_response_size_bytes as usize)
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion addresses a potential type mismatch between the rpc_max_response_size_bytes field (u32) and the max_response_body_size method's expected parameter type (likely usize). The proposed type conversion ensures compatibility and prevents potential runtime errors.

    Medium

    Previous suggestions

    Suggestions up to commit 6f87210
    CategorySuggestion                                                                                                                                    Impact
    General
    Use appropriate type for size

    Consider using usize instead of u32 for rpc_max_response_size_bytes to ensure
    compatibility with the max_response_body_size method, which typically expects a
    usize parameter.

    src/eth/rpc/rpc_config.rs [20-21]

     #[arg(long = "max-response-size-bytes", env = "MAX_RESPONSE_SIZE_BYTES", default_value = "10485760")]
    -pub rpc_max_response_size_bytes: u32,
    +pub rpc_max_response_size_bytes: usize,
    Suggestion importance[1-10]: 8

    __

    Why: Changing the type from u32 to usize is a significant improvement for size-related variables, ensuring better compatibility and preventing potential issues with large sizes on different architectures.

    Medium
    Ensure type compatibility for method

    Ensure that the max_response_body_size method accepts u32 as a parameter. If it
    requires usize, consider converting rpc_max_response_size_bytes to usize before
    passing it.

    src/eth/rpc/rpc_server.rs [153]

    -.max_response_body_size(rpc_config.rpc_max_response_size_bytes)
    +.max_response_body_size(rpc_config.rpc_max_response_size_bytes as usize)
    Suggestion importance[1-10]: 7

    __

    Why: This suggestion addresses a potential type mismatch between the config field and the method parameter, which could lead to compilation errors. The cast to usize ensures compatibility and prevents runtime issues.

    Medium

    @gabriel-aranha-cw gabriel-aranha-cw changed the title feat(rpc): add configurable max response size for JSON-RPC server enha(rpc): add configurable max response size for JSON-RPC server Mar 10, 2025
    @gabriel-aranha-cw gabriel-aranha-cw merged commit 462fa57 into main Mar 10, 2025
    42 checks passed
    @gabriel-aranha-cw gabriel-aranha-cw deleted the enha-expose-reponse-size-limit branch March 10, 2025 19:17
    @gabriel-aranha-cw
    Copy link
    Contributor Author

    Final benchmark:
    Run ID: bench-4170269060

    Git Info:

    Configuration:

    • Target Account Strategy: Default

    Leader Stats:
    RPS Stats: Max: 2011.00, Min: 1295.00, Avg: 1864.36, StdDev: 69.94
    TPS Stats: Max: 2031.00, Min: 1662.00, Avg: 1809.67, StdDev: 83.31

    Follower Stats:
    Imported Blocks/s: Max: 2.00, Min: 1.00, Avg: 1.63, StdDev: 0.48
    Imported Transactions/s: Max: 4009.00, Min: 1662.00, Avg: 3021.56, StdDev: 901.95

    Plots:

    # for free to join this conversation on GitHub. Already have an account? # to comment
    Labels
    None yet
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants