1082 - onboarding testing feedback #2380
Conversation
| * we enable EOA derivation. | ||
| */ | ||
| private fun createWalletFromPrefix(prefix: String, isCurrentAccount: Boolean): Wallet? { | ||
| private fun createWalletFromPrefix(prefix: String, userId: String?, isCurrentAccount: Boolean): Wallet? { |
There was a problem hiding this comment.
Why is a prefix required to store the key for a profile created with a seed phrase?
There was a problem hiding this comment.
@jaymengxy currently for RN seed phrase accounts, both the prefix (private key) and mnemonic are stored. The prefix storage was the original mechanism, and the mnemonic is additionally stored to enable EOA derivation (which requires the full seed phrase, not just the derived key).
In the current flow, when we detect the mnemonic exists, we create the wallet from the mnemonic (ignoring the prefix) because it gives us full HD wallet capabilities including EOA. The prefix is only used as a fallback for accounts without mnemonic storage.
We can re-architect it to remove the private key altogether but it would involve additional changes besides just fixing the account switching bug. I will go ahead and commit it to this branch.
There was a problem hiding this comment.
Our goal is not to completely remove the private key—that is a separate feature.
What we need to do right now is to clarify all profile types.
We need to revert the change made in the SDK-integration commit—where profiles were created using a PrivateKey—back to the original logic of using Android Keystore.
We can now distinguish between PrivateKey profiles and Android Keystore profiles based on the new prefix you added in that commit. We should avoid adding any additional prefixes; otherwise, it will complicate the differentiation process.
There was a problem hiding this comment.
Hi @jaymengxy , I just pushed a commit which refactors logic only affecting the new RN seed phrase accounts (not existing accounts, doesn't migrate or change any logic on the existing prefix-based accounts or legacy keystore accounts).
| // Using hasHDWalletKeystore which safely checks file existence without creating new keystores | ||
| if (!userId.isNullOrBlank() && AccountWalletManager.hasHDWalletKeystore(userId)) { | ||
| try { | ||
| val mnemonic = AccountWalletManager.getHDWalletMnemonicByUID(userId) |
There was a problem hiding this comment.
I don't see any prefix being used here either.
| account.prefix.isNullOrBlank() && !userId.isNullOrBlank() && AccountWalletManager.hasHDWalletKeystore(userId) -> { | ||
| logd(TAG, "Creating mnemonic-only wallet for account: ${account.userInfo.username} (cleaner architecture)") | ||
| createWalletFromHDMnemonic(userId, isCurrentAccount) | ||
| } |
There was a problem hiding this comment.
I think the code will execute createWalletFromHDMnemonic(userId, isCurrentAccount) regardless of whether !userId.isNullOrBlank() && AccountWalletManager.hasHDWalletKeystore(userId) holds true. Maybe we can just use else section
Related Issue
Closes #???
Addresses:
(1) During the account creation process, do we need to disable left swipe navigation on the page? If I swap back, it change my first profile(tester1) in profile list to the new random name. But not create a new account.https://dashboard.luciq.ai/bugs/pAiJhYSZNok1UxwcenhyA5Hl8DubzAQpavcE_pUBBME
(2) When creating a seed phrase account, the EOA address shows up quite slowly.
(3) After creating a seed phrase account, if I switch to another account, I cannot switch back.
Summary of Changes
Need Regression Testing
Risk Assessment
Additional Notes
Screenshots (if applicable)