Support QR wallets

[katat]

For the feature issue https://github.com/tallycash/extension/issues/2259

While getting familiar with the codebase, I drafted the QRHardwareService, which I think would outline the data model, new lib dependencies and interaction workflow to form a baseline to discuss the technical implementation in more detail if any.

The workflow and data model is outlined in the integration test. The data model relies on the encoding/decoding facilities from the keystone library and for now stores the cbor string to encapsulate the HD keyring properties in the database.

I think it would make more sense to spread the database table columns as defined here instead of a single cbor column. In addition, I am thinking if it is necessary to create another database table to store the mappings between addresses and QR HD wallets, as these mappings might be useful at certain points.

For the next, I will implement the other parts to interact with this newly created service, such as the reducer, event handling in the main service, and the UI component etc, and get them wire up.

I would keep test-driven the implementations for these pieces similar to how is done in the service in this draft PR, but in different domains, such as reducer, selector etc. If there is a guideline on the automated testing, please let me know. Note that I updated the setupJest.ts in order to run the test, which requires node environment as the somehow there is Uint8Array issue in the jsdom environment.

Also, please let me know if there is a design ready for this feature. Otherwise I would just draft the interfaces and workflows for everyone to review this PR and get a feeling of what the optimal UI design should be.

todo

• [x] with feature flag
• [x] delegated by SigningService
• [x] wire up with UI
• [ ] support signing message
• [ ] support importing non-HD QR wallet

[mhluongo]

@Shadowfiend drawing your attention here

[katat]

The UI has been wired up with the background service, and supports the following features:

• Import HD wallets from air-gapped wallets (I tested with airgap.it vault mobile app. In theory, it should also support keystone hardware. If you got a keystone wallet, please help test to see if it works.) The airgap.it vault doesn't seem to support exporting non-HD wallet(which seems to be a bug), so I left it for now.
• Sign transaction and typed data (The sign message feature will be implemented as outlined in the todo)

I think this is ready to get a sense of the user experiences and you may want to provide feedback on both the UI designs and the implementation level. So I update the PR to be ready for review.

To test the UI, please turn on both SUPPORT_QR_WALLETS and USE_UPDATED_SIGNING_UI.

Please feel free to comment.

[katat]

Hi,

I wonder if there is a chance to give this PR a round of review, so I can know whether this is going in the right direction since it involves many changes. Feel free to let me know if something should be ready before it is worth a review.

Thanks

[greg-nagy]

Hey

Sorry for the slow response! I will go through this tomorrow. Thanks for the PR and for the patience

[katat]

Thanks @greg-nagy for the comment. Sounds like it is in the right direction for me to continue.

One thing I think is necessary at this stage is the UI/UX design. For now, I just make it very simple with minimal/no guidelines on the UI. To complete this PR, I guess it would need to be done with a decent UI/UX design?

[Shadowfiend]

@katat are you intending on continuing work here? The PR is a bit stale, and would like to figure out what the right next steps are.

[katat]

Hi. I won’t be available for this in a month or two. Happy to see someone can pick this up. So please feel free to go ahead.

On Tue, 25 Apr 2023 at 2:02 AM, Antonio Salazar Cardozo < @.***> wrote:

@katat https://github.com/katat are you intending on continuing work here? The PR is a bit stale, and would like to figure out what the right next steps are.

— Reply to this email directly, view it on GitHub https://github.com/tahowallet/extension/pull/2321#issuecomment-1520603548, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMDRGAVLJW2R7HMBKWVJ43XC254TANCNFSM6AAAAAAQXN4DYM . You are receiving this because you were mentioned.Message ID: @.***>

-- Thanks, Kata

[image: twitter] https://twitter.com/katat [image: medium] @.***> [image: github] https://github.com/katat [image: upwork] https://www.upwork.com/freelancers/~016c0672d0126d4318 https://stackoverflow.com/story/katat [image: linkedin] https://www.linkedin.com/in/katachoi/ [image: angel.co] https://angel.co/katat-choi

[Shadowfiend]

Ok, going to close this PR for now, and we can revisit if and when some bandwidth becomes available to pick it up 🙏