From 6024d12b9661bb3c37ca9881b34a1e856f5ac2c9 Mon Sep 17 00:00:00 2001 From: Aniket Date: Sun, 11 Mar 2018 02:50:54 +0530 Subject: [PATCH 01/20] signing prefix added --- contracts/ECRecovery.sol | 54 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 contracts/ECRecovery.sol diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol new file mode 100644 index 00000000000..709ee43cb39 --- /dev/null +++ b/contracts/ECRecovery.sol @@ -0,0 +1,54 @@ +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"; + hash = keccak256(prefix, hash); + return ecrecover(hash, v, r, s); + } + } + +} From fb713432f353c8df574ff463ea4363e37bfa0188 Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 15:24:23 +0530 Subject: [PATCH 02/20] 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 e51c5e01c1621bd9134a1bd4fe83e3389e2a62ab Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 16:30:25 +0530 Subject: [PATCH 03/20] Successfully tested --- test/helpers/hashMessage.js | 8 +++++ test/library/ECRecovery.test.js | 62 +++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 test/helpers/hashMessage.js create mode 100644 test/library/ECRecovery.test.js diff --git a/test/helpers/hashMessage.js b/test/helpers/hashMessage.js new file mode 100644 index 00000000000..6e8a1f74e7f --- /dev/null +++ b/test/helpers/hashMessage.js @@ -0,0 +1,8 @@ +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()); + return utils.bufferToHex(utils.sha3(Buffer.concat([prefix, messageHex]))); +}; diff --git a/test/library/ECRecovery.test.js b/test/library/ECRecovery.test.js new file mode 100644 index 00000000000..fd99a0ae12f --- /dev/null +++ b/test/library/ECRecovery.test.js @@ -0,0 +1,62 @@ +var ECRecoveryMock = artifacts.require('ECRecoveryMock'); +var ECRecoveryLib = artifacts.require('ECRecovery'); + +var hashMessage = require('../helpers/hashMessage.js'); + +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 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)); + + // 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 c5235880d28d930c7d64d09c93ced1835ab1a348 Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 17:12:10 +0530 Subject: [PATCH 04/20] 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 86581c45489b07327b47621dd9ed8e86c3fd371f Mon Sep 17 00:00:00 2001 From: Aniket Date: Tue, 13 Mar 2018 17:17:29 +0530 Subject: [PATCH 05/20] 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 fa7809a4ea937f87f9a69fb7c0f7a26841fb6565 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Mon, 1 Oct 2018 15:49:33 +0530 Subject: [PATCH 06/20] 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/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 c06f6e87225..8816abf6db1 100644 --- a/.eslintrc +++ b/.eslintrc @@ -26,7 +26,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 4a53c5f9781..048e4d9fa47 100644 --- a/test/crowdsale/AllowanceCrowdsale.test.js +++ b/test/crowdsale/AllowanceCrowdsale.test.js @@ -42,7 +42,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 dbd33f1aa68..53721fcf393 100644 --- a/test/crowdsale/Crowdsale.test.js +++ b/test/crowdsale/Crowdsale.test.js @@ -83,7 +83,7 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount, + amount: expectedTokenAmount }); }); @@ -106,7 +106,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 ec41e494858..8fee5b35b50 100644 --- a/test/crowdsale/MintedCrowdsale.behavior.js +++ b/test/crowdsale/MintedCrowdsale.behavior.js @@ -21,7 +21,7 @@ function shouldBehaveLikeMintedCrowdsale ([_, investor, wallet, purchaser], rate purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount, + amount: expectedTokenAmount }); }); diff --git a/test/payment/escrow/Escrow.behavior.js b/test/payment/escrow/Escrow.behavior.js index 80bde2f5c5a..08f03247260 100644 --- a/test/payment/escrow/Escrow.behavior.js +++ b/test/payment/escrow/Escrow.behavior.js @@ -31,7 +31,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 }); }); @@ -80,7 +80,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 16d2cab7524..3ddd1362aac 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -60,7 +60,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount, + value: amount }); }); }); @@ -88,7 +88,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount, + value: amount }); }); @@ -122,7 +122,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount, + value: amount }); }); @@ -192,7 +192,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount, + value: amount }); }); @@ -277,7 +277,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: 0, + value: 0 }); }); @@ -334,7 +334,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount, + value: amount }); }); @@ -368,7 +368,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 6d1c7195b07..26f2ee741c1 100644 --- a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js @@ -28,7 +28,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - value: amount, + value: amount }); }); } @@ -74,7 +74,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 987b677524e..9334870716a 100644 --- a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js @@ -33,7 +33,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 b7600872c09..7850464b220 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -87,7 +87,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId, + tokenId: tokenId }); }); } else { @@ -95,7 +95,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId, + tokenId: tokenId }); }); } @@ -160,7 +160,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: owner, - tokenId: tokenId, + tokenId: tokenId }); }); @@ -336,7 +336,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Approval', { owner: owner, approved: address, - tokenId: tokenId, + tokenId: tokenId }); }); }; @@ -446,7 +446,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true, + approved: true }); }); }); @@ -468,7 +468,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true, + approved: true }); }); @@ -496,7 +496,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 97c33ae3630..dc3f496c73e 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -41,7 +41,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: ZERO_ADDRESS, to: newOwner, - tokenId: thirdTokenId, + tokenId: thirdTokenId }); }); }); @@ -86,7 +86,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - tokenId: tokenId, + tokenId: tokenId }); }); }); From 10abc85ac1eecf389a801f623380b26df9f913a5 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Mon, 1 Oct 2018 22:00:42 +0530 Subject: [PATCH 07/20] 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 8816abf6db1..c06f6e87225 100644 --- a/.eslintrc +++ b/.eslintrc @@ -26,7 +26,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 9bb567055fddd094294d5652d6d091046e15fc25 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Thu, 11 Oct 2018 14:18:33 +0530 Subject: [PATCH 08/20] fixes #1404 --- contracts/mocks/SafeERC20Helper.sol | 92 +++++++++++++------ contracts/token/ERC20/IERC20.sol | 12 ++- contracts/token/ERC20/SafeERC20.sol | 47 ++++++---- test/crowdsale/AllowanceCrowdsale.test.js | 2 +- test/crowdsale/Crowdsale.test.js | 4 +- test/crowdsale/MintedCrowdsale.behavior.js | 2 +- test/payment/escrow/Escrow.behavior.js | 4 +- test/token/ERC20/ERC20.test.js | 14 +-- test/token/ERC20/SafeERC20.test.js | 22 ++++- .../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, 148 insertions(+), 75 deletions(-) diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 9c0af131239..838a2512c5c 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -3,24 +3,38 @@ pragma solidity ^0.4.24; import "../token/ERC20/IERC20.sol"; import "../token/ERC20/SafeERC20.sol"; -contract ERC20FailingMock { - uint256 private _allowance; - - function transfer(address, uint256) public returns (bool) { - return false; - } - - function transferFrom(address, address, uint256) public returns (bool) { - return false; - } - - function approve(address, uint256) public returns (bool) { - return false; - } - - function allowance(address, address) public view returns (uint256) { - return 0; - } +contract ERC20FailingMock is IERC20 { + function totalSupply() public view returns (uint256) { + return 0; + } + + function transfer(address, uint256) public returns (bool) { + return false; + } + + function transferFrom(address, address, uint256) public returns (bool) { + return false; + } + + function approve(address, uint256) public returns (bool) { + 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; + } + + function allowance(address, address) public view returns (uint256) { + return 0; + } } contract ERC20SucceedingMock { @@ -38,9 +52,17 @@ contract ERC20SucceedingMock { return true; } - function setAllowance(uint256 allowance_) public { - _allowance = allowance_; - } + 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; + } function allowance(address, address) public view returns (uint256) { return _allowance; @@ -98,15 +120,31 @@ contract SafeERC20Helper { _succeeding.safeIncreaseAllowance(address(0), amount); } - function doSucceedingDecreaseAllowance(uint256 amount) public { - _succeeding.safeDecreaseAllowance(address(0), amount); - } + function doFailingIncreaseAllowance() public { + _failing.safeIncreaseAllowance(address(0), 0); + } + + function doFailingDecreaseAllowance() public { + _failing.safeDecreaseAllowance(address(0), 0); + } + + function doSucceedingTransfer() public { + _succeeding.safeTransfer(address(0), 0); + } function setAllowance(uint256 allowance_) public { ERC20SucceedingMock(_succeeding).setAllowance(allowance_); } - function allowance() public view returns (uint256) { - return _succeeding.allowance(address(0), address(0)); - } + 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 7c44280f07a..96c689463a3 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -17,7 +17,17 @@ interface IERC20 { function transferFrom(address from, address to, uint256 value) external returns (bool); - event Transfer(address indexed from, address indexed to, uint256 value); + 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, + 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 adb217e0b19..473c2c431ce 100644 --- a/contracts/token/ERC20/SafeERC20.sol +++ b/contracts/token/ERC20/SafeERC20.sol @@ -16,25 +16,34 @@ library SafeERC20 { require(token.transfer(to, value)); } - function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { - require(token.transferFrom(from, to, value)); - } + function safeApprove( + IERC20 token, + address spender, + uint256 value + ) + internal + { + require((value == 0) || (token.allowance(msg.sender, spender) == 0)); + require(token.approve(spender, value)); + } - function safeApprove(IERC20 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, + address spender, + uint256 addedValue + ) + internal + { + require(token.increaseAllowance(spender, addedValue)); + } - function safeIncreaseAllowance(IERC20 token, address spender, uint256 value) internal { - uint256 newAllowance = token.allowance(address(this), spender).add(value); - require(token.approve(spender, newAllowance)); - } - - function safeDecreaseAllowance(IERC20 token, address spender, uint256 value) internal { - uint256 newAllowance = token.allowance(address(this), spender).sub(value); - require(token.approve(spender, newAllowance)); - } + 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 048e4d9fa47..4a53c5f9781 100644 --- a/test/crowdsale/AllowanceCrowdsale.test.js +++ b/test/crowdsale/AllowanceCrowdsale.test.js @@ -42,7 +42,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 53721fcf393..dbd33f1aa68 100644 --- a/test/crowdsale/Crowdsale.test.js +++ b/test/crowdsale/Crowdsale.test.js @@ -83,7 +83,7 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); @@ -106,7 +106,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 8fee5b35b50..ec41e494858 100644 --- a/test/crowdsale/MintedCrowdsale.behavior.js +++ b/test/crowdsale/MintedCrowdsale.behavior.js @@ -21,7 +21,7 @@ function shouldBehaveLikeMintedCrowdsale ([_, investor, wallet, purchaser], rate purchaser: investor, beneficiary: investor, value: value, - amount: expectedTokenAmount + amount: expectedTokenAmount, }); }); diff --git a/test/payment/escrow/Escrow.behavior.js b/test/payment/escrow/Escrow.behavior.js index 08f03247260..80bde2f5c5a 100644 --- a/test/payment/escrow/Escrow.behavior.js +++ b/test/payment/escrow/Escrow.behavior.js @@ -31,7 +31,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, }); }); @@ -80,7 +80,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 3ddd1362aac..16d2cab7524 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -60,7 +60,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount + value: amount, }); }); }); @@ -88,7 +88,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -122,7 +122,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -192,7 +192,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, - value: amount + value: amount, }); }); @@ -277,7 +277,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: 0 + value: 0, }); }); @@ -334,7 +334,7 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { expectEvent.inLogs(logs, 'Approval', { owner: owner, spender: spender, - value: amount + value: amount, }); }); @@ -368,7 +368,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 4be44cf384d..b1308a5b77f 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -22,9 +22,17 @@ contract('SafeERC20', function () { await shouldFail.reverting(this.helper.doFailingApprove()); }); - it('reverts on increaseAllowance', async function () { - await shouldFail.reverting(this.helper.doFailingIncreaseAllowance()); - }); + 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(); + }); it('reverts on decreaseAllowance', async function () { await shouldFail.reverting(this.helper.doFailingDecreaseAllowance()); @@ -90,4 +98,12 @@ contract('SafeERC20', function () { }); }); }); + + 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 26f2ee741c1..6d1c7195b07 100644 --- a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js @@ -28,7 +28,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - value: amount + value: amount, }); }); } @@ -74,7 +74,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 9334870716a..987b677524e 100644 --- a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js @@ -33,7 +33,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 7850464b220..b7600872c09 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -87,7 +87,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId + tokenId: tokenId, }); }); } else { @@ -95,7 +95,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, - tokenId: tokenId + tokenId: tokenId, }); }); } @@ -160,7 +160,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: owner, - tokenId: tokenId + tokenId: tokenId, }); }); @@ -336,7 +336,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'Approval', { owner: owner, approved: address, - tokenId: tokenId + tokenId: tokenId, }); }); }; @@ -446,7 +446,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true + approved: true, }); }); }); @@ -468,7 +468,7 @@ function shouldBehaveLikeERC721 ( expectEvent.inLogs(logs, 'ApprovalForAll', { owner: owner, operator: operator, - approved: true + approved: true, }); }); @@ -496,7 +496,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 dc3f496c73e..97c33ae3630 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -41,7 +41,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: ZERO_ADDRESS, to: newOwner, - tokenId: thirdTokenId + tokenId: thirdTokenId, }); }); }); @@ -86,7 +86,7 @@ function shouldBehaveLikeMintAndBurnERC721 ( expectEvent.inLogs(logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, - tokenId: tokenId + tokenId: tokenId, }); }); }); From b40c76d0b1e2fc356c63a01d0d6eb1abd0196a73 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Thu, 11 Oct 2018 16:07:39 +0530 Subject: [PATCH 09/20] approve failing test --- contracts/mocks/SafeERC20Helper.sol | 10 +++++++--- test/token/ERC20/SafeERC20.test.js | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 838a2512c5c..c020df7dc03 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -64,9 +64,9 @@ contract ERC20SucceedingMock { return 0; } - function allowance(address, address) public view returns (uint256) { - return _allowance; - } + function allowance(address, address) public view returns (uint256) { + return 10; //non-zero allowance + } } contract SafeERC20Helper { @@ -140,6 +140,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 b1308a5b77f..687e6ee875b 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -99,6 +99,10 @@ 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(); }); From 8660a2c6f62b07a7060557f29ca0f629e42cbdd8 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Tue, 16 Oct 2018 12:37:47 +0530 Subject: [PATCH 10/20] suggested changes done --- contracts/mocks/SafeERC20Helper.sol | 31 ++++++++++++++++---- contracts/token/ERC20/IERC20.sol | 6 ---- contracts/token/ERC20/ISafeERC20.sol | 43 ++++++++++++++++++++++++++++ contracts/token/ERC20/SafeERC20.sol | 36 +++++++++++++++++------ 4 files changed, 96 insertions(+), 20 deletions(-) create mode 100644 contracts/token/ERC20/ISafeERC20.sol diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index c020df7dc03..52f618b0595 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,10 +36,18 @@ contract ERC20FailingMock is IERC20 { function allowance(address, address) public view returns (uint256) { return 0; } + + function setAllowance(uint256 value) public { + _allowance = value; + } } -contract ERC20SucceedingMock { - uint256 private _allowance; +contract ERC20SucceedingMock is ISafeERC20 { + uint256 private _allowance; + + function totalSupply() public view returns (uint256) { + return 0; + } function transfer(address, uint256) public returns (bool) { return true; @@ -65,15 +74,26 @@ contract ERC20SucceedingMock { } function allowance(address, address) public view returns (uint256) { - return 10; //non-zero allowance + return _allowance; + } + + function setAllowance(uint256 value) public { + _allowance = value; } } contract SafeERC20Helper { +<<<<<<< b40c76d0b1e2fc356c63a01d0d6eb1abd0196a73 using SafeERC20 for IERC20; IERC20 private _failing; IERC20 private _succeeding; +======= + using SafeERC20 for ISafeERC20; + + ISafeERC20 private _failing; + ISafeERC20 private _succeeding; +>>>>>>> suggested changes done constructor () public { _failing = IERC20(new ERC20FailingMock()); @@ -141,6 +161,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 96c689463a3..ac2d8533105 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -17,12 +17,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 473c2c431ce..1b91e823676 100644 --- a/contracts/token/ERC20/SafeERC20.sol +++ b/contracts/token/ERC20/SafeERC20.sol @@ -1,7 +1,7 @@ pragma solidity ^0.4.24; -import "./IERC20.sol"; -import "../../math/SafeMath.sol"; +import "./ERC20.sol"; +import "./ISafeERC20.sol"; /** * @title SafeERC20 @@ -10,25 +10,43 @@ import "../../math/SafeMath.sol"; * which allows you to call the safe operations as `token.safeTransfer(...)`, etc. */ library SafeERC20 { - using SafeMath for uint256; + function safeTransfer( + ISafeERC20 token, + address to, + uint256 value + ) + internal + { + require(token.transfer(to, value)); + } - function safeTransfer(IERC20 token, address to, uint256 value) internal { - require(token.transfer(to, value)); - } + function safeTransferFrom( + ISafeERC20 token, + address from, + address to, + uint256 value + ) + internal + { + require(token.transferFrom(from, to, value)); + } 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 ) @@ -38,7 +56,7 @@ library SafeERC20 { } function safeDecreaseAllowance( - IERC20 token, + ISafeERC20 token, address spender, uint256 subtractedValue ) From 51e4da155b05c48e1e1e56b099912f4dec71322c Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Tue, 16 Oct 2018 12:52:00 +0530 Subject: [PATCH 11/20] ISafeERC20 removed --- contracts/mocks/SafeERC20Helper.sol | 18 ++++-------- contracts/token/ERC20/IERC20.sol | 9 ++++++ contracts/token/ERC20/ISafeERC20.sol | 43 ---------------------------- contracts/token/ERC20/SafeERC20.sol | 12 ++++---- 4 files changed, 21 insertions(+), 61 deletions(-) delete mode 100644 contracts/token/ERC20/ISafeERC20.sol diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 52f618b0595..3392f7301ee 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,17 +83,11 @@ contract ERC20SucceedingMock is ISafeERC20 { } contract SafeERC20Helper { -<<<<<<< b40c76d0b1e2fc356c63a01d0d6eb1abd0196a73 - using SafeERC20 for IERC20; - IERC20 private _failing; - IERC20 private _succeeding; -======= - using SafeERC20 for ISafeERC20; + using SafeERC20 for IERC20; - ISafeERC20 private _failing; - ISafeERC20 private _succeeding; ->>>>>>> suggested changes done + IERC20 private _failing; + IERC20 private _succeeding; constructor () public { _failing = IERC20(new ERC20FailingMock()); diff --git a/contracts/token/ERC20/IERC20.sol b/contracts/token/ERC20/IERC20.sol index ac2d8533105..f2118d6aec5 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -17,6 +17,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 a781955434e2cdc0056b8c9659fc512710fc5052 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Mon, 22 Oct 2018 13:21:32 +0530 Subject: [PATCH 12/20] 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 3392f7301ee..0dd602ffe69 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -36,10 +36,6 @@ 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 { @@ -153,17 +149,4 @@ contract SafeERC20Helper { function doSucceedingApprove() public { _succeeding.safeApprove(address(0), 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 f2118d6aec5..ac2d8533105 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -17,15 +17,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 687e6ee875b..b3149658fca 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -98,16 +98,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 c3c5d767ff3ff97ebb85aaa5ebba76f106f608a7 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Fri, 21 Dec 2018 16:50:16 +0530 Subject: [PATCH 13/20] added examples --- contracts/examples/SampleTokenTimelock.sol | 32 ++++++ contracts/examples/SampleTokenVesting.sol | 23 +++++ contracts/mocks/SafeERC20Helper.sol | 112 +++++++-------------- contracts/token/ERC20/SafeERC20.sol | 77 +++++--------- package-lock.json | 28 ++---- test/helpers/hashMessage.js | 8 -- test/token/ERC20/SafeERC20.test.js | 18 ++-- 7 files changed, 132 insertions(+), 166 deletions(-) create mode 100644 contracts/examples/SampleTokenTimelock.sol create mode 100644 contracts/examples/SampleTokenVesting.sol delete mode 100644 test/helpers/hashMessage.js diff --git a/contracts/examples/SampleTokenTimelock.sol b/contracts/examples/SampleTokenTimelock.sol new file mode 100644 index 00000000000..ee54da6ca39 --- /dev/null +++ b/contracts/examples/SampleTokenTimelock.sol @@ -0,0 +1,32 @@ +pragma solidity ^0.4.24; + +import "../token/ERC20/ERC20Mintable.sol"; +import "../token/ERC20/ERC20Detailed.sol"; +import "../token/ERC20/TokenTimelock.sol"; + +/** + * @title SampleTimelockToken + * @dev Very simple ERC20 Token that can be minted. + * It is meant to be used in a tokentimelock contract. + */ +contract SampleTimelockToken is ERC20Mintable, ERC20Detailed { + constructor() public ERC20Detailed("Sample Timelock Token", "STT", 18) {} +} + + +/** + * @title SampleTokenTimelock + * @dev This is an example of a token lock for certain time. + */ + +contract SampleTokenTimelock is TokenTimelock{ + + constructor( + ERC20Mintable token, + address beneficiary, + uint256 releaseTime + ) + public + TokenTimelock(token, beneficiary, releaseTime){} + +} diff --git a/contracts/examples/SampleTokenVesting.sol b/contracts/examples/SampleTokenVesting.sol new file mode 100644 index 00000000000..8b86f9c7e34 --- /dev/null +++ b/contracts/examples/SampleTokenVesting.sol @@ -0,0 +1,23 @@ +pragma solidity ^0.4.24; + +import "../drafts/TokenVesting.sol"; + +/** + * @title SampleTokenVesting + * @dev This is an example of a token vesting for defined time period. + * Tokens to be vested will be sent directly to this contract. + */ + +contract SampleTokenVesting is TokenVesting{ + + constructor( + address beneficiary, + uint256 start, + uint256 cliffDuration, + uint256 duration, + bool revocable + ) + public + TokenVesting(beneficiary, start, cliffDuration, duration, revocable){} + +} \ No newline at end of file diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 0dd602ffe69..d8fa2d5f626 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -3,47 +3,28 @@ pragma solidity ^0.4.24; import "../token/ERC20/IERC20.sol"; import "../token/ERC20/SafeERC20.sol"; -contract ERC20FailingMock is IERC20 { - uint256 private _allowance; - function totalSupply() public view returns (uint256) { - return 0; - } - - function transfer(address, uint256) public returns (bool) { - return false; - } - - function transferFrom(address, address, uint256) public returns (bool) { - return false; - } - - function approve(address, uint256) public returns (bool) { - 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; - } - - function allowance(address, address) public view returns (uint256) { - return 0; - } -} +contract ERC20FailingMock { + uint256 private _allowance; + + function transfer(address, uint256) public returns (bool) { + return false; + } + + function transferFrom(address, address, uint256) public returns (bool) { + return false; + } + + function approve(address, uint256) public returns (bool) { + return false; + } -contract ERC20SucceedingMock is IERC20 { - uint256 private _allowance; + function allowance(address, address) public view returns (uint256) { + return 0; + } +} - function totalSupply() public view returns (uint256) { - return 0; - } +contract ERC20SucceedingMock { + uint256 private _allowance; function transfer(address, uint256) public returns (bool) { return true; @@ -57,33 +38,20 @@ 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; - } - - function allowance(address, address) public view returns (uint256) { - return _allowance; - } + function setAllowance(uint256 allowance_) public { + _allowance = allowance_; + } - function setAllowance(uint256 value) public { - _allowance = value; - } + function allowance(address, address) public view returns (uint256) { + return _allowance; + } } contract SafeERC20Helper { + using SafeERC20 for IERC20; - using SafeERC20 for IERC20; - - IERC20 private _failing; - IERC20 private _succeeding; + IERC20 private _failing; + IERC20 private _succeeding; constructor () public { _failing = IERC20(new ERC20FailingMock()); @@ -130,23 +98,15 @@ contract SafeERC20Helper { _succeeding.safeIncreaseAllowance(address(0), amount); } - function doFailingIncreaseAllowance() public { - _failing.safeIncreaseAllowance(address(0), 0); - } - - function doFailingDecreaseAllowance() public { - _failing.safeDecreaseAllowance(address(0), 0); - } - - function doSucceedingTransfer() public { - _succeeding.safeTransfer(address(0), 0); - } + function doSucceedingDecreaseAllowance(uint256 amount) public { + _succeeding.safeDecreaseAllowance(address(0), amount); + } function setAllowance(uint256 allowance_) public { ERC20SucceedingMock(_succeeding).setAllowance(allowance_); } - function doSucceedingApprove() public { - _succeeding.safeApprove(address(0), 0); - } -} + function allowance() public view returns (uint256) { + return _succeeding.allowance(address(0), address(0)); + } +} \ No newline at end of file diff --git a/contracts/token/ERC20/SafeERC20.sol b/contracts/token/ERC20/SafeERC20.sol index 0c846d0a02e..a8fac52a4c5 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 "../../math/SafeMath.sol"; /** * @title SafeERC20 @@ -10,58 +10,31 @@ import "./IERC20.sol"; * which allows you to call the safe operations as `token.safeTransfer(...)`, etc. */ library SafeERC20 { - function safeTransfer( - IERC20 token, - address to, - uint256 value - ) - internal - { - require(token.transfer(to, value)); - } + using SafeMath for uint256; - function safeTransferFrom( - IERC20 token, - address from, - address to, - uint256 value - ) - internal - { - require(token.transferFrom(from, to, value)); - } + function safeTransfer(IERC20 token, address to, uint256 value) internal { + require(token.transfer(to, value)); + } - function safeApprove( - IERC20 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 safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { + require(token.transferFrom(from, to, value)); + } - function safeIncreaseAllowance( - IERC20 token, - address spender, - uint256 addedValue - ) - internal - { - require(token.increaseAllowance(spender, addedValue)); - } + function safeApprove(IERC20 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 safeDecreaseAllowance( - IERC20 token, - address spender, - uint256 subtractedValue - ) - internal - { - require(token.decreaseAllowance(spender, subtractedValue)); - } -} + function safeIncreaseAllowance(IERC20 token, address spender, uint256 value) internal { + uint256 newAllowance = token.allowance(address(this), spender).add(value); + require(token.approve(spender, newAllowance)); + } + + function safeDecreaseAllowance(IERC20 token, address spender, uint256 value) internal { + uint256 newAllowance = token.allowance(address(this), spender).sub(value); + require(token.approve(spender, newAllowance)); + } +} \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index b5ce5da27c7..5912476894b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4007,14 +4007,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -4029,20 +4027,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -4159,8 +4154,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -4172,7 +4166,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4187,7 +4180,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -4195,14 +4187,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.2.4", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -4221,7 +4211,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -4302,8 +4291,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -4315,7 +4303,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -4437,7 +4424,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", diff --git a/test/helpers/hashMessage.js b/test/helpers/hashMessage.js deleted file mode 100644 index 6e8a1f74e7f..00000000000 --- a/test/helpers/hashMessage.js +++ /dev/null @@ -1,8 +0,0 @@ -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()); - return utils.bufferToHex(utils.sha3(Buffer.concat([prefix, messageHex]))); -}; diff --git a/test/token/ERC20/SafeERC20.test.js b/test/token/ERC20/SafeERC20.test.js index b3149658fca..a7888fb68be 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -22,17 +22,17 @@ 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 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 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(); - }); + it('should not throw on succeeding transfer', async function () { + await this.helper.doSucceedingTransfer(); + }); it('reverts on decreaseAllowance', async function () { await shouldFail.reverting(this.helper.doFailingDecreaseAllowance()); From 5592be00a0e42a81ebacf02480a14b950fbf968a Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Fri, 21 Dec 2018 17:02:06 +0530 Subject: [PATCH 14/20] fixes #706 --- contracts/token/ERC20/IERC20.sol | 8 ++------ package-lock.json | 30 ++++++++++++++++++++++-------- test/token/ERC20/SafeERC20.test.js | 12 ++---------- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/contracts/token/ERC20/IERC20.sol b/contracts/token/ERC20/IERC20.sol index ac2d8533105..1bf236091c1 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -17,11 +17,7 @@ interface IERC20 { function transferFrom(address from, address to, uint256 value) external returns (bool); - event Transfer( - address indexed from, - address indexed to, - uint256 value - ); + event Transfer(address indexed from, address indexed to, uint256 value); event Approval(address indexed owner, address indexed spender, uint256 value); -} +} \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 5912476894b..1ecb33dc8ad 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4007,12 +4007,14 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -4027,17 +4029,20 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -4154,7 +4159,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -4166,6 +4172,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4180,6 +4187,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -4187,12 +4195,14 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.2.4", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -4211,6 +4221,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -4291,7 +4302,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -4303,6 +4315,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -4424,6 +4437,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -9904,4 +9918,4 @@ } } } -} +} \ No newline at end of file diff --git a/test/token/ERC20/SafeERC20.test.js b/test/token/ERC20/SafeERC20.test.js index a7888fb68be..6935e33a7e2 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -22,18 +22,10 @@ contract('SafeERC20', function () { await shouldFail.reverting(this.helper.doFailingApprove()); }); - it('should throw on failed increaseAllowance', async function () { + it('reverts on 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(); - }); - it('reverts on decreaseAllowance', async function () { await shouldFail.reverting(this.helper.doFailingDecreaseAllowance()); }); @@ -98,4 +90,4 @@ contract('SafeERC20', function () { }); }); }); -}); +}); \ No newline at end of file From 3c8076727066ba297ad86d52b9c8a835245ba8f4 Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Fri, 21 Dec 2018 17:27:50 +0530 Subject: [PATCH 15/20] linting --- test/token/ERC20/SafeERC20.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/token/ERC20/SafeERC20.test.js b/test/token/ERC20/SafeERC20.test.js index 6935e33a7e2..4be44cf384d 100644 --- a/test/token/ERC20/SafeERC20.test.js +++ b/test/token/ERC20/SafeERC20.test.js @@ -90,4 +90,4 @@ contract('SafeERC20', function () { }); }); }); -}); \ No newline at end of file +}); From fcf722c2c26856c7e293f66a5d577bd06214645d Mon Sep 17 00:00:00 2001 From: Aniket-Engg Date: Wed, 16 Jan 2019 18:59:03 +0530 Subject: [PATCH 16/20] fixes #1604 --- contracts/examples/SampleTokenTimelock.sol | 32 ---------------------- contracts/examples/SampleTokenVesting.sol | 23 ---------------- contracts/token/ERC20/ERC20.sol | 27 ++++++++++-------- 3 files changed, 15 insertions(+), 67 deletions(-) delete mode 100644 contracts/examples/SampleTokenTimelock.sol delete mode 100644 contracts/examples/SampleTokenVesting.sol diff --git a/contracts/examples/SampleTokenTimelock.sol b/contracts/examples/SampleTokenTimelock.sol deleted file mode 100644 index ee54da6ca39..00000000000 --- a/contracts/examples/SampleTokenTimelock.sol +++ /dev/null @@ -1,32 +0,0 @@ -pragma solidity ^0.4.24; - -import "../token/ERC20/ERC20Mintable.sol"; -import "../token/ERC20/ERC20Detailed.sol"; -import "../token/ERC20/TokenTimelock.sol"; - -/** - * @title SampleTimelockToken - * @dev Very simple ERC20 Token that can be minted. - * It is meant to be used in a tokentimelock contract. - */ -contract SampleTimelockToken is ERC20Mintable, ERC20Detailed { - constructor() public ERC20Detailed("Sample Timelock Token", "STT", 18) {} -} - - -/** - * @title SampleTokenTimelock - * @dev This is an example of a token lock for certain time. - */ - -contract SampleTokenTimelock is TokenTimelock{ - - constructor( - ERC20Mintable token, - address beneficiary, - uint256 releaseTime - ) - public - TokenTimelock(token, beneficiary, releaseTime){} - -} diff --git a/contracts/examples/SampleTokenVesting.sol b/contracts/examples/SampleTokenVesting.sol deleted file mode 100644 index 8b86f9c7e34..00000000000 --- a/contracts/examples/SampleTokenVesting.sol +++ /dev/null @@ -1,23 +0,0 @@ -pragma solidity ^0.4.24; - -import "../drafts/TokenVesting.sol"; - -/** - * @title SampleTokenVesting - * @dev This is an example of a token vesting for defined time period. - * Tokens to be vested will be sent directly to this contract. - */ - -contract SampleTokenVesting is TokenVesting{ - - constructor( - address beneficiary, - uint256 start, - uint256 cliffDuration, - uint256 duration, - bool revocable - ) - public - TokenVesting(beneficiary, start, cliffDuration, duration, revocable){} - -} \ No newline at end of file diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 30f9642f8be..f00e4c0b408 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -70,10 +70,7 @@ contract ERC20 is IERC20 { * @param value The amount of tokens to be spent. */ function approve(address spender, uint256 value) public returns (bool) { - require(spender != address(0)); - - _allowed[msg.sender][spender] = value; - emit Approval(msg.sender, spender, value); + _approve(spender, value); return true; } @@ -103,10 +100,7 @@ contract ERC20 is IERC20 { * @param addedValue The amount of tokens to increase the allowance by. */ function increaseAllowance(address spender, uint256 addedValue) public returns (bool) { - require(spender != address(0)); - - _allowed[msg.sender][spender] = _allowed[msg.sender][spender].add(addedValue); - emit Approval(msg.sender, spender, _allowed[msg.sender][spender]); + _approve(spender, _allowed[msg.sender][spender].add(addedValue)); return true; } @@ -121,10 +115,7 @@ contract ERC20 is IERC20 { * @param subtractedValue The amount of tokens to decrease the allowance by. */ function decreaseAllowance(address spender, uint256 subtractedValue) public returns (bool) { - require(spender != address(0)); - - _allowed[msg.sender][spender] = _allowed[msg.sender][spender].sub(subtractedValue); - emit Approval(msg.sender, spender, _allowed[msg.sender][spender]); + _approve(spender, _allowed[msg.sender][spender].sub(subtractedValue)); return true; } @@ -142,6 +133,18 @@ contract ERC20 is IERC20 { emit Transfer(from, to, value); } + /** + * @dev Approve token for a specified addresses + * @param spender The address which will spend the funds. + * @param value The amount to be spent. + */ + function _approve(address spender, uint256 value) internal { + require(spender != address(0)); + + _allowed[msg.sender][spender] = value; + emit Approval(msg.sender, spender, value); + } + /** * @dev Internal function that mints an amount of the token and assigns it to * an account. This encapsulates the modification of balances such that the From 440d3932bbe145f61903a9b85696cea23d264e6b Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Wed, 16 Jan 2019 19:01:34 +0530 Subject: [PATCH 17/20] Update package-lock.json --- package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 3abf68f9b6f..bc241ff27b8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9068,4 +9068,4 @@ } } } -} \ No newline at end of file +} From a79b311883cdc8134c836d63cb174ffe210c2d25 Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Wed, 16 Jan 2019 19:02:12 +0530 Subject: [PATCH 18/20] Update SafeERC20.sol --- contracts/token/ERC20/SafeERC20.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/ERC20/SafeERC20.sol b/contracts/token/ERC20/SafeERC20.sol index 93a4a69a66c..25ac9bae799 100644 --- a/contracts/token/ERC20/SafeERC20.sol +++ b/contracts/token/ERC20/SafeERC20.sol @@ -37,4 +37,4 @@ library SafeERC20 { uint256 newAllowance = token.allowance(address(this), spender).sub(value); require(token.approve(spender, newAllowance)); } -} \ No newline at end of file +} From aec943086147c757e96ba6d2a8fbe8a8e3f8cfcb Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Wed, 16 Jan 2019 19:02:48 +0530 Subject: [PATCH 19/20] Update SafeERC20Helper.sol --- contracts/mocks/SafeERC20Helper.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/mocks/SafeERC20Helper.sol b/contracts/mocks/SafeERC20Helper.sol index 4456fe66830..c9c455e5865 100644 --- a/contracts/mocks/SafeERC20Helper.sol +++ b/contracts/mocks/SafeERC20Helper.sol @@ -105,4 +105,4 @@ contract SafeERC20Helper { function allowance() public view returns (uint256) { return _succeeding.allowance(address(0), address(0)); } -} \ No newline at end of file +} From cf8307d231f9b127373e21691efb10e03ee7e83f Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Wed, 16 Jan 2019 19:03:11 +0530 Subject: [PATCH 20/20] Update IERC20.sol --- contracts/token/ERC20/IERC20.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/ERC20/IERC20.sol b/contracts/token/ERC20/IERC20.sol index ab30d6d2d38..af4a4855847 100644 --- a/contracts/token/ERC20/IERC20.sol +++ b/contracts/token/ERC20/IERC20.sol @@ -20,4 +20,4 @@ interface IERC20 { event Transfer(address indexed from, address indexed to, uint256 value); event Approval(address indexed owner, address indexed spender, uint256 value); -} \ No newline at end of file +}