fix: seed word parsing

Moved detect_language into MnemonicLanguage and made it public.
Prevented a TariSeedWords object from becoming invalid in the event an invalid or inconsistent word was attempted to be pushed to it in wallet_ffi.
Differentiated between an invalid word and an inconsistent word.
Added word to string of WordNotFound error.

Review comments

base_layer/key_manager/src/error.rs

@@ -51,8 +51,8 @@ pub enum MnemonicError {
          defined natural languages"
     )]
     UnknownLanguage,
-    #[error("Only 2048 words for each language was selected to form Mnemonic word lists")]
-    WordNotFound,
+    #[error("Word not found: `{0}`")]
+    WordNotFound(String),
     #[error("A mnemonic word does not exist for the requested index")]
     IndexOutOfBounds,
     #[error("A problem encountered constructing a secret key from bytes or mnemonic sequence: `{0}`")]

base_layer/key_manager/src/mnemonic.rs

@@ -48,7 +48,7 @@ impl MnemonicLanguage {
     /// Detects the mnemonic language of a specific word by searching all defined mnemonic word lists
     pub fn from(mnemonic_word: &str) -> Result<MnemonicLanguage, MnemonicError> {
         let words = vec![mnemonic_word.to_string()];
-        detect_language(&words)
+        MnemonicLanguage::detect_language(&words)
     }
 
     /// Returns an iterator for the MnemonicLanguage enum group to allow iteration over all defined languages
@@ -77,6 +77,63 @@ impl MnemonicLanguage {
             MnemonicLanguage::Spanish => MNEMONIC_SPANISH_WORDS.len(),
         }
     }
+
+    /// Detects the language of a list of words
+    pub fn detect_language(words: &[String]) -> Result<MnemonicLanguage, MnemonicError> {
+        let count = words.iter().len();
+        match count.cmp(&1) {
+            Ordering::Less => {
+                return Err(MnemonicError::UnknownLanguage);
+            },
+            Ordering::Equal => {
+                let word = words.get(0).ok_or(MnemonicError::EncodeInvalidLength)?;
+                for language in MnemonicLanguage::iterator() {
+                    if find_mnemonic_index_from_word(word, language).is_ok() {
+                        return Ok(*language);
+                    }
+                }
+                return Err(MnemonicError::UnknownLanguage);
+            },
+            Ordering::Greater => {
+                for word in words {
+                    let mut languages = Vec::with_capacity(MnemonicLanguage::iterator().len());
+                    // detect all languages in which a word falls into
+                    for language in MnemonicLanguage::iterator() {
+                        if find_mnemonic_index_from_word(word, language).is_ok() {
+                            languages.push(*language);
+                        }
+                    }
+                    // check if at least one of the languages is consistent for all other words against languages
+                    // yielded from the initial word for this iteration
+                    for language in languages {
+                        let mut consistent = true;
+                        for compare in words {
+                            if compare != word && find_mnemonic_index_from_word(compare, &language).is_err() {
+                                consistent = false;
+                            }
+                        }
+                        if consistent {
+                            return Ok(language);
+                        }
+                    }
+                }
+            },
+        }
+
+        Err(MnemonicError::UnknownLanguage)
+    }
+
+    /// Finds the first word that is inconsistent in the set of words for a specified language. If multiple
+    /// inconsistent words exist it will need to be run repeatedly, removing the inconsistent word after each iteration
+    pub fn find_first_inconsistent_word(words: &[String], language: &MnemonicLanguage) -> Option<String> {
+        for word in words {
+            match find_mnemonic_index_from_word(word, language).is_err() {
+                true => return Some(word.to_string()),
+                false => {},
+            };
+        }
+        None
+    }
 }
 
 /// Finds and returns the index of a specific word in a mnemonic word list defined by the specified language
@@ -106,7 +163,7 @@ fn find_mnemonic_index_from_word(word: &str, language: &MnemonicLanguage) -> Res
     }
     match search_result {
         Ok(v) => Ok(v),
-        Err(_err) => Err(MnemonicError::WordNotFound),
+        Err(_err) => Err(MnemonicError::WordNotFound(word.to_string())),
     }
 }
 
@@ -154,54 +211,10 @@ pub fn from_bytes(bytes: Vec<u8>, language: &MnemonicLanguage) -> Result<Vec<Str
     Ok(mnemonic_sequence)
 }
 
-fn detect_language(words: &[String]) -> Result<MnemonicLanguage, MnemonicError> {
-    let count = words.iter().len();
-    match count.cmp(&1) {
-        Ordering::Less => {
-            return Err(MnemonicError::UnknownLanguage);
-        },
-        Ordering::Equal => {
-            let word = words.get(0).ok_or(MnemonicError::EncodeInvalidLength)?;
-            for language in MnemonicLanguage::iterator() {
-                if find_mnemonic_index_from_word(word, language).is_ok() {
-                    return Ok(*language);
-                }
-            }
-            return Err(MnemonicError::UnknownLanguage);
-        },
-        Ordering::Greater => {
-            for word in words {
-                let mut languages = Vec::with_capacity(MnemonicLanguage::iterator().len());
-                // detect all languages in which a word falls into
-                for language in MnemonicLanguage::iterator() {
-                    if find_mnemonic_index_from_word(word, language).is_ok() {
-                        languages.push(*language);
-                    }
-                }
-                // check if at least one of the languages is consistent for all other words against languages yielded
-                // from the initial word for this iteration
-                for language in languages {
-                    let mut consistent = true;
-                    for compare in words {
-                        if compare != word && find_mnemonic_index_from_word(compare, &language).is_err() {
-                            consistent = false;
-                        }
-                    }
-                    if consistent {
-                        return Ok(language);
-                    }
-                }
-            }
-        },
-    }
-
-    Err(MnemonicError::UnknownLanguage)
-}
-
 /// Generates a vector of bytes that represent the provided mnemonic sequence of words, the language of the mnemonic
 /// sequence is detected
 pub fn to_bytes(mnemonic_seq: &[String]) -> Result<Vec<u8>, MnemonicError> {
-    let language = self::detect_language(mnemonic_seq)?;
+    let language = MnemonicLanguage::detect_language(mnemonic_seq)?;
     to_bytes_with_language(mnemonic_seq, &language)
 }
 
@@ -336,7 +349,10 @@ mod test {
             "opera".to_string(),
             "abandon".to_string(),
         ];
-        assert_eq!(detect_language(&words1), Ok(MnemonicLanguage::English));
+        assert_eq!(
+            MnemonicLanguage::detect_language(&words1),
+            Ok(MnemonicLanguage::English)
+        );
 
         // English/Spanish + English/French + Italian/Spanish
         let words2 = vec![
@@ -346,7 +362,7 @@ mod test {
             "abandon".to_string(),
             "tipico".to_string(),
         ];
-        assert_eq!(detect_language(&words2).is_err(), true);
+        assert_eq!(MnemonicLanguage::detect_language(&words2).is_err(), true);
 
         // bounds check (last word is invalid)
         let words3 = vec![
@@ -356,16 +372,16 @@ mod test {
             "abandon".to_string(),
             "topazio".to_string(),
         ];
-        assert_eq!(detect_language(&words3).is_err(), true);
+        assert_eq!(MnemonicLanguage::detect_language(&words3).is_err(), true);
 
         // building up a word list: English/French + French -> French
         let mut words = Vec::with_capacity(3);
         words.push("concert".to_string());
-        assert_eq!(detect_language(&words), Ok(MnemonicLanguage::English));
+        assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::English));
         words.push("abandon".to_string());
-        assert_eq!(detect_language(&words), Ok(MnemonicLanguage::English));
+        assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::English));
         words.push("barbier".to_string());
-        assert_eq!(detect_language(&words), Ok(MnemonicLanguage::French));
+        assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::French));
     }
 
     #[test]

base_layer/wallet_ffi/src/enums.rs

@@ -27,4 +27,5 @@ pub enum SeedWordPushResult {
     SeedPhraseComplete,
     InvalidSeedPhrase,
     InvalidObject,
+    NoLanguageMatch,
 }

base_layer/wallet_ffi/src/lib.rs

@@ -1020,11 +1020,13 @@ pub unsafe extern "C" fn seed_words_get_at(
 ///
 /// ## Returns
 /// 'c_uchar' - Returns a u8 version of the `SeedWordPushResult` enum indicating whether the word was not a valid seed
-/// word, if the push was successful and whether the push was successful and completed the full Seed Phrase
+/// word, if the push was successful and whether the push was successful and completed the full Seed Phrase.
+///  `seed_words` is only modified in the event of a `SuccessfulPush`.
 ///     '0' -> InvalidSeedWord
 ///     '1' -> SuccessfulPush
 ///     '2' -> SeedPhraseComplete
 ///     '3' -> InvalidSeedPhrase
+///     '4' -> NoLanguageMatch,
 /// # Safety
 /// The ```string_destroy``` method must be called when finished with a string from rust to prevent a memory leak
 #[no_mangle]
@@ -1082,28 +1084,64 @@ pub unsafe extern "C" fn seed_words_push_word(
         },
     }
 
-    if MnemonicLanguage::from(word_string.as_str()).is_err() {
-        log::error!(target: LOG_TARGET, "{} is not a valid mnemonic seed word", word_string);
-        return SeedWordPushResult::InvalidSeedWord as u8;
+    // Seed words is currently empty, this is the first word
+    if (*seed_words).0.is_empty() {
+        (*seed_words).0.push(word_string);
+        return SeedWordPushResult::SuccessfulPush as u8;
     }
 
-    (*seed_words).0.push(word_string);
-    if (*seed_words).0.len() >= 24 {
-        return if let Err(e) = CipherSeed::from_mnemonic(&(*seed_words).0, None) {
+    // Try push to a temporary copy first to prevent existing object becoming invalid
+    let mut temp = (*seed_words).0.clone();
+
+    if let Ok(language) = MnemonicLanguage::detect_language(&temp) {
+        temp.push(word_string.clone());
+        // Check words in temp are still consistent for a language, note that detected language can change
+        // depending on word added
+        if MnemonicLanguage::detect_language(&temp).is_ok() {
+            if temp.len() >= 24 {
+                if let Err(e) = CipherSeed::from_mnemonic(&temp, None) {
+                    log::error!(
+                        target: LOG_TARGET,
+                        "Problem building valid private seed from seed phrase: {:?}",
+                        e
+                    );
+                    error = LibWalletError::from(WalletError::KeyManagerError(e)).code;
+                    ptr::swap(error_out, &mut error as *mut c_int);
+                    return SeedWordPushResult::InvalidSeedPhrase as u8;
+                };
+            }
+
+            (*seed_words).0.push(word_string);
+
+            // Note: test for a validity was already done so we can just check length here
+            if (*seed_words).0.len() < 24 {
+                SeedWordPushResult::SuccessfulPush as u8
+            } else {
+                SeedWordPushResult::SeedPhraseComplete as u8
+            }
+        } else {
             log::error!(
                 target: LOG_TARGET,
-                "Problem building valid private seed from seed phrase: {:?}",
-                e
+                "Words in seed phrase do not match any language after trying to add word: `{:?}`, previously words \
+                 were detected to be in: `{:?}`",
+                word_string,
+                language
             );
-            error = LibWalletError::from(WalletError::KeyManagerError(e)).code;
-            ptr::swap(error_out, &mut error as *mut c_int);
-            SeedWordPushResult::InvalidSeedPhrase as u8
-        } else {
-            SeedWordPushResult::SeedPhraseComplete as u8
-        };
+            SeedWordPushResult::NoLanguageMatch as u8
+        }
+    } else {
+        // Seed words are invalid, shouldn't normally be reachable
+        log::error!(
+            target: LOG_TARGET,
+            "Words in seed phrase do not match any language prior to adding word: `{:?}`",
+            word_string
+        );
+        let error_msg = "Invalid seed words object, no language can be detected.";
+        log::error!(target: LOG_TARGET, "{}", error_msg);
+        error = LibWalletError::from(InterfaceError::InvalidArgument(error_msg.to_string())).code;
+        ptr::swap(error_out, &mut error as *mut c_int);
+        SeedWordPushResult::InvalidObject as u8
     }
-
-    SeedWordPushResult::SuccessfulPush as u8
 }
 
 /// Frees memory for a TariSeedWords