Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

chore(p2p): log if rate limit was peer or global #12116

Conversation

Maddiaa0
Copy link
Member

Overview

When running testbench, some rate limit logs were unclear if they were due to peer specific limits ( misbehaviour ) or due
to global limits ( ddos prevention ) this change fixes the error message to include what offense has been observed

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Maddiaa0 Maddiaa0 marked this pull request as ready for review February 19, 2025 17:38
Copy link
Contributor

@natebeauregard natebeauregard left a comment

Choose a reason for hiding this comment

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

Left one question about the peer scoring refactor but otherwise LGTM!

return false;
default:
return true;
if (rateLimitStatus === RateLimitStatus.DeniedGlobal) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this conditional check that the RateLimitStatus is DeniedPeer instead of DeniedGlobal before penalizing?

Suggested change
if (rateLimitStatus === RateLimitStatus.DeniedGlobal) {
if (rateLimitStatus === RateLimitStatus.DeniedPeer) {

Copy link
Member Author

@Maddiaa0 Maddiaa0 Feb 19, 2025

Choose a reason for hiding this comment

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

oh god youre right, will add unit test too
edit: good news, existing unit test caught it, this was just a typo

@Maddiaa0 Maddiaa0 requested a review from charlielye as a code owner February 19, 2025 18:21
@Maddiaa0 Maddiaa0 removed the request for review from charlielye February 19, 2025 18:22
@Maddiaa0 Maddiaa0 enabled auto-merge (squash) February 19, 2025 18:24
@Maddiaa0 Maddiaa0 disabled auto-merge February 19, 2025 18:35
@Maddiaa0 Maddiaa0 merged commit 13ad91c into master Feb 19, 2025
13 checks passed
@Maddiaa0 Maddiaa0 deleted the md/02-19-chore_log_if_rate_limit_status_was_peer___global_rate_limit branch February 19, 2025 18:48
TomAFrench added a commit that referenced this pull request Feb 19, 2025
* master: (264 commits)
  chore(p2p): log if rate limit was peer or global (#12116)
  chore: @aztec/stdlib pt1 -> cleanup circuits js (#12039)
  chore(tests): shorten block times in e2e p2p tests (#12073)
  fix: darwin properly erroring (#12113)
  chore: add missing import (#12111)
  fix: yarn remake-constants (#12109)
  chore: fix error in oracle definition (#12090)
  fix: Don't consider skipping (#10598)
  fix: Use gas billed in block header building (#12101)
  fix(avm): disable wrong sha skippable (#12099)
  chore: Provide defaults for bb and acvm in release image (#12105)
  fix(avm): break TS dependency cycle (#12103)
  feat: IVC gates command in WASM (#11792)
  fix: SharedMutable compilation warnings (#12098)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  feat: Sync from noir (#12064)
  chore: Fix unbound CI variable on release image bootstrap (#12095)
  ...
TomAFrench added a commit that referenced this pull request Feb 19, 2025
* master: (264 commits)
  chore(p2p): log if rate limit was peer or global (#12116)
  chore: @aztec/stdlib pt1 -> cleanup circuits js (#12039)
  chore(tests): shorten block times in e2e p2p tests (#12073)
  fix: darwin properly erroring (#12113)
  chore: add missing import (#12111)
  fix: yarn remake-constants (#12109)
  chore: fix error in oracle definition (#12090)
  fix: Don't consider skipping (#10598)
  fix: Use gas billed in block header building (#12101)
  fix(avm): disable wrong sha skippable (#12099)
  chore: Provide defaults for bb and acvm in release image (#12105)
  fix(avm): break TS dependency cycle (#12103)
  feat: IVC gates command in WASM (#11792)
  fix: SharedMutable compilation warnings (#12098)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  feat: Sync from noir (#12064)
  chore: Fix unbound CI variable on release image bootstrap (#12095)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants