search

loomnetwork / erc721x

ERC721x is an extension of ERC721 that adds support for multi-fungible tokens and batch transfers, while being fully backward-compatible.
BSD 3-Clause "New" or "Revised" License
165 stars 54 forks source link

tokenURI does not support tokens with more than 6 digits IDs #18

Closed Pongch closed 5 years ago

Pongch commented 6 years ago

Currently with how token URIs are implemented:

tokenUri = "https://rinkeby.loom.games/erc721/zmb/000000.json";

        bytes memory _uriBytes = bytes(tokenUri);
        _uriBytes[38] = byte(48+(_tokenId / 100000) % 10);
        _uriBytes[39] = byte(48+(_tokenId / 10000) % 10);
        _uriBytes[40] = byte(48+(_tokenId / 1000) % 10);
        _uriBytes[41] = byte(48+(_tokenId / 100) % 10);
        _uriBytes[42] = byte(48+(_tokenId / 10) % 10);
        _uriBytes[43] = byte(48+(_tokenId / 1) % 10);

you are able to successfully mint a token with ID : 8999999 and check that it exists but by calling tokenURI() of the token ID, it will return 999999(missing the 7th digit)

There might be a better approach to this without having to manually specifying: '_uriBytes[38] = byte(48+(_tokenId / 100000) % 10);' Otherwise it might be ideal to revert the transaction if a user is attempting to mint tokens with more than 6 digits IDs

gakonst commented 6 years ago

Yeah this is part of https://github.com/loomnetwork/erc721x/issues/10, tbd in the future as we abstract it from the core contract.

gakonst commented 5 years ago

Fixed in https://github.com/loomnetwork/erc721x/commit/db5928ad03acdf2ecaa10db8ea8c0e754b6c716a, now the URI is passed via the constructor