-
Notifications
You must be signed in to change notification settings - Fork 28
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
All mnemonics #89
base: master
Are you sure you want to change the base?
All mnemonics #89
Conversation
Added 7 preset IPFS gateways. 2x Cloudflare, 2x ipfs.io, dweb, GreyH.at, and Infura Added 'ipfsGateway' setting to BRSharedPrefs. Added helper function in Utils class for creating an ipfs url from the hash.
Added BRConstants.IPFS_DEFAULT_HOST with default value "ipfs.io" Moved ipfs.io gateways to top of settings list since they are the default option.
- Added class for managing BIP39 wordlists, with helper functions for various use-cases. - Removed now-unused class Bip39Reader - Replaced all usages of Bip39Reader with Bip39Wordlist
Added tests around the new wordlist class, as well as expanding to include all language now.
…ersing the seed value from a given phrase.
…to writing down of your paper key. Generate displayed paper key in selected language, in the background the phrase is always stored as English. Renamed `BRCoreKey.getSeedFromPhrase` to `BRCoreKey.getDerivedPhraseKey` to match underlying JNI names.
My main issue with this currently, and where I could really use a second opinion, is that if I take a phrase, say: If I "translate" the phrase to spanish, I get This would (presumably) mean that a "translated" phrase would not actually restore back to the same wallet/private key, only one that is "translated" back into english before being used. |
And as I was typing that last comment I more-or-less came across the source of my confusion. I think the best fix here would just be prompting for the key phrase language sooner, prior to the key actually being generated, and then just storing the generated phrase in it's "native" language. All the remaining points in the code should simply work with a non-english phrase and key (binary anyway) stored in the settings. |
Actually, before I go about making any more changes, I would like to seek a second opinion on the best place to add the new prompt. At the moment, the flow of a new wallet looks like this. Activities are listed with the step performed to advance to the next activity.
A few possible ideas come to mind on how we can insert the prompt into the current flow.
Thoughts on any of these options? |
My opinion... Detect the language from the operating system. If non-US,
then default to using Spanish, Chinese, etc. Default to English if the
mnemonic table doesn't match the language used by the OS.
…On Fri, Apr 9, 2021 at 10:30 PM Ben Abraham ***@***.***> wrote:
Actually, before I go about making any more changes, I would like to seek
a second opinion on the best place to add the new prompt.
At the moment, the flow of a new wallet looks like this. Activities are
listed with the step performed to advance to the next activity.
1. Intro (Create)
2. About (Skip)
3. T&C (Accept)
4. Pin (Type)
5. PinConfirm (Type)
6. *Wallet seed created here*
7. WriteDown (Advance) <--- Where language prompt currently is
8. PaperKey (Next x12)
9. PaperKeyProve (Write x2)
10. Home (Hodl)
A few possible ideas come to mind on how we can insert the prompt into the
current flow.
- Between steps 5/6 (or 3/4), this is the most direct, but feels a
little awkward of a flow for users.
- On activity 3 itself, also a weird place for that prompt
- Move wallet creation after step 7, would solve ordering, but
introduce many potential edge-cases with existing (but not marked as backed
up yet) wallets
Thoughts on any of these options?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#89 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACGYTYRCFGGHEA5UVJFEYX3TH7H7BANCNFSM42UGFELA>
.
|
Phrase language selection is entirely based on system language at the time of phrase creation. After the phrase has been created, it is always interpreted as text going forward, so all existing references should continue to work with no issues. Added helper function for validating whole phrase. Added helper function for getting current locale in a version-aware manner.
Alright! I pulled out the language selection components and just let the locale detection choose which wordlist when creating a new wallet. I also added a version-aware helper function to support the changes to locale classes that didn't seem to be accounted for currently. So creating a new wallet and then switching to spanish looks like this (app restarted after language switch): The backup phrase remains in english despite the system and application being displayed in the selected locale. Any "translate" code no longer exists. |
…ordering of selected locales. Also fixed chinese languages not being selected properly.
Detecting the language from the OS is the best option for writing down the 12-words. |
Done
When restoring, the phrase the user enters is stored in |
PR to close #83 [Please add other mnemonic languages]
Status: Ready for Review
New test cases related to "translation" of phrases NOTE belowI've added a new class
Bip39Wordlist
for grouping all the functionality of a specific wordlist in one place. Most of the wordlist-related JNI functions are called from this class, making it much simpler to write wordlist-aware code.At the moment, all phrases are stored in English in theBRKeyStore
class, this change does not modify that underlying storage at all. This PR makes the assumption that we can leave english phrases as-is in the user preferences, and simply "translate" (not actually) the phrase to that of another language. If this assumption is faulty, please let me know :)The steps for "translating" the phrase are as follows:Extract the users always-english phrase fromBRKeyStore
.Derive the seed from their phrase based onBip39Wordlist.DEFAULT_WORDLIST
.Fetch theBip39Wordlist
for the target language.Use the seed from step 2. to create the phrase in the target language.Present generated phrase instead of english phrase.Because this is simply a "translation" for the purposes of the paper key, and no underlying storage is being changed, the only UI place this was implemented was inPaperKeyActivity
Note on tests:
PaperKeyTests
has been updated to test validating a phrase in each wordlist available. However I am having issues getting (some?) JNI functions working properly when ran in unit testing mode. Calling any JNI method throwsUnsatisfiedLinkError
during testing, which appears to be related to what android makes available during non-emulator testing.Looking for suggestions on the best way to handle this.
Screenshot: