OpenZeppelin / openzeppelin-contracts

OpenZeppelin Contracts is a library for secure smart contract development.
https://openzeppelin.com/contracts
MIT License
24.79k stars 11.76k forks source link

Add internal view method for _baseURI in ERC1155URIStorage.sol #4209

Open PierDev opened 1 year ago

PierDev commented 1 year ago

🧐 Motivation Hi, My contract inherit from ERC1155URIStorage. In _baseURI I store the ipfs access url: "https://ipfs.io/ipfs/" and then in _tokenURIs, all CID of my token. But on top of that, all tokens need to access to an other ipfs resource, and, because I can't access to _baseURI, I have to store "https://ipfs.io/ipfs/" a new time?

📝 Details A solution could be to add this view function add the end of the contract. / @dev Get _baseURI / function _getBaseURI() internal view returns (string memory) { return _baseURI; }

It can be useful for other use case, is not risky because it's only a view method ...

Amxx commented 1 year ago

Hello @PierDev

I don't understand this part

But on top of that, all tokens need to access to an other ipfs resource

Can you give more details about that part.

Also, please keep in mind that using a centralised web2 provider for IPFS is not recommanded. It is recommanded to do use ipfs://<ipfshash> instead

PierDev commented 1 year ago

Hi @Amxx, Thanks for your quick response! Here are some details to clarify, I hope it's understandable: In my use, all tokens give access to a resource, represented by an IPFS link (it can be a text for example) which will be accessible by a method (getText in the example) But they have different "levels", gold, silver, iron etc... And therefore a different "presentation" image. So I have: _baseURI: "ipfs://" //thanks for the info _tokenURIs = ["Q1...", "Q2..."...] and common_URI: "ipfs://QCommon..."

For my contract I modified ERC1155 URI Storage.sol by adding this so that i do not have to store "ipfs://" an other time: ` /**

Amxx commented 1 year ago

Hello again

2 things:


Also, you might be missing an obvious optimization here:

You can create a folder (lets call it "metadata" for the sake of the explanation, actual name is irrelevant). In this folder you would put the files (or folders) that correspond to your tokens metadata

metadata/1
metadata/2
metadata/3
...
metadata/X

If you "upload" this folder to IPFS, the folder will have a hash of its own (lets call that hash <Qmetadata>) You can then access any of the token info by looking up the token id. For example: ipfs://<Qmetadata>/3

This means you could use a simple ERC1155Uri (not the expensive uri storage) by setting the "gobal" uri to ipfs://<Qmetadata>/{id} (see this)

Example: This ERC721 contract (its an implementation for an upgradeable design) does:

  function _baseURI() internal view virtual override returns (string memory) {
    return string(bytes.concat("ipfs://", bytes(_ipfsHash), "/"));
  }

and then ERC721 does

    function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
        require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");

        string memory baseURI = _baseURI();
        return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : "";
    }
astorAtUST commented 9 months ago

Hi and apologies if it comes off-topic. I've been reviewing this issue as it seemed valid for an implementation I was doing, but, with current implementation of v5.x, it seems that the _exists(tokenId) modifier is no longer available.

Is there any other recommended check to ensure token metadata available or it does need to be implemented adhoc since now?