(Feature) Multiple Ledger accounts for one session

[vbaranov]

Problem: In the current version of NW on every unlock of HD account, the previous HD account is removed from the list of accounts.

Solution: This PR makes possible to add multiple HD accounts once from one Ledger in one device session* and work with them until next unlock.
Related changes in Ledger related dependency vbaranov/eth-ledger-bridge-keyring@949e378 and vbaranov/eth-ledger-bridge-keyring@097807e

*if device was forgotten, connecting it again and unlocking still will replace previously unlocked HD accounts
**PR is only for Ledger. Nothing is changed for Trezor: 1 device = 1 account.

[dennis00010011b]

1. I can't connect either Ledger or Trezor, the loader displayed all the time . There was no such problem in previous builds
2. Connect hardware wallet -> Trezor opens full browser window, but a popup has been opening in previous version

[vbaranov]

@dennis00010011b does it relate to the current PR only or to develop branch also?

[dennis00010011b]

@vbaranov
YEs

@dennis00010011b does it relate to the current PR only or to develop branch also?

Yes, this issue in develop, but not in master

[vbaranov]

@dennis00010011b I reproduced this issue.

Got minor updates regarding hd wallets from upstream

If the issue is still reproducible for you:

• check, that you are using the latest version of firmware
• login/logout from Nifty Wallet
• reconnect your HD device

Let me know, if the issue is still there

[dennis00010011b]

@vbaranov
Issue still here. But at the same time both Ledger and Trezor work perfect from master branch build and with MM.
In current build:

1. App don’t check if device is disconnected, there is no error about.
2. No error if waiting for long time and can’t connect to device
3. For Trezor connection a full page opens instead popup
   Tested with Ledger Nano S (firmware 1.4.2-latest), Trezor One

[vbaranov]

@dennis00010011b I've made downgrade of dependencies related to HD wallets. Could you please test once more?

[dennis00010011b]

@vbaranov
Actually only one account has added several times

[vbaranov]

@dennis00010011b I've tried to repeat this, but have no success. Looks like 1 account imported 13 times. Could you provide the steps to repeat this issue?

[dennis00010011b]

@vbaranov

Looks like 1 account imported 13 times

Yes, that's right
Tested with Ledger Nano S, firmware 1.4.2, app Ethereum v. 1.1.7
MacOS 10.13.6, Chrome
https://www.useloom.com/share/67aed3e8810246ed9e1aa1812cebf0d2

[dennis00010011b]

@vbaranov
I still can reproduce the issue

[dennis00010011b]

@vbaranov
Unlock Ledger: account is selected by default if any account already has been imported before.
Steps:

1. Connect Ledger
   2 .Import several accounts
2. Connect Ledger
3. Observe list of accounts and button 'Unlock'

Expected result:

• no accounts should be selected

Actual result:

• one account is selected

https://www.useloom.com/share/c30774ebc6244bd2afa60ab58e0260de

[dennis00010011b]

@vbaranov
Number in name of new imported/created accounts is not in order if Ledger/Trezor accounts have been added.
Example, if six Ledger accounts have been added then new created/imported account/contract has name Account 8 , not Account 2

[dennis00010011b]

@vbaranov
If Ledger device was disconnected then imported accounts are unavailable.
Expected that Ledger's accounts should be observable (like it works for Trezor)
https://www.useloom.com/share/1bc3a7c39b0e4ec2ac24255a1fd7989e

[vbaranov]

@dennis00010011b

Unlock Ledger: account is selected by default if any account already has been imported before.

Yes, currently selected HD account is selected in the HD accounts screen. It makes sense in the case of radio buttons when selecting another address will automatically deselect the current one, but in the case of checkboxes, it doesn' make sense. Ok, will remove this behaviour.

Number in name of new imported/created accounts is not in order if Ledger/Trezor accounts have been added.

Yes, this is intentional. The numeration is pass-through all types of accounts. I am not sure we could fix it without massive reorganization. I will keep it in mind in the future refactoring.

If Ledger device was disconnected then imported accounts are unavailable.
Expected that Ledger's accounts should be observable (like it works for Trezor)

Due to a long connection timeout, connection to HD device/unlocking, it makes sense to move the connection to HD device right before sending tx. Will do. Thanks.

[dennis00010011b]

@vbaranov

Found an issue, that unlocked HD account is always the last chosen HD account. Changing >to another HD account in the accounts dropdown doesn't unlock it actually (NW is still trying >to send tx from the last from the selected account from the HD accounts screen). Working >on it.

This issue reproducible again. There is no confirmation request on device's screen when try to send ether from account different that Ledge1 (see video)

https://www.useloom.com/share/5fed9bc8b9f44b7a8f44a41207c9d17d

[vbaranov]

@dennis00010011b

Yes, currently selected HD account is selected in the HD accounts screen. It makes sense in the case of radio buttons when selecting another address will automatically deselect the current one, but in the case of checkboxes, it doesn' make sense. Ok, will remove this behaviour.

Fixed 83b90b7

Due to a long connection timeout, connection to HD device/unlocking, it makes sense to move the connection to HD device right before sending tx. Will do. Thanks.

Decided not to move this logic to pending tx page to not make it heavier. I left logic on account change in dropdown but in the background. 56759f2 and 5b7010f

[vbaranov]

@dennis00010011b

This issue reproducible again. There is no confirmation request on device's screen when try to send ether from account different that Ledge1 (see video)

I cannot reproduce it. Does it still exist for you?

[dennis00010011b]

@vbaranov

This issue reproducible again. There is no confirmation request on device's screen when try to send ether from account different that Ledge1 (see video)

I cannot reproduce it. Does it still exist for you?

I cannot reproduce it anymore.
Tested it , no issues were found