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

eth, les: Allow exceeding maxPeers for trusted peers #16189

Merged
merged 1 commit into from
Feb 27, 2018

Conversation

shazow
Copy link
Contributor

@shazow shazow commented Feb 25, 2018

Trusted peers are supposed to be able to bypass the maxPeers limit. This logic was appropriately executed in p2p/server.go, but the eth handler and les handler also check maxPeers which were previously ignoring the trusted flag for peers.

I tested this locally with some empty blockchains (I'm on bad wifi, can't complete a sync right now), and the "too many peers" error did indeed go away after this fix.

Let me know if there is a good place to write a test for this, or if this is good enough as is (fairly simple change).

Fixes #3326, #14472

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

Seems good to me, but please merge the ifs:

if pm.peers.Len() >= pm.maxPeers && !p.Peer.Info().Network.Trusted {
	return p2p.DiscTooManyPeers
}

@karalabe karalabe added this to the 1.8.2 milestone Feb 26, 2018
@shazow
Copy link
Contributor Author

shazow commented Feb 26, 2018

@karalabe forcepushed, PTAL.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@karalabe karalabe merged commit 2e9c8fd into ethereum:master Feb 27, 2018
@shazow
Copy link
Contributor Author

shazow commented Feb 27, 2018

My pleasure! 🍮

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.

cannot connect to static/trusted peers with --maxpeers 0
2 participants