Review notes - Githubissues

infincia / bip39-rs

A Rust implementation of BIP-0039

Apache License 2.0

54 stars 61 forks source link

Review notes #20

Open dpc opened 4 years ago

dpc commented 4 years ago

Hi,

I've reviewed this crate using cargo-crev.

Some notes:

maybe you should not implement Debug for Seed and Mnemonic. Debug makes it easy to accidentally log them somewhere and thus leak them.
there are multiple issues that cargo clippy will point out - nothing serious, but it is a good idea to just do what Clippy says. :)

vorot93 commented 4 years ago

Your whole comment comes across as a giant coat rack. Too little substance about this project, too much about giving publicity to your tool.

Just sayin'.

dpc commented 4 years ago

@vorot93 I'm not aware of "coat rack" phrase, but if you mean that I link to the review to increase visibility to increase the awareness of it, then it's true. Shamelessly. :)

As this is a bitcoin-related crate, a review by a reasonably reputable person is a valuable thing (IMO). I've spent around 20-30 minutes going through the code and reviewing it and I'm letting the author know that i did and what I think could improve, with a cryptographic proof. Some companies would pay me money for it.

So I think it's a fair game. If somebody minds, feel free to delete/close.