[Merged by Bors] - Add an option to disable inbound rate limiter #4327

pawanjay176 · 2023-05-24T04:32:58Z

Issue Addressed

On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets.

Added an option to configure inbound rate limits as well.

lighthouse/tests/beacon_node.rs

jimmygchen

just a minor comment, the rest LGTM - feel free to merge if you need to test it out.

…utbound types

divagant-martian

Looks good! I left a suggestion that would remove the need for two flags

divagant-martian · 2023-05-29T17:49:19Z

beacon_node/lighthouse_network/src/rpc/mod.rs

-            .expect("Configuration parameters are valid");
+        let inbound_limiter = inbound_rate_limiter_config.map(|config| {
+            debug!(log, "Using inbound rate limiting params"; "config" => ?config);
+            RateLimiter::new_with_config(config.0)


neat improvement

divagant-martian · 2023-05-29T19:14:53Z

beacon_node/src/cli.rs

+        .arg(
+            Arg::with_name("inbound-rate-limiter")
+            .long("inbound-rate-limiter")
+            .help(
+                "Enables the inbound rate limiter (requests received by this node).\
+                \
+                Rate limit quotas per protocol can be set in the form of \
+                <protocol_name>:<tokens>/<time_in_seconds>. To set quotas for multiple protocols, \
+                separate them by ';'. If the inbound rate limiter is enabled and a protocol is not \
+                present in the configuration, the default quotas will be used."
+            )
+            .min_values(0)
+            .hidden(true)
+        )


A suggestion that removes the need for two flags 4880119

In summary, the option will be a "normal" one that requires a value. The value is either "disabled" or a configuration string. This way we don't need to add a second flag for disabling.

Feel free to cherry pick if you like it

Nice, I like this.

scripts/tests/doppelganger_protection.sh

beacon_node/src/cli.rs

paulhauner · 2023-06-01T23:43:00Z

bors r+

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

bors · 2023-06-01T23:48:23Z

Build failed (retrying...):

clippy

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

bors · 2023-06-01T23:53:40Z

Build failed (retrying...):

clippy

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

bors · 2023-06-02T00:13:03Z

Build failed (retrying...):

clippy

paulhauner · 2023-06-02T00:19:23Z

bors r-

bors · 2023-06-02T00:19:25Z

Canceled.

paulhauner · 2023-06-02T00:19:48Z

bors r+

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

bors · 2023-06-02T00:21:13Z

Build failed (retrying...):

beacon-chain-tests

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

bors · 2023-06-02T00:24:00Z

Build failed (retrying...):

debug-tests-ubuntu

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

bors · 2023-06-02T00:31:02Z

Build failed:

release-tests-ubuntu

paulhauner · 2023-06-02T02:16:32Z

bors r+

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

bors · 2023-06-02T02:37:26Z

Build failed (retrying...):

execution-engine-integration-ubuntu

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

michaelsproul · 2023-06-02T03:17:08Z

bors r-
bors r+

bors · 2023-06-02T03:17:11Z

This PR was included in a batch that was canceled, it will be automatically retried

bors · 2023-06-02T03:17:18Z

Canceled.

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

bors · 2023-06-02T06:00:20Z

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

## Issue Addressed On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

On deneb devnetv5, lighthouse keeps rate limiting peers which makes it harder to bootstrap new nodes as there are very few peers in the network. This PR adds an option to disable the inbound rate limiter for testnets. Added an option to configure inbound rate limits as well. Co-authored-by: Diva M <[email protected]>

jimmygchen reviewed May 24, 2023

View reviewed changes

lighthouse/tests/beacon_node.rs Outdated Show resolved Hide resolved

jimmygchen approved these changes May 24, 2023

View reviewed changes

pawanjay176 added the ready-for-review The code is ready for review label May 24, 2023

pawanjay176 added 6 commits May 25, 2023 14:27

Add an option to disable inbound rate limiter

cff2ad6

Make Inbound rate limits configurable; merge config for inbound and o…

af7ecd4

…utbound types

Fix test

76ca4da

fmt

5c34201

Add back cli option

7824bf9

Fix errors from rebase

ecd0b7f

pawanjay176 force-pushed the rate-limit-flag branch from 5ecc3c7 to ecd0b7f Compare May 25, 2023 19:57

pawanjay176 changed the base branch from deneb-free-blobs to unstable May 25, 2023 19:57

pawanjay176 requested a review from divagant-martian May 25, 2023 19:57

pawanjay176 added 2 commits May 25, 2023 15:19

empty commit to start CI

a32631d

fmt

38e5a4b

pawanjay176 mentioned this pull request May 25, 2023

Add is_subset methods for BitList and BitVector sigp/ssz_types#8

Merged

print geth logs to debug dg test

e611365

divagant-martian reviewed May 29, 2023

View reviewed changes

divagant-martian added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels May 29, 2023

divagant-martian and others added 4 commits May 30, 2023 13:30

remove double flag

381627c

Remove debug line

51c2c0c

Fix test

411fc86

Fix cli

a0cf404

divagant-martian reviewed May 30, 2023

View reviewed changes

beacon_node/src/cli.rs Outdated Show resolved Hide resolved

pawanjay176 added 2 commits May 30, 2023 14:18

Fix cli again

5e6e39a

fmt

7f0f9f5

pawanjay176 added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 1, 2023

bors bot changed the title ~~Add an option to disable inbound rate limiter~~ [Merged by Bors] - Add an option to disable inbound rate limiter Jun 2, 2023

bors bot closed this Jun 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Add an option to disable inbound rate limiter #4327

[Merged by Bors] - Add an option to disable inbound rate limiter #4327

pawanjay176 commented May 24, 2023 •

edited

Loading

jimmygchen left a comment

divagant-martian left a comment

divagant-martian May 29, 2023

divagant-martian May 29, 2023 •

edited

Loading

pawanjay176 May 30, 2023

paulhauner commented Jun 1, 2023

bors bot commented Jun 1, 2023

bors bot commented Jun 1, 2023

bors bot commented Jun 2, 2023

paulhauner commented Jun 2, 2023

bors bot commented Jun 2, 2023

paulhauner commented Jun 2, 2023

bors bot commented Jun 2, 2023

bors bot commented Jun 2, 2023

bors bot commented Jun 2, 2023

paulhauner commented Jun 2, 2023

bors bot commented Jun 2, 2023

michaelsproul commented Jun 2, 2023

bors bot commented Jun 2, 2023

bors bot commented Jun 2, 2023

bors bot commented Jun 2, 2023

[Merged by Bors] - Add an option to disable inbound rate limiter #4327

[Merged by Bors] - Add an option to disable inbound rate limiter #4327

Conversation

pawanjay176 commented May 24, 2023 • edited Loading

Issue Addressed

jimmygchen left a comment

Choose a reason for hiding this comment

divagant-martian left a comment

Choose a reason for hiding this comment

divagant-martian May 29, 2023

Choose a reason for hiding this comment

divagant-martian May 29, 2023 • edited Loading

Choose a reason for hiding this comment

pawanjay176 May 30, 2023

Choose a reason for hiding this comment

paulhauner commented Jun 1, 2023

bors bot commented Jun 1, 2023

bors bot commented Jun 1, 2023

bors bot commented Jun 2, 2023

paulhauner commented Jun 2, 2023

bors bot commented Jun 2, 2023

paulhauner commented Jun 2, 2023

bors bot commented Jun 2, 2023

bors bot commented Jun 2, 2023

bors bot commented Jun 2, 2023

paulhauner commented Jun 2, 2023

bors bot commented Jun 2, 2023

michaelsproul commented Jun 2, 2023

bors bot commented Jun 2, 2023

bors bot commented Jun 2, 2023

bors bot commented Jun 2, 2023

pawanjay176 commented May 24, 2023 •

edited

Loading

divagant-martian May 29, 2023 •

edited

Loading