-
Notifications
You must be signed in to change notification settings - Fork 132
feat(l1): periodically fetch node records from peers. #5446
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
Conversation
* uses similar flow as lookup * store node records for future purposes
| } | ||
|
|
||
| /// Record a response received. Check previously saved hash and reset it if it matches | ||
| pub async fn record_enr_response_received( |
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 this function is not used (and so, neither the RecordEnrResponseReceived message) because when we receive a ENRResponse we should record it directly (set_node_record), right?
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.
makes sense
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've instead removed set_node_record and record_enr_response_received would set it directly after matching the hash.
| .try_into() | ||
| .expect("first 32 bytes are the message hash"); | ||
|
|
||
| if self.udp_socket.send_to(&buf, contact.node.udp_addr()) |
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.
Now it seems we have two kind of messages that save the hash. We may want to refactor it to have a single send_and_get_hash function (or some better naming 😬) to implement this logic only once.
It can be done on a different PR, though
ElFantasma
left a comment
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.
This looks good to me
Thanks Lakshya for your contribution!
|
|
||
| pub n_find_node_sent: u64, | ||
| /// ENR associated with this contact, if it was provided by the peer. | ||
| pub record: Option<NodeRecord>, |
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.
We should set this to None if we receive a PING with a different seq-num. I opened issue #5488 for that
Motivation
Description
Towards #5423