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

Implement Ledger Live bridge #10293

Merged
merged 33 commits into from
Apr 26, 2021
Merged

Implement Ledger Live bridge #10293

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
f8f7221
uri intent
PatrykLucka Oct 21, 2020
8d15a7c
remove console log
PatrykLucka Oct 21, 2020
924cb65
remove open bridge on click
PatrykLucka Nov 3, 2020
08d5ecd
update add hardware wallet design
PatrykLucka Dec 8, 2020
770f4da
uri intent
PatrykLucka Oct 21, 2020
070c18f
remove console log
PatrykLucka Oct 21, 2020
75c22ae
remove open bridge on click
PatrykLucka Nov 3, 2020
bde398c
update yarn.lock
PatrykLucka Jan 11, 2021
6d78d85
Conditionally add assets
darkwing Jan 27, 2021
c0efd71
Introduce advanced feature toggle for using ledger live instead of u2f
darkwing Jan 27, 2021
23710d2
Add localization
darkwing Feb 10, 2021
718a52e
Send useLedgerLive setting to the user
darkwing Feb 11, 2021
e0f72d2
Fix metamask controller tests
darkwing Feb 11, 2021
60fafe5
Don't allow non-WebUSB users to turn on Ledger Live, provide more spe…
darkwing Feb 12, 2021
84ff751
Remove USB check
darkwing Feb 18, 2021
931b86c
Remove irrelevant test
darkwing Feb 22, 2021
d7ec62b
Use async setLedgerLivePreference in controller
darkwing Mar 2, 2021
de08464
Fix l10n
darkwing Mar 2, 2021
4b4ec1d
Don't use redux pattern for setLedgerLivePreference
darkwing Mar 3, 2021
ea39201
Remove rogue comma from l10n as result of rebase
darkwing Mar 16, 2021
08708ad
Await keyring retrieval
darkwing Mar 16, 2021
e631845
Communicate the ledger live preference in the MetaMask Controller sub…
darkwing Mar 22, 2021
5de54f6
Protect the updateTransportMethod call
darkwing Mar 25, 2021
96e1426
Restore eth-ledger-bridge-keyring package
darkwing Mar 25, 2021
09741f3
Add fallback for unsuccessful transporot update, localize text
darkwing Apr 14, 2021
629ab6e
Update to ledger keyring to 0.4.0
darkwing Apr 20, 2021
e473bcf
Address minor feedback
darkwing Apr 21, 2021
daaf61e
Show loading indicator for ledger live preference toggle
darkwing Apr 21, 2021
e1bf3a6
Use proper CSS naming for message link
darkwing Apr 22, 2021
972f2ca
Remove unused CSS class
darkwing Apr 22, 2021
e8c9000
Remove unnecessary pointer cursor
darkwing Apr 22, 2021
b907808
Re-throw error
darkwing Apr 22, 2021
7b5a27a
Update text in e2e test
darkwing Apr 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Remove USB check
  • Loading branch information
@darkwing
darkwing committed Apr 23, 2021
commit 84ff751a5ee9d27d7bd2ae2d6873b7a9415f914b
7 changes: 5 additions & 2 deletions app/scripts/metamask-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2689,8 +2689,11 @@ export default class MetamaskController extends EventEmitter {
this.preferencesController.setLedgerLivePreference(val);

// TODO: Calling async functionality in a sync function isn't great
this.getKeyringForDevice('ledger').then(keyring => {
console.log("[MMExtensionController] Calling keyring.updateTransportMethod with value", val)
this.getKeyringForDevice('ledger').then((keyring) => {
console.log(
'[MMExtensionController] Calling keyring.updateTransportMethod with value',
val,
);
keyring.updateTransportMethod(val);
});
Copy link
Member

@Gudahtt Gudahtt Apr 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We should probably re-throw the error here, so that the caller of setLedgerLivePreference can see that something failed.


Expand Down
6 changes: 0 additions & 6 deletions ui/app/components/ui/toggle-button/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,4 @@
visibility: hidden;
}
}

&--disabled {
opacity: 0.5;
pointer-events: none;
cursor: default;
}
}
1 change: 0 additions & 1 deletion ui/app/pages/settings/advanced-tab/advanced-tab.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,6 @@ export default class AdvancedTab extends PureComponent {
onToggle={(value) => setLedgerLivePreference(!value)}
Copy link
Member

Choose a reason for hiding this comment

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

If setLedgerLivePreference throws an error, it would be lost here 🤔

Maybe this is something to solve another time, but this might be a good case for an inline error. Showing something to indicate a "loading" state might be nice as well, at least if it's a perceptible length of time.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like you've added a loading indicator, but failure still isn't shown to the user, beyond the toggle not changing state.

I don't want to block on this, but we should definitely consider improving this in the future.

offLabel={t('off')}
onLabel={t('on')}
disabled={!('usb' in window.navigator)}
/>
</div>
</div>
Expand Down