Skip to content

fix: Use latest DecryptMessageManager EncryptMessageManager to extend controllers#29237

Merged
OGPoyraz merged 22 commits intomainfrom
temp/message-manager-transition
Jan 27, 2025
Merged

fix: Use latest DecryptMessageManager EncryptMessageManager to extend controllers#29237
OGPoyraz merged 22 commits intomainfrom
temp/message-manager-transition

Conversation

@OGPoyraz
Copy link
Copy Markdown
Member

@OGPoyraz OGPoyraz commented Dec 16, 2024

Description

This PR aims to adapt latest changes in the @metamask/message-manager DecryptMessageManager and EncryptionPublicKeyManager classes. The latest change in core mainly tries to adapt BaseControllerV2. Hence in the extension, minimum changes made in the wrapper classes to keep functionality as is.

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3747
Related core PR: MetaMask/core#5103

Manual testing steps

  1. Go to test-dapp and Encrypt / Decrypt section
  2. "Get Encryption Key", "Encrypt" and "Decrypt" must be functional and work without issue

Screenshots/Recordings

Encrypt.Decrypt.mov

Before

After

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.

@OGPoyraz OGPoyraz changed the title fix: Temp BaseController migration for Decrypt/Encrypt message manager fix: Use latest DecryptMessageManager EncryptMessageManager to extend controllers Jan 6, 2025
@OGPoyraz OGPoyraz force-pushed the temp/message-manager-transition branch from 4174afe to fba4880 Compare January 6, 2025 14:54
@OGPoyraz OGPoyraz marked this pull request as ready for review January 6, 2025 14:55
jpuri
jpuri previously approved these changes Jan 7, 2025
Comment on lines -178 to -179
this._decryptMessageManager.hub.on('updateBadge', () => {
this.hub.emit('updateBadge');
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

XMessageManagers will be emitting the X:updateBadge via messaging system hence these are not needed anymore, we directly listen manager event on background.js

matthewwalsh0
matthewwalsh0 previously approved these changes Jan 9, 2025
OGPoyraz added a commit to MetaMask/core that referenced this pull request Jan 14, 2025
…eControllerV2` (#5103)

## Explanation

<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

This PR aims to remove `BaseControllerV1` usage from
`AbstractMessageManager`. As expected this change affected both
`DecryptMessageManager` (`DMM`) and
`EncryptionPublicKeyManager`(`EPKM`).

Since extension already have wrapper classes for both `DMM` & `EPKM` in
the extension code, we want to keep changes minimal and make these both
classes in the core work like controller but let wrappers sync the state
in their classes as we currently do. You can find the extension PR in
the references below.

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

* Fixes : MetaMask/MetaMask-planning#3747
* Related extension PR:
MetaMask/metamask-extension#29237

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/message-manager`

- **BREAKING:** Base class of `DecryptMessageManager` and
`EncryptionPublicKeyManager`(`AbstractMessageManager`) now expects new
options to initialise
- **BREAKING:** Removed internal event emitter (`hub` property) from
`AbstractMessageManager`
- **BREAKING:** `unapprovedMessage` and `updateBadge` removed from
internal events. These events are now emitted from messaging system
- Controllers should now listen to `DerivedManagerName:X` event instead
of using internal event emitter.


## Checklist

- [X] I've updated the test suite for new or updated code as appropriate
- [X] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [X] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [X] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
@OGPoyraz
Copy link
Copy Markdown
Member Author

@metamaskbot update-policies

@metamaskbot
Copy link
Copy Markdown
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@metamaskbot metamaskbot requested review from a team as code owners January 17, 2025 13:14
@OGPoyraz OGPoyraz added the team-confirmations Push issues to confirmations team label Jan 17, 2025
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [c9c61f4]
Page Load Metrics (1755 ± 105 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30419741397574276
domContentLoaded136823161734221106
load138523311755218105
domInteractive2589482010
backgroundConnect470222110
firstReactRender1578372411
getState46314168
initialActions01000
loadScripts9751881129320598
setupStore66615189
uiStartup165427761994260125
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 38.15 KiB (0.64%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

jpuri
jpuri previously approved these changes Jan 17, 2025
@OGPoyraz
Copy link
Copy Markdown
Member Author

@metamaskbot update-policies

@metamaskbot
Copy link
Copy Markdown
Collaborator

No policy changes

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [aeeba44]
Page Load Metrics (2311 ± 130 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint27429551522955459
domContentLoaded196829402273267128
load202429582311272130
domInteractive28145572813
backgroundConnect1297482813
firstReactRender26102512512
getState106937199
initialActions01000
loadScripts144823641706237114
setupStore75015126
uiStartup229933632691329158
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 38.35 KiB (0.65%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@OGPoyraz
Copy link
Copy Markdown
Member Author

@metamaskbot update-policies

@metamaskbot
Copy link
Copy Markdown
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [1a26e5a]
Page Load Metrics (1784 ± 59 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15851981178611857
domContentLoaded15381949175711656
load15751990178412359
domInteractive246734105
backgroundConnect86431199
firstReactRender1593452512
getState55818189
initialActions01000
loadScripts1127142512999445
setupStore65412136
uiStartup17912333206413967
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 38.35 KiB (0.65%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@OGPoyraz OGPoyraz enabled auto-merge January 27, 2025 08:34
@OGPoyraz OGPoyraz added this pull request to the merge queue Jan 27, 2025
Merged via the queue into main with commit 75b04c9 Jan 27, 2025
@OGPoyraz OGPoyraz deleted the temp/message-manager-transition branch January 27, 2025 10:38
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2025
@metamaskbot metamaskbot added the release-12.12.0 Issue or pull request that will be included in release 12.12.0 label Jan 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-12.12.0 Issue or pull request that will be included in release 12.12.0 team-confirmations Push issues to confirmations team

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants