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.

Sign up for GitHub

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

Jump to bottom

Implement Ledger Live bridge #10293

Merged
merged 33 commits into from
Apr 26, 2021
Merged

Implement Ledger Live bridge #10293

merged 33 commits into from
Apr 26, 2021

Conversation

darkwing
Copy link
Contributor

@darkwing darkwing commented Jan 26, 2021 •
edited
Loading

Supercedes #10171

This depends upon:

Todo:

  • Add a setting to opt into this new functionality; if not, fall back to U2F
  • Implement fallback option vs. new functionality
  • Ensure design and verbiage integrity

@darkwing darkwing requested a review from Gudahtt January 26, 2021 22:02
@darkwing darkwing requested a review from a team as a code owner January 26, 2021 22:02
@darkwing darkwing mentioned this pull request Jan 26, 2021
Closed
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@darkwing darkwing force-pushed the ledger-live branch 4 times, most recently from 5b1d985 to aaf6cc7 Compare February 11, 2021 19:15
darkwing
darkwing commented Feb 11, 2021
View reviewed changes
package.json Outdated Show resolved Hide resolved
@darkwing darkwing force-pushed the ledger-live branch 3 times, most recently from bd51df0 to 55d68bb Compare February 22, 2021 22:00
@darkwing darkwing force-pushed the ledger-live branch 3 times, most recently from 050bfd3 to 1d61202 Compare March 2, 2021 18:23
brad-decker
brad-decker suggested changes Mar 3, 2021
View reviewed changes
ui/app/store/actions.js Outdated Show resolved Hide resolved
@brad-decker
Copy link
Contributor

I just spent some time using this and it works like a dream on my Mac in Chrome. 😍

@brad-decker
Copy link
Contributor

we should probably throw the "DO NOT MERGE" label on this until the reference to your fork for eth-ledger-bridge-keyring is updated to the new version of that library.

@darkwing darkwing added the DO-NOT-MERGE Pull requests that should not be merged label Mar 3, 2021
@darkwing darkwing requested a review from rachelcope March 15, 2021 22:28
@darkwing
Copy link
Contributor Author

Hello @rachelcope . Screenshots as promised. I did have to slightly change the text because we need to accommodate this feature being (1) turned on by the user and (2) turned off, thus the old way.

"Connect Hardware" with nothing selected

LedgerLiveNoneSelected

Trezor selected

TrezorSelected

Ledger selected, new Ledger Live turned on

LedgerSelectedWithLedgerLiveSettingOn

Ledger with Ledger Live off

LedgerPreferenceOff

darkwing added 23 commits April 23, 2021 17:06
@darkwing
Add localization
23710d2
@darkwing
Send useLedgerLive setting to the user
718a52e
@darkwing
Fix metamask controller tests
e0f72d2
@darkwing
Don't allow non-WebUSB users to turn on Ledger Live, provide more spe…
60fafe5 
…cific text
@darkwing
Remove USB check
84ff751
@darkwing
Remove irrelevant test
931b86c
@darkwing
Use async setLedgerLivePreference in controller
d7ec62b
@darkwing
Fix l10n
de08464
@darkwing
Don't use redux pattern for setLedgerLivePreference
4b4ec1d
@darkwing
Remove rogue comma from l10n as result of rebase
ea39201
@darkwing
Await keyring retrieval
08708ad
@darkwing
Communicate the ledger live preference in the MetaMask Controller sub…
e631845 
…mitPassword
@darkwing
Protect the updateTransportMethod call
5de54f6
@darkwing
Restore eth-ledger-bridge-keyring package
96e1426
@darkwing
Add fallback for unsuccessful transporot update, localize text
09741f3
@darkwing
Update to ledger keyring to 0.4.0
629ab6e
@darkwing
Address minor feedback
e473bcf
@darkwing
Show loading indicator for ledger live preference toggle
daaf61e
@darkwing
Use proper CSS naming for message link
e1bf3a6
@darkwing
Remove unused CSS class
972f2ca
@darkwing
Remove unnecessary pointer cursor
e8c9000
@darkwing
Re-throw error
b907808
@darkwing
Update text in e2e test
7b5a27a
@darkwing
Copy link
Contributor Author

This last commit is a reordering of commits to not conflict with @NiranjanaBinoy's from-import-ui.spec.js

@darkwing darkwing removed the DO-NOT-MERGE Pull requests that should not be merged label Apr 26, 2021
@darkwing darkwing merged commit 15b596a into MetaMask:develop Apr 26, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@brad-decker brad-decker brad-decker approved these changes

@rachelcope rachelcope rachelcope left review comments

@Gudahtt Gudahtt Gudahtt left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@darkwing @brad-decker @Gudahtt @rachelcope @PatrykLucka