Skip to content

Conversation

cskiraly
Copy link
Contributor

Refresh is doing some lookups and thus it could block for some time. We do not want the initializer of an iterator to block. If there is something blocking, it should happen when calling Next.

Here, next will start a lookup, which will wait if needed (no nodes), making sure the iterator's Next is not creating a busy loop.

Refresh is doing some lookups and thus it could block for some time.
We do not want the initialized of an iterator to block. If there is
something blocking, it should happen when calling Next.

Here, next will start a lookup, which will wait if needed (no nodes),
making sure the iterator's Next is not creating a busy loop.

Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
@cskiraly cskiraly marked this pull request as ready for review August 29, 2025 13:21
@cskiraly
Copy link
Contributor Author

This fixes ethereum/hive#1335

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

SGTM. discv4 does not refresh when calling RandomNodes(), I don't see a particular reason for v5 to. The table does also request a initial refresh upon ListenV5(..).

@lightclient
Copy link
Member

(seems like Felix prefers to go with something like #32518 though)

@fjl fjl changed the title p2p/discover: remove delay from iterator init p2p/discover: remove delay from discv5 RandomNodes Sep 10, 2025
@fjl fjl merged commit 1f7f95d into ethereum:master Sep 10, 2025
8 of 10 checks passed
Sahil-4555 pushed a commit to Sahil-4555/go-ethereum that referenced this pull request Oct 12, 2025
Refresh is doing some lookups and thus it could block for some time. We
do not want the initializer of an iterator to block. If there is
something blocking, it should happen when calling Next.

Here, next will start a lookup, which will wait if needed (no nodes),
making sure the iterator's Next is not creating a busy loop.

Signed-off-by: Csaba Kiraly <csaba.kiraly@gmail.com>
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.

3 participants