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

Published 20 hours ago • verified publisher icon MetaMask

fix: Apply major `ComposableController` API updates that fix broken functionality

Open MajorLift opened this issue 1 year ago • 3 comments

Description

This commit updates the @metamask/composable-controller to the upcoming release 8.0.0.

This involves fixing bugs outlined in https://github.com/MetaMask/metamask-mobile/issues/10073, and applying the major changes to the ComposableController API that has accumulated between the intervening versions.

TODO:

  • Write tests verifying that populated this.datamodel.state has the same structure as EngineState at runtime.
  • Verify that discrepancy between ComposableController generic params doesn't cause issues.
  • Explore possibility of testing that a controller is subscribed to certain events.

Changelog

Changed

  • BREAKING: Instantiate ComposableController class constructor option messenger with a RestrictedControllerMessenger instance derived from the controllerMessenger class field instance of Engine, instead of passing in the controllerMessenger instance directly (#10441)
    • BREAKING: Narrow external actions allowlist from GlobalActions['type'] to an empty union.
    • BREAKING: Narrow external events allowlist from GlobalEvents['type'] to a union of the stateChange events of all controllers included in the EngineState type.
  • BREAKING: Bump @metamask/composable-controller from ^3.0.0 to ^8.0.0 (#10441)
  • BREAKING: Convert the EngineState interface to use type alias syntax to ensure compatibility with types used in MetaMask controllers (#10441)

Fixed

  • BREAKING: Narrow Engine class datamodel class field from any to ComposableController<EngineState, Controllers[keyof Controllers]> (#10441)
  • ComposableController can be instantiated with any collection of child controller instances regardless of the BaseController package version they inherit from (#10441)
    • Fix type errors caused by child controllers inheriting from outdated BaseControllerV1 versions.
    • Restore previously suppressed type-level validation for controllers constructor option.
  • The Engine class state getter method now normalizes null into 0 for the conversionRate values of all native currencies keyed in the currencyRates object of CurrencyRatesControllerState (#10441)

Related issues

  • Closes https://github.com/MetaMask/metamask-mobile/issues/10073
  • Applies https://github.com/MetaMask/core/pull/4467/
  • Supersedes https://github.com/MetaMask/metamask-mobile/pull/10011

Manual testing steps

Screenshots/Recordings

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.

MajorLift avatar Jul 26 '24 16:07 MajorLift

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

github-actions[bot] avatar Jul 26 '24 16:07 github-actions[bot]

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/[email protected] None 0 70.3 kB metamaskbot

🚮 Removed packages: npm/@metamask/[email protected]

View full report↗︎

socket-security[bot] avatar Jul 26 '24 16:07 socket-security[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 179ff4fb79369862698979bb713e4b50e19d4fe1 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/82aa3b81-4cfc-40b0-a586-446d926d865b

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 06 '24 14:09 github-actions[bot]

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 1f02b8aa79c8d28b7f05e73e8f8c1a1a14761f00 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/02252d60-0f2f-4708-8e2c-015784e7e5ef

[!NOTE]

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 06 '24 18:09 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: ef79728ea188a36304f98b7d5ef395388a9caed3 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3f5559cd-123f-49bd-90ee-d24c923d0bea

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 06 '24 19:09 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 0b70f895fac432ea059074c1d7f7b688cb9f71e7 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/66686bcd-c69a-4382-9c5a-f7be2f3fc9e4

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 06 '24 19:09 github-actions[bot]

👍 Dependency issues cleared. 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 06 '24 20:09 socket-security[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 545abec12d01cb2e2dfcaebc57684faad5034201 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cea98bd8-42f0-4c63-b5b1-b281326b541b

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 06 '24 21:09 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 6e69e1c1a8ee54d2b2de87793d0e51801d661187 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/09cc3295-5fb4-4a74-9d02-ba50593b9729

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 09 '24 17:09 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 39bcf98298c1b9a8449fc15403e46c0f00b1a988 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/94141b1f-b0a5-44f0-9b54-f58c22e31472

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 11 '24 16:09 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 9aa4d23ea4231cbaa49bedfbac36a6970d0f302b Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/7363f6b0-a21a-4e3a-9f62-6e5151aa1603

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 11 '24 17:09 github-actions[bot]

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 3d7f48fd697cdc74bb05adcc62f1da4faf2715ac Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/10cd648c-60a2-4d3e-8d9f-eebe47bb763f

[!NOTE]

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 11 '24 18:09 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 1e7f6a4fa28fc66f9ce4605016956f3b536408ae Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/fd44952f-7e5a-4514-b326-f4cb5ef7ddb9

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 11 '24 18:09 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: c8758b31a96316c7add21f391b765922b016d4d9 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b59ca851-faa7-4bcc-9711-1df3230a4751

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Sep 11 '24 18:09 github-actions[bot]

Codecov Report

Attention: Patch coverage is 60.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 57.06%. Comparing base (1a83b1f) to head (153e072). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
app/core/EngineService/EngineService.ts 52.94% 8 Missing :warning:
app/core/Engine/Engine.ts 77.77% 0 Missing and 2 partials :warning:
app/core/BackgroundBridge/BackgroundBridge.js 0.00% 1 Missing :warning:
app/core/BackgroundBridge/WalletConnectPort.ts 0.00% 1 Missing :warning:
Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #10441   +/-   ##
=======================================
  Coverage   57.05%   57.06%           
=======================================
  Files        1817     1818    +1     
  Lines       40880    40881    +1     
  Branches     5166     5165    -1     
=======================================
+ Hits        23326    23329    +3     
+ Misses      15972    15970    -2     
  Partials     1582     1582

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Sep 12 '24 09:09 codecov-commenter

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: c8d030f5c0b07163cfab28700b8e6c4379a42df5 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/70cd4197-85ef-4265-adb5-f3a3e5668923

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Sep 20 '24 19:09 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: bba0f535ad095f2869a914b08af478f6fe5b9ae0 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/46cbabea-1452-4cea-be81-c716860829f6

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Sep 23 '24 12:09 github-actions[bot]

Quality Gate Passed Quality Gate passed

Issues
15 New issues
0 Accepted issues

Measures
0 Security Hotspots
83.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

sonarqubecloud[bot] avatar Oct 03 '24 12:10 sonarqubecloud[bot]

@MajorLift Is this PR still relevant? It looks like maybe, but it has also been open for a while I'm noticing, so I'm not sure if it needs to be updated or something.

mcmire avatar Oct 23 '24 18:10 mcmire

@MajorLift Is this PR still relevant? It looks like maybe, but it has also been open for a while I'm noticing, so I'm not sure if it needs to be updated or something.

Yes still relevant I am taking over this work

cryptodev-2s avatar Oct 23 '24 19:10 cryptodev-2s

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: fe66a38974b063587655980c3013bae3ac35548d Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/ebfbbc58-1f83-4239-8a71-224f9a3645bc

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Oct 23 '24 19:10 github-actions[bot]

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 49f4ae09341a4d52f9b0e854b7fcea8b5af1bd63 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/73593585-1c9f-4513-a048-8acd6ae4284e

[!NOTE]

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Nov 14 '24 18:11 github-actions[bot]

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: 24d9936b24f4e805a25bb3fde81f8283af053ca1 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f0a01847-8e29-401d-97fe-52a8bd283ba7

[!NOTE]

  • This comment will auto-update when build completes
  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar Nov 14 '24 18:11 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: eaf1ee285f0f17c41b48402bfda5cabb3fe62a38 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/005c893f-9abe-40dd-9801-d057e62e8b0a

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Nov 14 '24 18:11 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 3af972a8641c971399e2eb562a3ff0cf4f388c2d Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/05ec59d2-e2ac-4fec-84d1-0dec45ff184b

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Nov 19 '24 14:11 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 74fd0c55aceebadfffc77e1396c1db23b9281f41 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/0bcb4638-fcc4-48c6-bdac-79f742cd7dfd

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Nov 19 '24 15:11 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 9eab5bee454a99d414862e883e8cc3e209a1dc21 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/c44479ac-1a0f-414a-9f4a-f8a001069360

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Nov 19 '24 19:11 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 65d978c17f87cf65366d09c95e4681c2b93cfa0a Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/fcccbcca-7b0d-4654-8257-5104f73d57ee

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Nov 20 '24 17:11 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 3c014403ad5d82345277622cbafdf8d37acbaf77 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/eed2ea2a-7910-4dc7-adf0-bd1b4f307f2c

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Nov 20 '24 18:11 github-actions[bot]

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: e4da24ee724aecf89de820c58695aeafb8ce7016 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/27062cfc-54c6-4467-91a4-f1e046f2a979

[!NOTE]

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

[!TIP]

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions[bot] avatar Nov 20 '24 18:11 github-actions[bot]