Skip to content

Commit

Permalink
Optimized ERC721 transfers. (#1539)
Browse files Browse the repository at this point in the history 
* Added _transferToken.

* _transferFrom is now usable by derived contracts, abstracted away enumerable behavior.

* Removed unnecesary check from _clearApprovals
  • Loading branch information
@nventuro
nventuro authored Dec 11, 2018
1 parent 70fd243 commit 70e616d
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 29 deletions.
35 changes: 24 additions & 11 deletions contracts/token/ERC721/ERC721.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,8 @@ contract ERC721 is ERC165, IERC721 {
*/
function transferFrom(address from, address to, uint256 tokenId) public {
require(_isApprovedOrOwner(msg.sender, tokenId));
require(to != address(0));

_clearApproval(from, tokenId);
_removeTokenFrom(from, tokenId);
_addTokenTo(to, tokenId);

emit Transfer(from, to, tokenId);
_transferFrom(from, to, tokenId);
}

/**
Expand Down Expand Up @@ -217,7 +212,7 @@ contract ERC721 is ERC165, IERC721 {
* @param tokenId uint256 ID of the token being burned by the msg.sender
*/
function _burn(address owner, uint256 tokenId) internal {
_clearApproval(owner, tokenId);
_clearApproval(tokenId);
_removeTokenFrom(owner, tokenId);
emit Transfer(owner, address(0), tokenId);
}
Expand Down Expand Up @@ -249,6 +244,27 @@ contract ERC721 is ERC165, IERC721 {
_tokenOwner[tokenId] = address(0);
}

/**
* @dev Internal function to transfer ownership of a given token ID to another address.
* As opposed to transferFrom, this imposes no restrictions on msg.sender.
* @param from current owner of the token
* @param to address to receive the ownership of the given token ID
* @param tokenId uint256 ID of the token to be transferred
*/
function _transferFrom(address from, address to, uint256 tokenId) internal {
require(ownerOf(tokenId) == from);
require(to != address(0));

_clearApproval(tokenId);

_ownedTokensCount[from] = _ownedTokensCount[from].sub(1);
_ownedTokensCount[to] = _ownedTokensCount[to].add(1);

_tokenOwner[tokenId] = to;

emit Transfer(from, to, tokenId);
}

/**
* @dev Internal function to invoke `onERC721Received` on a target address
* The call is not executed if the target address is not a contract
Expand All @@ -269,12 +285,9 @@ contract ERC721 is ERC165, IERC721 {

/**
* @dev Private 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) private {
require(ownerOf(tokenId) == owner);
function _clearApproval(uint256 tokenId) private {
if (_tokenApprovals[tokenId] != address(0)) {
_tokenApprovals[tokenId] = address(0);
}
Expand Down
81 changes: 63 additions & 18 deletions contracts/token/ERC721/ERC721Enumerable.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,8 @@ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable {
*/
function _addTokenTo(address to, uint256 tokenId) internal {
super._addTokenTo(to, tokenId);
uint256 length = _ownedTokens[to].length;
_ownedTokens[to].push(tokenId);
_ownedTokensIndex[tokenId] = length;

_addTokenToOwnerEnumeration(to, tokenId);
}

/**
Expand All @@ -92,22 +91,26 @@ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable {
function _removeTokenFrom(address from, uint256 tokenId) internal {
super._removeTokenFrom(from, tokenId);

// To prevent a gap in the array, we store the last token in the index of the token to delete, and
// then delete the last slot.
uint256 tokenIndex = _ownedTokensIndex[tokenId];
uint256 lastTokenIndex = _ownedTokens[from].length.sub(1);
uint256 lastToken = _ownedTokens[from][lastTokenIndex];

_ownedTokens[from][tokenIndex] = lastToken;
// This also deletes the contents at the last position of the array
_ownedTokens[from].length--;

// 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
_removeTokenFromOwnerEnumeration(from, tokenId);

// Since the token is being destroyed, we also clear its index
// TODO(nventuro): 0 is still a valid index, so arguably this isnt really helpful, remove?
_ownedTokensIndex[tokenId] = 0;
_ownedTokensIndex[lastToken] = tokenIndex;
}

/**
* @dev Internal function to transfer ownership of a given token ID to another address.
* As opposed to transferFrom, this imposes no restrictions on msg.sender.
* @param from current owner of the token
* @param to address to receive the ownership of the given token ID
* @param tokenId uint256 ID of the token to be transferred
*/
function _transferFrom(address from, address to, uint256 tokenId) internal {
super._transferFrom(from, to, tokenId);

_removeTokenFromOwnerEnumeration(from, tokenId);

_addTokenToOwnerEnumeration(to, tokenId);
}

/**
Expand Down Expand Up @@ -144,7 +147,7 @@ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable {
_allTokensIndex[tokenId] = 0;
_allTokensIndex[lastToken] = tokenIndex;
}

/**
* @dev Gets the list of token IDs of the requested owner
* @param owner address owning the tokens
Expand All @@ -153,4 +156,46 @@ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable {
function _tokensOfOwner(address owner) internal view returns (uint256[] storage) {
return _ownedTokens[owner];
}

/**
* @dev Private function to add a token to this extension's ownership-tracking data structures.
* @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 _addTokenToOwnerEnumeration(address to, uint256 tokenId) private {
uint256 newOwnedTokensLength = _ownedTokens[to].push(tokenId);
// No need to use SafeMath since the length after a push cannot be zero
_ownedTokensIndex[tokenId] = newOwnedTokensLength - 1;
}

/**
* @dev Private function to remove a token from this extension's ownership-tracking data structures. Note that
* while the token is not assigned a new owner, the _ownedTokensIndex mapping is _not_ updated: this allows for
* gas optimizations e.g. when performing a transfer operation (avoiding double writes).
* This has O(1) time complexity, but alters the order of the _ownedTokens array.
* @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 _removeTokenFromOwnerEnumeration(address from, uint256 tokenId) private {
// To prevent a gap in from's tokens array, we store the last token in the index of the token to delete, and
// then delete the last slot (swap and pop).

uint256 lastTokenIndex = _ownedTokens[from].length.sub(1);
uint256 lastTokenId = _ownedTokens[from][lastTokenIndex];

uint256 tokenIndex = _ownedTokensIndex[tokenId];

_ownedTokens[from][tokenIndex] = lastTokenId; // Move the last token to the slot of the to-delete token
_ownedTokensIndex[lastTokenId] = tokenIndex; // Update the moved token's index

// Note that this will handle single-element arrays. In that case, both tokenIndex and lastTokenIndex are going
// to be zero. The swap operation will therefore have no effect, but the token _will_ be deleted during the
// 'pop' operation.

// This also deletes the contents at the last position of the array
_ownedTokens[from].length--;

// Note that _ownedTokensIndex[tokenId] hasn't been cleared: it still points to the old slot (now occcupied by
// lasTokenId).
}
}

0 comments on commit 70e616d

Please sign in to comment.