Skip to content

p2p: Filter Discv4 dial candidates based on forkID#31592

Closed
cskiraly wants to merge 9 commits intoethereum:masterfrom
cskiraly:discv4-filter
Closed

p2p: Filter Discv4 dial candidates based on forkID#31592
cskiraly wants to merge 9 commits intoethereum:masterfrom
cskiraly:discv4-filter

Conversation

@cskiraly
Copy link
Copy Markdown
Contributor

@cskiraly cskiraly commented Apr 8, 2025

Our Discovery/v4 based dial source was returning nodes without checking the forkid in the ENR, leading to lots of useless dials.

This PR adds a filter on top of the discovery/v4 stream of discovered nodes, returning only those that have a compatible forkID.

More in detail:

  • to check the ENR, we need to retrieve it from the node first using an extra Discovery/v4 query. We add AsyncFilterIterator, which can filter a stream of nodes using a blocking filter function, doing several async checks in parallel.
  • we set the AsyncFilterIterator to use the RetrieveENR function as both filter (retrieve could time out) and a modifier of the Node.
  • we apply another filter on forkID, as we do in Discovery/v5.
  • finally, we simplfy the mixing of discovery sources by moving the Discovery/v4 source from the Server to the Backend, next to the Discovery/v5 source.

Note: This PR is to be rebased after merging #31588

@cskiraly
Copy link
Copy Markdown
Contributor Author

I've kept the forkID based filter separate from the ENR resolver for code clarity. These could eventually be combined, but I think the code becomes less clear, and the difference in performance should be negligible.

Comment thread eth/backend.go Outdated
cskiraly added 9 commits June 1, 2025 11:38
Simple version that does the filtering, but misses pipelining,
waiting for ENRs to be retrieved one-by-one.

Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
It is not guaranteed that Next will be called until exhaustion
after Close was called. Hence, we need to empty override the
passed channel.

Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>

# Conflicts:
#	eth/backend.go
Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
@cskiraly
Copy link
Copy Markdown
Contributor Author

cskiraly commented Jun 1, 2025

rebased on current master

@fjl
Copy link
Copy Markdown
Contributor

fjl commented Jun 5, 2025

Turns out we already added the filter in #31944, so I guess this can be closed now.

@fjl fjl closed this Jun 5, 2025
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.

2 participants