-
Notifications
You must be signed in to change notification settings - Fork 436
Switch chacha #4360
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
base: main
Are you sure you want to change the base?
Switch chacha #4360
Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
TheBlueMatt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should try to remove the existing chacha20poly1305rfc module entirely. Is there some limitations of the upstream implementation that prevented that?
|
@TheBlueMatt at the moment, |
4f42780 to
388bbc2
Compare
TheBlueMatt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you still didn't manage to remove chacha20poly1305rfc.rs (and chacha20.rs and poly1305.rs)?
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
| struct ChaChaDualPolyReader<'a, R: Read> { | ||
| chacha: &'a mut ChaCha20, | ||
| poly: &'a mut Poly1305, | ||
| poly_aad: &'a mut Poly1305, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, we definitely don't want to do the full poly1305 validation twice. We'll have to upstream a Clone for Poly1305, mind opening a PR upstream at https://github.com/rust-bitcoin/rust-bitcoin/?
Integrated the crate across all modules (Router, PeerStorage, Onion Utils, etc.) and removed the deprecated internal implementation. Updated test vectors to align with the new 12-byte nonce construction and ensured SCID encryption remains consistent.
4ef960c to
7f74979
Compare
closes #4316
This PR switches the ChaChaPoly encryption algorithm to the
chacha20-poly1305 = "0.1.2"crate, the following impls have been modifiedEncryptedOurPeerStoragePeerChannelEncryptorThe
chacha20-poly1305dependency was only added because the issue description by @TheBlueMatt requested it be added, and also, thanks to @tnull for the reference implementation provided.