ERC721 "automatic" token URI

[abcoathup]

🧐 Motivation
Current ERC721 minting with token URI requires manually specifying the URI per token.

To reduce gas cost, the OpenSea example uses concatenation of a base token URI and the tokenID. (TradeableERC721Token.sol)

📝 Details

We want to provide an ERC721 extension ERC721DefaultURI with a base URI parameter, which overrides ERC721Metadata's tokenURI() with the concatenation. To discuss the API please comment on the forum thread about this.

[frangio]

Note that for concatenation of the base URI with the tokenId string we will use abi.encodePacked.

[logeekal]

As discussed in #1163 , I can take this up.

[pointtoken]

@logeekal Will your implementation make the baseTokenURI settable? I think this would be good, in the event that the URL needs to change. Is this code in a branch anywhere?

[logeekal]

Hello @pointtoken, I will start implementation today so no code branch as of now. And I am assuming that it should be settable so that everyone can set there own URI probably in constructor of their contract so that they do not have to do that with every mint operation.

What do you think @abcoathup, In Blockhorses I see that you have hard-coded it, so to be generic I also think it should be settable as well.

[abcoathup]

Hi @logeekal

I agree, I think that a private _baseTokenURI state variable should be set in the constructor via a parameter.

There should also be an internal setter function, so tokens inheriting from this contract can create a mechanism to change the _baseTokenURI as required.

In PeepethBadges.sol we had the private state variable _baseTokenURI that was set in the constructor, (though not via a parameter). We also had a mechanism for the owner to change the baseTokenURI, and I could see some projects wanting this ability to have some Role have the ability to change the baseTokenURI.

[pointtoken]

Once one embraces the idea of offchain metadata for an NFT, one is embracing the idea that the NFT has data which could change. As such, the URL itself of the metadata needs to be change-able. Classic example: the company that created the contract gets sold or folds or who knows what, and the the metadata server itself is no longer available. There could be a scenario where a new metadata server is deployed, thus the need to update the tokenURI, keeping the NFT's metadata URL to be accessed onchain. Otherwise, that contract become much less usable for people who hold the actual NFT.

[pointtoken]

Here's some code, although there may be better ways now:

` string public tokenURIPrefix = "https://api.nft.com/";

function updateTokenURIPrefix(string newPrefix) external onlyOwner {
    tokenURIPrefix = newPrefix;
}

function tokenURI(uint256 _tokenId) public view returns (string) {
return string(abi.encodePacked(tokenURIPrefix,uint2str(_tokenId)));
}

function uint2str(uint i) internal pure returns (string) {
    if (i == 0) return "0";
    uint j = i;
    uint length;
    while (j != 0){
        length++;
        j /= 10;
    }

    bytes memory bstr = new bytes(length);

    uint k = length - 1;
    while (i != 0){
        bstr[k--] = byte(48 + i % 10);
        i /= 10;
    }

    return string(bstr);

}

`

[abcoathup]

Hi @pointtoken

There is already a Strings library in drafts with a fromUint256 function.
https://github.com/OpenZeppelin/openzeppelin-solidity/blob/c5c0e22c8963971381ba8e68198cf3ef0131ae3e/contracts/drafts/Strings.sol#L13

I suggest that the _tokenURIPrefix state variable be private (and hence have an underscore) and that the setter _updateTokenURIPrefix be internal.

That way, inheriting contracts have the option to create an updateTokenURIPrefix function if they desire, with whatever Role or Owner they prefer to use. Projects can also choose not to have an update.

I agree that having metadata off chain and based on a URI rather than being content addressable means that the metadata is centralized.

[pointtoken]

ok, as long that it is easy to make public and settable by an owner of the contract

[pointtoken]

Just curious the status of this feature @abcoathup @logeekal -- timeframe when it might get merged to master?

[abcoathup]

@pointtoken I haven't done any more beyond the Strings library. I would love to see this implemented.

@logeekal are you still working on this?

[nventuro]

@nachomazzara from Decentraland commented that it is currently not possible for users to easily implement this feature (which they need) due to tokenURI() being external, which makes it impossible to call the base implementation via super. The workaround they've found is to override _setTokenURI and use a separate mapping, which is far from ideal and will probably be disallowed once Solidity 0.6.0 comes out.

If @logeekal will not end up tackling this, someone else should do it: we already have a concrete design proposal, and many users would be benefited by this feature.

[logeekal]

I apologize that I completely forgot about this and got involved in other task. I will take it up next week and get back with updates.

[nachomazzara]

To put in here what we thought (omit non-changed code):

pragma solidity ^0.5.0;

import "../../GSN/Context.sol";
import "./ERC721.sol";
import "./IERC721Metadata.sol";
import "../../introspection/ERC165.sol";

contract ERC721Metadata is Context, ERC165, ERC721, IERC721Metadata {
    //...
    
    // Base Token URI
    string private _baseTokenURI;

    //...
    
    // **PROPOSED CHANGE**: 
    // - `external` to `public` (Allow 'super')
    // - Concat baseURI with tokenURI
    function tokenURI(uint256 tokenId) public view returns (string memory) { 
        return string(abi.encodePacked(_baseTokenURI, _tokenURI(tokenId)));
    }
    

    /**
     * @dev Internal returns an URI for a given token ID.
     * Throws if the token ID does not exist. May return an empty string.
     * @param tokenId uint256 ID of the token to query
     */
    function _tokenURI(uint256 tokenId) internal view returns (string memory) { 
        require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");
        return _tokenURIs[tokenId];
    }
    
    /**
     * @dev Internal function to set the base token URI.
     * @param uri string base URI to assign
     */
    function _setBaseTokenURI(string memory uri) internal {
        _baseTokenURI = uri;
    }
}

It continues following the _INTERFACE_ID_ERC721_METADATA = 0x5b5e139f;.

The idea is not only to use the tokenId as the path for the URI as used here but just to have full control of what to save.

_setBaseTokenURI should be exposed to authorized users.

I'm wondering if we should emit an event once the baseTokenURI is changed. Indexers can be reactive to this, updating all the metadata references.

[nachomazzara]

Added a PR

[abcoathup]

Hi @nachomazzara,

Thank you 🙏

This solution gives flexibility, e.g. allows saving an IPFS hash, with the base URI being an IPFS gateway URI, where the base URI can be changed to support a change of IPFS gateway.

An example ERC721 token using this functionality as I envisaged could then be:
(perhaps we need to add a SimpleERC721Token.sol to examples?).

pragma solidity ^0.5.0;

import "../token/ERC721/ERC721Full.sol";
import "../token/ERC721/ERC721Mintable.sol";
import "../drafts/Strings.sol";

/**
 * @title MyERC721
 */
contract MyERC721 is ERC721Full, ERC721Mintable {
    constructor () public ERC721Mintable() ERC721Full("MyERC721", "MY721") {
        // solhint-disable-previous-line no-empty-blocks
        _setBaseTokenURI("https://baseUri/");
    }

    /**
     * @dev Function to mint tokens
     * @param to The address that will receive the minted tokens.
     * @return A boolean that indicates if the operation was successful.
     */
    function mint(address to) public onlyMinter returns (bool) {
        uint256 tokenId = totalSupply().add(1);
        _mint(to, tokenId);
        return true;
    }

    function tokenURI(uint256 tokenId) public view returns (string memory) {
        require(_exists(tokenId), "MyERC721: URI query for nonexistent token");
        return string(abi.encodePacked(_baseTokenURI(), Strings.fromUint256(tokenId)));
    }
}

[abcoathup]

Reopening as #1970 doesn't implement an "automatic" token URI. (but is the basis for adding this functionality.

e.g.:

function tokenURI(uint256 tokenId) public view returns (string memory) {
     return string(abi.encodePacked(baseTokenURI(), Strings.fromUint256(tokenId)));
}

Should we be creating an ERC721AutomaticTokenURI contract or is there a better way to handle this? (e.g. creating an example of how to implement?)

[frangio]

As mentioned in #2154, we want to bundle this by default in ERC721.

[KaiRo-at]

FWIW, I proposed a PR for this in #2174 based on the great work @frangio did in #1970.