ERC721 full implementation #682
Conversation
facuspagnuolo
force-pushed
the
feature/640_detailed_erc721
branch
3 times, most recently
from
2a51700 to
a11062c
Compare
facuspagnuolo
force-pushed
the
feature/640_detailed_erc721
branch
4 times, most recently
from
965db5f to
c9cb227
Compare
|
@frangio can you please review? |
|
@facuspagnuolo might not be my place to comment here, but I've been playing around with your contract from this PR today for an application I'm working on. OpenZeppelin has such a huge adoption this might become something of a reference implementation. As such I just wanted to clarify something. Are the |
|
@TimTinkers thanks for reaching out! As far as I know, those functions are not agreed to part of the standard yet, but are under discussion. We are still reviewing this implementation and discussing about |
facuspagnuolo
force-pushed
the
feature/640_detailed_erc721
branch
2 times, most recently
from
860fd59 to
faf04ff
Compare
facuspagnuolo
changed the title
facuspagnuolo
force-pushed
the
feature/640_detailed_erc721
branch
from
03a0662 to
2e1a695
Compare
| return owner; | ||
| function tokenOfOwnerByIndex(address _owner, uint256 _index) public view returns (uint256) { | ||
| require(_index < balanceOf(_owner)); | ||
| return tokensOf(_owner)[_index]; |
There was a problem hiding this comment.
This is an order of magnitude slower as it returns the entire array and then indexes the array, and would not work on large arrays. A better way would to make ownedTokens internal and use:
return ownedTokens[_owner][_index];
There was a problem hiding this comment.
Already on it! spalladino@b81b4e1#diff-bc9f12d1d1e8ef6bdba9bc4d73ae8329R57
|
Closing in favour of #803 |
Fixes #640
This PR includes a full implementation of the ERC721 including all the required and some of the optional functionality. Moreover, it provides an approve all implementation using operatable terminology.