-
Notifications
You must be signed in to change notification settings - Fork 20.2k
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
les: fix UDP connection query #22451
Conversation
les/client.go
Outdated
@@ -116,7 +116,7 @@ func New(stack *node.Node, config *ethconfig.Config) (*LightEthereum, error) { | |||
} | |||
|
|||
var prenegQuery vfc.QueryFunc | |||
if leth.p2pServer.DiscV5 != nil { | |||
if stack.Config().P2P.DiscoveryV5 { |
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 think we should use this check in all other places. It's true that the discV5 handler will be initialized later if the v5 is enabled, but it's so weird we use different check methods in different places.
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.
added a flag and an extra check with an error log just to be sure
Otherwise, lgtm |
This PR fixes multiple issues with the UDP connection pre-negotiation feature: - the enable condition was wrong (it checked the existence of the DiscV5 struct where it wasn't initialized yet, disabling the feature even if discv5 was enabled) - the server pool queried already connected nodes when the discovery iterators returned them again - servers responded positively before they were synced and really willing to accept connections Metrics are also added on the server side that count the positive and negative replies to served connection queries.
This PR fixes multiple issues with the UDP connection pre-negotiation feature: - the enable condition was wrong (it checked the existence of the DiscV5 struct where it wasn't initialized yet, disabling the feature even if discv5 was enabled) - the server pool queried already connected nodes when the discovery iterators returned them again - servers responded positively before they were synced and really willing to accept connections Metrics are also added on the server side that count the positive and negative replies to served connection queries.
This PR fixes multiple issues with the UDP connection pre-negotiation feature: - the enable condition was wrong (it checked the existence of the DiscV5 struct where it wasn't initialized yet, disabling the feature even if discv5 was enabled) - the server pool queried already connected nodes when the discovery iterators returned them again - servers responded positively before they were synced and really willing to accept connections Metrics are also added on the server side that count the positive and negative replies to served connection queries.
This PR fixes multiple issues with the UDP connection pre-negotiation feature:
Metrics are also added on the server side that count the positive and negative replies to served connection queries.