From cb82620c356f2811b13aac1fce3337569b9ecb3e Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Mon, 8 Apr 2024 12:16:49 +0200 Subject: [PATCH 1/5] fix: fix getting nft tokenURI --- .../src/Standards/NftStandards/ERC721/ERC721Standard.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/Standards/NftStandards/ERC721/ERC721Standard.ts b/packages/assets-controllers/src/Standards/NftStandards/ERC721/ERC721Standard.ts index d8bfd4bde7..d788e4b9b3 100644 --- a/packages/assets-controllers/src/Standards/NftStandards/ERC721/ERC721Standard.ts +++ b/packages/assets-controllers/src/Standards/NftStandards/ERC721/ERC721Standard.ts @@ -91,7 +91,10 @@ export class ERC721Standard { 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); }; From 86428435b8aeecd4d3955d5373171096f2a0b282 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Mon, 8 Apr 2024 14:42:06 +0200 Subject: [PATCH 2/5] fix: fix test --- .../src/AssetsContractController.test.ts | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index 21e3cd2c08..db8e7c62ee 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -608,7 +608,7 @@ describe('AssetsContractController', () => { messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); - it('should throw an error when address given is not an ERC-721 NFT', async () => { + it('should not throw an error when address given does not support NFT Metadata interface', async () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); assetsContract.configure({ provider }); @@ -630,17 +630,30 @@ describe('AssetsContractController', () => { result: '0x', }, }, + { + request: { + method: 'eth_call', + params: [ + { + to: '0x0000000000000000000000000000000000000000', + data: '0xc87b56dd0000000000000000000000000000000000000000000000000000000000000000', + }, + 'latest', + ], + }, + response: { + result: + '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002468747470733a2f2f6170692e676f6473756e636861696e65642e636f6d2f636172642f3000000000000000000000000000000000000000000000000000000000', + }, + }, ], }); - const result = async () => { - await assetsContract.getERC721TokenURI( - '0x0000000000000000000000000000000000000000', - '0', - ); - }; + const uri = await assetsContract.getERC721TokenURI( + '0x0000000000000000000000000000000000000000', + '0', + ); + expect(uri).toBe('https://api.godsunchained.com/card/0'); - const error = 'Contract does not support ERC721 metadata interface.'; - await expect(result).rejects.toThrow(error); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); From d339d0658bd7dea7d8438b2b65f304e6647420c2 Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Mon, 8 Apr 2024 14:52:29 +0200 Subject: [PATCH 3/5] fix: fix test --- ...tractControllerWithNetworkClientId.test.ts | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts index b69148ddba..f7b9488182 100644 --- a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts +++ b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts @@ -420,7 +420,7 @@ describe('AssetsContractController with NetworkClientId', () => { messenger.clearEventSubscriptions('NetworkController:stateChange'); }); - it('should throw an error when address given is not an ERC-721 NFT', async () => { + it('should not throw an error when address given is does not support NFT Metadata interface', async () => { const { assetsContract, messenger, networkClientConfiguration } = await setupAssetContractControllers(); mockNetworkWithDefaultChainId({ @@ -441,18 +441,30 @@ describe('AssetsContractController with NetworkClientId', () => { result: '0x', }, }, + { + request: { + method: 'eth_call', + params: [ + { + to: '0x0000000000000000000000000000000000000000', + data: '0xc87b56dd0000000000000000000000000000000000000000000000000000000000000000', + }, + 'latest', + ], + }, + response: { + result: + '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002468747470733a2f2f6170692e676f6473756e636861696e65642e636f6d2f636172642f3000000000000000000000000000000000000000000000000000000000', + }, + }, ], }); - const result = async () => { - await assetsContract.getERC721TokenURI( - '0x0000000000000000000000000000000000000000', - '0', - 'mainnet', - ); - }; - - const error = 'Contract does not support ERC721 metadata interface.'; - await expect(result).rejects.toThrow(error); + const uri = await assetsContract.getERC721TokenURI( + '0x0000000000000000000000000000000000000000', + '0', + 'mainnet', + ); + expect(uri).toBe('https://api.godsunchained.com/card/0'); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); From d8940d07d075cfb328f9977d91073dd9d4e8d1ce Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Thu, 11 Apr 2024 15:19:28 +0200 Subject: [PATCH 4/5] fix: expect error log in test --- .../assets-controllers/src/AssetsContractController.test.ts | 3 +++ .../src/AssetsContractControllerWithNetworkClientId.test.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index db8e7c62ee..81ce419fdc 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -612,6 +612,7 @@ describe('AssetsContractController', () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); assetsContract.configure({ provider }); + const errorLogSpy = jest.spyOn(console, 'error').mockImplementationOnce(() => {}); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -653,6 +654,8 @@ describe('AssetsContractController', () => { '0', ); expect(uri).toBe('https://api.godsunchained.com/card/0'); + expect(errorLogSpy).toHaveBeenCalledTimes(1); + expect(errorLogSpy.mock.calls).toContainEqual(['Contract does not support ERC721 metadata interface.']); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); diff --git a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts index f7b9488182..b6f4fa9001 100644 --- a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts +++ b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts @@ -459,12 +459,15 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); + const errorLogSpy = jest.spyOn(console, 'error').mockImplementationOnce(() => {}); const uri = await assetsContract.getERC721TokenURI( '0x0000000000000000000000000000000000000000', '0', 'mainnet', ); expect(uri).toBe('https://api.godsunchained.com/card/0'); + expect(errorLogSpy).toHaveBeenCalledTimes(1); + expect(errorLogSpy.mock.calls).toContainEqual(['Contract does not support ERC721 metadata interface.']); messenger.clearEventSubscriptions('NetworkController:stateChange'); }); From beaa51d90e5fcf7fdfb5236f7bb84e5a2a200e3c Mon Sep 17 00:00:00 2001 From: sahar-fehri Date: Thu, 11 Apr 2024 15:29:05 +0200 Subject: [PATCH 5/5] fix: fix lint --- .../src/AssetsContractController.test.ts | 10 ++++++++-- ...AssetsContractControllerWithNetworkClientId.test.ts | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/AssetsContractController.test.ts b/packages/assets-controllers/src/AssetsContractController.test.ts index 81ce419fdc..1ac2b3b0fc 100644 --- a/packages/assets-controllers/src/AssetsContractController.test.ts +++ b/packages/assets-controllers/src/AssetsContractController.test.ts @@ -612,7 +612,11 @@ describe('AssetsContractController', () => { const { assetsContract, messenger, provider, networkClientConfiguration } = await setupAssetContractControllers(); assetsContract.configure({ provider }); - const errorLogSpy = jest.spyOn(console, 'error').mockImplementationOnce(() => {}); + const errorLogSpy = jest + .spyOn(console, 'error') + .mockImplementationOnce(() => { + /**/ + }); mockNetworkWithDefaultChainId({ networkClientConfiguration, mocks: [ @@ -655,7 +659,9 @@ describe('AssetsContractController', () => { ); expect(uri).toBe('https://api.godsunchained.com/card/0'); expect(errorLogSpy).toHaveBeenCalledTimes(1); - expect(errorLogSpy.mock.calls).toContainEqual(['Contract does not support ERC721 metadata interface.']); + expect(errorLogSpy.mock.calls).toContainEqual([ + 'Contract does not support ERC721 metadata interface.', + ]); messenger.clearEventSubscriptions('NetworkController:networkDidChange'); }); diff --git a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts index b6f4fa9001..d064192b68 100644 --- a/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts +++ b/packages/assets-controllers/src/AssetsContractControllerWithNetworkClientId.test.ts @@ -459,7 +459,11 @@ describe('AssetsContractController with NetworkClientId', () => { }, ], }); - const errorLogSpy = jest.spyOn(console, 'error').mockImplementationOnce(() => {}); + const errorLogSpy = jest + .spyOn(console, 'error') + .mockImplementationOnce(() => { + /**/ + }); const uri = await assetsContract.getERC721TokenURI( '0x0000000000000000000000000000000000000000', '0', @@ -467,7 +471,9 @@ describe('AssetsContractController with NetworkClientId', () => { ); expect(uri).toBe('https://api.godsunchained.com/card/0'); expect(errorLogSpy).toHaveBeenCalledTimes(1); - expect(errorLogSpy.mock.calls).toContainEqual(['Contract does not support ERC721 metadata interface.']); + expect(errorLogSpy.mock.calls).toContainEqual([ + 'Contract does not support ERC721 metadata interface.', + ]); messenger.clearEventSubscriptions('NetworkController:stateChange'); });