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

decouple eth2 from store and lighthouse_network #6680

Open
wants to merge 11 commits into
base: unstable
Choose a base branch
from

Conversation

dknopik
Copy link
Member

@dknopik dknopik commented Dec 11, 2024

Issue Addressed

Proposed Changes

Remove dependencies on store and lighthouse_network from eth2. This was achieved as follows:

  • depend on enr and multiaddr directly instead of using lighthouse_network's reexports.
  • make lighthouse_network responsible for converting between API and internal types.
  • in two cases, remove complex internal types and use the generic serde_json::Value instead - this is not ideal, but should be fine for now, as this affects two internal non-spec endpoints which are meant for debugging, unstable, and subject to change without notice anyway. Inspired by Remove lighthouse_network dependency from eth2 crate #6679. The alternative is to move all relevant types to eth2 or types instead - what do you think?

Additional Info

In my opinion, this only partially fixes the issue above, as we probably still should split out lighthouse-specific features.

Copy link

mergify bot commented Jan 8, 2025

⚠️ The sha of the head commit of this PR conflicts with #6770. Mergify cannot evaluate rules on this PR. ⚠️

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

Thanks for starting this Daniel. Left a couple comments that may help not use serde_json::Value

common/eth2/src/lighthouse.rs Outdated Show resolved Hide resolved
common/eth2/src/lighthouse.rs Outdated Show resolved Hide resolved
@dknopik
Copy link
Member Author

dknopik commented Jan 20, 2025

@jxs, applied the changes we discussed, feel free to take a look again

Copy link
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

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

LGTM Daniel, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality v6.1.0 New release c. Q1 2025
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants