infincia / bip39-rs

A Rust implementation of BIP-0039
Apache License 2.0
54 stars 59 forks source link

Doesn't pass BIP-0039 test vectors #21

Open stevenroose opened 5 years ago

stevenroose commented 5 years ago

This crate has a bug. I noticed it doesn't have any unit test that verifies the implementation against the test vectors in the BIP39 spec.

    #[test]
    fn test_bip39() {
        let v_entropy = "68a79eaca2324873eacc50cb9c6eca8cc68ea5d936f98787c60c7ebc74e6ce7c";
        let v_mnem = "hamster diagram private dutch cause delay private meat slide toddler razor book happy fancy gospel tennis maple dilemma loan word shrug inflict delay length";
        let v_seed = "64c87cde7e12ecf6704ab95bb1408bef047c22db4cc7491c4271d170a1b213d20b385bc1588d9c7b38f1b39d415665b8a9030c9ec653d75e65f847d8fc1fc440";
        let v_passphrase = "TREZOR";

        let mnem = bip39::Mnemonic::from_phrase(v_mnem, bip39::Language::English).unwrap();
        let seed = bip39::Seed::new(&mnem, "TREZOR");
        assert_eq!(v_seed, &hex::encode(&seed.as_bytes()));

    }

That example is director from the spec and it fails.

vorot93 commented 5 years ago

Where in the spec did you find this example? The full list is here:

https://github.com/trezor/python-mnemonic/blob/master/vectors.json

Also, this library has been abandoned for a long time with tiny-bip39 taking its place.

stevenroose commented 5 years ago

My apologies. Don't remember where that test vector came from. Updated with a vector from the list; same result.

If the crate is abandoned, perhaps tiny-bip39 should take over the namespace bip39 or the bip39 crate should be flagged abandoned somehow. Especially if it's not spec-compliant it can cause fund loss.

gilescope commented 5 years ago

Please please please if it is not used any more can you direct people to the better crate on the main readme?

wigy-opensource-developer commented 5 years ago

This crate has a bug. I noticed it doesn't have any unit test that verifies the implementation against the test vectors in the BIP39 spec.

Actually, it is "only" the v0.6.0-beta.1 that has these problems. The v0.5.1 passes all tests. My impression is that @steveatinfincia merged in pull requests from @maciejhirsz and @QuestofIranon and then regretted those braking changes on the API, so he did not pull in the actual fixes provided by them.

If you prefer the API of 0.5, use this crate, if you prefer the API of 0.6, use tiny-bip39. We at Internet of People decided to go with the new API.

maciejhirsz commented 5 years ago

Looking over at crates.io, the number of downloads for 0.6.0-beta1 is troubling. I reckon it should at very least be yanked to keep people on 0.5.1 (CC @steveatinfincia).