-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
ERC721 full implementation #803
Changes from 11 commits
1192e68
ca163a8
71cbc51
3745025
559df81
d726c79
54a1d2e
851685c
3cef880
6f180a6
6fbe771
626742e
95a1f9a
15f9556
b332995
f4748da
fb4f728
6b98e4e
3f2ea8a
74db03b
981c6f7
73b77ae
eee5b0e
9deb637
fe6e4ff
4836279
619ae84
2e593f2
6c09d20
37929c8
3676b55
7815cc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
pragma solidity ^0.4.18; | ||
|
||
/** | ||
* Utility library of inline functions on addresses | ||
*/ | ||
library AddressUtils { | ||
|
||
/** | ||
* Returns whether there is code in the target address | ||
* @param addr address address to check | ||
* @return whether there is code in the target address | ||
*/ | ||
function isContract(address addr) internal view returns (bool) { | ||
uint size; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
assembly { size := extcodesize(addr) } | ||
return size > 0; | ||
} | ||
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: newline |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
pragma solidity ^0.4.18; | ||
|
||
import "../token/ERC721/ERC721BasicToken.sol"; | ||
|
||
/** | ||
* @title ERC721BasicTokenMock | ||
* This mock just provides a public mint and burn functions for testing purposes | ||
*/ | ||
contract ERC721BasicTokenMock is ERC721BasicToken { | ||
function mint(address _to, uint256 _tokenId) public { | ||
super.doMint(_to, _tokenId); | ||
} | ||
|
||
function burn(uint256 _tokenId) public { | ||
super.doBurn(ownerOf(_tokenId), _tokenId); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
pragma solidity ^0.4.18; | ||
|
||
import "../token/ERC721/ERC721Receiver.sol"; | ||
|
||
contract ERC721ReceiverMock is ERC721Receiver { | ||
bytes4 retval; | ||
bool reverts; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess the linter will cry if you don't make visibility explicit here |
||
|
||
event Received(address _address, uint256 _tokenId, bytes _data, uint256 _gas); | ||
|
||
function ERC721ReceiverMock(bytes4 _retval, bool _reverts) public { | ||
retval = _retval; | ||
reverts = _reverts; | ||
} | ||
|
||
function onERC721Received(address _address, uint256 _tokenId, bytes _data) public returns(bytes4) { | ||
require(!reverts); | ||
Received(_address, _tokenId, _data, msg.gas); | ||
return retval; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,16 +4,24 @@ import "../token/ERC721/ERC721Token.sol"; | |
|
||
/** | ||
* @title ERC721TokenMock | ||
* This mock just provides a public mint and burn functions for testing purposes. | ||
* This mock just provides a public mint and burn functions for testing purposes, | ||
* and a public setter for metadata URI | ||
*/ | ||
contract ERC721TokenMock is ERC721Token { | ||
function ERC721TokenMock() ERC721Token() public { } | ||
function ERC721TokenMock(string name, string symbol) | ||
ERC721Token(name, symbol) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if we have a convention for this, but I think modifiers should be indented. |
||
public | ||
{ } | ||
|
||
function mint(address _to, uint256 _tokenId) public { | ||
super._mint(_to, _tokenId); | ||
doMint(_to, _tokenId); | ||
} | ||
|
||
function burn(uint256 _tokenId) public { | ||
super._burn(_tokenId); | ||
doBurn(ownerOf(_tokenId), _tokenId); | ||
} | ||
|
||
function setTokenURI(uint256 _tokenId, string _uri) public { | ||
doSetTokenURI(_tokenId, _uri); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
pragma solidity ^0.4.18; | ||
|
||
import "./ERC721.sol"; | ||
|
||
/** | ||
* @title ERC-721 methods shipped in OpenZeppelin v1.7.0, removed in the latest version of the standard | ||
* @dev Only use this interface for compatibility with previously deployed contracts | ||
* @dev Use ERC721 for interacting with new contracts which are standard-compliant | ||
*/ | ||
contract DeprecatedERC721 is ERC721 { | ||
function takeOwnership(uint256 _tokenId) public; | ||
function transfer(address _to, uint256 _tokenId) public; | ||
function tokensOf(address _owner) public view returns (uint256[]); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,24 @@ | ||
pragma solidity ^0.4.18; | ||
|
||
/** | ||
* @title ERC721 interface | ||
* @dev see https://github.com/ethereum/eips/issues/721 | ||
*/ | ||
contract ERC721 { | ||
event Transfer(address indexed _from, address indexed _to, uint256 _tokenId); | ||
event Approval(address indexed _owner, address indexed _approved, uint256 _tokenId); | ||
import "./ERC721Basic.sol"; | ||
|
||
function balanceOf(address _owner) public view returns (uint256 _balance); | ||
function ownerOf(uint256 _tokenId) public view returns (address _owner); | ||
function transfer(address _to, uint256 _tokenId) public; | ||
function approve(address _to, uint256 _tokenId) public; | ||
function takeOwnership(uint256 _tokenId) public; | ||
/// @title ERC-721 Non-Fungible Token Standard, optional enumeration extension | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replace |
||
/// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md | ||
contract ERC721Enumerable is ERC721Basic { | ||
function totalSupply() public view returns (uint256); | ||
function tokenOfOwnerByIndex(address _owner, uint256 _index) public view returns (uint256 _tokenId); | ||
function tokenByIndex(uint256 _index) public view returns (uint256); | ||
} | ||
|
||
/// @title ERC-721 Non-Fungible Token Standard, optional metadata extension | ||
/// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md | ||
contract ERC721Metadata is ERC721Basic { | ||
function name() public view returns (string _name); | ||
function symbol() public view returns (string _symbol); | ||
function tokenURI(uint256 _tokenId) public view returns (string); | ||
} | ||
|
||
/// @title ERC-721 Non-Fungible Token Standard, full implementation interface | ||
/// @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md | ||
contract ERC721 is ERC721Basic, ERC721Enumerable, ERC721Metadata { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
pragma solidity ^0.4.18; | ||
|
||
/** | ||
* @title ERC721 Non-Fungible Token Standard basic interface | ||
* @dev see https://github.com/ethereum/eips/issues/721 | ||
*/ | ||
contract ERC721Basic { | ||
event Transfer(address indexed _from, address indexed _to, uint256 _tokenId); | ||
event Approval(address indexed _owner, address indexed _approved, uint256 _tokenId); | ||
event ApprovalForAll(address indexed _owner, address indexed _operator, bool _approved); | ||
|
||
function balanceOf(address _owner) public view returns (uint256 _balance); | ||
function ownerOf(uint256 _tokenId) public view returns (address _owner); | ||
function exists(uint256 _tokenId) public view returns (bool _exists); | ||
|
||
function approve(address _to, uint256 _tokenId) public; | ||
function getApproved(uint256 _tokenId) public view returns (address _operator); | ||
|
||
function setApprovalForAll(address _operator, bool _approved) public; | ||
function isApprovedForAll(address _owner, address _operator) public view returns (bool); | ||
|
||
function transferFrom(address _from, address _to, uint256 _tokenId) public; | ||
function safeTransferFrom(address _from, address _to, uint256 _tokenId) public; | ||
function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes _data) public; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A contract address doesn't have code until its constructor finishes executing; during execution of the constructor,
isContract
will not correctly detect that it is (or will be) a contract.How this affects ERC721 is not clear. Since the contract still doesn't have code, it would also be unable to execute
onERC721Received
, so it makes no difference whetherisContract
returns true or false. OTOH, this situation is likely a programmer error, and it will fail silently. Sadly I don't think there's a solution other than changing the interface to make it explicit that anonERC721Received
call is expected.Given the odd semantics, I'm not sure we should provide
isContract
as a standalone helper. I'm inclined to say no.If we do, we should place a disclaimer stating that it will return false if the contract constructor hasn't yet finished running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a disclaimer!