From fd6cd316b1dcf820e06343ec18c676cc587bdcad Mon Sep 17 00:00:00 2001 From: Vinko Bradvica Date: Fri, 4 May 2018 10:46:42 +0200 Subject: [PATCH 1/3] ERC827-like methods for ERC721Holdings --- contracts/ERC721Holdings.sol | 3 +- contracts/ERC721HoldingsExecuteCalls.sol | 19 ++ contracts/ERC721HoldingsExecuteCallsToken.sol | 38 ++++ contracts/ERC721HoldingsToken.sol | 3 +- .../mocks/ERC721HoldingsBasicTokenMock.sol | 8 +- contracts/mocks/MessageHelper.sol | 30 +++ test/ERC721HoldingsBasicToken.test.js | 21 -- test/ERC721HoldingsExecuteCalls.behaviour.js | 200 ++++++++++++++++++ test/ERC721HoldingsMintBurn.behaviour.js | 2 +- ...ur.js => ERC721HoldingsToken.behaviour.js} | 4 +- test/ERC721HoldingsToken.test.js | 23 ++ 11 files changed, 321 insertions(+), 30 deletions(-) create mode 100644 contracts/ERC721HoldingsExecuteCalls.sol create mode 100644 contracts/ERC721HoldingsExecuteCallsToken.sol create mode 100644 contracts/mocks/MessageHelper.sol delete mode 100644 test/ERC721HoldingsBasicToken.test.js create mode 100644 test/ERC721HoldingsExecuteCalls.behaviour.js rename test/{ERC721HoldingsBasicToken.behaviour.js => ERC721HoldingsToken.behaviour.js} (99%) create mode 100644 test/ERC721HoldingsToken.test.js diff --git a/contracts/ERC721Holdings.sol b/contracts/ERC721Holdings.sol index b4dc4be..01b90ba 100644 --- a/contracts/ERC721Holdings.sol +++ b/contracts/ERC721Holdings.sol @@ -1,6 +1,7 @@ pragma solidity ^0.4.23; import "./ERC721HoldingsBasic.sol"; +import "./ERC721HoldingsExecuteCalls.sol"; /** @@ -29,4 +30,4 @@ contract ERC721HoldingsMetadata is ERC721HoldingsBasic { * @title Holdings of ERC-721 Non-Fungible Token Standard, full implementation interface * @dev See https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md */ -contract ERC721Holdings is ERC721HoldingsBasic, ERC721HoldingsEnumerable, ERC721HoldingsMetadata {} +contract ERC721Holdings is ERC721HoldingsBasic, ERC721HoldingsEnumerable, ERC721HoldingsMetadata, ERC721HoldingsExecuteCalls {} diff --git a/contracts/ERC721HoldingsExecuteCalls.sol b/contracts/ERC721HoldingsExecuteCalls.sol new file mode 100644 index 0000000..270a66f --- /dev/null +++ b/contracts/ERC721HoldingsExecuteCalls.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.4.23; + +import "./ERC721HoldingsBasic.sol"; + +contract ERC721HoldingsExecuteCalls is ERC721HoldingsBasic { + function approveAndCall( + address _spender, + uint256 _tokenId, + bytes _data + ) public payable returns (bool); + + function transferFromAndCall( + address _from, + uint256 _to, + address _toOrigin, + uint256 _tokenId, + bytes _data + ) public payable returns (bool); +} \ No newline at end of file diff --git a/contracts/ERC721HoldingsExecuteCallsToken.sol b/contracts/ERC721HoldingsExecuteCallsToken.sol new file mode 100644 index 0000000..6e8ad2f --- /dev/null +++ b/contracts/ERC721HoldingsExecuteCallsToken.sol @@ -0,0 +1,38 @@ +pragma solidity ^0.4.23; + +import "./ERC721HoldingsBasicToken.sol"; + +contract ERC721HoldingsExecuteCallsToken is ERC721HoldingsBasicToken { + + function approveAndCall(address _spender, uint256 _tokenId, bytes _data) + public payable returns (bool) + { + super.approve(_spender, _tokenId); + + // solium-disable-next-line security/no-call-value + require(_spender.call.value(msg.value)(_data)); + + return true; + } + + // ERC721 doesn't implement transfer function + + function transferFromAndCall( + address _from, + uint256 _to, + address _toOrigin, + uint256 _tokenId, + bytes _data + ) + public payable returns (bool) + { + address _holderOwner = _ownerOf(_to, _toOrigin); + require(_holderOwner != address(this)); + + super.transferFrom(_from, _to, _toOrigin, _tokenId); + + // solium-disable-next-line security/no-call-value + require(_holderOwner.call.value(msg.value)(_data)); + return true; + } +} diff --git a/contracts/ERC721HoldingsToken.sol b/contracts/ERC721HoldingsToken.sol index 20e76ef..4078681 100644 --- a/contracts/ERC721HoldingsToken.sol +++ b/contracts/ERC721HoldingsToken.sol @@ -2,6 +2,7 @@ pragma solidity ^0.4.23; import "./ERC721Holdings.sol"; import "./ERC721HoldingsBasicToken.sol"; +import "./ERC721HoldingsExecuteCallsToken.sol"; /** @@ -9,7 +10,7 @@ import "./ERC721HoldingsBasicToken.sol"; * This implementation includes all the required and some optional functionality of the ERC721Holdings standard * @dev see https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md */ -contract ERC721HoldingsToken is ERC721Holdings, ERC721HoldingsBasicToken { +contract ERC721HoldingsToken is ERC721Holdings, ERC721HoldingsBasicToken, ERC721HoldingsExecuteCallsToken { // Token name string internal name_; diff --git a/contracts/mocks/ERC721HoldingsBasicTokenMock.sol b/contracts/mocks/ERC721HoldingsBasicTokenMock.sol index e406a39..27bd670 100644 --- a/contracts/mocks/ERC721HoldingsBasicTokenMock.sol +++ b/contracts/mocks/ERC721HoldingsBasicTokenMock.sol @@ -1,15 +1,15 @@ pragma solidity ^0.4.23; -import "../ERC721HoldingsBasicToken.sol"; +import "../ERC721HoldingsToken.sol"; /** - * @title ERC721HoldingsBasicTokenMock + * @title ERC721HoldingsTokenMock * This mock just provides a public mint and burn functions for testing purposes */ -contract ERC721HoldingsBasicTokenMock is ERC721HoldingsBasicToken { +contract ERC721HoldingsTokenMock is ERC721HoldingsToken { - constructor (address _nftAddress) ERC721HoldingsBasicToken(_nftAddress) public { + constructor (address _nftAddress) ERC721HoldingsToken("Test", "TST", _nftAddress) public { require(_nftAddress != address(0)); tokens = ERC721(_nftAddress); } diff --git a/contracts/mocks/MessageHelper.sol b/contracts/mocks/MessageHelper.sol new file mode 100644 index 0000000..47bcc73 --- /dev/null +++ b/contracts/mocks/MessageHelper.sol @@ -0,0 +1,30 @@ +pragma solidity ^0.4.21; + +contract MessageHelper { + + event Show(bytes32 b32, uint256 number, string text); + event Buy(bytes32 b32, uint256 number, string text, uint256 value); + + function showMessage( bytes32 message, uint256 number, string text ) public returns (bool) { + emit Show(message, number, text); + return true; + } + + function buyMessage( bytes32 message, uint256 number, string text ) public payable returns (bool) { + emit Buy(message, number, text, msg.value); + return true; + } + + function fail() public pure { + require(false); + } + + function call(address to, bytes data) public returns (bool) { + // solium-disable-next-line security/no-low-level-calls + if (to.call(data)) + return true; + else + return false; + } + +} \ No newline at end of file diff --git a/test/ERC721HoldingsBasicToken.test.js b/test/ERC721HoldingsBasicToken.test.js deleted file mode 100644 index f1d8900..0000000 --- a/test/ERC721HoldingsBasicToken.test.js +++ /dev/null @@ -1,21 +0,0 @@ -import shouldBehaveLikeERC721HoldingsBasicToken from './ERC721HoldingsBasicToken.behaviour'; -import shouldMintAndBurnERC721HoldingsToken from './ERC721HoldingsMintBurn.behaviour'; - -const BigNumber = web3.BigNumber; -const ERC721BasicToken = artifacts.require('mocks/ERC721BasicTokenMock'); -const ERC721HoldingsBasicToken = artifacts.require('mocks/ERC721HoldingsBasicTokenMock.sol'); - -require('chai') - .use(require('chai-as-promised')) - .use(require('chai-bignumber')(BigNumber)) - .should(); - -contract('ERC721HoldingsBasicToken', function (accounts) { - beforeEach(async function () { - this.avatars = await ERC721BasicToken.new({ from: accounts[0] }); - this.token = await ERC721HoldingsBasicToken.new(this.avatars.address, { from: accounts[0] }); - }); - - shouldBehaveLikeERC721HoldingsBasicToken(accounts); - shouldMintAndBurnERC721HoldingsToken(accounts); -}); diff --git a/test/ERC721HoldingsExecuteCalls.behaviour.js b/test/ERC721HoldingsExecuteCalls.behaviour.js new file mode 100644 index 0000000..f02cf52 --- /dev/null +++ b/test/ERC721HoldingsExecuteCalls.behaviour.js @@ -0,0 +1,200 @@ +const Message = artifacts.require('mocks/MessageHelper.sol'); + +import assertRevert from './helpers/assertRevert'; +const BigNumber = web3.BigNumber; + +require('chai') + .use(require('chai-as-promised')) + .use(require('chai-bignumber')(BigNumber)) + .should(); + +export default function shouldExecuteCallsERC721HoldingsToken (accounts) { + const firstTokenId = 1; + const secondTokenId = 2; + const unknownTokenId = 3; + + const firstAvatarId = 1; + const secondAvatarId = 2; + const thirdAvatarId = 3; + + const creator = accounts[0]; + const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; + + describe('like an execute calls ERC721HoldingsToken', function () { + beforeEach(async function () { + this.message = await Message.new({ from: creator }); + + await this.avatars.mint(creator, firstAvatarId, { from: creator }); + await this.avatars.mint(this.message.contract.address, secondAvatarId, { from: creator }); + await this.avatars.mint(this.token.contract.address, thirdAvatarId, { from: creator }); + + await this.token.mint(firstAvatarId, this.avatars.address, firstTokenId, { from: creator }); + await this.token.mint(firstAvatarId, this.avatars.address, secondTokenId, { from: creator }); + }); + + describe('Test Execute Calls methods', function () { + it('should allow payment through approve', async function () { + const extraData = this.message.contract.buyMessage.getData( + web3.toHex(123456), 666, 'Transfer Done' + ); + + const transaction = await this.token.approveAndCall( + this.message.contract.address, firstTokenId, extraData, { from: creator, value: 1 } + ); + + assert.equal(2, transaction.receipt.logs.length); + + const appproved = await this.token.getApproved(firstTokenId); + this.message.contract.address.should.be.equal(appproved); + + const balance = await web3.eth.getBalance(this.message.contract.address); + new BigNumber(1).should.be.bignumber.equal(balance); + }); + + it('should allow payment through transferFrom', async function () { + const extraData = this.message.contract.buyMessage.getData( + web3.toHex(123456), 666, 'Transfer Done' + ); + + await this.token.approve(accounts[1], firstTokenId, { from: creator }); + + const appproved = await this.token.getApproved(firstTokenId); + accounts[1].should.be.equal(appproved); + + const transaction = await this.token.transferFromAndCall( + accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, extraData, { from: accounts[1], value: 1 } + ); + + assert.equal(3, transaction.receipt.logs.length); + + const tokenBalance = await this.token.balanceOf(secondAvatarId, this.avatars.address); + new BigNumber(1).should.be.bignumber.equal(tokenBalance); + + const ethBalance = await web3.eth.getBalance(this.message.contract.address); + new BigNumber(1).should.be.bignumber.equal(ethBalance); + }); + + it('should revert funds of failure inside approve (with data)', async function () { + // showMessage is not payable, so it fails when called with msg.value + const extraData = this.message.contract.showMessage.getData( + web3.toHex(123456), 666, 'Transfer Done' + ); + + await this.token.approveAndCall( + this.message.contract.address, firstTokenId, extraData, { from: creator, value: 1 } + ).should.be.rejectedWith('revert'); + + // approval should not have gone through so approved address is still 0x0 + const appproved = await this.token.getApproved(firstTokenId); + ZERO_ADDRESS.should.be.equal(appproved); + + const ethBalance = await web3.eth.getBalance(this.message.contract.address); + new BigNumber(0).should.be.bignumber.equal(ethBalance); + }); + + it('should revert funds of failure inside transferFrom (with data)', async function () { + // showMessage is not payable, so it fails when called with msg.value + const extraData = this.message.contract.showMessage.getData( + web3.toHex(123456), 666, 'Transfer Done' + ); + + await this.token.approve(accounts[1], firstTokenId, { from: creator }); + + await this.token.transferFromAndCall( + accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, extraData, { from: accounts[1], value: 1 } + ).should.be.rejectedWith('revert');; + + // transferFrom should not have gone through so approved address is still accounts[1] + const appproved = await this.token.getApproved(firstTokenId); + accounts[1].should.be.equal(appproved); + + const tokenBalance = await this.token.balanceOf(secondAvatarId, this.avatars.address); + new BigNumber(0).should.be.bignumber.equal(tokenBalance); + + const ethBalance = await web3.eth.getBalance(this.message.contract.address); + new BigNumber(0).should.be.bignumber.equal(ethBalance); + }); + + it('should return correct allowance after approve (with data) and show the event on receiver contract', async function () { + const extraData = this.message.contract.showMessage.getData( + web3.toHex(123456), 666, 'Transfer Done' + ); + + const transaction = await this.token.approveAndCall( + this.message.contract.address, firstTokenId, extraData, { from: creator } + ); + + assert.equal(2, transaction.receipt.logs.length); + + const appproved = await this.token.getApproved(firstTokenId); + this.message.contract.address.should.be.equal(appproved); + }); + + it('should return correct balances after transferFrom (with data) and show the event on receiver contract', async function () { + const extraData = this.message.contract.showMessage.getData( + web3.toHex(123456), 666, 'Transfer Done' + ); + + await this.token.approve(accounts[1], firstTokenId, { from: accounts[0] }); + + const appproved = await this.token.getApproved(firstTokenId); + accounts[1].should.be.equal(appproved); + + const transaction = await this.token.transferFromAndCall( + accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, extraData, { from: accounts[1] } + ); + + assert.equal(3, transaction.receipt.logs.length); + + const tokenBalance = await this.token.balanceOf(secondAvatarId, this.avatars.address); + new BigNumber(1).should.be.bignumber.equal(tokenBalance); + }); + + it('should fail inside approve (with data)', async function () { + const extraData = this.message.contract.fail.getData(); + + await this.token.approveAndCall(this.message.contract.address, firstTokenId, extraData) + .should.be.rejectedWith('revert'); + + // approval should not have gone through so approved is still ZERO_ADDRESS + const appproved = await this.token.getApproved(firstTokenId); + ZERO_ADDRESS.should.be.equal(appproved); + }); + + it('should fail inside transferFrom (with data)', async function () { + const extraData = this.message.contract.fail.getData(); + + await this.token.approve(accounts[1], firstAvatarId, { from: creator }); + await this.token.transferFromAndCall(creator, secondAvatarId, this.avatars.address, firstTokenId, extraData, { from: creator }) + .should.be.rejectedWith('revert'); + + // transferFrom should have failed so balance is still 0 but approved is accounts[1] + const appproved = await this.token.getApproved(firstTokenId); + accounts[1].should.be.equal(appproved); + + const tokenBalance = await this.token.balanceOf(secondAvatarId, this.avatars.address); + new BigNumber(0).should.be.bignumber.equal(tokenBalance); + }); + + it('should fail approve (with data) when using token contract address as receiver', async function () { + const extraData = this.message.contract.showMessage.getData( + web3.toHex(123456), 666, 'Transfer Done' + ); + + await this.token.approveAndCall(this.token.contract.address, firstAvatarId, extraData, { from: accounts[0] }) + .should.be.rejectedWith('revert'); + }); + + it('should fail transferFrom (with data) when using token contract address as receiver', async function () { + const extraData = this.message.contract.showMessage.getData( + web3.toHex(123456), 666, 'Transfer Done' + ); + + await this.token.approve(accounts[1], firstTokenId, { from: accounts[0] }); + + await this.token.transferFromAndCall(accounts[0], thirdAvatarId, this.avatars.address, firstTokenId, extraData, { from: accounts[1] }) + .should.be.rejectedWith('revert'); + }); + }); + }); +}; \ No newline at end of file diff --git a/test/ERC721HoldingsMintBurn.behaviour.js b/test/ERC721HoldingsMintBurn.behaviour.js index 9dfb783..c147e27 100644 --- a/test/ERC721HoldingsMintBurn.behaviour.js +++ b/test/ERC721HoldingsMintBurn.behaviour.js @@ -17,7 +17,7 @@ export default function shouldMintAndBurnERC721HoldingsToken (accounts) { const creator = accounts[0]; const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; - describe('like a mintable and burnable ERC721Token', function () { + describe('like a mintable and burnable ERC721HoldingsToken', function () { beforeEach(async function () { await this.avatars.mint(creator, firstAvatarId, { from: creator }); await this.avatars.mint(creator, secondAvatarId, { from: creator }); diff --git a/test/ERC721HoldingsBasicToken.behaviour.js b/test/ERC721HoldingsToken.behaviour.js similarity index 99% rename from test/ERC721HoldingsBasicToken.behaviour.js rename to test/ERC721HoldingsToken.behaviour.js index 4242a1e..92e6266 100644 --- a/test/ERC721HoldingsBasicToken.behaviour.js +++ b/test/ERC721HoldingsToken.behaviour.js @@ -9,7 +9,7 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -export default function shouldBehaveLikeERC721HoldingsBasicToken (accounts) { +export default function shouldBehaveLikeERC721HoldingsToken (accounts) { const firstTokenId = 1; const secondTokenId = 2; const unknownTokenId = 3; @@ -20,7 +20,7 @@ export default function shouldBehaveLikeERC721HoldingsBasicToken (accounts) { const creator = accounts[0]; const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; - describe('like a ERC721HoldingsBasicToken', function () { + describe('like a ERC721HoldingsToken', function () { beforeEach(async function () { await this.avatars.mint(creator, firstAvatarId, { from: creator }); await this.avatars.mint(creator, secondAvatarId, { from: creator }); diff --git a/test/ERC721HoldingsToken.test.js b/test/ERC721HoldingsToken.test.js new file mode 100644 index 0000000..cb2029a --- /dev/null +++ b/test/ERC721HoldingsToken.test.js @@ -0,0 +1,23 @@ +import shouldBehaveLikeERC721HoldingsToken from './ERC721HoldingsToken.behaviour'; +import shouldMintAndBurnERC721HoldingsToken from './ERC721HoldingsMintBurn.behaviour'; +import shouldExecuteCallsERC721HoldingsToken from './ERC721HoldingsExecuteCalls.behaviour'; + +const BigNumber = web3.BigNumber; +const ERC721BasicToken = artifacts.require('mocks/ERC721BasicTokenMock'); +const ERC721HoldingsToken = artifacts.require('mocks/ERC721HoldingsTokenMock.sol'); + +require('chai') + .use(require('chai-as-promised')) + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('ERC721HoldingsToken', function (accounts) { + beforeEach(async function () { + this.avatars = await ERC721BasicToken.new({ from: accounts[0] }); + this.token = await ERC721HoldingsToken.new(this.avatars.address, { from: accounts[0] }); + }); + + shouldBehaveLikeERC721HoldingsToken(accounts); + shouldMintAndBurnERC721HoldingsToken(accounts); + shouldExecuteCallsERC721HoldingsToken(accounts); +}); From c82639cab88400cff45e37bf26d892ce5677b0a4 Mon Sep 17 00:00:00 2001 From: Vinko Bradvica Date: Fri, 4 May 2018 11:00:24 +0200 Subject: [PATCH 2/3] Fixed lint:sol errors --- contracts/ERC721HoldingsExecuteCalls.sol | 17 +++++------------ contracts/ERC721HoldingsExecuteCallsToken.sol | 6 ++---- contracts/mocks/MessageHelper.sol | 1 + 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/contracts/ERC721HoldingsExecuteCalls.sol b/contracts/ERC721HoldingsExecuteCalls.sol index 270a66f..5c40241 100644 --- a/contracts/ERC721HoldingsExecuteCalls.sol +++ b/contracts/ERC721HoldingsExecuteCalls.sol @@ -2,18 +2,11 @@ pragma solidity ^0.4.23; import "./ERC721HoldingsBasic.sol"; + contract ERC721HoldingsExecuteCalls is ERC721HoldingsBasic { - function approveAndCall( - address _spender, - uint256 _tokenId, - bytes _data - ) public payable returns (bool); + function approveAndCall(address _spender, uint256 _tokenId, bytes _data) + public payable returns (bool); - function transferFromAndCall( - address _from, - uint256 _to, - address _toOrigin, - uint256 _tokenId, - bytes _data - ) public payable returns (bool); + function transferFromAndCall(address _from, uint256 _to, address _toOrigin, uint256 _tokenId, bytes _data) + public payable returns (bool); } \ No newline at end of file diff --git a/contracts/ERC721HoldingsExecuteCallsToken.sol b/contracts/ERC721HoldingsExecuteCallsToken.sol index 6e8ad2f..542b1a2 100644 --- a/contracts/ERC721HoldingsExecuteCallsToken.sol +++ b/contracts/ERC721HoldingsExecuteCallsToken.sol @@ -2,6 +2,7 @@ pragma solidity ^0.4.23; import "./ERC721HoldingsBasicToken.sol"; + contract ERC721HoldingsExecuteCallsToken is ERC721HoldingsBasicToken { function approveAndCall(address _spender, uint256 _tokenId, bytes _data) @@ -15,15 +16,12 @@ contract ERC721HoldingsExecuteCallsToken is ERC721HoldingsBasicToken { return true; } - // ERC721 doesn't implement transfer function - function transferFromAndCall( address _from, uint256 _to, address _toOrigin, uint256 _tokenId, - bytes _data - ) + bytes _data) public payable returns (bool) { address _holderOwner = _ownerOf(_to, _toOrigin); diff --git a/contracts/mocks/MessageHelper.sol b/contracts/mocks/MessageHelper.sol index 47bcc73..aaffc22 100644 --- a/contracts/mocks/MessageHelper.sol +++ b/contracts/mocks/MessageHelper.sol @@ -1,5 +1,6 @@ pragma solidity ^0.4.21; + contract MessageHelper { event Show(bytes32 b32, uint256 number, string text); From e28fbeef492de1d2ee9705bd0af9dd61ee626c97 Mon Sep 17 00:00:00 2001 From: Vinko Bradvica Date: Fri, 4 May 2018 11:51:29 +0200 Subject: [PATCH 3/3] Replaced getData calls with encodeCall helpers --- contracts/ERC721HoldingsExecuteCalls.sol | 2 +- .../{mocks => test}/ERC721BasicTokenMock.sol | 0 .../ERC721HoldingsBasicTokenMock.sol | 0 contracts/{mocks => test}/MessageHelper.sol | 5 +- package-lock.json | 71 ++++++++++++++++ package.json | 1 + test/ERC721HoldingsExecuteCalls.behaviour.js | 81 +++++++------------ test/ERC721HoldingsToken.test.js | 4 +- test/helpers/encodeCall.js | 9 +++ 9 files changed, 115 insertions(+), 58 deletions(-) rename contracts/{mocks => test}/ERC721BasicTokenMock.sol (100%) rename contracts/{mocks => test}/ERC721HoldingsBasicTokenMock.sol (100%) rename contracts/{mocks => test}/MessageHelper.sol (96%) create mode 100644 test/helpers/encodeCall.js diff --git a/contracts/ERC721HoldingsExecuteCalls.sol b/contracts/ERC721HoldingsExecuteCalls.sol index 5c40241..1630d29 100644 --- a/contracts/ERC721HoldingsExecuteCalls.sol +++ b/contracts/ERC721HoldingsExecuteCalls.sol @@ -9,4 +9,4 @@ contract ERC721HoldingsExecuteCalls is ERC721HoldingsBasic { function transferFromAndCall(address _from, uint256 _to, address _toOrigin, uint256 _tokenId, bytes _data) public payable returns (bool); -} \ No newline at end of file +} diff --git a/contracts/mocks/ERC721BasicTokenMock.sol b/contracts/test/ERC721BasicTokenMock.sol similarity index 100% rename from contracts/mocks/ERC721BasicTokenMock.sol rename to contracts/test/ERC721BasicTokenMock.sol diff --git a/contracts/mocks/ERC721HoldingsBasicTokenMock.sol b/contracts/test/ERC721HoldingsBasicTokenMock.sol similarity index 100% rename from contracts/mocks/ERC721HoldingsBasicTokenMock.sol rename to contracts/test/ERC721HoldingsBasicTokenMock.sol diff --git a/contracts/mocks/MessageHelper.sol b/contracts/test/MessageHelper.sol similarity index 96% rename from contracts/mocks/MessageHelper.sol rename to contracts/test/MessageHelper.sol index aaffc22..d161390 100644 --- a/contracts/mocks/MessageHelper.sol +++ b/contracts/test/MessageHelper.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.21; +pragma solidity ^0.4.23; contract MessageHelper { @@ -27,5 +27,4 @@ contract MessageHelper { else return false; } - -} \ No newline at end of file +} diff --git a/package-lock.json b/package-lock.json index f067885..b71bc00 100644 --- a/package-lock.json +++ b/package-lock.json @@ -940,6 +940,21 @@ "integrity": "sha1-RqoXUftqL5PuXmibsQh9SxTGwgU=", "dev": true }, + "bindings": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.3.0.tgz", + "integrity": "sha512-DpLh5EzMR2kzvX1KIlVC0VkC3iZtHKTgdtZ0a3pglBZdaQFjt5S9g9xd1lE+YvXyfd6mtCeRnrUfOLYiTMlNSw==", + "dev": true + }, + "bip66": { + "version": "1.1.5", + "resolved": "https://registry.npmjs.org/bip66/-/bip66-1.1.5.tgz", + "integrity": "sha1-AfqHSHhcpwlV1QESF9GzE5lpyiI=", + "dev": true, + "requires": { + "safe-buffer": "5.1.2" + } + }, "bl": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/bl/-/bl-1.2.2.tgz", @@ -1678,6 +1693,17 @@ "integrity": "sha1-ZyIm3HTI95mtNTB9+TaroRrNYBg=", "dev": true }, + "drbg.js": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/drbg.js/-/drbg.js-1.0.1.tgz", + "integrity": "sha1-Pja2xCs3BDgjzbwzLVjzHiRFSAs=", + "dev": true, + "requires": { + "browserify-aes": "1.2.0", + "create-hash": "1.2.0", + "create-hmac": "1.1.7" + } + }, "duplexer3": { "version": "0.1.4", "resolved": "https://registry.npmjs.org/duplexer3/-/duplexer3-0.1.4.tgz", @@ -1778,6 +1804,29 @@ "xhr-request-promise": "0.1.2" } }, + "ethereumjs-abi": { + "version": "0.6.5", + "resolved": "https://registry.npmjs.org/ethereumjs-abi/-/ethereumjs-abi-0.6.5.tgz", + "integrity": "sha1-WmN+8Wq0NHP6cqKa2QhxQFs/UkE=", + "dev": true, + "requires": { + "bn.js": "4.11.6", + "ethereumjs-util": "4.5.0" + } + }, + "ethereumjs-util": { + "version": "4.5.0", + "resolved": "https://registry.npmjs.org/ethereumjs-util/-/ethereumjs-util-4.5.0.tgz", + "integrity": "sha1-PpQosxfuvaPXJg2FT93alUsfG8Y=", + "dev": true, + "requires": { + "bn.js": "4.11.6", + "create-hash": "1.2.0", + "keccakjs": "0.2.1", + "rlp": "2.0.0", + "secp256k1": "3.5.0" + } + }, "ethjs-abi": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/ethjs-abi/-/ethjs-abi-0.2.1.tgz", @@ -4408,6 +4457,12 @@ "inherits": "2.0.3" } }, + "rlp": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/rlp/-/rlp-2.0.0.tgz", + "integrity": "sha1-nbOE/0uJqPYVY9kjldhiWxjzr7A=", + "dev": true + }, "safe-buffer": { "version": "5.1.2", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz", @@ -4442,6 +4497,22 @@ "pbkdf2": "3.0.16" } }, + "secp256k1": { + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/secp256k1/-/secp256k1-3.5.0.tgz", + "integrity": "sha512-e5QIJl8W7Y4tT6LHffVcZAxJjvpgE5Owawv6/XCYPQljE9aP2NFFddQ8OYMKhdLshNu88FfL3qCN3/xYkXGRsA==", + "dev": true, + "requires": { + "bindings": "1.3.0", + "bip66": "1.1.5", + "bn.js": "4.11.6", + "create-hash": "1.2.0", + "drbg.js": "1.0.1", + "elliptic": "6.4.0", + "nan": "2.10.0", + "safe-buffer": "5.1.2" + } + }, "seek-bzip": { "version": "1.0.5", "resolved": "https://registry.npmjs.org/seek-bzip/-/seek-bzip-1.0.5.tgz", diff --git a/package.json b/package.json index efebfcf..c7046aa 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "chai": "^4.1.2", "chai-as-promised": "^7.1.1", "chai-bignumber": "^2.0.2", + "ethereumjs-abi": "^0.6.5", "ethjs-abi": "^0.2.1", "solium": "^1.1.7", "truffle": "^4.1.7", diff --git a/test/ERC721HoldingsExecuteCalls.behaviour.js b/test/ERC721HoldingsExecuteCalls.behaviour.js index f02cf52..30f252d 100644 --- a/test/ERC721HoldingsExecuteCalls.behaviour.js +++ b/test/ERC721HoldingsExecuteCalls.behaviour.js @@ -1,6 +1,7 @@ -const Message = artifacts.require('mocks/MessageHelper.sol'); +const Message = artifacts.require('test/MessageHelper.sol'); import assertRevert from './helpers/assertRevert'; +import encodeCall from './helpers/encodeCall'; const BigNumber = web3.BigNumber; require('chai') @@ -20,6 +21,20 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { const creator = accounts[0]; const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; + const buyMessageExtraData = encodeCall( + 'buyMessage', + ['bytes32', 'uint256', 'string'], + [web3.toHex(123456), 666, 'Transfer Done'] + ); + + const showMessageExtraData = encodeCall( + 'showMessage', + ['bytes32', 'uint256', 'string'], + [web3.toHex(123456), 666, 'Transfer Done'] + ); + + const failExtraData = encodeCall('fail', [], []); + describe('like an execute calls ERC721HoldingsToken', function () { beforeEach(async function () { this.message = await Message.new({ from: creator }); @@ -32,14 +47,10 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { await this.token.mint(firstAvatarId, this.avatars.address, secondTokenId, { from: creator }); }); - describe('Test Execute Calls methods', function () { + describe('test Execute Calls methods', function () { it('should allow payment through approve', async function () { - const extraData = this.message.contract.buyMessage.getData( - web3.toHex(123456), 666, 'Transfer Done' - ); - const transaction = await this.token.approveAndCall( - this.message.contract.address, firstTokenId, extraData, { from: creator, value: 1 } + this.message.contract.address, firstTokenId, buyMessageExtraData, { from: creator, value: 1 } ); assert.equal(2, transaction.receipt.logs.length); @@ -52,17 +63,13 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { }); it('should allow payment through transferFrom', async function () { - const extraData = this.message.contract.buyMessage.getData( - web3.toHex(123456), 666, 'Transfer Done' - ); - await this.token.approve(accounts[1], firstTokenId, { from: creator }); const appproved = await this.token.getApproved(firstTokenId); accounts[1].should.be.equal(appproved); const transaction = await this.token.transferFromAndCall( - accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, extraData, { from: accounts[1], value: 1 } + accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, buyMessageExtraData, { from: accounts[1], value: 1 } ); assert.equal(3, transaction.receipt.logs.length); @@ -75,13 +82,8 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { }); it('should revert funds of failure inside approve (with data)', async function () { - // showMessage is not payable, so it fails when called with msg.value - const extraData = this.message.contract.showMessage.getData( - web3.toHex(123456), 666, 'Transfer Done' - ); - await this.token.approveAndCall( - this.message.contract.address, firstTokenId, extraData, { from: creator, value: 1 } + this.message.contract.address, firstTokenId, showMessageExtraData, { from: creator, value: 1 } ).should.be.rejectedWith('revert'); // approval should not have gone through so approved address is still 0x0 @@ -93,15 +95,10 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { }); it('should revert funds of failure inside transferFrom (with data)', async function () { - // showMessage is not payable, so it fails when called with msg.value - const extraData = this.message.contract.showMessage.getData( - web3.toHex(123456), 666, 'Transfer Done' - ); - await this.token.approve(accounts[1], firstTokenId, { from: creator }); await this.token.transferFromAndCall( - accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, extraData, { from: accounts[1], value: 1 } + accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, showMessageExtraData, { from: accounts[1], value: 1 } ).should.be.rejectedWith('revert');; // transferFrom should not have gone through so approved address is still accounts[1] @@ -116,12 +113,8 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { }); it('should return correct allowance after approve (with data) and show the event on receiver contract', async function () { - const extraData = this.message.contract.showMessage.getData( - web3.toHex(123456), 666, 'Transfer Done' - ); - const transaction = await this.token.approveAndCall( - this.message.contract.address, firstTokenId, extraData, { from: creator } + this.message.contract.address, firstTokenId, showMessageExtraData, { from: creator } ); assert.equal(2, transaction.receipt.logs.length); @@ -131,17 +124,13 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { }); it('should return correct balances after transferFrom (with data) and show the event on receiver contract', async function () { - const extraData = this.message.contract.showMessage.getData( - web3.toHex(123456), 666, 'Transfer Done' - ); - await this.token.approve(accounts[1], firstTokenId, { from: accounts[0] }); const appproved = await this.token.getApproved(firstTokenId); accounts[1].should.be.equal(appproved); const transaction = await this.token.transferFromAndCall( - accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, extraData, { from: accounts[1] } + accounts[0], secondAvatarId, this.avatars.contract.address, firstTokenId, showMessageExtraData, { from: accounts[1] } ); assert.equal(3, transaction.receipt.logs.length); @@ -150,10 +139,8 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { new BigNumber(1).should.be.bignumber.equal(tokenBalance); }); - it('should fail inside approve (with data)', async function () { - const extraData = this.message.contract.fail.getData(); - - await this.token.approveAndCall(this.message.contract.address, firstTokenId, extraData) + it('should fail inside approve (with data)', async function () { + await this.token.approveAndCall(this.message.contract.address, firstTokenId, failExtraData) .should.be.rejectedWith('revert'); // approval should not have gone through so approved is still ZERO_ADDRESS @@ -162,10 +149,8 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { }); it('should fail inside transferFrom (with data)', async function () { - const extraData = this.message.contract.fail.getData(); - await this.token.approve(accounts[1], firstAvatarId, { from: creator }); - await this.token.transferFromAndCall(creator, secondAvatarId, this.avatars.address, firstTokenId, extraData, { from: creator }) + await this.token.transferFromAndCall(creator, secondAvatarId, this.avatars.address, firstTokenId, failExtraData, { from: creator }) .should.be.rejectedWith('revert'); // transferFrom should have failed so balance is still 0 but approved is accounts[1] @@ -177,24 +162,16 @@ export default function shouldExecuteCallsERC721HoldingsToken (accounts) { }); it('should fail approve (with data) when using token contract address as receiver', async function () { - const extraData = this.message.contract.showMessage.getData( - web3.toHex(123456), 666, 'Transfer Done' - ); - - await this.token.approveAndCall(this.token.contract.address, firstAvatarId, extraData, { from: accounts[0] }) + await this.token.approveAndCall(this.token.contract.address, firstAvatarId, showMessageExtraData, { from: accounts[0] }) .should.be.rejectedWith('revert'); }); it('should fail transferFrom (with data) when using token contract address as receiver', async function () { - const extraData = this.message.contract.showMessage.getData( - web3.toHex(123456), 666, 'Transfer Done' - ); - await this.token.approve(accounts[1], firstTokenId, { from: accounts[0] }); - await this.token.transferFromAndCall(accounts[0], thirdAvatarId, this.avatars.address, firstTokenId, extraData, { from: accounts[1] }) + await this.token.transferFromAndCall(accounts[0], thirdAvatarId, this.avatars.address, firstTokenId, showMessageExtraData, { from: accounts[1] }) .should.be.rejectedWith('revert'); }); }); }); -}; \ No newline at end of file +}; diff --git a/test/ERC721HoldingsToken.test.js b/test/ERC721HoldingsToken.test.js index cb2029a..6420932 100644 --- a/test/ERC721HoldingsToken.test.js +++ b/test/ERC721HoldingsToken.test.js @@ -3,8 +3,8 @@ import shouldMintAndBurnERC721HoldingsToken from './ERC721HoldingsMintBurn.behav import shouldExecuteCallsERC721HoldingsToken from './ERC721HoldingsExecuteCalls.behaviour'; const BigNumber = web3.BigNumber; -const ERC721BasicToken = artifacts.require('mocks/ERC721BasicTokenMock'); -const ERC721HoldingsToken = artifacts.require('mocks/ERC721HoldingsTokenMock.sol'); +const ERC721BasicToken = artifacts.require('test/ERC721BasicTokenMock'); +const ERC721HoldingsToken = artifacts.require('test/ERC721HoldingsTokenMock.sol'); require('chai') .use(require('chai-as-promised')) diff --git a/test/helpers/encodeCall.js b/test/helpers/encodeCall.js new file mode 100644 index 0000000..9705787 --- /dev/null +++ b/test/helpers/encodeCall.js @@ -0,0 +1,9 @@ +const abi = require('ethereumjs-abi') + +function encodeCall(name, args, values) { + const methodId = abi.methodID(name, args).toString('hex'); + const params = abi.rawEncode(args, values).toString('hex'); + return '0x' + methodId + params; +} + +module.exports = encodeCall;