Merge branch 'develop' into e2e/settings-about-metamask-links-test

.github/CONTRIBUTING.md

@@ -15,12 +15,26 @@ When you're done with your project / bugfix / feature and ready to submit a PR,
 
 - [ ] **Make sure you followed our [`coding guidelines`](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md)**: These guidelines aim to maintain consistency and readability across the codebase. They help ensure that the code is easy to understand, maintain, and modify, which is particularly important when working with multiple contributors.
 - [ ] **Test it**: For any new programmatic functionality, we like unit tests when possible, so if you can keep your code cleanly isolated, please do add a test file to the `tests` folder.
-- [ ] **Add to the CHANGELOG**: Help us keep track of all the moving pieces by adding an entry to the [`CHANGELOG.md`](https://github.com/MetaMask/metamask-extension/blob/develop/CHANGELOG.md) with a link to your PR.
 - [ ] **Meet the spec**: Make sure the PR adds functionality that matches the issue you're closing. This is especially important for bounties: sometimes design or implementation details are included in the conversation, so read carefully!
-- [ ] **Close the issue**: If this PR closes an open issue, add the line `Fixes #$ISSUE_NUMBER`. Ex. For closing issue 418, include the line `Fixes #418`. If it doesn't close the issue but addresses it partially, just include a reference to the issue number, like `#418`.
+- [ ] **Close the issue**: If this PR closes an open issue, fill out the "Related issues" section with `Fixes: #$ISSUE_NUMBER`. Ex. For closing issue 418, include the line `Fixes: #418`. If it doesn't close the issue but addresses it partially, just include a reference to the issue number, like `#418`.
 - [ ] **Keep it simple**: Try not to include multiple features in a single PR, and don't make extraneous changes outside the scope of your contribution. All those touched files make things harder to review ;)
 - [ ] **PR against `develop`**: Submit your PR against the `develop` branch. This is where we merge new features so they get some time to receive extra testing before being pushed to `master` for production. If your PR is a hot-fix that needs to be published urgently, you may submit a PR against the `master` branch, but this PR will receive tighter scrutiny before merging.
-- [ ] **Get reviewed by a core contributor**: Make sure you get a `:thumbsup`, `:+1`, or `LGTM` from a user with a `Member` badge before merging.
-- [ ] **Ensure the PR is correctly labeled.**: More detail about labels definitions can be found [here](https://github.com/MetaMask/metamask-extension/blob/develop/.github/LABELING_GUIDELINES.md).
+- [ ] **Get reviewed by MetaMask Internal Developers**: All PRs require 2 approvals from MetaMask Internal Developers before merging.
+- [ ] **Ensure the PR is correctly labeled.**: More detail about PR labels can be found [here](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md).
+- [ ] **PR Titles**: Must adhere to the [Conventional Commits specification](https://www.conventionalcommits.org)
+  - `<type>[optional scope]: <description>`
+  - Example: `feat(parser): add ability to parse arrays`
+  - Available types:
+    - feat: A new feature
+    - fix: A bug fix
+    - docs: Documentation only changes
+    - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
+    - refactor: A code change that neither fixes a bug nor adds a feature
+    - perf: A code change that improves performance
+    - test: Adding missing tests or correcting existing tests
+    - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
+    - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
+    - chore: Other changes that don't modify src or test files
+    - revert: Reverts a previous commit
 
 And that's it! Thanks for helping out.

.github/workflows/validate-conventional-commits.yml

@@ -0,0 +1,13 @@
+name: Validate Conventional Commit Title
+on:
+  pull_request:
+    types: [opened, edited, reopened]
+
+jobs:
+  pr-title-linter:
+    runs-on: ubuntu-latest
+    steps:
+    # this is a hash for amannn/[email protected]
+    - uses: amannn/action-semantic-pull-request@c3cd5d1ea3580753008872425915e343e351ab54
+      env:
+        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

app/manifest/v3/chrome.json

@@ -1,6 +1,6 @@
 {
   "content_security_policy": {
-    "extension_pages": "script-src 'self'; object-src 'none'; frame-ancestors 'self';"
+    "extension_pages": "script-src 'self'; object-src 'none'; frame-ancestors 'none';"
   },
   "externally_connectable": {
     "matches": ["https://metamask.io/*"],

app/scripts/app-init.js

@@ -165,10 +165,11 @@ const registerInPageContentScript = async () => {
 registerInPageContentScript();
 
 /**
- * Creates an offscreen document that can be used to load iframes containing
- * scripts that can communicate with the extension through the chrome runtime
- * API. Only one offscreen document may exist, so iframes are used within the
- * created offscreen document. See the offscreen folder for more details.
+ * Creates an offscreen document that can be used to load additional scripts
+ * and iframes that can communicate with the extension through the chrome
+ * runtime API. Only one offscreen document may exist, so any iframes required
+ * by extension can be embedded in the offscreen.html file. See the offscreen
+ * folder for more details.
  */
 async function createOffscreen() {
   if (await chrome.offscreen.hasDocument()) {

app/scripts/controllers/detect-tokens.test.js

@@ -259,6 +259,7 @@ describe('DetectTokensController', function () {
       networkConfigurations: {},
       onAccountRemoved: sinon.stub(),
       controllerMessenger: preferencesControllerMessenger,
+      onKeyringStateChange: sinon.stub(),
     });
     preferences.setUseTokenDetection(true);
 

app/scripts/controllers/mmi-controller.test.js

@@ -47,6 +47,7 @@ describe('MMIController', function () {
         onAccountRemoved: jest.fn(),
         provider: {},
         networkConfigurations: {},
+        onKeyringStateChange: jest.fn(),
       }),
       appStateController: new AppStateController({
         addUnlockListener: jest.fn(),

app/scripts/controllers/preferences.js

@@ -119,6 +119,18 @@ export default class PreferencesController {
     this.store.setMaxListeners(13);
     this.tokenListController = opts.tokenListController;
 
+    opts.onKeyringStateChange((state) => {
+      const accounts = new Set();
+      for (const keyring of state.keyrings) {
+        for (const address of keyring.accounts) {
+          accounts.add(address);
+        }
+      }
+      if (accounts.size > 0) {
+        this.syncAddresses(Array.from(accounts));
+      }
+    });
+
     global.setPreference = (key, value) => {
       return this.setFeatureFlag(key, value);
     };

app/scripts/controllers/preferences.test.js

@@ -24,6 +24,7 @@ const NETWORK_CONFIGURATION_DATA = {
 describe('preferences controller', () => {
   let preferencesController;
   let tokenListController;
+  let onKeyringStateChangeListener;
 
   beforeEach(() => {
     const tokenListMessenger = new ControllerMessenger().getRestricted({
@@ -41,6 +42,9 @@ describe('preferences controller', () => {
       initLangCode: 'en_US',
       tokenListController,
       networkConfigurations: NETWORK_CONFIGURATION_DATA,
+      onKeyringStateChange: (listener) => {
+        onKeyringStateChangeListener = listener;
+      },
     });
   });
 
@@ -364,6 +368,24 @@ describe('preferences controller', () => {
     });
   });
 
+  describe('onKeyringStateChange', () => {
+    it('should sync the identities with the keyring', () => {
+      const mockKeyringControllerState = {
+        keyrings: [
+          {
+            accounts: ['0x1', '0x2', '0x3', '0x4'],
+          },
+        ],
+      };
+
+      onKeyringStateChangeListener(mockKeyringControllerState);
+
+      expect(
+        Object.keys(preferencesController.store.getState().identities),
+      ).toStrictEqual(mockKeyringControllerState.keyrings[0].accounts);
+    });
+  });
+
   ///: BEGIN:ONLY_INCLUDE_IF(petnames)
   describe('setUseExternalNameSources', () => {
     it('should default to true', () => {

app/scripts/lib/createRPCMethodTrackingMiddleware.test.js

@@ -79,11 +79,16 @@ function getNext(timeout = 500) {
 const waitForSeconds = async (seconds) =>
   await new Promise((resolve) => setTimeout(resolve, SECOND * seconds));
 
-jest.mock('@metamask/controller-utils', () => ({
-  detectSIWE: jest.fn().mockImplementation(() => {
-    return { isSIWEMessage: false };
-  }),
-}));
+jest.mock('@metamask/controller-utils', () => {
+  const actual = jest.requireActual('@metamask/controller-utils');
+
+  return {
+    ...actual,
+    detectSIWE: jest.fn().mockImplementation(() => {
+      return { isSIWEMessage: false };
+    }),
+  };
+});
 
 describe('createRPCMethodTrackingMiddleware', () => {
   afterEach(() => {

app/scripts/lib/middleware/pending.js

@@ -13,7 +13,7 @@ export function createPendingNonceMiddleware({ getPendingNonce }) {
       next();
       return;
     }
-    res.result = await getPendingNonce(param);
+    res.result = await getPendingNonce(param, req.networkClientId);
   });
 }
 

app/scripts/lib/offscreen-bridge/lattice-offscreen-keyring.ts

@@ -12,14 +12,14 @@ import {
  * _getCreds method calls into window.open to open a new window for the lattice
  * connector. The solution here is to split the keyring execution so that the
  * portion that requires a normal functioning DOM is executed in the offscreen
- * document. The lattice-iframe.ts file in the offscreen directory is
- * responsible for the latter half of the original keyrings execution. The main
- * difference in the first half that this overwritten method is responsible for
- * is just to use the chrome.runtime.sendMessage method to send a message to
- * the offscreen document which picks up on the execution of opening the new
- * window to the lattice connector. Note that this file differs from the
- * ledger and trezor offscreen bridges because this keyring requires no bridge
- * to function appropriately.
+ * document. The lattice.ts file in the offscreen directory is responsible for
+ * the latter half of the original keyrings execution. The main difference in
+ * the first half that this overwritten method is responsible for is just to
+ * use the chrome.runtime.sendMessage method to send a message to the offscreen
+ * document which picks up on the execution of opening the new window to the
+ * lattice connector. Note that this file differs from the ledger and trezor
+ * offscreen bridges because this keyring requires no bridge to function
+ * appropriately.
  */
 class LatticeKeyringOffscreen extends LatticeKeyring {
   static type: string;

app/scripts/lib/offscreen-bridge/ledger-offscreen-bridge.ts

@@ -11,9 +11,10 @@ import {
  * that the keyring can call into for specific functions. The bridge then makes
  * whatever calls or requests it needs to in order to fulfill the request from
  * the keyring. In this case, the bridge is used to communicate with the
- * Offscreen Document. Inside the Offscreen document the ledger-iframe is
- * embedded that will listen for these calls and communicate with the
- * ledger device via the ledger keyring iframe.
+ * Offscreen Document. Inside the Offscreen document the ledger script is
+ * loaded and registers a listener for these calls and communicate with the
+ * ledger device via the ledger keyring iframe. The ledger keyring iframe is
+ * added to the offscreen.html file directly.
  */
 export class LedgerOffscreenBridge implements LedgerBridge {
   isDeviceConnected = false;

app/scripts/lib/offscreen-bridge/trezor-offscreen-bridge.ts

@@ -23,8 +23,8 @@ import {
  * that the keyring can call into for specific functions. The bridge then makes
  * whatever calls or requests it needs to in order to fulfill the request from
  * the keyring. In this case, the bridge is used to communicate with the
- * Offscreen Document. Inside the Offscreen document the trezor-iframe is
- * embedded that will listen for these calls and communicate with the
+ * Offscreen Document. Inside the Offscreen document the trezor script is
+ * loaded and registers a listener for these calls and communicate with the
  * trezor/connect-web library.
  */
 export class TrezorOffscreenBridge implements TrezorBridge {

app/scripts/lib/transaction/util.test.ts

@@ -139,10 +139,26 @@ describe('Transaction Utils', () => {
         ).toHaveBeenCalledTimes(1);
         expect(
           request.transactionController.addTransaction,
-        ).toHaveBeenCalledWith(
-          TRANSACTION_PARAMS_MOCK,
-          TRANSACTION_OPTIONS_MOCK,
-        );
+        ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, {
+          ...TRANSACTION_OPTIONS_MOCK,
+        });
+      });
+
+      it('adds transaction with networkClientId if process.env.TRANSACTION_MULTICHAIN is set', async () => {
+        process.env.TRANSACTION_MULTICHAIN = '1';
+
+        await addTransaction(request);
+
+        expect(
+          request.transactionController.addTransaction,
+        ).toHaveBeenCalledTimes(1);
+        expect(
+          request.transactionController.addTransaction,
+        ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, {
+          ...TRANSACTION_OPTIONS_MOCK,
+          networkClientId: 'mockNetworkClientId',
+        });
+        process.env.TRANSACTION_MULTICHAIN = '';
       });
 
       it('returns transaction meta', async () => {
@@ -418,10 +434,9 @@ describe('Transaction Utils', () => {
         ).toHaveBeenCalledTimes(1);
         expect(
           request.transactionController.addTransaction,
-        ).toHaveBeenCalledWith(
-          TRANSACTION_PARAMS_MOCK,
-          TRANSACTION_OPTIONS_MOCK,
-        );
+        ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, {
+          ...TRANSACTION_OPTIONS_MOCK,
+        });
 
         expect(request.ppomController.usePPOM).toHaveBeenCalledTimes(0);
       });
@@ -484,10 +499,9 @@ describe('Transaction Utils', () => {
         ).toHaveBeenCalledTimes(1);
         expect(
           request.transactionController.addTransaction,
-        ).toHaveBeenCalledWith(
-          TRANSACTION_PARAMS_MOCK,
-          TRANSACTION_OPTIONS_MOCK,
-        );
+        ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, {
+          ...TRANSACTION_OPTIONS_MOCK,
+        });
 
         expect(request.ppomController.usePPOM).toHaveBeenCalledTimes(0);
       });
@@ -504,10 +518,9 @@ describe('Transaction Utils', () => {
         ).toHaveBeenCalledTimes(1);
         expect(
           request.transactionController.addTransaction,
-        ).toHaveBeenCalledWith(
-          TRANSACTION_PARAMS_MOCK,
-          TRANSACTION_OPTIONS_MOCK,
-        );
+        ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, {
+          ...TRANSACTION_OPTIONS_MOCK,
+        });
 
         expect(request.ppomController.usePPOM).toHaveBeenCalledTimes(0);
       });
@@ -533,6 +546,27 @@ describe('Transaction Utils', () => {
         });
       });
 
+      it('adds transaction with networkClientId if process.env.TRANSACTION_MULTICHAIN is set', async () => {
+        process.env.TRANSACTION_MULTICHAIN = '1';
+
+        await addDappTransaction(dappRequest);
+
+        expect(
+          request.transactionController.addTransaction,
+        ).toHaveBeenCalledTimes(1);
+        expect(
+          request.transactionController.addTransaction,
+        ).toHaveBeenCalledWith(TRANSACTION_PARAMS_MOCK, {
+          ...TRANSACTION_OPTIONS_MOCK,
+          networkClientId: 'mockNetworkClientId',
+          method: DAPP_REQUEST_MOCK.method,
+          requireApproval: true,
+          securityAlertResponse: DAPP_REQUEST_MOCK.securityAlertResponse,
+          type: undefined,
+        });
+        process.env.TRANSACTION_MULTICHAIN = '';
+      });
+
       it('returns transaction hash', async () => {
         const transactionHash = await addDappTransaction(dappRequest);
         expect(transactionHash).toStrictEqual(TRANSACTION_META_MOCK.hash);

app/scripts/lib/transaction/util.ts

@@ -238,13 +238,17 @@ async function addTransactionOrUserOperation(
 async function addTransactionWithController(
   request: FinalAddTransactionRequest,
 ) {
-  const { transactionController, transactionOptions, transactionParams } =
-    request;
+  const {
+    transactionController,
+    transactionOptions,
+    transactionParams,
+    networkClientId,
+  } = request;
   const { result, transactionMeta } =
-    await transactionController.addTransaction(
-      transactionParams,
-      transactionOptions,
-    );
+    await transactionController.addTransaction(transactionParams, {
+      ...transactionOptions,
+      ...(process.env.TRANSACTION_MULTICHAIN ? { networkClientId } : {}),
+    });
 
   return {
     transactionMeta,