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

Change defaultDialRatio from 3 to 2 #29737

Closed
wants to merge 3 commits into from

Conversation

infosecual
Copy link

@infosecual infosecual commented May 8, 2024

This PR changes defaultDialRatio which limits incoming peer connections to a maximum ratio of total connections. When this ratio is 3 there are a few downstream consequences:

  • In some cases members of a geth-only network will not be able to reach their MaxPeers limit. Imagine a testnet with 10 geth nodes and each node being configured to MaxPeers == 9 to peer all peers. Each node will only be able to accept 3 incoming peers total. This would mean that some amount of peering would not be able to happen and not all nodes would be able to connect. You can imagine how this would cause issue with a small network of 3 peers if MaxPeers == 5 and there was no minimum peer setting set. Each geth node would only be able to find one outgoing peer because of the incoming peer limit that would result. In a perfect situtation all peers would have a connection with eachother since the total number of participants is less than MaxPeers. This low-peer scenario is easier to think about but the same issue holds true for MaxPeers == 50.
  • Testnets and other networks where people are less likely to open their firewall ports cause many participants to be out-bound-only peers. This will add to the issue where geth nodes will have a more difficult time finding peers. This is because geth will require more outgoing peers than incoming when defaultDialRatio == 3 and the network will be skewed to less nodes accepting geth's outgoing peer requests. While these peers would happily connect to a geth node that has open ports and the geth node could be under its peer limit, the geth node will decline incoming connections due to the defaultDialRatio. I believe that this is what was making it more difficult to for other clients to find geth peers on Goerli and Holesky.

This PR also fixes a small type in a fuzzing function. I did modify the RLP fuzzers and fuzz quite a bit but did not find any new issues so I have left those changes out.

@fjl
Copy link
Contributor

fjl commented May 8, 2024

Imagine a testnet with 10 geth nodes and each node being configured to MaxPeers == 9 to peer all peers. Each node will only be able to accept 3 incoming peers total.

This is not true because dial ratio works in the opposite way. With the default value (3), each node will dial out a maximum of 9 / 3 = 3 connections, and allow for 6 inbound connections.

@infosecual
Copy link
Author

I stand corrected. The ratio is inverted from the way I was reading it!

@infosecual infosecual closed this May 8, 2024
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.

None yet

2 participants