Skip to content

refactor: add EncryptionKey type parameter to ExportableKeyEncryptor#5395

Merged
mikesposito merged 7 commits into
mainfrom
mikesposito/encryptor-generic-type
Apr 15, 2025
Merged

refactor: add EncryptionKey type parameter to ExportableKeyEncryptor#5395
mikesposito merged 7 commits into
mainfrom
mikesposito/encryptor-generic-type

Conversation

@mikesposito

@mikesposito mikesposito commented Feb 26, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Explanation

The ExportableKeyEncryptor type used to identify encryptor that support exported keys has been made generic, with a type parameter EncryptionKey to let the consumer define the type of key that will be used for encryption (e.g. CryptoKey). The type parameter defaults to unknown, making this non-breaking.

References

Changelog

@metamask/keyring-controller

  • CHANGED: The ExportableKeyEncryptor is now a generic type with a type parameter EncryptionKey (#5395)
    • The type parameter defaults to unknown

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

mikesposito
mikesposito commented Feb 26, 2025
View reviewed changes
Comment thread packages/keyring-controller/CHANGELOG.md Outdated
@mikesposito mikesposito marked this pull request as ready for review February 26, 2025 10:01
@mikesposito mikesposito requested review from a team as code owners February 26, 2025 10:01
@mikesposito mikesposito mentioned this pull request Feb 26, 2025
Merged
7 tasks
@mikesposito mikesposito changed the title refactor: add generic EncryptionKey type to ExportableKeyEncryptor refactor: add EncryptionKey type parameter to ExportableKeyEncryptor Feb 26, 2025
cryptodev-2s
cryptodev-2s approved these changes Apr 14, 2025
View reviewed changes

@cryptodev-2s cryptodev-2s left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mikesposito mikesposito enabled auto-merge (squash) April 15, 2025 09:59
@mikesposito mikesposito merged commit 42a2c9c into main Apr 15, 2025
@mikesposito mikesposito deleted the mikesposito/encryptor-generic-type branch April 15, 2025 10:05
github-merge-queue Bot pushed a commit to MetaMask/metamask-mobile that referenced this pull request Apr 25, 2025
@mikesposito @NicolasMassart
chore: cp-7.25.0 bump @metamask/keyring-controller to ^21.0.4 (#1…
6de2b94 
…4702)

<!--
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**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

Updating `@metamask/keyring-controller` to `^21.0.4`

```markdown
## [21.0.4]

### Fixed

- Ensure no duplicate accounts are persisted ([#5710](MetaMask/core#5710))

## [21.0.3]

### Changed

- `ExportableKeyEncryptor` is now a generic type with a type parameter `EncryptionKey` ([#5395](MetaMask/core#5395))
  - The type parameter defaults to `unknown`

### Fixed

- Fixed wrong error message thrown when using the wrong password ([#5627](MetaMask/core#5627))

## [21.0.2]

### Changed

- Bump `@metamask/keyring-api` from `^17.2.0` to `^17.4.0` ([#5565](MetaMask/core#5565))
- Bump `@metamask/keyring-internal-api` from `^6.0.0` to `^6.0.1` ([#5565](MetaMask/core#5565))

### Fixed

- Ignore cached encryption key when the vault needs to upgrade its encryption parameters ([#5601](MetaMask/core#5601))


```

## **Related issues**

Fixes: #14228 

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

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

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/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: Nico MASSART <NicolasMassart@users.noreply.github.com>
runway-github Bot pushed a commit to MetaMask/metamask-mobile that referenced this pull request Apr 30, 2025
@mikesposito @NicolasMassart
chore(runway): cherry-pick chore: cp-7.25.0 bump `@metamask/keyring-c…
df5e74d 
…ontroller` to `^21.0.4` (#14702)

<!--
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**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

Updating `@metamask/keyring-controller` to `^21.0.4`

```markdown
## [21.0.4]

### Fixed

- Ensure no duplicate accounts are persisted ([#5710](MetaMask/core#5710))

## [21.0.3]

### Changed

- `ExportableKeyEncryptor` is now a generic type with a type parameter `EncryptionKey` ([#5395](MetaMask/core#5395))
  - The type parameter defaults to `unknown`

### Fixed

- Fixed wrong error message thrown when using the wrong password ([#5627](MetaMask/core#5627))

## [21.0.2]

### Changed

- Bump `@metamask/keyring-api` from `^17.2.0` to `^17.4.0` ([#5565](MetaMask/core#5565))
- Bump `@metamask/keyring-internal-api` from `^6.0.0` to `^6.0.1` ([#5565](MetaMask/core#5565))

### Fixed

- Ignore cached encryption key when the vault needs to upgrade its encryption parameters ([#5601](MetaMask/core#5601))


```

## **Related issues**

Fixes: #14228 

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

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

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/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: Nico MASSART <NicolasMassart@users.noreply.github.com>
EtherWizard33 pushed a commit to MetaMask/metamask-mobile that referenced this pull request May 2, 2025
@mikesposito @NicolasMassart
chore: cp-7.25.0 bump @metamask/keyring-controller to ^21.0.4 (#1…
b8e0cd9 
…4702)

<!--
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**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

Updating `@metamask/keyring-controller` to `^21.0.4`

```markdown
## [21.0.4]

### Fixed

- Ensure no duplicate accounts are persisted ([#5710](MetaMask/core#5710))

## [21.0.3]

### Changed

- `ExportableKeyEncryptor` is now a generic type with a type parameter `EncryptionKey` ([#5395](MetaMask/core#5395))
  - The type parameter defaults to `unknown`

### Fixed

- Fixed wrong error message thrown when using the wrong password ([#5627](MetaMask/core#5627))

## [21.0.2]

### Changed

- Bump `@metamask/keyring-api` from `^17.2.0` to `^17.4.0` ([#5565](MetaMask/core#5565))
- Bump `@metamask/keyring-internal-api` from `^6.0.0` to `^6.0.1` ([#5565](MetaMask/core#5565))

### Fixed

- Ignore cached encryption key when the vault needs to upgrade its encryption parameters ([#5601](MetaMask/core#5601))


```

## **Related issues**

Fixes: #14228 

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

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

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/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: Nico MASSART <NicolasMassart@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gantunesr gantunesr gantunesr approved these changes

@cryptodev-2s cryptodev-2s cryptodev-2s approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mikesposito @gantunesr @cryptodev-2s