bip39-rs icon indicating copy to clipboard operation
bip39-rs copied to clipboard

Doesn't pass BIP-0039 test vectors

Open stevenroose opened this issue 6 years ago • 5 comments

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.

stevenroose avatar Jul 26 '19 13:07 stevenroose

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.

vorot93 avatar Jul 26 '19 19:07 vorot93

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.

stevenroose avatar Jul 29 '19 10:07 stevenroose

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

gilescope avatar Jul 30 '19 05:07 gilescope

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.

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).

maciejhirsz avatar Jul 30 '19 11:07 maciejhirsz