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

discv5: fix potential infinite loop in randomNodes #754

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Conversation

cskiraly
Copy link
Contributor

randomNodes could do an infinite loop if used with a filter expression

  • added fix
  • added test (goes in infinite loop without the fix)

@cskiraly cskiraly changed the title fix potential infinite loop in randomNodes discv5: fix potential infinite loop in randomNodes Oct 23, 2024
@cskiraly cskiraly requested a review from kdeme October 23, 2024 20:53
@kdeme
Copy link
Contributor

kdeme commented Oct 24, 2024

Fine with merging this as it fixes your issue.

I forgot however that this randomNodes call existed still in the first place, I thought I removed that a long time ago as I didn't see a proper use case for it.

How are you using that exactly or in which use case? Because for getting random nodes from the DHT is is rather bad as it only selects from the routing table.

@cskiraly
Copy link
Contributor Author

just found it reading through the code (the codex version, actually).
I just don't like infinite loops being in the code, which is easy to achieve with a filter. As you say, the whole function could also be reimplemented, or redefined and then reimplemented.

@kdeme kdeme merged commit 30476de into master Oct 24, 2024
18 checks passed
@kdeme kdeme deleted the fix-randomNodes branch October 24, 2024 15:24
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