Repository: libp2p/go-libp2p
Pull request #2168 (2023-03-03 10:09:32)

rcmgr: fix README


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4d53dcd4..012815d3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@
 
 # [v0.27.0](https://github.com/libp2p/go-libp2p/releases/tag/v0.27.0) (unreleased)
 
+### Breaking Changes
+
+* The `LocalPrivateKey` method was removed from the `network.Conn` interface.
+
 # [v0.26.1](https://github.com/libp2p/go-libp2p/releases/tag/v0.26.1)
 
 This patch release fixes two bugs:
diff --git a/core/network/conn.go b/core/network/conn.go
index 56031ab0..97168915 100644
--- a/core/network/conn.go
+++ b/core/network/conn.go
@@ -53,9 +53,6 @@ type ConnSecurity interface {
 	// LocalPeer returns our peer ID
 	LocalPeer() peer.ID
 
-	// LocalPrivateKey returns our private key
-	LocalPrivateKey() ic.PrivKey
-
 	// RemotePeer returns the peer ID of the remote peer.
 	RemotePeer() peer.ID
 
diff --git a/core/sec/insecure/insecure.go b/core/sec/insecure/insecure.go
index 9b3664e2..9ed20f09 100644
--- a/core/sec/insecure/insecure.go
+++ b/core/sec/insecure/insecure.go
@@ -40,9 +40,7 @@ type Transport struct {
 
 var _ sec.SecureTransport = &Transport{}
 
-// NewWithIdentity constructs a new insecure transport. The provided private key
-// is stored and returned from LocalPrivateKey to satisfy the
-// SecureTransport interface, and the public key is sent to
+// NewWithIdentity constructs a new insecure transport. The public key is sent to
 // remote peers. No security is provided.
 func NewWithIdentity(protocolID protocol.ID, id peer.ID, key ci.PrivKey) *Transport {
 	return &Transport{
@@ -57,12 +55,6 @@ func (t *Transport) LocalPeer() peer.ID {
 	return t.id
 }
 
-// LocalPrivateKey returns the local private key.
-// This key is used only for identity generation and provides no security.
-func (t *Transport) LocalPrivateKey() ci.PrivKey {
-	return t.key
-}
-
 // SecureInbound *pretends to secure* an inbound connection to the given peer.
 // It sends the local peer's ID and public key, and receives the same from the remote peer.
 // No validation is performed as to the authenticity or ownership of the provided public key,
@@ -70,19 +62,18 @@ func (t *Transport) LocalPrivateKey() ci.PrivKey {
 //
 // SecureInbound may fail if the remote peer sends an ID and public key that are inconsistent
 // with each other, or if a network error occurs during the ID exchange.
-func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) {
+func (t *Transport) SecureInbound(_ context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) {
 	conn := &Conn{
-		Conn:         insecure,
-		local:        t.id,
-		localPrivKey: t.key,
+		Conn:        insecure,
+		local:       t.id,
+		localPubKey: t.key.GetPublic(),
 	}
 
-	err := conn.runHandshakeSync()
-	if err != nil {
+	if err := conn.runHandshakeSync(); err != nil {
 		return nil, err
 	}
 
-	if t.key != nil && p != "" && p != conn.remote {
+	if p != "" && p != conn.remote {
 		return nil, fmt.Errorf("remote peer sent unexpected peer ID. expected=%s received=%s", p, conn.remote)
 	}
 
@@ -97,19 +88,18 @@ func (t *Transport) SecureInbound(ctx context.Context, insecure net.Conn, p peer
 // SecureOutbound may fail if the remote peer sends an ID and public key that are inconsistent
 // with each other, or if the ID sent by the remote peer does not match the one dialed. It may
 // also fail if a network error occurs during the ID exchange.
-func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) {
+func (t *Transport) SecureOutbound(_ context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, error) {
 	conn := &Conn{
-		Conn:         insecure,
-		local:        t.id,
-		localPrivKey: t.key,
+		Conn:        insecure,
+		local:       t.id,
+		localPubKey: t.key.GetPublic(),
 	}
 
-	err := conn.runHandshakeSync()
-	if err != nil {
+	if err := conn.runHandshakeSync(); err != nil {
 		return nil, err
 	}
 
-	if t.key != nil && p != conn.remote {
+	if p != conn.remote {
 		return nil, fmt.Errorf("remote peer sent unexpected peer ID. expected=%s received=%s",
 			p, conn.remote)
 	}
@@ -117,19 +107,14 @@ func (t *Transport) SecureOutbound(ctx context.Context, insecure net.Conn, p pee
 	return conn, nil
 }
 
-func (t *Transport) ID() protocol.ID {
-	return t.protocolID
-}
+func (t *Transport) ID() protocol.ID { return t.protocolID }
 
 // Conn is the connection type returned by the insecure transport.
 type Conn struct {
 	net.Conn
 
-	local  peer.ID
-	remote peer.ID
-
-	localPrivKey ci.PrivKey
-	remotePubKey ci.PubKey
+	local, remote             peer.ID
+	localPubKey, remotePubKey ci.PubKey
 }
 
 func makeExchangeMessage(pubkey ci.PubKey) (*pb.Exchange, error) {
@@ -150,12 +135,12 @@ func makeExchangeMessage(pubkey ci.PubKey) (*pb.Exchange, error) {
 
 func (ic *Conn) runHandshakeSync() error {
 	// If we were initialized without keys, behave as in plaintext/1.0.0 (do nothing)
-	if ic.localPrivKey == nil {
+	if ic.localPubKey == nil {
 		return nil
 	}
 
 	// Generate an Exchange message
-	msg, err := makeExchangeMessage(ic.localPrivKey.GetPublic())
+	msg, err := makeExchangeMessage(ic.localPubKey)
 	if err != nil {
 		return err
 	}
@@ -239,11 +224,6 @@ func (ic *Conn) RemotePublicKey() ci.PubKey {
 	return ic.remotePubKey
 }
 
-// LocalPrivateKey returns the private key for the local peer.
-func (ic *Conn) LocalPrivateKey() ci.PrivKey {
-	return ic.localPrivKey
-}
-
 // ConnState returns the security connection's state information.
 func (ic *Conn) ConnState() network.ConnectionState {
 	return network.ConnectionState{}
diff --git a/p2p/host/resource-manager/README.md b/p2p/host/resource-manager/README.md
index 3e83c45b..85926100 100644
--- a/p2p/host/resource-manager/README.md
+++ b/p2p/host/resource-manager/README.md
@@ -34,7 +34,7 @@ scaledDefaultLimits := scalingLimits.AutoScale()
 
 // Tweak certain settings
 cfg := rcmgr.PartialLimitConfig{
-  System: &rcmgr.ResourceLimits{
+  System: rcmgr.ResourceLimits{
     // Allow unlimited outbound streams
     StreamsOutbound: rcmgr.Unlimited,
   },
@@ -447,7 +447,7 @@ Example Log:
 
 The log line above is an example log line that gets emitted if you enable debug
 logging in the resource manager. You can do this by setting the environment
-variable `GOLOG_LOG_LEVEL="rcmgr=info"`. By default only the error is
+variable `GOLOG_LOG_LEVEL="rcmgr=debug"`. By default only the error is
 returned to the caller, and nothing is logged by the resource manager itself.
 
 The log line message (and returned error) will tell you which resource limit was
diff --git a/p2p/host/routed/routed.go b/p2p/host/routed/routed.go
index 0cedd48f..c4601a50 100644
--- a/p2p/host/routed/routed.go
+++ b/p2p/host/routed/routed.go
@@ -114,7 +114,8 @@ func (rh *RoutedHost) Connect(ctx context.Context, pi peer.AddrInfo) error {
 		// try to connect again.
 		newAddrs, err := rh.findPeerAddrs(ctx, pi.ID)
 		if err != nil {
-			return fmt.Errorf("failed to find peers: %w", err)
+			log.Debugf("failed to find more peer addresses %s: %s", pi.ID, err)
+			return cerr
 		}
 
 		// Build lookup map
diff --git a/p2p/net/mock/mock_conn.go b/p2p/net/mock/mock_conn.go
index b6fe8f2d..970fc2fc 100644
--- a/p2p/net/mock/mock_conn.go
+++ b/p2p/net/mock/mock_conn.go
@@ -158,11 +158,6 @@ func (c *conn) LocalPeer() peer.ID {
 	return c.local
 }
 
-// LocalPrivateKey is the private key of the peer on our side.
-func (c *conn) LocalPrivateKey() ic.PrivKey {
-	return c.localPrivKey
-}
-
 // RemoteMultiaddr is the Multiaddr on the remote side
 func (c *conn) RemoteMultiaddr() ma.Multiaddr {
 	return c.remoteAddr
diff --git a/p2p/net/swarm/swarm_conn.go b/p2p/net/swarm/swarm_conn.go
index 406b100a..8cdabc41 100644
--- a/p2p/net/swarm/swarm_conn.go
+++ b/p2p/net/swarm/swarm_conn.go
@@ -168,11 +168,6 @@ func (c *Conn) RemotePeer() peer.ID {
 	return c.conn.RemotePeer()
 }
 
-// LocalPrivateKey is the public key of the peer on this side
-func (c *Conn) LocalPrivateKey() ic.PrivKey {
-	return c.conn.LocalPrivateKey()
-}
-
 // RemotePublicKey is the public key of the peer on the remote side
 func (c *Conn) RemotePublicKey() ic.PubKey {
 	return c.conn.RemotePublicKey()
diff --git a/p2p/protocol/circuitv2/relay/relay.go b/p2p/protocol/circuitv2/relay/relay.go
index 5a5e97bd..77c592e8 100644
--- a/p2p/protocol/circuitv2/relay/relay.go
+++ b/p2p/protocol/circuitv2/relay/relay.go
@@ -51,6 +51,7 @@ type Relay struct {
 	constraints *constraints
 	scope       network.ResourceScopeSpan
 	notifiee    network.Notifiee
+	wg          sync.WaitGroup
 
 	mx     sync.Mutex
 	rsvp   map[peer.ID]time.Time
@@ -98,6 +99,8 @@ func New(h host.Host, opts ...Option) (*Relay, error) {
 	h.SetStreamHandler(proto.ProtoIDv2Hop, r.handleStream)
 	r.notifiee = &network.NotifyBundle{DisconnectedF: r.disconnected}
 	h.Network().Notify(r.notifiee)
+
+	r.wg.Add(1)
 	go r.background()
 
 	return r, nil
@@ -105,17 +108,18 @@ func New(h host.Host, opts ...Option) (*Relay, error) {
 
 func (r *Relay) Close() error {
 	r.mx.Lock()
-	defer r.mx.Unlock()
 	if !r.closed {
 		r.closed = true
+		r.mx.Unlock()
+
 		r.host.RemoveStreamHandler(proto.ProtoIDv2Hop)
 		r.host.Network().StopNotify(r.notifiee)
 		r.scope.Done()
 		r.cancel()
-		for p := range r.rsvp {
-			r.host.ConnManager().UntagPeer(p, "relay-reservation")
-		}
+		r.wg.Wait()
+		return nil
 	}
+	r.mx.Unlock()
 	return nil
 }
 
@@ -564,6 +568,8 @@ func (r *Relay) background() {
 		case <-ticker.C:
 			r.gc()
 		case <-r.ctx.Done():
+			r.gc()
+			r.wg.Done()
 			return
 		}
 	}
@@ -576,7 +582,7 @@ func (r *Relay) gc() {
 	now := time.Now()
 
 	for p, expire := range r.rsvp {
-		if expire.Before(now) {
+		if r.closed || expire.Before(now) {
 			delete(r.rsvp, p)
 			r.host.ConnManager().UntagPeer(p, "relay-reservation")
 		}
diff --git a/p2p/security/noise/session.go b/p2p/security/noise/session.go
index 2f6c23c0..fa32ab8f 100644
--- a/p2p/security/noise/session.go
+++ b/p2p/security/noise/session.go
@@ -95,10 +95,6 @@ func (s *secureSession) LocalPeer() peer.ID {
 	return s.localID
 }
 
-func (s *secureSession) LocalPrivateKey() crypto.PrivKey {
-	return s.localKey
-}
-
 func (s *secureSession) LocalPublicKey() crypto.PubKey {
 	return s.localKey.GetPublic()
 }
diff --git a/p2p/security/tls/conn.go b/p2p/security/tls/conn.go
index 3ebc7aef..143da392 100644
--- a/p2p/security/tls/conn.go
+++ b/p2p/security/tls/conn.go
@@ -12,9 +12,7 @@ import (
 type conn struct {
 	*tls.Conn
 
-	localPeer peer.ID
-	privKey   ci.PrivKey
-
+	localPeer       peer.ID
 	remotePeer      peer.ID
 	remotePubKey    ci.PubKey
 	connectionState network.ConnectionState
@@ -26,10 +24,6 @@ func (c *conn) LocalPeer() peer.ID {
 	return c.localPeer
 }
 
-func (c *conn) LocalPrivateKey() ci.PrivKey {
-	return c.privKey
-}
-
 func (c *conn) RemotePeer() peer.ID {
 	return c.remotePeer
 }
diff --git a/p2p/security/tls/transport.go b/p2p/security/tls/transport.go
index b8c1dcfb..7c28efe3 100644
--- a/p2p/security/tls/transport.go
+++ b/p2p/security/tls/transport.go
@@ -168,7 +168,6 @@ func (t *Transport) setupConn(tlsConn *tls.Conn, remotePubKey ci.PubKey) (sec.Se
 	return &conn{
 		Conn:         tlsConn,
 		localPeer:    t.localPeer,
-		privKey:      t.privKey,
 		remotePeer:   remotePeerID,
 		remotePubKey: remotePubKey,
 		connectionState: network.ConnectionState{
diff --git a/p2p/transport/quic/conn.go b/p2p/transport/quic/conn.go
index 1154cf72..a2da81eb 100644
--- a/p2p/transport/quic/conn.go
+++ b/p2p/transport/quic/conn.go
@@ -18,7 +18,6 @@ type conn struct {
 	scope     network.ConnManagementScope
 
 	localPeer      peer.ID
-	privKey        ic.PrivKey
 	localMultiaddr ma.Multiaddr
 
 	remotePeerID    peer.ID
@@ -66,9 +65,6 @@ func (c *conn) AcceptStream() (network.MuxedStream, error) {
 // LocalPeer returns our peer ID
 func (c *conn) LocalPeer() peer.ID { return c.localPeer }
 
-// LocalPrivateKey returns our private key
-func (c *conn) LocalPrivateKey() ic.PrivKey { return c.privKey }
-
 // RemotePeer returns the peer ID of the remote peer.
 func (c *conn) RemotePeer() peer.ID { return c.remotePeerID }
 
diff --git a/p2p/transport/quic/listener.go b/p2p/transport/quic/listener.go
index 017189cb..73bb5026 100644
--- a/p2p/transport/quic/listener.go
+++ b/p2p/transport/quic/listener.go
@@ -133,7 +133,6 @@ func (l *listener) setupConnWithScope(qconn quic.Connection, connScope network.C
 		scope:           connScope,
 		localPeer:       l.localPeer,
 		localMultiaddr:  localMultiaddr,
-		privKey:         l.privKey,
 		remoteMultiaddr: remoteMultiaddr,
 		remotePeerID:    remotePeerID,
 		remotePubKey:    remotePubKey,
diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go
index 5ccebf93..2d68b660 100644
--- a/p2p/transport/quic/transport.go
+++ b/p2p/transport/quic/transport.go
@@ -153,7 +153,6 @@ func (t *transport) dialWithScope(ctx context.Context, raddr ma.Multiaddr, p pee
 		quicConn:        pconn,
 		transport:       t,
 		scope:           scope,
-		privKey:         t.privKey,
 		localPeer:       t.localPeer,
 		localMultiaddr:  localMultiaddr,
 		remotePubKey:    remotePubKey,


Test patch:
diff --git a/core/sec/insecure/insecure_test.go b/core/sec/insecure/insecure_test.go
index 8663f1d9..f6ceba41 100644
--- a/core/sec/insecure/insecure_test.go
+++ b/core/sec/insecure/insecure_test.go
@@ -103,6 +103,7 @@ func connect(t *testing.T, clientTpt, serverTpt *Transport, clientExpectsID, ser
 
 // Check the peer IDs
 func testIDs(t *testing.T, clientTpt, serverTpt *Transport, clientConn, serverConn sec.SecureConn) {
+	t.Helper()
 	require.Equal(t, clientConn.LocalPeer(), clientTpt.LocalPeer(), "Client Local Peer ID mismatch.")
 	require.Equal(t, clientConn.RemotePeer(), serverTpt.LocalPeer(), "Client Remote Peer ID mismatch.")
 	require.Equal(t, clientConn.LocalPeer(), serverConn.RemotePeer(), "Server Local Peer ID mismatch.")
@@ -110,9 +111,9 @@ func testIDs(t *testing.T, clientTpt, serverTpt *Transport, clientConn, serverCo
 
 // Check the keys
 func testKeys(t *testing.T, clientTpt, serverTpt *Transport, clientConn, serverConn sec.SecureConn) {
-	sk := serverConn.LocalPrivateKey()
-	require.True(t, sk.Equals(serverTpt.LocalPrivateKey()), "private key mismatch")
-	require.True(t, sk.GetPublic().Equals(clientConn.RemotePublicKey()), "public key mismatch")
+	t.Helper()
+	require.True(t, clientConn.RemotePublicKey().Equals(serverTpt.key.GetPublic()), "client conn key mismatch")
+	require.True(t, serverConn.RemotePublicKey().Equals(clientTpt.key.GetPublic()), "server conn key mismatch")
 }
 
 // Check sending and receiving messages
diff --git a/p2p/host/relaysvc/relay_test.go b/p2p/host/relaysvc/relay_test.go
index d995cac2..4117e119 100644
--- a/p2p/host/relaysvc/relay_test.go
+++ b/p2p/host/relaysvc/relay_test.go
@@ -34,7 +34,7 @@ func TestReachabilityChangeEvent(t *testing.T) {
 	require.Eventually(
 		t,
 		func() bool { rmgr.mutex.Lock(); defer rmgr.mutex.Unlock(); return rmgr.relay == nil },
-		1*time.Second,
+		3*time.Second,
 		100*time.Millisecond,
 		"relay should be nil on private reachability")
 
@@ -45,7 +45,7 @@ func TestReachabilityChangeEvent(t *testing.T) {
 	require.Eventually(
 		t,
 		func() bool { rmgr.mutex.Lock(); defer rmgr.mutex.Unlock(); return rmgr.relay == nil },
-		1*time.Second,
+		3*time.Second,
 		100*time.Millisecond,
 		"relay should be nil on unknown reachability")
 
diff --git a/p2p/net/connmgr/connmgr_test.go b/p2p/net/connmgr/connmgr_test.go
index 7468f671..69f82a39 100644
--- a/p2p/net/connmgr/connmgr_test.go
+++ b/p2p/net/connmgr/connmgr_test.go
@@ -796,7 +796,6 @@ type mockConn struct {
 
 func (m mockConn) Close() error                                          { panic("implement me") }
 func (m mockConn) LocalPeer() peer.ID                                    { panic("implement me") }
-func (m mockConn) LocalPrivateKey() crypto.PrivKey                       { panic("implement me") }
 func (m mockConn) RemotePeer() peer.ID                                   { panic("implement me") }
 func (m mockConn) RemotePublicKey() crypto.PubKey                        { panic("implement me") }
 func (m mockConn) LocalMultiaddr() ma.Multiaddr                          { panic("implement me") }
diff --git a/p2p/security/noise/transport_test.go b/p2p/security/noise/transport_test.go
index ada0cbfe..42477f83 100644
--- a/p2p/security/noise/transport_test.go
+++ b/p2p/security/noise/transport_test.go
@@ -159,15 +159,16 @@ func TestKeys(t *testing.T) {
 	defer initConn.Close()
 	defer respConn.Close()
 
-	sk := respConn.LocalPrivateKey()
-	pk := sk.GetPublic()
-
-	if !sk.Equals(respTransport.privateKey) {
-		t.Error("Private key Mismatch.")
+	pk1 := respConn.RemotePublicKey()
+	pk2 := initTransport.privateKey.GetPublic()
+	if !pk1.Equals(pk2) {
+		t.Errorf("Public key mismatch. expected %x got %x", pk1, pk2)
 	}
 
-	if !pk.Equals(initConn.RemotePublicKey()) {
-		t.Errorf("Public key mismatch. expected %x got %x", pk, initConn.RemotePublicKey())
+	pk3 := initConn.RemotePublicKey()
+	pk4 := respTransport.privateKey.GetPublic()
+	if !pk3.Equals(pk4) {
+		t.Errorf("Public key mismatch. expected %x got %x", pk3, pk4)
 	}
 }
 
diff --git a/p2p/security/tls/transport_test.go b/p2p/security/tls/transport_test.go
index a30c742a..2d3c2d97 100644
--- a/p2p/security/tls/transport_test.go
+++ b/p2p/security/tls/transport_test.go
@@ -111,8 +111,6 @@ func TestHandshakeSucceeds(t *testing.T) {
 
 		require.Equal(t, clientConn.LocalPeer(), clientID)
 		require.Equal(t, serverConn.LocalPeer(), serverID)
-		require.True(t, clientConn.LocalPrivateKey().Equals(clientKey), "client private key mismatch")
-		require.True(t, serverConn.LocalPrivateKey().Equals(serverKey), "server private key mismatch")
 		require.Equal(t, clientConn.RemotePeer(), serverID)
 		require.Equal(t, serverConn.RemotePeer(), clientID)
 		require.True(t, clientConn.RemotePublicKey().Equals(serverKey.GetPublic()), "server public key mismatch")
@@ -249,8 +247,6 @@ func TestHandshakeWithNextProtoSucceeds(t *testing.T) {
 
 		require.Equal(t, clientConn.LocalPeer(), clientID)
 		require.Equal(t, serverConn.LocalPeer(), serverID)
-		require.True(t, clientConn.LocalPrivateKey().Equals(clientKey), "client private key mismatch")
-		require.True(t, serverConn.LocalPrivateKey().Equals(serverKey), "server private key mismatch")
 		require.Equal(t, clientConn.RemotePeer(), serverID)
 		require.Equal(t, serverConn.RemotePeer(), clientID)
 		require.True(t, clientConn.RemotePublicKey().Equals(serverKey.GetPublic()), "server public key mismatch")
diff --git a/p2p/transport/quic/conn_test.go b/p2p/transport/quic/conn_test.go
index 69294587..189a38df 100644
--- a/p2p/transport/quic/conn_test.go
+++ b/p2p/transport/quic/conn_test.go
@@ -103,12 +103,10 @@ func testHandshake(t *testing.T, tc *connTestCase) {
 		defer serverConn.Close()
 
 		require.Equal(t, conn.LocalPeer(), clientID)
-		require.True(t, conn.LocalPrivateKey().Equals(clientKey), "local private key doesn't match")
 		require.Equal(t, conn.RemotePeer(), serverID)
 		require.True(t, conn.RemotePublicKey().Equals(serverKey.GetPublic()), "remote public key doesn't match")
 
 		require.Equal(t, serverConn.LocalPeer(), serverID)
-		require.True(t, serverConn.LocalPrivateKey().Equals(serverKey), "local private key doesn't match")
 		require.Equal(t, serverConn.RemotePeer(), clientID)
 		require.True(t, serverConn.RemotePublicKey().Equals(clientKey.GetPublic()), "remote public key doesn't match")
 	}