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

fix: a few edge cases for find_enr() + distinction of same logs #1350

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

KolbyML
Copy link
Member

@KolbyML KolbyML commented Jul 31, 2024

What was wrong?

2024-07-31T04:32:12.149516Z ERROR trin_history::events: Error processing portal history request, responding with empty TALKRESP error=Error while building accept message: unable to find ENR for NodeId request.discv5.id=75d0ab97e1527ba1

Which is still popping up just a lot less, I think this is due to a few different edge cases

Also it is annoying because we use the same log in 2 different functions

How was it fixed?

  • making the same logs more distinct
  • remove redundant find_enr() call
  • add a few more possible cases to find_enr()

@KolbyML KolbyML changed the title fix: fix a few edge cases for find_enr() + distinction of same logs fix: a few edge cases for find_enr() + distinction of same logs Jul 31, 2024
@@ -948,7 +948,7 @@ where
// Generate a connection ID for the uTP connection.
let enr = self.find_enr(source).ok_or_else(|| {
OverlayRequestError::AcceptError(
"unable to find ENR for NodeId".to_string(),
"handle_offer_content: unable to find ENR for NodeId".to_string(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be handle_find_content? that's the function we're inside at this point

@@ -2497,6 +2496,14 @@ where
return Some(entry.value().clone().enr());
}

if let kbucket::Entry::Pending(mut entry, _) = self.kbuckets.write().entry(&key) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd add a comment explaining the purpose/motivation for checking kbuckets before find_enr

@KolbyML KolbyML merged commit 9b00119 into ethereum:master Aug 2, 2024
8 checks passed
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