-
Notifications
You must be signed in to change notification settings - Fork 100
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
feat(p2p): limit inbound connections #1919
base: stage
Are you sure you want to change the base?
Conversation
// inboundRatio is the ratio of inbound connections to outbound connections | ||
inboundRatio = float64(0.8) |
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.
It's probably inboundShare
or inboundFraction
and not inboundRatio
(which would imply totalInboud = 0.8 * totalOutbound or something similar)
network/p2p/p2p_setup.go
Outdated
if n.idx == nil { | ||
return false | ||
} |
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.
Could maybe just use n.IsBadPeer
(instead of this and n.idx.IsBad(lg, peerID)
below)
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.
Couple more suggestions
if n.isBadPeer(n.logger, id) { | ||
if n.isBadOutbound(n.logger, id) { | ||
n.logger.Debug("preventing outbound connection due to bad peer", fields.PeerID(id)) | ||
return false | ||
} |
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.
doesn't InterceptAddrDial
seem a better place to check for this ?
if n.isBadPeer(n.logger, id) { | ||
n.logger.Debug("rejecting inbound connection due to bad peer", fields.PeerID(id)) | ||
if direction == libp2pnetwork.DirUnknown { | ||
return false | ||
} | ||
return true | ||
|
||
if direction == libp2pnetwork.DirOutbound { | ||
return n.isBadOutbound(n.logger, id) | ||
} else { | ||
return n.isBadInbound(n.logger, id) | ||
} |
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.
And here we essentially do isBadOutbound
check 2nd time (because the first time, I think, we did it in InterceptAddrDial
), which maybe means - instead of all this added code to InterceptSecured
we maybe should just check isBadInbound
in InterceptAccept
?
Although in InterceptAccept
we don't have peerID
looks like ...
Description
This code calculates the amount of inbound connections we have on every new connection and makes sure we're not passing the limit of 80% of our max peers in inbound connections. if we are at the limit we just drop the peer.