fix: regenerator-runtime and reenable SES (v1.1.0) on iOS (JSC)

[leotm]

Description

Fix recent spike in iOS crashes (sending ETH on Optimism)
and reenable SES on iOS (we disabled it temporarily for our v7.12.2 hotfix)
nb: flakey CI earlier was Node 18.19, not SES

Make [email protected] compatible with SES via a patch (updated since to a resolution)

• refactor generator function prototype iterator symbol
• update object property assignment to Object.defineProperty

...
├─┬ @metamask/[email protected]
│ └─┬ [email protected]
│   └── [email protected]
...

full regenerator-runtime dep tree

├─┬ @babel/[email protected]
│ └── [email protected]
├─┬ @metamask/[email protected]
│ └─┬ [email protected]
│   └── [email protected]
├─┬ @storybook/[email protected]
│ └─┬ @storybook/[email protected]
│   └── [email protected] deduped
├─┬ @storybook/[email protected]
│ └─┬ @storybook/[email protected]
│   ├─┬ @storybook/[email protected]
│   │ └── [email protected] deduped
│   ├─┬ @storybook/[email protected]
│   │ └── [email protected]
│   ├─┬ @storybook/[email protected]
│   │ └── [email protected] deduped
│   └── [email protected]
├─┬ @storybook/[email protected]
│ ├─┬ @storybook/[email protected]
│ │ ├─┬ @storybook/[email protected]
│ │ │ └── [email protected]
│ │ └── [email protected]
│ ├─┬ @storybook/[email protected]
│ │ ├─┬ @storybook/[email protected]
│ │ │ ├─┬ @storybook/[email protected]
│ │ │ │ └── [email protected] deduped
│ │ │ ├─┬ @storybook/[email protected]
│ │ │ │ └── [email protected] deduped
│ │ │ ├─┬ @storybook/[email protected]
│ │ │ │ └── [email protected] deduped
│ │ │ └── [email protected]
│ │ └── [email protected]
│ └─┬ @storybook/[email protected]
│   └── [email protected]
├─┬ [email protected]
│ ├─┬ [email protected]
│ │ └─┬ [email protected]
│ │   ├─┬ @jimp/[email protected]
│ │   │ └─┬ @jimp/[email protected]
│ │   │   └─┬ @jimp/[email protected]
│ │   │     └── [email protected]
│ │   └── [email protected]
│ └─┬ [email protected]
│   └─┬ [email protected]
│     └─┬ @babel/[email protected]
│       └── [email protected]
├─┬ [email protected] invalid: "^0.0.0-0 || 0.60 - 0.70 || 1000.0.0" from node_modules/@react-native-async-storage/async-storage, "^0.56.0" from node_modules/react-native-jazzicon, "^0.62.2" from node_modules/react-native-v8
│ └── [email protected]
└── [email protected]

Considerations

• further patch runtime.js other relevant parts
• or resolve @metamask/assets-controllers>babel-runtime>regenerator-runtime
  • from v0.11.1 to v0.13.8+ (containing fix)
• or resolve @metamask/assets-controllers>babel-runtime
  • from v6.26.0 to v7.8.4+ (regenerator-runtime: ^0.13.2, which locks to 0.13.11)
• ethjs patches still needed?

Remaining regenerator-runtime transitive deps are safe above v0.13.8 (see dep tree above)

Related issues

Fixes: #8015

• https://metamask.sentry.io/issues/4699573603/?project=2299799 sev1
  • app/lib/ens-ipfs/resolver.js26:42 in resolveEnsToIpfsContentId
    • const Registry = contract(registryAbi).at(registryAddress); an ethjs/ethjs-contract
  • Suspect Commit: b677fd9 update to resolveEnsToIpfsContentId
• https://metamask.sentry.io/issues/4701946024/?project=2299799

Fixes: #8017

• https://metamask.sentry.io/issues/4702324986/?project=2299799

Additional context

• Update SES lockdown metamask-extension#10663
• Cannot assign to read-only property (Property override mistake) endojs/endo#1855

Manual testing steps

Steps in Sentry issue breadcrumbs (iOS prod)

To enable SES in debug-mode, omit !__DEV__

metamask-mobile/patches/react-native+0.71.14.patch

Lines 11 to 12 in 46c1cea

	+if (Platform.OS === 'ios' && !global?.HermesInternal && !__DEV__) {
	+ require('../../../../ses.cjs'); // [email protected]

I've not managed to repro locally this way yet

Screenshots/Recordings

Before

After

Pre-merge author checklist

• I’ve followed MetaMask Coding Standards.
• I've clearly explained what problem this PR is solving and how it is solved.
• I've linked related issues
• I've included manual testing steps
• I've included screenshots/recordings if applicable
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
• I’ve properly set the pull request status:
  • In case it's not yet "ready for review", I've set it to "draft".
  • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

[github-actions]

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]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/27e3bafe-0f8a-4644-8d6e-42bd5b1d8333
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

[leotm]

converted back to draft until failing/cancelled unit tests resolved

[github-actions]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/fae1b7fb-caf2-4b2d-bffa-c491fe9b4f38
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

[leotm]

pr re-readied up, since CI flakey unit tests unrelated to this pr

issue appears fixed, based on Sentry logs, solution and our prev solution in mm-ext

[leotm]

once

• fix: CI flakey unit tests TypeErrors on react-native/jest/setup.js global.performance #8046

is merged, we can update this branch, and unit tests should be passing again

and PR is ready for QA

we shouldn't need to disable SES

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7e66fb0) 40.33% compared to head (b689c5c) 40.33%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8033   +/-   ##
=======================================
  Coverage   40.33%   40.33%           
=======================================
  Files        1235     1235           
  Lines       29949    29949           
  Branches     2875     2875           
=======================================
  Hits        12079    12079           
  Misses      17175    17175           
  Partials      695      695           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[github-actions]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a273882e-3ca2-4d7d-a332-9df00fe1e0c6
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

[leotm]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a273882e-3ca2-4d7d-a332-9df00fe1e0c6 You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

build_smoke_e2e_ios_android_stage ✅
run_smoke_e2e_ios_android_stage ❌
iOS Failed to find Detox executable at path: /Users/vagrant/git/node_modules/.bin/detox
Android Failed to find Detox executable at path: /bitrise/src/node_modules/.bin/detox

local testing

watchman shutdown-server && watchman watch-del-all && git clean -fdx && nvm use && export METAMASK_BUILD_TYPE="main" && export METAMASK_ENVIRONMENT="local" && yarn setup

nvm use && export METAMASK_BUILD_TYPE='main' && export METAMASK_ENVIRONMENT='local' && IGNORE_BOXLOGS_DEVELOPMENT="true" FORCE_BUNDLING=true yarn test:e2e:ios:bitrise:build ✅

nvm use && export METAMASK_BUILD_TYPE='main' && export METAMASK_ENVIRONMENT='local' && IGNORE_BOXLOGS_DEVELOPMENT="true" FORCE_BUNDLING=true yarn test:e2e:ios:bitrise:run --testNamePattern="Smoke" ✅

all smoke tests passing locally, update branch and re-run BitRise Smoke tests (Rebuild unsuccessful Workflows with remote access unsuccessful)

[leotm]

https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d36076eb-cdf7-4179-9d94-15ac845bc5a5 ✅

all regression tests also passing locally

[Cal-L]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a273882e-3ca2-4d7d-a332-9df00fe1e0c6 You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

build_smoke_e2e_ios_android_stage ✅ run_smoke_e2e_ios_android_stage ❌ iOS Failed to find Detox executable at path: /Users/vagrant/git/node_modules/.bin/detox Android Failed to find Detox executable at path: /bitrise/src/node_modules/.bin/detox

local testing

watchman shutdown-server && watchman watch-del-all && git clean -fdx && nvm use && export METAMASK_BUILD_TYPE="main" && export METAMASK_ENVIRONMENT="local" && yarn setup

nvm use && export METAMASK_BUILD_TYPE='main' && export METAMASK_ENVIRONMENT='local' && IGNORE_BOXLOGS_DEVELOPMENT="true" FORCE_BUNDLING=true yarn test:e2e:ios:bitrise:build ✅

nvm use && export METAMASK_BUILD_TYPE='main' && export METAMASK_ENVIRONMENT='local' && IGNORE_BOXLOGS_DEVELOPMENT="true" FORCE_BUNDLING=true yarn test:e2e:ios:bitrise:run --testNamePattern="Smoke" ✅

all smoke tests passing locally, update branch and re-run BitRise Smoke tests (`Rebuild unsuccessful Workflows with remote access` unsuccessful)

I believe there were issues with our E2E tests on Bitrise. @cortisiko @chrisleewilcox could you confirm?

[Cal-L]

@leotm Are you aware if the latest core controllers already resolve this issue? If not, could you follow up with a PR to core that performs one of the following?

• resolve @metamask/assets-controllers>babel-runtime>regenerator-runtime
  from v0.11.1 to v0.13.8+ (containing fix)
• resolve @metamask/assets-controllers>babel-runtime
  from v6.26.0 to v7.8.4+ (regenerator-runtime: ^0.13.2, which locks to 0.13.11)

[Cal-L]

left a comment and some questions

[leotm]

@leotm Are you aware if the latest core controllers already resolve this issue? If not, could you follow up with a PR to core that performs one of the following?

• resolve @metamask/assets-controllers>babel-runtime>regenerator-runtime
  from v0.11.1 to v0.13.8+ (containing fix)
• resolve @metamask/assets-controllers>babel-runtime
  from v6.26.0 to v7.8.4+ (regenerator-runtime: ^0.13.2, which locks to 0.13.11)

good question after checking core(main) appears fixed now

@metamask/[email protected]
└─┬ @metamask/[email protected] -> ./packages/transaction-controller
  └─┬ [email protected]
    └── [email protected]

since @metamask/assets-controllers no longer relies on regenerator-runtime
removed in @metamask/[email protected] (MetaMask/core#1504), which is@metamask/[email protected]
so upgrading our @metamask/assets-controllers from v7 to v11-24 is SES-compatible, running local tests on those
(noting we also have patches/@metamask+assets-controllers+7.0.0.patch)

edit: tried bumping @metamask/assets-controllers from v7 to v11

@tommasini @Gudahtt what would be the best way to upgrade our patch? (looking thru the 2.4k lines)

edit: after brief call with @tommasini doing our controller upgrades, our resolution seems best solution for now cc @Cal-L

---

also our final @metamask/core-monorepo SES lockdown incompatibility now lurks in @metamask/transaction-controller
so following up there with the resolution, unless we can do similar refeactor to remove babel-runtime deps?

[socket-security]

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

View full report↗︎

[Gudahtt]

I don't think updating that package would help; babel-runtime is the dependency of dozens of other packages as well, including some core packages.

[leotm]

also our final SES lockdown incompatibility now lurks in @metamask/transaction-controller
so following up there with the resolution, unless we can do similar refeactor to remove babel-runtime deps?

I don't think updating that package would help; babel-runtime is the dependency of dozens of other packages as well, including some core packages.

yeah i see what you mean

@metamask/[email protected]
└─┬ @metamask/[email protected] -> ./packages/transaction-controller
  ├── [email protected]
  └─┬ [email protected]
    └─┬ @metamask/[email protected]
      ├─┬ @metamask/[email protected]
      │ └── [email protected] deduped
      └─┬ @metamask/[email protected]
        └── [email protected] deduped

ok leaving it for now, unless a resolution like MetaMask/core#3809 would make sense over there too

[Cal-L]

LGTM

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud