core: make announced swarm addresses configurable#3948
Conversation
ghost
added
topic/interop
| AddrsFactory: addrsFactory, | ||
| DisableNatPortMap: cfg.Swarm.DisableNatPortMap, | ||
| } | ||
| peerhost, err := hostOption(ctx, n.Identity, n.Peerstore, n.Reporter, |
There was a problem hiding this comment.
maybe we could also have a ConstructSwarmOpts that holds all these other arguments.
There was a problem hiding this comment.
Once libp2p/go-libp2p#197 is merged I'd like to remove ConstructPeerHostOpts and use basichost.HostOpts, and would like to do the same with regard to swarm.
core/core.go
Outdated
| return func(allAddrs []ma.Multiaddr) []ma.Multiaddr { | ||
| var addrs []ma.Multiaddr | ||
| if len(annAddrs) > 0 { | ||
| addrs = annAddrs[:] |
There was a problem hiding this comment.
yeah good question :) removing that
There was a problem hiding this comment.
Oh yeah right. We build annAddrs once and cache it outside the closure, and the reslice is to prevent multiple calls to the closure from getting back the same slice. It's gonna be equal, but not the same.
|
@lgierth what is the next step here? |
ghost
force-pushed
the
feat/addresses-announce
branch
from
f003ad0 to
e040009
Compare
|
This should be good to go -- I rebased, let's see what CI says. |
|
Oh right this depends on a libp2p update. Will take care of that later today. |
|
@lgierth you should be unblocked here now :) |
|
Please rebase. |
ghost
force-pushed
the
feat/addresses-announce
branch
2 times, most recently
from
0a621b1 to
5ea91ee
Compare
|
I rebased, and looked at that reslice once more, it's gone now. Jenkins green |
Kubuxu
force-pushed
the
feat/addresses-announce
branch
from
5ea91ee to
952f658
Compare
License: MIT Signed-off-by: Lars Gierth <[email protected]>
integrate #3948 License: MIT Signed-off-by: vyzo <[email protected]>
|
@Kubuxu what was it you committed here, just another rebase? |
integrate #3948 License: MIT Signed-off-by: vyzo <[email protected]>
integrate #3948 License: MIT Signed-off-by: vyzo <[email protected]>
2 participants
Fixes #3613
Overall this is good for CR though.