metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard

[WIP]Keystone USB integration

Open soralit opened this issue 5 months ago • 8 comments

Description

This PR adds Keystone USB integration. It includes two steps:

  1. Connect with Keystone
  2. Sign Transactions / Messages with Keystone.

All the operations to communicate with Keystone will open a tab for requesting USB devices, like what Trezor did.

Code changes:

  1. Add Keystone USB Keyring related dependencies.
  2. Implements Connect Hardware UI for Keystone USB connection.
  3. Implements background chrome bridges.
  4. Implements frontend Keystone USB Bridge page for USB communication.

Open in GitHub Codespaces

Pre-merge author checklist

Pre-merge reviewer checklist

  • [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

soralit avatar Jun 30 '25 09:06 soralit

CLA Signature Action:

Thank you for your submission, we really appreciate it. We ask that you all read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:

I have read the CLA Document and I hereby sign the CLA

By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to this repository.

1 out of 2 committers have signed the CLA. :white_check_mark: @soralit :x: @qkin

GitHub can't find an account for qkin. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

github-actions[bot] avatar Jun 30 '25 09:06 github-actions[bot]

During my testing, i face some issues or this PR. Here are videos from difference flows:

import accounts Flow:

https://github.com/user-attachments/assets/78b6b479-e05c-4b8c-8b6c-e26dd85e8450

Issues:

  • The screen for Keystone USB Bridge need to be improved. the button Connect should be disabled after connect successfully or turn to disconnect.
  • After user approve connection in keystone. it is better to let screen focus back to Hardware accounts imported screen. because user may not be notice that accounts lists will appear in another tab.
  • In the video 0.37 second, for some reason, the accounts list selection list has suddently disappear, i need to re-click the keystone button to get the accounts list to appear again.
  • After selected the keystone account i want to import and click Unlock. i got Accounts already exists error, however i can see the Keystone 1 and Keystone 2 in the list, in this flow, Accounts already exists error should not be displayed, and screen should automatically back to main screen.

dawnseeker8 avatar Aug 12 '25 13:08 dawnseeker8

Connect to test-dapp

Main issues:

  • if user close this tab below tab Keystone USB bridge then all operations in test-dapp such as send token, sign transactions, sign message etc. will all failed. Screenshot 2025-08-12 at 14 52 36
  • if i keep the tab open, there are two times, keystone devices crashes when sign transaction, the device need to restarted. to connect again.

dawnseeker8 avatar Aug 12 '25 14:08 dawnseeker8

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​keystonehq/​metamask-keystone-webusb-bridge@​0.2.0681006192100
Added@​keystonehq/​metamask-keystone-usb-keyring@​0.2.0761007194100

View full report

socket-security[bot] avatar Sep 05 '25 03:09 socket-security[bot]

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

socket-security[bot] avatar Sep 05 '25 03:09 socket-security[bot]

Hi, @soralit Pleas help to resolve the conflict in this PR. so far the PR is ok, but we need to pass to metamask account team to do the final compatibility review.

dawnseeker8 avatar Sep 23 '25 18:09 dawnseeker8

Another change was comments from metamask account teams. account teams has release a new keyring type in https://github.com/MetaMask/accounts/blob/03d8fd01e7ad3895b8507a72b7ce4c1b78aaffa8/packages/keyring-utils/src/keyring.ts Your library https://github.com/KeystoneHQ/keystone-sdk-base/blob/master/packages/metamask-keystone-usb-keyring/src/KeystoneUSBKeyring.ts need to chnage accordingly to support them to provide the new account tree compatible feature.

Here is the simple example how to implement a new keyring. https://github.com/MetaMask/accounts/blob/03d8fd01e7ad3895b8507a72b7ce4c1b78aaffa8/packages/keyring-eth-simple/src/simple-keyring.ts

dawnseeker8 avatar Sep 24 '25 02:09 dawnseeker8

Another change was comments from metamask account teams. account teams has release a new keyring type in https://github.com/MetaMask/accounts/blob/03d8fd01e7ad3895b8507a72b7ce4c1b78aaffa8/packages/keyring-utils/src/keyring.ts Your library https://github.com/KeystoneHQ/keystone-sdk-base/blob/master/packages/metamask-keystone-usb-keyring/src/KeystoneUSBKeyring.ts need to chnage accordingly to support them to provide the new account tree compatible feature.

Here is the simple example how to implement a new keyring. https://github.com/MetaMask/accounts/blob/03d8fd01e7ad3895b8507a72b7ce4c1b78aaffa8/packages/keyring-eth-simple/src/simple-keyring.ts

Hi, @soralit How is the progress of keyring change on this PR? did you finish changing it base on account team's comments.

dawnseeker8 avatar Dec 09 '25 03:12 dawnseeker8