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

feat: quic #1265

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

feat: quic #1265

wants to merge 8 commits into from

Conversation

richard-ramos
Copy link
Member

@richard-ramos richard-ramos commented Feb 20, 2025

This PR enables quic transport with a proper TLS backend, and tests connection and comms between two nodes. There are still some pending items like dealing with port 0 in multiaddresses, as well as well as ensuring that connections/streams lifetime is handled correctly. This is to be done in upcoming PRs

Do note that one of the commits changes the not implemented assertion to something that gives a bit more context, so it's easier to know which abstract method a dev forgot to implement.

@richard-ramos richard-ramos force-pushed the tls branch 14 times, most recently from c1294d4 to 8d7e551 Compare March 3, 2025 20:40
Base automatically changed from tls to master March 4, 2025 12:05
@richard-ramos richard-ramos force-pushed the tls-quic branch 5 times, most recently from 18e5b7f to 55c664a Compare March 5, 2025 20:20
@richard-ramos richard-ramos marked this pull request as ready for review March 5, 2025 22:52
@richard-ramos
Copy link
Member Author

Looks like there are some issues with building mbed-tls in windows, macos(arm64) and linux(i386).
Will try to fix this in #1278 as it seems that certificate generation has trouble on those OSs

Copy link
Member

@vladopajic vladopajic left a comment

Choose a reason for hiding this comment

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

lgtm


let keypair = KeyPair(seckey: self.privateKey, pubkey: pubkey)

let certPair = generate(keypair, EncodingFormat.PEM)
Copy link
Member

@vladopajic vladopajic Mar 6, 2025

Choose a reason for hiding this comment

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

  • generate what? it's deductible from context that certificate is created but better naming would be appreciated. eg. certificate.generate or generateCertificate ...

  • also generate returns tuple, but why not named tuple? or why not simple type? usage like certPair[0], certPair[1] is not really readable and understandable unless you go into the function.

i know it's not related to this pr, but if you agree we can create issue for this, ideal for "good first issues" lable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants