Switch to use rust-bitcoin's ChaCha20Poly1305 - #42
Conversation
We take a dependency on `rust-bitcoin`'s new-ish `chacha20poly1305` crate and check compatibility with our previous hand-rolled implementation (might be redundant, but better safe than sorry).
|
👋 Thanks for assigning @tankyleo as a reviewer! |
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Basically LGTM, do we have fuzzers for these things?
|
🔔 1st Reminder Hey @tankyleo! This PR has been waiting for your review. |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Addressed pending comments.
do we have fuzzers for these things?
Not yet, might be worth to add eventually.
|
🔔 2nd Reminder Hey @tankyleo! This PR has been waiting for your review. |
tankyleo
left a comment
tankyleo
left a comment
There was a problem hiding this comment.
Compat tests passed on my machine (cargo test check_chacha)
|
Addressed all pending comments, let me know if I can squash. |
tankyleo
left a comment
tankyleo
left a comment
There was a problem hiding this comment.
Took another pass good to squash thank you.
We delete our homebrewn implementation and switch to use the recently-published `chacha20_poly1305` crate.
cdd6577 to
c1e572e
Compare
Thanks! Squashed without further changes! |
| let (wrapped_nonce_bytes, wrapped_nonce_tag_bytes) = remaining.split_at(NONCE_LENGTH); | ||
|
|
||
| let mut wrapped_nonce_tag = [0u8; TAG_LENGTH]; | ||
| wrapped_nonce_tag.copy_from_slice(&wrapped_nonce_tag_bytes[..TAG_LENGTH]); |
There was a problem hiding this comment.
a nit for the future: wrapped_nonce_tag_bytes is guaranteed to be of length TAG_LENGTH here
There was a problem hiding this comment.
a nit for the future:
wrapped_nonce_tag_bytesis guaranteed to be of lengthTAG_LENGTHhere
Yes, I'm aware, belt-and-suspenders it is!
|
🔔 1st Reminder Hey @TheBlueMatt! This PR has been waiting for your review. |
|
Going ahead landing this |
4 participants
Originally, we copied over our
ChaCha20Poly1305implementation from LDK. However, recentlyrust-bitcoinhas published thechacha20_poly1305crate, also including some performance improvements.Here, we switch over to use that new crate and drop our howmbrewn implementation (after adding a - probably unnecessary - testing step asserting that the two implementations are actually compatible).