Skip to content

Commit

Permalink
Version v11.15.0 remove unecessary pollings when mm closed (#24219)
Browse files Browse the repository at this point in the history
Cherry picks #24162

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24219?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

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

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] 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.

## **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.
  • Loading branch information
sahar-fehri committed Apr 30, 2024
1 parent 297ecc8 commit 3d70b1f
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 11 deletions.
40 changes: 40 additions & 0 deletions .yarn/patches/@metamask-assets-controllers-patch-0f46262fea.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
diff --git a/dist/NftDetectionController.js b/dist/NftDetectionController.js
index 24373e328d3600d1168914a3dc0bbbd905b19ebe..3877bebee24d1ad5cd2183b50547e8cef1846558 100644
--- a/dist/NftDetectionController.js
+++ b/dist/NftDetectionController.js
@@ -36,7 +36,7 @@ class NftDetectionController extends polling_controller_1.StaticIntervalPollingC
* @param config - Initial options used to configure this controller.
* @param state - Initial state to set on this controller.
*/
- constructor({ chainId: initialChainId, getNetworkClientById, onPreferencesStateChange, onNetworkStateChange, getOpenSeaApiKey, addNft, getNftApi, getNftState, }, config, state) {
+ constructor({ chainId: initialChainId, getNetworkClientById, onPreferencesStateChange, onNetworkStateChange, getOpenSeaApiKey, addNft, getNftApi, getNftState, disabled: initialDisabled, selectedAddress: initialSelectedAddress }, config, state) {
super(config, state);
/**
* Name of this controller used during composition
@@ -54,8 +54,8 @@ class NftDetectionController extends polling_controller_1.StaticIntervalPollingC
this.defaultConfig = {
interval: DEFAULT_INTERVAL,
chainId: initialChainId,
- selectedAddress: '',
- disabled: true,
+ selectedAddress: initialSelectedAddress,
+ disabled: initialDisabled,
};
this.initialize();
this.getNftState = getNftState;
diff --git a/dist/Standards/NftStandards/ERC721/ERC721Standard.js b/dist/Standards/NftStandards/ERC721/ERC721Standard.js
index d9286b0c0e607d2857f3ee7dad40d13a6c11d7d7..4e12e4b590b1f34a66602d63035f1905917f8c93 100644
--- a/dist/Standards/NftStandards/ERC721/ERC721Standard.js
+++ b/dist/Standards/NftStandards/ERC721/ERC721Standard.js
@@ -66,7 +66,10 @@ class ERC721Standard {
const contract = new contracts_1.Contract(address, metamask_eth_abis_1.abiERC721, this.provider);
const supportsMetadata = yield this.contractSupportsMetadataInterface(address);
if (!supportsMetadata) {
- throw new Error('Contract does not support ERC721 metadata interface.');
+ // Do not throw error here, supporting Metadata interface is optional even though majority of ERC721 nfts do support it.
+ // This change is made because of instances of NFTs that are ERC404( mixed ERC20 / ERC721 implementation).
+ // As of today, ERC404 is unofficial but some people use it, the contract does not support Metadata interface, but it has the tokenURI() fct.
+ console.error('Contract does not support ERC721 metadata interface.');
}
return contract.tokenURI(tokenId);
});
17 changes: 16 additions & 1 deletion app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,14 @@ export default class MetamaskController extends EventEmitter {
addNft: this.nftController.addNft.bind(this.nftController),
getNftApi: this.nftController.getNftApi.bind(this.nftController),
getNftState: () => this.nftController.state,
// added this to track previous value of useNftDetection, should be true on very first initializing of controller[]
disabled:
this.preferencesController.store.getState().useNftDetection ===
undefined
? true
: !this.preferencesController.store.getState().useNftDetection,
selectedAddress:
this.preferencesController.store.getState().selectedAddress,
});

this.metaMetricsController = new MetaMetricsController({
Expand Down Expand Up @@ -1104,6 +1112,7 @@ export default class MetamaskController extends EventEmitter {
this.controllerMessenger.subscribe('KeyringController:lock', () =>
this._onLock(),
);

this.controllerMessenger.subscribe(
'KeyringController:stateChange',
(state) => {
Expand Down Expand Up @@ -2240,7 +2249,12 @@ export default class MetamaskController extends EventEmitter {
const preferencesControllerState =
this.preferencesController.store.getState();

const { useCurrencyRateCheck } = preferencesControllerState;
const { useCurrencyRateCheck, useNftDetection } =
preferencesControllerState;

if (useNftDetection) {
this.nftDetectionController.start();
}

if (useCurrencyRateCheck) {
this.currencyRateController.startPollingByNetworkClientId(
Expand All @@ -2257,6 +2271,7 @@ export default class MetamaskController extends EventEmitter {
this.accountTracker.stop();
this.txController.stopIncomingTransactionPolling();
this.tokenDetectionController.disable();
this.nftDetectionController.stop();

const preferencesControllerState =
this.preferencesController.store.getState();
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
"@metamask/address-book-controller": "^3.1.7",
"@metamask/announcement-controller": "^5.0.1",
"@metamask/approval-controller": "^6.0.0",
"@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A26.0.0#~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch",
"@metamask/assets-controllers": "patch:@metamask/assets-controllers@patch%3A@metamask/assets-controllers@npm%253A26.0.0%23~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch%3A%3Aversion=26.0.0&hash=cf1d54#~/.yarn/patches/@metamask-assets-controllers-patch-0f46262fea.patch",
"@metamask/base-controller": "^4.1.0",
"@metamask/browser-passworder": "^4.3.0",
"@metamask/contract-metadata": "^2.5.0",
Expand Down
26 changes: 21 additions & 5 deletions test/e2e/tests/tokens/nft/auto-detect-nft.spec.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,45 @@
const { strict: assert } = require('assert');
const {
withFixtures,
defaultGanacheOptions,
unlockWallet,
withFixtures,
} = require('../../../helpers');
const FixtureBuilder = require('../../../fixture-builder');
const { setupAutoDetectMocking } = require('./mocks');

describe('NFT detection', function () {
/**
* TODO Revisit this test once we enable nft auto detection by default. Use .withPreferencesControllerNftDetectionEnabled().
*/
it('displays NFT media', async function () {
const driverOptions = { mock: true };
await withFixtures(
{
fixtures: new FixtureBuilder()
.withNetworkControllerOnMainnet()
.withPreferencesControllerNftDetectionEnabled()
.build(),
fixtures: new FixtureBuilder().withNetworkControllerOnMainnet().build(),
driverOptions,
ganacheOptions: defaultGanacheOptions,
title: this.test.fullTitle(),
testSpecificMock: setupAutoDetectMocking,
},
async ({ driver }) => {
await unlockWallet(driver);

// go to settings
await driver.clickElement(
'[data-testid="account-options-menu-button"]',
);
await driver.clickElement({ text: 'Settings', tag: 'div' });
await driver.clickElement({ text: 'Security & privacy', tag: 'div' });
await driver.clickElement(
'[data-testid="useNftDetection"] .toggle-button > div',
);
await driver.clickElement(
'.settings-page__header__title-container__close-button',
);
await driver.clickElement('[data-testid="home__asset-tab"]');

await driver.clickElement('[data-testid="home__nfts-tab"]');
await driver.delay(1000);
const collection = await driver.findElement(
'[data-testid="collection-expander-button"]',
);
Expand Down
7 changes: 5 additions & 2 deletions test/e2e/tests/tokens/nft/mocks.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
function setupAutoDetectMocking(server) {
function setupAutoDetectMocking(
server,
testAddress = '0x5cfe73b6021e818b776b421b1c4db2474086a7e1',
) {
const nfts = {
nfts: [
{
Expand Down Expand Up @@ -57,7 +60,7 @@ function setupAutoDetectMocking(server) {
// Get assets for account
server
.forGet(
'https://proxy.metafi.codefi.network/opensea/v1/api/v2/chain/ethereum/account/0x5cfe73b6021e818b776b421b1c4db2474086a7e1/nfts',
`https://proxy.metafi.codefi.network/opensea/v1/api/v2/chain/ethereum/account/${testAddress}/nfts`,
)
.withQuery({ limit: 200, next: '' })
.thenCallback(() => {
Expand Down
46 changes: 44 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4039,7 +4039,7 @@ __metadata:
languageName: node
linkType: hard

"@metamask/assets-controllers@patch:@metamask/assets-controllers@npm%3A26.0.0#~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch":
"@metamask/assets-controllers@patch:@metamask/assets-controllers@npm%3A26.0.0#~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch::version=26.0.0&hash=cf1d54":
version: 26.0.0
resolution: "@metamask/assets-controllers@patch:@metamask/assets-controllers@npm%3A26.0.0#~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch::version=26.0.0&hash=cf1d54"
dependencies:
Expand Down Expand Up @@ -4081,6 +4081,48 @@ __metadata:
languageName: node
linkType: hard

"@metamask/assets-controllers@patch:@metamask/assets-controllers@patch%3A@metamask/assets-controllers@npm%253A26.0.0%23~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch%3A%3Aversion=26.0.0&hash=cf1d54#~/.yarn/patches/@metamask-assets-controllers-patch-0f46262fea.patch":
version: 26.0.0
resolution: "@metamask/assets-controllers@patch:@metamask/assets-controllers@patch%3A@metamask/assets-controllers@npm%253A26.0.0%23~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch%3A%3Aversion=26.0.0&hash=cf1d54#~/.yarn/patches/@metamask-assets-controllers-patch-0f46262fea.patch::version=26.0.0&hash=5c145e"
dependencies:
"@ethereumjs/util": "npm:^8.1.0"
"@ethersproject/address": "npm:^5.7.0"
"@ethersproject/bignumber": "npm:^5.7.0"
"@ethersproject/contracts": "npm:^5.7.0"
"@ethersproject/providers": "npm:^5.7.0"
"@metamask/abi-utils": "npm:^2.0.2"
"@metamask/accounts-controller": "npm:^11.0.0"
"@metamask/approval-controller": "npm:^5.1.3"
"@metamask/base-controller": "npm:^4.1.1"
"@metamask/contract-metadata": "npm:^2.4.0"
"@metamask/controller-utils": "npm:^8.0.4"
"@metamask/eth-query": "npm:^4.0.0"
"@metamask/keyring-controller": "npm:^13.0.0"
"@metamask/metamask-eth-abis": "npm:3.0.0"
"@metamask/network-controller": "npm:^17.2.1"
"@metamask/polling-controller": "npm:^5.0.1"
"@metamask/preferences-controller": "npm:^8.0.0"
"@metamask/rpc-errors": "npm:^6.2.1"
"@metamask/utils": "npm:^8.3.0"
"@types/bn.js": "npm:^5.1.5"
"@types/uuid": "npm:^8.3.0"
async-mutex: "npm:^0.2.6"
bn.js: "npm:^5.2.1"
cockatiel: "npm:^3.1.2"
lodash: "npm:^4.17.21"
multiformats: "npm:^9.5.2"
single-call-balance-checker-abi: "npm:^1.0.0"
uuid: "npm:^8.3.2"
peerDependencies:
"@metamask/accounts-controller": ^11.0.0
"@metamask/approval-controller": ^5.1.2
"@metamask/keyring-controller": ^13.0.0
"@metamask/network-controller": ^17.2.0
"@metamask/preferences-controller": ^8.0.0
checksum: 4ffddf66427d5a00e489289b3e86ceafa1dfb1a4964113e63f8be7428735cd78000a19a14e0ad1131f7f40e99013510d1fad9952255add55d96a2437931997cb
languageName: node
linkType: hard

"@metamask/auto-changelog@npm:^2.1.0":
version: 2.6.1
resolution: "@metamask/auto-changelog@npm:2.6.1"
Expand Down Expand Up @@ -24857,7 +24899,7 @@ __metadata:
"@metamask/address-book-controller": "npm:^3.1.7"
"@metamask/announcement-controller": "npm:^5.0.1"
"@metamask/approval-controller": "npm:^6.0.0"
"@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A26.0.0#~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch"
"@metamask/assets-controllers": "patch:@metamask/assets-controllers@patch%3A@metamask/assets-controllers@npm%253A26.0.0%23~/.yarn/patches/@metamask-assets-controllers-npm-26.0.0-17c0e9432c.patch%3A%3Aversion=26.0.0&hash=cf1d54#~/.yarn/patches/@metamask-assets-controllers-patch-0f46262fea.patch"
"@metamask/auto-changelog": "npm:^2.1.0"
"@metamask/base-controller": "npm:^4.1.0"
"@metamask/browser-passworder": "npm:^4.3.0"
Expand Down

0 comments on commit 3d70b1f

Please sign in to comment.