gh-pages: Add method to share connection status #147
Conversation
digiwand
left a comment
digiwand
left a comment
There was a problem hiding this comment.
LGTM! 👍 I can await the await this.cleanUp() before approving
| } | ||
| catch(e) { | ||
| console.log('LEDGER:::Transport check error', e) | ||
| this.sendConnectionMessage(false) |
There was a problem hiding this comment.
this.onDisconnect() below also appears to call this.sendConnectionMessage(false). Maybe we don't need this line? Same comment applies to similar logic added in bundle.js
| // Ensure the correct (Ethereum) app is open; if not, immediately kill | ||
| // the connection as the wrong app is open and switching apps will call | ||
| // a disconnect from within the Ledger API | ||
| try { |
There was a problem hiding this comment.
This block needs to be skipped for Firefox because the U2F library throws on the await this.transport.send(0xb0, 0x01, 0x00, 0x00) line.
brad-decker
left a comment
brad-decker
left a comment
There was a problem hiding this comment.
Tested with my ledger and everything worked well. I switched between the various connection types as well just to ensure that I was testing the various code branches.
darkwing
force-pushed
the
gh-pages-disconnect
branch
3 times, most recently
from
963e99f to
1218a25
Compare
darkwing
force-pushed
the
gh-pages-disconnect
branch
from
1218a25 to
94dce64
Compare
| catch(e) { | ||
| console.log('LEDGER:::Transport check error', e) | ||
| // A race condition for checking connection status isn't a sign that | ||
| // connection status has changed, so don't disconnect on this type of error |
There was a problem hiding this comment.
[optional] It might be helpful if we add a link to the error name
@see {@link https://github.com/LedgerHQ/ledgerjs/blob/master/packages/hw-transport/src/Transport.ts#L276}
for more context of the 'TransportRaceCondition' error. This is subject to change.
3 participants
This PR adds a method for the Ledger to ensure the correct app is open during connection, as well as share track and share connectivity for the Hardware Connectivity UI (MetaMask/metamask-extension#12725)