Extract service implementation #2
Conversation
ghost
assigned 
vyzo
force-pushed
the
feat/extract-svc
branch
from
3260d67
to
b1b8425
Compare
|
I'm going to apply my "someone else reviews this first" OKR (unless this is really burning). |
|
we will need it to merge autorelay. |
|
|
||
| return h, a | ||
| } | ||
|
|
||
| // Note: these tests assume the host has only private inet addresses! | ||
| func connect(t *testing.T, a, b host.Host) { | ||
| pinfo := pstore.PeerInfo{ID: a.ID(), Addrs: a.Addrs()} |
There was a problem hiding this comment.
Maybe we should move the PeerInfo struct to somewhere more common. It's a pity to have to import peerstore as a client, even if you don't intend to use the peerstore itself.
Added a note about this: https://github.com/libp2p/go-libp2p-peerstore/issues/35.
There was a problem hiding this comment.
yeah, this gets to me every single time as peerstore is a pretty big dependency.
Maybe we should have it in go-libp2p-peer?
There was a problem hiding this comment.
Yeah. I think host is a better place as peer doesn't depend on multiaddrs. Noted for the (hopefully near) future.
autonat.go
Outdated
| func NewAutoNAT(ctx context.Context, h host.Host) AutoNAT { | ||
| func NewAutoNAT(ctx context.Context, h host.Host, ga ...GetAddrs) AutoNAT { | ||
| getAddrs := h.Addrs | ||
| if len(ga) > 0 { |
There was a problem hiding this comment.
Is there a reason we decided against the customary Option approach? I prefer that approach, as this appears very limiting in the future.
There was a problem hiding this comment.
turned into static argument per Magik's suggestion.
client.go
Outdated
| func NewAutoNATClient(h host.Host, ga ...GetAddrs) AutoNATClient { | ||
| getAddrs := h.Addrs | ||
| if len(ga) > 0 { | ||
| getAddrs = ga[0] |
There was a problem hiding this comment.
Could we get test coverage for this?
There was a problem hiding this comment.
it's covered in libp2p/go-libp2p-autonat-svc#1 which has an actual service implementation.
It's also implicitly covered in autonat_test which uses it internally.
Extract the service implementation from the package, so that there is no dependency on go-libp2p; this allows us to import the package in go-libp2p itself.
The service implementation will live in https://github.com/libp2p/go-libp2p-autonat-svc.
Also extracts IsPublicAddr to go-multiaddr-net.