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

chore: update resolved enr ip when using dns4-domain-name flag #1576

Closed
alrevuelta opened this issue Feb 26, 2023 · 4 comments · Fixed by #2030
Closed

chore: update resolved enr ip when using dns4-domain-name flag #1576

alrevuelta opened this issue Feb 26, 2023 · 4 comments · Fixed by #2030
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@alrevuelta
Copy link
Contributor

alrevuelta commented Feb 26, 2023

Background

When using dns4-domain-name eg

./build/wakunode2 --dns4-domain-name=google.com --discv5-discovery=true

The node creates an ENR with ip 0.0.0.0 and the multi is constructed using that ip /ip4/0.0.0.0/tcp/60000/p2p/16Uiu2HAmFj9mVkzj81XBFTVK1475iKoPPiakdgZtoPTtPaTYqqRy.

Example of the generated ENR with the above command (decode it here)

enr:-Ki4QAnkkUPeFFRuIxB9KkPQqO9lKIgFxc84egqGM7mBj7bdPGlUxsnqERtpD1Z2i0-nVxyyDquUlPAq7-u9KcQxbNkBgmlkgnY0gmlwhAAAAACKbXVsdGlhZGRyc5EADzYKZ29vZ2xlLmNvbQbqYIlzZWNwMjU2azGhAy2kCEHyIKJ1iAyAzger0hd5kzbxgpYJyOvLFfzElg9Yg3RjcILqYIV3YWt1MgE

The thing is that since discv5 doesn't understand our multiaddrs field, this node won't be discoverable by disv5, since the address that is being advertised is 0.0.0.0.

In other protocols using libp2p and discv5 (eg Ethereum) the address that is advertised in the enr is the ip that results of resolving dns4-domain-name.

Example. In prysm using the following, will lead to an ENR that contains the IP that resolved from google.com. See logs:

docker run prysmaticlabs/prysm-beacon-chain:latest --p2p-host-dns=google.com --accept-terms-of-use

Acceptance criteria

  • When running ./build/wakunode2 --dns4-domain-name=some_domain the constructed ENR ip field contains the resolved IP instead of just 0.0.0.0.
@jm-clius jm-clius added the good first issue Good for newcomers label Mar 14, 2023
@jm-clius jm-clius moved this to Icebox in Waku Mar 20, 2023
@gabrielmer gabrielmer self-assigned this Sep 11, 2023
@gabrielmer
Copy link
Contributor

Weekly Update

  • next: analyze and fix issue

@gabrielmer gabrielmer moved this from Icebox to In Progress in Waku Sep 11, 2023
@fryorcraken fryorcraken added the enhancement New feature or request label Sep 12, 2023
@gabrielmer
Copy link
Contributor

gabrielmer commented Sep 15, 2023

Weekly Update

  • achieved: implemented solution that does DNS IP resolution during node bringup when no external IP is found but a DNS address is provided.
    Validated and tested "happy paths" of the solution, raised draft PR and got feedback about the solution
  • next: discuss and define the system's behavior on errors, implement error handling and adding tests for this feature.

@gabrielmer
Copy link
Contributor

gabrielmer commented Sep 22, 2023

Weekly Update

  • achieved: added error handling and tests, received new feedback and addressed the comments
  • next: get the new version reviewed and merge if approved

@github-project-automation github-project-automation bot moved this from In Progress to Done in Waku Sep 27, 2023
@gabrielmer
Copy link
Contributor

Weekly Update

  • achieved: addressed feedback and merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants