Skip to content

fix(rpc): apply RST_STREAM frame limit by default - #7015

Open
waynercheung wants to merge 1 commit into
tronprotocol:release_v4.8.3from
waynercheung:fix/rpc-rst-stream-limit
Open

waynercheung wants to merge 1 commit into
tronprotocol:release_v4.8.3from
waynercheung:fix/rpc-rst-stream-limit

Conversation

@waynercheung

Copy link
Copy Markdown
Collaborator

What does this PR do?

Turns node.rpc.maxRstStream and node.rpc.secondsPerWindow into properly defaulted and validated options, and applies the gRPC HTTP/2 RST_STREAM frame limit unconditionally.

  • common (NodeConfig): default both options to 1000 / 5. In postProcess, reject negative values for either option and reject Integer.MAX_VALUE specifically for maxRstStream (which grpc-java treats as disabling the limit), and independently fall back each 0 to its default with a warning.
  • framework (RpcService): always call maxRstFramesPerWindow(...) with the configured values; validation stays solely in NodeConfig, consistent with the neighboring maxConcurrentCallsPerConnection option.
  • reference.conf, docs/configuration.md, config/README.md: document the new defaults and the 0 fallback kept for backward compatibility.

Why are these changes required?

Both options previously defaulted to 0, and RpcService configured maxRstFramesPerWindow only when both values were positive. As a result the RST_STREAM frame limit was left unset in a default deployment, and misconfigured values (negative for either option, or maxRstStream == Integer.MAX_VALUE) were accepted silently. This brings the two options in line with how the neighboring maxConcurrentCallsPerConnection option already behaves (a secure default plus validation), so the setting is consistent across the config, cannot be silently disabled, and rejects invalid values at startup.

This PR has been tested by:

  • Unit Tests
    • NodeConfigTest: default resolves to 1000/5; each 0 independently falls back to its default; negative values and maxRstStream == Integer.MAX_VALUE are rejected with PARAMETER_INIT; explicit positive values (including Integer.MAX_VALUE - 1 and an Integer.MAX_VALUE window) are preserved.
    • RpcServiceHttp2SecurityTest: the limit is enforced at the connection layer under the default, under the legacy 0/0, and under an explicit small limit; a connection at the limit still gets its PING acknowledged, and one more RST_STREAM past the limit is answered with GOAWAY(ENHANCE_YOUR_CALM).
    • The affected configuration and RPC test classes pass, along with framework Checkstyle and reference.conf validation.
  • Manual Testing
    • No separate manual testing; transport behavior is covered by the automated raw HTTP/2 tests above.

Follow up

None.

Extra details

  • Backward compatibility: an explicit 0 for either option now falls back to its default (with a startup warning) instead of leaving the limit unset; maxRstStream = 2147483647 (Integer.MAX_VALUE) is rejected at startup. Upgrade notes are included in docs/configuration.md and common/src/main/java/org/tron/core/config/README.md.

The node.rpc.maxRstStream and secondsPerWindow options defaulted to
0, which left the HTTP/2 RST_STREAM frame limit unset, and the limit
was configured only when both values were positive. Give them
sensible defaults, validate them at load time, and apply the limit
unconditionally so the configuration behaves consistently.

- NodeConfig: default maxRstStream/secondsPerWindow to 1000/5.
  In postProcess, reject negative values for either option and reject
  Integer.MAX_VALUE specifically for maxRstStream, which grpc-java
  treats as disabling the limit. Independently replace each zero with
  its default and log a warning.
- RpcService: always configure maxRstFramesPerWindow from the config
  values; validation stays solely in NodeConfig, consistent with the
  neighboring maxConcurrentCallsPerConnection option.
- reference.conf, docs/configuration.md and config/README.md:
  document the new defaults and the 0 fallback for compatibility.
- Tests: pin the Java and reference defaults, preserve positive
  overrides, and verify PING at the RST limit followed by GOAWAY
  after one additional reset using a prebuilt HTTP/2 frame burst.

Note: an explicit 0 now falls back to the default instead of leaving
the limit unset, and maxRstStream = 2147483647 is rejected at startup.
@github-actions
github-actions Bot requested a review from 317787106 October 9, 2026 10:12
@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.3 milestone Oct 9, 2026
@halibobo1205 halibobo1205 added the topic:api rpc/http related issue label Oct 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:api rpc/http related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants