refactor: Typescript conversion of ethereum-chain-utils.js #26103

NiranjanaBinoy · 2024-07-24T18:58:43Z

Description

Converting the level 6 dependency file app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js to typescript for contributing to metamask-controller.js.

Related issues

Part of #23014
Fixes: #26102

Manual testing steps

Go to this page...

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-07-24T18:58:55Z

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.

greptile-apps

PR Summary

Converted ethereum-chain-utils.js to TypeScript, enhancing type safety and maintainability.

File Added: app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts
- Converted functions like validateChainId, validateSwitchEthereumChainParams, validateAddEthereumChainParams, and switchChain to TypeScript.
File Added: app/scripts/lib/rpc-method-middleware/handlers/types.ts
- Defined TypeScript types for handler wrappers, approval flows, caveat retrieval, chain permissions, user approvals, and network settings.

Ensure thorough testing to validate type definitions against actual data structures and usage patterns.

_{2 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Converted ethereum-chain-utils.js to TypeScript, enhancing type safety and maintainability.

File Removed: app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js
- Deleted JavaScript file as part of TypeScript migration.
File Added: app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts
- Converted utility functions like validateChainId, validateSwitchEthereumChainParams, validateAddEthereumChainParams, and switchChain to TypeScript.
File Added: app/scripts/lib/rpc-method-middleware/handlers/types.ts
- Defined TypeScript types for handler wrappers, approval flows, caveat retrieval, chain permissions, user approvals, and network settings.

Ensure thorough testing to validate type definitions against actual data structures and usage patterns.

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

MajorLift

Looks like most of the as assertions being added that could be removed:

app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts

greptile-apps

PR Summary

(updates since last review)

Converted ethereum-chain-utils.js to TypeScript, enhancing type safety and maintainability.

File Removed: app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js
- Deleted JavaScript file as part of TypeScript migration.
File Added: app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts
- Converted utility functions like validateChainId, validateSwitchEthereumChainParams, validateAddEthereumChainParams, and switchChain to TypeScript.
File Modified: app/scripts/lib/rpc-method-middleware/handlers/types.ts
- Added new TypeScript type definitions for FindNetworkConfigurationBy, GetCaveat, GetChainPermissionsFeatureFlag, RequestPermittedChainsPermission, RequestUserApproval, and SetActiveNetwork.
- Removed HandlerWrapper type, indicating a possible refactor or simplification.

_{2 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Converted ethereum-chain-utils.js to TypeScript, enhancing type safety and maintainability.

File Removed: app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js
- Deleted JavaScript file as part of TypeScript migration.
File Added: app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts
- Converted utility functions like validateChainId, validateSwitchEthereumChainParams, validateAddEthereumChainParams, and switchChain to TypeScript.
File Modified: app/scripts/lib/rpc-method-middleware/handlers/types.ts
- Added new TypeScript type definitions for FindNetworkConfigurationBy, GetCaveat, GetChainPermissionsFeatureFlag, RequestPermittedChainsPermission, RequestUserApproval, and SetActiveNetwork.
- Removed HandlerWrapper type, indicating a possible refactor or simplification.

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

NiranjanaBinoy · 2024-07-25T01:48:51Z

app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts

-export function validateChainId(chainId) {
-  const _chainId = typeof chainId === 'string' && chainId.toLowerCase();
+export function validateChainId(chainId: Hex): Hex {
+  const _chainId = chainId.toLowerCase() as Hex;


@MajorLift When we do chainId.toLowerCase(), it seems to be returning type string. Is there any way for chainId to be Hex without the as assertion?

We can also define a type guard here. The value is Hex predicate is safe to use since any string that's validated by isPrefixedFormattedHexString must be equivalent to or narrower than the Hex type.

diff --git a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts index 002d6554fc..8622b1047c 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts +++ b/app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts @@ -58,9 +58,13 @@ export function findExistingNetwork( } export function validateChainId(chainId: Hex): Hex { - const _chainId = chainId.toLowerCase() as Hex; + const _chainId = chainId.toLowerCase(); - if (!isPrefixedFormattedHexString(_chainId)) { + if ( + !((value: string): value is Hex => isPrefixedFormattedHexString(value))( + _chainId, + ) + ) { throw ethErrors.rpc.invalidParams({ message: `Expected 0x-prefixed, unpadded, non-zero hexadecimal string 'chainId'. Received:\n${chainId}`, });

toLowerCase does unnecessarily widen the type of chainId, and an as assertion is one way to resolve this.

However, there's a better way to reason about the type of _chainId. Because the code immediately invokes the validator function isPrefixedFormattedHexString, we are provided with a runtime guarantee that any _chainId value that doesn't throw an error in the first if-block will be of a type narrower than Hex.

So now all we have to do is make sure this guarantee is reflected at the type-level as well, which we achieve by using the type guard with the value is Hex predicate.

This successfully narrows _chainId back to Hex past the first if-block. Without relying on any assertions, or introducing any uncertainty about value-level vs. type-level discrepancies.

…ypes to types.ts file

codecov · 2024-08-14T18:56:58Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.12%. Comparing base (cdd543d) to head (4111fb1).
Report is 878 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #26103      +/-   ##
===========================================
+ Coverage    69.98%   70.12%   +0.14%     
===========================================
  Files         1422     1428       +6     
  Lines        49926    50107     +181     
  Branches     13861    13897      +36     
===========================================
+ Hits         34940    35136     +196     
+ Misses       14986    14971      -15

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

metamaskbot · 2024-08-14T19:07:41Z

Builds ready [a60947e]

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, 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 (82 ± 24 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	68	297	112	48	23
		domContentLoaded	9	104	27	21	10
		load	42	281	82	51	24
		domInteractive	9	104	27	21	10

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

background: 180 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

sonarqubecloud · 2024-08-15T19:07:13Z

Quality Gate passed

Issues
3 New issues
0 Accepted issues

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

See analysis details on SonarCloud

metamaskbot · 2024-08-15T19:34:17Z

Builds ready [4111fb1]

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, 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 ± 10 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	64	134	96	20	9
		domContentLoaded	9	62	21	15	7
		load	43	116	67	20	10
		domInteractive	9	61	21	15	7

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

background: 183 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

github-actions · 2024-10-15T02:27:57Z

This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions.

NiranjanaBinoy added the team-extension-platform label Jul 24, 2024

NiranjanaBinoy self-assigned this Jul 24, 2024

NiranjanaBinoy requested a review from a team as a code owner July 24, 2024 18:58

greptile-apps bot reviewed Jul 24, 2024

View reviewed changes

MajorLift reviewed Jul 24, 2024

View reviewed changes

app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.ts Outdated Show resolved Hide resolved

greptile-apps bot reviewed Jul 24, 2024

View reviewed changes

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

NiranjanaBinoy commented Jul 25, 2024

View reviewed changes

NiranjanaBinoy force-pushed the ts-ethereum-chain-utils branch 2 times, most recently from e72e678 to 3b36759 Compare August 14, 2024 16:07

NiranjanaBinoy added 7 commits August 14, 2024 14:37

converting ethereum-chain-utils to typescript and moving reususable t…

f118bf9

…ypes to types.ts file

removing the javascript file

d055760

updating types

c168f1e

addressing review comments

e7a17fb

removing type assestion at chainId.toLowercase()

0b3e396

updating based on conflict

b18429b

updating based in comments in #23806

a60947e

NiranjanaBinoy force-pushed the ts-ethereum-chain-utils branch from 3b36759 to a60947e Compare August 14, 2024 18:37

updating based on switch-ethereum-chain migartion

4111fb1

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

github-actions bot added stale issues and PRs marked as stale and removed stale issues and PRs marked as stale labels Oct 15, 2024

gauthierpetetin unassigned NiranjanaBinoy Nov 4, 2024

MajorLift self-assigned this Nov 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Typescript conversion of ethereum-chain-utils.js #26103

refactor: Typescript conversion of ethereum-chain-utils.js #26103

NiranjanaBinoy commented Jul 24, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Jul 24, 2024

greptile-apps bot left a comment

greptile-apps bot left a comment

MajorLift left a comment •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot left a comment

NiranjanaBinoy Jul 25, 2024

MajorLift Jul 30, 2024 •

edited

Loading

MajorLift Jul 31, 2024 •

edited

Loading

codecov bot commented Aug 14, 2024 •

edited

Loading

metamaskbot commented Aug 14, 2024

sonarqubecloud bot commented Aug 15, 2024

metamaskbot commented Aug 15, 2024

github-actions bot commented Oct 15, 2024

refactor: Typescript conversion of ethereum-chain-utils.js #26103

Are you sure you want to change the base?

refactor: Typescript conversion of ethereum-chain-utils.js #26103

Conversation

NiranjanaBinoy commented Jul 24, 2024 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jul 24, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

MajorLift left a comment • edited Loading

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

NiranjanaBinoy Jul 25, 2024

Choose a reason for hiding this comment

MajorLift Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

MajorLift Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Aug 14, 2024 • edited Loading

Codecov Report

metamaskbot commented Aug 14, 2024

sonarqubecloud bot commented Aug 15, 2024

Quality Gate passed

metamaskbot commented Aug 15, 2024

github-actions bot commented Oct 15, 2024

NiranjanaBinoy commented Jul 24, 2024 •

edited by github-actions bot

Loading

MajorLift left a comment •

edited

Loading

MajorLift Jul 30, 2024 •

edited

Loading

MajorLift Jul 31, 2024 •

edited

Loading

codecov bot commented Aug 14, 2024 •

edited

Loading