Merge pull request #8 from rust-bitcoin/sebas-comments

Fixing minor things

Author: stevenroose

.travis.yml

@@ -19,8 +19,8 @@ before_install:
 script:
   - cargo build --verbose
   - cargo test --verbose
-  - cargo build --verbose --no-default-features
-  - cargo test --verbose --no-default-features
+  - cargo build --verbose --no-default-features --features all-languages
+  - cargo test --verbose --no-default-features --features all-languages
   - cargo build --verbose --features rand,all-languages
   - cargo test --verbose --features rand,all-languages
   # benchmarks

Cargo.toml

@@ -16,7 +16,7 @@ path = "src/lib.rs"
 
 [features]
 default = [ "std" ]
-std = []
+std = [ "unicode-normalization", "serde/std" ]
 
 # Note: English is the standard for bip39 so always included
 chinese-simplified = []
@@ -41,10 +41,11 @@ all-languages = [
 
 [dependencies]
 bitcoin_hashes = "0.9.4"
-unicode-normalization = "=0.1.9"
+rand_core = "0.4.0"
 
+unicode-normalization = { version = "=0.1.9", optional = true }
 rand = { version = "0.6.0", optional = true }
-serde = { version = "1.0", optional = true }
+serde = { version = "1.0", default-features = false, optional = true }
 
 [dev-dependencies]
 rand = { version = "0.6.0", optional = false }

src/language/mod.rs

@@ -1,6 +1,5 @@
 
-#[cfg(feature = "std")]
-use std::fmt;
+use core::fmt;
 
 mod english;
 #[cfg(feature = "chinese-simplified")]
@@ -153,7 +152,6 @@ impl Language {
 	}
 }
 
-#[cfg(feature = "std")]
 impl fmt::Display for Language {
 	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
 		fmt::Debug::fmt(self, f)

src/lib.rs

@@ -27,9 +27,13 @@
 #![deny(missing_docs)]
 
 #![cfg_attr(all(not(test), not(feature = "std")), no_std)]
-#[cfg(any(test, feature = "std"))] pub extern crate core;
+
+#[cfg(any(test, feature = "std"))]
+pub extern crate core;
 
 extern crate bitcoin_hashes;
+extern crate rand_core;
+
 #[cfg(feature = "std")]
 extern crate unicode_normalization;
 
@@ -38,8 +42,10 @@ extern crate rand;
 #[cfg(feature = "serde")]
 pub extern crate serde;
 
+use core::{fmt, str};
+
 #[cfg(feature = "std")]
-use std::{error, fmt, str};
+use std::error;
 #[cfg(feature = "std")]
 use std::borrow::Cow;
 
@@ -75,7 +81,6 @@ impl AmbiguousLanguages {
 	}
 
 	/// An iterator over the possible languages.
-	#[cfg(feature = "std")]
 	pub fn iter(&self) -> impl Iterator<Item = Language> + '_ {
 		Language::all().iter().enumerate().filter(move |(i, _)| self.0[*i]).map(|(_, l)| *l)
 	}
@@ -106,7 +111,6 @@ pub enum Error {
 	AmbiguousLanguages(AmbiguousLanguages),
 }
 
-#[cfg(feature = "std")]
 impl fmt::Display for Error {
 	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
 		match *self {
@@ -120,7 +124,17 @@ impl fmt::Display for Error {
 				"entropy was not between 128-256 bits or not a multiple of 32 bits: {} bits", c,
 			),
 			Error::InvalidChecksum => write!(f, "the mnemonic has an invalid checksum"),
-			Error::AmbiguousLanguages(a) => write!(f, "ambiguous word list: {:?}", a.to_vec()),
+			Error::AmbiguousLanguages(a) => {
+				write!(f, "ambiguous word list: ")?;
+				for (i, lang) in a.iter().enumerate() {
+					if i == 0 {
+						write!(f, "{}", lang)?;
+					} else {
+						write!(f, ", {}", lang)?;
+					}
+				}
+				Ok(())
+			}
 		}
 	}
 }
@@ -130,7 +144,7 @@ impl error::Error for Error {}
 
 /// A mnemonic code.
 ///
-/// The [std::str::FromStr] implementation will try to determine the language of the
+/// The [core::str::FromStr] implementation will try to determine the language of the
 /// mnemonic from all the supported languages. (Languages have to be explicitly enabled using
 /// the Cargo features.)
 ///
@@ -202,23 +216,63 @@ impl Mnemonic {
 		Mnemonic::from_entropy_in(Language::English, entropy)
 	}
 
-	/// Generate a new [Mnemonic] in the given language.
+	/// Generate a new [Mnemonic] in the given language
+	/// with the given randomness source.
 	/// For the different supported word counts, see documentation on [Mnemonic].
-	#[cfg(feature = "rand")]
-	pub fn generate_in(language: Language, word_count: usize) -> Result<Mnemonic, Error> {
+	///
+	/// Example:
+	///
+	/// ```
+	/// extern crate rand;
+	/// extern crate bip39;
+	///
+	/// use bip39::{Mnemonic, Language};
+	///
+	/// let mut rng = rand::thread_rng();
+	/// let m = Mnemonic::generate_in_with(&mut rng, Language::English, 24).unwrap();
+	/// ```
+	pub fn generate_in_with<R>(rng: &mut R, language: Language, word_count: usize) -> Result<Mnemonic, Error>
+		where R: rand_core::RngCore + rand_core::CryptoRng,
+	{
 		if word_count < MIN_NB_WORDS || word_count % 6 != 0 || word_count > MAX_NB_WORDS {
 			return Err(Error::BadWordCount(word_count));
 		}
 
 		let entropy_bytes = (word_count / 3) * 4;
-		let mut rng = rand::thread_rng();
 		let mut entropy = [0u8; (MAX_NB_WORDS / 3) * 4];
-		rand::RngCore::fill_bytes(&mut rng, &mut entropy[0..entropy_bytes]);
+		rand_core::RngCore::fill_bytes(rng, &mut entropy[0..entropy_bytes]);
 		Mnemonic::from_entropy_in(language, &entropy[0..entropy_bytes])
 	}
 
+	/// Generate a new [Mnemonic] in the given language.
+	/// For the different supported word counts, see documentation on [Mnemonic].
+	///
+	/// Example:
+	///
+	/// ```
+	/// extern crate bip39;
+	///
+	/// use bip39::{Mnemonic, Language};
+	///
+	/// let m = Mnemonic::generate_in(Language::English, 24).unwrap();
+	/// ```
+	#[cfg(feature = "rand")]
+	pub fn generate_in(language: Language, word_count: usize) -> Result<Mnemonic, Error> {
+		Mnemonic::generate_in_with(&mut rand::thread_rng(), language, word_count)
+	}
+
 	/// Generate a new [Mnemonic] in English.
 	/// For the different supported word counts, see documentation on [Mnemonic].
+	///
+	/// Example:
+	///
+	/// ```
+	/// extern crate bip39;
+	///
+	/// use bip39::{Mnemonic,};
+	///
+	/// let m = Mnemonic::generate(24).unwrap();
+	/// ```
 	#[cfg(feature = "rand")]
 	pub fn generate(word_count: usize) -> Result<Mnemonic, Error> {
 		Mnemonic::generate_in(Language::English, word_count)
@@ -240,18 +294,8 @@ impl Mnemonic {
 				return Err(Error::BadWordCount(0));
 			}
 
-			// For efficiency reasons, we add a special case for when there's
-			// only a single language enabled.
-			if langs.len() == 1 {
-				let lang = langs[0];
-				return if lang.find_word(first_word).is_some() {
-					Ok(lang)
-				} else {
-					Err(Error::UnknownWord(0))
-				};
-			}
-
-			// Otherwise we first try wordlists that have guaranteed unique words.
+			// We first try find the first word in wordlists that
+			// have guaranteed unique words.
 			for language in langs.iter().filter(|l| l.unique_words()) {
 				if language.find_word(first_word).is_some() {
 					return Ok(*language);
@@ -271,20 +315,22 @@ impl Mnemonic {
 		for (idx, word) in words.enumerate() {
 			// Scrap languages that don't have this word.
 			for (i, lang) in langs.iter().enumerate() {
-				if possible[i] {
-					possible[i] = lang.find_word(word).is_some();
-				}
+				possible[i] &= lang.find_word(word).is_some();
 			}
 
-			// If there is just one language left, return it.
-			let nb_possible = possible.iter().filter(|p| **p).count();
-			if nb_possible == 1 {
-				return Ok(*possible.iter().zip(langs.iter()).find(|&(p, _)| *p).map(|(_, l)| l).unwrap());
-			}
-
-			// If all languages were eliminated, it's an invalid word.
-			if nb_possible == 0 {
-				return Err(Error::UnknownWord(idx));
+			// Get an iterator over remaining possible languages.
+			let mut iter = possible.iter().zip(langs.iter()).filter(|(p, _)| **p).map(|(_, l)| l);
+
+			match iter.next() {
+				// If all languages were eliminated, it's an invalid word.
+				None => return Err(Error::UnknownWord(idx)),
+				// If not, see if there is a second one remaining.
+				Some(remaining) => {
+					if iter.next().is_none() {
+						// No second remaining, we found our language.
+						return Ok(*remaining);
+					}
+				}
 			}
 		}
 
@@ -389,8 +435,9 @@ impl Mnemonic {
 		const PBKDF2_ROUNDS: usize = 2048;
 		const PBKDF2_BYTES: usize = 64;
 
+		let nb_words = self.word_count();
 		let mut seed = [0u8; PBKDF2_BYTES];
-		pbkdf2::pbkdf2(&self.0, normalized_passphrase.as_bytes(), PBKDF2_ROUNDS, &mut seed);
+		pbkdf2::pbkdf2(&self.0[0..nb_words], normalized_passphrase.as_bytes(), PBKDF2_ROUNDS, &mut seed);
 		seed
 	}
 
@@ -451,7 +498,6 @@ impl Mnemonic {
 	}
 }
 
-#[cfg(feature = "std")]
 impl fmt::Display for Mnemonic {
 	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
 		for i in 0..self.0.len() {
@@ -468,12 +514,14 @@ impl fmt::Display for Mnemonic {
 	}
 }
 
-#[cfg(feature = "std")]
 impl str::FromStr for Mnemonic {
 	type Err = Error;
 
 	fn from_str(s: &str) -> Result<Mnemonic, Error> {
-		Mnemonic::parse(s)
+		#[cfg(feature = "std")]
+		{ Mnemonic::parse(s) }
+		#[cfg(not(feature = "std"))]
+		{ Mnemonic::parse_normalized(s) }
 	}
 }
 
@@ -483,17 +531,6 @@ mod tests {
 
 	use bitcoin_hashes::hex::FromHex;
 
-	#[cfg(feature = "rand")]
-	#[test]
-	fn test_bit_counts() {
-		let m = Mnemonic::generate(12).unwrap();
-		assert_eq!(m.word_count(), 12);
-		let m = Mnemonic::generate(18).unwrap();
-		assert_eq!(m.word_count(), 18);
-		let m = Mnemonic::generate(24).unwrap();
-		assert_eq!(m.word_count(), 24);
-	}
-
 	#[cfg(feature = "rand")]
 	#[test]
 	fn test_language_of() {
@@ -524,6 +561,14 @@ mod tests {
 		assert_eq!(amb.iter().collect::<Vec<_>>(), present_vec);
 	}
 
+	#[cfg(feature = "rand")]
+	#[test]
+	fn test_generate() {
+		let _ = Mnemonic::generate(24).unwrap();
+		let _ = Mnemonic::generate_in(Language::English, 24).unwrap();
+		let _ = Mnemonic::generate_in_with(&mut rand::thread_rng(), Language::English, 24).unwrap();
+	}
+
 	#[test]
 	fn test_vectors_english() {
 		// These vectors are tuples of

src/pbkdf2.rs

@@ -8,9 +8,6 @@ fn mnemonic_byte_len(mnemonic: &[&'static str]) -> usize {
 	let mut len = 0;
 	for i in 0..mnemonic.len() {
 		let word = &mnemonic[i];
-		if word.is_empty() {
-			break;
-		}
 		if i > 0 {
 			len += 1;
 		}
@@ -23,9 +20,6 @@ fn mnemonic_byte_len(mnemonic: &[&'static str]) -> usize {
 fn mnemonic_write_into(mnemonic: &[&'static str], engine: &mut sha512::HashEngine) {
 	for i in 0..mnemonic.len() {
 		let word = &mnemonic[i];
-		if word.is_empty() {
-			break;
-		}
 		if i > 0 {
 			engine.input(" ".as_bytes());
 		}
@@ -61,9 +55,6 @@ fn create_hmac_engine(mnemonic: &[&'static str]) -> hmac::HmacEngine<sha512::Has
 		let mut cursor = 0;
 		for i in 0..mnemonic.len() {
 			let word = &mnemonic[i];
-			if word.is_empty() {
-				break;
-			}
 			if i > 0 {
 				ipad[cursor] ^= ' ' as u8;
 				opad[cursor] ^= ' ' as u8;
@@ -88,8 +79,6 @@ fn create_hmac_engine(mnemonic: &[&'static str]) -> hmac::HmacEngine<sha512::Has
 // Method borrowed from rust-bitcoin's endian module.
 #[inline]
 fn u32_to_array_be(val: u32) -> [u8; 4] {
-	debug_assert_eq!(::core::mem::size_of::<u32>(), 4); // size_of isn't a constfn in 1.22
-
 	let mut res = [0; 4];
 	for i in 0..4 {
 		res[i] = ((val >> (4 - i - 1)*8) & 0xff) as u8;