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

[DDW-1156] Implement Hardware wallets restoration #1801

Conversation

thedanheller
Copy link
Contributor

@thedanheller thedanheller commented Dec 23, 2019

This PR updates Restore Wallet dialog with Hardware wallets restoration.

Todos

  • Steps logic
  • Steps content
    • Step wallet type
    • Step mnemonics
    • Step configuration
    • Step success
  • Integrate API endpoint
  • Bump backend to f003b3139ff883fd5b6bb0566c4aa9786c838eca with hw wallet support and recheck/fix integration
  • Add hardware wallet seeds to genesis block and get recovery phrases for testing
  • 'What kind of wallet would you like to restore?' message should change based on the wallet kind selected, as per copy:
    https://docs.google.com/document/d/17cWHcPvKLc0Dx2YDjaBHujX0UScEOLEKVhmrNgTz38c/edit
  • e2e tests - update existing tests to pass and add new tests based on the testing checklist below
  • check IDs of restored wallets in e2e tests
  • update copy for hw wallet types, we still don't have 12, 18 or 24 everywhere, as per copy
  • spinner on the button while submitting restoration request
  • HW wallet restoration needs to support 12, 18 and 24-word mnemonics. The document with the copy is updated.
  • Refactor api to remove code duplication for restoreByronRandomWallet, restoreByronIcarsWallet, restoreByronTrezorWallet, restoreByronLedgerWallet, under the hood these should call internal function for legacy restoration
  • Rename Nano to Ledger, since Ledger Nano X and Nano S are supported
  • Remove isLedger and unneeded query parameters
  • e2e test break when item that is being clicked is not visible because UI needs to be scrolled
  • discuss CSS styles needed for selectors in e2e test and fix this:
    image
  • Error is not cleared after test case 3, if you go back and re-enter valid recovery phrase from the test case 4, error is still visible on the last step. Write a test to cover this case!
  • Update screenshots, we need both English and Japanese. Without drop shadows on macOS which are the default when taking screenshots.
  • Linux build failing

Screenshots

EN-00001
EN-00002
EN-00003
EN-00004
EN-00005
EN-00006
JP-00001
JP-00002
JP-00003
JP-00004
JP-00005
JP-00006


Testing Checklist

We can verify that we have implemented this functionality properly by checking which wallet IDs we get after restoration. This is already covered by e2e tests and IDs there should not be changed.

  • E2e test is needed for test case 3.

  • Slack QA thread

  • 1. Restore Daedalus Balance wallet - 0.000010 ada
    ["prison", "census", "discover", "give", "sound", "behave", "hundred", "cave", "someone", "orchard", "just", "wild"],
    and IDlegacy_6eb9a6862e5656b4a52fa6fae8eb3a3e8f7c2bd6

  • 2. Restore Daedalus Rewards wallet - 0 ada
    combine mouse cool skirt truck outer result speed fringe sugar there usage lucky wild tail,
    and ID c2ebd8b727cc760fe2f0fb3d06a8630ccc8c70f5

  • 3. Invalid paper wallet mnemonic
    worry pluck anchor recycle predict grow inner inside face face subway meat away once february family rug make hub violin riot around coast play pluck grow face

  • 4. Valid paper wallet mnemonic - 0 ada
    season nice police near blame dress deal congress unusual more giggle pull general list crash gravity fashion notable voice resemble auto smart flat party thought unique amused
    legacy_699c20fef5469d2cabadf5a778932d06ca3364e2

  • 5. Yoroi Balance wallet - 1,000,000.000000 ada
    "defense", "brush", "fiscal", "cactus", "rotate", "trouble", "mean", "quantum", "shrug", "slight", "dignity", "corn", "immense", "first", "citizen"
    legacy_aab5517861cca76a53d83e24c84542ecac6c0a3d

  • 6. Yoroi Rewards wallet - 0 ada
    "defense", "brush", "fiscal", "cactus", "rotate", "trouble", "mean", "quantum", "shrug", "slight", "dignity", "corn", "immense", "first", "citizen"
    aab5517861cca76a53d83e24c84542ecac6c0a3d

  • 7. Ledger Balance - 12 words
    "struggle", "section", "scissors", "siren", "garbage", "yellow", "maximum", "finger", "duty", "require", "mule", "earn"
    legacy_64c76f5644be19e5ba4cbe717967e2fd057079b3

  • 8. Leger wallet - 18 words:
    "vague" , "wrist" , "poet" , "crazy" , "danger" , "dinner", "grace" , "home" , "naive" , "unfold" , "april" , "exile", "relief" , "rifle" , "ranch" , "tone" , "betray" , "wrong"

  • 9. Ledger walelt - 24 words:
    "recall" , "grace" , "sport" , "punch" , "exhibit" , "mad", "harbor" , "stand" , "obey" , "short" , "width" , "stem", "awkward" , "used" , "stairs" , "wool" , "ugly" , "trap", "season" , "stove" , "worth" , "toward" , "congress" , "jaguar"

  • 10. Trezor wallet - 12 words: walk", "license", "firm", "dwarf", "hundred", "pride", "ensure", "midnight", "unit", "keen", "warfare", "east"

  • 11. Trezor wallet - 18 words:
    "hen", "idea", "mimic", "frog", "second", "magnet", "egg", "indicate", "jar", "girl", "broccoli", "heart", "verify", "person", "present", "toe", "vibrant", "unable"

  • 12. Trezor wallet - 24. words:
    "slot", "young", "shoot", "surround", "equal", "trouble", "rice", "update", "rare", "dinosaur", "drastic", "kitten", "mom", "actress", "salon", "abuse", "happy", "satisfy"


Review Checklist

Basics

  • PR has been assigned and has appropriate labels (feature/bug/chore, release-x.x.x)
  • PR is updated to the most recent version of the target branch (and there are no conflicts)
  • PR has a good description that summarizes all changes and shows some screenshots or animated GIFs of important UI changes
  • CHANGELOG entry has been added to the top of the appropriate section (Features, Fixes, Chores) and is linked to the correct PR on GitHub
  • Automated tests: All acceptance and unit tests are passing (yarn test)
  • Manual tests (minimum tests should cover newly added feature/fix): App works correctly in development build (yarn dev)
  • Manual tests (minimum tests should cover newly added feature/fix): App works correctly in production build (yarn package / CI builds)
  • There are no flow errors or warnings (yarn flow:test)
  • There are no lint errors or warnings (yarn lint)
  • There are no prettier errors or warnings (yarn prettier:check)
  • There are no missing translations (running yarn manage:translations produces no changes)
  • Text changes are proofread and approved (Jane Wild / Amy Reeve)
  • Japanese text changes are proofread and approved (Junko Oda)
  • UI changes look good in all themes (Alexander Rukin)
  • Storybook works and no stories are broken (yarn storybook)
  • In case of dependency changes yarn.lock file is updated

Code Quality

  • Important parts of the code are properly commented and documented
  • Code is properly typed with flow
  • React components are split-up enough to avoid unnecessary re-renderings
  • Any code that only works in main process is neatly separated from components

Testing

  • New feature/change is covered by acceptance tests
  • New feature/change is manually tested and approved by QA team
  • All existing acceptance tests are still up-to-date
  • New feature/change is covered by Daedalus Testing scenario
  • All existing Daedalus Testing scenarios are still up-to-date

After Review

  • Merge the PR
  • Delete the source branch
  • Move the ticket to done column on the YouTrack board
  • Update Slack QA thread by marking it with a green checkmark

@thedanheller thedanheller self-assigned this Dec 23, 2019
DeeJayElly and others added 13 commits January 2, 2020 23:03
…are-wallets-restoration' into feature/ddw-1156-implement-hardware-wallets-restoration
… Ledger, since Ledger Nano X and Nano S are supported & Removed isLedger and unneeded query parameters
…are-wallets-restoration' into feature/ddw-1156-implement-hardware-wallets-restoration
@darko-mijic darko-mijic merged commit 2fbf777 into v2-integration Jan 9, 2020
@iohk-bors iohk-bors bot deleted the feature/ddw-1156-implement-hardware-wallets-restoration branch January 9, 2020 06:14
@nikolaglumac nikolaglumac removed the WIP label Jan 10, 2020
@nikolaglumac nikolaglumac added release-2.1.0-ITN1 Daedalus Incentivized Testnet - Rewards and removed ⏳release-vNext-ITN1 Daedalus Incentivized Testnet - Rewards labels Jan 22, 2020
@nikolaglumac nikolaglumac mentioned this pull request Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature release-2.1.0-ITN1 Daedalus Incentivized Testnet - Rewards
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants