rcmgr: Backwards compatibility if you wrap default impl

libp2p_test.go

@@ -7,14 +7,20 @@ import (
 	"fmt"
 	"regexp"
 	"testing"
+	"time"
 
 	"github.com/libp2p/go-libp2p/core/connmgr"
 	"github.com/libp2p/go-libp2p/core/crypto"
+	"github.com/libp2p/go-libp2p/core/event"
 	"github.com/libp2p/go-libp2p/core/host"
+	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/libp2p/go-libp2p/core/peerstore"
 	"github.com/libp2p/go-libp2p/core/routing"
 	"github.com/libp2p/go-libp2p/core/transport"
+	rcmgr "github.com/libp2p/go-libp2p/p2p/host/resource-manager"
 	"github.com/libp2p/go-libp2p/p2p/net/swarm"
+	"github.com/libp2p/go-libp2p/p2p/protocol/ping"
 	"github.com/libp2p/go-libp2p/p2p/security/noise"
 	tls "github.com/libp2p/go-libp2p/p2p/security/tls"
 	quic "github.com/libp2p/go-libp2p/p2p/transport/quic"
@@ -393,3 +399,48 @@ func TestMain(m *testing.M) {
 		goleak.IgnoreAnyFunction("github.com/jackpal/go-nat-pmp.(*Client).GetExternalAddress"),
 	)
 }
+
+func TestDialCircuitAddrWithWrappedResourceManager(t *testing.T) {
+	relay, err := New(EnableRelayService())
+	require.NoError(t, err)
+	defer relay.Close()
+
+	// Fake that the relay is publicly reachable
+	emitterForRelay, err := relay.EventBus().Emitter(&event.EvtLocalReachabilityChanged{})
+	require.NoError(t, err)
+	defer emitterForRelay.Close()
+	emitterForRelay.Emit(event.EvtLocalReachabilityChanged{Reachability: network.ReachabilityPublic})
+
+	peerBehindRelay, err := New(EnableAutoRelayWithStaticRelays([]peer.AddrInfo{{ID: relay.ID(), Addrs: relay.Addrs()}}))
+	require.NoError(t, err)
+	defer peerBehindRelay.Close()
+	// Emit an event to tell this peer it is private
+	emitterForPeerBehindRelay, err := peerBehindRelay.EventBus().Emitter(&event.EvtLocalReachabilityChanged{})
+	require.NoError(t, err)
+	defer emitterForPeerBehindRelay.Close()
+	emitterForPeerBehindRelay.Emit(event.EvtLocalReachabilityChanged{Reachability: network.ReachabilityPrivate})
+
+	// Use a wrapped resource manager to test that the circuit dialing works
+	// with it. Look at the PR introducing this test for context
+	type wrappedRcmgr struct{ network.ResourceManager }
+	mgr, err := rcmgr.NewResourceManager(rcmgr.NewFixedLimiter(rcmgr.DefaultLimits.AutoScale()))
+	require.NoError(t, err)
+	wmgr := wrappedRcmgr{mgr}
+	h, err := New(ResourceManager(wmgr))
+	require.NoError(t, err)
+	defer h.Close()
+
+	h.Peerstore().AddAddrs(relay.ID(), relay.Addrs(), 10*time.Minute)
+	h.Peerstore().AddAddr(peerBehindRelay.ID(),
+		ma.StringCast(
+			fmt.Sprintf("/p2p/%s/p2p-circuit", relay.ID()),
+		),
+		peerstore.TempAddrTTL,
+	)
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	res := <-ping.Ping(ctx, h, peerBehindRelay.ID())
+	require.NoError(t, res.Error)
+	defer cancel()
+}

p2p/host/resource-manager/rcmgr.go

@@ -316,14 +316,27 @@ func (r *resourceManager) nextStreamId() int64 {
 	return r.streamId
 }
 
-// OpenConnectionNoIP is like OpenConnection, but does not use IP information.
-// Used when we still want to limit the connection by other scopes, but don't
-// have IP information like when we are relaying.
-func (r *resourceManager) OpenConnectionNoIP(dir network.Direction, usefd bool, endpoint multiaddr.Multiaddr) (network.ConnManagementScope, error) {
-	return r.openConnection(dir, usefd, endpoint, netip.Addr{})
+func hasIPInMultiaddr(addr multiaddr.Multiaddr) bool {
+	foundIP := false
+	multiaddr.ForEach(addr, func(c multiaddr.Component) bool {
+		switch c.Protocol().Code {
+		case multiaddr.P_IP4, multiaddr.P_IP6:
+			foundIP = true
+			return false
+		default:
+			return true
+		}
+	})
+	return foundIP
 }
 
 func (r *resourceManager) OpenConnection(dir network.Direction, usefd bool, endpoint multiaddr.Multiaddr) (network.ConnManagementScope, error) {
+	if !hasIPInMultiaddr(endpoint) {
+		// We don't have IP information but we still want to limit the
+		// connection by other scopes.
+		return r.openConnection(dir, usefd, endpoint, netip.Addr{})
+	}
+
 	ip, err := manet.ToIP(endpoint)
 	if err != nil {
 		return nil, err

p2p/protocol/circuitv2/client/transport.go

@@ -48,23 +48,8 @@ func AddTransport(h host.Host, upgrader transport.Upgrader) error {
 var _ transport.Transport = (*Client)(nil)
 var _ io.Closer = (*Client)(nil)
 
-// If the resource manager supports OpenConnectionNoIP, we'll use it to open connections.
-// That's because the swarm is already limiting by IP address at the swarm
-// level, and here we want to limit by everything but the IP.
-// Some ResourceManager implementations may not care about IP addresses, so we
-// do our own interface check to see if they provide this option.
-type rcmgrOpenConnectionNoIPer interface {
-	OpenConnectionNoIP(network.Direction, bool, ma.Multiaddr) (network.ConnManagementScope, error)
-}
-
 func (c *Client) Dial(ctx context.Context, a ma.Multiaddr, p peer.ID) (transport.CapableConn, error) {
-	var connScope network.ConnManagementScope
-	var err error
-	if rcmgr, ok := c.host.Network().ResourceManager().(rcmgrOpenConnectionNoIPer); ok {
-		connScope, err = rcmgr.OpenConnectionNoIP(network.DirOutbound, false, a)
-	} else {
-		connScope, err = c.host.Network().ResourceManager().OpenConnection(network.DirOutbound, false, a)
-	}
+	connScope, err := c.host.Network().ResourceManager().OpenConnection(network.DirOutbound, false, a)
 
 	if err != nil {
 		return nil, err