From 7d8e3ca8b2957d0bdc1bc46fdc3164bd9572078f Mon Sep 17 00:00:00 2001 From: Paul Barclay Date: Thu, 28 Jun 2018 20:11:18 -0700 Subject: [PATCH] Align ERC721 Receiver with current ERC721 standard. (#1047) * Align ERC721 Receiver with current ERC721 standard. Adds a second address field to onERC721Received onERC721Received(address,address,uint256,bytes) Updates the function signature to 0x150b7a02 from 0xf0b9e5ba * Add _operator to onERC721Received * Fix error caused by formatOnSave * Fixed comments on ERC721Receiver Removed "Must use 50,000 gas or less" Corrected the function signature --- contracts/mocks/ERC721ReceiverMock.sol | 9 ++++++--- contracts/token/ERC721/ERC721BasicToken.sol | 10 +++++----- contracts/token/ERC721/ERC721Holder.sol | 2 +- contracts/token/ERC721/ERC721Receiver.sol | 14 ++++++++------ test/token/ERC721/ERC721BasicToken.behaviour.js | 16 ++++++++++++++-- 5 files changed, 34 insertions(+), 17 deletions(-) diff --git a/contracts/mocks/ERC721ReceiverMock.sol b/contracts/mocks/ERC721ReceiverMock.sol index 89e59868b..6fd00b6d4 100644 --- a/contracts/mocks/ERC721ReceiverMock.sol +++ b/contracts/mocks/ERC721ReceiverMock.sol @@ -8,7 +8,8 @@ contract ERC721ReceiverMock is ERC721Receiver { bool reverts; event Received( - address _address, + address _operator, + address _from, uint256 _tokenId, bytes _data, uint256 _gas @@ -20,7 +21,8 @@ contract ERC721ReceiverMock is ERC721Receiver { } function onERC721Received( - address _address, + address _operator, + address _from, uint256 _tokenId, bytes _data ) @@ -29,7 +31,8 @@ contract ERC721ReceiverMock is ERC721Receiver { { require(!reverts); emit Received( - _address, + _operator, + _from, _tokenId, _data, gasleft() // msg.gas was deprecated in solidityv0.4.21 diff --git a/contracts/token/ERC721/ERC721BasicToken.sol b/contracts/token/ERC721/ERC721BasicToken.sol index 05933675b..682e0260e 100644 --- a/contracts/token/ERC721/ERC721BasicToken.sol +++ b/contracts/token/ERC721/ERC721BasicToken.sol @@ -36,9 +36,9 @@ contract ERC721BasicToken is SupportsInterfaceWithLookup, ERC721Basic { using SafeMath for uint256; using AddressUtils for address; - // Equals to `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + // Equals to `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))` // which can be also obtained as `ERC721Receiver(0).onERC721Received.selector` - bytes4 private constant ERC721_RECEIVED = 0xf0b9e5ba; + bytes4 private constant ERC721_RECEIVED = 0x150b7a02; // Mapping from token ID to owner mapping (uint256 => address) internal tokenOwner; @@ -194,7 +194,7 @@ contract ERC721BasicToken is SupportsInterfaceWithLookup, ERC721Basic { * @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,uint256,bytes)"))`; otherwise, + * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, * the transfer is reverted. * * Requires the msg sender to be the owner, approved, or operator @@ -218,7 +218,7 @@ contract ERC721BasicToken is SupportsInterfaceWithLookup, ERC721Basic { * @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,uint256,bytes)"))`; otherwise, + * `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 @@ -346,7 +346,7 @@ contract ERC721BasicToken is SupportsInterfaceWithLookup, ERC721Basic { return true; } bytes4 retval = ERC721Receiver(_to).onERC721Received( - _from, _tokenId, _data); + msg.sender, _from, _tokenId, _data); return (retval == ERC721_RECEIVED); } } diff --git a/contracts/token/ERC721/ERC721Holder.sol b/contracts/token/ERC721/ERC721Holder.sol index 7ffd9d9c1..0f86aa80b 100644 --- a/contracts/token/ERC721/ERC721Holder.sol +++ b/contracts/token/ERC721/ERC721Holder.sol @@ -4,7 +4,7 @@ import "./ERC721Receiver.sol"; contract ERC721Holder is ERC721Receiver { - function onERC721Received(address, uint256, bytes) public returns(bytes4) { + function onERC721Received(address, address, uint256, bytes) public returns(bytes4) { return ERC721_RECEIVED; } } diff --git a/contracts/token/ERC721/ERC721Receiver.sol b/contracts/token/ERC721/ERC721Receiver.sol index fa67a988c..9c72450e6 100644 --- a/contracts/token/ERC721/ERC721Receiver.sol +++ b/contracts/token/ERC721/ERC721Receiver.sol @@ -9,24 +9,26 @@ pragma solidity ^0.4.24; contract ERC721Receiver { /** * @dev Magic value to be returned upon successful reception of an NFT - * Equals to `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`, + * Equals to `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`, * which can be also obtained as `ERC721Receiver(0).onERC721Received.selector` */ - bytes4 internal constant ERC721_RECEIVED = 0xf0b9e5ba; + bytes4 internal constant ERC721_RECEIVED = 0x150b7a02; /** * @notice Handle the receipt of an NFT * @dev The ERC721 smart contract calls this function on the recipient * after a `safetransfer`. This function MAY throw to revert and reject the - * transfer. This function MUST use 50,000 gas or less. Return of other - * than the magic value MUST result in the transaction being reverted. + * transfer. Return of other than the magic value MUST result in the + * transaction being reverted. * Note: the contract address is always the message sender. - * @param _from The sending address + * @param _operator The address which called `safeTransferFrom` function + * @param _from The address which previously owned the token * @param _tokenId The NFT identifier which is being transfered * @param _data Additional data with no specified format - * @return `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + * @return `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))` */ function onERC721Received( + address _operator, address _from, uint256 _tokenId, bytes _data diff --git a/test/token/ERC721/ERC721BasicToken.behaviour.js b/test/token/ERC721/ERC721BasicToken.behaviour.js index 40ae388aa..35f04af76 100644 --- a/test/token/ERC721/ERC721BasicToken.behaviour.js +++ b/test/token/ERC721/ERC721BasicToken.behaviour.js @@ -18,7 +18,7 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { const unknownTokenId = 3; const creator = accounts[0]; const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; - const RECEIVER_MAGIC_VALUE = '0xf0b9e5ba'; + const RECEIVER_MAGIC_VALUE = '0x150b7a02'; describe('like an ERC721BasicToken', function () { beforeEach(async function () { @@ -280,7 +280,19 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) { result.receipt.logs.length.should.be.equal(2); const [log] = decodeLogs([result.receipt.logs[1]], ERC721Receiver, this.receiver.address); log.event.should.be.eq('Received'); - log.args._address.should.be.equal(owner); + log.args._operator.should.be.equal(owner); + log.args._from.should.be.equal(owner); + log.args._tokenId.toNumber().should.be.equal(tokenId); + log.args._data.should.be.equal(data); + }); + + it('should call onERC721Received from approved', async function () { + const result = await transferFun.call(this, owner, this.to, tokenId, { from: approved }); + result.receipt.logs.length.should.be.equal(2); + const [log] = decodeLogs([result.receipt.logs[1]], ERC721Receiver, this.receiver.address); + log.event.should.be.eq('Received'); + log.args._operator.should.be.equal(approved); + log.args._from.should.be.equal(owner); log.args._tokenId.toNumber().should.be.equal(tokenId); log.args._data.should.be.equal(data); });