Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

refactor: state RPC remove max_rpc_payload_size #13649

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

niklasad1
Copy link
Member

@niklasad1 niklasad1 commented Mar 20, 2023

Close #13623

This limit on state_traceBlock RPC is not needed anymore as the JSON-RPC servers doesn't terminate the connection if the RPC max limit is exceeded anymore

Until this PR is merged, it's also possible to run --rpc-max-payload <LIMIT> for the limit in the state RPC to work properly.

This limit is not needed anymore as the JSON-RPC servers doesn't terminate
the connection if the RPC max limit is exceeded anymore
@niklasad1 niklasad1 requested a review from koute as a code owner March 20, 2023 09:28
@niklasad1 niklasad1 added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Mar 20, 2023
@niklasad1 niklasad1 requested a review from lexnv March 20, 2023 09:30
Copy link
Contributor

@koute koute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to also add a deprecation message which pops up when the user specifies this argument on the command line.

@niklasad1
Copy link
Member Author

We already got, sufficient?

      --rpc-max-payload <RPC_MAX_PAYLOAD>
          DEPRECATED, this has no affect anymore. Use `rpc_max_request_size` or `rpc_max_response_size` instead

However, it was a nit by me when I deprecated some of the RPC flags and missed e that just --rpc-max-payload was used by the this RPC method/tracing stuff.

@koute
Copy link
Contributor

koute commented Mar 21, 2023

We already got, sufficient?

      --rpc-max-payload <RPC_MAX_PAYLOAD>
          DEPRECATED, this has no affect anymore. Use `rpc_max_request_size` or `rpc_max_response_size` instead

Does this pop up also when it's used, or do the user has to look at --help? Because if it's only in --help then no one's going to react to it. The idea here is to get people to remove this parameter from their command line invocations so that we can completely nuke it in a few releases.

@niklasad1
Copy link
Member Author

☝️ is one --help indeed but I added some ugly warnings as well when it's used

DEPRECATED: `--rpc_max_payload` has been removed use `rpc-max-response-size or rpc-max-request-size` instead
Setting `rpc-max-response-size` to `max(rpc_max_payload, rpc_max_request_size)`
DEPRECATED: `--rpc_max_payload` has been removed use `rpc-max-response-size or rpc-max-request-size` instead
Setting HTTP `rpc-max-response-size` to `max(rpc_max_payload, rpc_max_response_size)`

@koute
Copy link
Contributor

koute commented Mar 21, 2023

point_up is one --help indeed but I added some ugly warnings as well when it's used

DEPRECATED: `--rpc_max_payload` has been removed use `rpc-max-response-size or rpc-max-request-size` instead
Setting `rpc-max-response-size` to `max(rpc_max_payload, rpc_max_request_size)`
DEPRECATED: `--rpc_max_payload` has been removed use `rpc-max-response-size or rpc-max-request-size` instead
Setting HTTP `rpc-max-response-size` to `max(rpc_max_payload, rpc_max_response_size)`

Yeah, if there's an actual warning then that's fine by me. 👍

@niklasad1
Copy link
Member Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 936e995 into master Mar 21, 2023
@paritytech-processbot paritytech-processbot bot deleted the na-fix-13623 branch March 21, 2023 15:25
breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
This limit is not needed anymore as the JSON-RPC servers doesn't terminate
the connection if the RPC max limit is exceeded anymore
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
This limit is not needed anymore as the JSON-RPC servers doesn't terminate
the connection if the RPC max limit is exceeded anymore
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[rpc] --rpc-max-response-size doesn't seem work correctly
4 participants