Skip to content

Commit

Permalink
fix: fix getting nft tokenURI (#4136)
Browse files Browse the repository at this point in the history
## Explanation

User has reached out because when they import their NFT they are not
able to see it.
The smart contract is deployed on linea-mainnet. It is a ERC404 contract
(mixed implementation of ERC721/ERC20).
The contract does not support Metadata interface but it has the function
tokenURI().

Contract address with this issue :
0x93A5467fbC9D7dbB5713752Ec5a934a5A752AC07

## References

<!--
Are there any issues that this pull request is tied to? Are there other
links that reviewers should consult to understand these changes better?

For example:

* Fixes #12345
* Related to #67890
-->

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/package-assets-controllers`

- **Fixed**: Instead of throwing error inside getTokenURI fct when the
contract does not support Metadata interface, i am only logging the
error.


## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
  • Loading branch information
sahar-fehri authored Apr 12, 2024
1 parent 498e748 commit bfe71c4
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 21 deletions.
40 changes: 31 additions & 9 deletions packages/assets-controllers/src/AssetsContractController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -608,10 +608,15 @@ 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 });
const errorLogSpy = jest
.spyOn(console, 'error')
.mockImplementationOnce(() => {
/**/
});
mockNetworkWithDefaultChainId({
networkClientConfiguration,
mocks: [
Expand All @@ -630,17 +635,34 @@ 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');
expect(errorLogSpy).toHaveBeenCalledTimes(1);
expect(errorLogSpy.mock.calls).toContainEqual([
'Contract does not support ERC721 metadata interface.',
]);

const error = 'Contract does not support ERC721 metadata interface.';
await expect(result).rejects.toThrow(error);
messenger.clearEventSubscriptions('NetworkController:networkDidChange');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -441,18 +441,39 @@ 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 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');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down

0 comments on commit bfe71c4

Please sign in to comment.