[Merged by Bors] - Add Experimental QUIC support by AgeManning · Pull Request #4577 · sigp/lighthouse

AgeManning · 2023-08-08T04:19:01Z

Issue Addressed

Proposed Changes

This PR adds QUIC support to Lighthouse. As this is not officially spec'd this will only work between lighthouse <-> lighthouse connections. We attempt a QUIC connection (if the node advertises it) and if it fails we fallback to TCP.

This should be a backwards compatible modification. We want to test this functionality on live networks to observe any improvements in bandwidth/latency.

NOTE: This also removes the websockets transport as I believe no one is really using it. It should be mentioned in our release however.

Discovery and Quic. Listen on both when starting th swarm.

or quic transport with tcp

on handle_pending_outbound_connection

on GenerateEvent, and therefore remove unnecessary Enr fetching on Network.

discovery: append quic multiaddrs along with tcp,

it's only a one liner for PeerManager delegation of discovered peers.

- remove duplicated check for empty subnets - remove no longer used `enr_for_peer`

as we seem to be calling everytime we call dial_peer().

mxinden

Excited for this to land.

Let me know in case there is anything I can do to help.

Ideally I would like to merge libp2p/rust-libp2p#4304 before cutting a stable release. That said, happy to be convinced otherwise and cut a stable release of libp2p-quic earlier.

beacon_node/lighthouse_network/Cargo.toml

beacon_node/lighthouse_network/src/service/utils.rs

## Issue Addressed #4402 ## Proposed Changes This PR adds QUIC support to Lighthouse. As this is not officially spec'd this will only work between lighthouse <-> lighthouse connections. We attempt a QUIC connection (if the node advertises it) and if it fails we fallback to TCP. This should be a backwards compatible modification. We want to test this functionality on live networks to observe any improvements in bandwidth/latency. NOTE: This also removes the websockets transport as I believe no one is really using it. It should be mentioned in our release however. Co-authored-by: João Oliveira <hello@jxs.pt>

bors · 2023-09-13T02:33:04Z

Build failed:

release-tests-ubuntu

AgeManning · 2023-09-13T02:37:36Z

bors retry

## Issue Addressed #4402 ## Proposed Changes This PR adds QUIC support to Lighthouse. As this is not officially spec'd this will only work between lighthouse <-> lighthouse connections. We attempt a QUIC connection (if the node advertises it) and if it fails we fallback to TCP. This should be a backwards compatible modification. We want to test this functionality on live networks to observe any improvements in bandwidth/latency. NOTE: This also removes the websockets transport as I believe no one is really using it. It should be mentioned in our release however. Co-authored-by: João Oliveira <hello@jxs.pt>

bors · 2023-09-13T03:00:07Z

Build failed:

release-tests-ubuntu

AgeManning · 2023-09-13T04:29:32Z

bors r+

## Issue Addressed #4402 ## Proposed Changes This PR adds QUIC support to Lighthouse. As this is not officially spec'd this will only work between lighthouse <-> lighthouse connections. We attempt a QUIC connection (if the node advertises it) and if it fails we fallback to TCP. This should be a backwards compatible modification. We want to test this functionality on live networks to observe any improvements in bandwidth/latency. NOTE: This also removes the websockets transport as I believe no one is really using it. It should be mentioned in our release however. Co-authored-by: João Oliveira <hello@jxs.pt>

bors · 2023-09-13T05:14:53Z

Build failed:

debug-tests-ubuntu

This reverts commit 231e7db.

divagant-martian · 2023-09-13T05:58:51Z

AgeManning · 2023-09-13T06:07:17Z

bors r+

## Issue Addressed #4402 ## Proposed Changes This PR adds QUIC support to Lighthouse. As this is not officially spec'd this will only work between lighthouse <-> lighthouse connections. We attempt a QUIC connection (if the node advertises it) and if it fails we fallback to TCP. This should be a backwards compatible modification. We want to test this functionality on live networks to observe any improvements in bandwidth/latency. NOTE: This also removes the websockets transport as I believe no one is really using it. It should be mentioned in our release however. Co-authored-by: João Oliveira <hello@jxs.pt>

AgeManning · 2023-09-13T06:24:21Z

bors cancel :(

bors · 2023-09-13T06:24:23Z

Canceled.

put them before the ban status checks to avoid negative counts as each FromSwarm::ConnectionClosed event is always paired with an earlier FromSwarm::ConnectionEstablished, and when we ban before incrementing metrics we return earlier.

AgeManning · 2023-09-14T09:41:02Z

bors r+

## Issue Addressed #4402 ## Proposed Changes This PR adds QUIC support to Lighthouse. As this is not officially spec'd this will only work between lighthouse <-> lighthouse connections. We attempt a QUIC connection (if the node advertises it) and if it fails we fallback to TCP. This should be a backwards compatible modification. We want to test this functionality on live networks to observe any improvements in bandwidth/latency. NOTE: This also removes the websockets transport as I believe no one is really using it. It should be mentioned in our release however. Co-authored-by: João Oliveira <hello@jxs.pt>

bors · 2023-09-14T10:11:53Z

Build failed:

release-tests-ubuntu

AgeManning · 2023-09-15T03:07:06Z

bors r+

## Issue Addressed #4402 ## Proposed Changes This PR adds QUIC support to Lighthouse. As this is not officially spec'd this will only work between lighthouse <-> lighthouse connections. We attempt a QUIC connection (if the node advertises it) and if it fails we fallback to TCP. This should be a backwards compatible modification. We want to test this functionality on live networks to observe any improvements in bandwidth/latency. NOTE: This also removes the websockets transport as I believe no one is really using it. It should be mentioned in our release however. Co-authored-by: João Oliveira <hello@jxs.pt>

bors · 2023-09-15T04:00:47Z

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@michaelsproul

## Issue Addressed Since adding Quic support on #4577, and due to `quinn`s api nature LH now triggers the [`ListenerClosed`](https://docs.rs/libp2p/0.52.3/libp2p/swarm/struct.ListenerClosed.html) event.. @michaelsproul noticed we are logging this event as `crit!` independently of the reason. This PR matches the reason, logging with `debug!` and `error!` (instead of `crit!`) according to its `Result` ## Additional Info LH will still log `crit!` until libp2p/rust-libp2p#4621 has been merged

@michaelsproul

## Issue Addressed Since adding Quic support on sigp#4577, and due to `quinn`s api nature LH now triggers the [`ListenerClosed`](https://docs.rs/libp2p/0.52.3/libp2p/swarm/struct.ListenerClosed.html) event.. @michaelsproul noticed we are logging this event as `crit!` independently of the reason. This PR matches the reason, logging with `debug!` and `error!` (instead of `crit!`) according to its `Result` ## Additional Info LH will still log `crit!` until libp2p/rust-libp2p#4621 has been merged

This PR adds QUIC support to Lighthouse. As this is not officially spec'd this will only work between lighthouse <-> lighthouse connections. We attempt a QUIC connection (if the node advertises it) and if it fails we fallback to TCP. This should be a backwards compatible modification. We want to test this functionality on live networks to observe any improvements in bandwidth/latency. NOTE: This also removes the websockets transport as I believe no one is really using it. It should be mentioned in our release however. Co-authored-by: João Oliveira <hello@jxs.pt>

AgeManning and others added 12 commits August 2, 2023 17:01

Initial quic support

61d4ae1

enable Quic transport on libp2p

3eb4937

allow support for the two different UDP ports,

0ebdc79

Discovery and Quic. Listen on both when starting th swarm.

Merge pull request #5 from jxs/quic

1344786

or quic transport with tcp

Add ENR structure for quick and specify listening criteria

29db04d

discovery: append quic multiaddrs along with tcp,

5261b86

on handle_pending_outbound_connection

Add some documentation for protocol developers

7305595

discovery return Enr's instead of PeerId's

6c97c5a

on GenerateEvent, and therefore remove unnecessary Enr fetching on Network.

isolate multiaddresses gathering to dial on PeerManager

cd66aee

Merge pull request #6 from jxs/quic

f98f143

discovery: append quic multiaddrs along with tcp,

Small logging and comment changes

d8bb593

Merge latest unstable

d931d2a

AgeManning added the Networking label Aug 8, 2023

AgeManning and others added 15 commits August 8, 2023 14:23

Correct line spacing in book docs

4c9159c

Appease the all-wise all-knowing clippy

3492646

fmt

c4f0a38

Correct quic port and improve logs

583c233

Network: cleanup remove no longer required fn

1821941

it's only a one liner for PeerManager delegation of discovered peers.

Discovery:

ae54809

- remove duplicated check for empty subnets - remove no longer used `enr_for_peer`

PeerManager: move should_dial() logic to dial_peer()

191e0ae

as we seem to be calling everytime we call dial_peer().

cargo fmt

31f45b7

fmt

d3953e0

Remove unused var

dcc78cc

Can't change a single line without fmt complaining

c7c1749

Add QUIC to default ENR builder and fight tests

14b42c1

Merge branch 'quic' of github.com:AgeManning/lighthouse into quic

0ff3c92

cargo fmt

632a57b

remove leftover println's

2def400

mxinden reviewed Aug 10, 2023

View reviewed changes

beacon_node/lighthouse_network/Cargo.toml Outdated Show resolved Hide resolved

beacon_node/lighthouse_network/src/service/utils.rs Outdated Show resolved Hide resolved

fix benchmarks

7de61b5

Revert "Nuke windows self-hosted runner"

a4faad5

This reverts commit 231e7db.

beacon node: move connected peer metrics,

41c8207

put them before the ban status checks to avoid negative counts as each FromSwarm::ConnectionClosed event is always paired with an earlier FromSwarm::ConnectionEstablished, and when we ban before incrementing metrics we return earlier.

jxs force-pushed the quic branch from 85b2df5 to 41c8207 Compare September 13, 2023 13:55

bors bot changed the title ~~Add Experimental QUIC support~~ [Merged by Bors] - Add Experimental QUIC support Sep 15, 2023

bors bot closed this Sep 15, 2023

jxs mentioned this pull request Sep 18, 2023

Add QUIC support #4402

Closed

jxs mentioned this pull request Oct 10, 2023

[Merged by Bors] - remove crit! logging from ListenerClosed event on Ok() #4821

Closed

Conversation

AgeManning commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue Addressed

Proposed Changes

Uh oh!

mxinden left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bors bot commented Sep 13, 2023

Uh oh!

AgeManning commented Sep 13, 2023

Uh oh!

bors bot commented Sep 13, 2023

Uh oh!

AgeManning commented Sep 13, 2023

Uh oh!

bors bot commented Sep 13, 2023

Uh oh!

divagant-martian commented Sep 13, 2023

Uh oh!

AgeManning commented Sep 13, 2023

Uh oh!

AgeManning commented Sep 13, 2023

Uh oh!

bors bot commented Sep 13, 2023

Uh oh!

AgeManning commented Sep 14, 2023

Uh oh!

bors bot commented Sep 14, 2023

Uh oh!

AgeManning commented Sep 15, 2023

Uh oh!

bors bot commented Sep 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AgeManning commented Aug 8, 2023 •

edited

Loading