Skip to content

Commit

Permalink
Add contract address validation
Browse files Browse the repository at this point in the history
Contract address validation has been added to the
`getCollectibleTokenURI` method. This validation was already present in
the one place where `getCollectibleTokenURI` is called, so overall this
is basically a refactor with no functional changes, except that the API
of each of these controllers has changed. It's a non-functional change
from mobile's perspective though.

It might seem more appropriate to throw an error if the address isn't
an NFT, rather than returning an empty string. But this is the pre-
existing behaviour, so it has been preserved for now.
  • Loading branch information
Gudahtt committed Mar 19, 2021
1 parent 47fbfa5 commit 09629fa
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 19 deletions.
6 changes: 6 additions & 0 deletions src/assets/AssetsContractController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ describe('AssetsContractController', () => {
expect(tokenId).toEqual('https://api.godsunchained.com/card/0');
});

it('should return empty string as URI when address given is not an NFT', async () => {
assetsContract.configure({ provider: MAINNET_PROVIDER });
const tokenId = await assetsContract.getCollectibleTokenURI('0x0000000000000000000000000000000000000000', 0);
expect(tokenId).toEqual('');
});

it('should get collectible name', async () => {
assetsContract.configure({ provider: MAINNET_PROVIDER });
const name = await assetsContract.getAssetName(GODSADDRESS);
Expand Down
4 changes: 4 additions & 0 deletions src/assets/AssetsContractController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ export class AssetsContractController extends BaseController<AssetsContractConfi
* @returns - Promise resolving to the 'tokenURI'
*/
async getCollectibleTokenURI(address: string, tokenId: number): Promise<string> {
const supportsMetadata = await this.contractSupportsMetadataInterface(address);
if (!supportsMetadata) {
return '';
}
const contract = this.web3.eth.contract(abiERC721).at(address);
return new Promise<string>((resolve, reject) => {
contract.tokenURI(tokenId, (error: Error, result: string) => {
Expand Down
21 changes: 2 additions & 19 deletions src/assets/AssetsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,24 +190,6 @@ export class AssetsController extends BaseController<AssetsConfig, AssetsState>
this.hub.emit(`${suggestedAssetMeta.id}:finished`, failedSuggestedAssetMeta);
}

/**
* Get collectible tokenURI API following ERC721
*
* @param contractAddress - ERC721 asset contract address
* @param tokenId - ERC721 asset identifier
* @returns - Collectible tokenURI
*/
private async getCollectibleTokenURI(contractAddress: string, tokenId: number): Promise<string> {
const assetsContract = this.context.AssetsContractController as AssetsContractController;
const supportsMetadata = await assetsContract.contractSupportsMetadataInterface(contractAddress);
/* istanbul ignore if */
if (!supportsMetadata) {
return '';
}
const tokenURI = await assetsContract.getCollectibleTokenURI(contractAddress, tokenId);
return tokenURI;
}

/**
* Request individual collectible information from OpenSea api
*
Expand Down Expand Up @@ -242,7 +224,8 @@ export class AssetsController extends BaseController<AssetsConfig, AssetsState>
contractAddress: string,
tokenId: number,
): Promise<CollectibleInformation> {
const tokenURI = await this.getCollectibleTokenURI(contractAddress, tokenId);
const assetsContract = this.context.AssetsContractController as AssetsContractController;
const tokenURI = await assetsContract.getCollectibleTokenURI(contractAddress, tokenId);
const object = await handleFetch(tokenURI);
const image = object.hasOwnProperty('image') ? 'image' : /* istanbul ignore next */ 'image_url';
return { image: object[image], name: object.name };
Expand Down

0 comments on commit 09629fa

Please sign in to comment.