Skip to content

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Aug 26, 2025

The main change this PR introduces is to flatten the rate limit configurations.
Currently it's a dictionary containing:

  • the aggregate rate limits (for messages subject to this limit)
  • the aggregate byte rate limit (for messages subject to this limit)
  • A dict for "transaction messages" (a name for the category of messages not subject to the aggregate limits), mapping message types to rate limits
  • A dict for "other" messages, which are subject to the aggregate limits, mapping message types to rate limits
  • the default rate limits for any message that doesn't have a specific limit configured (there are none)

The main changes:

  • remove the "default_settings", since it's not used
  • move the aggregate rate limits out into its own object
  • merge the "tx" and "non-tx" lists, instead indicating whether a message type is subject to the aggregate rate limits as part of the rate limit settings.

Gains

  • This removes the use of Any in the type hint, since the dictionary is now homogeneous
  • The rate limit settings is a simpler structure
  • There's no longer any need for a complex function to amend the v2 settings with the v1 settings. We can just use a regular dict.update()

@arvidn arvidn force-pushed the simplify-rate-limit-numbers branch 2 times, most recently from 0821062 to 6bb5a7f Compare August 27, 2025 09:15
Copy link

Pull Request Test Coverage Report for Build 17262573433

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 277 of 278 (99.64%) changed or added relevant lines in 3 files are covered.
  • 36 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.003%) to 91.293%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chia/_tests/core/server/test_rate_limits.py 245 246 99.59%
Files with Coverage Reduction New Missed Lines %
chia/full_node/pending_tx_cache.py 1 96.55%
chia/wallet/util/wallet_sync_utils.py 1 86.07%
chia/server/node_discovery.py 2 82.47%
chia/wallet/wallet_node.py 3 87.56%
chia/full_node/full_node.py 4 88.11%
chia/_tests/core/util/test_lockfile.py 25 77.31%
Totals Coverage Status
Change from base Build 17253036064: -0.003%
Covered Lines: 102719
Relevant Lines: 112384

💛 - Coveralls

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Sep 2, 2025
@arvidn arvidn force-pushed the simplify-rate-limit-numbers branch from 6bb5a7f to bca0002 Compare September 3, 2025 06:04
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Sep 3, 2025
@arvidn arvidn added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Sep 3, 2025
@arvidn arvidn marked this pull request as ready for review September 3, 2025 06:28
@arvidn arvidn requested a review from a team as a code owner September 3, 2025 06:28
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Sep 3, 2025
@arvidn arvidn force-pushed the simplify-rate-limit-numbers branch from bca0002 to 3e88de7 Compare September 4, 2025 06:10
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Sep 4, 2025
@arvidn arvidn requested a review from altendky September 4, 2025 13:45
@arvidn arvidn force-pushed the simplify-rate-limit-numbers branch from f9cee0b to ee1beb5 Compare September 8, 2025 20:21
@arvidn arvidn requested a review from altendky September 8, 2025 21:54
Copy link
Contributor

@wjblanke wjblanke left a comment

Choose a reason for hiding this comment

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

aok

@pmaslana pmaslana merged commit c1d15df into main Sep 11, 2025
576 of 580 checks passed
@pmaslana pmaslana deleted the simplify-rate-limit-numbers branch September 11, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants