Cherry-pick 167036f (#22856) into v11.10.0

fix: fix selectedAddress and nftController instantiation (#22856)

Noticed that when you freshly import the extension, usse testDapp to
deploy and mint an NFT, then manually import the NFT.
you will be able to see the NFT. Then when you switch to another account
and go back to the account that has the NFT;
1- you wont be able to see the NFTs you imported earlier
2- When you try to import them again you will get an error (fired from
core when trying to verify ownership)

Noticed that this was due to core having the wrong userAddress to check
ownership for, and then in metamask.js, the selectedAddress retrieved
does not match the user's selected Address

This behavior is also reported to be on v11.10.0, on this issue:
#22796

Fixes: #22796
Fixes: #22798

1. Remove and reimport the extension
2. Use testDapp to deploy and mint the NFT
3. Add the NFT manually by clicking "import NFT"
4. You should be able to see your newly import NFT
5. Create new account
6. Go back to the account that has the NFT
7. You should be able to see the NFT you imported

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

https://github.com/MetaMask/metamask-extension/assets/10994169/a74cbf0b-014b-4e39-82ac-e8452941fc6e

https://github.com/MetaMask/metamask-extension/assets/10994169/987dbb27-b938-4bce-9b43-4269d714ee33

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained what problem this PR is solving and how it
is solved.
- [ ] I've linked related issues
- [ ] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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 properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

- [ ] 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: Monte Lai <monte.lai@consensys.net>
Co-authored-by: Dan J Miller <danjm.com@gmail.com>

Author: 3 people

app/scripts/metamask-controller.js

@@ -2827,10 +2827,10 @@ export default class MetamaskController extends EventEmitter {
         ),
       // PreferencesController
       setSelectedAddress: (address) => {
-        this.preferencesController.setSelectedAddress(address);
         const account = this.accountsController.getAccountByAddress(address);
         if (account) {
           this.accountsController.setSelectedAccount(account.id);
+          this.preferencesController.setSelectedAddress(address);
         } else {
           throw new Error(`No account found for address: ${address}`);
         }
@@ -2875,8 +2875,13 @@ export default class MetamaskController extends EventEmitter {
       ///: END:ONLY_INCLUDE_IF
 
       // AccountsController
-      setSelectedInternalAccount:
-        accountsController.setSelectedAccount.bind(accountsController),
+      setSelectedInternalAccount: (id) => {
+        const account = this.accountsController.getAccount(id);
+        if (account) {
+          this.preferencesController.setSelectedAddress(account.address);
+          this.accountsController.setSelectedAccount(id);
+        }
+      },
 
       setAccountName:
         accountsController.setAccountName.bind(accountsController),

test/e2e/tests/nft/import-nft.spec.js

@@ -3,6 +3,8 @@ const {
   defaultGanacheOptions,
   withFixtures,
   unlockWallet,
+  findAnotherAccountFromAccountList,
+  waitForAccountRendered,
 } = require('../../helpers');
 const { SMART_CONTRACTS } = require('../../seeder/smart-contracts');
 const FixtureBuilder = require('../../fixture-builder');
@@ -57,6 +59,96 @@ describe('Import NFT', function () {
     );
   });
 
+  it('should continue to display an imported NFT after importing, adding a new account, and switching back', async function () {
+    await withFixtures(
+      {
+        dapp: true,
+        fixtures: new FixtureBuilder()
+          .withPermissionControllerConnectedToTestDapp()
+          .build(),
+        ganacheOptions: defaultGanacheOptions,
+        smartContract,
+        title: this.test.fullTitle(),
+      },
+      async ({ driver, _, contractRegistry }) => {
+        const contractAddress =
+          contractRegistry.getContractAddress(smartContract);
+        await unlockWallet(driver);
+
+        // After login, go to NFTs tab, open the import NFT form
+        await driver.clickElement('[data-testid="home__nfts-tab"]');
+        await driver.clickElement({ text: 'Import NFT', tag: 'button' });
+
+        // Enter a valid NFT that belongs to user and check success message appears
+        await driver.fill('#address', contractAddress);
+        await driver.fill('#token-id', '1');
+        await driver.clickElement(
+          '[data-testid="import-nfts-modal-import-button"]',
+        );
+
+        const newNftNotification = await driver.findElement({
+          text: 'NFT was successfully added!',
+          tag: 'h6',
+        });
+        assert.equal(await newNftNotification.isDisplayed(), true);
+
+        // Check the imported NFT and its image are displayed in the NFT tab
+        const importedNft = await driver.waitForSelector({
+          css: 'h5',
+          text: 'TestDappNFTs',
+        });
+        const importedNftImage = await driver.findElement(
+          '.nft-item__container',
+        );
+        assert.equal(await importedNft.isDisplayed(), true);
+        assert.equal(await importedNftImage.isDisplayed(), true);
+
+        await driver.clickElement({ text: 'Tokens', tag: 'button' });
+        await driver.clickElement('[data-testid="account-menu-icon"]');
+        await driver.clickElement(
+          '[data-testid="multichain-account-menu-popover-action-button"]',
+        );
+        await driver.clickElement({ text: 'Add a new account', tag: 'button' });
+
+        // set account name
+        await driver.fill('[placeholder="Account 2"]', '2nd account');
+        await driver.delay(400);
+        await driver.clickElement({ text: 'Create', tag: 'button' });
+
+        await driver.isElementPresent({
+          tag: 'span',
+          text: '2nd account',
+        });
+
+        const accountOneSelector = await findAnotherAccountFromAccountList(
+          driver,
+          1,
+          'Account 1',
+        );
+        await waitForAccountRendered(driver);
+        await driver.clickElement(accountOneSelector);
+
+        await driver.clickElement({ text: 'NFTs', tag: 'button' });
+        const nftIsStillDisplayed = await driver.isElementPresentAndVisible({
+          css: 'h5',
+          text: 'TestDappNFTs',
+        });
+        const nftImageIsStillDisplayed =
+          await driver.isElementPresentAndVisible('.nft-item__container');
+        assert.equal(
+          nftIsStillDisplayed,
+          true,
+          'Nft is no longer displayed after adding an account and switching back to account 1',
+        );
+        assert.equal(
+          nftImageIsStillDisplayed,
+          true,
+          'Nft image is no longer displayed after adding an account and switching back to account 1',
+        );
+      },
+    );
+  });
+
   it('should not be able to import an NFT that does not belong to user', async function () {
     await withFixtures(
       {

ui/ducks/metamask/metamask.js

@@ -14,6 +14,7 @@ import {
   checkNetworkAndAccountSupports1559,
   getAddressBook,
   getSelectedNetworkClientId,
+  getSelectedInternalAccount,
 } from '../../selectors';
 import * as actionConstants from '../../store/actionConstants';
 import { updateTransactionGasFees } from '../../store/actions';
@@ -271,19 +272,21 @@ export function getNftsDropdownState(state) {
 
 export const getNfts = (state) => {
   const {
-    metamask: { allNfts, selectedAddress },
+    metamask: { allNfts },
   } = state;
+  const { address: selectedAddress } = getSelectedInternalAccount(state);
+
   const { chainId } = getProviderConfig(state);
 
   return allNfts?.[selectedAddress]?.[chainId] ?? [];
 };
 
 export const getNftContracts = (state) => {
   const {
-    metamask: { allNftContracts, selectedAddress },
+    metamask: { allNftContracts },
   } = state;
+  const { address: selectedAddress } = getSelectedInternalAccount(state);
   const { chainId } = getProviderConfig(state);
-
   return allNftContracts?.[selectedAddress]?.[chainId] ?? [];
 };
 

ui/store/actions.test.js

@@ -889,15 +889,20 @@ describe('Actions', () => {
       const setSelectedInternalAccountSpy = sinon
         .stub()
         .callsFake((_, cb) => cb());
+      const setSelectedAddressSpy = sinon.stub().callsFake((_, cb) => cb());
 
       background.getApi.returns({
         setSelectedInternalAccount: setSelectedInternalAccountSpy,
+        setSelectedAddress: setSelectedAddressSpy,
       });
 
       setBackgroundConnection(background.getApi());
 
       await store.dispatch(actions.setSelectedAccount('0x123'));
       expect(setSelectedInternalAccountSpy.callCount).toStrictEqual(1);
+      expect(setSelectedInternalAccountSpy.calledWith('mock-id')).toBe(true);
+      expect(setSelectedAddressSpy.callCount).toStrictEqual(1);
+      expect(setSelectedAddressSpy.calledWith('0x123')).toBe(true);
     });
 
     it('displays warning if setSelectedAccount throws', async () => {
@@ -936,9 +941,11 @@ describe('Actions', () => {
       const setSelectedInternalAccountSpy = sinon
         .stub()
         .callsFake((_, cb) => cb(new Error('error')));
+      const setSelectedAddressSpy = sinon.stub().callsFake((_, cb) => cb());
 
       background.getApi.returns({
         setSelectedInternalAccount: setSelectedInternalAccountSpy,
+        setSelectedAddress: setSelectedAddressSpy,
       });
 
       setBackgroundConnection(background.getApi());

ui/store/actions.ts

@@ -1757,6 +1757,7 @@ export function setSelectedAccount(
       !currentTabIsConnectedToNextAddress;
 
     try {
+      await _setSelectedAddress(address);
       await _setSelectedInternalAccount(internalAccount.id);
       await forceUpdateMetamaskState(dispatch);
     } catch (error) {