From f2e5ed160cf7b4338b554b77b425a79675fe05e6 Mon Sep 17 00:00:00 2001 From: Aniket Date: Sun, 11 Mar 2018 02:50:54 +0530 Subject: [PATCH 01/15] signing prefix added --- contracts/ECRecovery.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol index 3876ca6e830..709ee43cb39 100644 --- a/contracts/ECRecovery.sol +++ b/contracts/ECRecovery.sol @@ -40,6 +40,13 @@ library ECRecovery { if (v != 27 && v != 28) { return (address(0)); } else { + + /* + * https://github.com/ethereum/go-ethereum/issues/3731 + */ + + bytes memory prefix = "\x19Ethereum Signed Message:\n32"; + hash = keccak256(prefix, hash); return ecrecover(hash, v, r, s); } } From 2bf341c178f8cc590f6791f7fc02db635ab57085 Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 15:24:23 +0530 Subject: [PATCH 02/15] Minor improvement --- contracts/ECRecovery.sol | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol index 709ee43cb39..c5b6d8f646c 100644 --- a/contracts/ECRecovery.sol +++ b/contracts/ECRecovery.sol @@ -41,13 +41,12 @@ library ECRecovery { return (address(0)); } else { - /* - * https://github.com/ethereum/go-ethereum/issues/3731 - */ + /* + * https://github.com/ethereum/go-ethereum/issues/3731 + */ - bytes memory prefix = "\x19Ethereum Signed Message:\n32"; - hash = keccak256(prefix, hash); - return ecrecover(hash, v, r, s); + bytes memory prefix = "\x19Ethereum Signed Message:\n32"; + return ecrecover(keccak256(prefix, hash), v, r, s); } } From bc7ea7ad913a9482fdb329b18b0151521ebd8f66 Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 15:50:25 +0530 Subject: [PATCH 03/15] Tests changed --- test/helpers/hashMessage.js | 2 +- test/library/ECRecovery.test.js | 38 ++++++++++++++++----------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/test/helpers/hashMessage.js b/test/helpers/hashMessage.js index 6e8a1f74e7f..08d965ccfc1 100644 --- a/test/helpers/hashMessage.js +++ b/test/helpers/hashMessage.js @@ -3,6 +3,6 @@ import utils from 'ethereumjs-util'; // Hash and add same prefix to the hash that testrpc use. module.exports = function (message) { const messageHex = Buffer.from(utils.sha3(message).toString('hex'), 'hex'); - const prefix = utils.toBuffer('\u0019Ethereum Signed Message:\n' + messageHex.length.toString()); + const prefix = utils.toBuffer(messageHex.length.toString()); return utils.bufferToHex(utils.sha3(Buffer.concat([prefix, messageHex]))); }; diff --git a/test/library/ECRecovery.test.js b/test/library/ECRecovery.test.js index 56852adf0af..405d5ff6782 100644 --- a/test/library/ECRecovery.test.js +++ b/test/library/ECRecovery.test.js @@ -13,25 +13,25 @@ contract('ECRecovery', function (accounts) { ecrecovery = await ECRecoveryMock.new(); }); - it('recover v0', async function () { - // Signature generated outside testrpc with method web3.eth.sign(signer, message) - let signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c'; - let message = web3.sha3(TEST_MESSAGE); - // eslint-disable-next-line max-len - let signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be89200'; - await ecrecovery.recover(message, signature); - assert.equal(signer, await ecrecovery.addrRecovered()); - }); - - it('recover v1', async function () { - // Signature generated outside testrpc with method web3.eth.sign(signer, message) - let signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e'; - let message = web3.sha3(TEST_MESSAGE); - // eslint-disable-next-line max-len - let signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e001'; - await ecrecovery.recover(message, signature); - assert.equal(signer, await ecrecovery.addrRecovered()); - }); + // it('recover v0', async function () { + // // Signature generated outside testrpc with method web3.eth.sign(signer, message) + // let signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c'; + // let message = web3.sha3(TEST_MESSAGE); + // // eslint-disable-next-line max-len + // let signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be89200'; + // await ecrecovery.recover(message, signature); + // assert.equal(signer, await ecrecovery.addrRecovered()); + // }); + + // it('recover v1', async function () { + // // Signature generated outside testrpc with method web3.eth.sign(signer, message) + // let signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e'; + // let message = web3.sha3(TEST_MESSAGE); + // // eslint-disable-next-line max-len + // let signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e001'; + // await ecrecovery.recover(message, signature); + // assert.equal(signer, await ecrecovery.addrRecovered()); + // }); it('recover using web3.eth.sign()', async function () { // Create the signature using account[0] From 5a720213d622650afc4c85b784c10d121615bc8d Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 16:30:25 +0530 Subject: [PATCH 04/15] Successfully tested --- test/helpers/hashMessage.js | 2 +- test/library/ECRecovery.test.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/helpers/hashMessage.js b/test/helpers/hashMessage.js index 08d965ccfc1..6e8a1f74e7f 100644 --- a/test/helpers/hashMessage.js +++ b/test/helpers/hashMessage.js @@ -3,6 +3,6 @@ import utils from 'ethereumjs-util'; // Hash and add same prefix to the hash that testrpc use. module.exports = function (message) { const messageHex = Buffer.from(utils.sha3(message).toString('hex'), 'hex'); - const prefix = utils.toBuffer(messageHex.length.toString()); + const prefix = utils.toBuffer('\u0019Ethereum Signed Message:\n' + messageHex.length.toString()); return utils.bufferToHex(utils.sha3(Buffer.concat([prefix, messageHex]))); }; diff --git a/test/library/ECRecovery.test.js b/test/library/ECRecovery.test.js index 405d5ff6782..fd99a0ae12f 100644 --- a/test/library/ECRecovery.test.js +++ b/test/library/ECRecovery.test.js @@ -38,7 +38,7 @@ contract('ECRecovery', function (accounts) { const signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); // Recover the signer address from the generated message and signature. - await ecrecovery.recover(hashMessage(TEST_MESSAGE), signature); + await ecrecovery.recover(web3.sha3(TEST_MESSAGE), signature); assert.equal(accounts[0], await ecrecovery.addrRecovered()); }); @@ -47,7 +47,7 @@ contract('ECRecovery', function (accounts) { const signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); // Recover the signer address from the generated message and wrong signature. - await ecrecovery.recover(hashMessage('Test'), signature); + await ecrecovery.recover(web3.sha3('Test'), signature); assert.notEqual(accounts[0], await ecrecovery.addrRecovered()); }); @@ -56,7 +56,7 @@ contract('ECRecovery', function (accounts) { let signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); // Recover the signer address from the generated message and wrong signature. - await ecrecovery.recover(hashMessage(TEST_MESSAGE).substring(2), signature); + await ecrecovery.recover(web3.sha3(TEST_MESSAGE).substring(2), signature); assert.equal('0x0000000000000000000000000000000000000000', await ecrecovery.addrRecovered()); }); }); From de0d2d46d0f544cd2a68bd56ea4237ca0dad2820 Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 17:12:10 +0530 Subject: [PATCH 05/15] Minor improvements --- test/library/ECRecovery.test.js | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/test/library/ECRecovery.test.js b/test/library/ECRecovery.test.js index fd99a0ae12f..1f9f41cd9ec 100644 --- a/test/library/ECRecovery.test.js +++ b/test/library/ECRecovery.test.js @@ -1,7 +1,6 @@ var ECRecoveryMock = artifacts.require('ECRecoveryMock'); var ECRecoveryLib = artifacts.require('ECRecovery'); -var hashMessage = require('../helpers/hashMessage.js'); contract('ECRecovery', function (accounts) { let ecrecovery; @@ -13,26 +12,6 @@ contract('ECRecovery', function (accounts) { ecrecovery = await ECRecoveryMock.new(); }); - // it('recover v0', async function () { - // // Signature generated outside testrpc with method web3.eth.sign(signer, message) - // let signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c'; - // let message = web3.sha3(TEST_MESSAGE); - // // eslint-disable-next-line max-len - // let signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be89200'; - // await ecrecovery.recover(message, signature); - // assert.equal(signer, await ecrecovery.addrRecovered()); - // }); - - // it('recover v1', async function () { - // // Signature generated outside testrpc with method web3.eth.sign(signer, message) - // let signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e'; - // let message = web3.sha3(TEST_MESSAGE); - // // eslint-disable-next-line max-len - // let signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e001'; - // await ecrecovery.recover(message, signature); - // assert.equal(signer, await ecrecovery.addrRecovered()); - // }); - it('recover using web3.eth.sign()', async function () { // Create the signature using account[0] const signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); From 7f32e5ceaa62a458fd68591fe895e4d43685411d Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 17:17:29 +0530 Subject: [PATCH 06/15] Minor improvements --- test/library/ECRecovery.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/library/ECRecovery.test.js b/test/library/ECRecovery.test.js index 1f9f41cd9ec..ef354a76f05 100644 --- a/test/library/ECRecovery.test.js +++ b/test/library/ECRecovery.test.js @@ -1,7 +1,6 @@ var ECRecoveryMock = artifacts.require('ECRecoveryMock'); var ECRecoveryLib = artifacts.require('ECRecovery'); - contract('ECRecovery', function (accounts) { let ecrecovery; const TEST_MESSAGE = 'OpenZeppelin'; From 68de8409c2599a2617a78b2c856072e4d85a5881 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Mon, 1 Oct 2018 15:49:33 +0530 Subject: [PATCH 07/15] Revert "Dangling commas are now required. (#1359)" This reverts commit a6889776f46adca374b6ebf014aa7b0038112a9d. --- .eslintrc | 2 +- test/crowdsale/AllowanceCrowdsale.test.js | 2 +- test/crowdsale/Crowdsale.test.js | 4 ++-- test/crowdsale/MintedCrowdsale.behavior.js | 2 +- test/payment/Escrow.behavior.js | 4 ++-- test/token/ERC20/ERC20.test.js | 14 +++++++------- .../ERC20/behaviors/ERC20Burnable.behavior.js | 4 ++-- .../ERC20/behaviors/ERC20Mintable.behavior.js | 2 +- test/token/ERC721/ERC721.behavior.js | 14 +++++++------- test/token/ERC721/ERC721MintBurn.behavior.js | 4 ++-- 10 files changed, 26 insertions(+), 26 deletions(-) diff --git a/.eslintrc b/.eslintrc index c15a4d51515..117135b36f9 100644 --- a/.eslintrc +++ b/.eslintrc @@ -25,7 +25,7 @@ // Code style "camelcase": ["error", {"properties": "always"}], - "comma-dangle": ["error", "always-multiline"], + "comma-dangle": ["warn", "always-multiline"], "comma-spacing": ["error", {"before": false, "after": true}], "dot-notation": ["error", {"allowKeywords": true, "allowPattern": ""}], "eol-last": ["error", "always"], diff --git a/test/crowdsale/AllowanceCrowdsale.test.js b/test/crowdsale/AllowanceCrowdsale.test.js index c0ff4cd91ba..8e57a841b24 100644 --- a/test/crowdsale/AllowanceCrowdsale.test.js +++ b/test/crowdsale/AllowanceCrowdsale.test.js @@ -46,7 +46,7 @@ contract('AllowanceCrowdsale', function ([_, investor, wallet, purchaser, tokenW purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount, + amount: expectedTokenAmount }); }); diff --git a/test/crowdsale/Crowdsale.test.js b/test/crowdsale/Crowdsale.test.js index 4113f7cb428..f691c951576 100644 --- a/test/crowdsale/Crowdsale.test.js +++ b/test/crowdsale/Crowdsale.test.js @@ -87,7 +87,7 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount, + amount: expectedTokenAmount }); }); @@ -111,7 +111,7 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { purchaser: purchaser, beneficiary: investor, value: value, - amount: expectedTokenAmount, + amount: expectedTokenAmount }); }); diff --git a/test/crowdsale/MintedCrowdsale.behavior.js b/test/crowdsale/MintedCrowdsale.behavior.js index ab8dc8259cc..c55850cd9a4 100644 --- a/test/crowdsale/MintedCrowdsale.behavior.js +++ b/test/crowdsale/MintedCrowdsale.behavior.js @@ -25,7 +25,7 @@ function shouldBehaveLikeMintedCrowdsale ([_, investor, wallet, purchaser], rate purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount, + amount: expectedTokenAmount }); }); diff --git a/test/payment/Escrow.behavior.js b/test/payment/Escrow.behavior.js index 57f1fe5d0ac..fce4bd69217 100644 --- a/test/payment/Escrow.behavior.js +++ b/test/payment/Escrow.behavior.js @@ -34,7 +34,7 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { const { logs } = await this.escrow.deposit(payee1, { from: primary, value: amount }); expectEvent.inLogs(logs, 'Deposited', { payee: payee1, - weiAmount: amount, + weiAmount: amount }); }); @@ -87,7 +87,7 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { const { logs } = await this.escrow.withdraw(payee1, { from: primary }); expectEvent.inLogs(logs, 'Withdrawn', { payee: payee1, - weiAmount: amount, + weiAmount: amount }); }); }); diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index e991c2ae6d1..283e8752a09 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -65,7 +65,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount, + value: amount }); }); }); @@ -93,7 +93,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount, + value: amount }); }); @@ -127,7 +127,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount, + value: amount }); }); @@ -197,7 +197,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount, + value: amount }); }); }); @@ -272,7 +272,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: 0, + value: 0 }); }); @@ -329,7 +329,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount, + value: amount }); }); @@ -363,7 +363,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount, + value: amount }); }); diff --git a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js index c52594d1876..9c7bada7acc 100644 --- a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js @@ -32,7 +32,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - value: amount, + value: amount }); }); } @@ -78,7 +78,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - value: amount, + value: amount }); }); } diff --git a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js index dbda9eeedbc..07b3837b086 100644 --- a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js @@ -38,7 +38,7 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) { expectEvent.inLogs(this.logs, 'Transfer', { from: ZERO_ADDRESS, to: anyone, - value: amount, + value: amount }); }); } diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js index ce56747def3..d2cefe743c7 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -92,7 +92,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId, + tokenId: tokenId }); }); } else { @@ -100,7 +100,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId, + tokenId: tokenId }); }); } @@ -165,7 +165,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: owner, - tokenId: tokenId, + tokenId: tokenId }); }); @@ -341,7 +341,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Approval', { owner: owner, approved: address, - tokenId: tokenId, + tokenId: tokenId }); }); }; @@ -451,7 +451,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true, + approved: true }); }); }); @@ -473,7 +473,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true, + approved: true }); }); @@ -501,7 +501,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true, + approved: true }); }); }); diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index 8aeccd5d8b7..a4289d27a74 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -45,7 +45,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: ZERO_ADDRESS, to: newOwner, - tokenId: thirdTokenId, + tokenId: thirdTokenId }); }); }); @@ -90,7 +90,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - tokenId: tokenId, + tokenId: tokenId }); }); }); From 75bc3107fcae9c4aa9d37b3afc28192d2d1d493c Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Mon, 1 Oct 2018 22:00:42 +0530 Subject: [PATCH 08/15] updates --- .eslintrc | 2 +- contracts/ECRecovery.sol | 53 --------------------------------- test/library/ECRecovery.test.js | 40 ------------------------- 3 files changed, 1 insertion(+), 94 deletions(-) delete mode 100644 contracts/ECRecovery.sol delete mode 100644 test/library/ECRecovery.test.js diff --git a/.eslintrc b/.eslintrc index 117135b36f9..c15a4d51515 100644 --- a/.eslintrc +++ b/.eslintrc @@ -25,7 +25,7 @@ // Code style "camelcase": ["error", {"properties": "always"}], - "comma-dangle": ["warn", "always-multiline"], + "comma-dangle": ["error", "always-multiline"], "comma-spacing": ["error", {"before": false, "after": true}], "dot-notation": ["error", {"allowKeywords": true, "allowPattern": ""}], "eol-last": ["error", "always"], diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol deleted file mode 100644 index c5b6d8f646c..00000000000 --- a/contracts/ECRecovery.sol +++ /dev/null @@ -1,53 +0,0 @@ -pragma solidity ^0.4.18; - - -/** - * @title Eliptic curve signature operations - * - * @dev Based on https://gist.github.com/axic/5b33912c6f61ae6fd96d6c4a47afde6d - */ - -library ECRecovery { - - /** - * @dev Recover signer address from a message by using his signature - * @param hash bytes32 message, the hash is the signed message. What is recovered is the signer address. - * @param sig bytes signature, the signature is generated using web3.eth.sign() - */ - function recover(bytes32 hash, bytes sig) public pure returns (address) { - bytes32 r; - bytes32 s; - uint8 v; - - //Check the signature length - if (sig.length != 65) { - return (address(0)); - } - - // Divide the signature in r, s and v variables - assembly { - r := mload(add(sig, 32)) - s := mload(add(sig, 64)) - v := byte(0, mload(add(sig, 96))) - } - - // Version of signature should be 27 or 28, but 0 and 1 are also possible versions - if (v < 27) { - v += 27; - } - - // If the version is correct return the signer address - if (v != 27 && v != 28) { - return (address(0)); - } else { - - /* - * https://github.com/ethereum/go-ethereum/issues/3731 - */ - - bytes memory prefix = "\x19Ethereum Signed Message:\n32"; - return ecrecover(keccak256(prefix, hash), v, r, s); - } - } - -} diff --git a/test/library/ECRecovery.test.js b/test/library/ECRecovery.test.js deleted file mode 100644 index ef354a76f05..00000000000 --- a/test/library/ECRecovery.test.js +++ /dev/null @@ -1,40 +0,0 @@ -var ECRecoveryMock = artifacts.require('ECRecoveryMock'); -var ECRecoveryLib = artifacts.require('ECRecovery'); - -contract('ECRecovery', function (accounts) { - let ecrecovery; - const TEST_MESSAGE = 'OpenZeppelin'; - - before(async function () { - const ecRecoveryLib = await ECRecoveryLib.new(); - ECRecoveryMock.link('ECRecovery', ecRecoveryLib.address); - ecrecovery = await ECRecoveryMock.new(); - }); - - it('recover using web3.eth.sign()', async function () { - // Create the signature using account[0] - const signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); - - // Recover the signer address from the generated message and signature. - await ecrecovery.recover(web3.sha3(TEST_MESSAGE), signature); - assert.equal(accounts[0], await ecrecovery.addrRecovered()); - }); - - it('recover using web3.eth.sign() should return wrong signer', async function () { - // Create the signature using account[0] - const signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); - - // Recover the signer address from the generated message and wrong signature. - await ecrecovery.recover(web3.sha3('Test'), signature); - assert.notEqual(accounts[0], await ecrecovery.addrRecovered()); - }); - - it('recover should fail when a wrong hash is sent', async function () { - // Create the signature using account[0] - let signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); - - // Recover the signer address from the generated message and wrong signature. - await ecrecovery.recover(web3.sha3(TEST_MESSAGE).substring(2), signature); - assert.equal('0x0000000000000000000000000000000000000000', await ecrecovery.addrRecovered()); - }); -}); From 23df893e0caa0fc4ed12d95f35d12ff139b0e554 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Thu, 11 Oct 2018 14:18:33 +0530 Subject: [PATCH 09/15] fixes #1404 --- contracts/mocks/SafeERC20Helper.sol | 32 +++++++++++++++++++ contracts/token/ERC20/IERC20.sol | 6 ++++ contracts/token/ERC20/SafeERC20.sol | 21 ++++++++++++ test/crowdsale/AllowanceCrowdsale.test.js | 2 +- test/crowdsale/Crowdsale.test.js | 4 +-- test/crowdsale/MintedCrowdsale.behavior.js | 2 +- test/payment/Escrow.behavior.js | 4 +-- test/token/ERC20/ERC20.test.js | 14 ++++---- test/token/ERC20/SafeERC20.test.js | 16 ++++++++++ .../ERC20/behaviors/ERC20Burnable.behavior.js | 4 +-- .../ERC20/behaviors/ERC20Mintable.behavior.js | 2 +- test/token/ERC721/ERC721.behavior.js | 14 ++++---- test/token/ERC721/ERC721MintBurn.behavior.js | 4 +-- 13 files changed, 100 insertions(+), 25 deletions(-) diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index c80c762d92f..7206e72d261 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -20,6 +20,14 @@ contract ERC20FailingMock is IERC20 { return false; } + function increaseAllowance(address, uint256) public returns (bool){ + return false; + } + + function decreaseAllowance(address, uint256) public returns (bool){ + return false; + } + function balanceOf(address) public view returns (uint256) { return 0; } @@ -46,6 +54,14 @@ contract ERC20SucceedingMock is IERC20 { return true; } + function increaseAllowance(address, uint256) public returns (bool){ + return true; + } + + function decreaseAllowance(address, uint256) public returns (bool){ + return true; + } + function balanceOf(address) public view returns (uint256) { return 0; } @@ -78,6 +94,14 @@ contract SafeERC20Helper { _failing.safeApprove(address(0), 0); } + function doFailingIncreaseAllowance() public { + _failing.safeIncreaseAllowance(address(0), 0); + } + + function doFailingDecreaseAllowance() public { + _failing.safeDecreaseAllowance(address(0), 0); + } + function doSucceedingTransfer() public { _succeeding.safeTransfer(address(0), 0); } @@ -89,4 +113,12 @@ contract SafeERC20Helper { function doSucceedingApprove() public { _succeeding.safeApprove(address(0), 0); } + + function doSucceedingIncreaseAllowance() public { + _succeeding.safeIncreaseAllowance(address(0), 0); + } + + function doSucceedingDecreaseAllowance() public { + _succeeding.safeDecreaseAllowance(address(0), 0); + } } diff --git a/contracts/token/ERC20/IERC20.sol b/contracts/token/ERC20/IERC20.sol index 37e5a447c06..fb56b2a14ef 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -20,6 +20,12 @@ interface IERC20 { function transferFrom(address from, address to, uint256 value) external returns (bool); + function increaseAllowance(address spender, uint256 addedValue) + external returns (bool); + + function decreaseAllowance(address spender, uint256 subtractedValue) + external returns (bool); + event Transfer( address indexed from, address indexed to, diff --git a/contracts/token/ERC20/SafeERC20.sol b/contracts/token/ERC20/SafeERC20.sol index 10c6ff2bcf6..58481b227ce 100644 --- a/contracts/token/ERC20/SafeERC20.sol +++ b/contracts/token/ERC20/SafeERC20.sol @@ -38,6 +38,27 @@ library SafeERC20 { ) internal { + require((value == 0) || (token.allowance(msg.sender, spender) == 0)); require(token.approve(spender, value)); } + + function safeIncreaseAllowance( + IERC20 token, + address spender, + uint256 addedValue + ) + internal + { + require(token.increaseAllowance(spender, addedValue)); + } + + function safeDecreaseAllowance( + IERC20 token, + address spender, + uint256 subtractedValue + ) + internal + { + require(token.decreaseAllowance(spender, subtractedValue)); + } } diff --git a/test/crowdsale/AllowanceCrowdsale.test.js b/test/crowdsale/AllowanceCrowdsale.test.js index d2d0ab93e78..4298dcce61a 100644 --- a/test/crowdsale/AllowanceCrowdsale.test.js +++ b/test/crowdsale/AllowanceCrowdsale.test.js @@ -46,7 +46,7 @@ contract('AllowanceCrowdsale', function ([_, investor, wallet, purchaser, tokenW purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); diff --git a/test/crowdsale/Crowdsale.test.js b/test/crowdsale/Crowdsale.test.js index 7d6bfe815b9..8875e91576e 100644 --- a/test/crowdsale/Crowdsale.test.js +++ b/test/crowdsale/Crowdsale.test.js @@ -87,7 +87,7 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); @@ -111,7 +111,7 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { purchaser: purchaser, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); diff --git a/test/crowdsale/MintedCrowdsale.behavior.js b/test/crowdsale/MintedCrowdsale.behavior.js index c55850cd9a4..ab8dc8259cc 100644 --- a/test/crowdsale/MintedCrowdsale.behavior.js +++ b/test/crowdsale/MintedCrowdsale.behavior.js @@ -25,7 +25,7 @@ function shouldBehaveLikeMintedCrowdsale ([_, investor, wallet, purchaser], rate purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); diff --git a/test/payment/Escrow.behavior.js b/test/payment/Escrow.behavior.js index 40c666b2dc2..c724c5395d9 100644 --- a/test/payment/Escrow.behavior.js +++ b/test/payment/Escrow.behavior.js @@ -34,7 +34,7 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { const { logs } = await this.escrow.deposit(payee1, { from: primary, value: amount }); expectEvent.inLogs(logs, 'Deposited', { payee: payee1, - weiAmount: amount + weiAmount: amount, }); }); @@ -87,7 +87,7 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { const { logs } = await this.escrow.withdraw(payee1, { from: primary }); expectEvent.inLogs(logs, 'Withdrawn', { payee: payee1, - weiAmount: amount + weiAmount: amount, }); }); }); diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index 49753f477d2..d60e9130cc1 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -64,7 +64,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount + value: amount, }); }); }); @@ -92,7 +92,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -126,7 +126,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -196,7 +196,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount + value: amount, }); }); }); @@ -271,7 +271,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: 0 + value: 0, }); }); @@ -328,7 +328,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -362,7 +362,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); diff --git a/test/token/ERC20/SafeERC20.test.js b/test/token/ERC20/SafeERC20.test.js index b6c87554b41..cf71190db67 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -22,6 +22,14 @@ contract('SafeERC20', function () { await shouldFail.reverting(this.helper.doFailingApprove()); }); + it('should throw on failed increaseAllowance', async function () { + await shouldFail.reverting(this.helper.doFailingIncreaseAllowance()); + }); + + it('should throw on failed decreaseAllowance', async function () { + await shouldFail.reverting(this.helper.doFailingDecreaseAllowance()); + }); + it('should not throw on succeeding transfer', async function () { await this.helper.doSucceedingTransfer(); }); @@ -33,4 +41,12 @@ contract('SafeERC20', function () { it('should not throw on succeeding approve', async function () { await this.helper.doSucceedingApprove(); }); + + it('should not throw on succeeding increaseAllowance', async function () { + await this.helper.doSucceedingIncreaseAllowance(); + }); + + it('should not throw on succeeding decreaseAllowance', async function () { + await this.helper.doSucceedingDecreaseAllowance(); + }); }); diff --git a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js index 8382e123fe0..6e550afcbf8 100644 --- a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js @@ -32,7 +32,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - value: amount + value: amount, }); }); } @@ -78,7 +78,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - value: amount + value: amount, }); }); } diff --git a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js index 79764f85a03..bb8c0fe3073 100644 --- a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js @@ -37,7 +37,7 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) { expectEvent.inLogs(this.logs, 'Transfer', { from: ZERO_ADDRESS, to: anyone, - value: amount + value: amount, }); }); } diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js index 967b20fb4c3..aea385a9968 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -92,7 +92,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId + tokenId: tokenId, }); }); } else { @@ -100,7 +100,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId + tokenId: tokenId, }); }); } @@ -165,7 +165,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: owner, - tokenId: tokenId + tokenId: tokenId, }); }); @@ -347,7 +347,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Approval', { owner: owner, approved: address, - tokenId: tokenId + tokenId: tokenId, }); }); }; @@ -457,7 +457,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true + approved: true, }); }); }); @@ -479,7 +479,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true + approved: true, }); }); @@ -507,7 +507,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true + approved: true, }); }); }); diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index 62485a044c0..1bb7d1bc7d7 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -45,7 +45,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: ZERO_ADDRESS, to: newOwner, - tokenId: thirdTokenId + tokenId: thirdTokenId, }); }); }); @@ -90,7 +90,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - tokenId: tokenId + tokenId: tokenId, }); }); }); From d8dd5ecbcfc87b7097ce97a4736d041348d516c6 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Thu, 11 Oct 2018 16:07:39 +0530 Subject: [PATCH 10/15] approve failing test --- contracts/mocks/SafeERC20Helper.sol | 6 +++++- test/token/ERC20/SafeERC20.test.js | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 7206e72d261..54cf0d08c82 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -67,7 +67,7 @@ contract ERC20SucceedingMock is IERC20 { } function allowance(address, address) public view returns (uint256) { - return 0; + return 10; //non-zero allowance } } @@ -114,6 +114,10 @@ contract SafeERC20Helper { _succeeding.safeApprove(address(0), 0); } + function doFailingApproveByValue() public { + _succeeding.safeApprove(address(0), 10); + } + function doSucceedingIncreaseAllowance() public { _succeeding.safeIncreaseAllowance(address(0), 0); } diff --git a/test/token/ERC20/SafeERC20.test.js b/test/token/ERC20/SafeERC20.test.js index cf71190db67..9d5fc59fa9c 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -42,6 +42,10 @@ contract('SafeERC20', function () { await this.helper.doSucceedingApprove(); }); + it('should throw while approving with non-zero existing allowance', async function () { + await shouldFail.reverting(this.helper.doFailingApproveByValue()); + }); + it('should not throw on succeeding increaseAllowance', async function () { await this.helper.doSucceedingIncreaseAllowance(); }); From ae96caa233b8bb5f65b4b9102974d1dfd0ab9f96 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Tue, 16 Oct 2018 12:37:47 +0530 Subject: [PATCH 11/15] suggested changes done --- contracts/mocks/SafeERC20Helper.sol | 26 ++++++++++++----- contracts/token/ERC20/IERC20.sol | 6 ---- contracts/token/ERC20/ISafeERC20.sol | 43 ++++++++++++++++++++++++++++ contracts/token/ERC20/SafeERC20.sol | 15 ++++++---- 4 files changed, 71 insertions(+), 19 deletions(-) create mode 100644 contracts/token/ERC20/ISafeERC20.sol diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 54cf0d08c82..4c5a1e1de8f 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -1,9 +1,10 @@ pragma solidity ^0.4.24; -import "../token/ERC20/IERC20.sol"; +import "../token/ERC20/ISafeERC20.sol"; import "../token/ERC20/SafeERC20.sol"; -contract ERC20FailingMock is IERC20 { +contract ERC20FailingMock is ISafeERC20 { + uint256 private _allowance; function totalSupply() public view returns (uint256) { return 0; } @@ -35,9 +36,15 @@ contract ERC20FailingMock is IERC20 { function allowance(address, address) public view returns (uint256) { return 0; } + + function setAllowance(uint256 value) public { + _allowance = value; + } } -contract ERC20SucceedingMock is IERC20 { +contract ERC20SucceedingMock is ISafeERC20 { + uint256 private _allowance; + function totalSupply() public view returns (uint256) { return 0; } @@ -67,15 +74,19 @@ contract ERC20SucceedingMock is IERC20 { } function allowance(address, address) public view returns (uint256) { - return 10; //non-zero allowance + return _allowance; + } + + function setAllowance(uint256 value) public { + _allowance = value; } } contract SafeERC20Helper { - using SafeERC20 for IERC20; + using SafeERC20 for ISafeERC20; - IERC20 private _failing; - IERC20 private _succeeding; + ISafeERC20 private _failing; + ISafeERC20 private _succeeding; constructor() public { _failing = new ERC20FailingMock(); @@ -115,6 +126,7 @@ contract SafeERC20Helper { } function doFailingApproveByValue() public { + _succeeding.setAllowance(10); _succeeding.safeApprove(address(0), 10); } diff --git a/contracts/token/ERC20/IERC20.sol b/contracts/token/ERC20/IERC20.sol index fb56b2a14ef..37e5a447c06 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -20,12 +20,6 @@ interface IERC20 { function transferFrom(address from, address to, uint256 value) external returns (bool); - function increaseAllowance(address spender, uint256 addedValue) - external returns (bool); - - function decreaseAllowance(address spender, uint256 subtractedValue) - external returns (bool); - event Transfer( address indexed from, address indexed to, diff --git a/contracts/token/ERC20/ISafeERC20.sol b/contracts/token/ERC20/ISafeERC20.sol new file mode 100644 index 00000000000..b04e010df54 --- /dev/null +++ b/contracts/token/ERC20/ISafeERC20.sol @@ -0,0 +1,43 @@ +pragma solidity ^0.4.24; + +/** + * @title SafeERC20 interface + * @dev ERC20 operations with allowance operations. For ERC20, see https://github.com/ethereum/EIPs/issues/20 + */ +interface ISafeERC20 { + function totalSupply() external view returns (uint256); + + function balanceOf(address who) external view returns (uint256); + + function allowance(address owner, address spender) + external view returns (uint256); + + function transfer(address to, uint256 value) external returns (bool); + + function approve(address spender, uint256 value) + external returns (bool); + + function transferFrom(address from, address to, uint256 value) + external returns (bool); + + function increaseAllowance(address spender, uint256 addedValue) + external returns (bool); + + function decreaseAllowance(address spender, uint256 subtractedValue) + external returns (bool); + + function setAllowance(uint256 value) + external returns (bool); + + event Transfer( + address indexed from, + address indexed to, + uint256 value + ); + + event Approval( + address indexed owner, + address indexed spender, + uint256 value + ); +} diff --git a/contracts/token/ERC20/SafeERC20.sol b/contracts/token/ERC20/SafeERC20.sol index 58481b227ce..1b91e823676 100644 --- a/contracts/token/ERC20/SafeERC20.sol +++ b/contracts/token/ERC20/SafeERC20.sol @@ -1,7 +1,7 @@ pragma solidity ^0.4.24; import "./ERC20.sol"; -import "./IERC20.sol"; +import "./ISafeERC20.sol"; /** * @title SafeERC20 @@ -11,7 +11,7 @@ import "./IERC20.sol"; */ library SafeERC20 { function safeTransfer( - IERC20 token, + ISafeERC20 token, address to, uint256 value ) @@ -21,7 +21,7 @@ library SafeERC20 { } function safeTransferFrom( - IERC20 token, + ISafeERC20 token, address from, address to, uint256 value @@ -32,18 +32,21 @@ library SafeERC20 { } function safeApprove( - IERC20 token, + ISafeERC20 token, address spender, uint256 value ) internal { + // safeApprove should only be called when setting an initial allowance, + // or when resetting it to zero. To increase and decrease it, use + // 'safeIncreaseAllowance' and 'safeDecreaseAllowance' require((value == 0) || (token.allowance(msg.sender, spender) == 0)); require(token.approve(spender, value)); } function safeIncreaseAllowance( - IERC20 token, + ISafeERC20 token, address spender, uint256 addedValue ) @@ -53,7 +56,7 @@ library SafeERC20 { } function safeDecreaseAllowance( - IERC20 token, + ISafeERC20 token, address spender, uint256 subtractedValue ) From e2fb20ad1f108f8d99805e4973ccf622a6addc75 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Tue, 16 Oct 2018 12:52:00 +0530 Subject: [PATCH 12/15] ISafeERC20 removed --- contracts/mocks/SafeERC20Helper.sol | 12 ++++---- contracts/token/ERC20/IERC20.sol | 9 ++++++ contracts/token/ERC20/ISafeERC20.sol | 43 ---------------------------- contracts/token/ERC20/SafeERC20.sol | 12 ++++---- 4 files changed, 21 insertions(+), 55 deletions(-) delete mode 100644 contracts/token/ERC20/ISafeERC20.sol diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 4c5a1e1de8f..ac2170e5253 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -1,9 +1,9 @@ pragma solidity ^0.4.24; -import "../token/ERC20/ISafeERC20.sol"; +import "../token/ERC20/IERC20.sol"; import "../token/ERC20/SafeERC20.sol"; -contract ERC20FailingMock is ISafeERC20 { +contract ERC20FailingMock is IERC20 { uint256 private _allowance; function totalSupply() public view returns (uint256) { return 0; @@ -42,7 +42,7 @@ contract ERC20FailingMock is ISafeERC20 { } } -contract ERC20SucceedingMock is ISafeERC20 { +contract ERC20SucceedingMock is IERC20 { uint256 private _allowance; function totalSupply() public view returns (uint256) { @@ -83,10 +83,10 @@ contract ERC20SucceedingMock is ISafeERC20 { } contract SafeERC20Helper { - using SafeERC20 for ISafeERC20; + using SafeERC20 for IERC20; - ISafeERC20 private _failing; - ISafeERC20 private _succeeding; + IERC20 private _failing; + IERC20 private _succeeding; constructor() public { _failing = new ERC20FailingMock(); diff --git a/contracts/token/ERC20/IERC20.sol b/contracts/token/ERC20/IERC20.sol index 37e5a447c06..9da28af9746 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -20,6 +20,15 @@ interface IERC20 { function transferFrom(address from, address to, uint256 value) external returns (bool); + function increaseAllowance(address spender, uint256 addedValue) + external returns (bool); + + function decreaseAllowance(address spender, uint256 subtractedValue) + external returns (bool); + + function setAllowance(uint256 value) + external returns (bool); + event Transfer( address indexed from, address indexed to, diff --git a/contracts/token/ERC20/ISafeERC20.sol b/contracts/token/ERC20/ISafeERC20.sol deleted file mode 100644 index b04e010df54..00000000000 --- a/contracts/token/ERC20/ISafeERC20.sol +++ /dev/null @@ -1,43 +0,0 @@ -pragma solidity ^0.4.24; - -/** - * @title SafeERC20 interface - * @dev ERC20 operations with allowance operations. For ERC20, see https://github.com/ethereum/EIPs/issues/20 - */ -interface ISafeERC20 { - function totalSupply() external view returns (uint256); - - function balanceOf(address who) external view returns (uint256); - - function allowance(address owner, address spender) - external view returns (uint256); - - function transfer(address to, uint256 value) external returns (bool); - - function approve(address spender, uint256 value) - external returns (bool); - - function transferFrom(address from, address to, uint256 value) - external returns (bool); - - function increaseAllowance(address spender, uint256 addedValue) - external returns (bool); - - function decreaseAllowance(address spender, uint256 subtractedValue) - external returns (bool); - - function setAllowance(uint256 value) - external returns (bool); - - event Transfer( - address indexed from, - address indexed to, - uint256 value - ); - - event Approval( - address indexed owner, - address indexed spender, - uint256 value - ); -} diff --git a/contracts/token/ERC20/SafeERC20.sol b/contracts/token/ERC20/SafeERC20.sol index 1b91e823676..0c846d0a02e 100644 --- a/contracts/token/ERC20/SafeERC20.sol +++ b/contracts/token/ERC20/SafeERC20.sol @@ -1,7 +1,7 @@ pragma solidity ^0.4.24; import "./ERC20.sol"; -import "./ISafeERC20.sol"; +import "./IERC20.sol"; /** * @title SafeERC20 @@ -11,7 +11,7 @@ import "./ISafeERC20.sol"; */ library SafeERC20 { function safeTransfer( - ISafeERC20 token, + IERC20 token, address to, uint256 value ) @@ -21,7 +21,7 @@ library SafeERC20 { } function safeTransferFrom( - ISafeERC20 token, + IERC20 token, address from, address to, uint256 value @@ -32,7 +32,7 @@ library SafeERC20 { } function safeApprove( - ISafeERC20 token, + IERC20 token, address spender, uint256 value ) @@ -46,7 +46,7 @@ library SafeERC20 { } function safeIncreaseAllowance( - ISafeERC20 token, + IERC20 token, address spender, uint256 addedValue ) @@ -56,7 +56,7 @@ library SafeERC20 { } function safeDecreaseAllowance( - ISafeERC20 token, + IERC20 token, address spender, uint256 subtractedValue ) From 5f888452b5071e8b0d43e6a7910318d2ace01270 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Mon, 22 Oct 2018 13:21:32 +0530 Subject: [PATCH 13/15] conflict fixes --- contracts/mocks/SafeERC20Helper.sol | 17 ----------------- contracts/token/ERC20/IERC20.sol | 9 --------- test/token/ERC20/SafeERC20.test.js | 12 ------------ 3 files changed, 38 deletions(-) diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index a8baf7c4527..7e639f86f2e 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -21,10 +21,6 @@ contract ERC20FailingMock { function allowance(address, address) public view returns (uint256) { return 0; } - - function setAllowance(uint256 value) public { - _allowance = value; - } } contract ERC20SucceedingMock { @@ -113,17 +109,4 @@ contract SafeERC20Helper { function allowance() public view returns (uint256) { return _succeeding.allowance(address(0), address(0)); } - - function doFailingApproveByValue() public { - _succeeding.setAllowance(10); - _succeeding.safeApprove(address(0), 10); - } - - function doSucceedingIncreaseAllowance() public { - _succeeding.safeIncreaseAllowance(address(0), 0); - } - - function doSucceedingDecreaseAllowance() public { - _succeeding.safeDecreaseAllowance(address(0), 0); - } } diff --git a/contracts/token/ERC20/IERC20.sol b/contracts/token/ERC20/IERC20.sol index 9da28af9746..37e5a447c06 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -20,15 +20,6 @@ interface IERC20 { function transferFrom(address from, address to, uint256 value) external returns (bool); - function increaseAllowance(address spender, uint256 addedValue) - external returns (bool); - - function decreaseAllowance(address spender, uint256 subtractedValue) - external returns (bool); - - function setAllowance(uint256 value) - external returns (bool); - event Transfer( address indexed from, address indexed to, diff --git a/test/token/ERC20/SafeERC20.test.js b/test/token/ERC20/SafeERC20.test.js index 38ef4d7f271..c8dea73822b 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -91,16 +91,4 @@ contract('SafeERC20', function () { }); }); }); - - it('should throw while approving with non-zero existing allowance', async function () { - await shouldFail.reverting(this.helper.doFailingApproveByValue()); - }); - - it('should not throw on succeeding increaseAllowance', async function () { - await this.helper.doSucceedingIncreaseAllowance(); - }); - - it('should not throw on succeeding decreaseAllowance', async function () { - await this.helper.doSucceedingDecreaseAllowance(); - }); }); From ac8c493297302e9fe38d64289a98a5de23695bb7 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Wed, 28 Nov 2018 12:08:29 +0530 Subject: [PATCH 14/15] fixes #1512 --- contracts/mocks/ERC721FullMock.sol | 8 ++++++-- contracts/token/ERC721/ERC721Enumerable.sol | 9 +++++++++ test/token/ERC721/ERC721Full.test.js | 8 ++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/contracts/mocks/ERC721FullMock.sol b/contracts/mocks/ERC721FullMock.sol index 5199ef41753..66999755759 100644 --- a/contracts/mocks/ERC721FullMock.sol +++ b/contracts/mocks/ERC721FullMock.sol @@ -7,8 +7,8 @@ import "../token/ERC721/ERC721Burnable.sol"; /** * @title ERC721FullMock - * This mock just provides a public mint and burn functions for testing purposes, - * and a public setter for metadata URI + * This mock just provides public functions for setting metadata URI, getting all tokens of an owner, + * checking token existence, removal of a token from an address */ contract ERC721FullMock is ERC721Full, ERC721Mintable, ERC721MetadataMintable, ERC721Burnable { constructor (string name, string symbol) public ERC721Mintable() ERC721Full(name, symbol) {} @@ -17,6 +17,10 @@ contract ERC721FullMock is ERC721Full, ERC721Mintable, ERC721MetadataMintable, E return _exists(tokenId); } + function tokensOfOwner(address owner) public view returns (uint256[] memory) { + return _tokensOfOwner(owner); + } + function setTokenURI(uint256 tokenId, string uri) public { _setTokenURI(tokenId, uri); } diff --git a/contracts/token/ERC721/ERC721Enumerable.sol b/contracts/token/ERC721/ERC721Enumerable.sol index ff8c1b9eecc..95622b75917 100644 --- a/contracts/token/ERC721/ERC721Enumerable.sol +++ b/contracts/token/ERC721/ERC721Enumerable.sol @@ -144,4 +144,13 @@ contract ERC721Enumerable is ERC165, ERC721, IERC721Enumerable { _allTokensIndex[tokenId] = 0; _allTokensIndex[lastToken] = tokenIndex; } + + /** + * @dev Gets the list of token IDs of the requested owner + * @param owner address owning the tokens + * @return uint256[] List of token IDs owned by the requested address + */ + function _tokensOfOwner(address owner) internal view returns (uint256[] storage) { + return _ownedTokens[owner]; + } } diff --git a/test/token/ERC721/ERC721Full.test.js b/test/token/ERC721/ERC721Full.test.js index 6789ca28aea..e06676d45b1 100644 --- a/test/token/ERC721/ERC721Full.test.js +++ b/test/token/ERC721/ERC721Full.test.js @@ -138,6 +138,14 @@ contract('ERC721Full', function ([ }); }); + describe('tokensOfOwner', function () { + it('returns total tokens of owner', async function () { + const tokenIds = await this.token.tokensOfOwner(owner); + tokenIds[0].should.be.bignumber.equal(firstTokenId); + tokenIds[1].should.be.bignumber.equal(secondTokenId); + }); + }); + describe('totalSupply', function () { it('returns total token supply', async function () { (await this.token.totalSupply()).should.be.bignumber.equal(2); From 94fa635730366b4e51f6f30862b034551354b5b4 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Dec 2018 11:26:58 +0530 Subject: [PATCH 15/15] Update test/token/ERC721/ERC721Full.test.js Co-Authored-By: Aniket-Engg <30843294+Aniket-Engg@users.noreply.github.com> --- test/token/ERC721/ERC721Full.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/token/ERC721/ERC721Full.test.js b/test/token/ERC721/ERC721Full.test.js index e06676d45b1..933f624eeb0 100644 --- a/test/token/ERC721/ERC721Full.test.js +++ b/test/token/ERC721/ERC721Full.test.js @@ -141,6 +141,7 @@ contract('ERC721Full', function ([ describe('tokensOfOwner', function () { it('returns total tokens of owner', async function () { const tokenIds = await this.token.tokensOfOwner(owner); + tokenIds.length.should.equal(2); tokenIds[0].should.be.bignumber.equal(firstTokenId); tokenIds[1].should.be.bignumber.equal(secondTokenId); });