From 1fbb63cc4e2c15ffc344155f8fa6e6f240d8dd74 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 6 Jan 2025 12:20:27 +0100 Subject: [PATCH 1/3] fix: Correct theme value for Snap UI footer buttons (#29434) ## **Description** The Snap UI footer buttons use the DS button which forces light theme, this does not work for the colors used in the Snap buttons, therefore we revert back to using the actually selected theme. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29434?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29388 ## **Screenshots/Recordings** ### **Before** ![Image](https://github.com/user-attachments/assets/fb14d5c1-44f9-473a-81bf-6f0f01c46686) ### **After** ![image](https://github.com/user-attachments/assets/c9f08368-58f5-43b3-a129-fe7689572242) --- .../app/snaps/snap-ui-footer-button/snap-ui-footer-button.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/components/app/snaps/snap-ui-footer-button/snap-ui-footer-button.tsx b/ui/components/app/snaps/snap-ui-footer-button/snap-ui-footer-button.tsx index ff3f530ae1e7..6b4dc63e9a3d 100644 --- a/ui/components/app/snaps/snap-ui-footer-button/snap-ui-footer-button.tsx +++ b/ui/components/app/snaps/snap-ui-footer-button/snap-ui-footer-button.tsx @@ -87,6 +87,7 @@ export const SnapUIFooterButton: FunctionComponent< alignItems: AlignItems.center, flexDirection: FlexDirection.Row, }} + data-theme={null} > {isSnapAction && !hideSnapBranding && !loading && ( From c7d14a001f534fabd09866824eef7ce6a34a885e Mon Sep 17 00:00:00 2001 From: Hassan Malik <41640681+hmalik88@users.noreply.github.com> Date: Mon, 6 Jan 2025 08:07:23 -0500 Subject: [PATCH 2/3] chore: replace local `isSnapId` definition with `isSnapId` from `@metamask/snaps-utils` (#29422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? Deduplication of code 2. What is the improvement/solution? Using the `isSnapId` function from the `@metamask/snaps-utils` package. ## **Related issues** Fixes: #29280 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --- app/scripts/metamask-controller.js | 7 +++---- ui/components/app/confirm/info/row/url.tsx | 2 +- .../connected-sites-list.component.js | 2 +- .../snap-metadata-modal/snap-metadata-modal.js | 7 +++++-- .../pages/permissions-page/permissions-page.js | 2 +- ui/helpers/utils/snaps.ts | 16 ---------------- .../info/personal-sign/personal-sign.test.tsx | 11 ++++------- .../confirm/info/personal-sign/personal-sign.tsx | 2 +- .../info/typed-sign-v1/typed-sign-v1.test.tsx | 11 ++++------- .../confirm/info/typed-sign-v1/typed-sign-v1.tsx | 2 +- .../confirm/info/typed-sign/typed-sign.test.tsx | 11 ++++------- .../confirm/info/typed-sign/typed-sign.tsx | 2 +- .../permissions-connect.component.js | 2 +- .../snaps/snap-install/snap-install.js | 2 +- .../snaps/snaps-connect/snaps-connect.js | 2 +- ui/pages/snaps/snap-view/snap-settings.js | 2 +- 16 files changed, 30 insertions(+), 53 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 62fe3c942589..0e568424a69c 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -146,12 +146,13 @@ import { TransactionType, } from '@metamask/transaction-controller'; -///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) import { + ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) getLocalizedSnapManifest, stripSnapPrefix, + ///: END:ONLY_INCLUDE_IF + isSnapId, } from '@metamask/snaps-utils'; -///: END:ONLY_INCLUDE_IF import { Interface } from '@ethersproject/abi'; import { abiERC1155, abiERC721 } from '@metamask/metamask-eth-abis'; @@ -245,8 +246,6 @@ import { } from '../../shared/lib/transactions-controller-utils'; import { getProviderConfig } from '../../shared/modules/selectors/networks'; import { endTrace, trace } from '../../shared/lib/trace'; -// eslint-disable-next-line import/no-restricted-paths -import { isSnapId } from '../../ui/helpers/utils/snaps'; import { BridgeStatusAction } from '../../shared/types/bridge-status'; import { ENVIRONMENT } from '../../development/build/constants'; import fetchWithCache from '../../shared/lib/fetch-with-cache'; diff --git a/ui/components/app/confirm/info/row/url.tsx b/ui/components/app/confirm/info/row/url.tsx index 807fc7298042..cc517880bb42 100644 --- a/ui/components/app/confirm/info/row/url.tsx +++ b/ui/components/app/confirm/info/row/url.tsx @@ -1,4 +1,5 @@ import React, { useCallback, useState } from 'react'; +import { isSnapId } from '@metamask/snaps-utils'; import { Box, Icon, @@ -18,7 +19,6 @@ import { } from '../../../../../helpers/constants/design-system'; import SnapAuthorshipPill from '../../../snaps/snap-authorship-pill'; import { SnapMetadataModal } from '../../../snaps/snap-metadata-modal'; -import { isSnapId } from '../../../../../helpers/utils/snaps'; export type ConfirmInfoRowUrlProps = { url: string; diff --git a/ui/components/app/connected-sites-list/connected-sites-list.component.js b/ui/components/app/connected-sites-list/connected-sites-list.component.js index d3b6cffa03fb..edb9ef4aaf50 100644 --- a/ui/components/app/connected-sites-list/connected-sites-list.component.js +++ b/ui/components/app/connected-sites-list/connected-sites-list.component.js @@ -1,11 +1,11 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; +import { isSnapId } from '@metamask/snaps-utils'; import Button from '../../ui/button'; import { AvatarFavicon, IconSize } from '../../component-library'; import { stripHttpsSchemeWithoutPort } from '../../../helpers/utils/util'; import SiteOrigin from '../../ui/site-origin'; import { Size } from '../../../helpers/constants/design-system'; -import { isSnapId } from '../../../helpers/utils/snaps'; import { SnapIcon } from '../snaps/snap-icon'; export default class ConnectedSitesList extends Component { diff --git a/ui/components/app/snaps/snap-metadata-modal/snap-metadata-modal.js b/ui/components/app/snaps/snap-metadata-modal/snap-metadata-modal.js index 93a57309e89a..ca5049d16598 100644 --- a/ui/components/app/snaps/snap-metadata-modal/snap-metadata-modal.js +++ b/ui/components/app/snaps/snap-metadata-modal/snap-metadata-modal.js @@ -1,7 +1,11 @@ import React from 'react'; import PropTypes from 'prop-types'; import { useSelector } from 'react-redux'; -import { getSnapPrefix, stripSnapPrefix } from '@metamask/snaps-utils'; +import { + getSnapPrefix, + isSnapId, + stripSnapPrefix, +} from '@metamask/snaps-utils'; import { getSnap, getSnapRegistryData, @@ -39,7 +43,6 @@ import { ShowMore } from '../show-more'; import SnapExternalPill from '../snap-version/snap-external-pill'; import { useSafeWebsite } from '../../../../hooks/snaps/useSafeWebsite'; import Tooltip from '../../../ui/tooltip'; -import { isSnapId } from '../../../../helpers/utils/snaps'; import { SnapIcon } from '../snap-icon'; export const SnapMetadataModal = ({ snapId, isOpen, onClose }) => { diff --git a/ui/components/multichain/pages/permissions-page/permissions-page.js b/ui/components/multichain/pages/permissions-page/permissions-page.js index 3f4680bf0350..fa9082b54af8 100644 --- a/ui/components/multichain/pages/permissions-page/permissions-page.js +++ b/ui/components/multichain/pages/permissions-page/permissions-page.js @@ -1,6 +1,7 @@ import React, { useEffect, useRef, useState } from 'react'; import { useHistory } from 'react-router-dom'; import { useSelector } from 'react-redux'; +import { isSnapId } from '@metamask/snaps-utils'; import { Content, Header, Page } from '../page'; import { Box, @@ -26,7 +27,6 @@ import { REVIEW_PERMISSIONS, } from '../../../../helpers/constants/routes'; import { getConnectedSitesListWithNetworkInfo } from '../../../../selectors'; -import { isSnapId } from '../../../../helpers/utils/snaps'; import { ConnectionListItem } from './connection-list-item'; export const PermissionsPage = () => { diff --git a/ui/helpers/utils/snaps.ts b/ui/helpers/utils/snaps.ts index c788393e83ab..d7527f89b482 100644 --- a/ui/helpers/utils/snaps.ts +++ b/ui/helpers/utils/snaps.ts @@ -1,21 +1,5 @@ -import { SnapId } from '@metamask/snaps-sdk'; import { isProduction } from '../../../shared/modules/environment'; -/** - * Check if the given value is a valid snap ID. - * - * NOTE: This function is a duplicate oF a yet to be released version in @metamask/snaps-utils. - * - * @param value - The value to check. - * @returns `true` if the value is a valid snap ID, and `false` otherwise. - */ -export function isSnapId(value: unknown): value is SnapId { - return ( - (typeof value === 'string' || value instanceof String) && - (value.startsWith('local:') || value.startsWith('npm:')) - ); -} - /** * Decode a snap ID fron a pathname. * diff --git a/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.test.tsx b/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.test.tsx index 336ef3f9f92f..471309bf5e31 100644 --- a/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import { TransactionType } from '@metamask/transaction-controller'; +import { isSnapId } from '@metamask/snaps-utils'; import { getMockConfirmState, getMockPersonalSignConfirmState, @@ -13,7 +14,6 @@ import { signatureRequestSIWE, unapprovedPersonalSignMsg, } from '../../../../../../../test/data/confirmations/personal_sign'; -import * as snapUtils from '../../../../../../helpers/utils/snaps'; import { SignatureRequestType } from '../../../../types/confirm'; import * as utils from '../../../../utils'; import PersonalSignInfo from './personal-sign'; @@ -43,13 +43,10 @@ jest.mock('../../../../../../../node_modules/@metamask/snaps-utils', () => { ...originalUtils, stripSnapPrefix: jest.fn().mockReturnValue('@metamask/examplesnap'), getSnapPrefix: jest.fn().mockReturnValue('npm:'), + isSnapId: jest.fn(), }; }); -jest.mock('../../../../../../helpers/utils/snaps', () => ({ - isSnapId: jest.fn(), -})); - describe('PersonalSignInfo', () => { it('renders correctly for personal sign request', () => { const state = getMockPersonalSignConfirmState(); @@ -149,7 +146,7 @@ describe('PersonalSignInfo', () => { getMockPersonalSignConfirmStateForRequest(signatureRequestSIWE); (utils.isSIWESignatureRequest as jest.Mock).mockReturnValue(false); - (snapUtils.isSnapId as unknown as jest.Mock).mockReturnValue(true); + (isSnapId as unknown as jest.Mock).mockReturnValue(true); const mockStore = configureMockStore([])(state); const { queryByText, getByText } = renderWithConfirmContextProvider( @@ -171,7 +168,7 @@ describe('PersonalSignInfo', () => { const state = getMockPersonalSignConfirmStateForRequest(signatureRequestSIWE); (utils.isSIWESignatureRequest as jest.Mock).mockReturnValue(false); - (snapUtils.isSnapId as unknown as jest.Mock).mockReturnValue(true); + (isSnapId as unknown as jest.Mock).mockReturnValue(true); const mockStore = configureMockStore([])(state); const { getByText, queryByText } = renderWithConfirmContextProvider( diff --git a/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.tsx b/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.tsx index 38238a2fa49e..7a83402f2ec6 100644 --- a/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.tsx +++ b/ui/pages/confirmations/components/confirm/info/personal-sign/personal-sign.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { useSelector } from 'react-redux'; +import { isSnapId } from '@metamask/snaps-utils'; import { ConfirmInfoRowText, ConfirmInfoRowUrl, @@ -27,7 +28,6 @@ import { TextColor, TextVariant, } from '../../../../../../helpers/constants/design-system'; -import { isSnapId } from '../../../../../../helpers/utils/snaps'; import { hexToText, sanitizeString, diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign-v1/typed-sign-v1.test.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign-v1/typed-sign-v1.test.tsx index 83696b69ac64..34ee667ea797 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign-v1/typed-sign-v1.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign-v1/typed-sign-v1.test.tsx @@ -2,10 +2,10 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import { TransactionType } from '@metamask/transaction-controller'; +import { isSnapId } from '@metamask/snaps-utils'; import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; import { getMockTypedSignConfirmStateForRequest } from '../../../../../../../test/data/confirmations/helper'; import { unapprovedTypedSignMsgV1 } from '../../../../../../../test/data/confirmations/typed_sign'; -import * as snapUtils from '../../../../../../helpers/utils/snaps'; import TypedSignInfoV1 from './typed-sign-v1'; jest.mock( @@ -25,13 +25,10 @@ jest.mock('../../../../../../../node_modules/@metamask/snaps-utils', () => { ...originalUtils, stripSnapPrefix: jest.fn().mockReturnValue('@metamask/examplesnap'), getSnapPrefix: jest.fn().mockReturnValue('npm:'), + isSnapId: jest.fn(), }; }); -jest.mock('../../../../../../helpers/utils/snaps', () => ({ - isSnapId: jest.fn(), -})); - describe('TypedSignInfo', () => { it('correctly renders typed sign data request', () => { const mockState = getMockTypedSignConfirmStateForRequest( @@ -65,7 +62,7 @@ describe('TypedSignInfo', () => { type: TransactionType.signTypedData, chainId: '0x5', }); - (snapUtils.isSnapId as unknown as jest.Mock).mockReturnValue(true); + (isSnapId as unknown as jest.Mock).mockReturnValue(true); const mockStore = configureMockStore([])(mockState); const { queryByText } = renderWithConfirmContextProvider( , @@ -88,7 +85,7 @@ describe('TypedSignInfo', () => { type: TransactionType.signTypedData, chainId: '0x5', }); - (snapUtils.isSnapId as unknown as jest.Mock).mockReturnValue(false); + (isSnapId as unknown as jest.Mock).mockReturnValue(false); const mockStore = configureMockStore([])(mockState); const { queryByText } = renderWithConfirmContextProvider( , diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign-v1/typed-sign-v1.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign-v1/typed-sign-v1.tsx index 5ebc5a9e0f56..6560c33b4491 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign-v1/typed-sign-v1.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign-v1/typed-sign-v1.tsx @@ -1,5 +1,6 @@ import React from 'react'; +import { isSnapId } from '@metamask/snaps-utils'; import { ConfirmInfoAlertRow } from '../../../../../../components/app/confirm/info/row/alert-row/alert-row'; import { ConfirmInfoRow, @@ -14,7 +15,6 @@ import { import { useConfirmContext } from '../../../../context/confirm'; import { ConfirmInfoRowTypedSignDataV1 } from '../../row/typed-sign-data-v1/typedSignDataV1'; import { ConfirmInfoSection } from '../../../../../../components/app/confirm/info/row/section'; -import { isSnapId } from '../../../../../../helpers/utils/snaps'; import { SigningInWithRow } from '../shared/sign-in-with-row/sign-in-with-row'; const TypedSignV1Info: React.FC = () => { diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.test.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.test.tsx index 640b663e5cc1..4c0ce81a076b 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.test.tsx @@ -5,6 +5,7 @@ import { TransactionType, } from '@metamask/transaction-controller'; +import { isSnapId } from '@metamask/snaps-utils'; import { getMockConfirmStateForTransaction, getMockTypedSignConfirmState, @@ -17,7 +18,6 @@ import { unapprovedTypedSignMsgV4, } from '../../../../../../../test/data/confirmations/typed_sign'; import { renderWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; -import * as snapUtils from '../../../../../../helpers/utils/snaps'; import TypedSignInfo from './typed-sign'; jest.mock( @@ -44,13 +44,10 @@ jest.mock('../../../../../../../node_modules/@metamask/snaps-utils', () => { ...originalUtils, stripSnapPrefix: jest.fn().mockReturnValue('@metamask/examplesnap'), getSnapPrefix: jest.fn().mockReturnValue('npm:'), + isSnapId: jest.fn(), }; }); -jest.mock('../../../../../../helpers/utils/snaps', () => ({ - isSnapId: jest.fn(), -})); - describe('TypedSignInfo', () => { it('renders origin for typed sign data request', () => { const state = getMockTypedSignConfirmState(); @@ -153,7 +150,7 @@ describe('TypedSignInfo', () => { type: TransactionType.signTypedData, chainId: '0x5', }); - (snapUtils.isSnapId as unknown as jest.Mock).mockReturnValue(true); + (isSnapId as unknown as jest.Mock).mockReturnValue(true); const mockStore = configureMockStore([])(mockState); const { queryByText } = renderWithConfirmContextProvider( , @@ -177,7 +174,7 @@ describe('TypedSignInfo', () => { type: TransactionType.signTypedData, chainId: '0x5', }); - (snapUtils.isSnapId as unknown as jest.Mock).mockReturnValue(false); + (isSnapId as unknown as jest.Mock).mockReturnValue(false); const mockStore = configureMockStore([])(mockState); const { queryByText } = renderWithConfirmContextProvider( , diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.tsx index d21ed2b1fca9..d1fb12bf675b 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { isValidAddress } from 'ethereumjs-util'; +import { isSnapId } from '@metamask/snaps-utils'; import { ConfirmInfoAlertRow } from '../../../../../../components/app/confirm/info/row/alert-row/alert-row'; import { parseTypedDataMessage } from '../../../../../../../shared/modules/transaction.utils'; import { RowAlertKey } from '../../../../../../components/app/confirm/info/row/constants'; @@ -21,7 +22,6 @@ import { import { useConfirmContext } from '../../../../context/confirm'; import { useTypesSignSimulationEnabledInfo } from '../../../../hooks/useTypesSignSimulationEnabledInfo'; import { ConfirmInfoRowTypedSignData } from '../../row/typed-sign-data/typedSignData'; -import { isSnapId } from '../../../../../../helpers/utils/snaps'; import { SigningInWithRow } from '../shared/sign-in-with-row/sign-in-with-row'; import { TypedSignV4Simulation } from './typed-sign-v4-simulation'; diff --git a/ui/pages/permissions-connect/permissions-connect.component.js b/ui/pages/permissions-connect/permissions-connect.component.js index 17f73d633953..f7dc90363363 100644 --- a/ui/pages/permissions-connect/permissions-connect.component.js +++ b/ui/pages/permissions-connect/permissions-connect.component.js @@ -3,6 +3,7 @@ import React, { Component } from 'react'; import { Switch, Route } from 'react-router-dom'; import { providerErrors, serializeError } from '@metamask/rpc-errors'; import { SubjectType } from '@metamask/permission-controller'; +import { isSnapId } from '@metamask/snaps-utils'; // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths import { isEthAddress } from '../../../app/scripts/lib/multichain/address'; @@ -19,7 +20,6 @@ import { // TODO: Remove restricted import // eslint-disable-next-line import/no-restricted-paths import { PermissionNames } from '../../../app/scripts/controllers/permissions'; -import { isSnapId } from '../../helpers/utils/snaps'; import ChooseAccount from './choose-account'; import PermissionsRedirect from './redirect'; import SnapsConnect from './snaps/snaps-connect'; diff --git a/ui/pages/permissions-connect/snaps/snap-install/snap-install.js b/ui/pages/permissions-connect/snaps/snap-install/snap-install.js index 93bdc3c5bd8f..0adeac87dfeb 100644 --- a/ui/pages/permissions-connect/snaps/snap-install/snap-install.js +++ b/ui/pages/permissions-connect/snaps/snap-install/snap-install.js @@ -1,6 +1,7 @@ import PropTypes from 'prop-types'; import React, { useCallback, useState } from 'react'; import { useSelector } from 'react-redux'; +import { isSnapId } from '@metamask/snaps-utils'; import { PageContainerFooter } from '../../../../components/ui/page-container'; import { useI18nContext } from '../../../../hooks/useI18nContext'; import SnapInstallWarning from '../../../../components/app/snaps/snap-install-warning'; @@ -34,7 +35,6 @@ import { useOriginMetadata } from '../../../../hooks/useOriginMetadata'; import { getSnapMetadata, getSnapsMetadata } from '../../../../selectors'; import { getSnapName } from '../../../../helpers/utils/util'; import PermissionConnectHeader from '../../../../components/app/permission-connect-header'; -import { isSnapId } from '../../../../helpers/utils/snaps'; export default function SnapInstall({ request, diff --git a/ui/pages/permissions-connect/snaps/snaps-connect/snaps-connect.js b/ui/pages/permissions-connect/snaps/snaps-connect/snaps-connect.js index b46eb070c3ed..645019848e25 100644 --- a/ui/pages/permissions-connect/snaps/snaps-connect/snaps-connect.js +++ b/ui/pages/permissions-connect/snaps/snaps-connect/snaps-connect.js @@ -1,6 +1,7 @@ import React, { useCallback, useState } from 'react'; import { useSelector } from 'react-redux'; import PropTypes from 'prop-types'; +import { isSnapId } from '@metamask/snaps-utils'; import { useI18nContext } from '../../../../hooks/useI18nContext'; import { Box, IconSize, Text } from '../../../../components/component-library'; import { @@ -26,7 +27,6 @@ import { getSnapMetadata, } from '../../../../selectors'; import { useOriginMetadata } from '../../../../hooks/useOriginMetadata'; -import { isSnapId } from '../../../../helpers/utils/snaps'; import { SnapIcon } from '../../../../components/app/snaps/snap-icon'; export default function SnapsConnect({ diff --git a/ui/pages/snaps/snap-view/snap-settings.js b/ui/pages/snaps/snap-view/snap-settings.js index 9db18ea7350e..2e8e02897abf 100644 --- a/ui/pages/snaps/snap-view/snap-settings.js +++ b/ui/pages/snaps/snap-view/snap-settings.js @@ -3,6 +3,7 @@ import { useDispatch, useSelector } from 'react-redux'; import PropTypes from 'prop-types'; import { useHistory } from 'react-router-dom'; import semver from 'semver'; +import { isSnapId } from '@metamask/snaps-utils'; import { useI18nContext } from '../../../hooks/useI18nContext'; import { BackgroundColor, @@ -51,7 +52,6 @@ import { DelineatorType } from '../../../helpers/constants/snaps'; import SnapUpdateAlert from '../../../components/app/snaps/snap-update-alert'; import { CONNECT_ROUTE } from '../../../helpers/constants/routes'; import { ShowMore } from '../../../components/app/snaps/show-more'; -import { isSnapId } from '../../../helpers/utils/snaps'; ///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps) import { KeyringSnapRemovalResultStatus } from './constants'; ///: END:ONLY_INCLUDE_IF From 4ee7e3f33faf439e90a6486c308eb6a6bda03b4a Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Mon, 6 Jan 2025 13:23:30 +0000 Subject: [PATCH 3/3] feat: add some authentication state to sentry logs (#29432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This adds some authentication controller state to sentry logs. We do not log sensitive and important info such as the `accessToken`. This should help with logging some of the service failures that rely on authentication. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29432?quickstart=1) ## **Related issues** Fixes: N/A ## **Manual testing steps** 1. Follow the steps in the [development readme](https://github.com/MetaMask/metamask-extension/blob/main/development/README.md#debugging-sentry) to invoke a sentry error - `DEBUG=metamask:sentry:*` and `METAMASK_DEBUG=1` can help to show the sentry state we are sending. ## **Screenshots/Recordings** ### **Before** ![Screenshot 2025-01-06 at 09 04 48](https://github.com/user-attachments/assets/0469fb5d-9eb3-4616-96f2-e863f2ddcb7c) ### **After** ![Screenshot 2025-01-06 at 11 28 23](https://github.com/user-attachments/assets/b81e6830-bb0f-4276-a345-0a0f7f097fdd) ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.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. --- app/scripts/constants/sentry-state.ts | 5 +++++ test/e2e/tests/metrics/errors.spec.js | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/app/scripts/constants/sentry-state.ts b/app/scripts/constants/sentry-state.ts index ab53ffb3f22d..705b33f450ba 100644 --- a/app/scripts/constants/sentry-state.ts +++ b/app/scripts/constants/sentry-state.ts @@ -42,6 +42,11 @@ export const SENTRY_BACKGROUND_STATE = { }, AuthenticationController: { isSignedIn: false, + sessionData: { + profile: true, + accessToken: false, + expiresIn: true, + }, }, NetworkOrderController: { orderedNetworkList: [], diff --git a/test/e2e/tests/metrics/errors.spec.js b/test/e2e/tests/metrics/errors.spec.js index 12a44e26fbf1..aaa5c0536485 100644 --- a/test/e2e/tests/metrics/errors.spec.js +++ b/test/e2e/tests/metrics/errors.spec.js @@ -904,6 +904,13 @@ describe('Sentry errors', function () { // the "resetState" method swapsFeatureFlags: true, }, + // Part of the AuthenticationController store, but initialized as undefined + // Only populated once the client is authenticated + sessionData: { + accessToken: false, + expiresIn: true, + profile: true, + }, // This can get erased due to a bug in the app state controller's // preferences state change handler timeoutMinutes: true,