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

Append Announces #7791

Closed
Jorropo opened this issue Nov 26, 2020 · 5 comments · Fixed by ipfs/go-ipfs-config#135 or #8177
Closed

Append Announces #7791

Jorropo opened this issue Nov 26, 2020 · 5 comments · Fixed by ipfs/go-ipfs-config#135 or #8177
Assignees
Labels
effort/hours Estimated to take one or several hours exp/novice Someone with a little familiarity can pick up kind/enhancement A net-new feature or improvement to an existing feature P3 Low: Not priority right now status/ready Ready to be worked

Comments

@Jorropo
Copy link
Contributor

Jorropo commented Nov 26, 2020

Hi, I would like to add announces on top of the listen addrs but,
currently if len(announces) > 0 the regular addrs are replaced by the announces :
https://github.com/ipfs/go-ipfs/blob/1bf8a723f95f8ca9bd5f520b0896b7afc9658a53/core/node/libp2p/addrs.go#L55-L59

I would like to create a way for announces to be added to listen addresses, not replace them.
Wich option would be fine to add ?

  • Add an .Addresses.AppendAnnounce bool key, if true addrs = append(allAddrs,annAddrs...) else the current logic.
  • Add an .Addresses.ExtraAnnounce []ma.Multiaddr wich is always appended after the current logic.
  • Replace the current logic by addrs = append(allAddrs,annAddrs...) (breaking change ?).
@Jorropo Jorropo added the kind/enhancement A net-new feature or improvement to an existing feature label Nov 26, 2020
@aschmahmann
Copy link
Contributor

@Jorropo implementing one of these proposals seems reasonable. It would be helpful to know a little more context around the use case you're working on.

@Jorropo
Copy link
Contributor Author

Jorropo commented Dec 18, 2020

@Jorropo implementing one of these proposals seems reasonable. It would be helpful to know a little more context around the use case you're working on.

Really there is none in this case, could be someone running behind a nat without nat traversal and wanting to announce his port forward while still announcing his local addrs.

@BigLep BigLep added effort/hours Estimated to take one or several hours exp/novice Someone with a little familiarity can pick up status/ready Ready to be worked P3 Low: Not priority right now labels Mar 29, 2021
Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Jun 6, 2021
@lidel lidel removed their assignment Jun 15, 2021
@TheDiscordian
Copy link
Member

@Jorropo It would be helpful to know a little more context around the use case you're working on.

I've run into a need for this when enabling WSS. My current solution is to grab everywhere I'm announcing to, make a big list, then append my 2 new announcements to that list. It'd be a lot more intuitive to be able to just append to the usual generated list (especially with dynamic IPs...).

Explicitly, currently I'm doing this:

"Announce": ["/ip4/127.0.0.1/tcp/4001","/ip4/127.0.0.1/tcp/4011/ws","/ip4/127.0.0.1/udp/4001/quic","/ip4/172.105.103.78/tcp/4001","/ip4/172.105.103.78/tcp/4011/ws","/ip4/172.105.103.78/udp/4001/quic","/ip6/2600:3c04::f03c:92ff:fecf:988b/tcp/4001","/ip6/2600:3c04::f03c:92ff:fecf:988b/tcp/4011/ws","/ip6/2600:3c04::f03c:92ff:fecf:988b/udp/4001/quic","/ip6/::1/tcp/4001","/ip6/::1/tcp/4011/ws","/ip6/::1/udp/4001/quic","/dns4/ipfs.thedisco.zone/tcp/4430/wss","/dns6/ipfs.thedisco.zone/tcp/4430/wss"]

When I think this would make more sense:

"Announce": [],
"ExtraAnnounce": ["/dns4/ipfs.thedisco.zone/tcp/4430/wss","/dns6/ipfs.thedisco.zone/tcp/4430/wss"]

@lidel
Copy link
Member

lidel commented Jul 6, 2021

@TheDiscordian see proposed config in #8177 – would appreciate feedback if it is intuitive enough.

Jorropo added a commit to Jorropo/go-ipfs that referenced this issue Jul 19, 2021
@BigLep
Copy link
Contributor

BigLep commented Nov 12, 2021

2021-11-12 discussion: we believe this is just waiting until @lidel frees up to get this reviewed/merged.

lidel pushed a commit to Jorropo/go-ipfs that referenced this issue Nov 15, 2021
lidel added a commit that referenced this issue Nov 30, 2021
* feat: Addresses.AppendAnnounce

Closes #7791

* fix: deduplicate Swarm.Announce and AppendAnnounce

#8177 (comment)
#8177 (comment)

Co-authored-by: Marcin Rataj <lidel@lidel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours exp/novice Someone with a little familiarity can pick up kind/enhancement A net-new feature or improvement to an existing feature P3 Low: Not priority right now status/ready Ready to be worked
Projects
None yet
5 participants