fix: remove btc account from permission connect lists by montelaidev · Pull Request #25980 · MetaMask/metamask-extension

montelaidev · 2024-07-19T13:16:01Z

Description

This PR filters BTC accounts from the permission connect list.

Related issues

Fixes:

Manual testing steps

Create a BTC account
Connect to a dapp
Open popup
Click network icon
Click on Connect more accounts
See that there aren't any btc accounts

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-19T13:16:15Z

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.

...ponents/multichain/connect-accounts-modal/__snapshots__/connect-accounts-modal.test.tsx.snap

ui/components/multichain/connect-accounts-modal/connect-accounts-modal.tsx

ui/pages/permissions-connect/permissions-connect.container.js

ui/pages/permissions-connect/permissions-connect.test.tsx

Co-authored-by: Charly Chevalier <charly.chevalier@consensys.net>

greptile-apps

PR Summary

This PR filters out BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

ui/components/multichain/connect-accounts-modal/connect-accounts-modal.tsx: Filters out non-EVM accounts using isEvmAccountType function.
ui/pages/permissions-connect/permissions-connect.container.js: Filters non-EVM accounts from the permissions connect list.
ui/components/multichain/connect-accounts-modal/connect-accounts-modal.test.tsx: Updates tests to ensure only EVM accounts are rendered.
ui/pages/permissions-connect/permissions-connect.test.tsx: Adds tests to verify BTC accounts are excluded from the permission connect list.
test/jest/mocks.ts: Migrates to TypeScript, enhancing type safety and maintainability.

_{10 file(s) reviewed, 25 comment(s)}
_{Edit PR Review Bot Settings}

app/scripts/lib/accounts/BalancesController.test.ts

app/scripts/lib/accounts/BalancesTracker.test.ts

test/jest/mocks.ts

…nnect-lists

greptile-apps

PR Summary

(updates since last review)

This PR focuses on filtering out BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

.circleci/config.yml: Removed jobs related to 'confirmation redesign' feature, simplifying CI configuration.
app/scripts/background.js: Enhanced initialization process with test-specific overrides for keyring bridges.
app/scripts/controllers/preferences.js: Added isRedesignedConfirmationsDeveloperEnabled state property.
app/scripts/lib/ppom/ppom-middleware.ts: Introduced isChainSupported function for dynamic chain support checks.
ui/selectors/selectors.js: Filtered out BTC accounts from permission connect lists.

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

greptile-apps

PR Summary

(updates since last review)

This PR filters BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

test/e2e/api-specs/ConfirmationRejectionRule.ts: Removed rejectButtonInsteadOfCancel array and associated logic, now consistently clicking 'Cancel' for all methods.
/ui/selectors/accounts.ts: Added utility functions to identify BTC accounts and filter them out.
/ui/pages/permissions-connect/choose-account: Updated ChooseAccount component to exclude BTC accounts.
/ui/components/ui/account-list/account-list.js: Modified handleEvmAccountClick to ensure only EVM accounts are selectable.
/app/scripts/controllers/permissions/background-api.js: Adjusted removePermittedAccount to handle BTC accounts appropriately.

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

…nnect-lists

greptile-apps

PR Summary

(updates since last review)

This PR filters BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

.circleci/config.yml: Removed test-e2e-swap-playwright job to streamline CI pipeline.
app/scripts/background.js: Added conditional logic to initialize socket connection for Mocha tests.
shared/lib/transactions-controller-utils.js: Introduced precision parameter to getSwapsTokensReceivedFromTxMeta for flexible rounding.
ui/pages/permissions-connect/choose-account: Updated ChooseAccount component to exclude BTC accounts.
test/e2e/background-socket/server-mocha-to-background.ts: Added WebSocket server class for enhanced end-to-end testing.

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

…nnect-lists

greptile-apps

PR Summary

(updates since last review)

This PR filters BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

app/scripts/metamask-controller.js: Added getIsConfirmationAdvancedDetailsOpen method to filter BTC accounts.
ui/pages/permissions-connect/choose-account: Updated ChooseAccount component to exclude BTC accounts.
ui/selectors/accounts.ts: Added utility functions to identify BTC accounts.
ui/components/multichain/account-list-item/account-list-item.js: Modified to handle BTC account exclusion.
ui/components/multichain/account-list-menu/account-list-menu.js: Updated account list menu to exclude BTC accounts.

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

greptile-apps

PR Summary

(updates since last review)

This PR filters BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

test/e2e/flask/btc/btc-dapp-connection.spec.ts: Removed BTC account interactions and assertions to align with the new filtering requirement.
app/scripts/metamask-controller.js: Added logic to filter BTC accounts from the permission connect list.
ui/pages/permissions-connect/choose-account: Updated ChooseAccount component to exclude BTC accounts.
ui/selectors/accounts.ts: Added utility functions to identify and filter BTC accounts.
ui/components/multichain/account-list-item/account-list-item.js: Modified to handle BTC account exclusion in the account list.

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

metamaskbot · 2024-07-25T12:51:43Z

Builds ready [db5849a]

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, 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 (159 ± 181 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	59	157	104	27	13
		domContentLoaded	9	73	31	19	9
		load	46	1795	159	376	181
		domInteractive	9	73	31	19	9

Bundle size diffs

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

ui/pages/permissions-connect/permissions-connect.test.tsx

Co-authored-by: Daniel Rocha <daniel.rocha@consensys.net>

greptile-apps

PR Summary

(updates since last review)

This PR filters BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

ui/pages/permissions-connect/permissions-connect.test.tsx: Added test case to verify BTC accounts are excluded from the permission connect list.
ui/selectors/accounts.ts: Added utility functions to identify and filter BTC accounts.
ui/selectors/accounts.test.ts: Updated tests to cover new BTC account filtering logic.

No major changes found since the last review.

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

greptile-apps

PR Summary

This PR filters BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

app/scripts/lib/accounts/BalancesController.test.ts: Removed TypeScript error suppression comments, improving type safety.
test/e2e/flask/btc/btc-dapp-connection.spec.ts: Updated to ensure BTC accounts are not present in the account selection list.
ui/components/multichain/connect-accounts-modal/connect-accounts-modal.tsx: Filters out non-EVM accounts using isEvmAccountType function.
ui/pages/permissions-connect/permissions-connect.container.js: Filters BTC accounts from the permissions connect list.
ui/pages/permissions-connect/permissions-connect.test.tsx: Added tests to verify BTC accounts are excluded from the permission connect list.

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

greptile-apps

PR Summary

(updates since last review)

This PR filters BTC accounts from the permission connect list to ensure only EVM-compatible accounts are displayed.

ui/components/multichain/connect-accounts-modal/connect-accounts-modal.tsx: Filters out non-EVM accounts using isEvmAccountType function.
ui/pages/permissions-connect/choose-account/choose-account.js: Filters BTC accounts from the permissions connect list.
ui/pages/permissions-connect/permissions-connect.container.js: Filters BTC accounts from the permissions connect list.
ui/helpers/utils/permissions.ts: Adds utility function to check for ETH permissions and non-EVM accounts.
test/e2e/flask/btc/btc-dapp-connection.spec.ts: Updated to ensure BTC accounts are not present in the account selection list.

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

sonarqubecloud · 2024-07-25T14:02:47Z

Quality Gate passed

Issues
0 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-07-25T14:29:59Z

Builds ready [6b4133d]

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, 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 (185 ± 233 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	81	186	121	25	12
		domContentLoaded	9	66	25	16	7
		load	47	2293	185	484	233
		domInteractive	9	66	25	16	7

Bundle size diffs

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

metamaskbot · 2024-08-30T17:08:50Z

Missing release label release-12.2.0 on PR. Adding release label release-12.2.0 on PR and removing other release labels(release-12.3.0), as PR was cherry-picked in branch 12.2.0.

montelaidev added the team-accounts-framework Accounts team label Jul 19, 2024

montelaidev self-assigned this Jul 19, 2024

montelaidev marked this pull request as ready for review July 19, 2024 13:48

montelaidev requested a review from a team as a code owner July 19, 2024 13:48

montelaidev added 6 commits July 19, 2024 23:08

fix: filter btc accounts from permission connect

6e633a3

fix: rename test

7c7f466

fix: connect account modals to filter btc accounts

d86e006

fix: lint

6cb0479

fix: lint

f1f2b86

fix: remove unused expect-error because mocks are now typed

b37090e

montelaidev force-pushed the fix/remove-btc-account-from-permission-connect-lists branch from f72235d to b37090e Compare July 19, 2024 15:09

ccharly reviewed Jul 19, 2024

View reviewed changes

montelaidev and others added 5 commits July 22, 2024 20:07

Apply suggestions from code review

462f392

Co-authored-by: Charly Chevalier <charly.chevalier@consensys.net>

fix: snapshot

243d779

fix: snapshot

e32a7b0

fix: lint

d3c26e3

fix: remove comments

a392c4e

greptile-apps bot reviewed Jul 22, 2024

View reviewed changes

montelaidev added 2 commits July 23, 2024 22:30

Merge branch 'develop' into fix/remove-btc-account-from-permission-co…

14a7de7

…nnect-lists

Merge branch 'develop' into fix/remove-btc-account-from-permission-co…

85156e8

…nnect-lists

greptile-apps bot reviewed Jul 23, 2024

View reviewed changes

Merge branch 'develop' into fix/remove-btc-account-from-permission-co…

e63dbc0

…nnect-lists

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

Merge branch 'develop' into fix/remove-btc-account-from-permission-co…

79fb46b

…nnect-lists

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

fix: e2e test

db5849a

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

MetaMask deleted a comment from greptile-apps bot Jul 25, 2024

danroc reviewed Jul 25, 2024

View reviewed changes

ui/pages/permissions-connect/permissions-connect.test.tsx Outdated Show resolved Hide resolved

danroc reviewed Jul 25, 2024

View reviewed changes

ui/pages/permissions-connect/permissions-connect.test.tsx Outdated Show resolved Hide resolved

Apply suggestions from code review

f8698b6

Co-authored-by: Daniel Rocha <daniel.rocha@consensys.net>

montelaidev dismissed ccharly’s stale review via f8698b6 July 25, 2024 13:29

danroc previously approved these changes Jul 25, 2024

View reviewed changes

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

danroc requested review from greptile-apps[bot] and removed request for greptile-apps[bot] July 25, 2024 13:30

ccharly previously approved these changes Jul 25, 2024

View reviewed changes

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

fix: test name in snapshot

6b4133d

montelaidev dismissed stale reviews from ccharly and danroc via 6b4133d July 25, 2024 14:00

greptile-apps bot reviewed Jul 25, 2024

View reviewed changes

ccharly approved these changes Jul 25, 2024

View reviewed changes

danroc approved these changes Jul 25, 2024

View reviewed changes

montelaidev merged commit 95f7753 into develop Jul 25, 2024

montelaidev deleted the fix/remove-btc-account-from-permission-connect-lists branch July 25, 2024 14:37

github-actions bot locked and limited conversation to collaborators Jul 25, 2024

metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 25, 2024

metamaskbot added release-12.2.0 Issue or pull request that will be included in release 12.2.0 and removed release-12.3.0 Issue or pull request that will be included in release 12.3.0 labels Aug 30, 2024

Conversation

montelaidev commented Jul 19, 2024

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Jul 19, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

metamaskbot commented Jul 25, 2024

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

sonarqubecloud bot commented Jul 25, 2024

Quality Gate passed

Uh oh!

metamaskbot commented Jul 25, 2024

Uh oh!

metamaskbot commented Aug 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants