fix: seed word parsing #3607
Merged
fix: seed word parsing #3607
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philipr-za
reviewed
Nov 23, 2021
base_layer/wallet_ffi/src/lib.rs
Outdated
| } else { | ||
| log::error!( | ||
| target: LOG_TARGET, | ||
| "Word in seed phrase is inconsistent: {:?}", |
There was a problem hiding this comment.
Do you mean that there is no language matching the words so far? If so I think this error is not very clear. If you saw it in the logs in isolation would you be able to determine what the problem is? Maybe something like "Current words in seed phrase do not match a supported language"
There was a problem hiding this comment.
What is meant here is that by adding the word, the language cannot be determined anymore since there are no words that are either in one language or words common to at least one of them. The error will be UnknownLanguage.
Perhaps a pre-detection of the language prior to this and adding "...previously words were detected to be in language" will help?
StriderDM
force-pushed
the
seed_word_parsing
branch
from
f4417e6
to
0e80f16
Compare
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
StriderDM
force-pushed
the
seed_word_parsing
branch
from
0e80f16
to
1629a6e
Compare
sdbondi
added a commit
to sdbondi/tari
that referenced
this pull request
Nov 23, 2021
* development: fix: seed word parsing (tari-project#3607) feat: add ban peers metric (tari-project#3605) feat!: expose reason for transaction cancellation for callback in wallet_ffi (tari-project#3601) test: improve cucumber scenario robustness (tari-project#3599) feat: use CipherSeed wallet birthday for recovery start point (tari-project#3602) docs: ignore RFC code blocks (tari-project#3603)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Moved
detect_languageintoMnemonicLanguageand made it public.Prevented a
TariSeedWordsobject 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 the string of WordNotFound error.
Motivation and Context
General fixes
How Has This Been Tested?
cargo test --all