feat(libp2p): direct connection through relay protocol (DCUtR) #1928

achingbrain · 2023-08-03T14:38:11Z

Only supports TCP Simultaneous Connect since there is no QUIC support yet

Implements the [DCUtR protocol](https://github.com/libp2p/specs/blob/master/relay/DCUtR.md). Only supports TCP Simultaneous Connect since there is no QUIC support yet

related to libp2p/js-libp2p#1928

SgtPooki

the updates with forced connections are great, and I think what was preventing my demo from succeeding.

SgtPooki · 2023-08-03T17:14:00Z

packages/interface-internal/src/connection-manager/index.ts

@@ -37,7 +41,7 @@ export interface ConnectionManager {
   * const connection = await libp2p.connectionManager.openConnection(peerId)
   * ```
   */
-  openConnection: (peer: PeerId | Multiaddr | Multiaddr[], options?: AbortOptions) => Promise<Connection>
+  openConnection: (peer: PeerId | Multiaddr | Multiaddr[], options?: OpenConnectionOptions) => Promise<Connection>


we could probably do this as a separate PR

It's needed to pass the force option to openConnection when we have an existing transient connection.

I mean, we could make this change in a smaller PR and get the code out prior to the whole DCUtR. it's beneficial in it's own right.

packages/libp2p/src/dcutr/dcutr.ts

packages/libp2p/test/connection-manager/index.node.ts

SgtPooki · 2023-08-10T00:15:06Z

I put the latest of your changes up at https://github.com/SgtPooki/helia-playground/tree/feat/half-dcutr-poc and it still seems like dial-backs aren't quite enough for unblocking the golden-path for helia-authored content being accessible from ipfs.io.

Am I missing something? I was thinking this PR (or at least the unilateral connection upgrade) would be enough to unblock the golden path scenario for Helia?

SgtPooki · 2023-08-10T02:53:09Z

one thing i'm noticing is that isPublicAndDialable isn't always filtering out p2p-circuit addresses:

Here are some that i can see in my testing of helia-playground:

dcutr.ts:355 libp2p:dcutr:B:trace unilateral publicAddresses left +1ms 
 
"/ip4/5.161.55.227/tcp/4001/p2p/12D3KooWSW4hoHmDXmY5rW7nCi9XmGTy3foFt72u86jNP53LTNBJ/p2p-circuit"
"/ip4/5.161.55.227/udp/4001/quic-v1/webtransport/certhash/uEiAVFKPNfMZ-n9o4QEUIoDKM3UrXRlrfCBwhflJAkO6fdA/certhash/uEiCAaSv97aZJ8eA7o41dza9EoLMHtv51dSdntNwYKRdu0g/p2p/12D3KooWSW4hoHmDXmY5rW7nCi9XmGTy3foFt72u86jNP53LTNBJ/p2p-circuit"
"/ip4/5.161.55.227/udp/4001/quic/p2p/12D3KooWSW4hoHmDXmY5rW7nCi9XmGTy3foFt72u86jNP53LTNBJ/p2p-circuit"
"/ip4/5.161.92.43/tcp/4001/p2p/12D3KooWFFhc8fPYnQXdWBCowxSV21EFYin3rU27p3NVgSMjN41k/p2p-circuit"
"/ip4/5.161.92.43/udp/4001/quic-v1/webtransport/certhash/uEiDfxpe8sEFZ2k4BRjku6zhmfMLrig2EtuydiK9UxpPpIw/certhash/uEiCx-6CWdlNH5f1qfOwhFfCqYWxUpzoVirpd9R0cIfkwqA/p2p/12D3KooWFFhc8fPYnQXdWBCowxSV21EFYin3rU27p3NVgSMjN41k/p2p-circuit"
"/ip4/5.161.92.43/udp/4001/quic/p2p/12D3KooWFFhc8fPYnQXdWBCowxSV21EFYin3rU27p3NVgSMjN41k/p2p-circuit"
"/ip6/2a01:4ff:f0:1e5a::1/tcp/4001/p2p/12D3KooWSW4hoHmDXmY5rW7nCi9XmGTy3foFt72u86jNP53LTNBJ/p2p-circuit"
"/ip6/2a01:4ff:f0:1e5a::1/udp/4001/quic-v1/webtransport/certhash/uEiAVFKPNfMZ-n9o4QEUIoDKM3UrXRlrfCBwhflJAkO6fdA/certhash/uEiCAaSv97aZJ8eA7o41dza9EoLMHtv51dSdntNwYKRdu0g/p2p/12D3KooWSW4hoHmDXmY5rW7nCi9XmGTy3foFt72u86jNP53LTNBJ/p2p-circuit"
"/ip6/2a01:4ff:f0:1e5a::1/udp/4001/quic/p2p/12D3KooWSW4hoHmDXmY5rW7nCi9XmGTy3foFt72u86jNP53LTNBJ/p2p-circuit"
"/ip6/2a01:4ff:f0:3b1e::1/tcp/4001/p2p/12D3KooWFFhc8fPYnQXdWBCowxSV21EFYin3rU27p3NVgSMjN41k/p2p-circuit"
"/ip6/2a01:4ff:f0:3b1e::1/udp/4001/quic-v1/webtransport/certhash/uEiDfxpe8sEFZ2k4BRjku6zhmfMLrig2EtuydiK9UxpPpIw/certhash/uEiCx-6CWdlNH5f1qfOwhFfCqYWxUpzoVirpd9R0cIfkwqA/p2p/12D3KooWFFhc8fPYnQXdWBCowxSV21EFYin3rU27p3NVgSMjN41k/p2p-circuit"
"/ip6/2a01:4ff:f0:3b1e::1/udp/4001/quic/p2p/12D3KooWFFhc8fPYnQXdWBCowxSV21EFYin3rU27p3NVgSMjN41k/p2p-circuit"

Opened multiformats/js-multiaddr-matcher#2 to address

achingbrain · 2023-08-10T10:58:40Z

one thing i'm noticing is that isPublicAndDialable isn't always filtering out p2p-circuit addresses

It's a bit of a weird one, network peers usually return their multiaddrs without a peer id, I guess the intention is including the peer id creates a lot of redundancy on the wire but it means we can't treat multiaddrs as opaque values.

Taking the first address as an example:

/ip4/5.161.55.227/tcp/4001/p2p/12D3KooWSW4hoHmDXmY5rW7nCi9XmGTy3foFt72u86jNP53LTNBJ/p2p-circuit

We can't actually dial this address as it needs to be: {relay-address}/p2p-circuit/{destination-peer-id} but it's just {relay-address}/p2p-circuit - we need {destination-peer-id} to be included otherwise we can't tell the relay which peer we are trying to dial in the Hop CONNECT message.

So we need to append the destination peer id ourselves, I've added this, hopefully it improves things.

whizzzkid

nits

packages/libp2p/src/dcutr/dcutr.ts

whizzzkid · 2023-08-11T04:39:51Z

packages/libp2p/src/dcutr/dcutr.ts

+    const output = []
+
+    for (const addr of addrs) {
+      if (addr == null || addr.length === 0) {
+        continue
+      }
+
+      try {
+        const ma = multiaddr(addr)
+
+        if (!this.isPublicAndDialable(ma)) {
+          continue
+        }
+
+        output.push(ma)
+      } catch {}
+    }
+
+    return output


why not just filter?

Suggested change

const output = []

for (const addr of addrs) {

if (addr == null || addr.length === 0) {

continue

}

try {

const ma = multiaddr(addr)

if (!this.isPublicAndDialable(ma)) {

continue

}

output.push(ma)

} catch {}

}

return output

return addrs

.filter(addr => addr != null)

.filter(addr => addr.length !== 0)

.filter(addr => {

try {

const ma = multiaddr(addr)

return this.isPublicAndDialable(ma))

} catch {}

})

The suggestion iterates over the list three times. Doing for..of means we only iterate over the list once.

nice, what I meant was, it can be filtered out in a single iteration instead of pushing into an output array.

Suggested change

const output = []

for (const addr of addrs) {

if (addr == null || addr.length === 0) {

continue

}

try {

const ma = multiaddr(addr)

if (!this.isPublicAndDialable(ma)) {

continue

}

output.push(ma)

} catch {}

}

return output

return addrs

.filter(addr => {

if (addr == null) return false

if (addr.length === 0) return false

try {

const ma = multiaddr(addr)

return this.isPublicAndDialable(ma))

} catch {

// swallow

} finally {

return false

}

})

It's also worth noting that for..of is faster than JS array methods since we're not creating new function contexts/stacks/etc for each iteration - I haven't measured anything so it might not be a bottleneck here but .forEach, .filter, .map etc are certainly not free.

for loops are usually faster in all languages. forEach/map/etc are convenience methods that may improve readability, but for the majority of code we maintain, we should probably default to using for loops.

NOTE: some optimizations in v8 have made forEach/map/etc more efficient, but for loops are the "lowest level" loop we have access to and are faster by default

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>

Co-authored-by: Marco Munizaga <git@marcopolo.io> Co-authored-by: Chad Nehemiah <chad.nehemiah94@gmail.com>

achingbrain · 2023-08-15T11:37:31Z

@SgtPooki @whizzzkid any further objections or can we merge this now?

feat: direct connection through relay protocol (DCUtR)

1cc42b5

Implements the [DCUtR protocol](https://github.com/libp2p/specs/blob/master/relay/DCUtR.md). Only supports TCP Simultaneous Connect since there is no QUIC support yet

achingbrain changed the title ~~fix: add gh credentials to publish docs~~ feat: direct connection through relay protocol (DCUtR) Aug 3, 2023

chore: fix linting

a474be1

achingbrain mentioned this pull request Aug 3, 2023

[🏆 Golden path scenario] Browser-authored content retrievable by another machine through the ipfs.io gateway directly ipfs/helia#182

Open

Merge branch 'master' into feat/dcutr

3b44c96

SgtPooki mentioned this pull request Aug 3, 2023

test: add tests for non-unilateral DCUtR over TCP #1932

Closed

p-shahi linked an issue Aug 4, 2023 that may be closed by this pull request

Use DCUtR to synchronize hole punching #1460

Closed

achingbrain added 2 commits August 7, 2023 14:55

Merge branch 'master' into feat/dcutr

d78c192

chore: ensure all multiaddrs have the same peer id

6052d90

SgtPooki mentioned this pull request Aug 7, 2023

add connectivity tests for circuit v2 libp2p/test-plans#116

Open

SgtPooki added a commit to SgtPooki/helia-playground that referenced this pull request Aug 8, 2023

feat: migrate to react and use dcutr stuff

8da3ffc

related to libp2p/js-libp2p#1928

achingbrain added 2 commits August 8, 2023 11:46

Merge remote-tracking branch 'origin/master' into feat/dcutr

3ed31d8

chore: add option to force opening a connection

c0062fd

SgtPooki reviewed Aug 10, 2023

View reviewed changes

SgtPooki added a commit to SgtPooki/helia-playground that referenced this pull request Aug 10, 2023

feat: latest-dcutr updates from libp2p/js-libp2p/pull/1928

f613fab

SgtPooki mentioned this pull request Aug 10, 2023

fix: recognize p2p-circuit addresses without trailing peerId multiformats/js-multiaddr-matcher#2

Closed

achingbrain changed the title ~~feat: direct connection through relay protocol (DCUtR)~~ feat(libp2p): direct connection through relay protocol (DCUtR) Aug 10, 2023

achingbrain added 2 commits August 10, 2023 11:52

chore: add peer id to addreses before filtering them

c42d87b

Merge branch 'master' into feat/dcutr

c7ccd63

Merge branch 'master' into feat/dcutr

7d1c021

whizzzkid reviewed Aug 11, 2023

View reviewed changes

achingbrain and others added 4 commits August 11, 2023 14:21

chore: apply suggestions from code review

f5f7fe9

Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>

feat(@libp2p/protocol-perf): Implement perf protocol (#1604)

8ba49c5

Co-authored-by: Marco Munizaga <git@marcopolo.io> Co-authored-by: Chad Nehemiah <chad.nehemiah94@gmail.com>

fix: update project config

68cfdbd

chore: add break

fa41c46

achingbrain force-pushed the feat/dcutr branch from 04e0c97 to fa41c46 Compare August 11, 2023 13:22

achingbrain requested a review from whizzzkid August 11, 2023 16:27

Merge remote-tracking branch 'origin/master' into feat/dcutr

fb1d4f4

SgtPooki mentioned this pull request Aug 11, 2023

feat: peerStore returns multiaddrs for a peer with peerIds postfixed where necessary. #1959

Closed

achingbrain added 2 commits August 13, 2023 10:15

chore: only throw if we have exhausted the retries

e6f6b99

chore: update logging

5088fe3

SgtPooki approved these changes Aug 15, 2023

View reviewed changes

achingbrain merged commit 87dc7e9 into master Aug 16, 2023
17 checks passed

achingbrain deleted the feat/dcutr branch August 16, 2023 06:52

SgtPooki mentioned this pull request Sep 20, 2023

Stream Migration Protocol libp2p/specs#328

Open

This was referenced Jan 18, 2024

chore(main): release 1.0.0 #2365

Closed

chore(main): release 1.0.0 #2366

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(libp2p): direct connection through relay protocol (DCUtR) #1928

feat(libp2p): direct connection through relay protocol (DCUtR) #1928

achingbrain commented Aug 3, 2023

SgtPooki left a comment

SgtPooki Aug 3, 2023

achingbrain Aug 11, 2023

SgtPooki Aug 11, 2023

SgtPooki commented Aug 10, 2023

SgtPooki commented Aug 10, 2023 •

edited

Loading

achingbrain commented Aug 10, 2023

whizzzkid left a comment

whizzzkid Aug 11, 2023

achingbrain Aug 11, 2023

whizzzkid Aug 12, 2023 •

edited

Loading

achingbrain Aug 13, 2023

achingbrain Aug 13, 2023 •

edited

Loading

SgtPooki Aug 15, 2023 •

edited

Loading

achingbrain commented Aug 15, 2023

feat(libp2p): direct connection through relay protocol (DCUtR) #1928

feat(libp2p): direct connection through relay protocol (DCUtR) #1928

Conversation

achingbrain commented Aug 3, 2023

SgtPooki left a comment

Choose a reason for hiding this comment

SgtPooki Aug 3, 2023

Choose a reason for hiding this comment

achingbrain Aug 11, 2023

Choose a reason for hiding this comment

SgtPooki Aug 11, 2023

Choose a reason for hiding this comment

SgtPooki commented Aug 10, 2023

SgtPooki commented Aug 10, 2023 • edited Loading

achingbrain commented Aug 10, 2023

whizzzkid left a comment

Choose a reason for hiding this comment

whizzzkid Aug 11, 2023

Choose a reason for hiding this comment

achingbrain Aug 11, 2023

Choose a reason for hiding this comment

whizzzkid Aug 12, 2023 • edited Loading

Choose a reason for hiding this comment

achingbrain Aug 13, 2023

Choose a reason for hiding this comment

achingbrain Aug 13, 2023 • edited Loading

Choose a reason for hiding this comment

SgtPooki Aug 15, 2023 • edited Loading

Choose a reason for hiding this comment

achingbrain commented Aug 15, 2023

SgtPooki commented Aug 10, 2023 •

edited

Loading

whizzzkid Aug 12, 2023 •

edited

Loading

achingbrain Aug 13, 2023 •

edited

Loading

SgtPooki Aug 15, 2023 •

edited

Loading