Skip to content

feat: add new keyring type for oneKey#5216

Merged
Akaryatrh merged 2 commits into
mainfrom
feat/793-oneKey-dedicated-keyring
Feb 12, 2025
Merged

feat: add new keyring type for oneKey#5216
Akaryatrh merged 2 commits into
mainfrom
feat/793-oneKey-dedicated-keyring

Conversation

@Akaryatrh

@Akaryatrh Akaryatrh commented Jan 30, 2025

Copy link
Copy Markdown
Contributor

Explanation

This PR adds support for a dedicated OneKey keyring (until now it was sharing the same keyring instance than Trezor) so it's considered as a standalone device and could get its own tag inside account list.

There are two others PRs:

References

Fixes: https://github.com/MetaMask/accounts-planning/issues/793

Changelog

@metamask/accounts-controller

  • utils: add OneKey keyring type
  • tests: update accounts controller unit test with OneKey keyring type

@metamask/keyring-controller

  • controller: add OneKey keyring type

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@Akaryatrh Akaryatrh force-pushed the feat/793-oneKey-dedicated-keyring branch from c7dd5e2 to 4893887 Compare February 7, 2025 10:55
@Akaryatrh Akaryatrh marked this pull request as ready for review February 7, 2025 10:56
@Akaryatrh Akaryatrh requested a review from a team as a code owner February 7, 2025 10:56
@Akaryatrh Akaryatrh marked this pull request as draft February 7, 2025 10:56
github-merge-queue Bot pushed a commit to MetaMask/accounts that referenced this pull request Feb 11, 2025
This PR adds support for a dedicated OneKey keyring (until now it was
sharing the same keyring instance than Trezor) so it's considered as a
standalone device and could get its own tag inside account list.

There are two others PRs:

- metamask-extension:
MetaMask/metamask-extension#29999
- core (accounts and keyring controllers):
MetaMask/core#5216


Fixes: MetaMask/accounts-planning#793
@Akaryatrh Akaryatrh merged commit 75230dc into main Feb 12, 2025
@Akaryatrh Akaryatrh deleted the feat/793-oneKey-dedicated-keyring branch February 12, 2025 09:25
github-merge-queue Bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Mar 21, 2025
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

This PR adds support for a dedicated OneKey keyring (until now it was
sharing the same keyring instance than Trezor) so it's considered as a
standalone device and could get its own tag inside account list.

There are two others PRs:
- eth-trezor-package: MetaMask/accounts#175
- Core (account and keyring controllers):
MetaMask/core#5216

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29999?quickstart=1)

## **Related issues**

Fixes: MetaMask/accounts-planning#793

## **Manual testing steps**

You need both OneKey and Trezor devices to test. Trezor device can be
replaced by [this simulator](https://github.com/trezor/trezor-user-env).
⚠️ Simulator takes over other devices, you need to disconnect first it
if you import a OneKey account.

1. Import one account from Trezor device.
2. Check that it gets the "Trezor" tag below account name in account
list
3. Import one account from OneKey device
4. Check that it gets the "OneKey" tag below account name in account
list and that account name matches the pattern "OneKey x"

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/user-attachments/assets/37adb368-7d9e-4358-8842-b3b07778d094

### **After**


https://github.com/user-attachments/assets/02ca4fec-f0b5-4822-bd95-b3b9ec9ee933

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants