-
Notifications
You must be signed in to change notification settings - Fork 369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mnemonic validation flexibility within Valora #6372
Merged
Merged
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
cc7a638
changing mnemonic logic to allow for any language recognition and to …
tarikbellamine 0fc03a0
simplify validation logic
tarikbellamine 737361a
normalizing mnemonics and word lists to ignore accents
tarikbellamine 1303448
add test
tarikbellamine ad6c727
remove unneeded test
tarikbellamine 4ea608e
pr feedback
tarikbellamine 892ef34
fixing incorrectly accented words rather than stripping accents
tarikbellamine 2ea4e91
add new tests
tarikbellamine ef4ef48
optimized functions by using a map instead of array
tarikbellamine 126ac5a
remove console logs
tarikbellamine 0743338
Lowercase mnemonic to noramlize irregular capitalizations
tarikbellamine 41f7d63
Merge branch 'master' into tarikbellamine/mnemonic-accents
mergify[bot] af8aa82
Merge branch 'master' into tarikbellamine/mnemonic-accents
mergify[bot] 027023c
Merge branch 'master' into tarikbellamine/mnemonic-accents
mergify[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
import { appendPath } from '@celo/base/lib/string' | ||
import { appendPath, normalizeAccents } from '@celo/base/lib/string' | ||
// Exports moved to @celo/base, forwarding them | ||
// here for backwards compatibility | ||
export { appendPath } from '@celo/base/lib/string' | ||
export { appendPath, normalizeAccents } from '@celo/base/lib/string' | ||
export const StringUtils = { | ||
appendPath, | ||
normalizeAccents, | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entire sure of removing these. In a way, if you know the language, you will avoid to search in every dictionary.
I'm more into the idea of searching in the dictionary that the user said, or in all of them otherwise (nil/null).
Thinking more of a common utils side, and not our use case in Valora
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about my approach this way:
Good - eliminate the possibility of user error (i.e., if there is a mismatch between
defaultLanguage
andmnemonic
language)Bad - slightly less efficient as it defaults to checking all languages. However, there are not very many languages and
bip39.validateMnemonic
appears very optimized and will fail quickly on a language mismatchWhat do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I agree. We could eventually have another function that requires an specific language if we see that we need to optimise that (which are not the actual cases due to we are only using these to create/update accounts, and won't be called too often)