ERC721WithData should extend ERC721URIStorage

This makes the contracts more consistent with OpenZeppelin examples. No signatures are changed, but there is one slight change in behavior: if you specify a "base URI", then individual token URIs are always understood to be a suffix for that base URI. The previous contract would ignore the base URI when a token URI was specified. Signed-off-by: Andrew Richardson <[email protected]>
hyperledger · Mar 4, 2024 · cb34958 · cb34958
1 parent 8f0f98f
commit cb34958
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 57 deletions.
diff --git a/samples/solidity/contracts/ERC721WithData.sol b/samples/solidity/contracts/ERC721WithData.sol
@@ -5,7 +5,7 @@ pragma solidity ^0.8.0;
 import '@openzeppelin/contracts/access/Ownable.sol';
 import '@openzeppelin/contracts/utils/Context.sol';
 import '@openzeppelin/contracts/utils/Strings.sol';
-import '@openzeppelin/contracts/token/ERC721/ERC721.sol';
+import '@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol';
 import './IERC721WithData.sol';
 
 /**
@@ -23,13 +23,10 @@ import './IERC721WithData.sol';
  *
  * This is a sample only and NOT a reference implementation.
  */
-contract ERC721WithData is Context, Ownable, ERC721, IERC721WithData {
+contract ERC721WithData is Context, Ownable, ERC721URIStorage, IERC721WithData {
     uint256 private _nextTokenId = 1;
     string private _baseTokenURI;
 
-    // Optional mapping for token URIs
-    mapping(uint256 => string) private _tokenURIs;
-
     constructor(
         string memory name,
         string memory symbol,
@@ -40,7 +37,7 @@ contract ERC721WithData is Context, Ownable, ERC721, IERC721WithData {
 
     function supportsInterface(
         bytes4 interfaceId
-    ) public view virtual override(ERC721, IERC165) returns (bool) {
+    ) public view virtual override(ERC721URIStorage, IERC165) returns (bool) {
         return
             interfaceId == type(IERC721WithData).interfaceId ||
             super.supportsInterface(interfaceId);
@@ -96,23 +93,7 @@ contract ERC721WithData is Context, Ownable, ERC721, IERC721WithData {
     }
 
     function _baseURI() internal view virtual override returns (string memory) {
-        bytes memory tempURITest = bytes(_baseTokenURI);
-        if (tempURITest.length == 0) {
-            return 'firefly://token/';
-        } else {
-            return _baseTokenURI;
-        }
-    }
-
-    function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
-        _requireOwned(tokenId);
-        string memory uri = _tokenURIs[tokenId];
-        return uri;
-    }
-
-    function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal {
-        _requireOwned(tokenId);
-        _tokenURIs[tokenId] = _tokenURI;
+        return _baseTokenURI;
     }
 
     function baseTokenUri() public view virtual override returns (string memory) {

diff --git a/samples/solidity/test/ERC721WithData.ts b/samples/solidity/test/ERC721WithData.ts
@@ -35,7 +35,7 @@ describe('ERC721WithData - Unit Tests', async function () {
   it('Create - Should create a new ERC721 instance with default state', async function () {
     expect(await deployedERC721WithData.name()).to.equal(contractName);
     expect(await deployedERC721WithData.symbol()).to.equal(contractSymbol);
-    expect(await deployedERC721WithData.baseTokenUri()).to.equal('firefly://token/');
+    expect(await deployedERC721WithData.baseTokenUri()).to.equal('');
   });
 
   it('Mint - Should mint successfully with a custom URI', async function () {
@@ -73,7 +73,6 @@ describe('ERC721WithData - Unit Tests', async function () {
       .to.emit(deployedERC721WithData, 'Transfer')
       .withArgs(ZERO_ADDRESS, deployerSignerA.address, 1);
     expect(await deployedERC721WithData.balanceOf(deployerSignerA.address)).to.equal(1);
-    expect(await deployedERC721WithData.tokenURI(1)).to.equal('firefly://token/1');
     // Signer A transfer token to Signer B
     await expect(
       deployedERC721WithData
@@ -248,36 +247,4 @@ describe('ERC721WithData - Unit Tests', async function () {
     expect(await deployedERC721WithData.balanceOf(signerB.address)).to.equal(1);
     expect(await deployedERC721WithData.balanceOf(signerC.address)).to.equal(1);
   });
-
-  it("URI - Minted token URIs should be 'firefly://token/<tokenId>'", async function () {
-    expect(await deployedERC721WithData.balanceOf(deployerSignerA.address)).to.equal(0);
-    expect(await deployedERC721WithData.balanceOf(signerB.address)).to.equal(0);
-    expect(await deployedERC721WithData.balanceOf(signerC.address)).to.equal(0);
-    // Signer A mints token to itself
-    await expect(
-      deployedERC721WithData.connect(deployerSignerA).mintWithData(deployerSignerA.address, '0x00'),
-    )
-      .to.emit(deployedERC721WithData, 'Transfer')
-      .withArgs(ZERO_ADDRESS, deployerSignerA.address, 1);
-    // Signer A mints token to Signer B
-    await expect(
-      deployedERC721WithData.connect(deployerSignerA).mintWithData(signerB.address, '0x00'),
-    )
-      .to.emit(deployedERC721WithData, 'Transfer')
-      .withArgs(ZERO_ADDRESS, signerB.address, 2);
-    // Signer A mints token to Signer C
-    await expect(
-      deployedERC721WithData.connect(deployerSignerA).mintWithData(signerC.address, '0x00'),
-    )
-      .to.emit(deployedERC721WithData, 'Transfer')
-      .withArgs(ZERO_ADDRESS, signerC.address, 3);
-
-    expect(await deployedERC721WithData.tokenURI(1)).to.equal('firefly://token/1');
-    expect(await deployedERC721WithData.tokenURI(2)).to.equal('firefly://token/2');
-    expect(await deployedERC721WithData.tokenURI(3)).to.equal('firefly://token/3');
-
-    expect(await deployedERC721WithData.balanceOf(deployerSignerA.address)).to.equal(1);
-    expect(await deployedERC721WithData.balanceOf(signerB.address)).to.equal(1);
-    expect(await deployedERC721WithData.balanceOf(signerC.address)).to.equal(1);
-  });
 });