Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimized ERC721 transfers. #1539

Merged
merged 4 commits into from
Dec 11, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions contracts/token/ERC721/ERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ contract ERC721 is ERC165, IERC721 {
require(to != address(0));

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

emit Transfer(from, to, tokenId);
}
Expand Down Expand Up @@ -249,6 +248,24 @@ contract ERC721 is ERC165, IERC721 {
_tokenOwner[tokenId] = address(0);
}

/**
* @dev Internal function to transfer a token ID from the list of a given address to another one.
* Note that this function is left internal to make ERC721Enumerable possible, but is not
* intended to be called by custom derived contracts: in particular, it emits no Transfer event,
nventuro marked this conversation as resolved.
Show resolved Hide resolved
* and doesn't clear approvals.
* @param from address representing the previous owner of the given token ID
* @param to address representing the new owner of the given token ID
* @param tokenId uint256 ID of the token to be transferred to the tokens list of the given address
*/
function _transferToken(address from, address to, uint256 tokenId) internal {
require(ownerOf(tokenId) == from);

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

_tokenOwner[tokenId] = to;
}

/**
* @dev Internal function to invoke `onERC721Received` on a target address
* The call is not executed if the target address is not a contract
Expand Down
29 changes: 29 additions & 0 deletions contracts/token/ERC721/ERC721Enumerable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,35 @@ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable {
_ownedTokensIndex[lastToken] = tokenIndex;
}

/**
* @dev Internal function to transfer a token ID from the list of a given address to another one.
* This function is internal due to language limitations, see the note in ERC721.sol.
* It is not intended to be called by custom derived contracts: in particular, it emits no Transfer event,
* and doesn't clear approvals.
* @param from address representing the previous owner of the given token ID
* @param to address representing the new owner of the given token ID
* @param tokenId uint256 ID of the token to be transferred from the tokens list of the given address
*/
function _transferToken(address from, address to, uint256 tokenId) internal {
super._transferToken(from, to, tokenId);

// To prevent a gap in from's array, we store from's 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 preTokenIndex = _ownedTokensIndex[tokenId];
_ownedTokens[from][preTokenIndex] = lastTokenId;
_ownedTokensIndex[lastTokenId] = preTokenIndex;

// This also deletes the contents at the last position of the array
frangio marked this conversation as resolved.
Show resolved Hide resolved
_ownedTokens[from].length--;

// Now we add the token to the recipient
_ownedTokens[to].push(tokenId);
_ownedTokensIndex[tokenId] = _ownedTokens[to].length.sub(1);
nventuro marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @dev Internal function to mint a new token
* Reverts if the given token ID already exists
Expand Down