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

leotm · 2023-12-07T15:33:10Z

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 regenerator-runtime@0.11.1 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/assets-controllers@7.0.0
│ └─┬ babel-runtime@6.26.0
│   └── regenerator-runtime@0.11.1
...

full regenerator-runtime dep tree

├─┬ @babel/runtime@7.23.2
│ └── regenerator-runtime@0.14.0
├─┬ @metamask/assets-controllers@7.0.0
│ └─┬ babel-runtime@6.26.0
│   └── regenerator-runtime@0.11.1
├─┬ @storybook/addon-knobs@5.3.21
│ └─┬ @storybook/api@5.3.21
│   └── regenerator-runtime@0.13.9 deduped
├─┬ @storybook/addon-ondevice-actions@6.5.6
│ └─┬ @storybook/addons@6.5.16
│   ├─┬ @storybook/api@6.5.16
│   │ └── regenerator-runtime@0.13.11 deduped
│   ├─┬ @storybook/router@6.5.16
│   │ └── regenerator-runtime@0.13.11
│   ├─┬ @storybook/theming@6.5.16
│   │ └── regenerator-runtime@0.13.11 deduped
│   └── regenerator-runtime@0.13.11
├─┬ @storybook/react-native@6.5.6
│ ├─┬ @storybook/client-api@6.5.16
│ │ ├─┬ @storybook/store@6.5.16
│ │ │ └── regenerator-runtime@0.13.11
│ │ └── regenerator-runtime@0.13.11
│ ├─┬ @storybook/core-client@6.5.16
│ │ ├─┬ @storybook/ui@6.5.16
│ │ │ ├─┬ @storybook/api@6.5.16
│ │ │ │ └── regenerator-runtime@0.13.11 deduped
│ │ │ ├─┬ @storybook/components@6.5.16
│ │ │ │ └── regenerator-runtime@0.13.11 deduped
│ │ │ ├─┬ @storybook/theming@6.5.16
│ │ │ │ └── regenerator-runtime@0.13.11 deduped
│ │ │ └── regenerator-runtime@0.13.11
│ │ └── regenerator-runtime@0.13.11
│ └─┬ @storybook/preview-web@6.5.16
│   └── regenerator-runtime@0.13.11
├─┬ appium@1.22.3
│ ├─┬ appium-android-driver@4.54.0
│ │ └─┬ jimp@0.16.2
│ │   ├─┬ @jimp/custom@0.16.2
│ │   │ └─┬ @jimp/core@0.16.2
│ │   │   └─┬ @jimp/utils@0.16.2
│ │   │     └── regenerator-runtime@0.13.11
│ │   └── regenerator-runtime@0.13.11
│ └─┬ appium-tizen-driver@1.1.1-beta.5
│   └─┬ jimp@0.5.6
│     └─┬ @babel/polyfill@7.12.1
│       └── regenerator-runtime@0.13.11
├─┬ react-native@0.71.14 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
│ └── regenerator-runtime@0.13.11
└── regenerator-runtime@0.13.9

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

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'); // ses@0.18.8

I've not managed to repro locally this way yet

Screenshots/Recordings

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.

Refactor gen func proto iterator symbol obj prop assignment to Object.defineProperty

github-actions · 2023-12-07T15:33:23Z

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.

patches/babel-runtime++regenerator-runtime+0.11.1.patch

github-actions · 2023-12-07T16:36:11Z

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 · 2023-12-07T16:39:36Z

converted back to draft until failing/cancelled unit tests resolved

github-actions · 2023-12-08T13:18:50Z

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 · 2023-12-08T13:18:57Z

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 · 2023-12-08T14:04:17Z

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 · 2023-12-09T14:14:41Z

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.

This reverts commit c5eba9d.

- Since was disabled as crash hotfix - CI unit test failures were due to Node v18.19 not SES - All regenerator-runtime vers now SES-compatible (v0.13.8+)

sonarcloud · 2023-12-13T22:46:57Z

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 · 2024-01-18T15:32:32Z

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 · 2024-01-18T16:27:08Z

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 · 2024-01-19T14:01:36Z

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

all regression tests also passing locally

patches/react-native+0.71.15.patch

Cal-L · 2024-01-19T22:44:44Z

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 · 2024-01-19T22:47:17Z

@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

ses.cjs

leotm · 2024-01-22T12:04:28Z

@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/core-monorepo@109.0.0
└─┬ @metamask/transaction-controller@20.0.0 -> ./packages/transaction-controller
  └─┬ babel-runtime@6.26.0
    └── regenerator-runtime@0.11.1

since @metamask/assets-controllers no longer relies on regenerator-runtime
removed in @metamask/core-monorepo@67.0.0 (MetaMask/core#1504), which is@metamask/assets-controllers@11.0.0
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?

And remove stale babel-runtime/regenerator-runtime resolution

socket-security · 2024-01-22T12:30:39Z

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

View full report↗︎

This reverts commit 38c0a31.

Gudahtt · 2024-01-22T13:29:30Z

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 · 2024-01-22T15:23:32Z

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/core-monorepo@109.0.0
└─┬ @metamask/transaction-controller@20.0.0 -> ./packages/transaction-controller
  ├── babel-runtime@6.26.0
  └─┬ eth-method-registry@3.0.0
    └─┬ @metamask/ethjs@0.5.1
      ├─┬ @metamask/ethjs-contract@0.3.4
      │ └── babel-runtime@6.26.0 deduped
      └─┬ @metamask/ethjs-query@0.5.3
        └── babel-runtime@6.26.0 deduped

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

Cal-L

LGTM

sonarcloud · 2024-01-23T01:16:46Z

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

Patch regenerator-runtime

c5eba9d

Refactor gen func proto iterator symbol obj prop assignment to Object.defineProperty

leotm added the team-lavamoat label Dec 7, 2023

leotm commented Dec 7, 2023

View reviewed changes

patches/babel-runtime++regenerator-runtime+0.11.1.patch Outdated Show resolved Hide resolved

leotm requested a review from Gudahtt December 7, 2023 16:34

leotm marked this pull request as ready for review December 7, 2023 16:34

leotm requested a review from a team as a code owner December 7, 2023 16:34

github-actions bot added the Run Smoke E2E label Dec 7, 2023

leotm marked this pull request as draft December 7, 2023 16:39

leotm mentioned this pull request Dec 7, 2023

fix: temp disable SES #8045

Closed

13 tasks

leotm added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Dec 8, 2023

leotm marked this pull request as ready for review December 8, 2023 13:17

Merge branch 'main' into fix/ses-regenerator-runtime

2fa0fc4

leotm added 4 commits December 11, 2023 17:41

Revert "Patch regenerator-runtime"

f3b5d76

This reverts commit c5eba9d.

Resolve babel-runtime/regenerator-runtime to ^0.13.8

24f6fb9

Merge branch 'main' into fix/ses-regenerator-runtime

26a150f

Re-enable SES

efdfb56

- Since was disabled as crash hotfix - CI unit test failures were due to Node v18.19 not SES - All regenerator-runtime vers now SES-compatible (v0.13.8+)

leotm changed the title ~~fix: regenerator-runtime~~ fix: regenerator-runtime and reenable SES on iOS (JSC) Dec 13, 2023

leotm mentioned this pull request Dec 20, 2023

☂️ React Native Compatibility Tracker 2.0 LavaMoat/docs#12

Open

76 tasks

gauthierpetetin added the team-mobile-client label Dec 20, 2023

Merge branch 'main' into fix/ses-regenerator-runtime

846f3b0

leotm removed the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Jan 17, 2024

Dedupe deps

6d43547

Remove stale resolution from conflict res

3b16601

leotm changed the title ~~fix: regenerator-runtime and reenable SES on iOS (JSC)~~ fix: regenerator-runtime and reenable SES (v1.1.0) on iOS (JSC) Jan 18, 2024

Merge branch 'main' into fix/ses-regenerator-runtime

4704ff4

leotm added Run E2E Smoke and removed Run E2E Smoke labels Jan 19, 2024

leotm commented Jan 19, 2024

View reviewed changes

patches/react-native+0.71.15.patch Show resolved Hide resolved

Cal-L reviewed Jan 19, 2024

View reviewed changes

ses.cjs Show resolved Hide resolved

Bump @metamask/assets-controllers from v7 to v11

38c0a31

And remove stale babel-runtime/regenerator-runtime resolution

Revert "Bump @metamask/assets-controllers from v7 to v11"

694e25d

This reverts commit 38c0a31.

leotm requested a review from Cal-L January 22, 2024 13:23

leotm mentioned this pull request Jan 22, 2024

Add @metamask/transaction-controller@20.0.0 resolution for SES MetaMask/core#3809

Closed

3 tasks

Merge branch 'main' into fix/ses-regenerator-runtime

b689c5c

Cal-L approved these changes Jan 23, 2024

View reviewed changes

leotm merged commit 6d26e45 into main Jan 23, 2024
27 of 28 checks passed

leotm deleted the fix/ses-regenerator-runtime branch January 23, 2024 13:06

github-actions bot locked and limited conversation to collaborators Jan 23, 2024

metamaskbot added the release-7.16.0 Issue or pull request that will be included in release 7.16.0 label Jan 23, 2024

gauthierpetetin added the team-mobile-platform label Feb 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

leotm commented Dec 7, 2023 •

edited

Loading

github-actions bot commented Dec 7, 2023

github-actions bot commented Dec 7, 2023

leotm commented Dec 7, 2023 •

edited

Loading

github-actions bot commented Dec 8, 2023

leotm commented Dec 8, 2023 •

edited

Loading

leotm commented Dec 8, 2023 •

edited

Loading

codecov-commenter commented Dec 9, 2023 •

edited

Loading

sonarcloud bot commented Dec 13, 2023

github-actions bot commented Jan 18, 2024

leotm commented Jan 18, 2024 •

edited

Loading

leotm commented Jan 19, 2024 •

edited

Loading

Cal-L commented Jan 19, 2024

Cal-L commented Jan 19, 2024

Cal-L left a comment

leotm commented Jan 22, 2024 •

edited

Loading

socket-security bot commented Jan 22, 2024 •

edited

Loading

Gudahtt commented Jan 22, 2024 •

edited

Loading

leotm commented Jan 22, 2024 •

edited

Loading

Cal-L left a comment

sonarcloud bot commented Jan 23, 2024

	+if (Platform.OS === 'ios' && !global?.HermesInternal && !__DEV__) {
	+ require('../../../../ses.cjs'); // ses@0.18.8

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

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

Conversation

leotm commented Dec 7, 2023 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Dec 7, 2023

github-actions bot commented Dec 7, 2023

leotm commented Dec 7, 2023 • edited Loading

github-actions bot commented Dec 8, 2023

leotm commented Dec 8, 2023 • edited Loading

leotm commented Dec 8, 2023 • edited Loading

codecov-commenter commented Dec 9, 2023 • edited Loading

Codecov Report

sonarcloud bot commented Dec 13, 2023

Quality Gate passed

github-actions bot commented Jan 18, 2024

leotm commented Jan 18, 2024 • edited Loading

leotm commented Jan 19, 2024 • edited Loading

Cal-L commented Jan 19, 2024

Cal-L commented Jan 19, 2024

Cal-L left a comment

Choose a reason for hiding this comment

leotm commented Jan 22, 2024 • edited Loading

socket-security bot commented Jan 22, 2024 • edited Loading

Gudahtt commented Jan 22, 2024 • edited Loading

leotm commented Jan 22, 2024 • edited Loading

Cal-L left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jan 23, 2024

Quality Gate passed

leotm commented Dec 7, 2023 •

edited

Loading

leotm commented Dec 7, 2023 •

edited

Loading

leotm commented Dec 8, 2023 •

edited

Loading

leotm commented Dec 8, 2023 •

edited

Loading

codecov-commenter commented Dec 9, 2023 •

edited

Loading

leotm commented Jan 18, 2024 •

edited

Loading

leotm commented Jan 19, 2024 •

edited

Loading

leotm commented Jan 22, 2024 •

edited

Loading

socket-security bot commented Jan 22, 2024 •

edited

Loading

Gudahtt commented Jan 22, 2024 •

edited

Loading

leotm commented Jan 22, 2024 •

edited

Loading