diff --git a/contracts/mocks/ERC721ReceiverNotImplementedMock.sol b/contracts/mocks/ERC721ReceiverNotImplementedMock.sol new file mode 100644 index 00000000000..e12d7dc7705 --- /dev/null +++ b/contracts/mocks/ERC721ReceiverNotImplementedMock.sol @@ -0,0 +1,14 @@ +pragma solidity ^0.5.0; + + +contract ERC721ReceiverNotImplementedMock{ + bytes4 private _retval; + bool private _reverts; + + event Received(address operator, address from, uint256 tokenId, bytes data, uint256 gas); + + constructor (bytes4 retval, bool reverts) public { + _retval = retval; + _reverts = reverts; + } +} \ No newline at end of file diff --git a/contracts/mocks/ERC721ReceiverRevertsMock.sol b/contracts/mocks/ERC721ReceiverRevertsMock.sol new file mode 100644 index 00000000000..46f3faf809d --- /dev/null +++ b/contracts/mocks/ERC721ReceiverRevertsMock.sol @@ -0,0 +1,23 @@ +pragma solidity ^0.5.0; + +import "../token/ERC721/IERC721Receiver.sol"; + +contract ERC721ReceiverRevertsMock is IERC721Receiver { + bytes4 private _retval; + bool private _reverts; + + event Received(address operator, address from, uint256 tokenId, bytes data, uint256 gas); + + constructor (bytes4 retval, bool reverts) public { + _retval = retval; + _reverts = reverts; + } + + function onERC721Received(address operator, address from, uint256 tokenId, bytes memory data) + public returns (bytes4) + { + require(!_reverts, "ERC721ReceiverMock: Transaction rejected by receiver"); + emit Received(operator, from, tokenId, data, gasleft()); + return _retval; + } +} diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index e5918740892..30990041c52 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -331,8 +331,24 @@ contract ERC721 is Context, ERC165, IERC721 { return true; } - bytes4 retval = IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, _data); - return (retval == _ERC721_RECEIVED); + bytes memory payload = abi.encodeWithSignature( + "onERC721Received(address,address,uint256,bytes)", + msg.sender, + from, + tokenId, + _data + ); + + //solhint-disable-next-line avoid-low-level-calls + (bool success, bytes memory returndata) = to.call(payload); + if (!success) { + if (returndata.length > 0){ + revert(string(returndata)); + }else + revert("ERC721: to address does not implement ERC721Received interface"); + } else {return true;} + // bytes4 retval = IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, _data); + // return (retval == _ERC721_RECEIVED); } /** diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js index 84ea623a174..a811903126b 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -6,6 +6,10 @@ const { shouldSupportInterfaces } = require('../../introspection/SupportsInterfa const ERC721ReceiverMock = artifacts.require('ERC721ReceiverMock.sol'); const ERC721Mock = artifacts.require('ERC721Mock.sol'); +// Required mock contracts +const ERC721NoReceiverMock = artifacts.require('./ERC721ReceiverNotImplementedMock.sol'); +const ERC721ReceiverRevertMock = artifacts.require('./ERC721ReceiverRevertsMock.sol'); + function shouldBehaveLikeERC721 ( creator, minter, @@ -296,25 +300,32 @@ function shouldBehaveLikeERC721 ( describe('to a receiver contract returning unexpected value', function () { it('reverts', async function () { - const invalidReceiver = await ERC721ReceiverMock.new('0x42', false); + const noReceiverImplemented = await ERC721NoReceiverMock.new(RECEIVER_MAGIC_VALUE, true); await expectRevert( - this.token.safeTransferFrom(owner, invalidReceiver.address, tokenId, { from: owner }), - 'ERC721: transfer to non ERC721Receiver implementer' + this.token.safeTransferFrom(owner, noReceiverImplemented.address, tokenId, { from: owner }), + 'ERC721: to address does not implement ERC721Received interface' ); }); }); describe('to a receiver contract that throws', function () { it('reverts', async function () { - const invalidReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, true); + const invalidReceiver = await ERC721ReceiverRevertMock.new(RECEIVER_MAGIC_VALUE, true); await expectRevert( this.token.safeTransferFrom(owner, invalidReceiver.address, tokenId, { from: owner }), - 'ERC721ReceiverMock: reverting' + 'ERC721ReceiverMock: Transaction rejected by receiver' ); }); }); describe('to a contract that does not implement the required function', function () { + it('reverts', async function () { + const noReceiverImplemented = await ERC721NoReceiverMock.new(RECEIVER_MAGIC_VALUE, true); + await expectRevert( + this.token.safeTransferFrom(owner, noReceiverImplemented.address, tokenId, { from: owner }), + 'ERC721: to address does not implement ERC721Received interface' + ); + }); it('reverts', async function () { const invalidReceiver = this.token; await expectRevert.unspecified( @@ -358,20 +369,20 @@ function shouldBehaveLikeERC721 ( context('to a receiver contract returning unexpected value', function () { it('reverts', async function () { - const invalidReceiver = await ERC721ReceiverMock.new('0x42', false); + const noReceiverImplemented = await ERC721NoReceiverMock.new(RECEIVER_MAGIC_VALUE, true); await expectRevert( - this.ERC721Mock.safeMint(invalidReceiver.address, tokenId), - 'ERC721: transfer to non ERC721Receiver implementer' + this.ERC721Mock.safeMint(noReceiverImplemented.address, tokenId), + 'ERC721: to address does not implement ERC721Received interface' ); }); }); context('to a receiver contract that throws', function () { it('reverts', async function () { - const invalidReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, true); + const invalidReceiver = await ERC721ReceiverRevertMock.new(RECEIVER_MAGIC_VALUE, true); await expectRevert( this.ERC721Mock.safeMint(invalidReceiver.address, tokenId), - 'ERC721ReceiverMock: reverting' + 'ERC721ReceiverMock: Transaction rejected by receiver' ); }); });