diff --git a/contracts/mocks/ERC721BasicTokenMock.sol b/contracts/mocks/ERC721BasicTokenMock.sol index 704728198d6..eaf81181324 100644 --- a/contracts/mocks/ERC721BasicTokenMock.sol +++ b/contracts/mocks/ERC721BasicTokenMock.sol @@ -9,10 +9,25 @@ import "../token/ERC721/ERC721BasicToken.sol"; */ contract ERC721BasicTokenMock is ERC721BasicToken { function mint(address _to, uint256 _tokenId) public { - super._mint(_to, _tokenId); + require(_to != address(0)); + require(!exists(_tokenId)); + + tokenOwner[_tokenId] = _to; + ownedTokensCount[_to] = ownedTokensCount[_to].add(1); + emit Transfer(address(0), _to, _tokenId); } function burn(uint256 _tokenId) public { - super._burn(ownerOf(_tokenId), _tokenId); + address _owner = ownerOf(_tokenId); + require(_owner != address(0)); + + if (tokenApprovals[_tokenId] != address(0)) { + tokenApprovals[_tokenId] = address(0); + emit Approval(_owner, address(0), _tokenId); + } + + tokenOwner[_tokenId] = address(0); + ownedTokensCount[_owner] = ownedTokensCount[_owner].sub(1); + emit Transfer(_owner, address(0), _tokenId); } } diff --git a/contracts/mocks/ERC721TokenMock.sol b/contracts/mocks/ERC721TokenMock.sol index 4695ad2968c..a1101b47858 100644 --- a/contracts/mocks/ERC721TokenMock.sol +++ b/contracts/mocks/ERC721TokenMock.sol @@ -14,11 +14,58 @@ contract ERC721TokenMock is ERC721Token { { } function mint(address _to, uint256 _tokenId) public { - super._mint(_to, _tokenId); + require(_to != address(0)); + require(!exists(_tokenId)); + + tokenOwner[_tokenId] = _to; + ownedTokensCount[_to] = ownedTokensCount[_to].add(1); + + ownedTokensIndex[_tokenId] = ownedTokens[_to].length; + ownedTokens[_to].push(_tokenId); + + allTokensIndex[_tokenId] = allTokens.length; + allTokens.push(_tokenId); + + emit Transfer(address(0), _to, _tokenId); } function burn(uint256 _tokenId) public { - super._burn(ownerOf(_tokenId), _tokenId); + address _owner = ownerOf(_tokenId); + require(_owner != address(0)); + + if (tokenApprovals[_tokenId] != address(0)) { + tokenApprovals[_tokenId] = address(0); + emit Approval(_owner, address(0), _tokenId); + } + + tokenOwner[_tokenId] = address(0); + ownedTokensCount[_owner] = ownedTokensCount[_owner].sub(1); + + uint256 removeOwnedTokenIndex = ownedTokensIndex[_tokenId]; + uint256 lastOwnedTokenIndex = ownedTokens[_owner].length.sub(1); + uint256 lastOwnedToken = ownedTokens[_owner][lastOwnedTokenIndex]; + + ownedTokens[_owner][removeOwnedTokenIndex] = lastOwnedToken; + ownedTokens[_owner][lastOwnedTokenIndex] = 0; + ownedTokens[_owner].length--; + ownedTokensIndex[_tokenId] = 0; + ownedTokensIndex[lastOwnedToken] = removeOwnedTokenIndex; + + if (bytes(tokenURIs[_tokenId]).length != 0) { + delete tokenURIs[_tokenId]; + } + + uint256 removeAllTokenIndex = allTokensIndex[_tokenId]; + uint256 lastAllTokenIndex = allTokens.length.sub(1); + uint256 lastAllToken = allTokens[lastAllTokenIndex]; + + allTokens[removeAllTokenIndex] = lastAllToken; + allTokens[lastAllTokenIndex] = 0; + allTokens.length--; + allTokensIndex[_tokenId] = 0; + allTokensIndex[lastAllToken] = removeAllTokenIndex; + + emit Transfer(_owner, address(0), _tokenId); } function setTokenURI(uint256 _tokenId, string _uri) public { diff --git a/contracts/token/ERC721/ERC721BasicToken.sol b/contracts/token/ERC721/ERC721BasicToken.sol index f2acdbfef8d..dd368c7eed0 100644 --- a/contracts/token/ERC721/ERC721BasicToken.sol +++ b/contracts/token/ERC721/ERC721BasicToken.sol @@ -167,6 +167,9 @@ contract ERC721BasicToken is SupportsInterfaceWithLookup, ERC721Basic { address spender = msg.sender; require( + // Disable solium check because of + // https://github.com/duaraghav8/Solium/issues/175 + // solium-disable-next-line operator-whitespace spender == owner || isApprovedForAll(owner, spender) || getApproved(_tokenId) == spender @@ -232,91 +235,6 @@ contract ERC721BasicToken is SupportsInterfaceWithLookup, ERC721Basic { require(checkAndCallSafeTransfer(_from, _to, _tokenId, _data)); } - /** - * @dev Returns whether the given spender can transfer a given token ID - * @param _spender address of the spender to query - * @param _tokenId uint256 ID of the token to be transferred - * @return bool whether the msg.sender is approved for the given token ID, - * is an operator of the owner, or is the owner of the token - */ - function isApprovedOrOwner( - address _spender, - uint256 _tokenId - ) - internal - view - returns (bool) - { - address owner = ownerOf(_tokenId); - // Disable solium check because of - // https://github.com/duaraghav8/Solium/issues/175 - // solium-disable-next-line operator-whitespace - return ( - _spender == owner || - getApproved(_tokenId) == _spender || - isApprovedForAll(owner, _spender) - ); - } - - /** - * @dev Internal function to mint a new token - * Reverts if the given token ID already exists - * @param _to The address that will own the minted token - * @param _tokenId uint256 ID of the token to be minted by the msg.sender - */ - function _mint(address _to, uint256 _tokenId) internal { - require(_to != address(0)); - addTokenTo(_to, _tokenId); - emit Transfer(address(0), _to, _tokenId); - } - - /** - * @dev Internal function to burn a specific token - * Reverts if the token does not exist - * @param _tokenId uint256 ID of the token being burned by the msg.sender - */ - function _burn(address _owner, uint256 _tokenId) internal { - clearApproval(_owner, _tokenId); - removeTokenFrom(_owner, _tokenId); - emit Transfer(_owner, address(0), _tokenId); - } - - /** - * @dev Internal function to clear current approval of a given token ID - * Reverts if the given address is not indeed the owner of the token - * @param _owner owner of the token - * @param _tokenId uint256 ID of the token to be transferred - */ - function clearApproval(address _owner, uint256 _tokenId) internal { - require(ownerOf(_tokenId) == _owner); - if (tokenApprovals[_tokenId] != address(0)) { - tokenApprovals[_tokenId] = address(0); - emit Approval(_owner, address(0), _tokenId); - } - } - - /** - * @dev Internal function to add a token ID to the list of a given address - * @param _to address representing the new owner of the given token ID - * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address - */ - function addTokenTo(address _to, uint256 _tokenId) internal { - require(tokenOwner[_tokenId] == address(0)); - tokenOwner[_tokenId] = _to; - ownedTokensCount[_to] = ownedTokensCount[_to].add(1); - } - - /** - * @dev Internal function to remove a token ID from the list of a given address - * @param _from address representing the previous owner of the given token ID - * @param _tokenId uint256 ID of the token to be removed from the tokens list of the given address - */ - function removeTokenFrom(address _from, uint256 _tokenId) internal { - require(ownerOf(_tokenId) == _from); - ownedTokensCount[_from] = ownedTokensCount[_from].sub(1); - tokenOwner[_tokenId] = address(0); - } - /** * @dev Internal function to invoke `onERC721Received` on a target address * The call is not executed if the target address is not a contract diff --git a/contracts/token/ERC721/ERC721Token.sol b/contracts/token/ERC721/ERC721Token.sol index 3a55bc142ba..601cbdbbacb 100644 --- a/contracts/token/ERC721/ERC721Token.sol +++ b/contracts/token/ERC721/ERC721Token.sol @@ -123,23 +123,17 @@ contract ERC721Token is SupportsInterfaceWithLookup, ERC721BasicToken, ERC721 { { super.transferFrom(_from, _to, _tokenId); - uint256 tokenIndex = ownedTokensIndex[_tokenId]; + uint256 removeTokenIndex = ownedTokensIndex[_tokenId]; uint256 lastTokenIndex = ownedTokens[_from].length.sub(1); uint256 lastToken = ownedTokens[_from][lastTokenIndex]; - ownedTokens[_from][tokenIndex] = lastToken; + ownedTokens[_from][removeTokenIndex] = lastToken; ownedTokens[_from][lastTokenIndex] = 0; - // Note that this will handle single-element arrays. In that case, both tokenIndex and lastTokenIndex are going to - // be zero. Then we can make sure that we will remove _tokenId from the ownedTokens list since we are first swapping - // the lastToken to the first position, and then dropping the element placed in the last position of the list - ownedTokens[_from].length--; - ownedTokensIndex[_tokenId] = 0; - ownedTokensIndex[lastToken] = tokenIndex; + ownedTokensIndex[lastToken] = removeTokenIndex; - uint256 length = ownedTokens[_to].length; ownedTokens[_to].push(_tokenId); - ownedTokensIndex[_tokenId] = length; + ownedTokensIndex[_tokenId] = ownedTokens[_to].length - 1; } /** * @dev Gets the total amount of tokens stored by the contract @@ -170,80 +164,4 @@ contract ERC721Token is SupportsInterfaceWithLookup, ERC721BasicToken, ERC721 { require(exists(_tokenId)); tokenURIs[_tokenId] = _uri; } - - /** - * @dev Internal function to add a token ID to the list of a given address - * @param _to address representing the new owner of the given token ID - * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address - */ - function addTokenTo(address _to, uint256 _tokenId) internal { - super.addTokenTo(_to, _tokenId); - uint256 length = ownedTokens[_to].length; - ownedTokens[_to].push(_tokenId); - ownedTokensIndex[_tokenId] = length; - } - - /** - * @dev Internal function to remove a token ID from the list of a given address - * @param _from address representing the previous owner of the given token ID - * @param _tokenId uint256 ID of the token to be removed from the tokens list of the given address - */ - function removeTokenFrom(address _from, uint256 _tokenId) internal { - super.removeTokenFrom(_from, _tokenId); - - uint256 tokenIndex = ownedTokensIndex[_tokenId]; - uint256 lastTokenIndex = ownedTokens[_from].length.sub(1); - uint256 lastToken = ownedTokens[_from][lastTokenIndex]; - - ownedTokens[_from][tokenIndex] = lastToken; - ownedTokens[_from][lastTokenIndex] = 0; - // Note that this will handle single-element arrays. In that case, both tokenIndex and lastTokenIndex are going to - // be zero. Then we can make sure that we will remove _tokenId from the ownedTokens list since we are first swapping - // the lastToken to the first position, and then dropping the element placed in the last position of the list - - ownedTokens[_from].length--; - ownedTokensIndex[_tokenId] = 0; - ownedTokensIndex[lastToken] = tokenIndex; - } - - /** - * @dev Internal function to mint a new token - * Reverts if the given token ID already exists - * @param _to address the beneficiary that will own the minted token - * @param _tokenId uint256 ID of the token to be minted by the msg.sender - */ - function _mint(address _to, uint256 _tokenId) internal { - super._mint(_to, _tokenId); - - allTokensIndex[_tokenId] = allTokens.length; - allTokens.push(_tokenId); - } - - /** - * @dev Internal function to burn a specific token - * Reverts if the token does not exist - * @param _owner owner of the token to burn - * @param _tokenId uint256 ID of the token being burned by the msg.sender - */ - function _burn(address _owner, uint256 _tokenId) internal { - super._burn(_owner, _tokenId); - - // Clear metadata (if any) - if (bytes(tokenURIs[_tokenId]).length != 0) { - delete tokenURIs[_tokenId]; - } - - // Reorg all tokens array - uint256 tokenIndex = allTokensIndex[_tokenId]; - uint256 lastTokenIndex = allTokens.length.sub(1); - uint256 lastToken = allTokens[lastTokenIndex]; - - allTokens[tokenIndex] = lastToken; - allTokens[lastTokenIndex] = 0; - - allTokens.length--; - allTokensIndex[_tokenId] = 0; - allTokensIndex[lastToken] = tokenIndex; - } - }