chore: Cherry-pick resimulate into `V12.5.1` by OGPoyraz · Pull Request #28178 · MetaMask/metamask-extension

OGPoyraz · 2024-10-30T10:16:16Z

This PR cherry-picks 2c86162#diff-63ab7391d870a62d8bcd3cc5d5371432068538deb98e5effcb899434ed8649bb

This PR aims to add re-simulation logic which recently added at MetaMask/core#4792 Patch note: Transaction controller patch adds the re-simulate feature, branched belove to keep track. https://github.com/MetaMask/core/tree/patch/extension-transaction-controller-37-2-0 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28104?quickstart=1) Fixes: MetaMask/MetaMask-planning#3380 TBD   ![1](https://github.com/user-attachments/assets/67fc06d4-2f01-4e95-b1da-e84f5145462e) ![2](https://github.com/user-attachments/assets/52153a4a-4c0d-44bd-990b-51f9b90eefb4) - [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/develop/.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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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. --------- Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>

OGPoyraz · 2024-10-30T10:16:34Z

@metamaskbot update-policies

socket-security · 2024-10-30T10:17:46Z

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

metamaskbot · 2024-10-30T10:17:55Z

Policy update failed. You can review the logs or retry the policy update here

OGPoyraz · 2024-10-30T10:19:38Z

@metamaskbot update-policies

metamaskbot · 2024-10-30T10:27:42Z

Policy update failed. You can review the logs or retry the policy update here

yarn.lock

OGPoyraz · 2024-10-30T11:56:26Z

@metamaskbot update-policies

socket-security · 2024-10-30T11:57:25Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/base-controller@7.0.2	None	`+1`	1.06 MB	metamaskbot
npm/@metamask/controller-utils@11.4.1	network	`0`	264 kB	metamaskbot

🚮 Removed packages: npm/@metamask/base-controller@7.0.1, npm/@metamask/controller-utils@11.3.0

View full report↗︎

metamaskbot · 2024-10-30T12:07:46Z

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

Upgrade the `TransactionController` to: - Support Etherscan API keys when polling for incoming transactions. - Populate `submitHistory` to aid with debug and persist even when resetting the account. Also adds the `ETHERSCAN_API_KEY` environment variable. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27611?quickstart=1) 1. Verify incoming transactions work on Mainnet and Sepolia with an API key set. 2. Verify `submitHistory` is populated in state logs after creating transactions and retrying, on both Infura and custom networks. - [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/develop/.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/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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.

metamaskbot · 2024-10-30T19:17:40Z

Builds ready [d213294]

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 (1808 ± 89 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1595	2297	1813	189	91
		domContentLoaded	1586	2162	1782	169	81
		load	1595	2301	1808	186	89
		domInteractive	13	195	55	51	25

danjm · 2024-10-30T20:10:25Z

app/scripts/metamask-controller.js

@@ -1849,6 +1849,10 @@ export default class MetamaskController extends EventEmitter {
          getCurrentChainId({ metamask: this.networkController.state })
        ],
      incomingTransactions: {


Why is this change needed here, but it is not on the commit being cherry-picked?

They were added in this PR: #27611

It was an earlier bump of the transaction controller. This cherry-pick includes both PRs

danjm · 2024-10-30T20:11:45Z

builds.yml

@@ -272,8 +272,10 @@ env:
  - SECURITY_ALERTS_API_ENABLED: ''
  # URL of security alerts API used to validate dApp requests
  - SECURITY_ALERTS_API_URL: 'http://localhost:3000'


Why is this change not in the commit being cherry-picked?

Same answer as here: https://github.com/MetaMask/metamask-extension/pull/28178/files#r1823521433

Gudahtt · 2024-10-30T22:32:02Z

app/scripts/lib/transaction/smart-transactions.ts

      signedCanceledTransactions: [],
      txParams: this.#txParams,
-      transactionMeta: this.#transactionMeta,
+      // TODO: Replace `any` with type - version mismatch between smart-transactions-controller and transaction-controller breaking type safety


Any idea what the type incompatibilities are here? The SmartTransactionsController is using types from v34.0.0 of the TransactionController, and I didn't see any breaking changes in the changelog that might explain this.

Ah, looks like it was this: MetaMask/core#4714

The additional two types aren't expected by the SmartTransactionsController. It looks like the SmartTransactionsController doesn't have any matchers for this property, so it should not cause any problems there.

Gudahtt · 2024-10-30T22:40:47Z

app/scripts/lib/transaction/smart-transactions.ts

+      // TODO: Replace `any` with type - version mismatch between smart-transactions-controller and transaction-controller breaking type safety
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any


Nit: @ts-expect-error is preferred because it will flag when the override is no longer needed

See https://github.com/MetaMask/contributor-docs/blob/main/docs/typescript.md#if-accompanied-by-a-todo-comment-ts-expect-error-is-acceptable-to-use-for-marking-errors-that-have-clear-plans-of-being-resolved for more details

Suggested change

// TODO: Replace `any` with type - version mismatch between smart-transactions-controller and transaction-controller breaking type safety

// eslint-disable-next-line @typescript-eslint/no-explicit-any

// @ts-expect-error version mismatch between smart-transactions-controller and transaction-

// controller breaking type safety

Gudahtt · 2024-10-30T22:48:13Z

test/e2e/tests/simulation-details/mock-request-buy-erc721.ts

        },
        {
-          return: `0x00000000000000000000000000000000${SENDER_ADDRESS_NO_0X_MOCK}`,
+          return: `0x000000000000000000000000${SENDER_ADDRESS_NO_0X_MOCK}`,


Looks like this is from yet another separate transaction controller bump: #27954

Gudahtt · 2024-10-30T22:50:32Z

ui/index.js

    // for more info)
    state.version = global.platform.getVersion();
    state.browser = window.navigator.userAgent;
-    state.completeTxList = await actions.getTransactions({


Reasons not entirely clear to me, but this appears to be from #27611

Gudahtt

LGTM!

OGPoyraz requested review from a team as code owners October 30, 2024 10:16

github-actions bot added the team-confirmations Push issues to confirmations team label Oct 30, 2024

metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Oct 30, 2024

Update yarn lock

e76296d

OGPoyraz commented Oct 30, 2024

View reviewed changes

yarn.lock Show resolved Hide resolved

Update yarn lock

814ed39

Update LavaMoat policies

2b926af

OGPoyraz and others added 5 commits October 30, 2024 14:44

Update snapshot

ae8b6fe

Update snapshot

45ae115

Fix lint

d91301b

Fix abi e2e tests

0e902e2

danjm reviewed Oct 30, 2024

View reviewed changes

Gudahtt reviewed Oct 30, 2024

View reviewed changes

Gudahtt approved these changes Oct 30, 2024

View reviewed changes

danjm approved these changes Oct 31, 2024

View reviewed changes

danjm merged commit 5dbbda3 into Version-v12.5.1 Oct 31, 2024

danjm deleted the chore/pick-resimulate branch October 31, 2024 02:46

github-actions bot locked and limited conversation to collaborators Oct 31, 2024

		// TODO: Replace `any` with type - version mismatch between smart-transactions-controller and transaction-controller breaking type safety
		// eslint-disable-next-line @typescript-eslint/no-explicit-any

Conversation

OGPoyraz commented Oct 30, 2024

Uh oh!

OGPoyraz commented Oct 30, 2024

Uh oh!

socket-security bot commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metamaskbot commented Oct 30, 2024

Uh oh!

OGPoyraz commented Oct 30, 2024

Uh oh!

metamaskbot commented Oct 30, 2024

Uh oh!

Uh oh!

OGPoyraz commented Oct 30, 2024

Uh oh!

socket-security bot commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metamaskbot commented Oct 30, 2024

Uh oh!

metamaskbot commented Oct 30, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gudahtt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

socket-security bot commented Oct 30, 2024 •

edited

Loading

socket-security bot commented Oct 30, 2024 •

edited

Loading