Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add thresshold for update NFT metadata #5134

Merged
merged 12 commits into from
Jan 16, 2025
Merged

fix: add thresshold for update NFT metadata #5134

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
760153e
fix: add thresshold for update NFT metadata
sahar-fehri Jan 13, 2025
c679aa0
fix: fix add check if incoming values are different than current values
sahar-fehri Jan 13, 2025
a8fc119
fix: test
sahar-fehri Jan 13, 2025
aab4f6b
fix: test
sahar-fehri Jan 13, 2025
8ddc08f
Merge branch 'main' into fix/add-threshold-for-update-nft-metadata-fct
sahar-fehri Jan 15, 2025
82f77b0
fix: fix test
sahar-fehri Jan 15, 2025
cb91517
fix: fix lint comment
sahar-fehri Jan 15, 2025
19581f7
fix: rm line
sahar-fehri Jan 15, 2025
ec7f4d4
Merge branch 'main' into fix/add-threshold-for-update-nft-metadata-fct
sahar-fehri Jan 15, 2025
ef12e4f
Update packages/assets-controllers/src/NftController.test.ts
sahar-fehri Jan 16, 2025
023dd64
fix: remove import
sahar-fehri Jan 16, 2025
60437d3
Merge branch 'main' into fix/add-threshold-for-update-nft-metadata-fct
sahar-fehri Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions packages/assets-controllers/src/NftController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4471,6 +4471,48 @@ describe('NftController', () => {
});

describe('updateNftMetadata', () => {
it('should not update Nft metadata when preferences change and current and incoming state are the same', async () => {
sahar-fehri marked this conversation as resolved.
Show resolved Hide resolved
const {
nftController,
triggerPreferencesStateChange,
triggerSelectedAccountChange,
} = setupController();
const spy = jest.spyOn(nftController, 'updateNftMetadata');
triggerSelectedAccountChange(OWNER_ACCOUNT);
// trigger preference change
triggerPreferencesStateChange({
...getDefaultPreferencesState(),
});

expect(spy).toHaveBeenCalledTimes(0);
});
it('should call update Nft metadata when preferences change is triggered and at least ipfsGateway, openSeaEnabled or isIpfsGatewayEnabled change', async () => {

mcmire marked this conversation as resolved.
Show resolved Hide resolved
const {
nftController,
mockGetAccount,
triggerPreferencesStateChange,
triggerSelectedAccountChange,
} = setupController({
defaultSelectedAccount: OWNER_ACCOUNT,
});
const spy = jest.spyOn(nftController, 'updateNftMetadata');
const testNetworkClientId = 'mainnet';
mockGetAccount.mockReturnValue(OWNER_ACCOUNT);
await nftController.addNft('0xtest', '3', {
nftMetadata: { name: '', description: '', image: '', standard: '' },
networkClientId: testNetworkClientId,
});

triggerSelectedAccountChange(OWNER_ACCOUNT);
// trigger preference change
triggerPreferencesStateChange({
...getDefaultPreferencesState(),
ipfsGateway: 'https://toto/ipfs/',
});

expect(spy).toHaveBeenCalledTimes(1);
});
it('should update Nft metadata successfully', async () => {
const tokenURI = 'https://api.pudgypenguins.io/lil/4';
const mockGetERC721TokenURI = jest.fn().mockResolvedValue(tokenURI);
Expand Down
30 changes: 20 additions & 10 deletions packages/assets-controllers/src/NftController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
ApprovalType,
NFT_API_BASE_URL,
NFT_API_VERSION,
NFT_UPDATE_THRESHOLD,
} from '@metamask/controller-utils';
import { type InternalAccount } from '@metamask/keyring-internal-api';
import type {
Expand Down Expand Up @@ -421,15 +422,21 @@ export class NftController extends BaseController<
'AccountsController:getSelectedAccount',
);
this.#selectedAccountId = selectedAccount.id;
this.#ipfsGateway = ipfsGateway;
this.#openSeaEnabled = openSeaEnabled;
this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled;

const needsUpdateNftMetadata =
(isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled;

if (needsUpdateNftMetadata && selectedAccount) {
await this.#updateNftUpdateForAccount(selectedAccount);
//Get current state values
if (
this.#ipfsGateway !== ipfsGateway ||
this.#openSeaEnabled !== openSeaEnabled ||
this.#isIpfsGatewayEnabled !== isIpfsGatewayEnabled
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this so that even if this gets triggered for any reason; we do not proceed to update the nftmetadata unless one of the concerned params has changed

) {
this.#ipfsGateway = ipfsGateway;
this.#openSeaEnabled = openSeaEnabled;
this.#isIpfsGatewayEnabled = isIpfsGatewayEnabled;
const needsUpdateNftMetadata =
(isIpfsGatewayEnabled && ipfsGateway !== '') || openSeaEnabled;

if (needsUpdateNftMetadata && selectedAccount) {
await this.#updateNftUpdateForAccount(selectedAccount);
}
}
}

Expand Down Expand Up @@ -2053,7 +2060,10 @@ export class NftController extends BaseController<
(singleNft) =>
!singleNft.name && !singleNft.description && !singleNft.image,
);
if (nftsToUpdate.length !== 0) {
if (
nftsToUpdate.length !== 0 &&
nftsToUpdate.length < NFT_UPDATE_THRESHOLD
) {
await this.updateNftMetadata({
nfts: nftsToUpdate,
userAddress: account.address,
Expand Down
1 change: 1 addition & 0 deletions packages/controller-utils/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
export const RPC = 'rpc';
export const FALL_BACK_VS_CURRENCY = 'ETH';
export const IPFS_DEFAULT_GATEWAY_URL = 'https://cloudflare-ipfs.com/ipfs/';
export const NFT_UPDATE_THRESHOLD = 500;
sahar-fehri marked this conversation as resolved.
Show resolved Hide resolved

// NETWORKS ID
// `toHex` not invoked to avoid cyclic dependency
Expand Down
Loading