-
Notifications
You must be signed in to change notification settings - Fork 388
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
Improve connected peer distribution #7026
base: master
Are you sure you want to change the base?
Conversation
if (node.Address.Address == _localhost) | ||
{ | ||
if (_logger.IsTrace) | ||
_logger.Trace($"Received localhost as node address from: {msg.FarPublicKey}, node: {node}"); | ||
continue; | ||
} | ||
else if (!NetworkStorage.NodesFilter.Set(node.Address.Address)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should be better placed in DiscoveryManager
.
@@ -17,10 +20,13 @@ namespace Nethermind.Network | |||
{ | |||
public class NetworkStorage : INetworkStorage | |||
{ | |||
public static LruKeyCache<IPAddress> NodesFilter { get; } = new(2048, "Ip Filter"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please refrain from static. It breaks encapsulation.
Might be better to store both ports. There is no agreement about having same ports; same ports may be easier to detect by ISP/bad actors |
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?