Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

Treat /dns4 addresses as public #377

Closed
joeltg opened this issue Oct 5, 2022 · 4 comments · Fixed by #406
Closed

Treat /dns4 addresses as public #377

joeltg opened this issue Oct 5, 2022 · 4 comments · Fixed by #406
Assignees
@joeltg
Labels
exp/beginner Can be confidently tackled by newcomers help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) released

Comments

@joeltg
Copy link
Contributor

joeltg commented Oct 5, 2022

Right now, both removePrivateAddresses and removePublicAddresses in src/utils.ts remove all multiaddrs that aren't /ip4 or /ip6.

Is there any reason we couldn't leave /dns4 addresses as public? The situation I'm in is that I'm deploying some bootstrap nodes on fly.io, which gives me a predictable DNS name but a dynamically-allocated ipv4, so I'd like to announce on /dns4/my-app-name.fly.dev/tcp/....
@mpetrunic
Copy link
Member

Triage notes: It's something we need @achingbrain input on

@mpetrunic mpetrunic moved this to Needs Investigation in js-libp2p Nov 15, 2022
@achingbrain achingbrain added kind/bug A bug in existing code (including security flaws) help wanted Seeking public contribution on this issue exp/beginner Can be confidently tackled by newcomers labels Nov 29, 2022
@achingbrain
Copy link
Member

This sounds like something that should be possible - would you like to submit a PR with a test?

@achingbrain achingbrain self-assigned this Nov 29, 2022
@joeltg joeltg mentioned this issue Nov 29, 2022
Merged
@justin0mcateer
Copy link

I also ran into this issue, I describe my use case and provide a small working (but not complete) patch in #407

@justin0mcateer justin0mcateer mentioned this issue Dec 5, 2022
Closed
@p-shahi p-shahi assigned joeltg and unassigned achingbrain Dec 6, 2022
@p-shahi p-shahi moved this from Needs Investigation to In Review in js-libp2p Dec 6, 2022
@p-shahi p-shahi linked a pull request Dec 6, 2022 that will close this issue
fix: treat /dns, /dns4, and /dns6 addrs as public #406
Merged
@p-shahi p-shahi removed this from js-libp2p Dec 6, 2022
github-actions bot pushed a commit that referenced this issue Dec 7, 2022
@semantic-release-bot
chore(release): 6.1.0 [skip ci]
aeaec5d 
## [6.1.0](v6.0.4...v6.1.0) (2022-12-07)

### Features

* allow passing ProvidersInit in KadDHT constructor ([#404](#404)) ([e64af85](e64af85))

### Bug Fixes

* treat /dns, /dns4, and /dns6 addrs as public ([#406](#406)) ([e27747a](e27747a)), closes [#377](#377)
@github-actions
Copy link

github-actions bot commented Dec 7, 2022

🎉 This issue has been resolved in version 6.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@joeltg joeltg

Labels
exp/beginner Can be confidently tackled by newcomers help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) released
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: treat /dns, /dns4, and /dns6 addrs as public
4 participants
@achingbrain @justin0mcateer @joeltg @mpetrunic