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

RPC: fix recursiveNodeLookup #669

Merged
merged 10 commits into from
Nov 6, 2024
Merged

RPC: fix recursiveNodeLookup #669

merged 10 commits into from
Nov 6, 2024

Conversation

ScottyPoi
Copy link
Collaborator

@ScottyPoi ScottyPoi commented Nov 5, 2024

Bug unconvered through use of glados-cartographer

portal_*RecursiveFindContent was not returning the expected array of ENRs.

Changes:

  1. NodeLookup now takes an optional refresh parameter
  • if true, NodeLookup will exit early when the target bucket is full
  • set to true in bucketRefresh
  1. NodeLookup now tracks all found ENRs in a Heap called this.foundNodes
  • Heap is automatically sorted by distance to target
  1. NodeLookup.startLookup() now returns the closest 16 ENRs from foundNodes heap
  • As ENR string array (sorted)
  1. RPC methods portal_*RecursiveFindNodes now returns ENR list from NodeLookup.startLookup()

This PR also implements the same method for state and beacon networks, which were previously without this method.

@ScottyPoi ScottyPoi marked this pull request as draft November 5, 2024 22:44
@ScottyPoi ScottyPoi marked this pull request as ready for review November 5, 2024 23:08
@ScottyPoi
Copy link
Collaborator Author

I didn't realize that this method was used by Glados. We otherwise only use the NodeLookup class during bucketRefresh, and recent updates to NodeLookup were optimized for this purpose. These optimizations may not be ideal for the glados-cartographer use of NodeLookup, which may be interested in tallying ENRs that exist in the network outside of the local routing table.

@ScottyPoi ScottyPoi marked this pull request as draft November 5, 2024 23:20
@ScottyPoi ScottyPoi marked this pull request as ready for review November 6, 2024 00:15
Copy link
Collaborator

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

I also added some fixes for the *traceGetContent work in the contentLookup folder related to glados-audit

@acolytec3 acolytec3 merged commit acfc73f into master Nov 6, 2024
4 of 5 checks passed
@acolytec3 acolytec3 deleted the rpc-nodelookup branch November 6, 2024 18:18
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