RSK `ledger` integration

[ahsan-javaiid]

Description

• This PR adds ledger integration for RSK
• Currently (main branch) is using hardcoded derivation path here in ledger service.
• When ledger service connect with RSK app on ledger with eth dpath then RSK app will simply reject it.
• So mainly this PR is making derivation path dynamic in ledger service and on UI as well to make it work for RSK

Demo

Type of Change

• [X] Fix/ Enhancement / Integration

Testing information

• Activate SUPPORT_RSK env flag
• Connect with ledger
• Activate RSK app on ledger
• Good to go
• Transaction signing is working

Testing Environment

SUPPORT_RSK=true

Checklist

• [X] Git hooks enabled
• [X] No lint errors
• [X] Git commit is signed

Latest build: extension-builds-2577 (as of Thu, 05 Jan 2023 06:23:14 GMT).

[ahsan-javaiid]

cc: @0xDaedalus @alepc253

[ahsan-javaiid]

Tally Team 👋

I have updated the pull request with ui based approach. Now before connecting to ledger user will be able to select dPath w.r.t ledger app.

See Demo video here:

https://user-images.githubusercontent.com/104683677/201657410-29363c75-9101-461d-97a6-33bad3af7c9e.mp4

• Also resolved previous feedback and not using any dependencies
• Ready for review and most welcome for any thoughts 🤔

[VladUXUI]

This is looking awesome! 👏 😍 We will probably update soon with new onboarding, but i'm happy for this to be shipped as is (ui/ux wise)

[ahsan-javaiid]

Hi Tally team 👋

Feel free to share if there are any further thoughts on this PR.

[ahsan-javaiid]

Hi Tally team 👋

Updated this pull request and implemented the ledger upgrade flow to include dropdown

Reference issue: https://github.com/tallycash/extension/issues/2623

Demo:

Demo video:

https://user-images.githubusercontent.com/104683677/205837481-d63cbfe9-fd77-4a8c-8405-2ecb4a6f1066.mov

Feel free to review the pull request and share thought's if anything needs to changed. 🙏

[VladUXUI]

This is looking good! Will build today and test it out, as i see some small UI clean-up needed. We since added BSC and Avalanche, so this will probably need to be updated a bit. The logic is quite simple i hope, if it has a Ledger app, then it need a different select

[VladUXUI]

Hey hey, so i just tested out the network selector from a ui pov, and added a few clean-up things. Pasting this as an image as it was easier for me to do.

Also i moved the Figma to a public file so you can access it and look at stuff. https://www.figma.com/file/EiXpBpNz2HcoeFJftgC5oc/Tally-Public?node-id=1209%3A2200

Do let me know if it isn't working

[VladUXUI]

The next page after network is selected needs a back button, just in case users change their mind

[ahsan-javaiid]

Update: Design improvements added 👍

[VladUXUI]

Looking good @ahsan-javaiid ! The two remaining items are:

• adding Binance & Avalanche as selection
• Adding a back button on Ledger connect page

Are these things that you would like to do? or should i ask the team to create another PR for these?

[ahsan-javaiid]

Update:

• Added new chains selection

• Added back button

Let me know if anything needs to be changed further!

[VladUXUI]

Looking very good on my end! Awesome job @ahsan-javaiid! Will let somebody from dev team to give final say

[kkosiorowska]

It looks great after the design improvements! :grinning: But I have a small problem with the AVAX network. I cannot connect by selecting this network. I don't have this issue for RSK or BNB chain.

[ahsan-javaiid]

Hi Tally team 👋

Users are eager to use Tally wallet with Roostock <> Ledger. I would be happy to do any further changes to get this pull request considered.

Feel free to share further thoughts on this pull request.

[alepc253]

hey @kkosiorowska @jagodarybacka, any update on this feature? is there anything else we could help with from Rootstock team?

[ahsan-javaiid]

Sorry for the delay 😭 Awesome job was done here!

From UI testing:

• I think this selector should be hidden for now as it is bugged and is not really changing dpath but is going back to network selector sceen
• with tabbed onboarding turned on SUPPORT_TABBED_ONBOARDING=true we should support a similar flow 🤔 but it shouldn't block this PR IMO

A couple of comments, mostly non-blocking, besides hiding bugged dpath selector

Done!

[VladUXUI]

With the addition of network selection at the beginning of the screen. Do we still need dpath selection? Curious how many users have custom dpath in Ledger

[jagodarybacka]

@VladUXUI I would say we do need it for the convenience of more advanced users, it would be a nice improvement here. For seed path import there is no way to choose the network there so we can either do it based on dpath selector or network selector 🤔

@ahsan-javaiid ty for fixes!

@0xDaedalus could you take a look - I think it's good to go and we can take care of tabbed onboarding later

[VladUXUI]

Yeah, agreed. But i would rather fix than hide? It looks like it's fixed

[ahsan-javaiid]

Hi team: 👋

Just a reminder to consider this pull request.🎗 Feel free to share if anything blocking on this pull request. 🚧

Hope to get this PR considered soon. 🙏

[jagodarybacka]

Hey @ahsan-javaiid I checked it yesterday and overall it looks nice. One thing I noticed we missed is that now we are missing Ledger live dpath when Ethereum is selected while importing Ledger. We will figure that out with @VladUXUI.

We are planning to release tabbed onboarding soon so we think its best to merge this PR after the initial release of tabbed onboarding is done 🔜 So no action is needed on your side right now, we will take care of it 🫶

[ahsan-javaiid]

Hi Taho Team: 👀 Eyes on this pull request if it can be prioritised. 👀 🙏

[Shadowfiend]

Hey folks, I wanted to poke at what is now #3385 before going deep on review here. Now that 3385 is written up, I'm going to try and give this some deep 👀 so we can get over the finish line 💪