fix: only mark a connection as transient if the relay has applied lim…

…its (#2575) In go-libp2p, transient connections are judged based on whether the relay node has restrictions on the current connection. Reference code: https://github.com/libp2p/go-libp2p/blob/fccfbe7a4b6841325100038b83f86cd4e097b9a1/p2p/protocol/circuitv2/client/dial.go#L181 This submission aligns js-libp2p logic with go-libp2p logic. --------- Co-authored-by: wlynxg <[email protected]> Co-authored-by: chad <[email protected]>
libp2p · Jun 6, 2024 · 4bd8e4f · 4bd8e4f
1 parent 440c9b3
commit 4bd8e4f
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 8 deletions.
diff --git a/packages/integration-tests/test/circuit-relay.node.ts b/packages/integration-tests/test/circuit-relay.node.ts
@@ -638,7 +638,7 @@ describe('circuit-relay', () => {
       expect(circuitListener[0].relayStore.listenerCount('relay:removed')).to.equal(1)
     })
 
-    it('should mark a relayed connection as transient', async () => {
+    it('should mark an outgoing relayed connection as transient', async () => {
       // discover relay and make reservation
       const connectionToRelay = await remote.dial(relay1.getMultiaddrs()[0])
 
@@ -655,6 +655,25 @@ describe('circuit-relay', () => {
       expect(connection).to.have.property('transient', true)
     })
 
+    it('should mark an incoming relayed connection as transient', async () => {
+      // discover relay and make reservation
+      const connectionToRelay = await remote.dial(relay1.getMultiaddrs()[0])
+
+      // connection to relay should not be marked transient
+      expect(connectionToRelay).to.have.property('transient', false)
+
+      await usingAsRelay(remote, relay1)
+
+      // dial the remote through the relay
+      const ma = getRelayAddress(remote)
+      await local.dial(ma)
+
+      // connection from local through relay should be marked transient
+      const connections = remote.getConnections(local.peerId)
+      expect(connections).to.have.lengthOf(1)
+      expect(connections).to.have.nested.property('[0].transient', true)
+    })
+
     it('should not open streams on a transient connection', async () => {
       // discover relay and make reservation
       await remote.dial(relay1.getMultiaddrs()[0])
@@ -1073,5 +1092,22 @@ describe('circuit-relay', () => {
       // for longer than that
       expect(finish - start).to.be.greaterThan(defaultDurationLimit)
     })
+
+    it('should not mark an outgoing connection as transient', async () => {
+      const ma = getRelayAddress(remote)
+
+      const connection = await local.dial(ma)
+      expect(connection).to.have.property('transient', false)
+    })
+
+    it('should not mark an incoming connection as transient', async () => {
+      const ma = getRelayAddress(remote)
+
+      await local.dial(ma)
+
+      const connections = remote.getConnections(local.peerId)
+      expect(connections).to.have.lengthOf(1)
+      expect(connections).to.have.nested.property('[0].transient', false)
+    })
   })
 })
diff --git a/packages/transport-circuit-relay-v2/src/server/index.ts b/packages/transport-circuit-relay-v2/src/server/index.ts
@@ -336,6 +336,7 @@ class CircuitRelayServer extends TypedEventEmitter<RelayServerEvents> implements
       return
     }
 
+    const limit = this.reservationStore.get(dstPeer)?.limit
     const destinationConnection = connections[0]
 
     const destinationStream = await this.stopHop({
@@ -345,7 +346,8 @@ class CircuitRelayServer extends TypedEventEmitter<RelayServerEvents> implements
         peer: {
           id: connection.remotePeer.toBytes(),
           addrs: []
-        }
+        },
+        limit
       }
     })
 
@@ -355,11 +357,14 @@ class CircuitRelayServer extends TypedEventEmitter<RelayServerEvents> implements
       return
     }
 
-    await hopstr.write({ type: HopMessage.Type.STATUS, status: Status.OK })
+    await hopstr.write({
+      type: HopMessage.Type.STATUS,
+      status: Status.OK,
+      limit
+    })
     const sourceStream = stream.unwrap()
 
     this.log('connection from %p to %p established - merging streams', connection.remotePeer, dstPeer)
-    const limit = this.reservationStore.get(dstPeer)?.limit
     // Short circuit the two streams to create the relayed connection
     createLimitedRelay(sourceStream, destinationStream, this.shutdownController.signal, limit, {
       log: this.log

diff --git a/packages/transport-circuit-relay-v2/src/transport/transport.ts b/packages/transport-circuit-relay-v2/src/transport/transport.ts
@@ -231,9 +231,9 @@ export class CircuitRelayTransport implements Transport {
         logger: this.logger
       })
 
-      this.log('new outbound transient connection %a', maConn.remoteAddr)
+      this.log('new outbound relayed connection %a', maConn.remoteAddr)
       return await this.upgrader.upgradeOutbound(maConn, {
-        transient: true
+        transient: status.limit != null
       })
     } catch (err: any) {
       this.log.error(`Circuit relay dial to destination ${destinationPeer.toString()} via relay ${connection.remotePeer.toString()} failed`, err)
@@ -346,9 +346,9 @@ export class CircuitRelayTransport implements Transport {
       logger: this.logger
     })
 
-    this.log('new inbound transient connection %a', maConn.remoteAddr)
+    this.log('new inbound relayed connection %a', maConn.remoteAddr)
     await this.upgrader.upgradeInbound(maConn, {
-      transient: true
+      transient: request.limit != null
     })
     this.log('%s connection %a upgraded', 'inbound', maConn.remoteAddr)
   }