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

[Libp2p] Use native Libp2p multiaddresses #3714

Merged
merged 11 commits into from
Oct 1, 2024
Merged

Conversation

rob-maron
Copy link
Collaborator

@rob-maron rob-maron commented Sep 30, 2024

This PR:

Changes Libp2p to use native multiaddressing. This allows us to use DNS and IPv6 entries directly instead of (for DNS) resolving it at runtime and using the resolved address.

This prevents the node from failing to start if the DNS entry does not yet exist, and also prevents the local resolution of a cloud address causing issues (e.g. espresso.node resolving to some AWS-local IP). It also allows for node addresses to be automatically updated through just changing the DNS entry.

To do this, we introduce the function derive_libp2p_multiaddress which derives the proper Multiaddress from a string. It supports IPv4, IPv6, and FQDNs. In the event that the configured FQDN does not exist, it will warn the user but still allow the node to start. It also includes testing for expected functionality.

Also does some cleanup, like removing the need for an orchestrator client to be created from ValidatorArgs

This PR does not:

Make any protocol-breaking changes

Key places to review:

derive_libp2p_multiaddress

How to test this PR:

I have already tested this in the sequencer and it works as expected

Copy link
Contributor

@ss-es ss-es left a comment

Choose a reason for hiding this comment

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

looks fine to me

@rob-maron rob-maron merged commit f498ea1 into main Oct 1, 2024
35 checks passed
@rob-maron rob-maron deleted the rm/libp2p-native-multiaddr branch October 1, 2024 13:44
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.

4 participants