diff --git a/contracts/mocks/ERC721Mock.sol b/contracts/mocks/ERC721Mock.sol index b02df5d21..c868a866a 100644 --- a/contracts/mocks/ERC721Mock.sol +++ b/contracts/mocks/ERC721Mock.sol @@ -4,9 +4,17 @@ import "../token/ERC721/ERC721.sol"; /** * @title ERC721Mock - * This mock just provides a public mint and burn functions for testing purposes + * This mock just provides a public safeMint, mint, and burn functions for testing purposes */ contract ERC721Mock is ERC721 { + function safeMint(address to, uint256 tokenId) public { + _safeMint(to, tokenId); + } + + function safeMint(address to, uint256 tokenId, bytes memory _data) public { + _safeMint(to, tokenId, _data); + } + function mint(address to, uint256 tokenId) public { _mint(to, tokenId); } diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 6b949ab63..e4459ecad 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -174,7 +174,24 @@ contract ERC721 is ERC165, IERC721 { * @param _data bytes data to send along with a safe transfer check */ function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory _data) public { - transferFrom(from, to, tokenId); + require(_isApprovedOrOwner(msg.sender, tokenId), "ERC721: transfer caller is not owner nor approved"); + _safeTransferFrom(from, to, tokenId, _data); + } + + /** + * @dev Safely transfers the ownership of a given token ID to another address + * If the target address is a contract, it must implement `onERC721Received`, + * which is called upon a safe transfer, and return the magic value + * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, + * the transfer is reverted. + * Requires the msg.sender to be the owner, approved, or operator + * @param from current owner of the token + * @param to address to receive the ownership of the given token ID + * @param tokenId uint256 ID of the token to be transferred + * @param _data bytes data to send along with a safe transfer check + */ + function _safeTransferFrom(address from, address to, uint256 tokenId, bytes memory _data) internal { + _transferFrom(from, to, tokenId); require(_checkOnERC721Received(from, to, tokenId, _data), "ERC721: transfer to non ERC721Receiver implementer"); } @@ -201,6 +218,36 @@ contract ERC721 is ERC165, IERC721 { return (spender == owner || getApproved(tokenId) == spender || isApprovedForAll(owner, spender)); } + /** + * @dev Internal function to safely mint a new token. + * Reverts if the given token ID already exists. + * If the target address is a contract, it must implement `onERC721Received`, + * which is called upon a safe transfer, and return the magic value + * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, + * the transfer is reverted. + * @param to The address that will own the minted token + * @param tokenId uint256 ID of the token to be minted + */ + function _safeMint(address to, uint256 tokenId) internal { + _safeMint(to, tokenId, ""); + } + + /** + * @dev Internal function to safely mint a new token. + * Reverts if the given token ID already exists. + * If the target address is a contract, it must implement `onERC721Received`, + * which is called upon a safe transfer, and return the magic value + * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, + * the transfer is reverted. + * @param to The address that will own the minted token + * @param tokenId uint256 ID of the token to be minted + * @param _data bytes data to send along with a safe transfer check + */ + function _safeMint(address to, uint256 tokenId, bytes memory _data) internal { + _mint(to, tokenId); + require(_checkOnERC721Received(address(0), to, tokenId, _data), "ERC721: transfer to non ERC721Receiver implementer"); + } + /** * @dev Internal function to mint a new token. * Reverts if the given token ID already exists. diff --git a/contracts/token/ERC721/ERC721Mintable.sol b/contracts/token/ERC721/ERC721Mintable.sol index 98cd7a3a8..faed7ad85 100644 --- a/contracts/token/ERC721/ERC721Mintable.sol +++ b/contracts/token/ERC721/ERC721Mintable.sol @@ -10,7 +10,7 @@ import "../../access/roles/MinterRole.sol"; contract ERC721Mintable is ERC721, MinterRole { /** * @dev Function to mint tokens. - * @param to The address that will receive the minted tokens. + * @param to The address that will receive the minted token. * @param tokenId The token id to mint. * @return A boolean that indicates if the operation was successful. */ @@ -18,4 +18,27 @@ contract ERC721Mintable is ERC721, MinterRole { _mint(to, tokenId); return true; } + + /** + * @dev Function to safely mint tokens. + * @param to The address that will receive the minted token. + * @param tokenId The token id to mint. + * @return A boolean that indicates if the operation was successful. + */ + function safeMint(address to, uint256 tokenId) public onlyMinter returns (bool) { + _safeMint(to, tokenId); + return true; + } + + /** + * @dev Function to safely mint tokens. + * @param to The address that will receive the minted token. + * @param tokenId The token id to mint. + * @param _data bytes data to send along with a safe transfer check. + * @return A boolean that indicates if the operation was successful. + */ + function safeMint(address to, uint256 tokenId, bytes memory _data) public onlyMinter returns (bool) { + _safeMint(to, tokenId, _data); + return true; + } } diff --git a/contracts/token/ERC721/ERC721Pausable.sol b/contracts/token/ERC721/ERC721Pausable.sol index 7cfd39ae6..5080d3808 100644 --- a/contracts/token/ERC721/ERC721Pausable.sol +++ b/contracts/token/ERC721/ERC721Pausable.sol @@ -16,7 +16,7 @@ contract ERC721Pausable is ERC721, Pausable { super.setApprovalForAll(to, approved); } - function transferFrom(address from, address to, uint256 tokenId) public whenNotPaused { - super.transferFrom(from, to, tokenId); + function _transferFrom(address from, address to, uint256 tokenId) internal whenNotPaused { + super._transferFrom(from, to, tokenId); } } diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js index 91a44aef3..f4aa7d64a 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -4,6 +4,7 @@ const { ZERO_ADDRESS } = constants; const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior'); const ERC721ReceiverMock = artifacts.require('ERC721ReceiverMock.sol'); +const ERC721Mock = artifacts.require('ERC721Mock.sol'); function shouldBehaveLikeERC721 ( creator, @@ -324,6 +325,68 @@ function shouldBehaveLikeERC721 ( }); }); + describe('safe mint', function () { + const fourthTokenId = new BN(4); + const tokenId = fourthTokenId; + const data = '0x42'; + + beforeEach(async function () { + this.ERC721Mock = await ERC721Mock.new(); + }); + + describe('via safeMint', function () { // regular minting is tested in ERC721Mintable.test.js and others + it('should call onERC721Received — with data', async function () { + this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, false); + const receipt = await this.ERC721Mock.safeMint(this.receiver.address, tokenId, data); + + await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', { + from: ZERO_ADDRESS, + tokenId: tokenId, + data: data, + }); + }); + + it('should call onERC721Received — without data', async function () { + this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, false); + const receipt = await this.ERC721Mock.safeMint(this.receiver.address, tokenId); + + await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', { + from: ZERO_ADDRESS, + tokenId: tokenId, + }); + }); + + context('to a receiver contract returning unexpected value', function () { + it('reverts', async function () { + const invalidReceiver = await ERC721ReceiverMock.new('0x42', false); + await expectRevert( + this.ERC721Mock.safeMint(invalidReceiver.address, tokenId), + 'ERC721: transfer to non ERC721Receiver implementer' + ); + }); + }); + + context('to a receiver contract that throws', function () { + it('reverts', async function () { + const invalidReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, true); + await expectRevert( + this.ERC721Mock.safeMint(invalidReceiver.address, tokenId), + 'ERC721ReceiverMock: reverting' + ); + }); + }); + + context('to a contract that does not implement the required function', function () { + it('reverts', async function () { + const invalidReceiver = this.token; + await expectRevert.unspecified( + this.ERC721Mock.safeMint(invalidReceiver.address, tokenId) + ); + }); + }); + }); + }); + describe('approve', function () { const tokenId = firstTokenId; diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index 63d411b10..415c95033 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -13,6 +13,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( const thirdTokenId = new BN(3); const unknownTokenId = new BN(4); const MOCK_URI = 'https://example.com'; + const data = '0x42'; describe('like a mintable and burnable ERC721', function () { beforeEach(async function () { @@ -72,6 +73,18 @@ function shouldBehaveLikeMintAndBurnERC721 ( }); }); + describe('safeMint', function () { + it('it can safely mint with data', async function () { + await this.token.methods['safeMint(address,uint256,bytes)'](...[newOwner, thirdTokenId, data], + { from: minter }); + }); + + it('it can safely mint without data', async function () { + await this.token.methods['safeMint(address,uint256)'](...[newOwner, thirdTokenId], + { from: minter }); + }); + }); + describe('burn', function () { const tokenId = firstTokenId; let logs = null;