From f8fa7f63fbf91e7e324bcf0a6a7f7d7b6446891f Mon Sep 17 00:00:00 2001 From: Facundo Spagnuolo Date: Mon, 26 Feb 2018 17:50:27 -0300 Subject: [PATCH 1/3] Rename current ERC721 implementation to BaseERC721 --- ...ERC721TokenMock.sol => BaseERC721TokenMock.sol} | 8 ++++---- .../token/ERC721/{ERC721.sol => BaseERC721.sol} | 6 +++--- .../{ERC721Token.sol => BaseERC721Token.sol} | 14 +++++++------- ...ERC721Token.test.js => BaseERC721Token.test.js} | 6 +++--- 4 files changed, 17 insertions(+), 17 deletions(-) rename contracts/mocks/{ERC721TokenMock.sol => BaseERC721TokenMock.sol} (60%) rename contracts/token/ERC721/{ERC721.sol => BaseERC721.sol} (77%) rename contracts/token/ERC721/{ERC721Token.sol => BaseERC721Token.sol} (95%) rename test/token/ERC721/{ERC721Token.test.js => BaseERC721Token.test.js} (99%) diff --git a/contracts/mocks/ERC721TokenMock.sol b/contracts/mocks/BaseERC721TokenMock.sol similarity index 60% rename from contracts/mocks/ERC721TokenMock.sol rename to contracts/mocks/BaseERC721TokenMock.sol index 5cb6cb67961..9e15b6896df 100644 --- a/contracts/mocks/ERC721TokenMock.sol +++ b/contracts/mocks/BaseERC721TokenMock.sol @@ -1,13 +1,13 @@ pragma solidity ^0.4.18; -import "../token/ERC721/ERC721Token.sol"; +import "../token/ERC721/BaseERC721Token.sol"; /** - * @title ERC721TokenMock + * @title BaseERC721TokenMock * This mock just provides a public mint and burn functions for testing purposes. */ -contract ERC721TokenMock is ERC721Token { - function ERC721TokenMock() ERC721Token() public { } +contract BaseERC721TokenMock is BaseERC721Token { + function BaseERC721TokenMock() BaseERC721Token() public { } function mint(address _to, uint256 _tokenId) public { super._mint(_to, _tokenId); diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/BaseERC721.sol similarity index 77% rename from contracts/token/ERC721/ERC721.sol rename to contracts/token/ERC721/BaseERC721.sol index f88376f6110..7918fdd9d2f 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/BaseERC721.sol @@ -1,10 +1,10 @@ pragma solidity ^0.4.18; /** - * @title ERC721 interface - * @dev see https://github.com/ethereum/eips/issues/721 + * @title Base ERC721 interface + * @dev see https://github.com/ethereum/eips/issues/721 and https://github.com/ethereum/EIPs/pull/841 */ -contract ERC721 { +contract BaseERC721 { event Transfer(address indexed _from, address indexed _to, uint256 _tokenId); event Approval(address indexed _owner, address indexed _approved, uint256 _tokenId); diff --git a/contracts/token/ERC721/ERC721Token.sol b/contracts/token/ERC721/BaseERC721Token.sol similarity index 95% rename from contracts/token/ERC721/ERC721Token.sol rename to contracts/token/ERC721/BaseERC721Token.sol index 4f8f268c3ba..18c34ef8558 100644 --- a/contracts/token/ERC721/ERC721Token.sol +++ b/contracts/token/ERC721/BaseERC721Token.sol @@ -1,13 +1,13 @@ pragma solidity ^0.4.18; -import "./ERC721.sol"; +import "./BaseERC721.sol"; import "../../math/SafeMath.sol"; /** - * @title ERC721Token + * @title BaseERC721Token * Generic implementation for the required functionality of the ERC721 standard */ -contract ERC721Token is ERC721 { +contract BaseERC721Token is BaseERC721 { using SafeMath for uint256; // Total amount of tokens @@ -136,14 +136,14 @@ contract ERC721Token is ERC721 { } /** - * @dev Tells whether the msg.sender is approved for the given token ID or not + * @dev Tells whether the given spender is approved for the given token ID or not * This function is not private so it can be extended in further implementations like the operatable ERC721 - * @param _owner address of the owner to query the approval of + * @param _spender address of the spender to query the approval of * @param _tokenId uint256 ID of the token to query the approval of * @return bool whether the msg.sender is approved for the given token ID or not */ - function isApprovedFor(address _owner, uint256 _tokenId) internal view returns (bool) { - return approvedFor(_tokenId) == _owner; + function isApprovedFor(address _spender, uint256 _tokenId) internal view returns (bool) { + return approvedFor(_tokenId) == _spender; } /** diff --git a/test/token/ERC721/ERC721Token.test.js b/test/token/ERC721/BaseERC721Token.test.js similarity index 99% rename from test/token/ERC721/ERC721Token.test.js rename to test/token/ERC721/BaseERC721Token.test.js index cb6546f4246..f3ed4cc6af1 100644 --- a/test/token/ERC721/ERC721Token.test.js +++ b/test/token/ERC721/BaseERC721Token.test.js @@ -1,13 +1,13 @@ import assertRevert from '../../helpers/assertRevert'; const BigNumber = web3.BigNumber; -const ERC721Token = artifacts.require('ERC721TokenMock.sol'); +const BaseERC721Token = artifacts.require('BaseERC721TokenMock.sol'); require('chai') .use(require('chai-as-promised')) .use(require('chai-bignumber')(BigNumber)) .should(); -contract('ERC721Token', accounts => { +contract('BaseERC721Token', accounts => { let token = null; const _firstTokenId = 1; const _secondTokenId = 2; @@ -16,7 +16,7 @@ contract('ERC721Token', accounts => { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; beforeEach(async function () { - token = await ERC721Token.new({ from: _creator }); + token = await BaseERC721Token.new({ from: _creator }); await token.mint(_creator, _firstTokenId, { from: _creator }); await token.mint(_creator, _secondTokenId, { from: _creator }); }); From 2e1a695c71baf7e9a0d004a575c6f33ce2f55a9f Mon Sep 17 00:00:00 2001 From: Facundo Spagnuolo Date: Mon, 26 Feb 2018 19:04:50 -0300 Subject: [PATCH 2/3] Implement ERC721 optional & approveAll functionality --- contracts/mocks/ERC721TokenMock.sol | 16 + contracts/token/ERC721/ERC721.sol | 19 ++ contracts/token/ERC721/ERC721Token.sol | 99 ++++++ test/token/ERC721/ERC721Token.test.js | 442 +++++++++++++++++++++++++ 4 files changed, 576 insertions(+) create mode 100644 contracts/mocks/ERC721TokenMock.sol create mode 100644 contracts/token/ERC721/ERC721.sol create mode 100644 contracts/token/ERC721/ERC721Token.sol create mode 100644 test/token/ERC721/ERC721Token.test.js diff --git a/contracts/mocks/ERC721TokenMock.sol b/contracts/mocks/ERC721TokenMock.sol new file mode 100644 index 00000000000..28b0ba70b76 --- /dev/null +++ b/contracts/mocks/ERC721TokenMock.sol @@ -0,0 +1,16 @@ +pragma solidity ^0.4.18; + +import "./BaseERC721TokenMock.sol"; +import "../token/ERC721/ERC721Token.sol"; + +/** + * @title ERC721TokenMock + * This mock just provides a public mint and burn functions for testing purposes. + */ +contract ERC721TokenMock is ERC721Token, BaseERC721TokenMock { + function ERC721TokenMock(string name, string symbol) + BaseERC721TokenMock() + ERC721Token(name, symbol) + public + { } +} diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol new file mode 100644 index 00000000000..87bb4bc4bf9 --- /dev/null +++ b/contracts/token/ERC721/ERC721.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.4.18; + +import "./BaseERC721.sol"; + + +/** + * @title Full ERC721 interface + * @dev see https://github.com/ethereum/eips/issues/721 and https://github.com/ethereum/EIPs/pull/841 + */ +contract ERC721 is BaseERC721 { + event OperatorApproval(address indexed _owner, address indexed _operator, bool _approved); + + function name() public view returns (string _name); + function symbol() public view returns (string _symbol); + function takeOwnershipFor(address _to, uint256 _tokenId) public; + function setOperatorApproval(address _to, bool _approved) public; + function isOperatorApprovedFor(address _owner, address _operator) public view returns (bool); + function tokenOfOwnerByIndex(address _owner, uint256 _index) public view returns (uint256 _tokenId); +} diff --git a/contracts/token/ERC721/ERC721Token.sol b/contracts/token/ERC721/ERC721Token.sol new file mode 100644 index 00000000000..202cb48035d --- /dev/null +++ b/contracts/token/ERC721/ERC721Token.sol @@ -0,0 +1,99 @@ +pragma solidity ^0.4.18; + +import "./ERC721.sol"; +import "./BaseERC721Token.sol"; + + +/** + * @title Full ERC721 Token + * This implementation includes all the required and some optional functionality of the ERC721 standard + * Moreover, it includes approve all functionality using operatable terminology + * @dev see https://github.com/ethereum/eips/issues/721 and https://github.com/ethereum/EIPs/pull/841 + */ +contract ERC721Token is ERC721, BaseERC721Token { + // Token name + string private _name; + + // Token symbol + string private _symbol; + + // Mapping from owner to operator approvals + mapping (address => mapping (address => bool)) private operatorApprovals; + + /** + * @dev Constructor function + */ + function ERC721Token(string name, string symbol) public { + _name = name; + _symbol = symbol; + } + + /** + * @dev Gets the token name + * @return string representing the token name + */ + function name() public view returns (string) { + return _name; + } + + /** + * @dev Gets the token symbol + * @return string representing the token symbol + */ + function symbol() public view returns (string) { + return _symbol; + } + + /** + * @dev Gets the token ID at a given index of the tokens list of the requested owner + * @param _owner address owning the tokens list to be accessed + * @param _index uint256 representing the index to be accessed of the requested tokens list + * @return uint256 token ID at the given index of the tokens list owned by the requested address + */ + function tokenOfOwnerByIndex(address _owner, uint256 _index) public view returns (uint256) { + require(_index < balanceOf(_owner)); + return tokensOf(_owner)[_index]; + } + + /** + * @dev Claims the ownership of a given token ID for a given recipient + * @param _to address which you want to transfer the token to + * @param _tokenId uint256 ID of the token being claimed by the msg.sender + */ + function takeOwnershipFor(address _to, uint256 _tokenId) public { + require(isApprovedFor(msg.sender, _tokenId)); + clearApprovalAndTransfer(ownerOf(_tokenId), _to, _tokenId); + } + + /** + * @dev Sets the approval of a given operator + * @param _to operator address to set the approval + * @param _approved representing the status of the approval to be set + */ + function setOperatorApproval(address _to, bool _approved) public { + require(_to != msg.sender); + operatorApprovals[msg.sender][_to] = _approved; + OperatorApproval(msg.sender, _to, _approved); + } + + /** + * @dev Tells whether an operator is approved by a given owner + * @param _owner owner address which you want to query the approval of + * @param _operator operator address which you want to query the approval of + * @return bool whether the given operator is approved by the given owner + */ + function isOperatorApprovedFor(address _owner, address _operator) public view returns (bool) { + return operatorApprovals[_owner][_operator]; + } + + /** + * @dev Tells whether the given spender is approved for the given token ID or + * if the given owner is an operator approved by the owner of the token + * @param _spender address of the spender to query the approval of + * @param _tokenId uint256 ID of the token to query the approval of + * @return bool whether the msg.sender is approved for the given token ID or not + */ + function isApprovedFor(address _spender, uint256 _tokenId) internal view returns (bool) { + return super.isApprovedFor(_spender, _tokenId) || isOperatorApprovedFor(ownerOf(_tokenId), _spender); + } +} diff --git a/test/token/ERC721/ERC721Token.test.js b/test/token/ERC721/ERC721Token.test.js new file mode 100644 index 00000000000..fe31ca2df20 --- /dev/null +++ b/test/token/ERC721/ERC721Token.test.js @@ -0,0 +1,442 @@ +import assertRevert from '../../helpers/assertRevert'; +const BigNumber = web3.BigNumber; +const ERC721Token = artifacts.require('ERC721TokenMock.sol'); + +require('chai') + .use(require('chai-as-promised')) + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('ERC721Token', accounts => { + let token = null; + const _name = 'Non Fungible Token'; + const _symbol = 'NFT'; + const _firstTokenId = 1; + const _secondTokenId = 2; + const _unknownTokenId = 3; + const _creator = accounts[0]; + const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; + + beforeEach(async function () { + token = await ERC721Token.new(_name, _symbol, { from: _creator }); + await token.mint(_creator, _firstTokenId, { from: _creator }); + await token.mint(_creator, _secondTokenId, { from: _creator }); + }); + + describe('name', function () { + it('has a name', async function () { + const name = await token.name(); + name.should.be.equal(_name); + }); + }); + + describe('symbol', function () { + it('has a symbol', async function () { + const symbol = await token.symbol(); + symbol.should.be.equal(_symbol); + }); + }); + + describe('tokenOfOwnerByIndex', function () { + describe('when the given address owns some tokens', function () { + const owner = _creator; + + describe('when the given index is lower than the amount of tokens owned by the given address', function () { + const index = 0; + + it('returns the token ID placed at the given index', async function () { + const tokenId = await token.tokenOfOwnerByIndex(owner, index); + tokenId.should.be.bignumber.equal(_firstTokenId); + }); + }); + + describe('when the index is greater than or equal to the total tokens owned by the given address', function () { + const index = 2; + + it('reverts', async function () { + await assertRevert(token.tokenOfOwnerByIndex(owner, index)); + }); + }); + }); + + describe('when the given address does not own any token', function () { + const owner = accounts[1]; + + it('reverts', async function () { + await assertRevert(token.tokenOfOwnerByIndex(owner, 0)); + }); + }); + }); + + describe('setOperatorApproval', function () { + const sender = _creator; + + describe('when the operator willing to approve is not the owner', function () { + const operator = accounts[1]; + + describe('when there is no operator approval set by the sender', function () { + it('approves the operator', async function () { + await token.setOperatorApproval(operator, true, { from: sender }); + + const isApproved = await token.isOperatorApprovedFor(sender, operator); + isApproved.should.be.true; + }); + + it('emits an approval event', async function () { + const { logs } = await token.setOperatorApproval(operator, true, { from: sender }); + + logs.length.should.be.equal(1); + logs[0].event.should.be.eq('OperatorApproval'); + logs[0].args._owner.should.be.equal(sender); + logs[0].args._operator.should.be.equal(operator); + logs[0].args._approved.should.be.true; + }); + }); + + describe('when the operator was set as not approved', function () { + beforeEach(async function () { + await token.setOperatorApproval(operator, false, { from: sender }); + }); + + it('approves the operator', async function () { + await token.setOperatorApproval(operator, true, { from: sender }); + + const isApproved = await token.isOperatorApprovedFor(sender, operator); + isApproved.should.be.true; + }); + + it('emits an approval event', async function () { + const { logs } = await token.setOperatorApproval(operator, true, { from: sender }); + + logs.length.should.be.equal(1); + logs[0].event.should.be.eq('OperatorApproval'); + logs[0].args._owner.should.be.equal(sender); + logs[0].args._operator.should.be.equal(operator); + logs[0].args._approved.should.be.true; + }); + + it('can unset the operator approval', async function () { + await token.setOperatorApproval(operator, false, { from: sender }); + + const isApproved = await token.isOperatorApprovedFor(sender, operator); + isApproved.should.be.false; + }); + }); + + describe('when the operator was already approved', function () { + beforeEach(async function () { + await token.setOperatorApproval(operator, true, { from: sender }); + }); + + it('keeps the approval to the given address and does not emit an approval event', async function () { + await token.setOperatorApproval(operator, true, { from: sender }); + + const isApproved = await token.isOperatorApprovedFor(sender, operator); + isApproved.should.be.true; + }); + + it('emits an approval event', async function () { + const { logs } = await token.setOperatorApproval(operator, true, { from: sender }); + + logs.length.should.be.equal(1); + logs[0].event.should.be.eq('OperatorApproval'); + logs[0].args._owner.should.be.equal(sender); + logs[0].args._operator.should.be.equal(operator); + logs[0].args._approved.should.be.true; + }); + }); + }); + + describe('when the operator is the owner', function () { + const operator = _creator; + + it('reverts', async function () { + await assertRevert(token.setOperatorApproval(operator, true, { from: sender })); + }); + }); + }); + + describe('takeOwnership', function () { + const tokenId = _firstTokenId; + + describe('when the sender was approved by the owner of the token ID', function () { + const sender = accounts[1]; + + beforeEach(async function () { + await token.setOperatorApproval(sender, true, { from: _creator }); + }); + + it('transfers the ownership of the given token ID to the given address', async function () { + await token.takeOwnership(tokenId, { from: sender }); + + const newOwner = await token.ownerOf(tokenId); + newOwner.should.be.equal(sender); + }); + + it('clears the approval for the token ID', async function () { + await token.takeOwnership(tokenId, { from: sender }); + + const approvedAccount = await token.approvedFor(tokenId); + approvedAccount.should.be.equal(ZERO_ADDRESS); + }); + + it('emits an approval and transfer events', async function () { + const { logs } = await token.takeOwnership(tokenId, { from: sender }); + + logs.length.should.be.equal(2); + + logs[0].event.should.be.eq('Approval'); + logs[0].args._owner.should.be.equal(_creator); + logs[0].args._approved.should.be.equal(ZERO_ADDRESS); + logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + + logs[1].event.should.be.eq('Transfer'); + logs[1].args._from.should.be.equal(_creator); + logs[1].args._to.should.be.equal(sender); + logs[1].args._tokenId.should.be.bignumber.equal(tokenId); + }); + + it('adjusts owners balances', async function () { + const previousBalance = await token.balanceOf(_creator); + + await token.takeOwnership(tokenId, { from: sender }); + + const newOwnerBalance = await token.balanceOf(sender); + newOwnerBalance.should.be.bignumber.equal(1); + + const previousOwnerBalance = await token.balanceOf(_creator); + previousOwnerBalance.should.be.bignumber.equal(previousBalance - 1); + }); + + it('places the last token of the sender in the position of the transferred token', async function () { + const firstTokenIndex = 0; + const lastTokenIndex = await token.balanceOf(_creator) - 1; + const lastToken = await token.tokenOfOwnerByIndex(_creator, lastTokenIndex); + + await token.takeOwnership(tokenId, { from: sender }); + + const swappedToken = await token.tokenOfOwnerByIndex(_creator, firstTokenIndex); + swappedToken.should.be.bignumber.equal(lastToken); + await assertRevert(token.tokenOfOwnerByIndex(_creator, lastTokenIndex)); + }); + + it('adds the token to the tokens list of the new owner', async function () { + await token.takeOwnership(tokenId, { from: sender }); + + const tokenIDs = await token.tokensOf(sender); + tokenIDs.length.should.be.equal(1); + tokenIDs[0].should.be.bignumber.equal(tokenId); + }); + }); + }); + + describe('takeOwnershipFor', function () { + describe('when the given token ID was already tracked by this contract', function () { + const tokenId = _firstTokenId; + + describe('when the sender has the approval for the token ID', function () { + const sender = accounts[1]; + + beforeEach(async function () { + await token.approve(sender, tokenId, { from: _creator }); + }); + + describe('when the recipient is the zero address', function () { + const to = ZERO_ADDRESS; + + it('reverts', async function () { + await assertRevert(token.takeOwnershipFor(to, tokenId, { from: sender })); + }); + }); + + describe('when the recipient is the owner', function () { + const to = _creator; + + it('reverts', async function () { + await assertRevert(token.takeOwnershipFor(to, tokenId, { from: sender })); + }); + }); + + describe('when the recipient is not the zero address neither the owner', function () { + const to = accounts[2]; + + it('transfers the ownership of the given token ID to the given address', async function () { + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const newOwner = await token.ownerOf(tokenId); + newOwner.should.be.equal(to); + }); + + it('clears the approval for the token ID', async function () { + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const approvedAccount = await token.approvedFor(tokenId); + approvedAccount.should.be.equal(ZERO_ADDRESS); + }); + + it('emits an approval and transfer events', async function () { + const { logs } = await token.takeOwnershipFor(to, tokenId, { from: sender }); + + logs.length.should.be.equal(2); + + logs[0].event.should.be.eq('Approval'); + logs[0].args._owner.should.be.equal(_creator); + logs[0].args._approved.should.be.equal(ZERO_ADDRESS); + logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + + logs[1].event.should.be.eq('Transfer'); + logs[1].args._from.should.be.equal(_creator); + logs[1].args._to.should.be.equal(to); + logs[1].args._tokenId.should.be.bignumber.equal(tokenId); + }); + + it('adjusts owners balances', async function () { + const previousBalance = await token.balanceOf(_creator); + + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const newOwnerBalance = await token.balanceOf(to); + newOwnerBalance.should.be.bignumber.equal(1); + + const previousOwnerBalance = await token.balanceOf(_creator); + previousOwnerBalance.should.be.bignumber.equal(previousBalance - 1); + }); + + it('places the last token of the sender in the position of the transferred token', async function () { + const firstTokenIndex = 0; + const lastTokenIndex = await token.balanceOf(_creator) - 1; + const lastToken = await token.tokenOfOwnerByIndex(_creator, lastTokenIndex); + + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const swappedToken = await token.tokenOfOwnerByIndex(_creator, firstTokenIndex); + swappedToken.should.be.bignumber.equal(lastToken); + await assertRevert(token.tokenOfOwnerByIndex(_creator, lastTokenIndex)); + }); + + it('adds the token to the tokens list of the new owner', async function () { + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const tokenIDs = await token.tokensOf(to); + tokenIDs.length.should.be.equal(1); + tokenIDs[0].should.be.bignumber.equal(tokenId); + }); + }); + }); + + describe('when the sender was approved by the owner of the token ID', function () { + const sender = accounts[1]; + + beforeEach(async function () { + await token.setOperatorApproval(sender, true, { from: _creator }); + }); + + describe('when the recipient is the zero address', function () { + const to = ZERO_ADDRESS; + + it('reverts', async function () { + await assertRevert(token.takeOwnershipFor(to, tokenId, { from: sender })); + }); + }); + + describe('when the recipient is the owner', function () { + const to = _creator; + + it('reverts', async function () { + await assertRevert(token.takeOwnershipFor(to, tokenId, { from: sender })); + }); + }); + + describe('when the recipient is not the zero address neither the owner', function () { + const to = accounts[2]; + + it('transfers the ownership of the given token ID to the given address', async function () { + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const newOwner = await token.ownerOf(tokenId); + newOwner.should.be.equal(to); + }); + + it('clears the approval for the token ID', async function () { + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const approvedAccount = await token.approvedFor(tokenId); + approvedAccount.should.be.equal(ZERO_ADDRESS); + }); + + it('emits an approval and transfer events', async function () { + const { logs } = await token.takeOwnershipFor(to, tokenId, { from: sender }); + + logs.length.should.be.equal(2); + + logs[0].event.should.be.eq('Approval'); + logs[0].args._owner.should.be.equal(_creator); + logs[0].args._approved.should.be.equal(ZERO_ADDRESS); + logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + + logs[1].event.should.be.eq('Transfer'); + logs[1].args._from.should.be.equal(_creator); + logs[1].args._to.should.be.equal(to); + logs[1].args._tokenId.should.be.bignumber.equal(tokenId); + }); + + it('adjusts owners balances', async function () { + const previousBalance = await token.balanceOf(_creator); + + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const newOwnerBalance = await token.balanceOf(to); + newOwnerBalance.should.be.bignumber.equal(1); + + const previousOwnerBalance = await token.balanceOf(_creator); + previousOwnerBalance.should.be.bignumber.equal(previousBalance - 1); + }); + + it('places the last token of the sender in the position of the transferred token', async function () { + const firstTokenIndex = 0; + const lastTokenIndex = await token.balanceOf(_creator) - 1; + const lastToken = await token.tokenOfOwnerByIndex(_creator, lastTokenIndex); + + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const swappedToken = await token.tokenOfOwnerByIndex(_creator, firstTokenIndex); + swappedToken.should.be.bignumber.equal(lastToken); + await assertRevert(token.tokenOfOwnerByIndex(_creator, lastTokenIndex)); + }); + + it('adds the token to the tokens list of the new owner', async function () { + await token.takeOwnershipFor(to, tokenId, { from: sender }); + + const tokenIDs = await token.tokensOf(to); + tokenIDs.length.should.be.equal(1); + tokenIDs[0].should.be.bignumber.equal(tokenId); + }); + }); + }); + + describe('when the sender does not have an approval for the token ID', function () { + const sender = accounts[1]; + + it('reverts', async function () { + await assertRevert(token.takeOwnershipFor(accounts[2], tokenId, { from: sender })); + }); + }); + + describe('when the sender is already the owner of the token', function () { + const sender = _creator; + + it('reverts', async function () { + await assertRevert(token.takeOwnershipFor(accounts[2], tokenId, { from: sender })); + }); + }); + }); + + describe('when the given token ID was not tracked by the contract before', function () { + const tokenId = _unknownTokenId; + + it('reverts', async function () { + await assertRevert(token.takeOwnershipFor(accounts[2], tokenId, { from: _creator })); + }); + }); + }); +}); From e9f0b74d9d8afe0ff06b4a63bf273963db14a466 Mon Sep 17 00:00:00 2001 From: Facundo Spagnuolo Date: Mon, 26 Feb 2018 19:20:10 -0300 Subject: [PATCH 3/3] Implement ERC721 token with metadata --- contracts/mocks/ERC721TokenMetadataMock.sol | 16 ++++ contracts/token/ERC721/ERC721Metadata.sol | 15 ++++ .../token/ERC721/ERC721TokenMetadata.sol | 39 ++++++++ test/token/ERC721/ERC721TokenMetadata.test.js | 88 +++++++++++++++++++ 4 files changed, 158 insertions(+) create mode 100644 contracts/mocks/ERC721TokenMetadataMock.sol create mode 100644 contracts/token/ERC721/ERC721Metadata.sol create mode 100644 contracts/token/ERC721/ERC721TokenMetadata.sol create mode 100644 test/token/ERC721/ERC721TokenMetadata.test.js diff --git a/contracts/mocks/ERC721TokenMetadataMock.sol b/contracts/mocks/ERC721TokenMetadataMock.sol new file mode 100644 index 00000000000..ac68a0fffaf --- /dev/null +++ b/contracts/mocks/ERC721TokenMetadataMock.sol @@ -0,0 +1,16 @@ +pragma solidity ^0.4.18; + +import "./ERC721TokenMock.sol"; +import "../token/ERC721/ERC721TokenMetadata.sol"; + +/** + * @title ERC721TokenMetadataMock + * This mock just provides a public mint and burn functions for testing purposes. + */ +contract ERC721TokenMetadataMock is ERC721TokenMetadata, ERC721TokenMock { + function ERC721TokenMetadataMock(string name, string symbol) + ERC721TokenMock(name, symbol) + ERC721TokenMetadata(name, symbol) + public + { } +} diff --git a/contracts/token/ERC721/ERC721Metadata.sol b/contracts/token/ERC721/ERC721Metadata.sol new file mode 100644 index 00000000000..eb755e341c0 --- /dev/null +++ b/contracts/token/ERC721/ERC721Metadata.sol @@ -0,0 +1,15 @@ +pragma solidity ^0.4.18; + +import "./ERC721.sol"; + + +/** + * @title Full ERC721 interface with metadata + * @dev see https://github.com/ethereum/eips/issues/721 and https://github.com/ethereum/EIPs/pull/841 + */ +contract ERC721Metadata is ERC721 { + event MetadataUpdated(address _owner, uint256 _tokenId, string _newMetadata); + + function setTokenMetadata(uint256 _tokenId, string _metadata) public; + function tokenMetadata(uint256 _tokenId) public view returns (string infoUrl); +} diff --git a/contracts/token/ERC721/ERC721TokenMetadata.sol b/contracts/token/ERC721/ERC721TokenMetadata.sol new file mode 100644 index 00000000000..580710de03a --- /dev/null +++ b/contracts/token/ERC721/ERC721TokenMetadata.sol @@ -0,0 +1,39 @@ +pragma solidity ^0.4.18; + +import "./ERC721Token.sol"; +import "./ERC721Metadata.sol"; + + +/** + * @title Full ERC721 Token with metadata + * This implementation includes all the functionality of the ERC721 standard besides metadata functionality + * @dev see https://github.com/ethereum/eips/issues/721 and https://github.com/ethereum/EIPs/pull/841 + */ +contract ERC721TokenMetadata is ERC721Metadata, ERC721Token { + // Tokens metadata + mapping (uint256 => string) private _metadata; + + /** + * @dev Constructor function + */ + function ERC721TokenMetadata(string name, string symbol) ERC721Token(name, symbol) public {} + + /** + * @dev Gets the metadata of the given token ID + * @param _tokenId uint256 ID of the token to query the metadata of + * @return string representing the metadata of the given token ID + */ + function tokenMetadata(uint256 _tokenId) public view returns (string) { + return _metadata[_tokenId]; + } + + /** + * @dev Sets the metadata of the given token ID + * @param _tokenId uint256 ID of the token to set the metadata of + * @param _newMetadata string representing the new metadata to be set + */ + function setTokenMetadata(uint256 _tokenId, string _newMetadata) public onlyOwnerOf(_tokenId) { + _metadata[_tokenId] = _newMetadata; + MetadataUpdated(msg.sender, _tokenId, _newMetadata); + } +} diff --git a/test/token/ERC721/ERC721TokenMetadata.test.js b/test/token/ERC721/ERC721TokenMetadata.test.js new file mode 100644 index 00000000000..c848b16ff08 --- /dev/null +++ b/test/token/ERC721/ERC721TokenMetadata.test.js @@ -0,0 +1,88 @@ +import assertRevert from '../../helpers/assertRevert'; +const BigNumber = web3.BigNumber; +const ERC721TokenMetadata = artifacts.require('ERC721TokenMetadataMock.sol'); + +require('chai') + .use(require('chai-as-promised')) + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('ERC721TokenMetadata', accounts => { + let token = null; + const _name = 'Non Fungible Token'; + const _symbol = 'NFT'; + const _firstTokenId = 1; + const _secondTokenId = 2; + const _unknownTokenId = 3; + const _creator = accounts[0]; + + beforeEach(async function () { + token = await ERC721TokenMetadata.new(_name, _symbol, { from: _creator }); + await token.mint(_creator, _firstTokenId, { from: _creator }); + await token.mint(_creator, _secondTokenId, { from: _creator }); + }); + + describe('tokenMetadata', function () { + describe('when no metadata was set', function () { + it('the given token has no metadata', async function () { + const metadata = await token.tokenMetadata(_firstTokenId); + + metadata.should.be.equal(''); + }); + }); + + describe('when some metadata was set', function () { + it('returns the metadata of the given token', async function () { + await token.setTokenMetadata(_firstTokenId, 'dummy metadata', { from: _creator }); + + const metadata = await token.tokenMetadata(_firstTokenId); + + metadata.should.be.equal('dummy metadata'); + }); + }); + }); + + describe('setTokenMetadata', function () { + describe('when the sender is not the token owner', function () { + const sender = accounts[1]; + + it('reverts', async function () { + await assertRevert(token.setTokenMetadata(_firstTokenId, 'new metadata', { from: sender })); + }); + }); + + describe('when the sender is the owner of the token', function () { + const sender = _creator; + + describe('when the given token ID was tracked by this contract before', function () { + const tokenId = _firstTokenId; + + it('updates the metadata of the given token ID', async function () { + await token.setTokenMetadata(_firstTokenId, 'new metadata', { from: sender }); + + const metadata = await token.tokenMetadata(tokenId); + + metadata.should.be.equal('new metadata'); + }); + + it('emits a metadata updated event', async function () { + const { logs } = await token.setTokenMetadata(tokenId, 'new metadata', { from: sender }); + + logs.length.should.be.equal(1); + logs[0].event.should.be.eq('MetadataUpdated'); + logs[0].args._owner.should.be.equal(sender); + logs[0].args._tokenId.should.be.bignumber.equal(tokenId); + logs[0].args._newMetadata.should.be.equal('new metadata'); + }); + }); + + describe('when the given token ID was not tracked by this contract before', function () { + const tokenId = _unknownTokenId; + + it('reverts', async function () { + await assertRevert(token.setTokenMetadata(tokenId, 'new metadata'), { from: sender }); + }); + }); + }); + }); +});