chore(deps): force secp256k1 4.0.4 version with resolutions #28177

ccharly · 2024-10-30T10:05:17Z

Description

Forcing resolutions to fix yarn audit warnings and more specifically this issue:

GHSA-584q-6j8j-r5pm

I decided to be very explicit about the resolution itself based on the output of:

$ yarn why secp256k1
├─ ethereum-cryptography@npm:0.1.3
│  └─ secp256k1@npm:4.0.4 (via npm:^4.0.1)
│
├─ ganache@npm:7.9.2
│  └─ secp256k1@npm:4.0.3 (via npm:4.0.3)
│
├─ ganache@patch:ganache@npm%3A7.9.2#~/.yarn/patches/ganache-npm-7.9.2-a70dc8da34.patch::version=7.9.2&hash=7d7c66
│  └─ secp256k1@npm:4.0.3 (via npm:4.0.3)
│
├─ gridplus-sdk@npm:2.5.1
│  └─ secp256k1@npm:4.0.2 (via npm:4.0.2)
│
└─ hdkey@npm:2.1.0
   └─ secp256k1@npm:4.0.4 (via npm:^4.0.0)

We could also have a more straightforward resolution like:

  ...
  "resolutions": {
    ...
    "secp256k1": "4.0.4"
  }
  ...

But that could also catch version with different major.

Let me know what would be the preferred solution here.

Related issues

Fixes: GHSA-584q-6j8j-r5pm

Manual testing steps

N/A

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
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 format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-10-30T10:05:31Z

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.

socket-security · 2024-10-30T10:05:33Z

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

🚮 Removed packages: npm/[email protected]

View full report↗︎

metamaskbot · 2024-10-30T11:08:04Z

Builds ready [8a67e17]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2274 ± 218 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	374	2618	1730	748	359
		domContentLoaded	1781	3635	2239	456	219
		load	1791	3673	2274	455	218
		domInteractive	24	203	64	42	20
		backgroundConnect	8	76	39	22	10
		firstReactRender	74	300	124	50	24
		getState	6	88	30	23	11
		initialActions	0	1	0	0	0
		loadScripts	1259	2848	1679	395	190
		setupStore	12	99	35	26	12
		uiStartup	2008	4084	2570	529	254

Bundle size diffs [🚀 Bundle size reduced!]

background: -88.63 KiB (-1.78%)
ui: 1.06 KiB (0.01%)
common: 740 Bytes (0.01%)

metamaskbot · 2024-10-30T12:53:09Z

Builds ready [2674d2d]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2493 ± 363 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	360	3909	2219	815	391
		domContentLoaded	1777	4522	2454	757	364
		load	1827	4543	2493	757	363
		domInteractive	27	259	79	54	26
		backgroundConnect	9	154	43	37	18
		firstReactRender	52	302	130	58	28
		getState	6	66	25	22	10
		initialActions	0	1	0	0	0
		loadScripts	1293	3497	1845	631	303
		setupStore	12	100	43	26	13
		uiStartup	1987	5148	2833	838	402

Bundle size diffs [🚀 Bundle size reduced!]

background: -88.63 KiB (-1.78%)
ui: 1.06 KiB (0.01%)
common: 740 Bytes (0.01%)

metamaskbot · 2024-10-30T14:18:54Z

Builds ready [13c075a]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1908 ± 173 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1584	3107	1902	326	157
		domContentLoaded	1574	2646	1848	238	114
		load	1583	3287	1908	360	173
		domInteractive	18	241	50	46	22
		backgroundConnect	9	556	56	120	58
		firstReactRender	45	195	100	36	17
		getState	5	368	37	79	38
		initialActions	0	0	0	0	0
		loadScripts	1131	1720	1344	153	74
		setupStore	10	121	41	33	16
		uiStartup	1794	4487	2189	573	275

Bundle size diffs [🚀 Bundle size reduced!]

background: -88.63 KiB (-1.78%)
ui: 1.06 KiB (0.01%)
common: 740 Bytes (0.01%)

naugtur · 2024-10-30T15:05:48Z

lavamoat/browserify/main/policy.json

@@ -4273,22 +4273,6 @@
        "TextEncoder": true
      }
    },
-    "eth-lattice-keyring>gridplus-sdk>secp256k1": {


this change indicates there's now one less version of the package.
This adds up with what we see in the lockfile too.
I can confirm you've successfully upgraded secp256k1 to 4.0.4 only.

naugtur · 2024-10-30T15:06:17Z

package.json

+    "secp256k1@npm:^4.0.0": "4.0.4",
+    "secp256k1@npm:^4.0.1": "4.0.4",
+    "secp256k1@npm:4.0.2": "4.0.4",
+    "secp256k1@npm:4.0.3": "4.0.4"


It's a bit messy, but if it's not ending up in main branch later, it doesn't matter

The base branch was changed.

Gudahtt

LGTM!

Cherry-picks #28145 to v12.5.1, and brings #28177 into v12.5.1 as well. The latter originally targeted v12.6.0, but we need it for a hotfix ahead of that. The PR description for that was: ## **Description** Forcing resolutions to fix `yarn audit` warnings and more specifically this issue: - GHSA-584q-6j8j-r5pm I decided to be very explicit about the resolution itself based on the output of: ```console $ yarn why secp256k1 ├─ ethereum-cryptography@npm:0.1.3 │ └─ secp256k1@npm:4.0.4 (via npm:^4.0.1) │ ├─ ganache@npm:7.9.2 │ └─ secp256k1@npm:4.0.3 (via npm:4.0.3) │ ├─ ganache@patch:ganache@npm%3A7.9.2#~/.yarn/patches/ganache-npm-7.9.2-a70dc8da34.patch::version=7.9.2&hash=7d7c66 │ └─ secp256k1@npm:4.0.3 (via npm:4.0.3) │ ├─ gridplus-sdk@npm:2.5.1 │ └─ secp256k1@npm:4.0.2 (via npm:4.0.2) │ └─ hdkey@npm:2.1.0 └─ secp256k1@npm:4.0.4 (via npm:^4.0.0) ``` We could also have a more straightforward resolution like: ```json ... "resolutions": { ... "secp256k1": "4.0.4" } ... ``` But that could also catch version with different major. Let me know what would be the preferred solution here. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28176?quickstart=1) ## **Related issues** Fixes: GHSA-584q-6j8j-r5pm ## **Manual testing steps** N/A ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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-extension/blob/develop/.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: Charly Chevalier <[email protected]> Co-authored-by: sahar-fehri <[email protected]>

metamaskbot · 2024-10-31T20:32:23Z

Builds ready [3df0a16]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1871 ± 118 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1567	2507	1863	221	106
		domContentLoaded	1552	2281	1819	178	86
		load	1561	2661	1871	246	118
		domInteractive	18	112	49	26	12
		backgroundConnect	9	376	53	89	43
		firstReactRender	47	251	106	52	25
		getState	4	195	26	49	23
		initialActions	0	52	3	11	5
		loadScripts	1135	1832	1343	143	69
		setupStore	10	239	41	53	25
		uiStartup	1728	4364	2185	556	267

Bundle size diffs [🚀 Bundle size reduced!]

background: -168.37 KiB (-3.37%)
ui: 4.01 KiB (0.05%)
common: 107.58 KiB (1.36%)

ccharly requested review from a team as code owners October 30, 2024 10:05

github-actions bot added the team-accounts label Oct 30, 2024

ccharly mentioned this pull request Oct 30, 2024

chore(deps): force secp256k1 4.0.4 version with resolutions #28176

Closed

7 tasks

ccharly force-pushed the fix/Version-v12.6.0-secp256k1-temporary-resolutions branch from 8a67e17 to 2674d2d Compare October 30, 2024 12:12

danjm previously approved these changes Oct 30, 2024

View reviewed changes

naugtur reviewed Oct 30, 2024

View reviewed changes

danjm changed the base branch from Version-v12.6.0 to Version-v12.5.1 October 30, 2024 15:14

danjm requested review from a team and kumavis as code owners October 30, 2024 15:14

danjm changed the base branch from Version-v12.5.1 to Version-v12.6.0 October 30, 2024 15:15

danjm mentioned this pull request Oct 30, 2024

fix (cherry-pick): Fix audit failures v12.5.1 #28187

Merged

7 tasks

Gudahtt approved these changes Oct 30, 2024

View reviewed changes

Gudahtt closed this Oct 31, 2024

Gudahtt force-pushed the fix/Version-v12.6.0-secp256k1-temporary-resolutions branch from 13c075a to 3df0a16 Compare October 31, 2024 19:57

ccharly merged commit 3df0a16 into Version-v12.6.0 Oct 31, 2024
77 of 79 checks passed

ccharly deleted the fix/Version-v12.6.0-secp256k1-temporary-resolutions branch October 31, 2024 19:57

github-actions bot locked and limited conversation to collaborators Oct 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(deps): force secp256k1 4.0.4 version with resolutions #28177

chore(deps): force secp256k1 4.0.4 version with resolutions #28177

ccharly commented Oct 30, 2024

github-actions bot commented Oct 30, 2024

socket-security bot commented Oct 30, 2024 •

edited

Loading

metamaskbot commented Oct 30, 2024

metamaskbot commented Oct 30, 2024

metamaskbot commented Oct 30, 2024

naugtur Oct 30, 2024

naugtur Oct 30, 2024

Gudahtt left a comment

metamaskbot commented Oct 31, 2024

chore(deps): force secp256k1 4.0.4 version with resolutions #28177

chore(deps): force secp256k1 4.0.4 version with resolutions #28177

Conversation

ccharly commented Oct 30, 2024

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Oct 30, 2024

socket-security bot commented Oct 30, 2024 • edited Loading

metamaskbot commented Oct 30, 2024

metamaskbot commented Oct 30, 2024

metamaskbot commented Oct 30, 2024

naugtur Oct 30, 2024

Choose a reason for hiding this comment

naugtur Oct 30, 2024

Choose a reason for hiding this comment

Gudahtt left a comment

Choose a reason for hiding this comment

metamaskbot commented Oct 31, 2024

socket-security bot commented Oct 30, 2024 •

edited

Loading