refactor(nodebuilder/p2p): Refactor NATStatus implementation

nodebuilder/p2p/host.go

@@ -17,6 +17,8 @@ import (
 	routedhost "github.com/libp2p/go-libp2p/p2p/host/routed"
 	"github.com/libp2p/go-libp2p/p2p/net/conngater"
 	"go.uber.org/fx"
+
+	"github.com/celestiaorg/celestia-node/nodebuilder/node"
 )
 
 // RoutedHost constructs a wrapped Host that may fallback to address discovery,
@@ -45,9 +47,8 @@ func Host(cfg Config, params hostParams, bw *metrics.BandwidthCounter, rm networ
 		libp2p.DefaultMuxers,
 	}
 
-	// TODO(@Wondertan): Other, non Celestia bootstrapper may also enable NATService to contribute the
-	// network.
-	if cfg.Bootstrapper {
+	// All node types except light (bridge, full) will enable NATService
+	if params.Tp != node.Light {
 		opts = append(opts, libp2p.EnableNATService())
 	}
 
@@ -76,4 +77,6 @@ type hostParams struct {
 	PStore    peerstore.Peerstore
 	ConnMngr  connmgr.ConnManager
 	ConnGater *conngater.BasicConnectionGater
+
+	Tp node.Type
 }

nodebuilder/p2p/p2p.go

@@ -4,14 +4,13 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/libp2p/go-libp2p-core/host"
 	"github.com/libp2p/go-libp2p-core/metrics"
 	"github.com/libp2p/go-libp2p-core/network"
 	"github.com/libp2p/go-libp2p-core/peer"
 	"github.com/libp2p/go-libp2p-core/protocol"
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
 	rcmgr "github.com/libp2p/go-libp2p-resource-manager"
-	"github.com/libp2p/go-libp2p/p2p/host/autonat"
+	basichost "github.com/libp2p/go-libp2p/p2p/host/basic"
 	"github.com/libp2p/go-libp2p/p2p/net/conngater"
 	ma "github.com/multiformats/go-multiaddr"
 )
@@ -27,7 +26,7 @@ type API struct {
 	Connect              func(ctx context.Context, pi peer.AddrInfo) error
 	ClosePeer            func(id peer.ID) error
 	Connectedness        func(id peer.ID) network.Connectedness
-	NATStatus            func() network.Reachability
+	NATStatus            func() (network.Reachability, error)
 	BlockPeer            func(p peer.ID) error
 	UnblockPeer          func(p peer.ID) error
 	ListBlockedPeers     func() []peer.ID
@@ -62,7 +61,7 @@ type Module interface {
 	// Connectedness returns a state signaling connection capabilities.
 	Connectedness(id peer.ID) network.Connectedness
 	// NATStatus returns the current NAT status.
-	NATStatus() network.Reachability
+	NATStatus() (network.Reachability, error)
 
 	// BlockPeer adds a peer to the set of blocked peers.
 	BlockPeer(p peer.ID) error
@@ -105,24 +104,21 @@ type Module interface {
 type manager struct {
 	host      HostBase
 	ps        *pubsub.PubSub
-	nat       autonat.AutoNAT
 	connGater *conngater.BasicConnectionGater
 	bw        *metrics.BandwidthCounter
 	rm        network.ResourceManager
 }
 
 func newManager(
-	host host.Host,
+	host HostBase,
 	ps *pubsub.PubSub,
-	nat autonat.AutoNAT,
 	cg *conngater.BasicConnectionGater,
 	bw *metrics.BandwidthCounter,
 	rm network.ResourceManager,
 ) Module {
 	return &manager{
 		host:      host,
 		ps:        ps,
-		nat:       nat,
 		connGater: cg,
 		bw:        bw,
 		rm:        rm,
@@ -164,8 +160,18 @@ func (m *manager) Connectedness(id peer.ID) network.Connectedness {
 	return m.host.Network().Connectedness(id)
 }
 
-func (m *manager) NATStatus() network.Reachability {
-	return m.nat.Status()
+func (m *manager) NATStatus() (network.Reachability, error) {
+	basic, ok := m.host.(*basichost.BasicHost)
+	if !ok {
+		return 0, fmt.Errorf("does not impl") // todo
+	}
+	// return a nice error if autonat is not enabled (e.g. light nodes
+	// do not have autonat enabled)
+	// TODO does this even happen where autonat is nil?
+	if basic.GetAutoNat() == nil {
+		return 0, fmt.Errorf("does not have") // todo
+	}
+	return basic.GetAutoNat().Status(), nil
 }
 
 func (m *manager) BlockPeer(p peer.ID) error {

nodebuilder/p2p/p2p_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/libp2p/go-libp2p-core/network"
 	"github.com/libp2p/go-libp2p-core/protocol"
 	pubsub "github.com/libp2p/go-libp2p-pubsub"
-	"github.com/libp2p/go-libp2p/p2p/host/autonat"
 	mocknet "github.com/libp2p/go-libp2p/p2p/net/mock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -26,7 +25,7 @@ func TestP2PModule_Host(t *testing.T) {
 	require.NoError(t, err)
 	host, peer := net.Hosts()[0], net.Hosts()[1]
 
-	mgr := newManager(host, nil, nil, nil, nil, nil)
+	mgr := newManager(host, nil, nil, nil, nil)
 
 	// test all methods on `manager.host`
 	assert.Equal(t, host.ID(), mgr.Info().ID)
@@ -51,7 +50,7 @@ func TestP2PModule_ConnManager(t *testing.T) {
 	peer, err := libp2p.New()
 	require.NoError(t, err)
 
-	mgr := newManager(host, nil, nil, nil, nil, nil)
+	mgr := newManager(host, nil, nil, nil, nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
@@ -68,16 +67,15 @@ func TestP2PModule_ConnManager(t *testing.T) {
 // TestP2PModule_Autonat tests P2P Module methods on
 // the node's instance of AutoNAT.
 func TestP2PModule_Autonat(t *testing.T) {
-	net, err := mocknet.FullMeshConnected(2)
-	require.NoError(t, err)
-	host := net.Hosts()[0]
-
-	nat, err := autonat.New(host)
+	host, err := libp2p.New(libp2p.EnableNATService())
 	require.NoError(t, err)
 
-	mgr := newManager(host, nil, nat, nil, nil, nil)
+	mgr := newManager(host, nil, nil, nil, nil)
 
-	assert.Equal(t, nat.Status(), mgr.NATStatus())
+	status, err := mgr.NATStatus()
+	assert.NoError(t, err)
+	// TODO ??????
+	t.Log(status)
 }
 
 // TestP2PModule_Bandwidth tests P2P Module methods on
@@ -105,7 +103,7 @@ func TestP2PModule_Bandwidth(t *testing.T) {
 		require.NoError(t, err)
 	})
 
-	mgr := newManager(host, nil, nil, nil, bw, nil)
+	mgr := newManager(host, nil, nil, bw, nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
@@ -159,7 +157,7 @@ func TestP2PModule_Pubsub(t *testing.T) {
 	gs, err := pubsub.NewGossipSub(ctx, host)
 	require.NoError(t, err)
 
-	mgr := newManager(host, gs, nil, nil, nil, nil)
+	mgr := newManager(host, gs, nil, nil, nil)
 
 	topicStr := "test-topic"
 
@@ -193,7 +191,7 @@ func TestP2PModule_ConnGater(t *testing.T) {
 	gater, err := ConnectionGater(datastore.NewMapDatastore())
 	require.NoError(t, err)
 
-	mgr := newManager(nil, nil, nil, gater, nil, nil)
+	mgr := newManager(nil, nil, gater, nil, nil)
 
 	assert.NoError(t, mgr.BlockPeer("badpeer"))
 	assert.Len(t, mgr.ListBlockedPeers(), 1)
@@ -207,7 +205,7 @@ func TestP2PModule_ResourceManager(t *testing.T) {
 	rm, err := ResourceManager()
 	require.NoError(t, err)
 
-	mgr := newManager(nil, nil, nil, nil, nil, rm)
+	mgr := newManager(nil, nil, nil, nil, rm)
 
 	state, err := mgr.ResourceState()
 	require.NoError(t, err)