Skip to content
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

Add private key onboarding flow #3119

Merged
merged 14 commits into from
Apr 4, 2023
Merged

Conversation

jagodarybacka
Copy link
Contributor

@jagodarybacka jagodarybacka commented Mar 6, 2023

Resolves #3069

What

This PR adds private key flow to tabbed onboarding and allows wallet to show these wallets in the UI correctly

  • make seed phrase safe input a separate, shared component
  • refactor seed phrase form to make it reusable
  • add new page for private key import
  • show private key imported wallets in a new section on the accounts list
Screen.Recording.2023-03-08.at.12.03.47.mov

image

Testing

  1. Go through the private key onboarding flow
  2. Import account with private key
  3. Check how it behaves in the wallet
  4. Import other accounts, check wallets list - private key wallets should be in Generated and imported section

Latest build: extension-builds-3119 (as of Tue, 04 Apr 2023 09:23:44 GMT).

Make separate import seed form component to allow private key import
page to reuse the same page layout.
Add page, new icons and link to the private key page on tabbed
onboarding flow.
@jagodarybacka jagodarybacka added this to the Private Key Import milestone Mar 6, 2023
@jagodarybacka jagodarybacka self-assigned this Mar 6, 2023
@jagodarybacka jagodarybacka marked this pull request as ready for review March 8, 2023 14:26
@jagodarybacka
Copy link
Contributor Author

Outstanding issue - private key input validation
For mnemonic, we are only checking the length. What rules do we want to include here? Check if it is a valid hex string? Check length?

@jagodarybacka jagodarybacka linked an issue Mar 8, 2023 that may be closed by this pull request
@VladUXUI
Copy link
Contributor

VladUXUI commented Mar 9, 2023

Check if it is a valid hex string? Check length?

Can we do both?

@jagodarybacka jagodarybacka requested a review from a team as a code owner March 14, 2023 17:02
Copy link
Contributor

@0xDaedalus 0xDaedalus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - feel free to merge and address comments in a separate PR - or add a commit here and then merge.

background/redux-slices/selectors/signingSelectors.ts Outdated Show resolved Hide resolved
ui/_locales/en/messages.json Outdated Show resolved Hide resolved
@@ -51,6 +51,14 @@ type WalletTypeInfo = {
category: string
}

const accountTypes = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can do Object.values(AccountType) here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typescript is not happy to iterate over enum AccountType like that 😞

AccountType.ReadOnly,
AccountType.Ledger,
]
const existingAccountTypes = accountTypes.filter(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

Co-authored-by: Daedalus <94649004+0xDaedalus@users.noreply.github.com>
Co-authored-by: Daedalus <94649004+0xDaedalus@users.noreply.github.com>
@jagodarybacka jagodarybacka merged commit 278c4a6 into keyring-with-pk Apr 4, 2023
@jagodarybacka jagodarybacka deleted the pk-import-on-onboarding branch April 4, 2023 09:24
Shadowfiend added a commit that referenced this pull request Jul 7, 2023
### About this PR
This PR will contain all changes for features:
- [x] allow importing private keys in the keyring service (initial scope
of this PR)
- [x] import private keys with plain text
(#3119)
- [x] import private keys with JSON
(#3177)
- [x] backup account with mnemonic and private key export
(#3252)
---

## Initial scope 
Resolves #3070

### What

Handle importing wallets with private keys in the keyring service:
* save pk imported accounts in the encrypted vault
* handle serialization and deserialization of pk imported wallets
* handle signing messages with both keyrings and pk wallets
* handle communication between service and redux store about pk wallets
* fix types and field names in the redux store

Cleanup and add more tests:
- move existing keyring integration tests to the right folder
- write more unit tests to cover private key imported wallets
- cleanup some duplicated tests
- add util functions to the test's `factories`
- add function to mock local storage

### Testing
- make sure unit tests and E2E are passing
- test manually adding accounts and signing transactions

Latest build:
[extension-builds-3089](https://github.com/tahowallet/extension/suites/14105826655/artifacts/789186059)
(as of Thu, 06 Jul 2023 12:56:07 GMT).
@kkosiorowska kkosiorowska mentioned this pull request Jul 13, 2023
kkosiorowska pushed a commit that referenced this pull request Jul 14, 2023
## What's Changed
* Add private key onboarding flow by @jagodarybacka in
#3119
* Private key JSON import by @jagodarybacka in
#3177
* Allow export of private keys and mnemonics by @jagodarybacka in
#3248
* Export private key form by @jagodarybacka in
#3255
* Unlock screen for the account backup by @kkosiorowska in
#3257
* Show mnemonic menu by @jagodarybacka in
#3259
* Fix background blur issue by @jagodarybacka in
#3265
* Account backup UI fixes by @jagodarybacka in
#3270
* Fix unhiding removed accounts by @jagodarybacka in
#3282
* New error for incorrectly decrypted JSON file by @jagodarybacka in
#3293
* Export private keys from HD wallet addresses by @jagodarybacka in
#3253
* Refactor keyring redux slice to remove `importing` field by
@jagodarybacka in #3309
* 📚 Accounts backup by @kkosiorowska in
#3252
* Catch Enter keypress on Unlock screen by @jagodarybacka in
#3355
* Rename `keyring` to `internal signer` and other improvements by
@jagodarybacka in #3331
* 🗝 QA - Accounts backup and private key import by @jagodarybacka in
#3266
* Remove private key signers if they are replaced by accounts from HD
wallet by @jagodarybacka in
#3377
* RFB 4: One-Off Keyring Design by @Shadowfiend in
#3372
* Copy to clipboard warning by @kkosiorowska in
#3488
* Allow setting custom auto-lock timer by @hyphenized in
#3477
* Use Argon2 for encrypted vaults by @jagodarybacka in
#3502
* 👑 Private keys import and accounts backup by @jagodarybacka in
#3089
* Untrusted assets should not block the addition of custom tokens by
@kkosiorowska in #3491
* Flip updated dApp connections flag by @Shadowfiend in
#3492
* v0.41.0 by @Shadowfiend in
#3531
* Switch to a given network if adding a network that is already added.
by @0xDaedalus in #3154
* Remove waiting for Loading Doggo component in E2E tests by
@jagodarybacka in #3541
* Squeeze content to better fit on Swaps page by @jagodarybacka in
#3542
* Refactor of terms for verified/unverified assets by @kkosiorowska in
#3528
* Fix ChainList styling by @fulldecent in
#3547
* Update release checklist by @jagodarybacka in
#3548
* Fix custom asset price fetching by @hyphenized in
#3508
* Sticky Defaults: Make Taho-as-default replace MetaMask in almost all
cases by @Shadowfiend in
#3546

## New Contributors
* @fulldecent made their first contribution in
#3547

**Full Changelog**:
v0.41.0...v0.42.0

Latest build:
[extension-builds-3549](https://github.com/tahowallet/extension/suites/14268975651/artifacts/801826435)
(as of Thu, 13 Jul 2023 09:51:56 GMT).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update import page validation to validate and accept private keys
3 participants