From 6f11eda56785b6ca1bd253a6fc6a3498eef5bc5f Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 20 Dec 2024 16:13:27 -0330 Subject: [PATCH 1/3] ci: Migrate lint CI steps (#29371) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Migrate lint steps from CircleCI to GitHub Actions. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29371?quickstart=1) ## **Related issues** Relates to #28572 These changes were extracted from #29256 ## **Manual testing steps** Branch from here, create a new draft PR, Introduce lint errors, then ensure the jobs fail. https://github.com/MetaMask/metamask-extension/pull/29390 ## **Screenshots/Recordings** N/A ## **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. --- .circleci/config.yml | 82 ---------------------- .github/workflows/main.yml | 20 ++++++ .github/workflows/test-lint-changelog.yml | 23 ++++++ .github/workflows/test-lint-lockfile.yml | 21 ++++++ .github/workflows/test-lint-shellcheck.yml | 15 ++++ .github/workflows/test-lint.yml | 21 ++++++ 6 files changed, 100 insertions(+), 82 deletions(-) create mode 100644 .github/workflows/test-lint-changelog.yml create mode 100644 .github/workflows/test-lint-lockfile.yml create mode 100644 .github/workflows/test-lint-shellcheck.yml create mode 100644 .github/workflows/test-lint.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index 5598e6450cfe..60bb80eaf449 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -25,10 +25,6 @@ executors: resource_class: medium+ environment: NODE_OPTIONS: --max_old_space_size=4096 - shellcheck: - docker: - - image: koalaman/shellcheck-alpine@sha256:dfaf08fab58c158549d3be64fb101c626abc5f16f341b569092577ae207db199 - resource_class: small playwright: docker: - image: mcr.microsoft.com/playwright:v1.44.1-focal @@ -184,16 +180,6 @@ workflows: - prep-build-ts-migration-dashboard: requires: - prep-deps - - test-lint: - requires: - - prep-deps - - test-lint-shellcheck - - test-lint-lockfile: - requires: - - prep-deps - - test-lint-changelog: - requires: - - prep-deps - test-e2e-chrome-webpack: <<: *main_master_rc_only requires: @@ -285,10 +271,6 @@ workflows: - validate-lavamoat-allow-scripts - validate-lavamoat-policy-build - validate-lavamoat-policy-webapp - - test-lint - - test-lint-shellcheck - - test-lint-lockfile - - test-lint-changelog - validate-source-maps - validate-source-maps-beta - validate-source-maps-flask @@ -954,20 +936,6 @@ jobs: name: Rerun workflows from failed command: yarn ci-rerun-from-failed - test-lint: - executor: node-browsers-medium - steps: - - run: *shallow-git-clone-and-enable-vnc - - run: sudo corepack enable - - attach_workspace: - at: . - - run: - name: Lint - command: yarn lint - - run: - name: Verify locales - command: yarn verify-locales --quiet - test-storybook: executor: node-browsers-medium-plus steps: @@ -982,56 +950,6 @@ jobs: name: Test Storybook command: yarn test-storybook:ci - test-lint-shellcheck: - executor: shellcheck - steps: - - checkout - - run: apk add --no-cache bash jq yarn - - run: - name: ShellCheck Lint - command: ./development/shellcheck.sh - - test-lint-lockfile: - executor: node-browsers-medium-plus - steps: - - run: *shallow-git-clone-and-enable-vnc - - run: sudo corepack enable - - attach_workspace: - at: . - - run: - name: lockfile-lint - command: yarn lint:lockfile - - run: - name: check yarn resolutions - command: yarn --check-resolutions - - test-lint-changelog: - executor: node-browsers-small - steps: - - run: *shallow-git-clone-and-enable-vnc - - run: sudo corepack enable - - attach_workspace: - at: . - - when: - condition: - not: - matches: - pattern: /^Version-v(\d+)[.](\d+)[.](\d+)$/ - value: << pipeline.git.branch >> - steps: - - run: - name: Validate changelog - command: yarn lint:changelog - - when: - condition: - matches: - pattern: /^Version-v(\d+)[.](\d+)[.](\d+)$/ - value: << pipeline.git.branch >> - steps: - - run: - name: Validate release candidate changelog - command: .circleci/scripts/validate-changelog-in-rc.sh - test-e2e-chrome-webpack: executor: node-browsers-medium-plus parallelism: 20 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d8850502e7da..ee29c54e94ac 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -28,6 +28,22 @@ jobs: run: ${{ steps.download-actionlint.outputs.executable }} -color shell: bash + test-lint-shellcheck: + name: Test lint shellcheck + uses: ./.github/workflows/test-lint-shellcheck.yml + + test-lint: + name: Test lint + uses: ./.github/workflows/test-lint.yml + + test-lint-changelog: + name: Test lint changelog + uses: ./.github/workflows/test-lint-changelog.yml + + test-lint-lockfile: + name: Test lint lockfile + uses: ./.github/workflows/test-lint-lockfile.yml + test-deps-audit: name: Test deps audit uses: ./.github/workflows/test-deps-audit.yml @@ -53,6 +69,10 @@ jobs: runs-on: ubuntu-latest needs: - check-workflows + - test-lint-shellcheck + - test-lint + - test-lint-changelog + - test-lint-lockfile - test-yarn-dedupe - test-deps-depcheck - run-tests diff --git a/.github/workflows/test-lint-changelog.yml b/.github/workflows/test-lint-changelog.yml new file mode 100644 index 000000000000..66c0219551f4 --- /dev/null +++ b/.github/workflows/test-lint-changelog.yml @@ -0,0 +1,23 @@ +name: Test lint changelog + +on: + workflow_call: + +jobs: + test-lint-changelog: + name: Test lint changelog + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup environment + uses: metamask/github-tools/.github/actions/setup-environment@main + + - name: Validate changelog + if: ${{ !startsWith(github.head_ref || github.ref_name, 'Version-v') }} + run: yarn lint:changelog + + - name: Validate release candidate changelog + if: ${{ startsWith(github.head_ref || github.ref_name, 'Version-v') }} + run: .circleci/scripts/validate-changelog-in-rc.sh diff --git a/.github/workflows/test-lint-lockfile.yml b/.github/workflows/test-lint-lockfile.yml new file mode 100644 index 000000000000..cc84318624ce --- /dev/null +++ b/.github/workflows/test-lint-lockfile.yml @@ -0,0 +1,21 @@ +name: Test lint lockfile + +on: + workflow_call: + +jobs: + test-lint-lockfile: + name: Test lint lockfile + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup environment + uses: metamask/github-tools/.github/actions/setup-environment@main + + - name: Lint lockfile + run: yarn lint:lockfile + + - name: Check yarn resolutions + run: yarn --check-resolutions diff --git a/.github/workflows/test-lint-shellcheck.yml b/.github/workflows/test-lint-shellcheck.yml new file mode 100644 index 000000000000..c4127902a2f4 --- /dev/null +++ b/.github/workflows/test-lint-shellcheck.yml @@ -0,0 +1,15 @@ +name: Test lint shellcheck + +on: + workflow_call: + +jobs: + test-lint-shellcheck: + name: Test lint shellcheck + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: ShellCheck Lint + run: ./development/shellcheck.sh diff --git a/.github/workflows/test-lint.yml b/.github/workflows/test-lint.yml new file mode 100644 index 000000000000..df40a3a7ef27 --- /dev/null +++ b/.github/workflows/test-lint.yml @@ -0,0 +1,21 @@ +name: Test lint + +on: + workflow_call: + +jobs: + test-lint: + name: Test lint + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup environment + uses: metamask/github-tools/.github/actions/setup-environment@main + + - name: Lint + run: yarn lint + + - name: Verify locales + run: yarn verify-locales --quiet From ce8b502529b1131c65506d23a6ec7b5f68c5b526 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 20 Dec 2024 17:23:21 -0330 Subject: [PATCH 2/3] ci: Migrate LavaMoat validation to GitHub Actions (#29369) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Migrate LavaMoat policy validation from CircleCI to GitHub actions. No functional changes. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29369?quickstart=1) ## **Related issues** Relates to #28572 These changes were extracted from #29256 ## **Manual testing steps** * Checkout this branch (`migrate-lavamoat-validation`), then from there create a new branch to test with * On this new branch, make a dependency change with a policy impact (e.g. add or remove a package, upgrade something, etc.), but make sure the build still passes (validation requires a passing build) * Create a draft PR, and verify that the policy validation fails * Use the `metamaskbot update-policies` bot command to update the policies, then verify the validation now succeeds. PR with errors - https://github.com/MetaMask/metamask-extension/pull/29396 Failure - https://github.com/MetaMask/metamask-extension/actions/runs/12434996100/job/34719873040?pr=29396 Passing - https://github.com/MetaMask/metamask-extension/actions/runs/12435253146/job/34720674397?pr=29396 ## **Screenshots/Recordings** N/A ## **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. --- .circleci/config.yml | 60 ------------------- .github/workflows/main.yml | 15 +++++ .../validate-lavamoat-allow-scripts.yml | 25 ++++++++ .../validate-lavamoat-policy-build.yml | 27 +++++++++ .../validate-lavamoat-policy-webapp.yml | 30 ++++++++++ 5 files changed, 97 insertions(+), 60 deletions(-) create mode 100644 .github/workflows/validate-lavamoat-allow-scripts.yml create mode 100644 .github/workflows/validate-lavamoat-policy-build.yml create mode 100644 .github/workflows/validate-lavamoat-policy-webapp.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index 60bb80eaf449..f800ab484ebe 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -123,18 +123,6 @@ workflows: - master requires: - prep-deps - - validate-lavamoat-allow-scripts: - requires: - - prep-deps - - validate-lavamoat-policy-build: - requires: - - prep-deps - - validate-lavamoat-policy-webapp: - matrix: - parameters: - build-type: [main, beta, flask, mmi] - requires: - - prep-deps - prep-build-mmi: requires: - prep-deps @@ -268,9 +256,6 @@ workflows: - prep-build-flask-mv2 - all-tests-pass: requires: - - validate-lavamoat-allow-scripts - - validate-lavamoat-policy-build - - validate-lavamoat-policy-webapp - validate-source-maps - validate-source-maps-beta - validate-source-maps-flask @@ -481,51 +466,6 @@ jobs: at: . - run: yarn tsx .circleci/scripts/validate-locales-only.ts - validate-lavamoat-allow-scripts: - executor: node-browsers-small - steps: - - run: *shallow-git-clone-and-enable-vnc - - run: sudo corepack enable - - attach_workspace: - at: . - - run: - name: Validate allow-scripts config - command: yarn allow-scripts auto - - run: - name: Check working tree - command: .circleci/scripts/check-working-tree.sh - - validate-lavamoat-policy-build: - executor: node-browsers-medium - steps: - - run: *shallow-git-clone-and-enable-vnc - - run: sudo corepack enable - - attach_workspace: - at: . - - run: - name: Validate LavaMoat build policy - command: yarn lavamoat:build:auto - - run: - name: Check working tree - command: .circleci/scripts/check-working-tree.sh - - validate-lavamoat-policy-webapp: - executor: node-browsers-medium-plus - parameters: - build-type: - type: string - steps: - - run: *shallow-git-clone-and-enable-vnc - - run: sudo corepack enable - - attach_workspace: - at: . - - run: - name: Validate LavaMoat << parameters.build-type >> policy - command: yarn lavamoat:webapp:auto:ci '--build-types=<< parameters.build-type >>' - - run: - name: Check working tree - command: .circleci/scripts/check-working-tree.sh - prep-build: executor: node-linux-medium steps: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ee29c54e94ac..6e5a5121f336 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -56,6 +56,18 @@ jobs: name: Test deps depcheck uses: ./.github/workflows/test-deps-depcheck.yml + validate-lavamoat-allow-scripts: + name: Validate lavamoat allow scripts + uses: ./.github/workflows/validate-lavamoat-allow-scripts.yml + + validate-lavamoat-policy-build: + name: Validate lavamoat policy build + uses: ./.github/workflows/validate-lavamoat-policy-build.yml + + validate-lavamoat-policy-webapp: + name: Validate lavamoat policy webapp + uses: ./.github/workflows/validate-lavamoat-policy-webapp.yml + run-tests: name: Run tests uses: ./.github/workflows/run-tests.yml @@ -75,6 +87,9 @@ jobs: - test-lint-lockfile - test-yarn-dedupe - test-deps-depcheck + - validate-lavamoat-allow-scripts + - validate-lavamoat-policy-build + - validate-lavamoat-policy-webapp - run-tests - wait-for-circleci-workflow-status outputs: diff --git a/.github/workflows/validate-lavamoat-allow-scripts.yml b/.github/workflows/validate-lavamoat-allow-scripts.yml new file mode 100644 index 000000000000..637a2d9aeb54 --- /dev/null +++ b/.github/workflows/validate-lavamoat-allow-scripts.yml @@ -0,0 +1,25 @@ +name: Validate lavamoat allow scripts + +on: + workflow_call: + +jobs: + validate-lavamoat-allow-scripts: + name: Validate lavamoat allow scripts + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup environment + uses: metamask/github-tools/.github/actions/setup-environment@main + + - name: Validate allow-scripts config + run: yarn allow-scripts auto + + - name: Check working tree + run: | + if ! git diff --exit-code; then + echo "::error::Working tree dirty." + exit 1 + fi diff --git a/.github/workflows/validate-lavamoat-policy-build.yml b/.github/workflows/validate-lavamoat-policy-build.yml new file mode 100644 index 000000000000..4524cc26a546 --- /dev/null +++ b/.github/workflows/validate-lavamoat-policy-build.yml @@ -0,0 +1,27 @@ +name: Validate lavamoat policy build + +on: + workflow_call: + +jobs: + validate-lavamoat-policy-build: + name: Validate lavamoat policy build + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup environment + uses: metamask/github-tools/.github/actions/setup-environment@main + + - name: Validate lavamoat build policy + run: yarn lavamoat:build:auto + env: + INFURA_PROJECT_ID: 00000000000 + + - name: Check working tree + run: | + if ! git diff --exit-code; then + echo "::error::Working tree dirty." + exit 1 + fi diff --git a/.github/workflows/validate-lavamoat-policy-webapp.yml b/.github/workflows/validate-lavamoat-policy-webapp.yml new file mode 100644 index 000000000000..37ff9ede00fc --- /dev/null +++ b/.github/workflows/validate-lavamoat-policy-webapp.yml @@ -0,0 +1,30 @@ +name: Validate lavamoat policy webapp + +on: + workflow_call: + +jobs: + validate-lavamoat-policy-webapp: + name: Validate lavamoat policy webapp + runs-on: ubuntu-latest + strategy: + matrix: + build-type: [main, beta, flask, mmi] + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup environment + uses: metamask/github-tools/.github/actions/setup-environment@main + + - name: Validate lavamoat ${{ matrix.build-type }} policy + run: yarn lavamoat:webapp:auto:ci --build-types=${{ matrix.build-type }} + env: + INFURA_PROJECT_ID: 00000000000 + + - name: Check working tree + run: | + if ! git diff --exit-code; then + echo "::error::Working tree dirty." + exit 1 + fi From f64a2d003e6e69be0305c43534334461fadc6e7e Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Fri, 20 Dec 2024 20:55:52 +0000 Subject: [PATCH 3/3] fix: hide first interaction alert if token transfer recipient is internal account (#29389) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Hide the first-time interaction alert if the transaction is a token transfer, and the recipient is an internal account. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29389?quickstart=1) ## **Related issues** Fixes: #29225 ## **Manual testing steps** Create token transfer to internal account and verify no alert is displayed. ## **Screenshots/Recordings** ### **Before** ### **After** ## **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. --- .../info/hooks/useTransferRecipient.test.ts | 74 +++++++++++++++++++ .../info/hooks/useTransferRecipient.ts | 20 +++++ .../transaction-flow-section.tsx | 15 +--- .../useFirstTimeInteractionAlert.test.ts | 67 ++++++++++------- .../useFirstTimeInteractionAlert.ts | 6 +- 5 files changed, 141 insertions(+), 41 deletions(-) create mode 100644 ui/pages/confirmations/components/confirm/info/hooks/useTransferRecipient.test.ts create mode 100644 ui/pages/confirmations/components/confirm/info/hooks/useTransferRecipient.ts diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useTransferRecipient.test.ts b/ui/pages/confirmations/components/confirm/info/hooks/useTransferRecipient.test.ts new file mode 100644 index 000000000000..26d007ba148b --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/hooks/useTransferRecipient.test.ts @@ -0,0 +1,74 @@ +import { + TransactionMeta, + TransactionType, +} from '@metamask/transaction-controller'; +import { renderHookWithConfirmContextProvider } from '../../../../../../../test/lib/confirmations/render-helpers'; +import { getMockConfirmStateForTransaction } from '../../../../../../../test/data/confirmations/helper'; +import { genUnapprovedContractInteractionConfirmation } from '../../../../../../../test/data/confirmations/contract-interaction'; +import { genUnapprovedTokenTransferConfirmation } from '../../../../../../../test/data/confirmations/token-transfer'; +import { useTransferRecipient } from './useTransferRecipient'; + +const ADDRESS_MOCK = '0x2e0D7E8c45221FcA00d74a3609A0f7097035d09B'; +const ADDRESS_2_MOCK = '0x2e0D7E8c45221FcA00d74a3609A0f7097035d09C'; + +const TRANSACTION_METADATA_MOCK = + genUnapprovedContractInteractionConfirmation() as TransactionMeta; + +function runHook(transaction?: TransactionMeta) { + const state = transaction + ? getMockConfirmStateForTransaction(transaction) + : {}; + + const { result } = renderHookWithConfirmContextProvider( + useTransferRecipient, + state, + ); + + return result.current as string | undefined; +} + +describe('useTransferRecipient', () => { + it('returns undefined if no transaction', () => { + expect(runHook()).toBeUndefined(); + }); + + it('returns parameter to address if simple send', () => { + expect( + runHook({ + ...TRANSACTION_METADATA_MOCK, + type: TransactionType.simpleSend, + txParams: { + ...TRANSACTION_METADATA_MOCK.txParams, + to: ADDRESS_MOCK, + }, + }), + ).toBe(ADDRESS_MOCK); + }); + + it('returns data to address if token data', () => { + expect( + runHook({ + ...TRANSACTION_METADATA_MOCK, + txParams: { + ...TRANSACTION_METADATA_MOCK.txParams, + to: ADDRESS_2_MOCK, + data: genUnapprovedTokenTransferConfirmation().txParams.data, + }, + }), + ).toBe(ADDRESS_MOCK); + }); + + it('returns parameter to address if token data but type is simple send', () => { + expect( + runHook({ + ...TRANSACTION_METADATA_MOCK, + type: TransactionType.simpleSend, + txParams: { + ...TRANSACTION_METADATA_MOCK.txParams, + to: ADDRESS_2_MOCK, + data: genUnapprovedTokenTransferConfirmation().txParams.data, + }, + }), + ).toBe(ADDRESS_2_MOCK); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useTransferRecipient.ts b/ui/pages/confirmations/components/confirm/info/hooks/useTransferRecipient.ts new file mode 100644 index 000000000000..fcfd98fedaf4 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/hooks/useTransferRecipient.ts @@ -0,0 +1,20 @@ +import { + TransactionMeta, + TransactionType, +} from '@metamask/transaction-controller'; +import { useConfirmContext } from '../../../../context/confirm'; +import { useTokenTransactionData } from './useTokenTransactionData'; + +export function useTransferRecipient() { + const { currentConfirmation: transactionMetadata } = + useConfirmContext(); + + const transactionData = useTokenTransactionData(); + const transactionType = transactionMetadata?.type; + const transactionTo = transactionMetadata?.txParams?.to; + const transferTo = transactionData?.args?._to as string | undefined; + + return transactionType === TransactionType.simpleSend + ? transactionTo + : transferTo; +} diff --git a/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.tsx b/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.tsx index fe9b9f319c9f..5ed2b103b809 100644 --- a/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.tsx +++ b/ui/pages/confirmations/components/confirm/info/token-transfer/transaction-flow-section.tsx @@ -1,7 +1,4 @@ -import { - TransactionMeta, - TransactionType, -} from '@metamask/transaction-controller'; +import { TransactionMeta } from '@metamask/transaction-controller'; import React from 'react'; import { ConfirmInfoSection } from '../../../../../../components/app/confirm/info/row/section'; import { @@ -22,7 +19,7 @@ import { ConfirmInfoAlertRow } from '../../../../../../components/app/confirm/in import { RowAlertKey } from '../../../../../../components/app/confirm/info/row/constants'; import { useI18nContext } from '../../../../../../hooks/useI18nContext'; import { useConfirmContext } from '../../../../context/confirm'; -import { useTokenTransactionData } from '../hooks/useTokenTransactionData'; +import { useTransferRecipient } from '../hooks/useTransferRecipient'; export const TransactionFlowSection = () => { const t = useI18nContext(); @@ -30,13 +27,7 @@ export const TransactionFlowSection = () => { const { currentConfirmation: transactionMeta } = useConfirmContext(); - const parsedTransactionData = useTokenTransactionData(); - - const recipientAddress = - transactionMeta.type === TransactionType.simpleSend - ? transactionMeta.txParams.to - : parsedTransactionData?.args?._to; - + const recipientAddress = useTransferRecipient(); const { chainId } = transactionMeta; return ( diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts index 93da09a9674e..7fe4ceb1d2e2 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.test.ts @@ -1,17 +1,19 @@ -import { ApprovalType } from '@metamask/controller-utils'; import { TransactionMeta, TransactionStatus, TransactionType, } from '@metamask/transaction-controller'; -import { getMockConfirmState } from '../../../../../../test/data/confirmations/helper'; +import { getMockConfirmStateForTransaction } from '../../../../../../test/data/confirmations/helper'; import { renderHookWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers'; import { Severity } from '../../../../../helpers/constants/design-system'; import { RowAlertKey } from '../../../../../components/app/confirm/info/row/constants'; +import { genUnapprovedTokenTransferConfirmation } from '../../../../../../test/data/confirmations/token-transfer'; import { useFirstTimeInteractionAlert } from './useFirstTimeInteractionAlert'; -const ACCOUNT_ADDRESS = '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'; +const ACCOUNT_ADDRESS_MOCK = '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'; +const ACCOUNT_ADDRESS_2_MOCK = '0x2e0d7e8c45221fca00d74a3609a0f7097035d09b'; +const CONTRACT_ADDRESS_MOCK = '0x0dcd5d886577d5081b0c52e242ef29e70be3e7be'; const TRANSACTION_ID_MOCK = '123-456'; const TRANSACTION_META_MOCK = { @@ -21,7 +23,7 @@ const TRANSACTION_META_MOCK = { status: TransactionStatus.unapproved, type: TransactionType.contractInteraction, txParams: { - from: ACCOUNT_ADDRESS, + from: ACCOUNT_ADDRESS_MOCK, }, time: new Date().getTime() - 10000, } as TransactionMeta; @@ -33,28 +35,20 @@ function runHook({ currentConfirmation?: TransactionMeta; internalAccountAddresses?: string[]; } = {}) { - const pendingApprovals = currentConfirmation - ? { - [currentConfirmation.id as string]: { - id: currentConfirmation.id, - type: ApprovalType.Transaction, - }, - } - : {}; - - const transactions = currentConfirmation ? [currentConfirmation] : []; - const internalAccounts = { accounts: internalAccountAddresses?.map((address) => ({ address })) ?? [], }; - const state = getMockConfirmState({ - metamask: { - internalAccounts, - pendingApprovals, - transactions, - }, - }); + const state = currentConfirmation + ? getMockConfirmStateForTransaction( + currentConfirmation as TransactionMeta, + { + metamask: { + internalAccounts, + }, + }, + ) + : {}; const response = renderHookWithConfirmContextProvider( useFirstTimeInteractionAlert, @@ -101,15 +95,35 @@ describe('useFirstTimeInteractionAlert', () => { const firstTimeConfirmation = { ...TRANSACTION_META_MOCK, isFirstTimeInteraction: true, + type: TransactionType.simpleSend, + txParams: { + ...TRANSACTION_META_MOCK.txParams, + to: ACCOUNT_ADDRESS_2_MOCK, + }, + }; + expect( + runHook({ + currentConfirmation: firstTimeConfirmation, + internalAccountAddresses: [ACCOUNT_ADDRESS_2_MOCK], + }), + ).toEqual([]); + }); + + it('returns no alerts if token transfer recipient is internal account', () => { + const firstTimeConfirmation = { + ...TRANSACTION_META_MOCK, + isFirstTimeInteraction: true, + type: TransactionType.tokenMethodTransfer, txParams: { ...TRANSACTION_META_MOCK.txParams, - to: ACCOUNT_ADDRESS, + to: CONTRACT_ADDRESS_MOCK, + data: genUnapprovedTokenTransferConfirmation().txParams.data, }, }; expect( runHook({ currentConfirmation: firstTimeConfirmation, - internalAccountAddresses: [ACCOUNT_ADDRESS], + internalAccountAddresses: [ACCOUNT_ADDRESS_2_MOCK], }), ).toEqual([]); }); @@ -118,15 +132,16 @@ describe('useFirstTimeInteractionAlert', () => { const firstTimeConfirmation = { ...TRANSACTION_META_MOCK, isFirstTimeInteraction: true, + type: TransactionType.simpleSend, txParams: { ...TRANSACTION_META_MOCK.txParams, - to: ACCOUNT_ADDRESS.toLowerCase(), + to: ACCOUNT_ADDRESS_2_MOCK.toLowerCase(), }, }; expect( runHook({ currentConfirmation: firstTimeConfirmation, - internalAccountAddresses: [ACCOUNT_ADDRESS.toUpperCase()], + internalAccountAddresses: [ACCOUNT_ADDRESS_2_MOCK.toUpperCase()], }), ).toEqual([]); }); diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts index c74552575667..f83f5d1ce30e 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useFirstTimeInteractionAlert.ts @@ -8,14 +8,14 @@ import { Severity } from '../../../../../helpers/constants/design-system'; import { RowAlertKey } from '../../../../../components/app/confirm/info/row/constants'; import { useConfirmContext } from '../../../context/confirm'; import { getInternalAccounts } from '../../../../../selectors'; +import { useTransferRecipient } from '../../../components/confirm/info/hooks/useTransferRecipient'; export function useFirstTimeInteractionAlert(): Alert[] { const t = useI18nContext(); const { currentConfirmation } = useConfirmContext(); const internalAccounts = useSelector(getInternalAccounts); - - const { txParams, isFirstTimeInteraction } = currentConfirmation ?? {}; - const { to } = txParams ?? {}; + const to = useTransferRecipient(); + const { isFirstTimeInteraction } = currentConfirmation ?? {}; const isInternalAccount = internalAccounts.some( (account) => account.address?.toLowerCase() === to?.toLowerCase(),