Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dcutr #824

Merged
merged 97 commits into from
Apr 14, 2023
Merged

Dcutr #824

merged 97 commits into from
Apr 14, 2023

Conversation

diegomrsantos
Copy link
Contributor

@diegomrsantos diegomrsantos commented Dec 19, 2022

This PR partially implements https://github.com/libp2p/specs/blob/master/relay/DCUtR.md. Currently, only the TCP protocol is supported. Additionally, the unilateral connection upgrade is handled by the Hole Punching service instead.

@diegomrsantos diegomrsantos marked this pull request as draft December 19, 2022 18:01
@codecov
Copy link

codecov bot commented Dec 19, 2022

Codecov Report

Merging #824 (8e8b277) into unstable (80cca0e) will decrease coverage by 0.01%.
The diff coverage is 83.84%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           unstable     #824      +/-   ##
============================================
- Coverage     84.21%   84.21%   -0.01%     
============================================
  Files            87       90       +3     
  Lines         15125    15239     +114     
============================================
+ Hits          12738    12833      +95     
- Misses         2387     2406      +19     
Impacted Files Coverage Δ
libp2p/protocols/connectivity/dcutr/client.nim 71.73% <75.00%> (ø)
libp2p/protocols/connectivity/dcutr/server.nim 72.72% <75.00%> (ø)
libp2p/dial.nim 46.03% <100.00%> (-0.75%) ⬇️
libp2p/dialer.nim 93.72% <100.00%> (+0.63%) ⬆️
libp2p/peerstore.nim 98.97% <100.00%> (+1.04%) ⬆️
libp2p/protocols/connectivity/dcutr/core.nim 100.00% <100.00%> (ø)
libp2p/switch.nim 91.13% <100.00%> (-0.05%) ⬇️
libp2p/transports/transport.nim 90.19% <100.00%> (-0.72%) ⬇️
libp2p/upgrademngrs/muxedupgrade.nim 90.27% <100.00%> (+0.39%) ⬆️
libp2p/upgrademngrs/upgrade.nim 96.87% <100.00%> (-0.27%) ⬇️

... and 3 files with indirect coverage changes

@Menduist Menduist mentioned this pull request Dec 21, 2022
Base automatically changed from transport-hole-punching to unstable April 6, 2023 13:23
import ./helpers

type
SwitchStub* = ref object of Switch
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be moved to its own file in the HP PR.

libp2p/dialer.nim Outdated Show resolved Hide resolved

if peerDialableAddrs.len > maxDialableAddrs:
peerDialableAddrs = peerDialableAddrs[0..<maxDialableAddrs]
var futs = peerDialableAddrs.mapIt(switch.connect(stream.peerId, @[it], forceDial = true, reuseConnection = false))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other side

await conn.writeLp(pb.buffer)

proc getHolePunchableAddrs*(addrs: seq[MultiAddress]): seq[MultiAddress] =
addrs.filterIt(TCP.matchPartial(it))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure matchPartial is appropriate, it will also match WS addresses, no?

Copy link
Contributor Author

@diegomrsantos diegomrsantos Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to match.

Menduist
Menduist previously approved these changes Apr 14, 2023
Copy link
Contributor

@Menduist Menduist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM expect last comment to check

@diegomrsantos diegomrsantos merged commit b7726bf into unstable Apr 14, 2023
@diegomrsantos diegomrsantos deleted the dcutr branch April 14, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants