Skip to content

chore(node/p2p): Forward Discovery Events #1495

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

Merged
merged 3 commits into from
Apr 28, 2025
Merged

Conversation

refcell
Copy link
Collaborator

@refcell refcell commented Apr 23, 2025

Description

Fixes the kona-node discovery driver to properly forward valid ENRs discovered through discv5 to the gossip driver

@Copilot Copilot AI review requested due to automatic review settings April 23, 2025 17:59
@refcell refcell added A-node Area: cl node (eq. Go op-node) handles single-chain consensus K-chore Kind: chore labels Apr 23, 2025
@refcell
Copy link
Collaborator Author

refcell commented Apr 23, 2025

📚 $\text{Stack Overview}$

Pulls submitted in this stack:

This comment was automatically generated by st.

@refcell refcell self-assigned this Apr 23, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses the issue of properly forwarding valid ENRs from the discv5 discovery driver to the gossip driver. Key changes include:

  • Adding a retry loop to continuously attempt to start the event stream.
  • Incorporating new event handling branches for both Discovered and SessionEstablished events.
  • Forwarding valid ENRs to the store and asynchronously sending them to the swarm.

Copy link

codecov bot commented Apr 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.8%. Comparing base (ac74b1b) to head (3b5a72c).
Report is 6 commits behind head on main.

✅ All tests successful. No failed tests found.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@refcell refcell force-pushed the rf/forward-discovery-enrs branch from 5cd291f to 2bd854f Compare April 23, 2025 20:29
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

check out reth's discv5 workaround for geth enrs. sigp/discv5 is much stricter about correct enr configuration for its peers than geth, so we had to ask sigp to add this special new event for being geth compatible when using sigp/discv5 for EL
https://github.com/paradigmxyz/reth/blob/dd4aa1a85b5babbcedf2a5978cd88b631bc486e8/crates/net/discv5/src/lib.rs#L255-L283

@refcell refcell force-pushed the rf/forward-discovery-enrs branch from 5524501 to 07b7de8 Compare April 28, 2025 14:43
@refcell refcell force-pushed the rf/forward-discovery-enrs branch from 07b7de8 to 8f15f7c Compare April 28, 2025 14:49
@theochap
Copy link
Member

Looks great thanks for taking care of it!

@refcell refcell added this pull request to the merge queue Apr 28, 2025
Merged via the queue into main with commit aa1b972 Apr 28, 2025
20 checks passed
@refcell refcell deleted the rf/forward-discovery-enrs branch April 28, 2025 17:56
@github-project-automation github-project-automation bot moved this from In Review to Done in Project Tracking Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-node Area: cl node (eq. Go op-node) handles single-chain consensus K-chore Kind: chore
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants