fix(deps): Bump `@metamask/eth-json-rpc-middleware` to `^14.0.0`, `@metamask/transaction-controller` to `^35.1.1` #26143

MajorLift · 2024-07-25T20:38:52Z

Description

Updates @metamask/eth-json-rpc-middleware from ^12.1.1 to ^14.0.0.

This version bump comes with a large number of regressions, most of them type errors.
This is because the package's dependencies are also updated by multiple major versions, and the changes include improved, stricter types (especially in @metamask/utils).

Related issues

Changelog

Added

Add and export PPOMMiddlewareRequest type for JsonRpcRequest types that include the securityAlertResponse property.
- securityAlertResponse is defined as both optional and nullable.
Add PPOMRequest type for eth-sendTransaction requests.

Changed

BREAKING: Bump @metamask/eth-json-rpc-middleware from ^12.1.1 to ^14.0.0.
BREAKING: Bump @metamask/transaction-controller from ^34.0.0 to ^35.1.1.
BREAKING: Redefine SecurityAlertsAPIRequest as a JsonRpcRequest type that accepts unknown[] as its params type.
Widen the request parameters of the functions validateWithController and validateWithAPI to include SecurityAlertsAPIRequest.
Bump @trezor/connect-web from 9.2.2 to 9.3.0.

Fixed

BREAKING: Narrow Params generic parameter of createPPOMMiddleware function from JsonRpcParams to (string | { to: string })[].
Add Params generic parameter to handleSnapRequest function, which defaults to JsonRpcParams.
- handleSnapRequest can now be typed correctly with any params object.

Security

BREAKING: Typed signature validation only replaces 0X prefix with 0x, and contract address normalization is removed for decimal and octal values.
- Threat actors have been manipulating eth_signTypedData_v4 fields to cause failures in blockaid's detectors.
- Extension crashes with an error when performing Malicious permit with a non-0x prefixed integer address.
- This fixes an issue where the key value row or petname component disappears if a signed address is prefixed by "0X" instead of "0x".

Manual testing steps

Screenshots/Recordings

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-07-25T20:39:07Z

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.

legobeat · 2024-07-25T23:53:21Z

As this is updating transitive @metamask/eth-block-tracker from v9 to v11, I guess it makes sense to look at updating to that in general?

Related (blocking? not sure):

MajorLift · 2024-08-12T19:53:39Z

@SocketSecurity ignore npm/@storybook/addon-actions@7.6.20, npm/@storybook/addon-docs@7.6.20, npm/@storybook/blocks@7.6.20, npm/@storybook/channels@7.6.20, npm/@storybook/client-logger@7.6.20, npm/@storybook/components@7.6.20, npm/@storybook/core-client@7.6.20, npm/@storybook/core-common@7.6.20, npm/@storybook/core-events@7.6.20, npm/@storybook/csf-plugin@7.6.20, npm/@storybook/csf-tools@7.6.20, npm/@storybook/docs-tools@7.6.20, npm/@storybook/manager-api@7.6.20, npm/@storybook/node-logger@7.6.20, npm/@storybook/postinstall@7.6.20, npm/@storybook/preview-api@7.6.20, npm/@storybook/react-dom-shim@7.6.20, npm/@storybook/react@7.6.20, npm/@storybook/router@7.6.20, npm/@storybook/theming@7.6.20, npm/@storybook/types@7.6.20

@storybook

MajorLift · 2024-08-12T19:54:17Z

@SocketSecurity ignore npm/@metamask/eth-block-tracker@11.0.1, npm/lavamoat-core@15.4.0, npm/lavamoat-tofu@7.3.0

Internal packages

MajorLift · 2024-08-13T16:33:59Z

@SocketSecurity ignore npm/diff@5.2.0
benign author change

metamaskbot · 2024-08-20T13:33:56Z

Builds ready [3c40934]

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
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (86 ± 20 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	63	269	111	43	21
		domContentLoaded	41	240	82	43	21
		load	48	241	86	41	20
		domInteractive	10	147	34	31	15

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 192.1 KiB (5.77%)
ui: 120 Bytes (0.00%)
common: 207.83 KiB (2.64%)

MajorLift · 2024-08-20T13:37:02Z

@legobeat I bumped @metamask/transaction-controller to latest (3c40934), but it looks like there are additional blockers for this:

@metamask/eth-token-tracker@9.0.0 is on @metamask/eth-block-tracker@10.0.0
@metamask/network-controller@20.2.0 is on @metamask/eth-block-tracker@9.0.3

Given that we might not have time to fully upgrade eth-block-tracker in this PR (it needs to be merged urgently), how would you feel about extracting the transaction-controller version bump to a separate ticket?

MajorLift · 2024-08-20T14:09:44Z

@bschorchit Thanks for checking in on this. We're just waiting on review and approval especially from codeowners.

I just bumped this to ~~priority-2~~ priority-1 on the PR review queue, and I'll make a post today requesting another round of reviews.

gantunesr

Approved for team-accounts.

Files reviewed,

app/scripts/lib/accounts/BalancesController.ts
app/scripts/lib/snap-keyring/snap-keyring.test.ts

hmalik88 · 2024-08-20T17:16:38Z

app/scripts/snaps/preinstalled-snaps.ts

@@ -5,8 +5,8 @@ import BitcoinWalletSnap from '@metamask/bitcoin-wallet-snap/dist/preinstalled-s
 ///: END:ONLY_INCLUDE_IF

 // The casts here are less than ideal but we expect the SnapController to validate the inputs.
-const PREINSTALLED_SNAPS: readonly PreinstalledSnap[] = Object.freeze([
-  MessageSigningSnap as unknown as PreinstalledSnap,
+const PREINSTALLED_SNAPS = Object.freeze<PreinstalledSnap[]>([


Suggested change

const PREINSTALLED_SNAPS = Object.freeze<PreinstalledSnap[]>([

const PREINSTALLED_SNAPS = Object.freeze<ReadOnlyArray<PreinstalledSnap>>([

Can we do this instead since before the type was readonly?

This would be redundant since the return type of Object.freeze<T> is readonly T. By hovering over PREINSTALLED_SNAPS, you can verify that its type is readonly PreinstalledSnap[] like before.

If we want to be extra careful about validating the type maybe we could add a satisfies clause to replace the type annotation that was originally there?

diff --git a/app/scripts/snaps/preinstalled-snaps.ts b/app/scripts/snaps/preinstalled-snaps.ts index 6be0bb7f35..95b9c54e69 100644 --- a/app/scripts/snaps/preinstalled-snaps.ts +++ b/app/scripts/snaps/preinstalled-snaps.ts @@ -10,6 +10,6 @@ const PREINSTALLED_SNAPS = Object.freeze<PreinstalledSnap[]>([ ///: BEGIN:ONLY_INCLUDE_IF(build-flask) BitcoinWalletSnap as unknown as PreinstalledSnap, ///: END:ONLY_INCLUDE_IF -]); +]) satisfies readonly PreinstalledSnap[]; export default PREINSTALLED_SNAPS;

@daniela2000111 Is there a bug related to the changes in preinstalled-snaps.ts?

Ah yes that's correct! Fine to leave as is :)

Curious about this bug report ^

sonarcloud · 2024-08-20T20:20:59Z

Quality Gate passed

Issues
0 New issues
1 Accepted issue

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

See analysis details on SonarCloud

MajorLift · 2024-08-20T20:28:45Z

app/scripts/snaps/preinstalled-snaps.ts

-  AccountWatcherSnap as unknown as PreinstalledSnap,
+const PREINSTALLED_SNAPS = Object.freeze<PreinstalledSnap[]>([
+  MessageSigningSnap as PreinstalledSnap,
+  AccountWatcherSnap as PreinstalledSnap,


@k-g-j I removed an as unknown as cast introduced in #26402 while merging develop into this branch.

metamaskbot · 2024-08-20T21:07:16Z

Builds ready [4d13778]

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
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (67 ± 7 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	64	149	95	23	11
		domContentLoaded	41	94	63	15	7
		load	45	101	67	15	7
		domInteractive	9	62	27	14	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 192.1 KiB (5.72%)
ui: 120 Bytes (0.00%)
common: 208.37 KiB (2.54%)

legobeat · 2024-08-20T23:15:00Z

@legobeat I bumped @metamask/transaction-controller to latest (3c40934), but it looks like there are additional blockers for this:
* `@metamask/eth-token-tracker@9.0.0` is on `@metamask/eth-block-tracker@10.0.0`

* `@metamask/network-controller@20.2.0` is on `@metamask/eth-block-tracker@9.0.3`
Given that we might not have time to fully upgrade eth-block-tracker in this PR (it needs to be merged urgently), how would you feel about extracting the transaction-controller version bump to a separate ticket?

FWIW, existing PR bumping network-controller to be in line would resolve the above, I believe: deps(network-controller): @metamask/eth-block-tracker@^9.0.3->^10.0.0 core#4424
It has just been rebased.
Also related: feat: upgrade network controller to v20 #26150

legobeat · 2024-08-21T01:43:45Z

@SocketSecurity ignore npm/@storybook/addon-actions@7.6.20, npm/@storybook/addon-docs@7.6.20, npm/@storybook/blocks@7.6.20, npm/@storybook/channels@7.6.20, npm/@storybook/client-logger@7.6.20, npm/@storybook/components@7.6.20, npm/@storybook/core-client@7.6.20, npm/@storybook/core-common@7.6.20, npm/@storybook/core-events@7.6.20, npm/@storybook/csf-plugin@7.6.20, npm/@storybook/csf-tools@7.6.20, npm/@storybook/docs-tools@7.6.20, npm/@storybook/manager-api@7.6.20, npm/@storybook/node-logger@7.6.20, npm/@storybook/postinstall@7.6.20, npm/@storybook/preview-api@7.6.20, npm/@storybook/react-dom-shim@7.6.20, npm/@storybook/react@7.6.20, npm/@storybook/router@7.6.20, npm/@storybook/theming@7.6.20, npm/@storybook/types@7.6.20

@storybook

@MajorLift What is the context behind bumping some @storybook/ packages from 7.6.19 to 7.6.20 as part of this PR?

There are still 7.6.19 versions remaining through transitive pins, with many now being duplicated. Would either of these options be reasonable?

Revert @storybook/ bumps in lockfile in this PR to stay on 7.6.19 for the time being
Break out full 7.6.20 upgrade in a separate PR

metamaskbot · 2024-09-03T15:06:46Z

More than one release label on PR. Keeping the lowest one (release-12.1.0) on PR and removing other release labels (release-12.5.0).

MajorLift added the team-wallet-framework label Jul 25, 2024

MajorLift self-assigned this Jul 25, 2024

MajorLift force-pushed the bump/eth-json-rpc-provider-14.0.0 branch 3 times, most recently from 04c7968 to 67468d3 Compare July 31, 2024 16:02

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

MajorLift force-pushed the bump/eth-json-rpc-provider-14.0.0 branch from faea0b2 to 176f838 Compare July 31, 2024 16:25

This comment was marked as resolved.

Sign in to view

MajorLift force-pushed the bump/eth-json-rpc-provider-14.0.0 branch 2 times, most recently from 4f2217c to 4abc716 Compare July 31, 2024 20:19

MajorLift added the team-confirmations Push issues to confirmations team label Aug 1, 2024

MajorLift force-pushed the bump/eth-json-rpc-provider-14.0.0 branch from 4abc716 to 520ac80 Compare August 1, 2024 21:19

MajorLift force-pushed the bump/eth-json-rpc-provider-14.0.0 branch 2 times, most recently from 38d7512 to c6c724a Compare August 12, 2024 19:51

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

MajorLift force-pushed the bump/eth-json-rpc-provider-14.0.0 branch from 3a41bd3 to 4fa246f Compare August 13, 2024 16:29

This comment was marked as outdated.

Sign in to view

This comment was marked as duplicate.

Sign in to view

This comment was marked as outdated.

Sign in to view

Bump @metamask/transaction-controller from ^34.0.0 to ^35.1.1

3c40934

MajorLift changed the title ~~fix(deps): Bump @metamask/eth-json-rpc-middleware from ^12.1.1 to ^14.0.0~~ fix(deps): Bump @metamask/eth-json-rpc-middleware to ^14.0.0, @metamask/transaction-controller to ^35.1.1 Aug 20, 2024

MajorLift added the release-blocker This bug is blocking the next release label Aug 20, 2024

gantunesr previously approved these changes Aug 20, 2024

View reviewed changes

hmalik88 reviewed Aug 20, 2024

View reviewed changes

Merge branch 'develop' into bump/eth-json-rpc-provider-14.0.0

fb5ffd3

MajorLift dismissed gantunesr’s stale review via fb5ffd3 August 20, 2024 20:12

MajorLift mentioned this pull request Aug 20, 2024

chore(deps): Bump @metamask/utils from ^8.2.1 to ^9.1.0 #26551

Merged

8 tasks

This comment was marked as resolved.

Sign in to view

Dedupe lockfile

4d13778

MajorLift commented Aug 20, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

owencraston approved these changes Aug 20, 2024

View reviewed changes

sleepytanya approved these changes Aug 20, 2024

View reviewed changes

hmalik88 approved these changes Aug 20, 2024

View reviewed changes

legobeat mentioned this pull request Aug 20, 2024

deps(network-controller): @metamask/eth-block-tracker@^9.0.3->^10.0.0 MetaMask/core#4424

Merged

7 tasks

benjisclowder merged commit 27655eb into develop Aug 21, 2024
78 checks passed

benjisclowder deleted the bump/eth-json-rpc-provider-14.0.0 branch August 21, 2024 08:19

github-actions bot locked and limited conversation to collaborators Aug 21, 2024

github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Aug 21, 2024

metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 21, 2024

metamaskbot removed the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Sep 3, 2024

	const PREINSTALLED_SNAPS = Object.freeze<PreinstalledSnap[]>([
	const PREINSTALLED_SNAPS = Object.freeze<ReadOnlyArray<PreinstalledSnap>>([

fix(deps): Bump @metamask/eth-json-rpc-middleware to ^14.0.0, @metamask/transaction-controller to ^35.1.1 #26143

fix(deps): Bump @metamask/eth-json-rpc-middleware to ^14.0.0, @metamask/transaction-controller to ^35.1.1 #26143

Conversation

MajorLift commented Jul 25, 2024 • edited Loading

Description

Related issues

Changelog

Added

Changed

Fixed

Security

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jul 25, 2024

legobeat commented Jul 25, 2024 • edited Loading

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

MajorLift commented Aug 12, 2024 • edited Loading

MajorLift commented Aug 12, 2024

This comment was marked as outdated.

This comment was marked as outdated.

MajorLift commented Aug 13, 2024

This comment was marked as outdated.

This comment was marked as duplicate.

This comment was marked as outdated.

metamaskbot commented Aug 20, 2024

MajorLift commented Aug 20, 2024 • edited Loading

MajorLift commented Aug 20, 2024 • edited Loading

gantunesr left a comment

Choose a reason for hiding this comment

hmalik88 Aug 20, 2024

Choose a reason for hiding this comment

MajorLift Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

daniela2000111 Aug 20, 2024

Choose a reason for hiding this comment

MajorLift Aug 20, 2024

Choose a reason for hiding this comment

hmalik88 Aug 20, 2024

Choose a reason for hiding this comment

hmalik88 Aug 20, 2024

Choose a reason for hiding this comment

This comment was marked as resolved.

sonarcloud bot commented Aug 20, 2024

Quality Gate passed

MajorLift Aug 20, 2024

Choose a reason for hiding this comment

This comment was marked as resolved.

metamaskbot commented Aug 20, 2024

legobeat commented Aug 20, 2024 • edited Loading

legobeat commented Aug 21, 2024 • edited Loading

metamaskbot commented Sep 3, 2024

fix(deps): Bump `@metamask/eth-json-rpc-middleware` to `^14.0.0`, `@metamask/transaction-controller` to `^35.1.1` #26143

fix(deps): Bump `@metamask/eth-json-rpc-middleware` to `^14.0.0`, `@metamask/transaction-controller` to `^35.1.1` #26143

MajorLift commented Jul 25, 2024 •

edited

Loading

legobeat commented Jul 25, 2024 •

edited

Loading

MajorLift commented Aug 12, 2024 •

edited

Loading

MajorLift commented Aug 20, 2024 •

edited

Loading

MajorLift commented Aug 20, 2024 •

edited

Loading

MajorLift Aug 20, 2024 •

edited

Loading

legobeat commented Aug 20, 2024 •

edited

Loading

legobeat commented Aug 21, 2024 •

edited

Loading