Organize some of the duplicated TLS code into a separate crate #3835
+161
−204
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
November 28, 2024 11:57
… to avoid duplication
KirillLykov
reviewed
Dec 2, 2024
|
|
||
| [dependencies] | ||
| solana-sdk = { workspace = true } | ||
|
|
KirillLykov
reviewed
Dec 2, 2024
| solana-signer = { workspace = true } | ||
| solana-pubkey = { workspace = true } | ||
|
|
||
|
|
KirillLykov
reviewed
Dec 2, 2024
| @@ -0,0 +1,14 @@ | |||
| //! Collection of TLS related code fragments that end up popping up everywhere where quic is used. | |||
| //! Aggregated here to avoid bugs due to conflicting implementations of the same functionality | |||
There was a problem hiding this comment.
Suggested change
| //! Aggregated here to avoid bugs due to conflicting implementations of the same functionality | |
| //! Aggregated here to avoid bugs due to conflicting implementations of the same functionality. |
KirillLykov
reviewed
Dec 2, 2024
| @@ -0,0 +1 @@ | |||
| A collection of utility functions and structures needed to bridge the conceptual gap between conventional TLS security model in protocols like QUIC and what Solana does | |||
There was a problem hiding this comment.
Suggested change
| A collection of utility functions and structures needed to bridge the conceptual gap between conventional TLS security model in protocols like QUIC and what Solana does | |
| A collection of utility functions and structures needed to bridge the conceptual gap between conventional TLS security model in protocols like QUIC and what Solana does. |
|
Looks like a good change to me |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
A whole bunch of places in the code have been defining the same structure SkipServerVerification for use in quic related code. There were at least 3 independent definitions for this struct with zero functional differences in different project locations.
This created unnecessary duplication as well as strange code dependencies where turbine would depend on quic-client for this one struct, even though logically it makes no sense for turbine to depend on quic-client.
Summary of Changes
The following moved to tls-utils crate:
SkipServerVerification
SkipClientVerification
new_dummy_x509_certificate
All references to them redirected. Multiple redundant definitions removed.