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.

Sign up for GitHub

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

use a basichost, not a blankhost when constructing autonat #1257

Closed
wants to merge 2 commits into from
Closed

use a basichost, not a blankhost when constructing autonat #1257

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a52909
use a basichost, not a blankhost when constructing autonat
marten-seemann Dec 8, 2021
fdc7b1d
add a host option to disable identify, use it for the autonat host
marten-seemann Dec 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/libp2p/go-libp2p/p2p/protocol/holepunch"

autonat "github.com/libp2p/go-libp2p-autonat"
blankhost "github.com/libp2p/go-libp2p-blankhost"
discovery "github.com/libp2p/go-libp2p-discovery"
swarm "github.com/libp2p/go-libp2p-swarm"
tptu "github.com/libp2p/go-libp2p-transport-upgrader"
Expand Down Expand Up @@ -307,12 +306,16 @@ func (cfg *Config) NewNode() (host.Host, error) {
Peerstore: ps,
}

dialer, err := autoNatCfg.makeSwarm()
dialerSwarm, err := autoNatCfg.makeSwarm()
if err != nil {
h.Close()
return nil, err
}
dialerHost, err := bhost.NewHost(dialerSwarm, nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we sure there is nothing listening? We need identify only and nothing else.

Also, whats this nil we are passing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identify is always added:

go-libp2p/p2p/host/basic/basic_host.go

Lines 223 to 227 in 0f76f17

if h.disableSignedPeerRecord {
h.ids, err = identify.NewIDService(h, identify.UserAgent(opts.UserAgent), identify.DisableSignedPeerRecord())
} else {
h.ids, err = identify.NewIDService(h, identify.UserAgent(opts.UserAgent))
}

The nil is a HostOpts:

go-libp2p/p2p/host/basic/basic_host.go

Lines 113 to 155 in 0f76f17

type HostOpts struct {
// MultistreamMuxer is essential for the *BasicHost and will use a sensible default value if omitted.
MultistreamMuxer *msmux.MultistreamMuxer
// NegotiationTimeout determines the read and write timeouts on streams.
// If 0 or omitted, it will use DefaultNegotiationTimeout.
// If below 0, timeouts on streams will be deactivated.
NegotiationTimeout time.Duration
// AddrsFactory holds a function which can be used to override or filter the result of Addrs.
// If omitted, there's no override or filtering, and the results of Addrs and AllAddrs are the same.
AddrsFactory AddrsFactory
// MultiaddrResolves holds the go-multiaddr-dns.Resolver used for resolving
// /dns4, /dns6, and /dnsaddr addresses before trying to connect to a peer.
MultiaddrResolver *madns.Resolver
// NATManager takes care of setting NAT port mappings, and discovering external addresses.
// If omitted, this will simply be disabled.
NATManager func(network.Network) NATManager
// ConnManager is a libp2p connection manager
ConnManager connmgr.ConnManager
// EnablePing indicates whether to instantiate the ping service
EnablePing bool
// EnableRelayService enables the circuit v2 relay (if we're publicly reachable).
EnableRelayService bool
// RelayServiceOpts are options for the circuit v2 relay.
RelayServiceOpts []relayv2.Option
// UserAgent sets the user-agent for the host.
UserAgent string
// DisableSignedPeerRecord disables the generation of Signed Peer Records on this host.
DisableSignedPeerRecord bool
// EnableHolePunching enables the peer to initiate/respond to hole punching attempts for NAT traversal.
EnableHolePunching bool
// HolePunchingOptions are options for the hole punching service
HolePunchingOptions []holepunch.Option
}
.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also neuter the peerstore?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should pass a noop one, which we may have to shim.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "neuter"? We need some kind of peerstore, as autonat is first adding the peer info, and then connecting (since the swarm doesn't allow us to dial an address directly).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, indeed.

Can we make a variant with minimal overhead that just immediately removes everything when the peer is disconnected?
We dont need a peer manager and we dont care about caching addrs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe that should be part of autonat? The peerstore exposes a RemovePeer method, so we could call that one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that might be the right thing to do, esp if we move the pstoremgr responsibility to the host.

Only concern is we keep addrs for some 10min, as recently connected.

if err != nil {
h.Close()
return nil, err
}
dialerHost := blankhost.NewBlankHost(dialer)
if err := autoNatCfg.addTransports(dialerHost); err != nil {
dialerHost.Close()
h.Close()
Expand Down