Improved some ERC721 internal shenanigans #1450
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… and _removeTokenFrom.
come-maiz
reviewed
Oct 18, 2018
contracts/token/ERC721/ERC721.sol
Outdated
| /** | ||
| * @dev Internal function to add a token ID to the list of a given address | ||
| * Note that this function is left internal to make ERC721Enumerable possible, but is not | ||
| * intended to be calld by custom derived contracts: in particular, it emits no Transfer event. |
contracts/token/ERC721/ERC721.sol
Outdated
| @@ -288,6 +277,8 @@ contract ERC721 is ERC165, IERC721 { | |||
|
|
|||
| /** | |||
| * @dev Internal function to remove a token ID from the list of a given address | |||
| * Note that this function is left internal to make ERC721Enumerable possible, but is not | |||
| * intended to be calld by custom derived contracts: in particular, it emits no Transfer event. | |||
| @@ -72,6 +72,8 @@ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable { | |||
|
|
|||
| /** | |||
| * @dev Internal function to add a token ID to the list of a given address | |||
| * This function is internal due to language limitations, see the note in ERC721.sol. | |||
| * It is not intended to be calld by custom derived contracts: in particular, it emits no Transfer event. | |||
| @@ -84,6 +86,8 @@ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable { | |||
|
|
|||
| /** | |||
| * @dev Internal function to remove a token ID from the list of a given address | |||
| * This function is internal due to language limitations, see the note in ERC721.sol. | |||
| * It is not intended to be calld by custom derived contracts: in particular, it emits no Transfer event. | |||
come-maiz
pushed a commit
that referenced
this pull request
Oct 21, 2018
* Made _clearApproval private, added clarifying comments in _addTokenTo and _removeTokenFrom. * Added approval information. (cherry picked from commit 8204f6a)
vittominacori
added a commit
to vittominacori/zeppelin-solidity
that referenced
this pull request
Nov 21, 2018
…lidity into feature/token-recover * 'master' of https://github.com/vittominacori/zeppelin-solidity: (98 commits) Renamed roles private variables to adhere to code style. (OpenZeppelin#1507) Remove extraneous quantity check, fixes OpenZeppelin#1454 (OpenZeppelin#1455) Remove redundant require statements (OpenZeppelin#1409) Add the step to delete the build dir to the RELEASE notes (OpenZeppelin#1467) add an address typecast to this per issue OpenZeppelin#1457 (OpenZeppelin#1471) add improvement in simpletoken example OpenZeppelin#1458 (OpenZeppelin#1473) SafeMath Test Coverage Improved (OpenZeppelin#1477) The beneficiary parameter of claimRefund is replaced with refundee (OpenZeppelin#1481) fix ERC20.sol#L174 and ERC20.sol#L187 should be casted to an address type. (OpenZeppelin#1470) Fix/add comment erc721 burnable OpenZeppelin#1464 (OpenZeppelin#1469) Release v2.0.0 Release candidate v2.0.0-rc.4 Improved some ERC721 internal shenanigans (OpenZeppelin#1450) Add warning about trading tokens before refundable crowdsale goal is met (OpenZeppelin#1452) Crowdsale.buyTokens is now nonReentrant. (OpenZeppelin#1438) InitialRate must be strictly larger than finalRate. (OpenZeppelin#1441) Fixed how allowance crowdsale checks remaining tokens. (OpenZeppelin#1449) Deleted unnecessary import. (OpenZeppelin#1437) Made SampleCrowdsale a bit clearer. (OpenZeppelin#1448) Now setting the finalized flag before doing finalization to prevent possbile reentrancy issues. (OpenZeppelin#1447) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
_addTokenTo,_removeTokenFromand_clearApprovalare allinternal, but shouldn't be directly called from derived contracts, since they are incomplete (they don't emit events, because they lack the required context to know which ones to emit). I made_clearApprovalprivate, since it shouldn't generally be required, and left the other two as-is with a comment, sinceinternalis required forEC721Enumerable.