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

Make QUIC The Default Transport #14688

Merged
merged 4 commits into from
Dec 3, 2024
Merged

Make QUIC The Default Transport #14688

merged 4 commits into from
Dec 3, 2024

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Dec 3, 2024

What type of PR is this?

Feature

What does this PR do? Why is it needed?

This makes QUIC the default transport protocol in prysm and will make every prysm node connect to other nodes via QUIC if supported.

Which issues(s) does this PR fix?

N.A

Other notes for review

Acknowledgements

  • I have read CONTRIBUTING.md.
  • I have made an appropriate entry to CHANGELOG.md.
  • I have added a description to this PR with sufficient context for reviewers to understand this PR.

@nisdas nisdas added the Ready For Review A pull request ready for code review label Dec 3, 2024
@nisdas nisdas requested a review from a team as a code owner December 3, 2024 08:16
@prestonvanloon
Copy link
Member

This would be a good time to update quic-go to the latest. It has been a while since we updated it.

@nalepae nalepae marked this pull request as draft December 3, 2024 15:06
@prestonvanloon prestonvanloon marked this pull request as ready for review December 3, 2024 16:01
Copy link
Member

@prestonvanloon prestonvanloon left a comment

Choose a reason for hiding this comment

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

I reviewed libp2p updates and LGTM

@nalepae
Copy link
Contributor

nalepae commented Dec 3, 2024

I reviewed libp2p updates and LGTM

(Reviewed them as well!)

@nalepae nalepae added this pull request to the merge queue Dec 3, 2024
Merged via the queue into develop with commit d09885b Dec 3, 2024
15 checks passed
@nalepae nalepae deleted the makeQUICDefault branch December 3, 2024 17:07
@skylenet
Copy link

skylenet commented Dec 4, 2024

Hey folks. Just making a note that this could be breaking someones setup if they used the 13000 port for -p2p-udp-port . Maybe it's worth to add a note to the release notes / changelog. This happened to us on our kurtosis setup and I was wondering why it was telling me that port 13000 is already beeing used. Until I noticed that there's this new quic port.

skylenet added a commit to ethpandaops/ethereum-package that referenced this pull request Dec 4, 2024
…he new quic port (#845)

Prysm added QUIC to their client as per
prysmaticlabs/prysm#14688 . This currently broke
our runs using the master branch prysm image due to the QUIC port
running on 13000 by default. Which was conflicting with the settings we
were providing for the UDP port:

```sh
--p2p-quic-port value                                The QUIC port used by libp2p. (default: 13000)
--p2p-tcp-port value                                 The TCP port used by libp2p. (default: 13000)
--p2p-udp-port value                                 The UDP port used by the discovery service discv5. (default: 12000)
```

I've also added the CLI flags for it, but still commented out until a
stable release is done.
@nisdas
Copy link
Member Author

nisdas commented Dec 4, 2024

Thanks for bringing this up @skylenet, that is a good point. We will add this to our release notes to flag that this port will be taken up.

@kasey kasey added the changelog/changed Changelog Section: Changed label Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/changed Changelog Section: Changed Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants