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.

Sign up for GitHub

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

Fix/erc721 #993

Merged
merged 2 commits into from
Jun 12, 2018
Merged

Fix/erc721 #993

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d36668c
fix: defer lookup-specific info to implementations
shrugs Jun 10, 2018
8ce165f
fix: change inheritance order, fix import
shrugs Jun 12, 2018
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
Next Next commit
fix: defer lookup-specific info to implementations
  • Loading branch information
@shrugs
shrugs committed Jun 12, 2018
commit d36668cc23d32bf85552be79ce1ed9b8a5300cb4
30 changes: 0 additions & 30 deletions contracts/token/ERC721/ERC721.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,6 @@ import "./ERC721Basic.sol";
* @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
*/
contract ERC721Enumerable is ERC721Basic {

bytes4 private constant InterfaceId_ERC721Enumerable = 0x780e9d63;
/**
* 0x780e9d63 ===
* bytes4(keccak256('totalSupply()')) ^
* bytes4(keccak256('tokenOfOwnerByIndex(address,uint256)')) ^
* bytes4(keccak256('tokenByIndex(uint256)'))
*/

constructor()
public
{
_registerInterface(InterfaceId_ERC721Enumerable);
}

function totalSupply() public view returns (uint256);
function tokenOfOwnerByIndex(
address _owner,
Expand All @@ -41,21 +26,6 @@ contract ERC721Enumerable is ERC721Basic {
* @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
*/
contract ERC721Metadata is ERC721Basic {

bytes4 private constant InterfaceId_ERC721Metadata = 0x5b5e139f;
/**
* 0x5b5e139f ===
* bytes4(keccak256('name()')) ^
* bytes4(keccak256('symbol()')) ^
* bytes4(keccak256('tokenURI(uint256)'))
*/

constructor()
public
{
_registerInterface(InterfaceId_ERC721Metadata);
}

function name() external view returns (string _name);
function symbol() external view returns (string _symbol);
function tokenURI(uint256 _tokenId) public view returns (string);
Expand Down
30 changes: 1 addition & 29 deletions contracts/token/ERC721/ERC721Basic.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "../../introspection/SupportsInterfaceWithLookup.sol";
* @title ERC721 Non-Fungible Token Standard basic interface
* @dev see https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
*/
contract ERC721Basic is SupportsInterfaceWithLookup {
contract ERC721Basic is ERC165 {
event Transfer(
address indexed _from,
address indexed _to,
Expand All @@ -24,34 +24,6 @@ contract ERC721Basic is SupportsInterfaceWithLookup {
bool _approved
);

bytes4 private constant InterfaceId_ERC721 = 0x80ac58cd;
/*
* 0x80ac58cd ===
* bytes4(keccak256('balanceOf(address)')) ^
* bytes4(keccak256('ownerOf(uint256)')) ^
* bytes4(keccak256('approve(address,uint256)')) ^
* bytes4(keccak256('getApproved(uint256)')) ^
* bytes4(keccak256('setApprovalForAll(address,bool)')) ^
* bytes4(keccak256('isApprovedForAll(address,address)')) ^
* bytes4(keccak256('transferFrom(address,address,uint256)')) ^
* bytes4(keccak256('safeTransferFrom(address,address,uint256)')) ^
* bytes4(keccak256('safeTransferFrom(address,address,uint256,bytes)'))
*/

bytes4 private constant InterfaceId_ERC721Exists = 0x4f558e79;
/*
* 0x4f558e79 ===
* bytes4(keccak256('exists(uint256)'))
*/

constructor ()
public
{
// register the supported interfaces to conform to ERC721 via ERC165
_registerInterface(InterfaceId_ERC721);
_registerInterface(InterfaceId_ERC721Exists);
}

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);
Expand Down
38 changes: 33 additions & 5 deletions contracts/token/ERC721/ERC721BasicToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,35 @@ import "./ERC721Basic.sol";
import "./ERC721Receiver.sol";
import "../../math/SafeMath.sol";
import "../../AddressUtils.sol";
import "../../introspection/SupportsInterfaceWithLookup.sol";


/**
* @title ERC721 Non-Fungible Token Standard basic implementation
* @dev see https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
*/
contract ERC721BasicToken is ERC721Basic {
contract ERC721BasicToken is ERC721Basic, SupportsInterfaceWithLookup {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later on there is

contract ERC721Token is SupportsInterfaceWithLookup, ERC721BasicToken, ERC721 {

SupportsInterfaceWithLookup being last in one inheritance list and first in another might bring problems with linearization later on. I would stick to always first or always last. I'm not sure about any of this though. 😣 Damn C3.


bytes4 private constant InterfaceId_ERC721 = 0x80ac58cd;
/*
* 0x80ac58cd ===
* bytes4(keccak256('balanceOf(address)')) ^
* bytes4(keccak256('ownerOf(uint256)')) ^
* bytes4(keccak256('approve(address,uint256)')) ^
* bytes4(keccak256('getApproved(uint256)')) ^
* bytes4(keccak256('setApprovalForAll(address,bool)')) ^
* bytes4(keccak256('isApprovedForAll(address,address)')) ^
* bytes4(keccak256('transferFrom(address,address,uint256)')) ^
* bytes4(keccak256('safeTransferFrom(address,address,uint256)')) ^
* bytes4(keccak256('safeTransferFrom(address,address,uint256,bytes)'))
*/

bytes4 private constant InterfaceId_ERC721Exists = 0x4f558e79;
/*
* 0x4f558e79 ===
* bytes4(keccak256('exists(uint256)'))
*/

using SafeMath for uint256;
using AddressUtils for address;

Expand Down Expand Up @@ -48,6 +70,14 @@ contract ERC721BasicToken is ERC721Basic {
_;
}

constructor()
public
{
// register the supported interfaces to conform to ERC721 via ERC165
_registerInterface(InterfaceId_ERC721);
_registerInterface(InterfaceId_ERC721Exists);
}

/**
* @dev Gets the balance of the specified address
* @param _owner address to query the balance of
Expand Down Expand Up @@ -92,10 +122,8 @@ contract ERC721BasicToken is ERC721Basic {
require(_to != owner);
require(msg.sender == owner || isApprovedForAll(owner, msg.sender));

if (getApproved(_tokenId) != address(0) || _to != address(0)) {
tokenApprovals[_tokenId] = _to;
emit Approval(owner, _to, _tokenId);
}
tokenApprovals[_tokenId] = _to;
emit Approval(owner, _to, _tokenId);
}

/**
Expand Down
24 changes: 23 additions & 1 deletion contracts/token/ERC721/ERC721Token.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ pragma solidity ^0.4.23;

import "./ERC721.sol";
import "./ERC721BasicToken.sol";
import "../../introspection/SupportsInterfaceWithLookup.sol";


/**
Expand All @@ -10,7 +11,24 @@ import "./ERC721BasicToken.sol";
* Moreover, it includes approve all functionality using operator terminology
* @dev see https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md
*/
contract ERC721Token is ERC721, ERC721BasicToken {
contract ERC721Token is SupportsInterfaceWithLookup, ERC721BasicToken, ERC721 {

bytes4 private constant InterfaceId_ERC721Enumerable = 0x780e9d63;
/**
* 0x780e9d63 ===
* bytes4(keccak256('totalSupply()')) ^
* bytes4(keccak256('tokenOfOwnerByIndex(address,uint256)')) ^
* bytes4(keccak256('tokenByIndex(uint256)'))
*/

bytes4 private constant InterfaceId_ERC721Metadata = 0x5b5e139f;
/**
* 0x5b5e139f ===
* bytes4(keccak256('name()')) ^
* bytes4(keccak256('symbol()')) ^
* bytes4(keccak256('tokenURI(uint256)'))
*/

// Token name
string internal name_;

Expand Down Expand Up @@ -38,6 +56,10 @@ contract ERC721Token is ERC721, ERC721BasicToken {
constructor(string _name, string _symbol) public {
name_ = _name;
symbol_ = _symbol;

// register the supported interfaces to conform to ERC721 via ERC165
_registerInterface(InterfaceId_ERC721Enumerable);
_registerInterface(InterfaceId_ERC721Metadata);
}

/**
Expand Down
19 changes: 15 additions & 4 deletions test/token/ERC721/ERC721BasicToken.behaviour.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,20 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) {
log.args._tokenId.toNumber().should.be.equal(tokenId);
log.args._data.should.be.equal(data);
});

describe('with an invalid token id', function () {
it('reverts', async function () {
await assertRevert(
transferFun.call(
this,
owner,
this.to,
unknownTokenId,
{ from: owner },
)
);
});
});
});
};

Expand Down Expand Up @@ -366,10 +380,7 @@ export default function shouldBehaveLikeERC721BasicToken (accounts) {
});

itClearsApproval();

it('does not emit an approval event', async function () {
logs.length.should.be.equal(0);
});
itEmitsApprovalEvent(ZERO_ADDRESS);
});

context('when there was a prior approval', function () {
Expand Down