From c9d1c8feac06f13354d2c474080b9a157039660c Mon Sep 17 00:00:00 2001 From: Simon de la Rouviere Date: Wed, 25 Apr 2018 15:56:02 -0400 Subject: [PATCH 1/4] First WIP of EIP721. --- contracts/eip721/EIP721.sol | 311 +++++++++ .../eip721/EIP721EnumerableInterface.sol | 28 + contracts/eip721/EIP721Interface.sol | 113 ++++ contracts/eip721/EIP721MetadataInterface.sol | 31 + .../eip721/EIP721TokenReceiverInterface.sol | 18 + contracts/eip721/TestEIP721Implementation.sol | 38 ++ contracts/eip721/TestNonStandardReceiver.sol | 6 + contracts/eip721/TestReceiver.sol | 21 + package.json | 2 +- test/eip721/eip721.js | 605 ++++++++++++++++++ 10 files changed, 1172 insertions(+), 1 deletion(-) create mode 100644 contracts/eip721/EIP721.sol create mode 100644 contracts/eip721/EIP721EnumerableInterface.sol create mode 100644 contracts/eip721/EIP721Interface.sol create mode 100644 contracts/eip721/EIP721MetadataInterface.sol create mode 100644 contracts/eip721/EIP721TokenReceiverInterface.sol create mode 100644 contracts/eip721/TestEIP721Implementation.sol create mode 100644 contracts/eip721/TestNonStandardReceiver.sol create mode 100644 contracts/eip721/TestReceiver.sol create mode 100644 test/eip721/eip721.js diff --git a/contracts/eip721/EIP721.sol b/contracts/eip721/EIP721.sol new file mode 100644 index 00000000..016560e9 --- /dev/null +++ b/contracts/eip721/EIP721.sol @@ -0,0 +1,311 @@ +pragma solidity ^0.4.21; +import "./EIP721Interface.sol"; +import "./EIP721MetadataInterface.sol"; +import "./EIP721EnumerableInterface.sol"; +import "./EIP721TokenReceiverInterface.sol"; + +/* +This is a full implementation of all ERC721's features. +Influenced by OpenZeppelin's implementation with some stylistic changes. +https://github.com/OpenZeppelin/zeppelin-solidity/tree/master/contracts/token/ERC721 +*/ + +contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInterface, ERC165Interface { + string public name; + string public symbol; + + // all tokens + uint256[] internal allTokens; + mapping(uint256 => uint256) internal allTokensIndex; + // Array of tokens owned by a specific owner + mapping(address => uint256[]) internal ownedTokens; + // Mapping from token ID to owner + mapping(uint256 => address) internal ownerOfToken; + // Mapping of the token ID to where it is in the owner's array. + mapping(uint256 => uint256) internal ownedTokensIndex; + + // Mapping of a token to a specifically approved owner. + mapping(uint256 => address) internal approvedOwnerOfToken; + + // An operator is allowed to manage all assets of another owner. + mapping(address => mapping (address => bool)) internal approvedOperators; + + mapping(uint256 => string) internal tokenURIs; + + // base ERC721 interface = 0x80ac58cd + // metadata interface = 0x5b5e139f + // enumerable interface = 0x780e9d63 + bytes4 internal constant ERC721_BASE_INTERFACE_SIGNATURE = 0x80ac58cd; + bytes4 internal constant ERC721_METADATA_INTERFACE_SIGNATURE = 0x5b5e139f; + bytes4 internal constant ERC721_ENUMERABLE_INTERFACE_SIGNATURE = 0x780e9d63; + + /* Modifiers */ + modifier tokenExists(uint256 _tokenId) { + require(ownerOfToken[_tokenId] != 0); + _; + } + + // checks: is the owner of the token == msg.sender? + // OR has the owner of the token granted msg.sender access to operate? + modifier allowedToOperate(uint256 _tokenId) { + require(checkIfAllowedToOperate(_tokenId)); + _; + } + + modifier allowedToTransfer(address _from, address _to, uint256 _tokenId) { + require(checkIfAllowedToOperate(_tokenId) || approvedOwnerOfToken[_tokenId] == msg.sender); + require(ownerOfToken[_tokenId] == _from); + require(_to != 0); //not allowed to burn in transfer method + _; + } + + /// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE + /// TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE + /// THEY MAY BE PERMANENTLY LOST + /// @dev Throws unless `msg.sender` is the current owner, an authorized + /// operator, or the approved address for this NFT. Throws if `_from` is + /// not the current owner. Throws if `_to` is the zero address. Throws if + /// `_tokenId` is not a valid NFT. + /// @param _from The current owner of the NFT + /// @param _to The new owner + /// @param _tokenId The NFT to transfer + function transferFrom(address _from, address _to, uint256 _tokenId) external payable + tokenExists(_tokenId) + allowedToTransfer(_from, _to, _tokenId) { + //transfer token + settleTransfer(_from, _to, _tokenId); + } + + /// @notice Transfers the ownership of an NFT from one address to another address + /// @dev Throws unless `msg.sender` is the current owner, an authorized + /// operator, or the approved address for this NFT. Throws if `_from` is + /// not the current owner. Throws if `_to` is the zero address. Throws if + /// `_tokenId` is not a valid NFT. When transfer is complete, this function + /// checks if `_to` is a smart contract (code size > 0). If so, it calls + /// `onERC721Received` on `_to` and throws if the return value is not + /// `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`. + /// @param _from The current owner of the NFT + /// @param _to The new owner + /// @param _tokenId The NFT to transfer + /// @param data Additional data with no specified format, sent in call to `_to` + function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable + tokenExists(_tokenId) + allowedToTransfer(_from, _to, _tokenId) { + settleTransfer(_from, _to, _tokenId); + + // check if a smart contract + uint256 size; + assembly { size := extcodesize(_to) } // solhint-disable-line no-inline-assembly + if (size > 0) { + // call on onERC721Received. + require(EIP721TokenReceiverInterface(_to).onERC721Received(_from, _tokenId, data) == 0xf0b9e5ba); + } + } + + /// @notice Transfers the ownership of an NFT from one address to another address + /// @dev This works identically to the other function with an extra data parameter, + /// except this function just sets data to "" + /// @param _from The current owner of the NFT + /// @param _to The new owner + /// @param _tokenId The NFT to transfer + function safeTransferFrom(address _from, address _to, uint256 _tokenId) external payable + tokenExists(_tokenId) + allowedToTransfer(_from, _to, _tokenId) { + settleTransfer(_from, _to, _tokenId); + + // check if a smart contract + uint256 size; + assembly { size := extcodesize(_to) } // solhint-disable-line no-inline-assembly + if (size > 0) { + // call on onERC721Received. + require(EIP721TokenReceiverInterface(_to).onERC721Received(_from, _tokenId, "") == 0xf0b9e5ba); + } + } + + /// @notice Set or reaffirm the approved address for an NFT + /// @dev The zero address indicates there is no approved address. + /// @dev Throws unless `msg.sender` is the current NFT owner, or an authorized + /// operator of the current owner. + /// @param _approved The new approved NFT controller + /// @param _tokenId The NFT to approve + function approve(address _approved, uint256 _tokenId) external payable + tokenExists(_tokenId) + allowedToOperate(_tokenId) { + address owner = ownerOfToken[_tokenId]; + internalApprove(owner, _approved, _tokenId); + } + + /// @notice Enable or disable approval for a third party ("operator") to manage + /// all of `msg.sender`'s assets. + /// @dev Emits the ApprovalForAll event. The contract MUST allow + /// multiple operators per owner. + /// @param _operator Address to add to the set of authorized operators. + /// @param _approved True if the operator is approved, false to revoke approval + function setApprovalForAll(address _operator, bool _approved) external { + require(_operator != msg.sender); // can't make oneself an operator + approvedOperators[msg.sender][_operator] = _approved; + emit ApprovalForAll(msg.sender, _operator, _approved); + } + + /* public View Functions */ + /// @notice Count NFTs tracked by this contract + /// @return A count of valid NFTs tracked by this contract, where each one of + /// them has an assigned and queryable owner not equal to the zero address + function totalSupply() external view returns (uint256) { + return allTokens.length; + } + + /// @notice Find the owner of an NFT + /// @param _tokenId The identifier for an NFT + /// @dev NFTs assigned to zero address are considered invalid, and queries + /// about them do throw. + /// @return The address of the owner of the NFT + function ownerOf(uint256 _tokenId) external view + tokenExists(_tokenId) returns (address) { + return ownerOfToken[_tokenId]; + } + + /// @notice Enumerate valid NFTs + /// @dev Throws if `_index` >= `totalSupply()`. + /// @param _index A counter less than `totalSupply()` + /// @return The token identifier for the `_index`th NFT, + /// (sort order not specified) + function tokenByIndex(uint256 _index) external view returns (uint256) { + require(_index < allTokens.length); + return allTokens[_index]; + } + + /// @notice Enumerate NFTs assigned to an owner + /// @dev Throws if `_index` >= `balanceOf(_owner)` or if + /// `_owner` is the zero address, representing invalid NFTs. + /// @param _owner An address where we are interested in NFTs owned by them + /// @param _index A counter less than `balanceOf(_owner)` + /// @return The token identifier for the `_index`th NFT assigned to `_owner`, + /// (sort order not specified) + function tokenOfOwnerByIndex(address _owner, uint256 _index) external view + tokenExists(_tokenId) returns (uint256 _tokenId) { + require(_index < ownedTokens[_owner].length); + return ownedTokens[_owner][_index]; + } + + /// @notice Count all NFTs assigned to an owner + /// @dev NFTs assigned to the zero address are considered invalid, and this + /// function throws for queries about the zero address. + /// @param _owner An address for whom to query the balance + /// @return The number of NFTs owned by `_owner`, possibly zero + function balanceOf(address _owner) external view returns (uint256) { + require(_owner != 0); + return ownedTokens[_owner].length; + } + + /// @notice Get the approved address for a single NFT + /// @dev Throws if `_tokenId` is not a valid NFT + /// @param _tokenId The NFT to find the approved address for + // todo: The approved address for this NFT, or the zero address if there is none + function getApproved(uint256 _tokenId) external view + tokenExists(_tokenId) returns (address) { + return approvedOwnerOfToken[_tokenId]; + } + + /// @notice Query if an address is an authorized operator for another address + /// @param _owner The address that owns the NFTs + /// @param _operator The address that acts on behalf of the owner + /// @return True if `_operator` is an approved operator for `_owner`, false otherwise + function isApprovedForAll(address _owner, address _operator) external view returns (bool) { + return approvedOperators[_owner][_operator]; + } + + /// @notice A distinct Uniform Resource Identifier (URI) for a given asset. + /// @dev Throws if `_tokenId` is not a valid NFT. URIs are defined in RFC + /// 3986. The URI may point to a JSON file that conforms to the "ERC721 + /// Metadata JSON Schema". + function tokenURI(uint256 _tokenId) external view returns (string) { + return tokenURIs[_tokenId]; + } + + /// @notice Query if a contract implements an interface + /// @param interfaceID The interface identifier, as specified in ERC-165 + /// @dev Interface identification is specified in ERC-165. This function + /// uses less than 30,000 gas. + /// @return `true` if the contract implements `interfaceID` and + /// `interfaceID` is not 0xffffffff, `false` otherwise + function supportsInterface(bytes4 interfaceID) external view returns (bool) { + + if (interfaceID == ERC721_BASE_INTERFACE_SIGNATURE || + interfaceID == ERC721_METADATA_INTERFACE_SIGNATURE || + interfaceID == ERC721_ENUMERABLE_INTERFACE_SIGNATURE) { + return true; + } else { return false; } + } + + /* -- Internal Functions -- */ + function checkIfAllowedToOperate(uint256 _tokenId) internal view returns (bool) { + return ownerOfToken[_tokenId] == msg.sender || approvedOperators[ownerOfToken[_tokenId]][msg.sender]; + } + + function internalApprove(address _owner, address _approved, uint256 _tokenId) internal { + require(_approved != _owner); //can't approve to owner to itself + + // Note: the following code is equivalent to: require(getApproved(_tokenId) != 0) || _approved != 0); + // However: I found the following easier to read & understand. + if (approvedOwnerOfToken[_tokenId] == 0 && _approved == 0) { + revert(); // add reason for revert? + } else { + approvedOwnerOfToken[_tokenId] = _approved; + emit Approval(_owner, _approved, _tokenId); + } + } + + function settleTransfer(address _from, address _to, uint256 _tokenId) internal { + //clear pending approvals if there are any + if (approvedOwnerOfToken[_tokenId] != 0) { + internalApprove(_from, 0, _tokenId); + } + + removeToken(_from, _tokenId); + addToken(_to, _tokenId); + + emit Transfer(_from, _to, _tokenId); + } + + function addToken(address _to, uint256 _tokenId) internal { + allTokens.push(_tokenId); + allTokensIndex[_tokenId] = allTokens.length-1; + + // set token to be owned by address _to + ownerOfToken[_tokenId] = _to; + // add that token to an array keeping track of tokens owned by that address + ownedTokens[_to].push(_tokenId); + // shorten length + ownedTokensIndex[_tokenId] = ownedTokens[_to].length-1; + } + + function removeToken(address _from, uint256 _tokenId) internal { + + // remove token from allTokens array. + uint256 allIndex = allTokensIndex[_tokenId]; + uint256 allTokensLength = allTokens.length; + //1) Put last item into index of token to be removed. + allTokens[allIndex] = allTokens[allTokensLength - 1]; + allTokensIndex[allTokensLength - 1] = allIndex; + //2) delete last item (since it's now a duplicate) + delete allTokens[allTokensLength-1]; + //3) reduce length of array + allTokens.length -= 1; + + // remove token from owner array. + // get the index of where this token is in the owner's array + uint256 ownerIndex = ownedTokensIndex[_tokenId]; + uint256 ownerLength = ownedTokens[_from].length; + /* Remove Token From Index */ + //1) Put last item into index of token to be removed. + ownedTokens[_from][ownerIndex] = ownedTokens[_from][ownerLength-1]; + ownedTokensIndex[ownerLength-1] = ownerIndex; + //2) delete last item (since it's now a duplicate) + delete ownedTokens[_from][ownerLength-1]; + //3) reduce length of array + ownedTokens[_from].length -= 1; + + delete ownerOfToken[_tokenId]; + } +} diff --git a/contracts/eip721/EIP721EnumerableInterface.sol b/contracts/eip721/EIP721EnumerableInterface.sol new file mode 100644 index 00000000..4659cc07 --- /dev/null +++ b/contracts/eip721/EIP721EnumerableInterface.sol @@ -0,0 +1,28 @@ +pragma solidity ^0.4.21; + + +/// @title ERC-721 Non-Fungible Token Standard, optional enumeration extension +/// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md +/// Note: the ERC-165 identifier for this interface is 0x780e9d63 +interface EIP721EnumerableInterface { + /// @notice Count NFTs tracked by this contract + /// @return A count of valid NFTs tracked by this contract, where each one of + /// them has an assigned and queryable owner not equal to the zero address + function totalSupply() external view returns (uint256); + + /// @notice Enumerate valid NFTs + /// @dev Throws if `_index` >= `totalSupply()`. + /// @param _index A counter less than `totalSupply()` + /// @return The token identifier for the `_index`th NFT, + /// (sort order not specified) + function tokenByIndex(uint256 _index) external view returns (uint256); + + /// @notice Enumerate NFTs assigned to an owner + /// @dev Throws if `_index` >= `balanceOf(_owner)` or if + /// `_owner` is the zero address, representing invalid NFTs. + /// @param _owner An address where we are interested in NFTs owned by them + /// @param _index A counter less than `balanceOf(_owner)` + /// @return The token identifier for the `_index`th NFT assigned to `_owner`, + /// (sort order not specified) + function tokenOfOwnerByIndex(address _owner, uint256 _index) external view returns (uint256 _tokenId); +} diff --git a/contracts/eip721/EIP721Interface.sol b/contracts/eip721/EIP721Interface.sol new file mode 100644 index 00000000..a6583738 --- /dev/null +++ b/contracts/eip721/EIP721Interface.sol @@ -0,0 +1,113 @@ +pragma solidity ^0.4.21; + + +/// @title ERC-721 Non-Fungible Token Standard +/// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md +/// Note: the ERC-165 identifier for this interface is 0x6466353c +interface EIP721Interface { + + /// @dev This emits when ownership of any NFT changes by any mechanism. + /// This event emits when NFTs are created (`from` == 0) and destroyed + /// (`to` == 0). Exception: during contract creation, any number of NFTs + /// may be created and assigned without emitting Transfer. At the time of + /// any transfer, the approved address for that NFT (if any) is reset to none. + event Transfer(address indexed _from, address indexed _to, uint256 _tokenId); + + /// @dev This emits when the approved address for an NFT is changed or + /// reaffirmed. The zero address indicates there is no approved address. + /// When a Transfer event emits, this also indicates that the approved + /// address for that NFT (if any) is reset to none. + event Approval(address indexed _owner, address indexed _approved, uint256 _tokenId); + + /// @dev This emits when an operator is enabled or disabled for an owner. + /// The operator can manage all NFTs of the owner. + event ApprovalForAll(address indexed _owner, address indexed _operator, bool _approved); + + /// @notice Transfers the ownership of an NFT from one address to another address + /// @dev Throws unless `msg.sender` is the current owner, an authorized + /// operator, or the approved address for this NFT. Throws if `_from` is + /// not the current owner. Throws if `_to` is the zero address. Throws if + /// `_tokenId` is not a valid NFT. When transfer is complete, this function + /// checks if `_to` is a smart contract (code size > 0). If so, it calls + /// `onERC721Received` on `_to` and throws if the return value is not + /// `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`. + /// @param _from The current owner of the NFT + /// @param _to The new owner + /// @param _tokenId The NFT to transfer + /// @param data Additional data with no specified format, sent in call to `_to` + function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable; + + /// @notice Transfers the ownership of an NFT from one address to another address + /// @dev This works identically to the other function with an extra data parameter, + /// except this function just sets data to "" + /// @param _from The current owner of the NFT + /// @param _to The new owner + /// @param _tokenId The NFT to transfer + function safeTransferFrom(address _from, address _to, uint256 _tokenId) external payable; + + /// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE + /// TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE + /// THEY MAY BE PERMANENTLY LOST + /// @dev Throws unless `msg.sender` is the current owner, an authorized + /// operator, or the approved address for this NFT. Throws if `_from` is + /// not the current owner. Throws if `_to` is the zero address. Throws if + /// `_tokenId` is not a valid NFT. + /// @param _from The current owner of the NFT + /// @param _to The new owner + /// @param _tokenId The NFT to transfer + function transferFrom(address _from, address _to, uint256 _tokenId) external payable; + + /// @notice Set or reaffirm the approved address for an NFT + /// @dev The zero address indicates there is no approved address. + /// @dev Throws unless `msg.sender` is the current NFT owner, or an authorized + /// operator of the current owner. + /// @param _approved The new approved NFT controller + /// @param _tokenId The NFT to approve + function approve(address _approved, uint256 _tokenId) external payable; + + /// @notice Enable or disable approval for a third party ("operator") to manage + /// all of `msg.sender`'s assets. + /// @dev Emits the ApprovalForAll event. The contract MUST allow + /// multiple operators per owner. + /// @param _operator Address to add to the set of authorized operators. + /// @param _approved True if the operator is approved, false to revoke approval + function setApprovalForAll(address _operator, bool _approved) external; + + /// @notice Get the approved address for a single NFT + /// @dev Throws if `_tokenId` is not a valid NFT + /// @param _tokenId The NFT to find the approved address for + /// @return The approved address for this NFT, or the zero address if there is none + function getApproved(uint256 _tokenId) external view returns (address); + + /// @notice Query if an address is an authorized operator for another address + /// @param _owner The address that owns the NFTs + /// @param _operator The address that acts on behalf of the owner + /// @return True if `_operator` is an approved operator for `_owner`, false otherwise + function isApprovedForAll(address _owner, address _operator) external view returns (bool); + + /// @notice Count all NFTs assigned to an owner + /// @dev NFTs assigned to the zero address are considered invalid, and this + /// function throws for queries about the zero address. + /// @param _owner An address for whom to query the balance + /// @return The number of NFTs owned by `_owner`, possibly zero + function balanceOf(address _owner) external view returns (uint256); + + /// @notice Find the owner of an NFT + /// @param _tokenId The identifier for an NFT + /// @dev NFTs assigned to zero address are considered invalid, and queries + /// about them do throw. + /// @return The address of the owner of the NFT + function ownerOf(uint256 _tokenId) external view returns (address); + +} + + +interface ERC165Interface { + /// @notice Query if a contract implements an interface + /// @param interfaceID The interface identifier, as specified in ERC-165 + /// @dev Interface identification is specified in ERC-165. This function + /// uses less than 30,000 gas. + /// @return `true` if the contract implements `interfaceID` and + /// `interfaceID` is not 0xffffffff, `false` otherwise + function supportsInterface(bytes4 interfaceID) external view returns (bool); +} diff --git a/contracts/eip721/EIP721MetadataInterface.sol b/contracts/eip721/EIP721MetadataInterface.sol new file mode 100644 index 00000000..ec975596 --- /dev/null +++ b/contracts/eip721/EIP721MetadataInterface.sol @@ -0,0 +1,31 @@ +pragma solidity ^0.4.21; + + +/// @title ERC-721 Non-Fungible Token Standard, optional metadata extension +/// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md +/// Note: the ERC-165 identifier for this interface is 0x5b5e139f +/* +Note on implementation: +EIP721 defines visbility, originally as 'external pure', however this is only possible if you hardcode +the name & symbol into the contract. eg + +function name() external pure returns (string _name) { + return 'NFT name'; +} + +However, I don't think is meaningful. This is the only 2 functions that aren't not +implemented as expected. It does not however change the interface signature. +*/ +interface EIP721MetadataInterface { + /// @notice A descriptive name for a collection of NFTs in this contract + // function name() external pure returns (string _name); + + /// @notice An abbreviated name for NFTs in this contract + // function symbol() external pure returns (string _symbol); + + /// @notice A distinct Uniform Resource Identifier (URI) for a given asset. + /// @dev Throws if `_tokenId` is not a valid NFT. URIs are defined in RFC + /// 3986. The URI may point to a JSON file that conforms to the "ERC721 + /// Metadata JSON Schema". + function tokenURI(uint256 _tokenId) external view returns (string); +} diff --git a/contracts/eip721/EIP721TokenReceiverInterface.sol b/contracts/eip721/EIP721TokenReceiverInterface.sol new file mode 100644 index 00000000..d86c3227 --- /dev/null +++ b/contracts/eip721/EIP721TokenReceiverInterface.sol @@ -0,0 +1,18 @@ +pragma solidity ^0.4.21; + + +/// @dev Note: the ERC-165 identifier for this interface is 0xf0b9e5ba +interface EIP721TokenReceiverInterface { + /// @notice Handle the receipt of an NFT + /// @dev The ERC721 smart contract calls this function on the recipient + /// after a `transfer`. 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. + /// Note: the contract address is always the message sender. + /// @param _from The sending address + /// @param _tokenId The NFT identifier which is being transfered + /// @param data Additional data with no specified format + /// @return `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + /// unless throwing + function onERC721Received(address _from, uint256 _tokenId, bytes data) external returns(bytes4); +} diff --git a/contracts/eip721/TestEIP721Implementation.sol b/contracts/eip721/TestEIP721Implementation.sol new file mode 100644 index 00000000..b7ee7cf7 --- /dev/null +++ b/contracts/eip721/TestEIP721Implementation.sol @@ -0,0 +1,38 @@ +pragma solidity ^0.4.21; +import "./EIP721.sol"; + + +/* +A Test Implementation that allows and admin to mint/burn NFTs. +--- +The API names here should not be regarded as conforming to a specific API. +Perhaps EIP 612 should be used here: https://github.com/ethereum/EIPs/pull/621 +*/ +contract TestEIP721Implementation is EIP721 { + address public admin; + uint256 public counter = 0; + + function TestEIP721Implementation() public { + admin = msg.sender; + name = "Test Collectible"; + symbol = "TCL"; + } + + function createToken(address _minter) public { + require(msg.sender == admin); + addToken(_minter, counter); + emit Transfer(0, _minter, counter); + counter += 1; // every new token gets a new ID + } + + function burnToken(uint256 _tokenId) public { + require(ownerOfToken[_tokenId] == msg.sender); //token should be in control of owner + removeToken(msg.sender, _tokenId); + emit Transfer(msg.sender, 0, _tokenId); + } + + function setTokenURI(uint256 _tokenID, string URI) public { + require(msg.sender == admin); + tokenURIs[_tokenID] = URI; + } +} diff --git a/contracts/eip721/TestNonStandardReceiver.sol b/contracts/eip721/TestNonStandardReceiver.sol new file mode 100644 index 00000000..1caa57d5 --- /dev/null +++ b/contracts/eip721/TestNonStandardReceiver.sol @@ -0,0 +1,6 @@ +pragma solidity ^0.4.21; + +// solhint-disable +contract TestNonStandardReceiver { + // call to onERC721Received will call the fallback function and subsequently throw. +} diff --git a/contracts/eip721/TestReceiver.sol b/contracts/eip721/TestReceiver.sol new file mode 100644 index 00000000..cf19480d --- /dev/null +++ b/contracts/eip721/TestReceiver.sol @@ -0,0 +1,21 @@ +pragma solidity ^0.4.21; +import "./EIP721TokenReceiverInterface.sol"; + + +contract TestReceiver { + + /// @notice Handle the receipt of an NFT + /// @dev The ERC721 smart contract calls this function on the recipient + /// after a `transfer`. 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. + /// Note: the contract address is always the message sender. + /// @param _from The sending address + /// @param _tokenId The NFT identifier which is being transfered + /// @param data Additional data with no specified format + /// @return `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + /// unless throwing + function onERC721Received(address _from, uint256 _tokenId, bytes data) public pure returns(bytes4) { + return 0xf0b9e5ba; + } +} diff --git a/package.json b/package.json index f12d0362..8d773125 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ }, "homepage": "https://github.com/ConsenSys/Tokens#readme", "dependencies": { - "truffle": "4.1.5", + "truffle": "4.1.7", "truffle-hdwallet-provider": "0.0.3" }, "devDependencies": { diff --git a/test/eip721/eip721.js b/test/eip721/eip721.js new file mode 100644 index 00000000..b27c5a2d --- /dev/null +++ b/test/eip721/eip721.js @@ -0,0 +1,605 @@ +const { assertRevert } = require('../helpers/assertRevert'); + +const testEip721 = artifacts.require('TestEIP721Implementation'); +const testReceiver = artifacts.require('TestReceiver'); +const TestNonStandardReceiver = artifacts.require('TestNonStandardReceiver'); +let nft; +let admin; + +// NOTE: This disable is for all the event logs args having underscores +/* eslint-disable no-underscore-dangle */ + +contract('TestEIP72Implementation', (accounts) => { + beforeEach(async () => { + nft = await testEip721.new({ gas: 6720000, from: accounts[0] }); + admin = await nft.admin.call(); + }); + + it('creation: create one token', async () => { + const result = await nft.createToken(admin, { from: accounts[0] }); + + // verify Transfer event (from == 0 if creating token) + assert.strictEqual(result.logs[0].event, 'Transfer'); + assert.strictEqual(result.logs[0].args._from, '0x0000000000000000000000000000000000000000'); + assert.strictEqual(result.logs[0].args._to, admin); + assert.strictEqual(result.logs[0].args._tokenId.toString(), '0'); + + const totalSupply = await nft.totalSupply.call(); + const adminBalance = await nft.balanceOf.call(admin); + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(totalSupply.toString(), '1'); + assert.strictEqual(adminBalance.toString(), '1'); + assert.strictEqual(admin, owner); + }); + + it('creation: create one token then retrieve one that does not exist (should fail)', async () => { + await nft.createToken(admin, { from: accounts[0] }); + + const owner = await nft.ownerOf.call(0); + assert.strictEqual(admin, owner); + await assertRevert(nft.ownerOf.call(1)); + }); + + it('creation: create multiple tokens to one user', async () => { + await nft.createToken(accounts[0], { from: accounts[0] }); + await nft.createToken(accounts[0], { from: accounts[0] }); + + const user1 = await nft.ownerOf.call(0); + const user2 = await nft.ownerOf.call(1); + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[0]); + + assert.strictEqual(totalSupply.toString(), '2'); + assert.strictEqual(balance.toString(), '2'); + assert.strictEqual(user1, accounts[0]); + assert.strictEqual(user2, accounts[0]); + }); + + it('creation: create multiple tokens to multiple users', async () => { + await nft.createToken(accounts[0], { from: accounts[0] }); + await nft.createToken(accounts[0], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + const totalSupply = await nft.totalSupply.call(); + const user1 = await nft.ownerOf.call(0); + const user2 = await nft.ownerOf.call(1); + const user3 = await nft.ownerOf.call(2); + const user4 = await nft.ownerOf.call(3); + const balance1 = await nft.balanceOf.call(accounts[0]); + const balance2 = await nft.balanceOf.call(accounts[1]); + + assert.strictEqual(totalSupply.toString(), '4'); + assert.strictEqual(balance1.toString(), '2'); + assert.strictEqual(balance2.toString(), '2'); + assert.strictEqual(user1, accounts[0]); + assert.strictEqual(user2, accounts[0]); + assert.strictEqual(user3, accounts[1]); + assert.strictEqual(user4, accounts[1]); + }); + + it('burn: create one token the burn/remove it', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + const burn = await nft.burnToken(0, { from: accounts[1] }); + // verify Transfer event (to == 0 if burning token) + assert.strictEqual(burn.logs[0].event, 'Transfer'); + assert.strictEqual(burn.logs[0].args._from, accounts[1]); + assert.strictEqual(burn.logs[0].args._to, '0x0000000000000000000000000000000000000000'); + assert.strictEqual(burn.logs[0].args._tokenId.toString(), '0'); + + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[1]); + + assert.strictEqual(totalSupply.toString(), '0'); + assert.strictEqual(balance.toString(), '0'); + await assertRevert(nft.ownerOf.call(0)); + }); + + it('burn: create one token and burn/remove a different ID (should fail)', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await assertRevert(nft.burnToken(1, { from: accounts[1] })); + }); + + it('burn: create one token and burn/remove from a different owner (should fail)', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await assertRevert(nft.burnToken(0, { from: accounts[2] })); + }); + + it('creation: create 2 tokens to one user and then burn/remove one (second token).', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.burnToken(1, { from: accounts[1] }); + + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[1]); + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(totalSupply.toString(), '1'); + assert.strictEqual(balance.toString(), '1'); + assert.strictEqual(owner, accounts[1]); + await assertRevert(nft.ownerOf.call(1)); + }); + + it('creation: create 2 token to one user then burn one and then create new one to same user.', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.burnToken(1, { from: accounts[1] }); + + await nft.createToken(accounts[1], { from: accounts[0] }); + + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[1]); + const owner = await nft.ownerOf.call(0); + const owner2 = await nft.ownerOf.call(2); + + assert.strictEqual(totalSupply.toString(), '2'); + assert.strictEqual(balance.toString(), '2'); + assert.strictEqual(owner, accounts[1]); + assert.strictEqual(owner2, accounts[1]); + await assertRevert(nft.ownerOf.call(1)); + }); + + it('creation: create 3 tokens to one user and then remove middle token.', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.burnToken(1, { from: accounts[1] }); + + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[1]); + const owner = await nft.ownerOf.call(0); // accounts 1 + const owner2 = await nft.ownerOf.call(2); // accounts 1 + + assert.strictEqual(totalSupply.toString(), '2'); + assert.strictEqual(balance.toString(), '2'); + assert.strictEqual(owner, accounts[1]); + assert.strictEqual(owner2, accounts[1]); + await assertRevert(nft.ownerOf.call(1)); + }); + + it('creation: create 3 tokens to one user and then remove first token.', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.burnToken(0, { from: accounts[1] }); + + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[1]); + const owner = await nft.ownerOf.call(1); + const owner2 = await nft.ownerOf.call(2); + + assert.strictEqual(totalSupply.toString(), '2'); + assert.strictEqual(balance.toString(), '2'); + assert.strictEqual(owner, accounts[1]); + assert.strictEqual(owner2, accounts[1]); + await assertRevert(nft.ownerOf.call(0)); + }); + + it('creation: create 3 tokens to one user and then remove last token.', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.burnToken(2, { from: accounts[1] }); + + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[1]); + const owner = await nft.ownerOf.call(0); + const owner2 = await nft.ownerOf.call(1); + + assert.strictEqual(totalSupply.toString(), '2'); + assert.strictEqual(balance.toString(), '2'); + assert.strictEqual(owner, accounts[1]); + assert.strictEqual(owner2, accounts[1]); + await assertRevert(nft.ownerOf.call(2)); + }); + + // create 3 token to one user and then burn/remove all. + it('creation: create 5 tokens to one user and then burn/remove all.', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.burnToken(0, { from: accounts[1] }); + await nft.burnToken(1, { from: accounts[1] }); + await nft.burnToken(2, { from: accounts[1] }); + await nft.burnToken(3, { from: accounts[1] }); + await nft.burnToken(4, { from: accounts[1] }); + + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[1]); + + assert.strictEqual(totalSupply.toString(), '0'); + assert.strictEqual(balance.toString(), '0'); + await assertRevert(nft.ownerOf.call(0)); + await assertRevert(nft.ownerOf.call(1)); + await assertRevert(nft.ownerOf.call(2)); + }); + + it('transfer: transfer token successfully', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + const result = await nft.transferFrom(accounts[1], accounts[2], 0, { from: accounts[1] }); + // verify Transfer event + assert.strictEqual(result.logs[0].event, 'Transfer'); + assert.strictEqual(result.logs[0].args._from, accounts[1]); + assert.strictEqual(result.logs[0].args._to, accounts[2]); + assert.strictEqual(result.logs[0].args._tokenId.toString(), '0'); + + const totalSupply = await nft.totalSupply.call(); + + const balance1 = await nft.balanceOf.call(accounts[1]); + + const balance2 = await nft.balanceOf.call(accounts[2]); + + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(totalSupply.toString(), '1'); + assert.strictEqual(balance1.toString(), '0'); + assert.strictEqual(balance2.toString(), '1'); + + assert.strictEqual(owner, accounts[2]); + }); + + it('transfer: transfer token to oneself (should be allowed)', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.transferFrom(accounts[1], accounts[1], 0, { from: accounts[1] }); + + const totalSupply = await nft.totalSupply.call(); + const balance = await nft.balanceOf.call(accounts[1]); + + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(totalSupply.toString(), '1'); + assert.strictEqual(balance.toString(), '1'); + assert.strictEqual(owner, accounts[1]); + }); + + it('transfer: transfer token fail by token not exisitng', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await assertRevert(nft.transferFrom(accounts[1], accounts[1], 1, { from: accounts[1] })); + }); + + it('transfer: transfer token fail by token sending to zero', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await assertRevert(nft.transferFrom(accounts[1], 0, 0, { from: accounts[1] })); + }); + + it('transfer: transfer token fail by not being owner', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await assertRevert(nft.transferFrom(accounts[3], accounts[2], 0, { from: accounts[3] })); + }); + + it('safe transfer: test receiver success', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + const receiver = await testReceiver.new({ gas: 6720000, from: accounts[0] }); + + await nft.safeTransferFrom(accounts[1], receiver.address, 0, { from: accounts[1] }); + + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(owner, receiver.address); + }); + + it('safe transfer: test receiver failure (non standard interface)', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + const receiver = await TestNonStandardReceiver.new({ gas: 6720000, from: accounts[0] }); + + // eslint-disable-next-line max-len + await assertRevert(nft.safeTransferFrom(accounts[1], receiver.address, 0, { from: accounts[1] })); + }); + + it('safe transfer: should succeed to transfer to non contract account', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.safeTransferFrom(accounts[1], accounts[2], 0, { from: accounts[1] }); + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(owner, accounts[2]); + }); + + it('approve: approve token successfully', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + const approve = await nft.approve(accounts[2], 0, { from: accounts[1] }); + + // verify Approval event + assert.strictEqual(approve.logs[0].event, 'Approval'); + assert.strictEqual(approve.logs[0].args._owner, accounts[1]); + assert.strictEqual(approve.logs[0].args._approved, accounts[2]); + assert.strictEqual(approve.logs[0].args._tokenId.toString(), '0'); + + const allowed = await nft.getApproved.call(0); + assert.strictEqual(allowed, accounts[2]); + }); + + it('approve: approve token the clear approval', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.approve(accounts[2], 0, { from: accounts[1] }); + + const approve = await nft.approve(0, 0, { from: accounts[1] }); + + // verify Approval event + assert.strictEqual(approve.logs[0].event, 'Approval'); + assert.strictEqual(approve.logs[0].args._owner, accounts[1]); + assert.strictEqual(approve.logs[0].args._approved, '0x0000000000000000000000000000000000000000'); + assert.strictEqual(approve.logs[0].args._tokenId.toString(), '0'); + + const allowed = await nft.getApproved.call(0); + assert.strictEqual(allowed, '0x0000000000000000000000000000000000000000'); + }); + + it('approve: approve token failure by tokens not existing', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await assertRevert(nft.approve(accounts[2], 1, { from: accounts[1] })); + await assertRevert(nft.approve(accounts[2], 2, { from: accounts[1] })); + }); + + it('approve: approve token fail by not being owner', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await assertRevert(nft.approve(accounts[2], 0, { from: accounts[3] })); + }); + + it('approve: approve token fail by approving same owner', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await assertRevert(nft.approve(accounts[1], 0, { from: accounts[1] })); + }); + + it('approve: create token to 1, approve token to 2, then successfully then transferFrom to other account 3', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.approve(accounts[2], 0, { from: accounts[1] }); + const transferFrom = await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); // eslint-disable-line max-len + + // verify Approval events (clears approval) + assert.strictEqual(transferFrom.logs[0].event, 'Approval'); + assert.strictEqual(transferFrom.logs[0].args._owner, accounts[1]); + assert.strictEqual(transferFrom.logs[0].args._approved, '0x0000000000000000000000000000000000000000'); + assert.strictEqual(transferFrom.logs[0].args._tokenId.toString(), '0'); + + // verify Transfer events + assert.strictEqual(transferFrom.logs[1].event, 'Transfer'); + assert.strictEqual(transferFrom.logs[1].args._from, accounts[1]); + assert.strictEqual(transferFrom.logs[1].args._to, accounts[3]); + assert.strictEqual(transferFrom.logs[1].args._tokenId.toString(), '0'); + + const totalSupply = await nft.totalSupply.call(); + const balance1 = await nft.balanceOf.call(accounts[1]); + + const balance2 = await nft.balanceOf.call(accounts[3]); + + const owner = await nft.ownerOf.call(0); + const allowed = await nft.getApproved.call(0); + + assert.strictEqual(totalSupply.toString(), '1'); + + assert.strictEqual(balance1.toString(), '0'); + assert.strictEqual(balance2.toString(), '1'); + + assert.strictEqual(owner, accounts[3]); + + assert.strictEqual(allowed, '0x0000000000000000000000000000000000000000'); + }); + + it('approve: create token to 1, approve token to 2, then successfully then transferFrom to approved account 2', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.approve(accounts[2], 0, { from: accounts[1] }); + await nft.transferFrom(accounts[1], accounts[2], 0, { from: accounts[2] }); + + const totalSupply = await nft.totalSupply.call(); + const balance1 = await nft.balanceOf.call(accounts[1]); + + const balance2 = await nft.balanceOf.call(accounts[2]); + + const owner = await nft.ownerOf.call(0); + const allowed = await nft.getApproved.call(0); + + assert.strictEqual(totalSupply.toString(), '1'); + + assert.strictEqual(balance1.toString(), '0'); + assert.strictEqual(balance2.toString(), '1'); + + assert.strictEqual(owner, accounts[2]); + + assert.strictEqual(allowed, '0x0000000000000000000000000000000000000000'); + }); + + it('approve: create 2 tokens to 1, approve tokens to 2, then successfully then transferFrom 1 to account 3', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.approve(accounts[2], 0, { from: accounts[1] }); + await nft.approve(accounts[2], 1, { from: accounts[1] }); + await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); + + const totalSupply = await nft.totalSupply.call(); + const balance1 = await nft.balanceOf.call(accounts[1]); + + const balance2 = await nft.balanceOf.call(accounts[3]); + + const owner1 = await nft.ownerOf.call(0); + const owner2 = await nft.ownerOf.call(1); + const allowed1 = await nft.getApproved.call(0); + const allowed2 = await nft.getApproved.call(1); + + assert.strictEqual(totalSupply.toString(), '2'); + + assert.strictEqual(balance1.toString(), '1'); + assert.strictEqual(balance2.toString(), '1'); + + assert.strictEqual(owner1, accounts[3]); + assert.strictEqual(owner2, accounts[1]); + + assert.strictEqual(allowed1, '0x0000000000000000000000000000000000000000'); + assert.strictEqual(allowed2, accounts[2]); + }); + + it('approve: approve token successfully then fail transferFrom by token not existing', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.approve(accounts[2], 0, { from: accounts[1] }); + + await assertRevert(nft.transferFrom(accounts[1], accounts[3], 1, { from: accounts[2] })); + }); + + it('approve: approve token successfully then fail transferFrom by different owner', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.approve(accounts[2], 0, { from: accounts[1] }); + + await assertRevert(nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[3] })); + }); + + it('approve: approve token successfully then fail transferFrom by transferring to zero', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.approve(accounts[2], 0, { from: accounts[1] }); + await assertRevert(nft.transferFrom(accounts[1], 0, 0, { from: accounts[2] })); + }); + + it('uri: set URI and retrieve it', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.setTokenURI(0, 'newURI', { from: accounts[0] }); + + const uri = await nft.tokenURI.call(0); + assert.strictEqual(uri, 'newURI'); + }); + + it('operators: set operator', async () => { + const result = await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); + + assert.strictEqual(result.logs[0].event, 'ApprovalForAll'); + assert.strictEqual(result.logs[0].args._owner, accounts[1]); + assert.strictEqual(result.logs[0].args._operator, accounts[2]); + assert.strictEqual(result.logs[0].args._approved, true); + + // eslint-disable-next-line max-len + const isApprovedForAll = await nft.isApprovedForAll(accounts[1], accounts[2], { from: accounts[4] }); + + assert.strictEqual(isApprovedForAll, true); + }); + + it('operator: set operator & turn off operator', async () => { + await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); + const result = await nft.setApprovalForAll(accounts[2], false, { from: accounts[1] }); + + assert.strictEqual(result.logs[0].event, 'ApprovalForAll'); + assert.strictEqual(result.logs[0].args._owner, accounts[1]); + assert.strictEqual(result.logs[0].args._operator, accounts[2]); + assert.strictEqual(result.logs[0].args._approved, false); + + // eslint-disable-next-line max-len + const isApprovedForAll = await nft.isApprovedForAll(accounts[1], accounts[2], { from: accounts[4] }); + + assert.strictEqual(isApprovedForAll, false); + }); + + it('operators: set operator & test transferFrom (with no approval)', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); + + await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(owner, accounts[3]); + }); + + it('operators: set operator & approve & test transferFrom', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); + + await nft.approve(accounts[2], 0, { from: accounts[1] }); + const result = await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); + + // verify the clearing of the approval even WITH an operator + assert.strictEqual(result.logs[0].event, 'Approval'); + assert.strictEqual(result.logs[0].args._owner, accounts[1]); + assert.strictEqual(result.logs[0].args._approved, '0x0000000000000000000000000000000000000000'); + assert.strictEqual(result.logs[0].args._tokenId.toString(), '0'); + + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(owner, accounts[3]); + }); + + it('operators: set operator & test transferFrom & failure (transfer from someone else)', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); + + await assertRevert(nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[4] })); + }); + + it('operators: set operator & approve (other token) [1] & test transferFrom for first [0]', async () => { + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); + + await nft.approve(accounts[4], 1, { from: accounts[1] }); + await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); + + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(owner, accounts[3]); + }); + + it('operators: set operator & set approve THROUGH operator for other account & transferFrom ', async () => { + // owner = 1 + // operator = 2 + // approved = 4 + // transferred = 3 + await nft.createToken(accounts[1], { from: accounts[0] }); + + await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); + + await nft.approve(accounts[4], 0, { from: accounts[2] }); // operator approving + await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[4] }); + + const owner = await nft.ownerOf.call(0); + + assert.strictEqual(owner, accounts[3]); + }); + + it('interface checks (ERC165): base, metadata & enumerable interfaces', async () => { + // base ERC721 interface = 0x80ac58cd + // metadata interface = 0x5b5e139f + // enumerable interface = 0x780e9d63 + const baseInterface = await nft.supportsInterface.call(0x80ac58cd); + const metadataInterface = await nft.supportsInterface.call(0x5b5e139f); + const enumerableInterface = await nft.supportsInterface.call(0x780e9d63); + + // a sequence of 4 bytes that aren't implemented + // to check if supportsInterface catches unimplemented interfaces + const notImplementedInterface = await nft.supportsInterface.call(0x780e9d61); + + assert.strictEqual(baseInterface, true); + assert.strictEqual(metadataInterface, true); + assert.strictEqual(enumerableInterface, true); + assert.strictEqual(notImplementedInterface, false); + }); + + it('naming: naming & symbol check', async () => { + const name = await nft.name.call(); + const symbol = await nft.symbol.call(); + + assert.strictEqual(name, 'Test Collectible'); + assert.strictEqual(symbol, 'TCL'); + }); +}); From 99ac72b00d2e510f63c04da824aaa2cbd79d8664 Mon Sep 17 00:00:00 2001 From: Simon de la Rouviere Date: Wed, 9 May 2018 16:36:34 -0400 Subject: [PATCH 2/4] Fixed bug along with addition of other smaller improvements/changes. --- contracts/eip721/EIP721.sol | 34 +-- contracts/eip721/EIP721Interface.sol | 4 +- contracts/eip721/TestEIP721Implementation.sol | 2 +- test/eip721/eip721.js | 208 +++++++++--------- 4 files changed, 127 insertions(+), 121 deletions(-) diff --git a/contracts/eip721/EIP721.sol b/contracts/eip721/EIP721.sol index 016560e9..1b7019f9 100644 --- a/contracts/eip721/EIP721.sol +++ b/contracts/eip721/EIP721.sol @@ -16,6 +16,7 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt // all tokens uint256[] internal allTokens; + // mapping of token IDs to its index in all Tokens array. mapping(uint256 => uint256) internal allTokensIndex; // Array of tokens owned by a specific owner mapping(address => uint256[]) internal ownedTokens; @@ -28,7 +29,7 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt mapping(uint256 => address) internal approvedOwnerOfToken; // An operator is allowed to manage all assets of another owner. - mapping(address => mapping (address => bool)) internal approvedOperators; + mapping(address => mapping (address => bool)) internal operators; mapping(uint256 => string) internal tokenURIs; @@ -38,6 +39,7 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt bytes4 internal constant ERC721_BASE_INTERFACE_SIGNATURE = 0x80ac58cd; bytes4 internal constant ERC721_METADATA_INTERFACE_SIGNATURE = 0x5b5e139f; bytes4 internal constant ERC721_ENUMERABLE_INTERFACE_SIGNATURE = 0x780e9d63; + bytes4 internal constant ONERC721RECEIVED_FUNCTION_SIGNATURE = 0xf0b9e5ba; /* Modifiers */ modifier tokenExists(uint256 _tokenId) { @@ -87,8 +89,8 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt /// @param _from The current owner of the NFT /// @param _to The new owner /// @param _tokenId The NFT to transfer - /// @param data Additional data with no specified format, sent in call to `_to` - function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable + /// @param _data Additional data with no specified format, sent in call to `_to` + function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes _data) external payable tokenExists(_tokenId) allowedToTransfer(_from, _to, _tokenId) { settleTransfer(_from, _to, _tokenId); @@ -98,7 +100,7 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt assembly { size := extcodesize(_to) } // solhint-disable-line no-inline-assembly if (size > 0) { // call on onERC721Received. - require(EIP721TokenReceiverInterface(_to).onERC721Received(_from, _tokenId, data) == 0xf0b9e5ba); + require(EIP721TokenReceiverInterface(_to).onERC721Received(_from, _tokenId, _data) == ONERC721RECEIVED_FUNCTION_SIGNATURE); } } @@ -143,7 +145,7 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt /// @param _approved True if the operator is approved, false to revoke approval function setApprovalForAll(address _operator, bool _approved) external { require(_operator != msg.sender); // can't make oneself an operator - approvedOperators[msg.sender][_operator] = _approved; + operators[msg.sender][_operator] = _approved; emit ApprovalForAll(msg.sender, _operator, _approved); } @@ -212,7 +214,7 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt /// @param _operator The address that acts on behalf of the owner /// @return True if `_operator` is an approved operator for `_owner`, false otherwise function isApprovedForAll(address _owner, address _operator) external view returns (bool) { - return approvedOperators[_owner][_operator]; + return operators[_owner][_operator]; } /// @notice A distinct Uniform Resource Identifier (URI) for a given asset. @@ -240,7 +242,7 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt /* -- Internal Functions -- */ function checkIfAllowedToOperate(uint256 _tokenId) internal view returns (bool) { - return ownerOfToken[_tokenId] == msg.sender || approvedOperators[ownerOfToken[_tokenId]][msg.sender]; + return ownerOfToken[_tokenId] == msg.sender || operators[ownerOfToken[_tokenId]][msg.sender]; } function internalApprove(address _owner, address _approved, uint256 _tokenId) internal { @@ -269,14 +271,16 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt } function addToken(address _to, uint256 _tokenId) internal { + // add new token to all tokens allTokens.push(_tokenId); + // add new token to index of all tokens. allTokensIndex[_tokenId] = allTokens.length-1; // set token to be owned by address _to ownerOfToken[_tokenId] = _to; // add that token to an array keeping track of tokens owned by that address ownedTokens[_to].push(_tokenId); - // shorten length + // add newly pushed to index. ownedTokensIndex[_tokenId] = ownedTokens[_to].length-1; } @@ -287,10 +291,11 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt uint256 allTokensLength = allTokens.length; //1) Put last item into index of token to be removed. allTokens[allIndex] = allTokens[allTokensLength - 1]; - allTokensIndex[allTokensLength - 1] = allIndex; - //2) delete last item (since it's now a duplicate) + //2) Take last item that beens moved to the removed token & update its index + allTokensIndex[allTokens[allTokensLength-1]] = allIndex; + //3) delete last item (since it's now a duplicate) delete allTokens[allTokensLength-1]; - //3) reduce length of array + //4) reduce length of array allTokens.length -= 1; // remove token from owner array. @@ -300,10 +305,11 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt /* Remove Token From Index */ //1) Put last item into index of token to be removed. ownedTokens[_from][ownerIndex] = ownedTokens[_from][ownerLength-1]; - ownedTokensIndex[ownerLength-1] = ownerIndex; - //2) delete last item (since it's now a duplicate) + //2) Take last item that beens moved to the removed token & update its index + ownedTokensIndex[ownedTokens[_from][ownerLength-1]] = ownerIndex; + //3) delete last item (since it's now a duplicate) delete ownedTokens[_from][ownerLength-1]; - //3) reduce length of array + //4) reduce length of array ownedTokens[_from].length -= 1; delete ownerOfToken[_tokenId]; diff --git a/contracts/eip721/EIP721Interface.sol b/contracts/eip721/EIP721Interface.sol index a6583738..cd999320 100644 --- a/contracts/eip721/EIP721Interface.sol +++ b/contracts/eip721/EIP721Interface.sol @@ -34,8 +34,8 @@ interface EIP721Interface { /// @param _from The current owner of the NFT /// @param _to The new owner /// @param _tokenId The NFT to transfer - /// @param data Additional data with no specified format, sent in call to `_to` - function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable; + /// @param _data Additional data with no specified format, sent in call to `_to` + function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes _data) external payable; /// @notice Transfers the ownership of an NFT from one address to another address /// @dev This works identically to the other function with an extra data parameter, diff --git a/contracts/eip721/TestEIP721Implementation.sol b/contracts/eip721/TestEIP721Implementation.sol index b7ee7cf7..38107e93 100644 --- a/contracts/eip721/TestEIP721Implementation.sol +++ b/contracts/eip721/TestEIP721Implementation.sol @@ -10,7 +10,7 @@ Perhaps EIP 612 should be used here: https://github.com/ethereum/EIPs/pull/621 */ contract TestEIP721Implementation is EIP721 { address public admin; - uint256 public counter = 0; + uint256 public counter = 10; function TestEIP721Implementation() public { admin = msg.sender; diff --git a/test/eip721/eip721.js b/test/eip721/eip721.js index b27c5a2d..42b0ba5a 100644 --- a/test/eip721/eip721.js +++ b/test/eip721/eip721.js @@ -22,11 +22,11 @@ contract('TestEIP72Implementation', (accounts) => { assert.strictEqual(result.logs[0].event, 'Transfer'); assert.strictEqual(result.logs[0].args._from, '0x0000000000000000000000000000000000000000'); assert.strictEqual(result.logs[0].args._to, admin); - assert.strictEqual(result.logs[0].args._tokenId.toString(), '0'); + assert.strictEqual(result.logs[0].args._tokenId.toString(), '10'); const totalSupply = await nft.totalSupply.call(); const adminBalance = await nft.balanceOf.call(admin); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(totalSupply.toString(), '1'); assert.strictEqual(adminBalance.toString(), '1'); @@ -36,17 +36,17 @@ contract('TestEIP72Implementation', (accounts) => { it('creation: create one token then retrieve one that does not exist (should fail)', async () => { await nft.createToken(admin, { from: accounts[0] }); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(admin, owner); - await assertRevert(nft.ownerOf.call(1)); + await assertRevert(nft.ownerOf.call(11)); }); it('creation: create multiple tokens to one user', async () => { await nft.createToken(accounts[0], { from: accounts[0] }); await nft.createToken(accounts[0], { from: accounts[0] }); - const user1 = await nft.ownerOf.call(0); - const user2 = await nft.ownerOf.call(1); + const user1 = await nft.ownerOf.call(10); + const user2 = await nft.ownerOf.call(11); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[0]); @@ -63,10 +63,10 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); const totalSupply = await nft.totalSupply.call(); - const user1 = await nft.ownerOf.call(0); - const user2 = await nft.ownerOf.call(1); - const user3 = await nft.ownerOf.call(2); - const user4 = await nft.ownerOf.call(3); + const user1 = await nft.ownerOf.call(10); + const user2 = await nft.ownerOf.call(11); + const user3 = await nft.ownerOf.call(12); + const user4 = await nft.ownerOf.call(13); const balance1 = await nft.balanceOf.call(accounts[0]); const balance2 = await nft.balanceOf.call(accounts[1]); @@ -82,67 +82,67 @@ contract('TestEIP72Implementation', (accounts) => { it('burn: create one token the burn/remove it', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - const burn = await nft.burnToken(0, { from: accounts[1] }); + const burn = await nft.burnToken(10, { from: accounts[1] }); // verify Transfer event (to == 0 if burning token) assert.strictEqual(burn.logs[0].event, 'Transfer'); assert.strictEqual(burn.logs[0].args._from, accounts[1]); assert.strictEqual(burn.logs[0].args._to, '0x0000000000000000000000000000000000000000'); - assert.strictEqual(burn.logs[0].args._tokenId.toString(), '0'); + assert.strictEqual(burn.logs[0].args._tokenId.toString(), '10'); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[1]); assert.strictEqual(totalSupply.toString(), '0'); assert.strictEqual(balance.toString(), '0'); - await assertRevert(nft.ownerOf.call(0)); + await assertRevert(nft.ownerOf.call(10)); }); it('burn: create one token and burn/remove a different ID (should fail)', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await assertRevert(nft.burnToken(1, { from: accounts[1] })); + await assertRevert(nft.burnToken(11, { from: accounts[1] })); }); it('burn: create one token and burn/remove from a different owner (should fail)', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await assertRevert(nft.burnToken(0, { from: accounts[2] })); + await assertRevert(nft.burnToken(10, { from: accounts[2] })); }); it('creation: create 2 tokens to one user and then burn/remove one (second token).', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.burnToken(1, { from: accounts[1] }); + await nft.burnToken(11, { from: accounts[1] }); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[1]); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(totalSupply.toString(), '1'); assert.strictEqual(balance.toString(), '1'); assert.strictEqual(owner, accounts[1]); - await assertRevert(nft.ownerOf.call(1)); + await assertRevert(nft.ownerOf.call(11)); }); it('creation: create 2 token to one user then burn one and then create new one to same user.', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.burnToken(1, { from: accounts[1] }); + await nft.burnToken(11, { from: accounts[1] }); await nft.createToken(accounts[1], { from: accounts[0] }); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[1]); - const owner = await nft.ownerOf.call(0); - const owner2 = await nft.ownerOf.call(2); + const owner = await nft.ownerOf.call(10); + const owner2 = await nft.ownerOf.call(12); assert.strictEqual(totalSupply.toString(), '2'); assert.strictEqual(balance.toString(), '2'); assert.strictEqual(owner, accounts[1]); assert.strictEqual(owner2, accounts[1]); - await assertRevert(nft.ownerOf.call(1)); + await assertRevert(nft.ownerOf.call(11)); }); it('creation: create 3 tokens to one user and then remove middle token.', async () => { @@ -150,18 +150,18 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.burnToken(1, { from: accounts[1] }); + await nft.burnToken(11, { from: accounts[1] }); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[1]); - const owner = await nft.ownerOf.call(0); // accounts 1 - const owner2 = await nft.ownerOf.call(2); // accounts 1 + const owner = await nft.ownerOf.call(10); // accounts 1 + const owner2 = await nft.ownerOf.call(12); // accounts 1 assert.strictEqual(totalSupply.toString(), '2'); assert.strictEqual(balance.toString(), '2'); assert.strictEqual(owner, accounts[1]); assert.strictEqual(owner2, accounts[1]); - await assertRevert(nft.ownerOf.call(1)); + await assertRevert(nft.ownerOf.call(11)); }); it('creation: create 3 tokens to one user and then remove first token.', async () => { @@ -169,18 +169,18 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.burnToken(0, { from: accounts[1] }); + await nft.burnToken(10, { from: accounts[1] }); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[1]); - const owner = await nft.ownerOf.call(1); - const owner2 = await nft.ownerOf.call(2); + const owner = await nft.ownerOf.call(11); + const owner2 = await nft.ownerOf.call(12); assert.strictEqual(totalSupply.toString(), '2'); assert.strictEqual(balance.toString(), '2'); assert.strictEqual(owner, accounts[1]); assert.strictEqual(owner2, accounts[1]); - await assertRevert(nft.ownerOf.call(0)); + await assertRevert(nft.ownerOf.call(10)); }); it('creation: create 3 tokens to one user and then remove last token.', async () => { @@ -188,18 +188,18 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.burnToken(2, { from: accounts[1] }); + await nft.burnToken(12, { from: accounts[1] }); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[1]); - const owner = await nft.ownerOf.call(0); - const owner2 = await nft.ownerOf.call(1); + const owner = await nft.ownerOf.call(10); + const owner2 = await nft.ownerOf.call(11); assert.strictEqual(totalSupply.toString(), '2'); assert.strictEqual(balance.toString(), '2'); assert.strictEqual(owner, accounts[1]); assert.strictEqual(owner2, accounts[1]); - await assertRevert(nft.ownerOf.call(2)); + await assertRevert(nft.ownerOf.call(12)); }); // create 3 token to one user and then burn/remove all. @@ -210,30 +210,30 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.burnToken(0, { from: accounts[1] }); - await nft.burnToken(1, { from: accounts[1] }); - await nft.burnToken(2, { from: accounts[1] }); - await nft.burnToken(3, { from: accounts[1] }); - await nft.burnToken(4, { from: accounts[1] }); + await nft.burnToken(10, { from: accounts[1] }); + await nft.burnToken(11, { from: accounts[1] }); + await nft.burnToken(12, { from: accounts[1] }); + await nft.burnToken(13, { from: accounts[1] }); + await nft.burnToken(14, { from: accounts[1] }); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[1]); assert.strictEqual(totalSupply.toString(), '0'); assert.strictEqual(balance.toString(), '0'); - await assertRevert(nft.ownerOf.call(0)); - await assertRevert(nft.ownerOf.call(1)); - await assertRevert(nft.ownerOf.call(2)); + await assertRevert(nft.ownerOf.call(10)); + await assertRevert(nft.ownerOf.call(11)); + await assertRevert(nft.ownerOf.call(12)); }); it('transfer: transfer token successfully', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - const result = await nft.transferFrom(accounts[1], accounts[2], 0, { from: accounts[1] }); + const result = await nft.transferFrom(accounts[1], accounts[2], 10, { from: accounts[1] }); // verify Transfer event assert.strictEqual(result.logs[0].event, 'Transfer'); assert.strictEqual(result.logs[0].args._from, accounts[1]); assert.strictEqual(result.logs[0].args._to, accounts[2]); - assert.strictEqual(result.logs[0].args._tokenId.toString(), '0'); + assert.strictEqual(result.logs[0].args._tokenId.toString(), '10'); const totalSupply = await nft.totalSupply.call(); @@ -241,7 +241,7 @@ contract('TestEIP72Implementation', (accounts) => { const balance2 = await nft.balanceOf.call(accounts[2]); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(totalSupply.toString(), '1'); assert.strictEqual(balance1.toString(), '0'); @@ -253,12 +253,12 @@ contract('TestEIP72Implementation', (accounts) => { it('transfer: transfer token to oneself (should be allowed)', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.transferFrom(accounts[1], accounts[1], 0, { from: accounts[1] }); + await nft.transferFrom(accounts[1], accounts[1], 10, { from: accounts[1] }); const totalSupply = await nft.totalSupply.call(); const balance = await nft.balanceOf.call(accounts[1]); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(totalSupply.toString(), '1'); assert.strictEqual(balance.toString(), '1'); @@ -268,28 +268,28 @@ contract('TestEIP72Implementation', (accounts) => { it('transfer: transfer token fail by token not exisitng', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await assertRevert(nft.transferFrom(accounts[1], accounts[1], 1, { from: accounts[1] })); + await assertRevert(nft.transferFrom(accounts[1], accounts[1], 11, { from: accounts[1] })); }); it('transfer: transfer token fail by token sending to zero', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await assertRevert(nft.transferFrom(accounts[1], 0, 0, { from: accounts[1] })); + await assertRevert(nft.transferFrom(accounts[1], 0, 10, { from: accounts[1] })); }); it('transfer: transfer token fail by not being owner', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await assertRevert(nft.transferFrom(accounts[3], accounts[2], 0, { from: accounts[3] })); + await assertRevert(nft.transferFrom(accounts[3], accounts[2], 10, { from: accounts[3] })); }); it('safe transfer: test receiver success', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); const receiver = await testReceiver.new({ gas: 6720000, from: accounts[0] }); - await nft.safeTransferFrom(accounts[1], receiver.address, 0, { from: accounts[1] }); + await nft.safeTransferFrom(accounts[1], receiver.address, 10, { from: accounts[1] }); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(owner, receiver.address); }); @@ -299,13 +299,13 @@ contract('TestEIP72Implementation', (accounts) => { const receiver = await TestNonStandardReceiver.new({ gas: 6720000, from: accounts[0] }); // eslint-disable-next-line max-len - await assertRevert(nft.safeTransferFrom(accounts[1], receiver.address, 0, { from: accounts[1] })); + await assertRevert(nft.safeTransferFrom(accounts[1], receiver.address, 10, { from: accounts[1] })); }); it('safe transfer: should succeed to transfer to non contract account', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.safeTransferFrom(accounts[1], accounts[2], 0, { from: accounts[1] }); - const owner = await nft.ownerOf.call(0); + await nft.safeTransferFrom(accounts[1], accounts[2], 10, { from: accounts[1] }); + const owner = await nft.ownerOf.call(10); assert.strictEqual(owner, accounts[2]); }); @@ -313,79 +313,79 @@ contract('TestEIP72Implementation', (accounts) => { it('approve: approve token successfully', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - const approve = await nft.approve(accounts[2], 0, { from: accounts[1] }); + const approve = await nft.approve(accounts[2], 10, { from: accounts[1] }); // verify Approval event assert.strictEqual(approve.logs[0].event, 'Approval'); assert.strictEqual(approve.logs[0].args._owner, accounts[1]); assert.strictEqual(approve.logs[0].args._approved, accounts[2]); - assert.strictEqual(approve.logs[0].args._tokenId.toString(), '0'); + assert.strictEqual(approve.logs[0].args._tokenId.toString(), '10'); - const allowed = await nft.getApproved.call(0); + const allowed = await nft.getApproved.call(10); assert.strictEqual(allowed, accounts[2]); }); it('approve: approve token the clear approval', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.approve(accounts[2], 0, { from: accounts[1] }); + await nft.approve(accounts[2], 10, { from: accounts[1] }); - const approve = await nft.approve(0, 0, { from: accounts[1] }); + const approve = await nft.approve(0, 10, { from: accounts[1] }); // verify Approval event assert.strictEqual(approve.logs[0].event, 'Approval'); assert.strictEqual(approve.logs[0].args._owner, accounts[1]); assert.strictEqual(approve.logs[0].args._approved, '0x0000000000000000000000000000000000000000'); - assert.strictEqual(approve.logs[0].args._tokenId.toString(), '0'); + assert.strictEqual(approve.logs[0].args._tokenId.toString(), '10'); - const allowed = await nft.getApproved.call(0); + const allowed = await nft.getApproved.call(10); assert.strictEqual(allowed, '0x0000000000000000000000000000000000000000'); }); it('approve: approve token failure by tokens not existing', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await assertRevert(nft.approve(accounts[2], 1, { from: accounts[1] })); - await assertRevert(nft.approve(accounts[2], 2, { from: accounts[1] })); + await assertRevert(nft.approve(accounts[2], 11, { from: accounts[1] })); + await assertRevert(nft.approve(accounts[2], 12, { from: accounts[1] })); }); it('approve: approve token fail by not being owner', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await assertRevert(nft.approve(accounts[2], 0, { from: accounts[3] })); + await assertRevert(nft.approve(accounts[2], 10, { from: accounts[3] })); }); it('approve: approve token fail by approving same owner', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await assertRevert(nft.approve(accounts[1], 0, { from: accounts[1] })); + await assertRevert(nft.approve(accounts[1], 10, { from: accounts[1] })); }); it('approve: create token to 1, approve token to 2, then successfully then transferFrom to other account 3', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.approve(accounts[2], 0, { from: accounts[1] }); - const transferFrom = await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); // eslint-disable-line max-len + await nft.approve(accounts[2], 10, { from: accounts[1] }); + const transferFrom = await nft.transferFrom(accounts[1], accounts[3], 10, { from: accounts[2] }); // eslint-disable-line max-len // verify Approval events (clears approval) assert.strictEqual(transferFrom.logs[0].event, 'Approval'); assert.strictEqual(transferFrom.logs[0].args._owner, accounts[1]); assert.strictEqual(transferFrom.logs[0].args._approved, '0x0000000000000000000000000000000000000000'); - assert.strictEqual(transferFrom.logs[0].args._tokenId.toString(), '0'); + assert.strictEqual(transferFrom.logs[0].args._tokenId.toString(), '10'); // verify Transfer events assert.strictEqual(transferFrom.logs[1].event, 'Transfer'); assert.strictEqual(transferFrom.logs[1].args._from, accounts[1]); assert.strictEqual(transferFrom.logs[1].args._to, accounts[3]); - assert.strictEqual(transferFrom.logs[1].args._tokenId.toString(), '0'); + assert.strictEqual(transferFrom.logs[1].args._tokenId.toString(), '10'); const totalSupply = await nft.totalSupply.call(); const balance1 = await nft.balanceOf.call(accounts[1]); const balance2 = await nft.balanceOf.call(accounts[3]); - const owner = await nft.ownerOf.call(0); - const allowed = await nft.getApproved.call(0); + const owner = await nft.ownerOf.call(10); + const allowed = await nft.getApproved.call(10); assert.strictEqual(totalSupply.toString(), '1'); @@ -400,16 +400,16 @@ contract('TestEIP72Implementation', (accounts) => { it('approve: create token to 1, approve token to 2, then successfully then transferFrom to approved account 2', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.approve(accounts[2], 0, { from: accounts[1] }); - await nft.transferFrom(accounts[1], accounts[2], 0, { from: accounts[2] }); + await nft.approve(accounts[2], 10, { from: accounts[1] }); + await nft.transferFrom(accounts[1], accounts[2], 10, { from: accounts[2] }); const totalSupply = await nft.totalSupply.call(); const balance1 = await nft.balanceOf.call(accounts[1]); const balance2 = await nft.balanceOf.call(accounts[2]); - const owner = await nft.ownerOf.call(0); - const allowed = await nft.getApproved.call(0); + const owner = await nft.ownerOf.call(10); + const allowed = await nft.getApproved.call(10); assert.strictEqual(totalSupply.toString(), '1'); @@ -425,19 +425,19 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.approve(accounts[2], 0, { from: accounts[1] }); - await nft.approve(accounts[2], 1, { from: accounts[1] }); - await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); + await nft.approve(accounts[2], 10, { from: accounts[1] }); + await nft.approve(accounts[2], 11, { from: accounts[1] }); + await nft.transferFrom(accounts[1], accounts[3], 10, { from: accounts[2] }); const totalSupply = await nft.totalSupply.call(); const balance1 = await nft.balanceOf.call(accounts[1]); const balance2 = await nft.balanceOf.call(accounts[3]); - const owner1 = await nft.ownerOf.call(0); - const owner2 = await nft.ownerOf.call(1); - const allowed1 = await nft.getApproved.call(0); - const allowed2 = await nft.getApproved.call(1); + const owner1 = await nft.ownerOf.call(10); + const owner2 = await nft.ownerOf.call(11); + const allowed1 = await nft.getApproved.call(10); + const allowed2 = await nft.getApproved.call(11); assert.strictEqual(totalSupply.toString(), '2'); @@ -454,31 +454,31 @@ contract('TestEIP72Implementation', (accounts) => { it('approve: approve token successfully then fail transferFrom by token not existing', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.approve(accounts[2], 0, { from: accounts[1] }); + await nft.approve(accounts[2], 10, { from: accounts[1] }); - await assertRevert(nft.transferFrom(accounts[1], accounts[3], 1, { from: accounts[2] })); + await assertRevert(nft.transferFrom(accounts[1], accounts[3], 11, { from: accounts[2] })); }); it('approve: approve token successfully then fail transferFrom by different owner', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.approve(accounts[2], 0, { from: accounts[1] }); + await nft.approve(accounts[2], 10, { from: accounts[1] }); - await assertRevert(nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[3] })); + await assertRevert(nft.transferFrom(accounts[1], accounts[3], 10, { from: accounts[3] })); }); it('approve: approve token successfully then fail transferFrom by transferring to zero', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.approve(accounts[2], 0, { from: accounts[1] }); - await assertRevert(nft.transferFrom(accounts[1], 0, 0, { from: accounts[2] })); + await nft.approve(accounts[2], 10, { from: accounts[1] }); + await assertRevert(nft.transferFrom(accounts[1], 0, 10, { from: accounts[2] })); }); it('uri: set URI and retrieve it', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); - await nft.setTokenURI(0, 'newURI', { from: accounts[0] }); + await nft.setTokenURI(10, 'newURI', { from: accounts[0] }); - const uri = await nft.tokenURI.call(0); + const uri = await nft.tokenURI.call(10); assert.strictEqual(uri, 'newURI'); }); @@ -515,8 +515,8 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); - await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); - const owner = await nft.ownerOf.call(0); + await nft.transferFrom(accounts[1], accounts[3], 10, { from: accounts[2] }); + const owner = await nft.ownerOf.call(10); assert.strictEqual(owner, accounts[3]); }); @@ -525,16 +525,16 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); - await nft.approve(accounts[2], 0, { from: accounts[1] }); - const result = await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); + await nft.approve(accounts[2], 10, { from: accounts[1] }); + const result = await nft.transferFrom(accounts[1], accounts[3], 10, { from: accounts[2] }); // verify the clearing of the approval even WITH an operator assert.strictEqual(result.logs[0].event, 'Approval'); assert.strictEqual(result.logs[0].args._owner, accounts[1]); assert.strictEqual(result.logs[0].args._approved, '0x0000000000000000000000000000000000000000'); - assert.strictEqual(result.logs[0].args._tokenId.toString(), '0'); + assert.strictEqual(result.logs[0].args._tokenId.toString(), '10'); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(owner, accounts[3]); }); @@ -543,7 +543,7 @@ contract('TestEIP72Implementation', (accounts) => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); - await assertRevert(nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[4] })); + await assertRevert(nft.transferFrom(accounts[1], accounts[3], 10, { from: accounts[4] })); }); it('operators: set operator & approve (other token) [1] & test transferFrom for first [0]', async () => { @@ -552,10 +552,10 @@ contract('TestEIP72Implementation', (accounts) => { await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); - await nft.approve(accounts[4], 1, { from: accounts[1] }); - await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[2] }); + await nft.approve(accounts[4], 11, { from: accounts[1] }); + await nft.transferFrom(accounts[1], accounts[3], 10, { from: accounts[2] }); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(owner, accounts[3]); }); @@ -569,10 +569,10 @@ contract('TestEIP72Implementation', (accounts) => { await nft.setApprovalForAll(accounts[2], true, { from: accounts[1] }); - await nft.approve(accounts[4], 0, { from: accounts[2] }); // operator approving - await nft.transferFrom(accounts[1], accounts[3], 0, { from: accounts[4] }); + await nft.approve(accounts[4], 10, { from: accounts[2] }); // operator approving + await nft.transferFrom(accounts[1], accounts[3], 10, { from: accounts[4] }); - const owner = await nft.ownerOf.call(0); + const owner = await nft.ownerOf.call(10); assert.strictEqual(owner, accounts[3]); }); From 360f0ac1d5f45ab0539fccd7e71cedaaf3853ceb Mon Sep 17 00:00:00 2001 From: Simon de la Rouviere Date: Tue, 5 Jun 2018 10:53:26 +0200 Subject: [PATCH 3/4] Bring in final updates to ERC721. Also bumped the solidity linter. --- contracts/eip721/EIP721Interface.sol | 4 ++-- contracts/eip721/EIP721MetadataInterface.sol | 16 ++-------------- .../eip721/EIP721TokenReceiverInterface.sol | 4 ++-- package.json | 2 +- 4 files changed, 7 insertions(+), 19 deletions(-) diff --git a/contracts/eip721/EIP721Interface.sol b/contracts/eip721/EIP721Interface.sol index cd999320..39761366 100644 --- a/contracts/eip721/EIP721Interface.sol +++ b/contracts/eip721/EIP721Interface.sol @@ -11,13 +11,13 @@ interface EIP721Interface { /// (`to` == 0). Exception: during contract creation, any number of NFTs /// may be created and assigned without emitting Transfer. At the time of /// any transfer, the approved address for that NFT (if any) is reset to none. - event Transfer(address indexed _from, address indexed _to, uint256 _tokenId); + event Transfer(address indexed _from, address indexed _to, uint256 indexed _tokenId); /// @dev This emits when the approved address for an NFT is changed or /// reaffirmed. The zero address indicates there is no approved address. /// When a Transfer event emits, this also indicates that the approved /// address for that NFT (if any) is reset to none. - event Approval(address indexed _owner, address indexed _approved, uint256 _tokenId); + event Approval(address indexed _owner, address indexed _approved, uint256 indexed _tokenId); /// @dev This emits when an operator is enabled or disabled for an owner. /// The operator can manage all NFTs of the owner. diff --git a/contracts/eip721/EIP721MetadataInterface.sol b/contracts/eip721/EIP721MetadataInterface.sol index ec975596..d3740985 100644 --- a/contracts/eip721/EIP721MetadataInterface.sol +++ b/contracts/eip721/EIP721MetadataInterface.sol @@ -4,24 +4,12 @@ pragma solidity ^0.4.21; /// @title ERC-721 Non-Fungible Token Standard, optional metadata extension /// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md /// Note: the ERC-165 identifier for this interface is 0x5b5e139f -/* -Note on implementation: -EIP721 defines visbility, originally as 'external pure', however this is only possible if you hardcode -the name & symbol into the contract. eg - -function name() external pure returns (string _name) { - return 'NFT name'; -} - -However, I don't think is meaningful. This is the only 2 functions that aren't not -implemented as expected. It does not however change the interface signature. -*/ interface EIP721MetadataInterface { /// @notice A descriptive name for a collection of NFTs in this contract - // function name() external pure returns (string _name); + // function name() external view returns (string _name); /// @notice An abbreviated name for NFTs in this contract - // function symbol() external pure returns (string _symbol); + // function symbol() external view returns (string _symbol); /// @notice A distinct Uniform Resource Identifier (URI) for a given asset. /// @dev Throws if `_tokenId` is not a valid NFT. URIs are defined in RFC diff --git a/contracts/eip721/EIP721TokenReceiverInterface.sol b/contracts/eip721/EIP721TokenReceiverInterface.sol index d86c3227..f7b1573a 100644 --- a/contracts/eip721/EIP721TokenReceiverInterface.sol +++ b/contracts/eip721/EIP721TokenReceiverInterface.sol @@ -6,8 +6,8 @@ interface EIP721TokenReceiverInterface { /// @notice Handle the receipt of an NFT /// @dev The ERC721 smart contract calls this function on the recipient /// after a `transfer`. 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 _tokenId The NFT identifier which is being transfered diff --git a/package.json b/package.json index 8d773125..ba77f2ba 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "eslint-plugin-mocha": "4.11.0", "eslint-plugin-node": "5.1.0", "eslint-plugin-react": "7.5.1", - "solhint": "1.1.10", + "solhint": "1.2.1", "solidity-coverage": "0.5.0" } } From ddfa466b63f8f090a238dbc7ef800938c5a7d3f0 Mon Sep 17 00:00:00 2001 From: Simon de la Rouviere Date: Tue, 3 Jul 2018 13:17:17 +0200 Subject: [PATCH 4/4] bump to spec, bump truffle, tidy and patch tests --- .solhintignore | 1 + contracts/eip721/EIP721.sol | 29 ++++++------ .../eip721/EIP721EnumerableInterface.sol | 2 +- contracts/eip721/EIP721Interface.sol | 12 ++--- contracts/eip721/EIP721MetadataInterface.sol | 2 +- .../eip721/EIP721TokenReceiverInterface.sol | 19 ++++---- contracts/eip721/TestEIP721Implementation.sol | 4 +- contracts/eip721/TestNonStandardReceiver.sol | 2 +- contracts/eip721/TestReceiver.sol | 31 ++++++------- package.json | 2 +- test/eip721/eip721.js | 44 +++++++++++++++++-- 11 files changed, 92 insertions(+), 56 deletions(-) create mode 100644 .solhintignore diff --git a/.solhintignore b/.solhintignore new file mode 100644 index 00000000..e2c2d30b --- /dev/null +++ b/.solhintignore @@ -0,0 +1 @@ +Migrations.sol diff --git a/contracts/eip721/EIP721.sol b/contracts/eip721/EIP721.sol index 1b7019f9..783454b7 100644 --- a/contracts/eip721/EIP721.sol +++ b/contracts/eip721/EIP721.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.24; import "./EIP721Interface.sol"; import "./EIP721MetadataInterface.sol"; import "./EIP721EnumerableInterface.sol"; @@ -33,13 +33,10 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt mapping(uint256 => string) internal tokenURIs; - // base ERC721 interface = 0x80ac58cd - // metadata interface = 0x5b5e139f - // enumerable interface = 0x780e9d63 bytes4 internal constant ERC721_BASE_INTERFACE_SIGNATURE = 0x80ac58cd; bytes4 internal constant ERC721_METADATA_INTERFACE_SIGNATURE = 0x5b5e139f; bytes4 internal constant ERC721_ENUMERABLE_INTERFACE_SIGNATURE = 0x780e9d63; - bytes4 internal constant ONERC721RECEIVED_FUNCTION_SIGNATURE = 0xf0b9e5ba; + bytes4 internal constant ONERC721RECEIVED_FUNCTION_SIGNATURE = 0x150b7a02; /* Modifiers */ modifier tokenExists(uint256 _tokenId) { @@ -85,12 +82,12 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt /// `_tokenId` is not a valid NFT. When transfer is complete, this function /// checks if `_to` is a smart contract (code size > 0). If so, it calls /// `onERC721Received` on `_to` and throws if the return value is not - /// `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`. + /// `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`. /// @param _from The current owner of the NFT /// @param _to The new owner /// @param _tokenId The NFT to transfer - /// @param _data Additional data with no specified format, sent in call to `_to` - function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes _data) external payable + /// @param data Additional data with no specified format, sent in call to `_to` + function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable tokenExists(_tokenId) allowedToTransfer(_from, _to, _tokenId) { settleTransfer(_from, _to, _tokenId); @@ -100,7 +97,7 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt assembly { size := extcodesize(_to) } // solhint-disable-line no-inline-assembly if (size > 0) { // call on onERC721Received. - require(EIP721TokenReceiverInterface(_to).onERC721Received(_from, _tokenId, _data) == ONERC721RECEIVED_FUNCTION_SIGNATURE); + require(EIP721TokenReceiverInterface(_to).onERC721Received(msg.sender, _from, _tokenId, data) == ONERC721RECEIVED_FUNCTION_SIGNATURE); } } @@ -120,13 +117,13 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt assembly { size := extcodesize(_to) } // solhint-disable-line no-inline-assembly if (size > 0) { // call on onERC721Received. - require(EIP721TokenReceiverInterface(_to).onERC721Received(_from, _tokenId, "") == 0xf0b9e5ba); + require(EIP721TokenReceiverInterface(_to).onERC721Received(msg.sender, _from, _tokenId, "") == ONERC721RECEIVED_FUNCTION_SIGNATURE); } } - /// @notice Set or reaffirm the approved address for an NFT + /// @notice Change or reaffirm the approved address for an NFT. /// @dev The zero address indicates there is no approved address. - /// @dev Throws unless `msg.sender` is the current NFT owner, or an authorized + /// Throws unless `msg.sender` is the current NFT owner, or an authorized /// operator of the current owner. /// @param _approved The new approved NFT controller /// @param _tokenId The NFT to approve @@ -289,9 +286,9 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt // remove token from allTokens array. uint256 allIndex = allTokensIndex[_tokenId]; uint256 allTokensLength = allTokens.length; - //1) Put last item into index of token to be removed. + //1) Put last tokenID into index of tokenID to be removed. allTokens[allIndex] = allTokens[allTokensLength - 1]; - //2) Take last item that beens moved to the removed token & update its index + //2) Take last tokenID that has been moved to the removed token & update its new index allTokensIndex[allTokens[allTokensLength-1]] = allIndex; //3) delete last item (since it's now a duplicate) delete allTokens[allTokensLength-1]; @@ -303,9 +300,9 @@ contract EIP721 is EIP721Interface, EIP721MetadataInterface, EIP721EnumerableInt uint256 ownerIndex = ownedTokensIndex[_tokenId]; uint256 ownerLength = ownedTokens[_from].length; /* Remove Token From Index */ - //1) Put last item into index of token to be removed. + //1) Put last tokenID into index of token to be removed. ownedTokens[_from][ownerIndex] = ownedTokens[_from][ownerLength-1]; - //2) Take last item that beens moved to the removed token & update its index + //2) Take last item that has been moved to the removed token & update its index ownedTokensIndex[ownedTokens[_from][ownerLength-1]] = ownerIndex; //3) delete last item (since it's now a duplicate) delete ownedTokens[_from][ownerLength-1]; diff --git a/contracts/eip721/EIP721EnumerableInterface.sol b/contracts/eip721/EIP721EnumerableInterface.sol index 4659cc07..dfb4a81f 100644 --- a/contracts/eip721/EIP721EnumerableInterface.sol +++ b/contracts/eip721/EIP721EnumerableInterface.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.24; /// @title ERC-721 Non-Fungible Token Standard, optional enumeration extension diff --git a/contracts/eip721/EIP721Interface.sol b/contracts/eip721/EIP721Interface.sol index 39761366..ded022a7 100644 --- a/contracts/eip721/EIP721Interface.sol +++ b/contracts/eip721/EIP721Interface.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.24; /// @title ERC-721 Non-Fungible Token Standard @@ -30,12 +30,12 @@ interface EIP721Interface { /// `_tokenId` is not a valid NFT. When transfer is complete, this function /// checks if `_to` is a smart contract (code size > 0). If so, it calls /// `onERC721Received` on `_to` and throws if the return value is not - /// `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))`. + /// `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`. /// @param _from The current owner of the NFT /// @param _to The new owner /// @param _tokenId The NFT to transfer - /// @param _data Additional data with no specified format, sent in call to `_to` - function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes _data) external payable; + /// @param data Additional data with no specified format, sent in call to `_to` + function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable; /// @notice Transfers the ownership of an NFT from one address to another address /// @dev This works identically to the other function with an extra data parameter, @@ -57,9 +57,9 @@ interface EIP721Interface { /// @param _tokenId The NFT to transfer function transferFrom(address _from, address _to, uint256 _tokenId) external payable; - /// @notice Set or reaffirm the approved address for an NFT + /// @notice Change or reaffirm the approved address for an NFT. /// @dev The zero address indicates there is no approved address. - /// @dev Throws unless `msg.sender` is the current NFT owner, or an authorized + /// Throws unless `msg.sender` is the current NFT owner, or an authorized /// operator of the current owner. /// @param _approved The new approved NFT controller /// @param _tokenId The NFT to approve diff --git a/contracts/eip721/EIP721MetadataInterface.sol b/contracts/eip721/EIP721MetadataInterface.sol index d3740985..224df41c 100644 --- a/contracts/eip721/EIP721MetadataInterface.sol +++ b/contracts/eip721/EIP721MetadataInterface.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.24; /// @title ERC-721 Non-Fungible Token Standard, optional metadata extension diff --git a/contracts/eip721/EIP721TokenReceiverInterface.sol b/contracts/eip721/EIP721TokenReceiverInterface.sol index f7b1573a..3a8b0fc8 100644 --- a/contracts/eip721/EIP721TokenReceiverInterface.sol +++ b/contracts/eip721/EIP721TokenReceiverInterface.sol @@ -1,18 +1,19 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.24; -/// @dev Note: the ERC-165 identifier for this interface is 0xf0b9e5ba +/// @dev Note: the ERC-165 identifier for this interface is 0x150b7a02. interface EIP721TokenReceiverInterface { /// @notice Handle the receipt of an NFT - /// @dev The ERC721 smart contract calls this function on the recipient + /// Note: the application will get the prior owner of the token /// after a `transfer`. This function MAY throw to revert and reject the - /// transfer. Return of other than the magic value MUST result in the + /// 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 _tokenId The NFT identifier which is being transfered - /// @param data Additional data with no specified format - /// @return `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + /// @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 transferred + /// @param _data Additional data with no specified format + /// @return `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))` /// unless throwing - function onERC721Received(address _from, uint256 _tokenId, bytes data) external returns(bytes4); + function onERC721Received(address _operator, address _from, uint256 _tokenId, bytes _data) external returns(bytes4); } diff --git a/contracts/eip721/TestEIP721Implementation.sol b/contracts/eip721/TestEIP721Implementation.sol index 38107e93..94e8d99b 100644 --- a/contracts/eip721/TestEIP721Implementation.sol +++ b/contracts/eip721/TestEIP721Implementation.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.24; import "./EIP721.sol"; @@ -12,7 +12,7 @@ contract TestEIP721Implementation is EIP721 { address public admin; uint256 public counter = 10; - function TestEIP721Implementation() public { + constructor() public { admin = msg.sender; name = "Test Collectible"; symbol = "TCL"; diff --git a/contracts/eip721/TestNonStandardReceiver.sol b/contracts/eip721/TestNonStandardReceiver.sol index 1caa57d5..e5d6ba90 100644 --- a/contracts/eip721/TestNonStandardReceiver.sol +++ b/contracts/eip721/TestNonStandardReceiver.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.24; // solhint-disable contract TestNonStandardReceiver { diff --git a/contracts/eip721/TestReceiver.sol b/contracts/eip721/TestReceiver.sol index cf19480d..31923440 100644 --- a/contracts/eip721/TestReceiver.sol +++ b/contracts/eip721/TestReceiver.sol @@ -1,21 +1,22 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.24; import "./EIP721TokenReceiverInterface.sol"; -contract TestReceiver { +contract TestReceiver is EIP721TokenReceiverInterface { - /// @notice Handle the receipt of an NFT - /// @dev The ERC721 smart contract calls this function on the recipient - /// after a `transfer`. 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. - /// Note: the contract address is always the message sender. - /// @param _from The sending address - /// @param _tokenId The NFT identifier which is being transfered - /// @param data Additional data with no specified format - /// @return `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` - /// unless throwing - function onERC721Received(address _from, uint256 _tokenId, bytes data) public pure returns(bytes4) { - return 0xf0b9e5ba; + /// @notice Handle the receipt of an NFT + /// Note: the application will get the prior owner of the token + /// after a `transfer`. This function MAY throw to revert and reject the + /// 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 _operator The address which called `safeTransferFrom` function + /// @param _from The address which previously owned the token + /// @param _tokenId The NFT identifier which is being transferred + /// @param _data Additional data with no specified format + /// @return `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))` + /// unless throwing + function onERC721Received(address _operator, address _from, uint256 _tokenId, bytes _data) external returns(bytes4) { //solhint-disable-line no-unused-vars + return bytes4(keccak256("onERC721Received(address,address,uint256,bytes)")); } } diff --git a/package.json b/package.json index ba77f2ba..a5aedbc6 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ }, "homepage": "https://github.com/ConsenSys/Tokens#readme", "dependencies": { - "truffle": "4.1.7", + "truffle": "4.1.13", "truffle-hdwallet-provider": "0.0.3" }, "devDependencies": { diff --git a/test/eip721/eip721.js b/test/eip721/eip721.js index 42b0ba5a..83398011 100644 --- a/test/eip721/eip721.js +++ b/test/eip721/eip721.js @@ -202,8 +202,8 @@ contract('TestEIP72Implementation', (accounts) => { await assertRevert(nft.ownerOf.call(12)); }); - // create 3 token to one user and then burn/remove all. - it('creation: create 5 tokens to one user and then burn/remove all.', async () => { + // create 5 tokens to one user and then burn/remove all, then create 5 more. + it('creation: create 5 tokens to one user, check state and then burn/remove all, then create 5 more, the burn from the other end.', async () => { await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); await nft.createToken(accounts[1], { from: accounts[0] }); @@ -216,14 +216,50 @@ contract('TestEIP72Implementation', (accounts) => { await nft.burnToken(13, { from: accounts[1] }); await nft.burnToken(14, { from: accounts[1] }); - const totalSupply = await nft.totalSupply.call(); - const balance = await nft.balanceOf.call(accounts[1]); + let totalSupply = await nft.totalSupply.call(); + let balance = await nft.balanceOf.call(accounts[1]); assert.strictEqual(totalSupply.toString(), '0'); assert.strictEqual(balance.toString(), '0'); await assertRevert(nft.ownerOf.call(10)); await assertRevert(nft.ownerOf.call(11)); await assertRevert(nft.ownerOf.call(12)); + await assertRevert(nft.ownerOf.call(13)); + await assertRevert(nft.ownerOf.call(14)); + + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + await nft.createToken(accounts[1], { from: accounts[0] }); + + totalSupply = await nft.totalSupply.call(); + balance = await nft.balanceOf.call(accounts[1]); + + assert.strictEqual(totalSupply.toString(), '5'); + assert.strictEqual(balance.toString(), '5'); + assert.equal(await nft.ownerOf.call(15), accounts[1]); + assert.equal(await nft.ownerOf.call(16), accounts[1]); + assert.equal(await nft.ownerOf.call(17), accounts[1]); + assert.equal(await nft.ownerOf.call(18), accounts[1]); + assert.equal(await nft.ownerOf.call(19), accounts[1]); + + await nft.burnToken(19, { from: accounts[1] }); + await nft.burnToken(18, { from: accounts[1] }); + await nft.burnToken(17, { from: accounts[1] }); + await nft.burnToken(16, { from: accounts[1] }); + await nft.burnToken(15, { from: accounts[1] }); + + totalSupply = await nft.totalSupply.call(); + balance = await nft.balanceOf.call(accounts[1]); + + assert.strictEqual(totalSupply.toString(), '0'); + assert.strictEqual(balance.toString(), '0'); + await assertRevert(nft.ownerOf.call(15)); + await assertRevert(nft.ownerOf.call(16)); + await assertRevert(nft.ownerOf.call(17)); + await assertRevert(nft.ownerOf.call(18)); + await assertRevert(nft.ownerOf.call(19)); }); it('transfer: transfer token successfully', async () => {