Simplify ERC721 revert reasons (#3438)

CHANGELOG.md

@@ -12,7 +12,7 @@
  * `SafeCast`: add support for many more types, using procedural code generation. ([#3245](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3245))
  * `MerkleProof`: add `multiProofVerify` to prove multiple values are part of a Merkle tree. ([#3276](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3276))
  * `MerkleProof`: add calldata versions of the functions to avoid copying input arrays to memory and save gas. ([#3200](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3200))
- * `ERC721`, `ERC1155`: simplified revert reasons. ([#3254](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3254))
+ * `ERC721`, `ERC1155`: simplified revert reasons. ([#3254](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3254), ([#3438](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3438)))
  * `ERC721`: removed redundant require statement. ([#3434](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3434))
  * `PaymentSplitter`: add `releasable` getters. ([#3350](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3350))
 

contracts/token/ERC721/ERC721.sol

@@ -69,7 +69,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
      */
     function ownerOf(uint256 tokenId) public view virtual override returns (address) {
         address owner = _owners[tokenId];
-        require(owner != address(0), "ERC721: owner query for nonexistent token");
+        require(owner != address(0), "ERC721: invalid token ID");
         return owner;
     }
 
@@ -91,7 +91,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
      * @dev See {IERC721Metadata-tokenURI}.
      */
     function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
-        require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");
+        _requireMinted(tokenId);
 
         string memory baseURI = _baseURI();
         return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : "";
@@ -125,7 +125,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
      * @dev See {IERC721-getApproved}.
      */
     function getApproved(uint256 tokenId) public view virtual override returns (address) {
-        require(_exists(tokenId), "ERC721: approved query for nonexistent token");
+        _requireMinted(tokenId);
 
         return _tokenApprovals[tokenId];
     }
@@ -374,6 +374,13 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
         emit ApprovalForAll(owner, operator, approved);
     }
 
+    /**
+     * @dev Reverts if the `tokenId` has not been minted yet.
+     */
+    function _requireMinted(uint256 tokenId) internal view virtual {
+        require(_exists(tokenId), "ERC721: invalid token ID");
+    }
+
     /**
      * @dev Internal function to invoke {IERC721Receiver-onERC721Received} on a target address.
      * The call is not executed if the target address is not a contract.

contracts/token/ERC721/extensions/ERC721URIStorage.sol

@@ -18,7 +18,7 @@ abstract contract ERC721URIStorage is ERC721 {
      * @dev See {IERC721Metadata-tokenURI}.
      */
     function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
-        require(_exists(tokenId), "ERC721URIStorage: URI query for nonexistent token");
+        _requireMinted(tokenId);
 
         string memory _tokenURI = _tokenURIs[tokenId];
         string memory base = _baseURI();

test/token/ERC721/ERC721.behavior.js

@@ -66,7 +66,7 @@ function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, another
 
         it('reverts', async function () {
           await expectRevert(
-            this.token.ownerOf(tokenId), 'ERC721: owner query for nonexistent token',
+            this.token.ownerOf(tokenId), 'ERC721: invalid token ID',
           );
         });
       });
@@ -201,7 +201,7 @@ function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, another
           it('reverts', async function () {
             await expectRevert(
               transferFunction.call(this, owner, other, nonExistentTokenId, { from: owner }),
-              'ERC721: owner query for nonexistent token',
+              'ERC721: invalid token ID',
             );
           });
         });
@@ -276,7 +276,7 @@ function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, another
                     nonExistentTokenId,
                     { from: owner },
                   ),
-                  'ERC721: owner query for nonexistent token',
+                  'ERC721: invalid token ID',
                 );
               });
             });
@@ -534,7 +534,7 @@ function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, another
       context('when the given token ID does not exist', function () {
         it('reverts', async function () {
           await expectRevert(this.token.approve(approved, nonExistentTokenId, { from: operator }),
-            'ERC721: owner query for nonexistent token');
+            'ERC721: invalid token ID');
         });
       });
     });
@@ -623,7 +623,7 @@ function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, another
         it('reverts', async function () {
           await expectRevert(
             this.token.getApproved(nonExistentTokenId),
-            'ERC721: approved query for nonexistent token',
+            'ERC721: invalid token ID',
           );
         });
       });
@@ -678,7 +678,7 @@ function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, another
   describe('_burn', function () {
     it('reverts when burning a non-existent token id', async function () {
       await expectRevert(
-        this.token.burn(nonExistentTokenId), 'ERC721: owner query for nonexistent token',
+        this.token.burn(nonExistentTokenId), 'ERC721: invalid token ID',
       );
     });
 
@@ -704,13 +704,13 @@ function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, another
         it('deletes the token', async function () {
           expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1');
           await expectRevert(
-            this.token.ownerOf(firstTokenId), 'ERC721: owner query for nonexistent token',
+            this.token.ownerOf(firstTokenId), 'ERC721: invalid token ID',
           );
         });
 
         it('reverts when burning a token id that has been deleted', async function () {
           await expectRevert(
-            this.token.burn(firstTokenId), 'ERC721: owner query for nonexistent token',
+            this.token.burn(firstTokenId), 'ERC721: invalid token ID',
           );
         });
       });
@@ -846,7 +846,7 @@ function shouldBehaveLikeERC721Enumerable (errorPrefix, owner, newOwner, approve
   describe('_burn', function () {
     it('reverts when burning a non-existent token id', async function () {
       await expectRevert(
-        this.token.burn(firstTokenId), 'ERC721: owner query for nonexistent token',
+        this.token.burn(firstTokenId), 'ERC721: invalid token ID',
       );
     });
 
@@ -906,7 +906,7 @@ function shouldBehaveLikeERC721Metadata (errorPrefix, name, symbol, owner) {
 
       it('reverts when queried for non existent token id', async function () {
         await expectRevert(
-          this.token.tokenURI(nonExistentTokenId), 'ERC721Metadata: URI query for nonexistent token',
+          this.token.tokenURI(nonExistentTokenId), 'ERC721: invalid token ID',
         );
       });
 

test/token/ERC721/extensions/ERC721Burnable.test.js

@@ -37,7 +37,7 @@ contract('ERC721Burnable', function (accounts) {
         it('burns the given token ID and adjusts the balance of the owner', async function () {
           await expectRevert(
             this.token.ownerOf(tokenId),
-            'ERC721: owner query for nonexistent token',
+            'ERC721: invalid token ID',
           );
           expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1');
         });
@@ -60,7 +60,7 @@ contract('ERC721Burnable', function (accounts) {
         context('getApproved', function () {
           it('reverts', async function () {
             await expectRevert(
-              this.token.getApproved(tokenId), 'ERC721: approved query for nonexistent token',
+              this.token.getApproved(tokenId), 'ERC721: invalid token ID',
             );
           });
         });
@@ -69,7 +69,7 @@ contract('ERC721Burnable', function (accounts) {
       describe('when the given token ID was not tracked by this contract', function () {
         it('reverts', async function () {
           await expectRevert(
-            this.token.burn(unknownTokenId, { from: owner }), 'ERC721: owner query for nonexistent token',
+            this.token.burn(unknownTokenId, { from: owner }), 'ERC721: invalid token ID',
           );
         });
       });

test/token/ERC721/extensions/ERC721URIStorage.test.js

@@ -31,7 +31,7 @@ contract('ERC721URIStorage', function (accounts) {
 
     it('reverts when queried for non existent token id', async function () {
       await expectRevert(
-        this.token.tokenURI(nonExistentTokenId), 'ERC721URIStorage: URI query for nonexistent token',
+        this.token.tokenURI(nonExistentTokenId), 'ERC721: invalid token ID',
       );
     });
 
@@ -78,7 +78,7 @@ contract('ERC721URIStorage', function (accounts) {
 
       expect(await this.token.exists(firstTokenId)).to.equal(false);
       await expectRevert(
-        this.token.tokenURI(firstTokenId), 'ERC721URIStorage: URI query for nonexistent token',
+        this.token.tokenURI(firstTokenId), 'ERC721: invalid token ID',
       );
     });
 
@@ -89,7 +89,7 @@ contract('ERC721URIStorage', function (accounts) {
 
       expect(await this.token.exists(firstTokenId)).to.equal(false);
       await expectRevert(
-        this.token.tokenURI(firstTokenId), 'ERC721URIStorage: URI query for nonexistent token',
+        this.token.tokenURI(firstTokenId), 'ERC721: invalid token ID',
       );
     });
   });