From 3b66e70897a412ba6b126e40b3d9ecd3d021efe3 Mon Sep 17 00:00:00 2001 From: Jakub Bogacz Date: Tue, 18 Sep 2018 20:51:22 +0200 Subject: [PATCH 01/33] Add ERC20 token with snapshoting mechanism (#1209) --- contracts/mocks/ERC20SnapshotMock.sol | 12 +++ contracts/token/ERC20/ERC20Snapshot.sol | 96 ++++++++++++++++++++ contracts/utils/Arrays.sol | 35 ++++++++ test/token/ERC20/ERC20Snapshot.test.js | 113 ++++++++++++++++++++++++ 4 files changed, 256 insertions(+) create mode 100644 contracts/mocks/ERC20SnapshotMock.sol create mode 100644 contracts/token/ERC20/ERC20Snapshot.sol create mode 100644 contracts/utils/Arrays.sol create mode 100644 test/token/ERC20/ERC20Snapshot.test.js diff --git a/contracts/mocks/ERC20SnapshotMock.sol b/contracts/mocks/ERC20SnapshotMock.sol new file mode 100644 index 00000000000..f8c386afba6 --- /dev/null +++ b/contracts/mocks/ERC20SnapshotMock.sol @@ -0,0 +1,12 @@ +pragma solidity ^0.4.24; + +import "../token/ERC20/ERC20Snapshot.sol"; + + +contract ERC20SnapshotMock is ERC20Snapshot { + + constructor(address initialAccount, uint256 initialBalance) public { + _mint(initialAccount, initialBalance); + } + +} diff --git a/contracts/token/ERC20/ERC20Snapshot.sol b/contracts/token/ERC20/ERC20Snapshot.sol new file mode 100644 index 00000000000..1d2ca8e2689 --- /dev/null +++ b/contracts/token/ERC20/ERC20Snapshot.sol @@ -0,0 +1,96 @@ +pragma solidity ^0.4.24; + +import "./ERC20.sol"; +import "../../utils/Arrays.sol"; + + +/** + * @title ERC20Snapshot token + * @dev An ERC20 token which enables taking snapshots of accounts' balances. This can be useful to + * safely implement voting weighed by balance. + */ +contract ERC20Snapshot is ERC20 { + + using Arrays for uint256[]; + + // The 0 id represents no snapshot was taken yet. + uint256 public currentSnapshotId; + + mapping (address => uint256[]) private snapshotIds; + mapping (address => uint256[]) private snapshotBalances; + + event Snapshot(uint256 id); + + function snapshot() external returns (uint256) { + currentSnapshotId += 1; + emit Snapshot(currentSnapshotId); + return currentSnapshotId; + } + + function snapshotsLength(address account) external view returns (uint256) { + return snapshotIds[account].length; + } + + function balanceOfAt( + address account, + uint256 snapshotId + ) + external + view + returns (uint256) + { + require( + snapshotId > 0 && snapshotId <= currentSnapshotId, + "Parameter snapshotId has to be greater than 0 and lower/equal currentSnapshot"); + + uint256 idx = snapshotIds[account].findUpperBound(snapshotId); + + if (idx == snapshotIds[account].length) { + return balanceOf(account); + } else { + return snapshotBalances[account][idx]; + } + } + + function transfer( + address to, + uint256 value + ) + public + returns (bool) + { + updateSnapshot(msg.sender); + updateSnapshot(to); + return super.transfer(to, value); + } + + function transferFrom( + address from, + address to, + uint256 value + ) + public + returns (bool) + { + updateSnapshot(from); + updateSnapshot(to); + return super.transferFrom(from, to, value); + } + + function updateSnapshot(address account) internal { + if (lastSnapshotId(account) < currentSnapshotId) { + snapshotIds[account].push(currentSnapshotId); + snapshotBalances[account].push(balanceOf(account)); + } + } + + function lastSnapshotId(address account) internal view returns (uint256) { + uint256[] storage snapshots = snapshotIds[account]; + if (snapshots.length == 0) { + return 0; + } else { + return snapshots[snapshots.length - 1]; + } + } + +} diff --git a/contracts/utils/Arrays.sol b/contracts/utils/Arrays.sol new file mode 100644 index 00000000000..06fdaa4efb9 --- /dev/null +++ b/contracts/utils/Arrays.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.4.23; + +import "../math/Math.sol"; + + +library Arrays { + function findUpperBound( + uint256[] storage array, + uint256 element + ) + internal + view + returns (uint256) + { + uint256 low = 0; + uint256 high = array.length; + + while (low < high) { + uint256 mid = Math.average(low, high); + + if (array[mid] > element) { + high = mid; + } else { + low = mid + 1; + } + } + + // At this point at `low` is the exclusive upper bound. We will return the inclusive upper bound. + if (low > 0 && array[low - 1] == element) { + return low - 1; + } else { + return low; + } + } +} diff --git a/test/token/ERC20/ERC20Snapshot.test.js b/test/token/ERC20/ERC20Snapshot.test.js new file mode 100644 index 00000000000..ba6f4f84a26 --- /dev/null +++ b/test/token/ERC20/ERC20Snapshot.test.js @@ -0,0 +1,113 @@ +const ERC20Snapshot = artifacts.require('ERC20SnapshotMock'); +const { expectThrow } = require('../../helpers/expectThrow'); +const { EVMRevert } = require('../../helpers/EVMRevert'); +const BigNumber = web3.BigNumber; + +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('ERC20Snapshot', function ([_, owner, recipient, spender]) { + beforeEach(async function () { + this.token = await ERC20Snapshot.new(owner, 100); + }); + + context('there is no snapshots yet', function () { + describe('balanceOfAt', function () { + it('rejected for snapshotId parameter equals to 0', async function () { + await expectThrow( + this.token.balanceOfAt(owner, 0), + EVMRevert, + ); + }); + + it('rejected for snapshotId parameter greather than currentSnapshotId', async function () { + await expectThrow( + this.token.balanceOfAt(owner, 1), + EVMRevert, + ); + }); + }); + + describe('after transfer', function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('balanceOf returns correct value', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(90); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(10); + }); + + it('snapshots do not exist', async function () { + (await this.token.snapshotsLength(owner)).should.be.bignumber.equal(0); + (await this.token.snapshotsLength(recipient)).should.be.bignumber.equal(0); + }); + }); + }); + + context('snapshots exist', function () { + beforeEach(async function () { + await this.token.snapshot(); + }); + + describe('accounts do not have snapshots yet', function () { + it('snapshot value of the owner account equals to his balance', async function () { + const balanceOfAt = (await this.token.balanceOfAt(owner, 1)); + const balanceOf = (await this.token.balanceOf(owner)); + balanceOfAt.should.be.bignumber.equal(balanceOf); + }); + + it('snapshot value of the recipient account equals to balance', async function () { + const balanceOfAt = (await this.token.balanceOfAt(recipient, 1)); + const balanceOf = (await this.token.balanceOf(recipient)); + balanceOfAt.should.be.bignumber.equal(balanceOf); + }); + }); + + describe('accounts have already one snapshot', function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('snapshot keeps previous balance', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + + it('account has correct balance', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(90); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(10); + }); + }); + + describe('snapshot keeps previous balance even for multiple transfers', async function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('snapshot has previous balance', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + + it('account has correct balance', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(80); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(20); + }); + }); + + describe('snapshot keeps correct values for transfer from action', async function () { + beforeEach(async function () { + await this.token.approve(spender, 20, { from: owner }); + await this.token.transferFrom(owner, recipient, 20, { from: spender }); + }); + + it('spender and recipient snapshot is stored', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + }); + }); +}); From 1302e6ad899c7a79027cee7e601f1612d32175f6 Mon Sep 17 00:00:00 2001 From: jbogacz Date: Sun, 23 Sep 2018 11:51:10 +0200 Subject: [PATCH 02/33] Methods visibility corrected Require reason message removed Inline documentation in NatSpec added Snapshot added to _mint and _burn method --- contracts/mocks/ERC20SnapshotMock.sol | 7 ++ contracts/token/ERC20/ERC20Snapshot.sol | 86 +++++++++++++++++-------- contracts/utils/Arrays.sol | 16 +++-- test/token/ERC20/ERC20Snapshot.test.js | 33 ++++++++-- 4 files changed, 107 insertions(+), 35 deletions(-) diff --git a/contracts/mocks/ERC20SnapshotMock.sol b/contracts/mocks/ERC20SnapshotMock.sol index f8c386afba6..51c67a4502e 100644 --- a/contracts/mocks/ERC20SnapshotMock.sol +++ b/contracts/mocks/ERC20SnapshotMock.sol @@ -9,4 +9,11 @@ contract ERC20SnapshotMock is ERC20Snapshot { _mint(initialAccount, initialBalance); } + function mint(address account, uint256 amount) public { + _mint(account, amount); + } + + function burn(address account, uint256 amount) public { + _burn(account, amount); + } } diff --git a/contracts/token/ERC20/ERC20Snapshot.sol b/contracts/token/ERC20/ERC20Snapshot.sol index 1d2ca8e2689..cc34e0c618e 100644 --- a/contracts/token/ERC20/ERC20Snapshot.sol +++ b/contracts/token/ERC20/ERC20Snapshot.sol @@ -6,42 +6,46 @@ import "../../utils/Arrays.sol"; /** * @title ERC20Snapshot token - * @dev An ERC20 token which enables taking snapshots of accounts' balances. This can be useful to - * safely implement voting weighed by balance. + * @dev An ERC20 token which enables taking snapshots of account balances. + * This can be useful to safely implement voting weighed by balance. */ contract ERC20Snapshot is ERC20 { using Arrays for uint256[]; // The 0 id represents no snapshot was taken yet. - uint256 public currentSnapshotId; + uint256 private currentSnapshotId; mapping (address => uint256[]) private snapshotIds; mapping (address => uint256[]) private snapshotBalances; event Snapshot(uint256 id); + /** + * @dev Increments current snapshot. Emites Snapshot event. + * @return An uint256 representing current snapshot id. + */ function snapshot() external returns (uint256) { currentSnapshotId += 1; emit Snapshot(currentSnapshotId); return currentSnapshotId; } - function snapshotsLength(address account) external view returns (uint256) { - return snapshotIds[account].length; - } - + /** + * @dev Returns account balance for specific snapshot. + * @param account address The address to query the balance of. + * @param snapshotId uint256 The snapshot id for which to query the balance of. + * @return An uint256 representing the amount owned by the passed address for specific snapshot. + */ function balanceOfAt( address account, uint256 snapshotId ) - external - view - returns (uint256) + public + view + returns (uint256) { - require( - snapshotId > 0 && snapshotId <= currentSnapshotId, - "Parameter snapshotId has to be greater than 0 and lower/equal currentSnapshot"); + require(snapshotId > 0 && snapshotId <= currentSnapshotId); uint256 idx = snapshotIds[account].findUpperBound(snapshotId); @@ -52,39 +56,69 @@ contract ERC20Snapshot is ERC20 { } } - function transfer( - address to, - uint256 value - ) - public - returns (bool) - { + /** + * @dev Transfer token for a specified address. It takes balance snapshot for the sender and recipient account + * before transfer is done. + * @param to The address to transfer to. + * @param value The amount to be transferred. + */ + function transfer(address to, uint256 value) public returns (bool) { updateSnapshot(msg.sender); updateSnapshot(to); return super.transfer(to, value); } + /** + * @dev Transfer tokens from one address to another. It takes balance snapshot of both accounts before + * the transfer is done. + * @param from address The address which you want to send tokens from + * @param to address The address which you want to transfer to + * @param value uint256 the amount of tokens to be transferred + */ function transferFrom( - address from, - address to, + address from, + address to, uint256 value ) - public - returns (bool) + public + returns (bool) { updateSnapshot(from); updateSnapshot(to); return super.transferFrom(from, to, value); } - function updateSnapshot(address account) internal { + /** + * @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 + * proper events are emitted. Takes snapshot before tokens are minted. + * @param account The account that will receive the created tokens. + * @param amount The amount that will be created. + */ + function _mint(address account, uint256 amount) internal { + updateSnapshot(account); + super._mint(account, amount); + } + + /** + * @dev Internal function that burns an amount of the token of a given + * account. Takes snapshot before tokens are burned. + * @param account The account whose tokens will be burnt. + * @param amount The amount that will be burnt. + */ + function _burn(address account, uint256 amount) internal { + updateSnapshot(account); + super._burn(account, amount); + } + + function updateSnapshot(address account) private { if (lastSnapshotId(account) < currentSnapshotId) { snapshotIds[account].push(currentSnapshotId); snapshotBalances[account].push(balanceOf(account)); } } - function lastSnapshotId(address account) internal view returns (uint256) { + function lastSnapshotId(address account) private view returns (uint256) { uint256[] storage snapshots = snapshotIds[account]; if (snapshots.length == 0) { return 0; diff --git a/contracts/utils/Arrays.sol b/contracts/utils/Arrays.sol index 06fdaa4efb9..8d12dde62ff 100644 --- a/contracts/utils/Arrays.sol +++ b/contracts/utils/Arrays.sol @@ -3,14 +3,22 @@ pragma solidity ^0.4.23; import "../math/Math.sol"; +/** + * @title Arrays + * @dev Utility library of inline array functions + */ library Arrays { + + /** + * @dev Find upper bound for searching element. Utilize binary search function with O(log(n)) cost. + */ function findUpperBound( - uint256[] storage array, + uint256[] storage array, uint256 element ) - internal - view - returns (uint256) + internal + view + returns (uint256) { uint256 low = 0; uint256 high = array.length; diff --git a/test/token/ERC20/ERC20Snapshot.test.js b/test/token/ERC20/ERC20Snapshot.test.js index ba6f4f84a26..65534723791 100644 --- a/test/token/ERC20/ERC20Snapshot.test.js +++ b/test/token/ERC20/ERC20Snapshot.test.js @@ -38,11 +38,6 @@ contract('ERC20Snapshot', function ([_, owner, recipient, spender]) { (await this.token.balanceOf(owner)).should.be.bignumber.equal(90); (await this.token.balanceOf(recipient)).should.be.bignumber.equal(10); }); - - it('snapshots do not exist', async function () { - (await this.token.snapshotsLength(owner)).should.be.bignumber.equal(0); - (await this.token.snapshotsLength(recipient)).should.be.bignumber.equal(0); - }); }); }); @@ -109,5 +104,33 @@ contract('ERC20Snapshot', function ([_, owner, recipient, spender]) { (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); }); }); + + describe('new tokens are minted', function () { + beforeEach(async function () { + await this.token.mint(owner, 50); + }); + + it('snapshot keeps balance before mint', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + }); + + it('current balance is greater after mint', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(150); + }); + }); + + describe('chunk tokens are burned', function () { + beforeEach(async function () { + await this.token.burn(owner, 30); + }); + + it('snapshot keeps balance before burn', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + }); + + it('crrent balance is lower after burn', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(70); + }); + }); }); }); From 21198bf1c1161fabfd0299ecac491f45b702a056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 26 Sep 2018 11:36:41 -0300 Subject: [PATCH 03/33] Roles now emit events in construction and when renouncing. (#1329) * release candidate v2.0.0-rc.1 * fix linter error (cherry picked from commit c12a1c6898c5db300bfeceea50438b328601705d) * Roles now emit events in construction and when renouncing. --- contracts/access/roles/CapperRole.sol | 12 ++++++++---- contracts/access/roles/MinterRole.sol | 12 ++++++++---- contracts/access/roles/PauserRole.sol | 12 ++++++++---- contracts/access/roles/SignerRole.sol | 12 ++++++++---- ethpm.json | 2 +- package-lock.json | 2 +- package.json | 2 +- test/access/roles/PublicRole.behavior.js | 5 +++++ 8 files changed, 40 insertions(+), 19 deletions(-) diff --git a/contracts/access/roles/CapperRole.sol b/contracts/access/roles/CapperRole.sol index 756cdd48fc1..a79e8e694c2 100644 --- a/contracts/access/roles/CapperRole.sol +++ b/contracts/access/roles/CapperRole.sol @@ -11,7 +11,7 @@ contract CapperRole { Roles.Role private cappers; constructor() public { - cappers.add(msg.sender); + _addCapper(msg.sender); } modifier onlyCapper() { @@ -24,12 +24,16 @@ contract CapperRole { } function addCapper(address account) public onlyCapper { - cappers.add(account); - emit CapperAdded(account); + _addCapper(account); } function renounceCapper() public { - cappers.remove(msg.sender); + _removeCapper(msg.sender); + } + + function _addCapper(address account) internal { + cappers.add(account); + emit CapperAdded(account); } function _removeCapper(address account) internal { diff --git a/contracts/access/roles/MinterRole.sol b/contracts/access/roles/MinterRole.sol index cc95473c7ed..d908e3004da 100644 --- a/contracts/access/roles/MinterRole.sol +++ b/contracts/access/roles/MinterRole.sol @@ -11,7 +11,7 @@ contract MinterRole { Roles.Role private minters; constructor() public { - minters.add(msg.sender); + _addMinter(msg.sender); } modifier onlyMinter() { @@ -24,12 +24,16 @@ contract MinterRole { } function addMinter(address account) public onlyMinter { - minters.add(account); - emit MinterAdded(account); + _addMinter(account); } function renounceMinter() public { - minters.remove(msg.sender); + _removeMinter(msg.sender); + } + + function _addMinter(address account) internal { + minters.add(account); + emit MinterAdded(account); } function _removeMinter(address account) internal { diff --git a/contracts/access/roles/PauserRole.sol b/contracts/access/roles/PauserRole.sol index 06347e7cfe9..d59839d6bd0 100644 --- a/contracts/access/roles/PauserRole.sol +++ b/contracts/access/roles/PauserRole.sol @@ -11,7 +11,7 @@ contract PauserRole { Roles.Role private pausers; constructor() public { - pausers.add(msg.sender); + _addPauser(msg.sender); } modifier onlyPauser() { @@ -24,12 +24,16 @@ contract PauserRole { } function addPauser(address account) public onlyPauser { - pausers.add(account); - emit PauserAdded(account); + _addPauser(account); } function renouncePauser() public { - pausers.remove(msg.sender); + _removePauser(msg.sender); + } + + function _addPauser(address account) internal { + pausers.add(account); + emit PauserAdded(account); } function _removePauser(address account) internal { diff --git a/contracts/access/roles/SignerRole.sol b/contracts/access/roles/SignerRole.sol index e2729a25573..bbf8fd36fd6 100644 --- a/contracts/access/roles/SignerRole.sol +++ b/contracts/access/roles/SignerRole.sol @@ -11,7 +11,7 @@ contract SignerRole { Roles.Role private signers; constructor() public { - signers.add(msg.sender); + _addSigner(msg.sender); } modifier onlySigner() { @@ -24,12 +24,16 @@ contract SignerRole { } function addSigner(address account) public onlySigner { - signers.add(account); - emit SignerAdded(account); + _addSigner(account); } function renounceSigner() public { - signers.remove(msg.sender); + _removeSigner(msg.sender); + } + + function _addSigner(address account) internal { + signers.add(account); + emit SignerAdded(account); } function _removeSigner(address account) internal { diff --git a/ethpm.json b/ethpm.json index 27a2947263e..bb2674762cc 100644 --- a/ethpm.json +++ b/ethpm.json @@ -1,6 +1,6 @@ { "package_name": "zeppelin", - "version": "1.12.0", + "version": "2.0.0-rc.1", "description": "Secure Smart Contract library for Solidity", "authors": [ "OpenZeppelin Community " diff --git a/package-lock.json b/package-lock.json index 3fa9ab04e48..29034359c34 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "openzeppelin-solidity", - "version": "1.12.0", + "version": "2.0.0-rc.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index d3eedb5c278..235e500eedb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openzeppelin-solidity", - "version": "1.12.0", + "version": "2.0.0-rc.1", "description": "Secure Smart Contract library for Solidity", "files": [ "build", diff --git a/test/access/roles/PublicRole.behavior.js b/test/access/roles/PublicRole.behavior.js index 1e489a46886..f1c723b7821 100644 --- a/test/access/roles/PublicRole.behavior.js +++ b/test/access/roles/PublicRole.behavior.js @@ -89,6 +89,11 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role (await this.contract[`is${rolename}`](authorized)).should.equal(false); }); + it(`emits a ${rolename}Removed event`, async function () { + const { logs } = await this.contract[`renounce${rolename}`]({ from: authorized }); + expectEvent.inLogs(logs, `${rolename}Removed`, { account: authorized }); + }); + it('doesn\'t revert when renouncing unassigned role', async function () { await this.contract[`renounce${rolename}`]({ from: anyone }); }); From 396680b856d832aa94d28b462c0d5f378265e575 Mon Sep 17 00:00:00 2001 From: Leo Arias Date: Wed, 26 Sep 2018 09:00:08 -0600 Subject: [PATCH 04/33] Add the missing test for ERC721Holder (#1249) * Add the missing test for ERC721Holder * fix lint * Move the holder test to a separate file --- test/token/ERC721/ERC721Holder.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/token/ERC721/ERC721Holder.test.js diff --git a/test/token/ERC721/ERC721Holder.test.js b/test/token/ERC721/ERC721Holder.test.js new file mode 100644 index 00000000000..9f6eb856a13 --- /dev/null +++ b/test/token/ERC721/ERC721Holder.test.js @@ -0,0 +1,19 @@ +const ERC721Holder = artifacts.require('ERC721Holder.sol'); +const ERC721Mintable = artifacts.require('ERC721MintableBurnableImpl.sol'); + +require('chai') + .should(); + +contract('ERC721Holder', function ([creator]) { + it('receives an ERC721 token', async function () { + const token = await ERC721Mintable.new({ from: creator }); + const tokenId = 1; + await token.mint(creator, tokenId, { from: creator }); + + const receiver = await ERC721Holder.new(); + await token.approve(receiver.address, tokenId, { from: creator }); + await token.safeTransferFrom(creator, receiver.address, tokenId); + + (await token.ownerOf(tokenId)).should.be.equal(receiver.address); + }); +}); From db2e1d2c746164f7460c6f311d9dc9d7b916e18a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 26 Sep 2018 12:57:02 -0300 Subject: [PATCH 05/33] Removed assertions from Escrow and SplitPayment. (#1349) --- contracts/payment/Escrow.sol | 1 - contracts/payment/SplitPayment.sol | 1 - 2 files changed, 2 deletions(-) diff --git a/contracts/payment/Escrow.sol b/contracts/payment/Escrow.sol index 21f7d6edd4c..7fb118e3a2c 100644 --- a/contracts/payment/Escrow.sol +++ b/contracts/payment/Escrow.sol @@ -39,7 +39,6 @@ contract Escrow is Secondary { */ function withdraw(address payee) public onlyPrimary { uint256 payment = _deposits[payee]; - assert(address(this).balance >= payment); _deposits[payee] = 0; diff --git a/contracts/payment/SplitPayment.sol b/contracts/payment/SplitPayment.sol index e0e057ae84e..8bfcc7690c3 100644 --- a/contracts/payment/SplitPayment.sol +++ b/contracts/payment/SplitPayment.sol @@ -84,7 +84,6 @@ contract SplitPayment { ); require(payment != 0); - assert(address(this).balance >= payment); _released[account] = _released[account].add(payment); _totalReleased = _totalReleased.add(payment); From 5fdeaa81d5bd68abbb8789c949da32d28209a85a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 26 Sep 2018 16:05:43 -0300 Subject: [PATCH 06/33] Removed mintingFinished. (#1351) * Removed mintingFinished from ERC20Mintable. * Removed MintingFinished from ERC721Mintable. * Removed MintingFinished event. --- contracts/token/ERC20/ERC20Mintable.sol | 32 ----- contracts/token/ERC721/ERC721Mintable.sol | 33 ----- .../ERC20/behaviors/ERC20Mintable.behavior.js | 128 +++--------------- test/token/ERC721/ERC721MintBurn.behavior.js | 29 ---- 4 files changed, 18 insertions(+), 204 deletions(-) diff --git a/contracts/token/ERC20/ERC20Mintable.sol b/contracts/token/ERC20/ERC20Mintable.sol index d6bd9b5eac3..386abf64d27 100644 --- a/contracts/token/ERC20/ERC20Mintable.sol +++ b/contracts/token/ERC20/ERC20Mintable.sol @@ -8,22 +8,6 @@ import "../../access/roles/MinterRole.sol"; * @dev ERC20 minting logic */ contract ERC20Mintable is ERC20, MinterRole { - event MintingFinished(); - - bool private _mintingFinished = false; - - modifier onlyBeforeMintingFinished() { - require(!_mintingFinished); - _; - } - - /** - * @return true if the minting is finished. - */ - function mintingFinished() public view returns(bool) { - return _mintingFinished; - } - /** * @dev Function to mint tokens * @param to The address that will receive the minted tokens. @@ -36,25 +20,9 @@ contract ERC20Mintable is ERC20, MinterRole { ) public onlyMinter - onlyBeforeMintingFinished returns (bool) { _mint(to, amount); return true; } - - /** - * @dev Function to stop minting new tokens. - * @return True if the operation was successful. - */ - function finishMinting() - public - onlyMinter - onlyBeforeMintingFinished - returns (bool) - { - _mintingFinished = true; - emit MintingFinished(); - return true; - } } diff --git a/contracts/token/ERC721/ERC721Mintable.sol b/contracts/token/ERC721/ERC721Mintable.sol index f7cd0e9f94f..9dc0ad75199 100644 --- a/contracts/token/ERC721/ERC721Mintable.sol +++ b/contracts/token/ERC721/ERC721Mintable.sol @@ -8,22 +8,6 @@ import "../../access/roles/MinterRole.sol"; * @dev ERC721 minting logic */ contract ERC721Mintable is ERC721Full, MinterRole { - event MintingFinished(); - - bool private _mintingFinished = false; - - modifier onlyBeforeMintingFinished() { - require(!_mintingFinished); - _; - } - - /** - * @return true if the minting is finished. - */ - function mintingFinished() public view returns(bool) { - return _mintingFinished; - } - /** * @dev Function to mint tokens * @param to The address that will receive the minted tokens. @@ -36,7 +20,6 @@ contract ERC721Mintable is ERC721Full, MinterRole { ) public onlyMinter - onlyBeforeMintingFinished returns (bool) { _mint(to, tokenId); @@ -50,26 +33,10 @@ contract ERC721Mintable is ERC721Full, MinterRole { ) public onlyMinter - onlyBeforeMintingFinished returns (bool) { mint(to, tokenId); _setTokenURI(tokenId, tokenURI); return true; } - - /** - * @dev Function to stop minting new tokens. - * @return True if the operation was successful. - */ - function finishMinting() - public - onlyMinter - onlyBeforeMintingFinished - returns (bool) - { - _mintingFinished = true; - emit MintingFinished(); - return true; - } } diff --git a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js index 5046fc62e50..16126d99aa8 100644 --- a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js @@ -11,136 +11,44 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; describe('as a mintable token', function () { - describe('mintingFinished', function () { - context('when token minting is not finished', function () { - it('returns false', async function () { - (await this.token.mintingFinished()).should.equal(false); - }); - }); - - context('when token minting is finished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from: minter }); - }); - - it('returns true', async function () { - (await this.token.mintingFinished()).should.equal(true); - }); - }); - }); + describe('mint', function () { + const amount = 100; - describe('finishMinting', function () { context('when the sender has minting permission', function () { const from = minter; - context('when token minting was not finished', function () { - it('finishes token minting', async function () { - await this.token.finishMinting({ from }); - - (await this.token.mintingFinished()).should.equal(true); - }); - - it('emits a mint finished event', async function () { - const { logs } = await this.token.finishMinting({ from }); - - await expectEvent.inLogs(logs, 'MintingFinished'); - }); - }); - - context('when token minting was already finished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from }); - }); - - it('reverts', async function () { - await assertRevert(this.token.finishMinting({ from })); - }); + context('for a zero amount', function () { + shouldMint(0); }); - }); - - context('when the sender doesn\'t have minting permission', function () { - const from = anyone; - context('when token minting was not finished', function () { - it('reverts', async function () { - await assertRevert(this.token.finishMinting({ from })); - }); + context('for a non-zero amount', function () { + shouldMint(amount); }); - context('when token minting was already finished', function () { + function shouldMint (amount) { beforeEach(async function () { - await this.token.finishMinting({ from: minter }); - }); - - it('reverts', async function () { - await assertRevert(this.token.finishMinting({ from })); - }); - }); - }); - }); - - describe('mint', function () { - const amount = 100; - - context('when the sender has minting permission', function () { - const from = minter; - - context('when token minting is not finished', function () { - context('for a zero amount', function () { - shouldMint(0); + ({ logs: this.logs } = await this.token.mint(anyone, amount, { from })); }); - context('for a non-zero amount', function () { - shouldMint(amount); + it('mints the requested amount', async function () { + (await this.token.balanceOf(anyone)).should.be.bignumber.equal(amount); }); - function shouldMint (amount) { - beforeEach(async function () { - ({ logs: this.logs } = await this.token.mint(anyone, amount, { from })); - }); - - it('mints the requested amount', async function () { - (await this.token.balanceOf(anyone)).should.be.bignumber.equal(amount); - }); - - it('emits a mint and a transfer event', async function () { - const transferEvent = expectEvent.inLogs(this.logs, 'Transfer', { - from: ZERO_ADDRESS, - to: anyone, - }); - transferEvent.args.value.should.be.bignumber.equal(amount); + it('emits a mint and a transfer event', async function () { + const transferEvent = expectEvent.inLogs(this.logs, 'Transfer', { + from: ZERO_ADDRESS, + to: anyone, }); - } - }); - - context('when token minting is finished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from: minter }); + transferEvent.args.value.should.be.bignumber.equal(amount); }); - - it('reverts', async function () { - await assertRevert(this.token.mint(anyone, amount, { from })); - }); - }); + } }); context('when the sender doesn\'t have minting permission', function () { const from = anyone; - context('when token minting is not finished', function () { - it('reverts', async function () { - await assertRevert(this.token.mint(anyone, amount, { from })); - }); - }); - - context('when token minting is already finished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from: minter }); - }); - - it('reverts', async function () { - await assertRevert(this.token.mint(anyone, amount, { from })); - }); + it('reverts', async function () { + await assertRevert(this.token.mint(anyone, amount, { from })); }); }); }); diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index cadfe87cc38..31db1d1767e 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -117,35 +117,6 @@ function shouldBehaveLikeMintAndBurnERC721 ( }); }); }); - - describe('finishMinting', function () { - it('allows the minter to finish minting', async function () { - const { logs } = await this.token.finishMinting({ from: minter }); - expectEvent.inLogs(logs, 'MintingFinished'); - }); - }); - - context('mintingFinished', function () { - beforeEach(async function () { - await this.token.finishMinting({ from: minter }); - }); - - describe('mint', function () { - it('reverts', async function () { - await assertRevert( - this.token.mint(owner, thirdTokenId, { from: minter }) - ); - }); - }); - - describe('mintWithTokenURI', function () { - it('reverts', async function () { - await assertRevert( - this.token.mintWithTokenURI(owner, thirdTokenId, MOCK_URI, { from: minter }) - ); - }); - }); - }); }); } From ae109f69ccf4b7264d8ca3dc0d2c9b658744bc0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 26 Sep 2018 16:32:50 -0300 Subject: [PATCH 07/33] Improved bounty tests. (#1350) * Improved bounty tests. * Fixed linter errors. * Addressed review comments. --- ...ounty.sol => BreakInvariantBountyMock.sol} | 18 ++- .../mocks/InsecureInvariantTargetBounty.sol | 18 --- test/BreakInvariantBounty.test.js | 147 +++++++++--------- 3 files changed, 91 insertions(+), 92 deletions(-) rename contracts/mocks/{SecureInvariantTargetBounty.sol => BreakInvariantBountyMock.sol} (51%) delete mode 100644 contracts/mocks/InsecureInvariantTargetBounty.sol diff --git a/contracts/mocks/SecureInvariantTargetBounty.sol b/contracts/mocks/BreakInvariantBountyMock.sol similarity index 51% rename from contracts/mocks/SecureInvariantTargetBounty.sol rename to contracts/mocks/BreakInvariantBountyMock.sol index 9f2525bf0ef..9343ef01269 100644 --- a/contracts/mocks/SecureInvariantTargetBounty.sol +++ b/contracts/mocks/BreakInvariantBountyMock.sol @@ -5,14 +5,24 @@ pragma solidity ^0.4.24; // solium-disable-next-line max-len import {BreakInvariantBounty, Target} from "../bounties/BreakInvariantBounty.sol"; -contract SecureInvariantTargetMock is Target { - function checkInvariant() public returns(bool) { +contract TargetMock is Target { + bool private exploited; + + function exploitVulnerability() public { + exploited = true; + } + + function checkInvariant() public returns (bool) { + if (exploited) { + return false; + } + return true; } } -contract SecureInvariantTargetBounty is BreakInvariantBounty { +contract BreakInvariantBountyMock is BreakInvariantBounty { function _deployContract() internal returns (address) { - return new SecureInvariantTargetMock(); + return new TargetMock(); } } diff --git a/contracts/mocks/InsecureInvariantTargetBounty.sol b/contracts/mocks/InsecureInvariantTargetBounty.sol deleted file mode 100644 index cac749d6359..00000000000 --- a/contracts/mocks/InsecureInvariantTargetBounty.sol +++ /dev/null @@ -1,18 +0,0 @@ -pragma solidity ^0.4.24; - -// When this line is split, truffle parsing fails. -// See: https://github.com/ethereum/solidity/issues/4871 -// solium-disable-next-line max-len -import {BreakInvariantBounty, Target} from "../bounties/BreakInvariantBounty.sol"; - -contract InsecureInvariantTargetMock is Target { - function checkInvariant() public returns(bool) { - return false; - } -} - -contract InsecureInvariantTargetBounty is BreakInvariantBounty { - function _deployContract() internal returns (address) { - return new InsecureInvariantTargetMock(); - } -} diff --git a/test/BreakInvariantBounty.test.js b/test/BreakInvariantBounty.test.js index 94a5fa0f7c7..a29e85080ed 100644 --- a/test/BreakInvariantBounty.test.js +++ b/test/BreakInvariantBounty.test.js @@ -2,97 +2,104 @@ const { ethGetBalance, ethSendTransaction } = require('./helpers/web3'); const expectEvent = require('./helpers/expectEvent'); const { assertRevert } = require('./helpers/assertRevert'); -const SecureInvariantTargetBounty = artifacts.require('SecureInvariantTargetBounty'); -const InsecureInvariantTargetBounty = artifacts.require('InsecureInvariantTargetBounty'); +const BreakInvariantBountyMock = artifacts.require('BreakInvariantBountyMock'); +const TargetMock = artifacts.require('TargetMock'); require('chai') .use(require('chai-bignumber')(web3.BigNumber)) .should(); -const sendReward = async (from, to, value) => ethSendTransaction({ - from, - to, - value, -}); - const reward = new web3.BigNumber(web3.toWei(1, 'ether')); -contract('BreakInvariantBounty', function ([_, owner, researcher, nonTarget]) { - context('against secure contract', function () { - beforeEach(async function () { - this.bounty = await SecureInvariantTargetBounty.new({ from: owner }); - }); +contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTarget]) { + beforeEach(async function () { + this.bounty = await BreakInvariantBountyMock.new({ from: owner }); + }); - it('can set reward', async function () { - await sendReward(owner, this.bounty.address, reward); + it('can set reward', async function () { + await ethSendTransaction({ from: owner, to: this.bounty.address, value: reward }); + (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(reward); + }); - const balance = await ethGetBalance(this.bounty.address); - balance.should.be.bignumber.equal(reward); + context('with reward', function () { + beforeEach(async function () { + await ethSendTransaction({ from: owner, to: this.bounty.address, value: reward }); }); - context('with reward', function () { - beforeEach(async function () { - const result = await this.bounty.createTarget({ from: researcher }); - const event = expectEvent.inLogs(result.logs, 'TargetCreated'); - - this.targetAddress = event.args.createdAddress; - - await sendReward(owner, this.bounty.address, reward); + describe('destroy', function () { + it('returns all balance to the owner', async function () { + const ownerPreBalance = await ethGetBalance(owner); + await this.bounty.destroy({ from: owner, gasPrice: 0 }); + const ownerPostBalance = await ethGetBalance(owner); - const balance = await ethGetBalance(this.bounty.address); - balance.should.be.bignumber.equal(reward); + (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0); + ownerPostBalance.sub(ownerPreBalance).should.be.bignumber.equal(reward); }); - it('cannot claim reward', async function () { - await assertRevert( - this.bounty.claim(this.targetAddress, { from: researcher }), - ); + it('reverts when called by anyone', async function () { + await assertRevert(this.bounty.destroy({ from: anyone })); }); }); - }); - - context('against broken contract', function () { - beforeEach(async function () { - this.bounty = await InsecureInvariantTargetBounty.new(); - const result = await this.bounty.createTarget({ from: researcher }); - const event = expectEvent.inLogs(result.logs, 'TargetCreated'); - - this.targetAddress = event.args.createdAddress; - await sendReward(owner, this.bounty.address, reward); - }); - - it('can claim reward', async function () { - await this.bounty.claim(this.targetAddress, { from: researcher }); - const claim = await this.bounty.claimed(); - - claim.should.equal(true); - - const researcherPrevBalance = await ethGetBalance(researcher); - - await this.bounty.withdrawPayments(researcher, { gasPrice: 0 }); - const updatedBalance = await ethGetBalance(this.bounty.address); - updatedBalance.should.be.bignumber.equal(0); - - const researcherCurrBalance = await ethGetBalance(researcher); - researcherCurrBalance.sub(researcherPrevBalance).should.be.bignumber.equal(reward); - }); + describe('claim', function () { + it('is initially unclaimed', async function () { + (await this.bounty.claimed()).should.equal(false); + }); - it('cannot claim reward from non-target', async function () { - await assertRevert( - this.bounty.claim(nonTarget, { from: researcher }) - ); - }); + it('can create claimable target', async function () { + const { logs } = await this.bounty.createTarget({ from: researcher }); + expectEvent.inLogs(logs, 'TargetCreated'); + }); - context('reward claimed', function () { - beforeEach(async function () { - await this.bounty.claim(this.targetAddress, { from: researcher }); + context('with target', async function () { + beforeEach(async function () { + const { logs } = await this.bounty.createTarget({ from: researcher }); + const event = expectEvent.inLogs(logs, 'TargetCreated'); + this.target = TargetMock.at(event.args.createdAddress); + }); + + context('before exploiting vulnerability', async function () { + it('reverts when claiming reward', async function () { + await assertRevert(this.bounty.claim(this.target.address, { from: researcher })); + }); + }); + + context('after exploiting vulnerability', async function () { + beforeEach(async function () { + await this.target.exploitVulnerability({ from: researcher }); + }); + + it('sends the reward to the researcher', async function () { + await this.bounty.claim(this.target.address, { from: anyone }); + + const researcherPreBalance = await ethGetBalance(researcher); + await this.bounty.withdrawPayments(researcher); + const researcherPostBalance = await ethGetBalance(researcher); + + researcherPostBalance.sub(researcherPreBalance).should.be.bignumber.equal(reward); + (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0); + }); + + context('after claiming', async function () { + beforeEach(async function () { + await this.bounty.claim(this.target.address, { from: researcher }); + }); + + it('is claimed', async function () { + (await this.bounty.claimed()).should.equal(true); + }); + + it('no longer accepts rewards', async function () { + await assertRevert(ethSendTransaction({ from: owner, to: this.bounty.address, value: reward })); + }); + }); + }); }); - it('should no longer be payable', async function () { - await assertRevert( - sendReward(owner, this.bounty.address, reward) - ); + context('with non-target', function () { + it('reverts when claiming reward', async function () { + await assertRevert(this.bounty.claim(nonTarget, { from: researcher })); + }); }); }); }); From 947de54ceea3abe4783ac2e417b6d9bfb93d708e Mon Sep 17 00:00:00 2001 From: Jakub Bogacz Date: Wed, 26 Sep 2018 22:04:41 +0200 Subject: [PATCH 08/33] Add BigNumber support to expectEvent/inLogs (#1026) (#1338) * Add BigNumber support to expectEvent/inLogs (#1026) * switched direct logs array check to expectEvent method in AllowanceCrowdsale.test.js * Refactor expectEvent.inLogs function to use simple value number check * Introduced should.be.bignumber method to compare BigNumber values * Destructure transaction object to extract logs field --- test/crowdsale/AllowanceCrowdsale.test.js | 17 +++++++----- test/helpers/expectEvent.js | 32 ++++++++++++++--------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/test/crowdsale/AllowanceCrowdsale.test.js b/test/crowdsale/AllowanceCrowdsale.test.js index c35707d6d4c..74f26bbf1e4 100644 --- a/test/crowdsale/AllowanceCrowdsale.test.js +++ b/test/crowdsale/AllowanceCrowdsale.test.js @@ -1,10 +1,11 @@ +const expectEvent = require('../helpers/expectEvent'); const { ether } = require('../helpers/ether'); const { assertRevert } = require('../helpers/assertRevert'); const { ethGetBalance } = require('../helpers/web3'); const BigNumber = web3.BigNumber; -const should = require('chai') +require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); @@ -41,12 +42,14 @@ contract('AllowanceCrowdsale', function ([_, investor, wallet, purchaser, tokenW describe('high-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor }); - const event = logs.find(e => e.event === 'TokensPurchased'); - should.exist(event); - event.args.purchaser.should.equal(investor); - event.args.beneficiary.should.equal(investor); - event.args.value.should.be.bignumber.equal(value); - event.args.amount.should.be.bignumber.equal(expectedTokenAmount); + expectEvent.inLogs( + logs, + 'TokensPurchased', + { + purchaser: investor, + beneficiary: investor, + value: value, + amount: expectedTokenAmount }); }); it('should assign tokens to sender', async function () { diff --git a/test/helpers/expectEvent.js b/test/helpers/expectEvent.js index 37fa1083eb3..b857b6636bf 100644 --- a/test/helpers/expectEvent.js +++ b/test/helpers/expectEvent.js @@ -1,24 +1,18 @@ -const should = require('chai').should(); +const BigNumber = web3.BigNumber; +const should = require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); function inLogs (logs, eventName, eventArgs = {}) { const event = logs.find(function (e) { if (e.event === eventName) { - let matches = true; - for (const [k, v] of Object.entries(eventArgs)) { - if (e.args[k] !== v) { - matches = false; - } - } - - if (matches) { - return true; + contains(e.args, k, v); } + return true; } }); - should.exist(event); - return event; } @@ -27,6 +21,20 @@ async function inTransaction (tx, eventName, eventArgs = {}) { return inLogs(logs, eventName, eventArgs); } +function contains (args, key, value) { + if (isBigNumber(args[key])) { + args[key].should.be.bignumber.equal(value); + } else { + args[key].should.be.equal(value); + } +} + +function isBigNumber (object) { + return object.isBigNumber || + object instanceof BigNumber || + (object.constructor && object.constructor.name === 'BigNumber'); +} + module.exports = { inLogs, inTransaction, From 75c0a59bb484eb0f04ec76eb8fa287d758af8ee5 Mon Sep 17 00:00:00 2001 From: Leo Arias Date: Wed, 26 Sep 2018 14:36:45 -0600 Subject: [PATCH 09/33] Add missing tests to ECSDA (#1248) * fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly * Add missing tests to ECRecovery * fix lint * Reorganize the tests * Reuse signature * fix static errors * Apply suggestions by @frangion and @nventuro * Remove only * More suggestions * Remove unnecessary max-len * remove only --- test/library/ECDSA.test.js | 135 +++++++++++++++++++++++++++---------- 1 file changed, 98 insertions(+), 37 deletions(-) diff --git a/test/library/ECDSA.test.js b/test/library/ECDSA.test.js index 7fe7056538a..f8d333a5f05 100644 --- a/test/library/ECDSA.test.js +++ b/test/library/ECDSA.test.js @@ -11,56 +11,117 @@ const WRONG_MESSAGE = web3.sha3('Nope'); contract('ECDSA', function ([_, anyone]) { beforeEach(async function () { - this.mock = await ECDSAMock.new(); + this.ecdsa = await ECDSAMock.new(); }); - it('recover v0', async function () { - // Signature generated outside ganache with method web3.eth.sign(signer, message) - const signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c'; - // eslint-disable-next-line max-len - const signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be89200'; - (await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer); - }); + context('recover with valid signature', function () { + context('with v0 signature', function () { + // Signature generated outside ganache with method web3.eth.sign(signer, message) + const signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c'; + // eslint-disable-next-line max-len + const signatureWithoutVersion = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892'; - it('recover v1', async function () { - // Signature generated outside ganache with method web3.eth.sign(signer, message) - const signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e'; - // eslint-disable-next-line max-len - const signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e001'; - (await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer); - }); + context('with 00 as version value', function () { + it('works', async function () { + const version = '00'; + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + }); + }); - it('recover using web3.eth.sign()', async function () { - // Create the signature - const signature = signMessage(anyone, TEST_MESSAGE); + context('with 27 as version value', function () { + it('works', async function () { + const version = '1b'; // 27 = 1b. + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + }); + }); - // Recover the signer address from the generated message and signature. - (await this.mock.recover( - toEthSignedMessageHash(TEST_MESSAGE), - signature - )).should.equal(anyone); - }); + context('with wrong version', function () { + it('returns 0', async function () { + // The last two hex digits are the signature version. + // The only valid values are 0, 1, 27 and 28. + const version = '02'; + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( + '0x0000000000000000000000000000000000000000'); + }); + }); + }); - it('recover using web3.eth.sign() should return wrong signer', async function () { - // Create the signature - const signature = signMessage(anyone, TEST_MESSAGE); + context('with v1 signature', function () { + const signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e'; + // eslint-disable-next-line max-len + const signatureWithoutVersion = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0'; - // Recover the signer address from the generated message and wrong signature. - (await this.mock.recover(WRONG_MESSAGE, signature)).should.not.equal(anyone); + context('with 01 as version value', function () { + it('works', async function () { + const version = '01'; + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + }); + }); + + context('with 28 signature', function () { + it('works', async function () { + const version = '1c'; // 28 = 1c. + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer); + }); + }); + + context('with wrong version', function () { + it('returns 0', async function () { + // The last two hex digits are the signature version. + // The only valid values are 0, 1, 27 and 28. + const version = '02'; + const signature = signatureWithoutVersion + version; + (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal( + '0x0000000000000000000000000000000000000000'); + }); + }); + }); + + context('using web3.eth.sign', function () { + context('with correct signature', function () { + it('returns signer address', async function () { + // Create the signature + const signature = signMessage(anyone, TEST_MESSAGE); + + // Recover the signer address from the generated message and signature. + (await this.ecdsa.recover( + toEthSignedMessageHash(TEST_MESSAGE), + signature + )).should.equal(anyone); + }); + }); + + context('with wrong signature', function () { + it('does not return signer address', async function () { + // Create the signature + const signature = signMessage(anyone, TEST_MESSAGE); + + // Recover the signer address from the generated message and wrong signature. + (await this.ecdsa.recover(WRONG_MESSAGE, signature)).should.not.equal(anyone); + }); + }); + }); }); - // @TODO - remove `skip` once we upgrade to solc^0.5 - it.skip('recover should revert when a small hash is sent', async function () { - // Create the signature - const signature = signMessage(anyone, TEST_MESSAGE); - await expectThrow( - this.mock.recover(TEST_MESSAGE.substring(2), signature) - ); + context('with small hash', function () { + // @TODO - remove `skip` once we upgrade to solc^0.5 + it.skip('reverts', async function () { + // Create the signature + const signature = signMessage(anyone, TEST_MESSAGE); + await expectThrow( + this.ecdsa.recover(TEST_MESSAGE.substring(2), signature) + ); + }); }); context('toEthSignedMessage', function () { it('should prefix hashes correctly', async function () { - (await this.mock.toEthSignedMessageHash(TEST_MESSAGE)).should.equal(toEthSignedMessageHash(TEST_MESSAGE)); + (await this.ecdsa.toEthSignedMessageHash(TEST_MESSAGE)).should.equal(toEthSignedMessageHash(TEST_MESSAGE)); }); }); }); From 536262f2ec5d6103129283a023b467d8887adce3 Mon Sep 17 00:00:00 2001 From: Jakub Bogacz Date: Thu, 27 Sep 2018 16:33:28 +0200 Subject: [PATCH 10/33] Feature/use expect event in test logs assertions #1232 (#1343) * Add BigNumber support to expectEvent/inLogs (#1026) * switched direct logs array check to expectEvent method in AllowanceCrowdsale.test.js * Refactor expectEvent.inLogs function to use simple value number check * Introduced should.be.bignumber method to compare BigNumber values * Use expectEvent to test logs (#1232) * Removed trailing space --- test/crowdsale/AllowanceCrowdsale.test.js | 14 ++-- test/crowdsale/Crowdsale.test.js | 27 +++---- test/crowdsale/FinalizableCrowdsale.test.js | 6 +- test/crowdsale/MintedCrowdsale.behavior.js | 15 ++-- test/payment/Escrow.behavior.js | 18 ++--- test/payment/RefundEscrow.test.js | 10 ++- test/token/ERC20/ERC20.test.js | 65 +++++++++-------- test/token/ERC20/ERC20Pausable.test.js | 7 +- .../ERC20/behaviors/ERC20Burnable.behavior.js | 18 ++--- .../ERC20/behaviors/ERC20Mintable.behavior.js | 4 +- test/token/ERC721/ERC721.behavior.js | 71 ++++++++++--------- test/token/ERC721/ERC721MintBurn.behavior.js | 14 ++-- 12 files changed, 135 insertions(+), 134 deletions(-) diff --git a/test/crowdsale/AllowanceCrowdsale.test.js b/test/crowdsale/AllowanceCrowdsale.test.js index 74f26bbf1e4..8e57a841b24 100644 --- a/test/crowdsale/AllowanceCrowdsale.test.js +++ b/test/crowdsale/AllowanceCrowdsale.test.js @@ -42,14 +42,12 @@ contract('AllowanceCrowdsale', function ([_, investor, wallet, purchaser, tokenW describe('high-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor }); - expectEvent.inLogs( - logs, - 'TokensPurchased', - { - purchaser: investor, - beneficiary: investor, - value: value, - amount: expectedTokenAmount }); + expectEvent.inLogs(logs, 'TokensPurchased', { + purchaser: investor, + beneficiary: investor, + value: value, + amount: expectedTokenAmount + }); }); it('should assign tokens to sender', async function () { diff --git a/test/crowdsale/Crowdsale.test.js b/test/crowdsale/Crowdsale.test.js index 556855796d4..f691c951576 100644 --- a/test/crowdsale/Crowdsale.test.js +++ b/test/crowdsale/Crowdsale.test.js @@ -1,10 +1,11 @@ +const expectEvent = require('../helpers/expectEvent'); const { assertRevert } = require('../helpers/assertRevert'); const { ether } = require('../helpers/ether'); const { ethGetBalance } = require('../helpers/web3'); const BigNumber = web3.BigNumber; -const should = require('chai') +require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); @@ -82,12 +83,12 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { describe('high-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor }); - const event = logs.find(e => e.event === 'TokensPurchased'); - should.exist(event); - event.args.purchaser.should.equal(investor); - event.args.beneficiary.should.equal(investor); - event.args.value.should.be.bignumber.equal(value); - event.args.amount.should.be.bignumber.equal(expectedTokenAmount); + expectEvent.inLogs(logs, 'TokensPurchased', { + purchaser: investor, + beneficiary: investor, + value: value, + amount: expectedTokenAmount + }); }); it('should assign tokens to sender', async function () { @@ -106,12 +107,12 @@ contract('Crowdsale', function ([_, investor, wallet, purchaser]) { describe('low-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.buyTokens(investor, { value: value, from: purchaser }); - const event = logs.find(e => e.event === 'TokensPurchased'); - should.exist(event); - event.args.purchaser.should.equal(purchaser); - event.args.beneficiary.should.equal(investor); - event.args.value.should.be.bignumber.equal(value); - event.args.amount.should.be.bignumber.equal(expectedTokenAmount); + expectEvent.inLogs(logs, 'TokensPurchased', { + purchaser: purchaser, + beneficiary: investor, + value: value, + amount: expectedTokenAmount + }); }); it('should assign tokens to beneficiary', async function () { diff --git a/test/crowdsale/FinalizableCrowdsale.test.js b/test/crowdsale/FinalizableCrowdsale.test.js index 187c67a36f8..b024fe1d43f 100644 --- a/test/crowdsale/FinalizableCrowdsale.test.js +++ b/test/crowdsale/FinalizableCrowdsale.test.js @@ -1,3 +1,4 @@ +const expectEvent = require('../helpers/expectEvent'); const { advanceBlock } = require('../helpers/advanceToBlock'); const { increaseTimeTo, duration } = require('../helpers/increaseTime'); const { latestTime } = require('../helpers/latestTime'); @@ -6,7 +7,7 @@ const { EVMRevert } = require('../helpers/EVMRevert'); const BigNumber = web3.BigNumber; -const should = require('chai') +require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); @@ -50,7 +51,6 @@ contract('FinalizableCrowdsale', function ([_, wallet, anyone]) { it('logs finalized', async function () { await increaseTimeTo(this.afterClosingTime); const { logs } = await this.crowdsale.finalize({ from: anyone }); - const event = logs.find(e => e.event === 'CrowdsaleFinalized'); - should.exist(event); + expectEvent.inLogs(logs, 'CrowdsaleFinalized'); }); }); diff --git a/test/crowdsale/MintedCrowdsale.behavior.js b/test/crowdsale/MintedCrowdsale.behavior.js index f5d61328c3a..c55850cd9a4 100644 --- a/test/crowdsale/MintedCrowdsale.behavior.js +++ b/test/crowdsale/MintedCrowdsale.behavior.js @@ -1,8 +1,9 @@ +const expectEvent = require('../helpers/expectEvent'); const { ethGetBalance } = require('../helpers/web3'); const BigNumber = web3.BigNumber; -const should = require('chai') +require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); @@ -20,12 +21,12 @@ function shouldBehaveLikeMintedCrowdsale ([_, investor, wallet, purchaser], rate describe('high-level purchase', function () { it('should log purchase', async function () { const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor }); - const event = logs.find(e => e.event === 'TokensPurchased'); - should.exist(event); - event.args.purchaser.should.equal(investor); - event.args.beneficiary.should.equal(investor); - event.args.value.should.be.bignumber.equal(value); - event.args.amount.should.be.bignumber.equal(expectedTokenAmount); + expectEvent.inLogs(logs, 'TokensPurchased', { + purchaser: investor, + beneficiary: investor, + value: value, + amount: expectedTokenAmount + }); }); it('should assign tokens to sender', async function () { diff --git a/test/payment/Escrow.behavior.js b/test/payment/Escrow.behavior.js index 761aecd890d..fce4bd69217 100644 --- a/test/payment/Escrow.behavior.js +++ b/test/payment/Escrow.behavior.js @@ -31,10 +31,11 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { }); it('emits a deposited event', async function () { - const receipt = await this.escrow.deposit(payee1, { from: primary, value: amount }); - - const event = expectEvent.inLogs(receipt.logs, 'Deposited', { payee: payee1 }); - event.args.weiAmount.should.be.bignumber.equal(amount); + const { logs } = await this.escrow.deposit(payee1, { from: primary, value: amount }); + expectEvent.inLogs(logs, 'Deposited', { + payee: payee1, + weiAmount: amount + }); }); it('can add multiple deposits on a single account', async function () { @@ -83,10 +84,11 @@ function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { it('emits a withdrawn event', async function () { await this.escrow.deposit(payee1, { from: primary, value: amount }); - const receipt = await this.escrow.withdraw(payee1, { from: primary }); - - const event = expectEvent.inLogs(receipt.logs, 'Withdrawn', { payee: payee1 }); - event.args.weiAmount.should.be.bignumber.equal(amount); + const { logs } = await this.escrow.withdraw(payee1, { from: primary }); + expectEvent.inLogs(logs, 'Withdrawn', { + payee: payee1, + weiAmount: amount + }); }); }); }); diff --git a/test/payment/RefundEscrow.test.js b/test/payment/RefundEscrow.test.js index e412816c405..c499f4009dd 100644 --- a/test/payment/RefundEscrow.test.js +++ b/test/payment/RefundEscrow.test.js @@ -53,9 +53,8 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee it('only the primary account can enter closed state', async function () { await expectThrow(this.escrow.close({ from: beneficiary }), EVMRevert); - const receipt = await this.escrow.close({ from: primary }); - - expectEvent.inLogs(receipt.logs, 'Closed'); + const { logs } = await this.escrow.close({ from: primary }); + expectEvent.inLogs(logs, 'Closed'); }); context('closed state', function () { @@ -93,9 +92,8 @@ contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee it('only the primary account can enter refund state', async function () { await expectThrow(this.escrow.enableRefunds({ from: beneficiary }), EVMRevert); - const receipt = await this.escrow.enableRefunds({ from: primary }); - - expectEvent.inLogs(receipt.logs, 'RefundsEnabled'); + const { logs } = await this.escrow.enableRefunds({ from: primary }); + expectEvent.inLogs(logs, 'RefundsEnabled'); }); context('refund state', function () { diff --git a/test/token/ERC20/ERC20.test.js b/test/token/ERC20/ERC20.test.js index 3662a41a795..283e8752a09 100644 --- a/test/token/ERC20/ERC20.test.js +++ b/test/token/ERC20/ERC20.test.js @@ -62,12 +62,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits a transfer event', async function () { const { logs } = await this.token.transfer(to, amount, { from: owner }); - const event = expectEvent.inLogs(logs, 'Transfer', { + expectEvent.inLogs(logs, 'Transfer', { from: owner, to: to, + value: amount }); - - event.args.value.should.be.bignumber.equal(amount); }); }); }); @@ -91,11 +90,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.approve(spender, amount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount + }); }); describe('when there was no approved amount before', function () { @@ -125,11 +124,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.approve(spender, amount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount + }); }); describe('when there was no approved amount before', function () { @@ -195,11 +194,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits a transfer event', async function () { const { logs } = await this.token.transferFrom(owner, to, amount, { from: spender }); - logs.length.should.equal(1); - logs[0].event.should.equal('Transfer'); - logs[0].args.from.should.equal(owner); - logs[0].args.to.should.equal(to); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: to, + value: amount + }); }); }); @@ -270,11 +269,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.decreaseAllowance(spender, approvedAmount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(0); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: 0 + }); }); it('decreases the spender allowance subtracting the requested amount', async function () { @@ -327,11 +326,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.increaseAllowance(spender, amount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount + }); }); describe('when there was no approved amount before', function () { @@ -361,11 +360,11 @@ contract('ERC20', function ([_, owner, recipient, anotherAccount]) { it('emits an approval event', async function () { const { logs } = await this.token.increaseAllowance(spender, amount, { from: owner }); - logs.length.should.equal(1); - logs[0].event.should.equal('Approval'); - logs[0].args.owner.should.equal(owner); - logs[0].args.spender.should.equal(spender); - logs[0].args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + spender: spender, + value: amount + }); }); describe('when there was no approved amount before', function () { diff --git a/test/token/ERC20/ERC20Pausable.test.js b/test/token/ERC20/ERC20Pausable.test.js index fa733a3f61e..a524e2442ce 100644 --- a/test/token/ERC20/ERC20Pausable.test.js +++ b/test/token/ERC20/ERC20Pausable.test.js @@ -1,3 +1,4 @@ +const expectEvent = require('../../helpers/expectEvent'); const { assertRevert } = require('../../helpers/assertRevert'); const ERC20PausableMock = artifacts.require('ERC20PausableMock'); @@ -30,8 +31,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA it('emits a Pause event', async function () { const { logs } = await this.token.pause({ from }); - logs.length.should.equal(1); - logs[0].event.should.equal('Paused'); + expectEvent.inLogs(logs, 'Paused'); }); }); @@ -72,8 +72,7 @@ contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherA it('emits an Unpause event', async function () { const { logs } = await this.token.unpause({ from }); - logs.length.should.equal(1); - logs[0].event.should.equal('Unpaused'); + expectEvent.inLogs(logs, 'Unpaused'); }); }); diff --git a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js index cf0190046f3..9c7bada7acc 100644 --- a/test/token/ERC20/behaviors/ERC20Burnable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Burnable.behavior.js @@ -29,10 +29,11 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { }); it('emits a transfer event', async function () { - const event = expectEvent.inLogs(this.logs, 'Transfer'); - event.args.from.should.equal(owner); - event.args.to.should.equal(ZERO_ADDRESS); - event.args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(this.logs, 'Transfer', { + from: owner, + to: ZERO_ADDRESS, + value: amount + }); }); } }); @@ -74,10 +75,11 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) { }); it('emits a transfer event', async function () { - const event = expectEvent.inLogs(this.logs, 'Transfer'); - event.args.from.should.equal(owner); - event.args.to.should.equal(ZERO_ADDRESS); - event.args.value.should.be.bignumber.equal(amount); + expectEvent.inLogs(this.logs, 'Transfer', { + from: owner, + to: ZERO_ADDRESS, + value: amount + }); }); } }); diff --git a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js index 16126d99aa8..07b3837b086 100644 --- a/test/token/ERC20/behaviors/ERC20Mintable.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Mintable.behavior.js @@ -35,11 +35,11 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) { }); it('emits a mint and a transfer event', async function () { - const transferEvent = expectEvent.inLogs(this.logs, 'Transfer', { + expectEvent.inLogs(this.logs, 'Transfer', { from: ZERO_ADDRESS, to: anyone, + value: amount }); - transferEvent.args.value.should.be.bignumber.equal(amount); }); } }); diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js index ecdc3352f3b..d2cefe743c7 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -1,3 +1,4 @@ +const expectEvent = require('../../helpers/expectEvent'); const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior'); const { assertRevert } = require('../../helpers/assertRevert'); const { decodeLogs } = require('../../helpers/decodeLogs'); @@ -88,19 +89,19 @@ function shouldBehaveLikeERC721 ( if (approved) { it('emit only a transfer event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Transfer'); - logs[0].args.from.should.be.equal(owner); - logs[0].args.to.should.be.equal(this.toWhom); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: this.toWhom, + tokenId: tokenId + }); }); } else { it('emits only a transfer event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Transfer'); - logs[0].args.from.should.be.equal(owner); - logs[0].args.to.should.be.equal(this.toWhom); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: this.toWhom, + tokenId: tokenId + }); }); } @@ -161,11 +162,11 @@ function shouldBehaveLikeERC721 ( }); it('emits only a transfer event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Transfer'); - logs[0].args.from.should.be.equal(owner); - logs[0].args.to.should.be.equal(owner); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: owner, + tokenId: tokenId + }); }); it('keeps the owner balance', async function () { @@ -337,11 +338,11 @@ function shouldBehaveLikeERC721 ( const itEmitsApprovalEvent = function (address) { it('emits an approval event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Approval'); - logs[0].args.owner.should.be.equal(owner); - logs[0].args.approved.should.be.equal(address); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Approval', { + owner: owner, + approved: address, + tokenId: tokenId + }); }); }; @@ -447,11 +448,11 @@ function shouldBehaveLikeERC721 ( it('emits an approval event', async function () { const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner }); - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args.owner.should.be.equal(owner); - logs[0].args.operator.should.be.equal(operator); - logs[0].args.approved.should.equal(true); + expectEvent.inLogs(logs, 'ApprovalForAll', { + owner: owner, + operator: operator, + approved: true + }); }); }); @@ -469,11 +470,11 @@ function shouldBehaveLikeERC721 ( it('emits an approval event', async function () { const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner }); - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args.owner.should.be.equal(owner); - logs[0].args.operator.should.be.equal(operator); - logs[0].args.approved.should.equal(true); + expectEvent.inLogs(logs, 'ApprovalForAll', { + owner: owner, + operator: operator, + approved: true + }); }); it('can unset the operator approval', async function () { @@ -497,11 +498,11 @@ function shouldBehaveLikeERC721 ( it('emits an approval event', async function () { const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner }); - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('ApprovalForAll'); - logs[0].args.owner.should.be.equal(owner); - logs[0].args.operator.should.be.equal(operator); - logs[0].args.approved.should.equal(true); + expectEvent.inLogs(logs, 'ApprovalForAll', { + owner: owner, + operator: operator, + approved: true + }); }); }); }); diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index 31db1d1767e..a4289d27a74 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -42,11 +42,11 @@ function shouldBehaveLikeMintAndBurnERC721 ( }); it('emits a transfer and minted event', async function () { - await expectEvent.inLogs(logs, 'Transfer', { + expectEvent.inLogs(logs, 'Transfer', { from: ZERO_ADDRESS, to: newOwner, + tokenId: thirdTokenId }); - logs[0].args.tokenId.should.be.bignumber.equal(thirdTokenId); }); }); @@ -87,11 +87,11 @@ function shouldBehaveLikeMintAndBurnERC721 ( }); it('emits a burn event', async function () { - logs.length.should.be.equal(1); - logs[0].event.should.be.equal('Transfer'); - logs[0].args.from.should.be.equal(owner); - logs[0].args.to.should.be.equal(ZERO_ADDRESS); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: ZERO_ADDRESS, + tokenId: tokenId + }); }); }); From 1a4e5346ed40e5c02f5ca75d3cdfccb694c641d9 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 27 Sep 2018 19:40:18 -0300 Subject: [PATCH 11/33] Add SignatureBouncer test for when msg.data is too short (#1360) * add test for msg.data too short * fix test to hit that branch * Update SignatureBouncer.test.js --- contracts/mocks/SignatureBouncerMock.sol | 7 +++++++ test/drafts/SignatureBouncer.test.js | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/contracts/mocks/SignatureBouncerMock.sol b/contracts/mocks/SignatureBouncerMock.sol index ad949403385..85076eae6ea 100644 --- a/contracts/mocks/SignatureBouncerMock.sol +++ b/contracts/mocks/SignatureBouncerMock.sol @@ -63,4 +63,11 @@ contract SignatureBouncerMock is SignatureBouncer, SignerRoleMock { { } + + function tooShortMsgData() + public + onlyValidSignatureAndData("") + view + { + } } diff --git a/test/drafts/SignatureBouncer.test.js b/test/drafts/SignatureBouncer.test.js index b320ca98c32..c234687fc31 100644 --- a/test/drafts/SignatureBouncer.test.js +++ b/test/drafts/SignatureBouncer.test.js @@ -128,6 +128,12 @@ contract('SignatureBouncer', function ([_, signer, otherSigner, anyone, authoriz ) ); }); + + it('does not allow msg.data shorter than SIGNATURE_SIZE', async function () { + await assertRevert( + this.sigBouncer.tooShortMsgData({ from: authorizedUser }) + ); + }); }); }); From a6889776f46adca374b6ebf014aa7b0038112a9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 28 Sep 2018 22:20:43 -0300 Subject: [PATCH 12/33] Dangling commas are now required. (#1359) --- .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 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/test/crowdsale/AllowanceCrowdsale.test.js b/test/crowdsale/AllowanceCrowdsale.test.js index 8e57a841b24..c0ff4cd91ba 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 f691c951576..4113f7cb428 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 fce4bd69217..57f1fe5d0ac 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 283e8752a09..e991c2ae6d1 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 9c7bada7acc..c52594d1876 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 07b3837b086..dbda9eeedbc 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 d2cefe743c7..ce56747def3 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 a4289d27a74..8aeccd5d8b7 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 fa1dfbd113dee461ab9860514c9ad487fabfcfee Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Mon, 1 Oct 2018 18:09:57 +0530 Subject: [PATCH 13/33] Fix #1358 Test helper to send funds (#1367) * signing prefix added * Minor improvement * Tests changed * Successfully tested * Minor improvements * Minor improvements * Revert "Dangling commas are now required. (#1359)" This reverts commit a6889776f46adca374b6ebf014aa7b0038112a9d. * fixes #1358 * linting done * suggested changes --- test/BreakInvariantBounty.test.js | 5 +++-- test/helpers/sendTransaction.js | 9 +++++++++ test/payment/SplitPayment.test.js | 9 +++++---- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/test/BreakInvariantBounty.test.js b/test/BreakInvariantBounty.test.js index a29e85080ed..4c1f8b06c4e 100644 --- a/test/BreakInvariantBounty.test.js +++ b/test/BreakInvariantBounty.test.js @@ -1,4 +1,5 @@ const { ethGetBalance, ethSendTransaction } = require('./helpers/web3'); +const { sendEther } = require('./helpers/sendTransaction'); const expectEvent = require('./helpers/expectEvent'); const { assertRevert } = require('./helpers/assertRevert'); @@ -17,13 +18,13 @@ contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTar }); it('can set reward', async function () { - await ethSendTransaction({ from: owner, to: this.bounty.address, value: reward }); + await sendEther(owner, this.bounty.address, reward); (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(reward); }); context('with reward', function () { beforeEach(async function () { - await ethSendTransaction({ from: owner, to: this.bounty.address, value: reward }); + await sendEther(owner, this.bounty.address, reward); }); describe('destroy', function () { diff --git a/test/helpers/sendTransaction.js b/test/helpers/sendTransaction.js index e49376b4488..9d4f3068ff4 100644 --- a/test/helpers/sendTransaction.js +++ b/test/helpers/sendTransaction.js @@ -15,7 +15,16 @@ function sendTransaction (target, name, argsTypes, argsValues, opts) { return target.sendTransaction(Object.assign({ data: encodedData }, opts)); } +function sendEther (from, to, value) { + web3.eth.sendTransaction({ + from: from, + to: to, + value: value, + gasPrice: 0, + }); +} module.exports = { findMethod, sendTransaction, + sendEther, }; diff --git a/test/payment/SplitPayment.test.js b/test/payment/SplitPayment.test.js index 05cc0b70100..cf9ef0e0184 100644 --- a/test/payment/SplitPayment.test.js +++ b/test/payment/SplitPayment.test.js @@ -1,4 +1,5 @@ -const { ethGetBalance, ethSendTransaction } = require('../helpers/web3'); +const { ethGetBalance } = require('../helpers/web3'); +const { sendEther } = require('./../helpers/sendTransaction'); const BigNumber = web3.BigNumber; @@ -58,7 +59,7 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, }); it('should accept payments', async function () { - await ethSendTransaction({ from: owner, to: this.contract.address, value: amount }); + await sendEther(owner, this.contract.address, amount); (await ethGetBalance(this.contract.address)).should.be.bignumber.equal(amount); }); @@ -76,12 +77,12 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, }); it('should throw if non-payee want to claim', async function () { - await ethSendTransaction({ from: payer1, to: this.contract.address, value: amount }); + await sendEther(payer1, this.contract.address, amount); await expectThrow(this.contract.release(nonpayee1), EVMRevert); }); it('should distribute funds to payees', async function () { - await ethSendTransaction({ from: payer1, to: this.contract.address, value: amount }); + await sendEther(payer1, this.contract.address, amount); // receive funds const initBalance = await ethGetBalance(this.contract.address); From 6ae041bca61ff2d6c25acc865b0152cf54122528 Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Mon, 1 Oct 2018 20:53:47 +0530 Subject: [PATCH 14/33] Fix/#1355 test helper to check balance difference (#1368) * signing prefix added * Minor improvement * Tests changed * Successfully tested * Minor improvements * Minor improvements * Revert "Dangling commas are now required. (#1359)" This reverts commit a6889776f46adca374b6ebf014aa7b0038112a9d. * fixex #1355 * linting * suggested changes * Update BreakInvariantBounty.test.js --- test/BreakInvariantBounty.test.js | 9 +++------ test/helpers/balanceDiff.js | 10 ++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) create mode 100644 test/helpers/balanceDiff.js diff --git a/test/BreakInvariantBounty.test.js b/test/BreakInvariantBounty.test.js index 4c1f8b06c4e..ff8e0f7bd03 100644 --- a/test/BreakInvariantBounty.test.js +++ b/test/BreakInvariantBounty.test.js @@ -1,5 +1,6 @@ const { ethGetBalance, ethSendTransaction } = require('./helpers/web3'); const { sendEther } = require('./helpers/sendTransaction'); +const { balanceDifference } = require('./helpers/balanceDiff'); const expectEvent = require('./helpers/expectEvent'); const { assertRevert } = require('./helpers/assertRevert'); @@ -72,12 +73,8 @@ contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTar it('sends the reward to the researcher', async function () { await this.bounty.claim(this.target.address, { from: anyone }); - - const researcherPreBalance = await ethGetBalance(researcher); - await this.bounty.withdrawPayments(researcher); - const researcherPostBalance = await ethGetBalance(researcher); - - researcherPostBalance.sub(researcherPreBalance).should.be.bignumber.equal(reward); + (await balanceDifference(researcher, () => this.bounty.withdrawPayments(researcher))) + .should.be.bignumber.equal(reward); (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0); }); diff --git a/test/helpers/balanceDiff.js b/test/helpers/balanceDiff.js new file mode 100644 index 00000000000..8f88ba40045 --- /dev/null +++ b/test/helpers/balanceDiff.js @@ -0,0 +1,10 @@ +async function balanceDifference (account, promise) { + const balanceBefore = web3.eth.getBalance(account); + await promise(); + const balanceAfter = web3.eth.getBalance(account); + return balanceAfter.minus(balanceBefore); +} + +module.exports = { + balanceDifference, +}; From 34bc709bc2e5117cdd762e92a29ed88557532e61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Tue, 2 Oct 2018 07:07:11 -0300 Subject: [PATCH 15/33] Merged latestTime, increaseTime and duration into a time helper. (#1364) --- test/crowdsale/FinalizableCrowdsale.test.js | 15 ++++++------ .../IncreasingPriceCrowdsale.test.js | 23 +++++++++--------- test/crowdsale/PostDeliveryCrowdsale.test.js | 13 +++++----- test/crowdsale/RefundableCrowdsale.test.js | 15 ++++++------ test/crowdsale/TimedCrowdsale.test.js | 19 +++++++-------- test/examples/SampleCrowdsale.test.js | 23 +++++++++--------- test/helpers/latestTime.js | 11 --------- test/helpers/{increaseTime.js => time.js} | 21 ++++++++++------ test/token/ERC20/TokenTimelock.test.js | 15 ++++++------ test/token/ERC20/TokenVesting.test.js | 24 +++++++++---------- 10 files changed, 84 insertions(+), 95 deletions(-) delete mode 100644 test/helpers/latestTime.js rename test/helpers/{increaseTime.js => time.js} (78%) diff --git a/test/crowdsale/FinalizableCrowdsale.test.js b/test/crowdsale/FinalizableCrowdsale.test.js index b024fe1d43f..6eea7f2f25a 100644 --- a/test/crowdsale/FinalizableCrowdsale.test.js +++ b/test/crowdsale/FinalizableCrowdsale.test.js @@ -1,7 +1,6 @@ const expectEvent = require('../helpers/expectEvent'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); @@ -23,9 +22,9 @@ contract('FinalizableCrowdsale', function ([_, wallet, anyone]) { }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await ERC20.new(); this.crowdsale = await FinalizableCrowdsaleImpl.new( @@ -38,18 +37,18 @@ contract('FinalizableCrowdsale', function ([_, wallet, anyone]) { }); it('can be finalized by anyone after ending', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: anyone }); }); it('cannot be finalized twice', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: anyone }); await expectThrow(this.crowdsale.finalize({ from: anyone }), EVMRevert); }); it('logs finalized', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); const { logs } = await this.crowdsale.finalize({ from: anyone }); expectEvent.inLogs(logs, 'CrowdsaleFinalized'); }); diff --git a/test/crowdsale/IncreasingPriceCrowdsale.test.js b/test/crowdsale/IncreasingPriceCrowdsale.test.js index 767b5f8e568..2024d77516e 100644 --- a/test/crowdsale/IncreasingPriceCrowdsale.test.js +++ b/test/crowdsale/IncreasingPriceCrowdsale.test.js @@ -1,7 +1,6 @@ const { ether } = require('../helpers/ether'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { assertRevert } = require('../helpers/assertRevert'); const BigNumber = web3.BigNumber; @@ -29,9 +28,9 @@ contract('IncreasingPriceCrowdsale', function ([_, investor, wallet, purchaser]) beforeEach(async function () { await advanceBlock(); - this.startTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.startTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.startTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.startTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await SimpleToken.new(); }); @@ -61,43 +60,43 @@ contract('IncreasingPriceCrowdsale', function ([_, investor, wallet, purchaser]) }); it('at start', async function () { - await increaseTimeTo(this.startTime); + await time.increaseTo(this.startTime); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(initialRate)); }); it('at time 150', async function () { - await increaseTimeTo(this.startTime + 150); + await time.increaseTo(this.startTime + 150); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime150)); }); it('at time 300', async function () { - await increaseTimeTo(this.startTime + 300); + await time.increaseTo(this.startTime + 300); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime300)); }); it('at time 1500', async function () { - await increaseTimeTo(this.startTime + 1500); + await time.increaseTo(this.startTime + 1500); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime1500)); }); it('at time 30', async function () { - await increaseTimeTo(this.startTime + 30); + await time.increaseTo(this.startTime + 30); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime30)); }); it('at time 150000', async function () { - await increaseTimeTo(this.startTime + 150000); + await time.increaseTo(this.startTime + 150000); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime150000)); }); it('at time 450000', async function () { - await increaseTimeTo(this.startTime + 450000); + await time.increaseTo(this.startTime + 450000); await this.crowdsale.buyTokens(investor, { value, from: purchaser }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value.mul(rateAtTime450000)); }); diff --git a/test/crowdsale/PostDeliveryCrowdsale.test.js b/test/crowdsale/PostDeliveryCrowdsale.test.js index 94ebd808b15..866e4da15b4 100644 --- a/test/crowdsale/PostDeliveryCrowdsale.test.js +++ b/test/crowdsale/PostDeliveryCrowdsale.test.js @@ -1,6 +1,5 @@ const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); const { ether } = require('../helpers/ether'); @@ -24,9 +23,9 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await SimpleToken.new(); this.crowdsale = await PostDeliveryCrowdsaleImpl.new( this.openingTime, this.closingTime, rate, wallet, this.token.address @@ -36,7 +35,7 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { context('after opening time', function () { beforeEach(async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); }); context('with bought tokens', function () { @@ -57,7 +56,7 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { context('after closing time', function () { beforeEach(async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); }); it('allows beneficiaries to withdraw tokens', async function () { diff --git a/test/crowdsale/RefundableCrowdsale.test.js b/test/crowdsale/RefundableCrowdsale.test.js index 749ec5cac59..77e0bfebb93 100644 --- a/test/crowdsale/RefundableCrowdsale.test.js +++ b/test/crowdsale/RefundableCrowdsale.test.js @@ -1,7 +1,6 @@ const { ether } = require('../helpers/ether'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); const { ethGetBalance } = require('../helpers/web3'); @@ -27,9 +26,9 @@ contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyon }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.preWalletBalance = await ethGetBalance(wallet); this.token = await SimpleToken.new(); @@ -61,7 +60,7 @@ contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyon context('after opening time', function () { beforeEach(async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); }); it('denies refunds', async function () { @@ -75,7 +74,7 @@ contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyon context('after closing time and finalization', function () { beforeEach(async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: anyone }); }); @@ -95,7 +94,7 @@ contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyon context('after closing time and finalization', function () { beforeEach(async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: anyone }); }); diff --git a/test/crowdsale/TimedCrowdsale.test.js b/test/crowdsale/TimedCrowdsale.test.js index df3e01c817f..cfbc3d3cf26 100644 --- a/test/crowdsale/TimedCrowdsale.test.js +++ b/test/crowdsale/TimedCrowdsale.test.js @@ -1,7 +1,6 @@ const { ether } = require('../helpers/ether'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); @@ -25,21 +24,21 @@ contract('TimedCrowdsale', function ([_, investor, wallet, purchaser]) { }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await SimpleToken.new(); }); it('rejects an opening time in the past', async function () { await expectThrow(TimedCrowdsaleImpl.new( - (await latestTime()) - duration.days(1), this.closingTime, rate, wallet, this.token.address + (await time.latest()) - time.duration.days(1), this.closingTime, rate, wallet, this.token.address ), EVMRevert); }); it('rejects a closing time before the opening time', async function () { await expectThrow(TimedCrowdsaleImpl.new( - this.openingTime, this.openingTime - duration.seconds(1), rate, wallet, this.token.address + this.openingTime, this.openingTime - time.duration.seconds(1), rate, wallet, this.token.address ), EVMRevert); }); @@ -53,7 +52,7 @@ contract('TimedCrowdsale', function ([_, investor, wallet, purchaser]) { it('should be ended only after end', async function () { (await this.crowdsale.hasClosed()).should.equal(false); - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); (await this.crowdsale.isOpen()).should.equal(false); (await this.crowdsale.hasClosed()).should.equal(true); }); @@ -66,14 +65,14 @@ contract('TimedCrowdsale', function ([_, investor, wallet, purchaser]) { }); it('should accept payments after start', async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); (await this.crowdsale.isOpen()).should.equal(true); await this.crowdsale.send(value); await this.crowdsale.buyTokens(investor, { value: value, from: purchaser }); }); it('should reject payments after end', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await expectThrow(this.crowdsale.send(value), EVMRevert); await expectThrow(this.crowdsale.buyTokens(investor, { value: value, from: purchaser }), EVMRevert); }); diff --git a/test/examples/SampleCrowdsale.test.js b/test/examples/SampleCrowdsale.test.js index e4714353536..303ddf70400 100644 --- a/test/examples/SampleCrowdsale.test.js +++ b/test/examples/SampleCrowdsale.test.js @@ -1,7 +1,6 @@ const { ether } = require('../helpers/ether'); const { advanceBlock } = require('../helpers/advanceToBlock'); -const { increaseTimeTo, duration } = require('../helpers/increaseTime'); -const { latestTime } = require('../helpers/latestTime'); +const time = require('../helpers/time'); const { expectThrow } = require('../helpers/expectThrow'); const { EVMRevert } = require('../helpers/EVMRevert'); const { assertRevert } = require('../helpers/assertRevert'); @@ -27,9 +26,9 @@ contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { }); beforeEach(async function () { - this.openingTime = (await latestTime()) + duration.weeks(1); - this.closingTime = this.openingTime + duration.weeks(1); - this.afterClosingTime = this.closingTime + duration.seconds(1); + this.openingTime = (await time.latest()) + time.duration.weeks(1); + this.closingTime = this.openingTime + time.duration.weeks(1); + this.afterClosingTime = this.closingTime + time.duration.seconds(1); this.token = await SampleCrowdsaleToken.new({ from: deployer }); this.crowdsale = await SampleCrowdsale.new( @@ -68,7 +67,7 @@ contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { const investmentAmount = ether(1); const expectedTokenAmount = RATE.mul(investmentAmount); - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); await this.crowdsale.buyTokens(investor, { value: investmentAmount, from: investor }); (await this.token.balanceOf(investor)).should.be.bignumber.equal(expectedTokenAmount); @@ -76,23 +75,23 @@ contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { }); it('should reject payments after end', async function () { - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await expectThrow(this.crowdsale.send(ether(1)), EVMRevert); await expectThrow(this.crowdsale.buyTokens(investor, { value: ether(1), from: investor }), EVMRevert); }); it('should reject payments over cap', async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); await this.crowdsale.send(CAP); await expectThrow(this.crowdsale.send(1), EVMRevert); }); it('should allow finalization and transfer funds to wallet if the goal is reached', async function () { - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); await this.crowdsale.send(GOAL); const beforeFinalization = await ethGetBalance(wallet); - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: owner }); const afterFinalization = await ethGetBalance(wallet); @@ -102,9 +101,9 @@ contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { it('should allow refunds if the goal is not reached', async function () { const balanceBeforeInvestment = await ethGetBalance(investor); - await increaseTimeTo(this.openingTime); + await time.increaseTo(this.openingTime); await this.crowdsale.sendTransaction({ value: ether(1), from: investor, gasPrice: 0 }); - await increaseTimeTo(this.afterClosingTime); + await time.increaseTo(this.afterClosingTime); await this.crowdsale.finalize({ from: owner }); await this.crowdsale.claimRefund(investor, { gasPrice: 0 }); diff --git a/test/helpers/latestTime.js b/test/helpers/latestTime.js deleted file mode 100644 index fee9521d677..00000000000 --- a/test/helpers/latestTime.js +++ /dev/null @@ -1,11 +0,0 @@ -const { ethGetBlock } = require('./web3'); - -// Returns the time of the last mined block in seconds -async function latestTime () { - const block = await ethGetBlock('latest'); - return block.timestamp; -} - -module.exports = { - latestTime, -}; diff --git a/test/helpers/increaseTime.js b/test/helpers/time.js similarity index 78% rename from test/helpers/increaseTime.js rename to test/helpers/time.js index b22ba6b04b3..b7f3c000105 100644 --- a/test/helpers/increaseTime.js +++ b/test/helpers/time.js @@ -1,7 +1,13 @@ -const { latestTime } = require('./latestTime'); +const { ethGetBlock } = require('./web3'); + +// Returns the time of the last mined block in seconds +async function latest () { + const block = await ethGetBlock('latest'); + return block.timestamp; +} // Increases ganache time by the passed duration in seconds -function increaseTime (duration) { +function increase (duration) { const id = Date.now(); return new Promise((resolve, reject) => { @@ -31,12 +37,12 @@ function increaseTime (duration) { * * @param target time in seconds */ -async function increaseTimeTo (target) { - const now = (await latestTime()); +async function increaseTo (target) { + const now = (await latest()); if (target < now) throw Error(`Cannot increase current time(${now}) to a moment in the past(${target})`); const diff = target - now; - return increaseTime(diff); + return increase(diff); } const duration = { @@ -49,7 +55,8 @@ const duration = { }; module.exports = { - increaseTime, - increaseTimeTo, + latest, + increase, + increaseTo, duration, }; diff --git a/test/token/ERC20/TokenTimelock.test.js b/test/token/ERC20/TokenTimelock.test.js index c10b1b82137..04441fd1ebc 100644 --- a/test/token/ERC20/TokenTimelock.test.js +++ b/test/token/ERC20/TokenTimelock.test.js @@ -1,5 +1,4 @@ -const { latestTime } = require('../../helpers/latestTime'); -const { increaseTimeTo, duration } = require('../../helpers/increaseTime'); +const time = require('../../helpers/time'); const { expectThrow } = require('../../helpers/expectThrow'); const BigNumber = web3.BigNumber; @@ -20,7 +19,7 @@ contract('TokenTimelock', function ([_, minter, beneficiary]) { }); it('rejects a release time in the past', async function () { - const pastReleaseTime = (await latestTime()) - duration.years(1); + const pastReleaseTime = (await time.latest()) - time.duration.years(1); await expectThrow( TokenTimelock.new(this.token.address, beneficiary, pastReleaseTime) ); @@ -28,7 +27,7 @@ contract('TokenTimelock', function ([_, minter, beneficiary]) { context('once deployed', function () { beforeEach(async function () { - this.releaseTime = (await latestTime()) + duration.years(1); + this.releaseTime = (await time.latest()) + time.duration.years(1); this.timelock = await TokenTimelock.new(this.token.address, beneficiary, this.releaseTime); await this.token.mint(this.timelock.address, amount, { from: minter }); }); @@ -44,24 +43,24 @@ contract('TokenTimelock', function ([_, minter, beneficiary]) { }); it('cannot be released just before time limit', async function () { - await increaseTimeTo(this.releaseTime - duration.seconds(3)); + await time.increaseTo(this.releaseTime - time.duration.seconds(3)); await expectThrow(this.timelock.release()); }); it('can be released just after limit', async function () { - await increaseTimeTo(this.releaseTime + duration.seconds(1)); + await time.increaseTo(this.releaseTime + time.duration.seconds(1)); await this.timelock.release(); (await this.token.balanceOf(beneficiary)).should.be.bignumber.equal(amount); }); it('can be released after time limit', async function () { - await increaseTimeTo(this.releaseTime + duration.years(1)); + await time.increaseTo(this.releaseTime + time.duration.years(1)); await this.timelock.release(); (await this.token.balanceOf(beneficiary)).should.be.bignumber.equal(amount); }); it('cannot be released twice', async function () { - await increaseTimeTo(this.releaseTime + duration.years(1)); + await time.increaseTo(this.releaseTime + time.duration.years(1)); await this.timelock.release(); await expectThrow(this.timelock.release()); (await this.token.balanceOf(beneficiary)).should.be.bignumber.equal(amount); diff --git a/test/token/ERC20/TokenVesting.test.js b/test/token/ERC20/TokenVesting.test.js index 1fe9326f5fa..7c4fbcc8e36 100644 --- a/test/token/ERC20/TokenVesting.test.js +++ b/test/token/ERC20/TokenVesting.test.js @@ -1,7 +1,6 @@ const { expectThrow } = require('../../helpers/expectThrow'); const { EVMRevert } = require('../../helpers/EVMRevert'); -const { latestTime } = require('../../helpers/latestTime'); -const { increaseTimeTo, duration } = require('../../helpers/increaseTime'); +const time = require('../../helpers/time'); const { ethGetBlock } = require('../../helpers/web3'); const BigNumber = web3.BigNumber; @@ -18,9 +17,10 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; beforeEach(async function () { - this.start = (await latestTime()) + duration.minutes(1); // +1 minute so it starts after contract instantiation - this.cliffDuration = duration.years(1); - this.duration = duration.years(2); + // +1 minute so it starts after contract instantiation + this.start = (await time.latest()) + time.duration.minutes(1); + this.cliffDuration = time.duration.years(1); + this.duration = time.duration.years(2); }); it('rejects a duration shorter than the cliff', async function () { @@ -65,12 +65,12 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('can be released after cliff', async function () { - await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(1)); + await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(1)); await this.vesting.release(this.token.address); }); it('should release proper amount after cliff', async function () { - await increaseTimeTo(this.start + this.cliffDuration); + await time.increaseTo(this.start + this.cliffDuration); const { receipt } = await this.vesting.release(this.token.address); const block = await ethGetBlock(receipt.blockNumber); @@ -87,7 +87,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { for (let i = 1; i <= checkpoints; i++) { const now = this.start + this.cliffDuration + i * (vestingPeriod / checkpoints); - await increaseTimeTo(now); + await time.increaseTo(now); await this.vesting.release(this.token.address); const expectedVesting = amount.mul(now - this.start).div(this.duration).floor(); @@ -97,7 +97,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should have released all after end', async function () { - await increaseTimeTo(this.start + this.duration); + await time.increaseTo(this.start + this.duration); await this.vesting.release(this.token.address); (await this.token.balanceOf(beneficiary)).should.bignumber.equal(amount); (await this.vesting.released(this.token.address)).should.bignumber.equal(amount); @@ -120,7 +120,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should return the non-vested tokens when revoked by owner', async function () { - await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12)); + await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(12)); const vested = await this.vesting.vestedAmount(this.token.address); @@ -130,7 +130,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should keep the vested tokens when revoked by owner', async function () { - await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12)); + await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(12)); const vestedPre = await this.vesting.vestedAmount(this.token.address); @@ -142,7 +142,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should fail to be revoked a second time', async function () { - await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12)); + await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(12)); await this.vesting.vestedAmount(this.token.address); From 269981ee6a02096a20f6997d87e8d43b78906284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Tue, 2 Oct 2018 10:37:44 -0300 Subject: [PATCH 16/33] Created test utils directory --- test/{ => utils}/Address.test.js | 0 test/{ => utils}/ReentrancyGuard.test.js | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename test/{ => utils}/Address.test.js (100%) rename test/{ => utils}/ReentrancyGuard.test.js (100%) diff --git a/test/Address.test.js b/test/utils/Address.test.js similarity index 100% rename from test/Address.test.js rename to test/utils/Address.test.js diff --git a/test/ReentrancyGuard.test.js b/test/utils/ReentrancyGuard.test.js similarity index 100% rename from test/ReentrancyGuard.test.js rename to test/utils/ReentrancyGuard.test.js From f4d6f404424669e0eef7faa05fd89ab9945f265b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Tue, 2 Oct 2018 10:48:54 -0300 Subject: [PATCH 17/33] Fixed test path. --- test/utils/ReentrancyGuard.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/utils/ReentrancyGuard.test.js b/test/utils/ReentrancyGuard.test.js index 7086f8d94f7..27430652c10 100644 --- a/test/utils/ReentrancyGuard.test.js +++ b/test/utils/ReentrancyGuard.test.js @@ -1,4 +1,4 @@ -const { expectThrow } = require('./helpers/expectThrow'); +const { expectThrow } = require('../helpers/expectThrow'); const ReentrancyMock = artifacts.require('ReentrancyMock'); const ReentrancyAttack = artifacts.require('ReentrancyAttack'); From 43ebb4fc433979d19c4cbec4aa0c25e0e3e9e5f7 Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Wed, 3 Oct 2018 00:15:59 +0300 Subject: [PATCH 18/33] ERC20 internal transfer method (#1370) --- contracts/token/ERC20/ERC20.sol | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 8fd420a2909..3d18c1dec67 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -58,12 +58,7 @@ contract ERC20 is IERC20 { * @param value The amount to be transferred. */ function transfer(address to, uint256 value) public returns (bool) { - require(value <= _balances[msg.sender]); - require(to != address(0)); - - _balances[msg.sender] = _balances[msg.sender].sub(value); - _balances[to] = _balances[to].add(value); - emit Transfer(msg.sender, to, value); + _transfer(msg.sender, to, value); return true; } @@ -98,14 +93,10 @@ contract ERC20 is IERC20 { public returns (bool) { - require(value <= _balances[from]); require(value <= _allowed[from][msg.sender]); - require(to != address(0)); - _balances[from] = _balances[from].sub(value); - _balances[to] = _balances[to].add(value); _allowed[from][msg.sender] = _allowed[from][msg.sender].sub(value); - emit Transfer(from, to, value); + _transfer(from, to, value); return true; } @@ -157,6 +148,21 @@ contract ERC20 is IERC20 { return true; } + /** + * @dev Transfer token for a specified addresses + * @param from The address to transfer from. + * @param to The address to transfer to. + * @param value The amount to be transferred. + */ + function _transfer(address from, address to, uint256 value) internal { + require(value <= _balances[from]); + require(to != address(0)); + + _balances[from] = _balances[from].sub(value); + _balances[to] = _balances[to].add(value); + emit Transfer(from, to, 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 f3888bb0b0a2eba5282a1bd5828135910b65bc8d Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Wed, 3 Oct 2018 20:53:08 +0530 Subject: [PATCH 19/33] Removing unrequired `_burn()` override (#1373) * signing prefix added * Minor improvement * Tests changed * Successfully tested * Minor improvements * Minor improvements * Revert "Dangling commas are now required. (#1359)" This reverts commit a6889776f46adca374b6ebf014aa7b0038112a9d. * updates * fixes #1371 * Removed extra whitespace --- contracts/token/ERC20/ERC20Burnable.sol | 8 -------- 1 file changed, 8 deletions(-) diff --git a/contracts/token/ERC20/ERC20Burnable.sol b/contracts/token/ERC20/ERC20Burnable.sol index 014dc7fa8d7..ba4a2ccbed6 100644 --- a/contracts/token/ERC20/ERC20Burnable.sol +++ b/contracts/token/ERC20/ERC20Burnable.sol @@ -24,12 +24,4 @@ contract ERC20Burnable is ERC20 { function burnFrom(address from, uint256 value) public { _burnFrom(from, value); } - - /** - * @dev Overrides ERC20._burn in order for burn and burnFrom to emit - * an additional Burn event. - */ - function _burn(address who, uint256 value) internal { - super._burn(who, value); - } } From c87433e0c242f922d37d28710ced32584b561234 Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Wed, 3 Oct 2018 21:20:01 +0530 Subject: [PATCH 20/33] Prevents Bounty from being claimed twice (#1374) * signing prefix added * Minor improvement * Tests changed * Successfully tested * Minor improvements * Minor improvements * Revert "Dangling commas are now required. (#1359)" This reverts commit a6889776f46adca374b6ebf014aa7b0038112a9d. * updates * fixes #1356 * Removed extra semicolon. --- contracts/bounties/BreakInvariantBounty.sol | 1 + test/BreakInvariantBounty.test.js | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/contracts/bounties/BreakInvariantBounty.sol b/contracts/bounties/BreakInvariantBounty.sol index 016d3f2f55d..6fd3b6e671d 100644 --- a/contracts/bounties/BreakInvariantBounty.sol +++ b/contracts/bounties/BreakInvariantBounty.sol @@ -45,6 +45,7 @@ contract BreakInvariantBounty is PullPayment, Ownable { * @param target contract */ function claim(Target target) public { + require(!_claimed); address researcher = _researchers[target]; require(researcher != address(0)); // Check Target contract invariants diff --git a/test/BreakInvariantBounty.test.js b/test/BreakInvariantBounty.test.js index ff8e0f7bd03..50ae96aaa05 100644 --- a/test/BreakInvariantBounty.test.js +++ b/test/BreakInvariantBounty.test.js @@ -90,6 +90,10 @@ contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTar it('no longer accepts rewards', async function () { await assertRevert(ethSendTransaction({ from: owner, to: this.bounty.address, value: reward })); }); + + it('reverts when reclaimed', async function () { + await assertRevert(this.bounty.claim(this.target.address, { from: researcher })); + }); }); }); }); From 5c228805ad195654c6fb2a3276f1f323d4762666 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 3 Oct 2018 23:13:46 -0300 Subject: [PATCH 21/33] Update issue templates (#1376) * Update issue templates * improve pull request template * remove previous issue template --- .github/ISSUE_TEMPLATE.md | 34 ----------------------- .github/ISSUE_TEMPLATE/bug_report.md | 21 ++++++++++++++ .github/ISSUE_TEMPLATE/feature_request.md | 14 ++++++++++ .github/PULL_REQUEST_TEMPLATE.md | 23 ++++++++------- 4 files changed, 48 insertions(+), 44 deletions(-) delete mode 100644 .github/ISSUE_TEMPLATE.md create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md deleted file mode 100644 index cafae1e9e0f..00000000000 --- a/.github/ISSUE_TEMPLATE.md +++ /dev/null @@ -1,34 +0,0 @@ -## 🎉 Description - - - -- [ ] 🐛 This is a bug report. -- [ ] 📈 This is a feature request. - - - -## 💻 Environment - -Next, we need to know what your environment looks like. - -- Which version of OpenZeppelin are you using? -- What network are you deploying to? Ganache? Ropsten? -- How are you deploying your OpenZeppelin-backed contracts? truffle? Remix? Let us know! - -## 📝 Details - -Describe the problem you have been experiencing in more detail. Include as much information as you think is relevant. Keep in mind that transactions can fail for many reasons; context is key here. - -## 🔢 Code To Reproduce Issue [ Good To Have ] - -Please remember that with sample code it's easier to reproduce the bug and it's much faster to fix it. - -``` -insert short code snippets here -``` - - - -## 👍 Other Information - - diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 00000000000..d932e7632ab --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,21 @@ +--- +name: Bug report +about: Report a bug in OpenZeppelin + +--- + + + + + +**💻 Environment** + + + +**📝 Details** + + + +**🔢 Code to reproduce bug** + + diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 00000000000..404854c009f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,14 @@ +--- +name: Feature request +about: Suggest an idea for OpenZeppelin + +--- + +**🧐 Motivation** + + +**📝 Details** + + + + diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 72b331668a3..2d743d0fe1f 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,17 +1,20 @@ - + -Fixes # + -# 🚀 Description +Fixes # - + - - -- [ ] 📘 I've reviewed the [OpenZeppelin Contributor Guidelines](../blob/master/CONTRIBUTING.md) -- [ ] ✅ I've added tests where applicable to test my new functionality. -- [ ] 📖 I've made sure that my contracts are well-documented. -- [ ] 🎨 I've run the JS/Solidity linters and fixed any issues (`npm run lint:fix`). + From ace14d3ad73f54c02a1328f939fa2d2afb395882 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 3 Oct 2018 23:17:06 -0300 Subject: [PATCH 22/33] Add note about compiling artifacts to releasing steps (#1377) * add note about compiling artifacts to release notes * add explanation of truffle bug --- RELEASING.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/RELEASING.md b/RELEASING.md index 6514b4cf54c..40360afcf93 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -34,6 +34,12 @@ git push upstream vX.Y.Z-rc.R Draft the release notes in our [GitHub releases](https://github.com/OpenZeppelin/openzeppelin-solidity/releases). Make sure to mark it as a pre-release! Try to be consistent with our previous release notes in the title and format of the text. Release candidates don't need a detailed changelog, but make sure to include a link to GitHub's compare page. +Before publishing on npm you need to generate the build artifacts. This is not done automatically at the moment because of a bug in Truffle. Since some of the contracts should not be included in the package, this is a _hairy_ process that you need to do with care. + +1. Delete the `contracts/mocks` and `contracts/examples` directories. +2. Run `truffle compile`. (Note that the Truffle process may never exit and you will have to interrupt it.) +3. Recover the directories using `git checkout`. It doesn't matter if you do this now or later. + Once the CI run for the new tag is green, publish on npm under the `next` tag. ``` @@ -62,6 +68,12 @@ git push upstream vX.Y.Z Draft the release notes in GitHub releases. Try to be consistent with our previous release notes in the title and format of the text. Make sure to include a detailed changelog. +Before publishing on npm you need to generate the build artifacts. This is not done automatically at the moment because of a bug in Truffle. Since some of the contracts should not be included in the package, this is a _hairy_ process that you need to do with care. + +1. Delete the `contracts/mocks` and `contracts/examples` directories. +2. Run `truffle compile`. (Note that the Truffle process may never exit and you will have to interrupt it.) +3. Recover the directories using `git checkout`. It doesn't matter if you do this now or later. + Once the CI run for the new tag is green, publish on npm. ``` From fd4de776519e2bd64dc6ac0efb87e0f603c6608f Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Thu, 4 Oct 2018 16:21:52 +0530 Subject: [PATCH 23/33] Replaces `amount` with `value` for consistency (#1378) * fixes #1372 * done in ERC20Capped and ERC20Mintable --- contracts/token/ERC20/ERC20.sol | 32 ++++++++++++------------- contracts/token/ERC20/ERC20Capped.sol | 8 +++---- contracts/token/ERC20/ERC20Mintable.sol | 6 ++--- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 3d18c1dec67..25b2694a24f 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -168,28 +168,28 @@ contract ERC20 is IERC20 { * an account. This encapsulates the modification of balances such that the * proper events are emitted. * @param account The account that will receive the created tokens. - * @param amount The amount that will be created. + * @param value The amount that will be created. */ - function _mint(address account, uint256 amount) internal { + function _mint(address account, uint256 value) internal { require(account != 0); - _totalSupply = _totalSupply.add(amount); - _balances[account] = _balances[account].add(amount); - emit Transfer(address(0), account, amount); + _totalSupply = _totalSupply.add(value); + _balances[account] = _balances[account].add(value); + emit Transfer(address(0), account, value); } /** * @dev Internal function that burns an amount of the token of a given * account. * @param account The account whose tokens will be burnt. - * @param amount The amount that will be burnt. + * @param value The amount that will be burnt. */ - function _burn(address account, uint256 amount) internal { + function _burn(address account, uint256 value) internal { require(account != 0); - require(amount <= _balances[account]); + require(value <= _balances[account]); - _totalSupply = _totalSupply.sub(amount); - _balances[account] = _balances[account].sub(amount); - emit Transfer(account, address(0), amount); + _totalSupply = _totalSupply.sub(value); + _balances[account] = _balances[account].sub(value); + emit Transfer(account, address(0), value); } /** @@ -197,15 +197,15 @@ contract ERC20 is IERC20 { * account, deducting from the sender's allowance for said account. Uses the * internal burn function. * @param account The account whose tokens will be burnt. - * @param amount The amount that will be burnt. + * @param value The amount that will be burnt. */ - function _burnFrom(address account, uint256 amount) internal { - require(amount <= _allowed[account][msg.sender]); + function _burnFrom(address account, uint256 value) internal { + require(value <= _allowed[account][msg.sender]); // Should https://github.com/OpenZeppelin/zeppelin-solidity/issues/707 be accepted, // this function needs to emit an event with the updated approval. _allowed[account][msg.sender] = _allowed[account][msg.sender].sub( - amount); - _burn(account, amount); + value); + _burn(account, value); } } diff --git a/contracts/token/ERC20/ERC20Capped.sol b/contracts/token/ERC20/ERC20Capped.sol index 0a73159b6ac..9f3ea8bfc04 100644 --- a/contracts/token/ERC20/ERC20Capped.sol +++ b/contracts/token/ERC20/ERC20Capped.sol @@ -27,19 +27,19 @@ contract ERC20Capped is ERC20Mintable { /** * @dev Function to mint tokens * @param to The address that will receive the minted tokens. - * @param amount The amount of tokens to mint. + * @param value The amount of tokens to mint. * @return A boolean that indicates if the operation was successful. */ function mint( address to, - uint256 amount + uint256 value ) public returns (bool) { - require(totalSupply().add(amount) <= _cap); + require(totalSupply().add(value) <= _cap); - return super.mint(to, amount); + return super.mint(to, value); } } diff --git a/contracts/token/ERC20/ERC20Mintable.sol b/contracts/token/ERC20/ERC20Mintable.sol index 386abf64d27..eb01f06e74e 100644 --- a/contracts/token/ERC20/ERC20Mintable.sol +++ b/contracts/token/ERC20/ERC20Mintable.sol @@ -11,18 +11,18 @@ contract ERC20Mintable is ERC20, MinterRole { /** * @dev Function to mint tokens * @param to The address that will receive the minted tokens. - * @param amount The amount of tokens to mint. + * @param value The amount of tokens to mint. * @return A boolean that indicates if the operation was successful. */ function mint( address to, - uint256 amount + uint256 value ) public onlyMinter returns (bool) { - _mint(to, amount); + _mint(to, value); return true; } } From b41b125c15c9ccf8a002a43ed64b5349982c2fb6 Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Thu, 4 Oct 2018 18:27:37 +0530 Subject: [PATCH 24/33] `this` is used in tests (#1380) * signing prefix added * Minor improvement * Tests changed * Successfully tested * Minor improvements * Minor improvements * Revert "Dangling commas are now required. (#1359)" This reverts commit a6889776f46adca374b6ebf014aa7b0038112a9d. * updates * fixes #1200 * suggested change --- test/examples/SimpleToken.test.js | 18 ++++++++---------- test/library/MerkleProof.test.js | 10 ++++------ test/token/ERC20/ERC20Detailed.test.js | 10 ++++------ test/utils/ReentrancyGuard.test.js | 13 +++++-------- 4 files changed, 21 insertions(+), 30 deletions(-) diff --git a/test/examples/SimpleToken.test.js b/test/examples/SimpleToken.test.js index a28cf4641ab..f59a8200017 100644 --- a/test/examples/SimpleToken.test.js +++ b/test/examples/SimpleToken.test.js @@ -8,34 +8,32 @@ require('chai') .should(); contract('SimpleToken', function ([_, creator]) { - let token; - const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; beforeEach(async function () { - token = await SimpleToken.new({ from: creator }); + this.token = await SimpleToken.new({ from: creator }); }); it('has a name', async function () { - (await token.name()).should.equal('SimpleToken'); + (await this.token.name()).should.equal('SimpleToken'); }); it('has a symbol', async function () { - (await token.symbol()).should.equal('SIM'); + (await this.token.symbol()).should.equal('SIM'); }); it('has 18 decimals', async function () { - (await token.decimals()).should.be.bignumber.equal(18); + (await this.token.decimals()).should.be.bignumber.equal(18); }); it('assigns the initial total supply to the creator', async function () { - const totalSupply = await token.totalSupply(); - const creatorBalance = await token.balanceOf(creator); + const totalSupply = await this.token.totalSupply(); + const creatorBalance = await this.token.balanceOf(creator); creatorBalance.should.be.bignumber.equal(totalSupply); - const receipt = await web3.eth.getTransactionReceipt(token.transactionHash); - const logs = decodeLogs(receipt.logs, SimpleToken, token.address); + const receipt = await web3.eth.getTransactionReceipt(this.token.transactionHash); + const logs = decodeLogs(receipt.logs, SimpleToken, this.token.address); logs.length.should.equal(1); logs[0].event.should.equal('Transfer'); logs[0].args.from.valueOf().should.equal(ZERO_ADDRESS); diff --git a/test/library/MerkleProof.test.js b/test/library/MerkleProof.test.js index 6674880b742..b5d8fbe07e6 100644 --- a/test/library/MerkleProof.test.js +++ b/test/library/MerkleProof.test.js @@ -7,10 +7,8 @@ require('chai') .should(); contract('MerkleProof', function () { - let merkleProof; - beforeEach(async function () { - merkleProof = await MerkleProofWrapper.new(); + this.merkleProof = await MerkleProofWrapper.new(); }); describe('verify', function () { @@ -24,7 +22,7 @@ contract('MerkleProof', function () { const leaf = bufferToHex(sha3(elements[0])); - (await merkleProof.verify(proof, root, leaf)).should.equal(true); + (await this.merkleProof.verify(proof, root, leaf)).should.equal(true); }); it('should return false for an invalid Merkle proof', async function () { @@ -40,7 +38,7 @@ contract('MerkleProof', function () { const badProof = badMerkleTree.getHexProof(badElements[0]); - (await merkleProof.verify(badProof, correctRoot, correctLeaf)).should.equal(false); + (await this.merkleProof.verify(badProof, correctRoot, correctLeaf)).should.equal(false); }); it('should return false for a Merkle proof of invalid length', async function () { @@ -54,7 +52,7 @@ contract('MerkleProof', function () { const leaf = bufferToHex(sha3(elements[0])); - (await merkleProof.verify(badProof, root, leaf)).should.equal(false); + (await this.merkleProof.verify(badProof, root, leaf)).should.equal(false); }); }); }); diff --git a/test/token/ERC20/ERC20Detailed.test.js b/test/token/ERC20/ERC20Detailed.test.js index 71b22a7b9a8..7965b7e9dc5 100644 --- a/test/token/ERC20/ERC20Detailed.test.js +++ b/test/token/ERC20/ERC20Detailed.test.js @@ -7,25 +7,23 @@ require('chai') const ERC20DetailedMock = artifacts.require('ERC20DetailedMock'); contract('ERC20Detailed', function () { - let detailedERC20 = null; - const _name = 'My Detailed ERC20'; const _symbol = 'MDT'; const _decimals = 18; beforeEach(async function () { - detailedERC20 = await ERC20DetailedMock.new(_name, _symbol, _decimals); + this.detailedERC20 = await ERC20DetailedMock.new(_name, _symbol, _decimals); }); it('has a name', async function () { - (await detailedERC20.name()).should.be.equal(_name); + (await this.detailedERC20.name()).should.be.equal(_name); }); it('has a symbol', async function () { - (await detailedERC20.symbol()).should.be.equal(_symbol); + (await this.detailedERC20.symbol()).should.be.equal(_symbol); }); it('has an amount of decimals', async function () { - (await detailedERC20.decimals()).should.be.bignumber.equal(_decimals); + (await this.detailedERC20.decimals()).should.be.bignumber.equal(_decimals); }); }); diff --git a/test/utils/ReentrancyGuard.test.js b/test/utils/ReentrancyGuard.test.js index 27430652c10..3952b2fcbc0 100644 --- a/test/utils/ReentrancyGuard.test.js +++ b/test/utils/ReentrancyGuard.test.js @@ -9,17 +9,14 @@ require('chai') .should(); contract('ReentrancyGuard', function () { - let reentrancyMock; - beforeEach(async function () { - reentrancyMock = await ReentrancyMock.new(); - const initialCounter = await reentrancyMock.counter(); - initialCounter.should.be.bignumber.equal(0); + this.reentrancyMock = await ReentrancyMock.new(); + (await this.reentrancyMock.counter()).should.be.bignumber.equal(0); }); it('should not allow remote callback', async function () { const attacker = await ReentrancyAttack.new(); - await expectThrow(reentrancyMock.countAndCall(attacker.address)); + await expectThrow(this.reentrancyMock.countAndCall(attacker.address)); }); // The following are more side-effects than intended behavior: @@ -27,10 +24,10 @@ contract('ReentrancyGuard', function () { // in the side-effects. it('should not allow local recursion', async function () { - await expectThrow(reentrancyMock.countLocalRecursive(10)); + await expectThrow(this.reentrancyMock.countLocalRecursive(10)); }); it('should not allow indirect local recursion', async function () { - await expectThrow(reentrancyMock.countThisRecursive(10)); + await expectThrow(this.reentrancyMock.countThisRecursive(10)); }); }); From 744f567f40bcd31f821a35d2a9a004602e28a1e9 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 4 Oct 2018 11:10:08 -0300 Subject: [PATCH 25/33] Separate ERC721Mintable (#1365) * separate part of ERC721Mintable into ERC721MetadataMintable * remove mint and burn from 721 tests * Fixed linter error. * fix ERC721 mint tests * Minor fixes. --- contracts/mocks/ERC721FullMock.sol | 7 ++-- .../mocks/ERC721MintableBurnableImpl.sol | 3 +- .../token/ERC721/ERC721MetadataMintable.sol | 32 +++++++++++++++++++ contracts/token/ERC721/ERC721Mintable.sol | 18 ++--------- test/token/ERC721/ERC721Full.test.js | 2 -- test/token/ERC721/ERC721MintBurn.behavior.js | 4 +-- 6 files changed, 43 insertions(+), 23 deletions(-) create mode 100644 contracts/token/ERC721/ERC721MetadataMintable.sol diff --git a/contracts/mocks/ERC721FullMock.sol b/contracts/mocks/ERC721FullMock.sol index d555b1f832d..2d85e737220 100644 --- a/contracts/mocks/ERC721FullMock.sol +++ b/contracts/mocks/ERC721FullMock.sol @@ -2,14 +2,17 @@ pragma solidity ^0.4.24; import "../token/ERC721/ERC721Full.sol"; import "../token/ERC721/ERC721Mintable.sol"; +import "../token/ERC721/ERC721MetadataMintable.sol"; import "../token/ERC721/ERC721Burnable.sol"; /** - * @title ERC721Mock + * @title ERC721FullMock * This mock just provides a public mint and burn functions for testing purposes, * and a public setter for metadata URI */ -contract ERC721FullMock is ERC721Full, ERC721Mintable, ERC721Burnable { +contract ERC721FullMock + is ERC721Full, ERC721Mintable, ERC721MetadataMintable, ERC721Burnable { + constructor(string name, string symbol) public ERC721Mintable() ERC721Full(name, symbol) diff --git a/contracts/mocks/ERC721MintableBurnableImpl.sol b/contracts/mocks/ERC721MintableBurnableImpl.sol index 03ff3ac378a..51d3ff4f6bc 100644 --- a/contracts/mocks/ERC721MintableBurnableImpl.sol +++ b/contracts/mocks/ERC721MintableBurnableImpl.sol @@ -2,13 +2,14 @@ pragma solidity ^0.4.24; import "../token/ERC721/ERC721Full.sol"; import "../token/ERC721/ERC721Mintable.sol"; +import "../token/ERC721/ERC721MetadataMintable.sol"; import "../token/ERC721/ERC721Burnable.sol"; /** * @title ERC721MintableBurnableImpl */ contract ERC721MintableBurnableImpl - is ERC721Full, ERC721Mintable, ERC721Burnable { + is ERC721Full, ERC721Mintable, ERC721MetadataMintable, ERC721Burnable { constructor() ERC721Mintable() diff --git a/contracts/token/ERC721/ERC721MetadataMintable.sol b/contracts/token/ERC721/ERC721MetadataMintable.sol new file mode 100644 index 00000000000..95a9f21246e --- /dev/null +++ b/contracts/token/ERC721/ERC721MetadataMintable.sol @@ -0,0 +1,32 @@ +pragma solidity ^0.4.24; + +import "./ERC721Metadata.sol"; +import "../../access/roles/MinterRole.sol"; + + +/** + * @title ERC721MetadataMintable + * @dev ERC721 minting logic with metadata + */ +contract ERC721MetadataMintable is ERC721, ERC721Metadata, MinterRole { + /** + * @dev Function to mint tokens + * @param to The address that will receive the minted tokens. + * @param tokenId The token id to mint. + * @param tokenURI The token URI of the minted token. + * @return A boolean that indicates if the operation was successful. + */ + function mintWithTokenURI( + address to, + uint256 tokenId, + string tokenURI + ) + public + onlyMinter + returns (bool) + { + _mint(to, tokenId); + _setTokenURI(tokenId, tokenURI); + return true; + } +} diff --git a/contracts/token/ERC721/ERC721Mintable.sol b/contracts/token/ERC721/ERC721Mintable.sol index 9dc0ad75199..74b7b0186f5 100644 --- a/contracts/token/ERC721/ERC721Mintable.sol +++ b/contracts/token/ERC721/ERC721Mintable.sol @@ -1,13 +1,13 @@ pragma solidity ^0.4.24; -import "./ERC721Full.sol"; +import "./ERC721.sol"; import "../../access/roles/MinterRole.sol"; /** * @title ERC721Mintable * @dev ERC721 minting logic */ -contract ERC721Mintable is ERC721Full, MinterRole { +contract ERC721Mintable is ERC721, MinterRole { /** * @dev Function to mint tokens * @param to The address that will receive the minted tokens. @@ -25,18 +25,4 @@ contract ERC721Mintable is ERC721Full, MinterRole { _mint(to, tokenId); return true; } - - function mintWithTokenURI( - address to, - uint256 tokenId, - string tokenURI - ) - public - onlyMinter - returns (bool) - { - mint(to, tokenId); - _setTokenURI(tokenId, tokenURI); - return true; - } } diff --git a/test/token/ERC721/ERC721Full.test.js b/test/token/ERC721/ERC721Full.test.js index f4666d0d9b0..30e4547c82a 100644 --- a/test/token/ERC721/ERC721Full.test.js +++ b/test/token/ERC721/ERC721Full.test.js @@ -1,6 +1,5 @@ const { assertRevert } = require('../../helpers/assertRevert'); const { shouldBehaveLikeERC721 } = require('./ERC721.behavior'); -const { shouldBehaveLikeMintAndBurnERC721 } = require('./ERC721MintBurn.behavior'); const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior'); const BigNumber = web3.BigNumber; @@ -221,7 +220,6 @@ contract('ERC721Full', function ([ }); shouldBehaveLikeERC721(creator, minter, accounts); - shouldBehaveLikeMintAndBurnERC721(creator, minter, accounts); shouldSupportInterfaces([ 'ERC165', diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index 8aeccd5d8b7..bcf7e9ed303 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -52,13 +52,13 @@ function shouldBehaveLikeMintAndBurnERC721 ( describe('when the given owner address is the zero address', function () { it('reverts', async function () { - await assertRevert(this.token.mint(ZERO_ADDRESS, thirdTokenId)); + await assertRevert(this.token.mint(ZERO_ADDRESS, thirdTokenId, { from: minter })); }); }); describe('when the given token ID was already tracked by this contract', function () { it('reverts', async function () { - await assertRevert(this.token.mint(owner, firstTokenId)); + await assertRevert(this.token.mint(owner, firstTokenId, { from: minter })); }); }); }); From 308e5e9cc0bc9a65436cf3ae933551e1a62167ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 4 Oct 2018 11:15:22 -0300 Subject: [PATCH 26/33] Removed unnecessary Secondary inheritance from RefundEscrow. (#1381) --- contracts/payment/RefundEscrow.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/payment/RefundEscrow.sol b/contracts/payment/RefundEscrow.sol index 3eca6bbcfbb..c3770dbbb88 100644 --- a/contracts/payment/RefundEscrow.sol +++ b/contracts/payment/RefundEscrow.sol @@ -1,7 +1,6 @@ pragma solidity ^0.4.24; import "./ConditionalEscrow.sol"; -import "../ownership/Secondary.sol"; /** * @title RefundEscrow @@ -9,7 +8,7 @@ import "../ownership/Secondary.sol"; * The primary account may close the deposit period, and allow for either withdrawal * by the beneficiary, or refunds to the depositors. */ -contract RefundEscrow is Secondary, ConditionalEscrow { +contract RefundEscrow is ConditionalEscrow { enum State { Active, Refunding, Closed } event Closed(); From b17de011dc336ed329d70fb0c7950b67b8192292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 5 Oct 2018 22:18:59 -0300 Subject: [PATCH 27/33] Removed old, unused mocks. (#1382) --- contracts/mocks/ForceEther.sol | 15 ---------- contracts/mocks/MessageHelper.sol | 49 ------------------------------- 2 files changed, 64 deletions(-) delete mode 100644 contracts/mocks/ForceEther.sol delete mode 100644 contracts/mocks/MessageHelper.sol diff --git a/contracts/mocks/ForceEther.sol b/contracts/mocks/ForceEther.sol deleted file mode 100644 index bca5646f354..00000000000 --- a/contracts/mocks/ForceEther.sol +++ /dev/null @@ -1,15 +0,0 @@ -pragma solidity ^0.4.24; - -// @title Force Ether into a contract. -// @notice even -// if the contract is not payable. -// @notice To use, construct the contract with the target as argument. -// @author Remco Bloemen -contract ForceEther { - - constructor() public payable { } - - function destroyAndSend(address recipient) public { - selfdestruct(recipient); - } -} diff --git a/contracts/mocks/MessageHelper.sol b/contracts/mocks/MessageHelper.sol deleted file mode 100644 index 54c08441484..00000000000 --- a/contracts/mocks/MessageHelper.sol +++ /dev/null @@ -1,49 +0,0 @@ -pragma solidity ^0.4.24; - -contract MessageHelper { - - event Show(bytes32 b32, uint256 number, string text); - event Buy(bytes32 b32, uint256 number, string text, uint256 value); - - function showMessage( - bytes32 _message, - uint256 _number, - string _text - ) - public - returns (bool) - { - emit Show(_message, _number, _text); - return true; - } - - function buyMessage( - bytes32 _message, - uint256 _number, - string _text - ) - public - payable - returns (bool) - { - emit Buy( - _message, - _number, - _text, - msg.value); - return true; - } - - function fail() public { - require(false); - } - - function call(address _to, bytes _data) public returns (bool) { - // solium-disable-next-line security/no-low-level-calls - if (_to.call(_data)) - return true; - else - return false; - } - -} From 41f84f8b402efcda35f84ea9d7ffca4bb5f86499 Mon Sep 17 00:00:00 2001 From: Aniket <30843294+Aniket-Engg@users.noreply.github.com> Date: Mon, 8 Oct 2018 19:21:06 +0530 Subject: [PATCH 28/33] Removed selfdestruct from BreakInvariantBounty (#1385) * signing prefix added * Minor improvement * Tests changed * Successfully tested * Minor improvements * Minor improvements * Revert "Dangling commas are now required. (#1359)" This reverts commit a6889776f46adca374b6ebf014aa7b0038112a9d. * updates * fixes #1384 * introduced claimable and cancelBounty * cancelBounty tests * Update BreakInvariantBounty.test.js --- contracts/bounties/BreakInvariantBounty.sol | 26 +++++---- test/BreakInvariantBounty.test.js | 63 ++++++++++++++------- 2 files changed, 58 insertions(+), 31 deletions(-) diff --git a/contracts/bounties/BreakInvariantBounty.sol b/contracts/bounties/BreakInvariantBounty.sol index 6fd3b6e671d..52077a5392a 100644 --- a/contracts/bounties/BreakInvariantBounty.sol +++ b/contracts/bounties/BreakInvariantBounty.sol @@ -8,24 +8,25 @@ import "../ownership/Ownable.sol"; * @dev This bounty will pay out to a researcher if they break invariant logic of the contract. */ contract BreakInvariantBounty is PullPayment, Ownable { - bool private _claimed; + bool private _claimable = true; mapping(address => address) private _researchers; event TargetCreated(address createdAddress); + event BountyCanceled(); /** * @dev Fallback function allowing the contract to receive funds, if they haven't already been claimed. */ function() external payable { - require(!_claimed); + require(_claimable); } /** - * @dev Determine if the bounty was claimed. - * @return true if the bounty was claimed, false otherwise. + * @dev Determine if the bounty is claimable. + * @return false if the bounty was claimed, true otherwise. */ - function claimed() public view returns(bool) { - return _claimed; + function claimable() public view returns(bool) { + return _claimable; } /** @@ -45,20 +46,23 @@ contract BreakInvariantBounty is PullPayment, Ownable { * @param target contract */ function claim(Target target) public { - require(!_claimed); + require(_claimable); address researcher = _researchers[target]; require(researcher != address(0)); // Check Target contract invariants require(!target.checkInvariant()); _asyncTransfer(researcher, address(this).balance); - _claimed = true; + _claimable = false; } /** - * @dev Transfers the current balance to the owner and terminates the contract. + * @dev Cancels the bounty and transfers all funds to the owner */ - function destroy() public onlyOwner { - selfdestruct(owner()); + function cancelBounty() public onlyOwner{ + require(_claimable); + _asyncTransfer(owner(), address(this).balance); + _claimable = false; + emit BountyCanceled(); } /** diff --git a/test/BreakInvariantBounty.test.js b/test/BreakInvariantBounty.test.js index 50ae96aaa05..5995f64f599 100644 --- a/test/BreakInvariantBounty.test.js +++ b/test/BreakInvariantBounty.test.js @@ -1,4 +1,5 @@ const { ethGetBalance, ethSendTransaction } = require('./helpers/web3'); +const { ether } = require('./helpers/ether'); const { sendEther } = require('./helpers/sendTransaction'); const { balanceDifference } = require('./helpers/balanceDiff'); const expectEvent = require('./helpers/expectEvent'); @@ -11,7 +12,7 @@ require('chai') .use(require('chai-bignumber')(web3.BigNumber)) .should(); -const reward = new web3.BigNumber(web3.toWei(1, 'ether')); +const reward = ether(1); contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTarget]) { beforeEach(async function () { @@ -28,24 +29,9 @@ contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTar await sendEther(owner, this.bounty.address, reward); }); - describe('destroy', function () { - it('returns all balance to the owner', async function () { - const ownerPreBalance = await ethGetBalance(owner); - await this.bounty.destroy({ from: owner, gasPrice: 0 }); - const ownerPostBalance = await ethGetBalance(owner); - - (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0); - ownerPostBalance.sub(ownerPreBalance).should.be.bignumber.equal(reward); - }); - - it('reverts when called by anyone', async function () { - await assertRevert(this.bounty.destroy({ from: anyone })); - }); - }); - describe('claim', function () { - it('is initially unclaimed', async function () { - (await this.bounty.claimed()).should.equal(false); + it('is initially claimable', async function () { + (await this.bounty.claimable()).should.equal(true); }); it('can create claimable target', async function () { @@ -83,8 +69,8 @@ contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTar await this.bounty.claim(this.target.address, { from: researcher }); }); - it('is claimed', async function () { - (await this.bounty.claimed()).should.equal(true); + it('is not claimable', async function () { + (await this.bounty.claimable()).should.equal(false); }); it('no longer accepts rewards', async function () { @@ -104,5 +90,42 @@ contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTar }); }); }); + + describe('cancelBounty', function () { + context('before canceling', function () { + it('is claimable', async function () { + (await this.bounty.claimable()).should.equal(true); + }); + + it('can be canceled by the owner', async function () { + const { logs } = await this.bounty.cancelBounty({ from: owner }); + expectEvent.inLogs(logs, 'BountyCanceled'); + (await balanceDifference(owner, () => this.bounty.withdrawPayments(owner))) + .should.be.bignumber.equal(reward); + }); + + it('reverts when canceled by anyone', async function () { + await assertRevert(this.bounty.cancelBounty({ from: anyone })); + }); + }); + + context('after canceling', async function () { + beforeEach(async function () { + await this.bounty.cancelBounty({ from: owner }); + }); + + it('is not claimable', async function () { + (await this.bounty.claimable()).should.equal(false); + }); + + it('no longer accepts rewards', async function () { + await assertRevert(ethSendTransaction({ from: owner, to: this.bounty.address, value: reward })); + }); + + it('reverts when recanceled', async function () { + await assertRevert(this.bounty.cancelBounty({ from: owner })); + }); + }); + }); }); }); From f7e53d90fa638553ffc93e93fe1b12fc081bb774 Mon Sep 17 00:00:00 2001 From: Jakub Bogacz Date: Mon, 8 Oct 2018 16:01:33 +0200 Subject: [PATCH 29/33] Add Arrays library with unit tests (#1209) (#1375) * Add Arrays library with unit tests (#1209) * prepared due to snapshot token requirements * add library with method to find upper bound * add unit test for basic and edge cases * Imporove documentation for Arrays library Simplify Arrays.test.js to use short arrays as test date * Added comment for uint256 mid variable. * Explaned why uint256 mid variable calculated as Math.average is safe to use as index of array. --- contracts/mocks/ArraysImpl.sol | 18 +++++++ contracts/utils/Arrays.sol | 56 ++++++++++++++++++++++ test/utils/Arrays.test.js | 87 ++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+) create mode 100644 contracts/mocks/ArraysImpl.sol create mode 100644 contracts/utils/Arrays.sol create mode 100644 test/utils/Arrays.test.js diff --git a/contracts/mocks/ArraysImpl.sol b/contracts/mocks/ArraysImpl.sol new file mode 100644 index 00000000000..8a2b9ec51ae --- /dev/null +++ b/contracts/mocks/ArraysImpl.sol @@ -0,0 +1,18 @@ +pragma solidity ^0.4.24; + +import "../utils/Arrays.sol"; + +contract ArraysImpl { + + using Arrays for uint256[]; + + uint256[] private array; + + constructor(uint256[] _array) public { + array = _array; + } + + function findUpperBound(uint256 _element) external view returns (uint256) { + return array.findUpperBound(_element); + } +} diff --git a/contracts/utils/Arrays.sol b/contracts/utils/Arrays.sol new file mode 100644 index 00000000000..6882ea99595 --- /dev/null +++ b/contracts/utils/Arrays.sol @@ -0,0 +1,56 @@ +pragma solidity ^0.4.23; + +import "../math/Math.sol"; + + +/** + * @title Arrays + * @dev Utility library of inline array functions + */ +library Arrays { + + /** + * @dev Upper bound search function which is kind of binary search algoritm. It searches sorted + * array to find index of the element value. If element is found then returns it's index otherwise + * it returns index of first element which is grater than searched value. If searched element is + * bigger than any array element function then returns first index after last element (i.e. all + * values inside the array are smaller than the target). Complexity O(log n). + * @param array The array sorted in ascending order. + * @param element The element's value to be find. + * @return The calculated index value. Returns 0 for empty array. + */ + function findUpperBound( + uint256[] storage array, + uint256 element + ) + internal + view + returns (uint256) + { + if (array.length == 0) { + return 0; + } + + uint256 low = 0; + uint256 high = array.length; + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds down (it does integer division with truncation). + if (array[mid] > element) { + high = mid; + } else { + low = mid + 1; + } + } + + // At this point `low` is the exclusive upper bound. We will return the inclusive upper bound. + if (low > 0 && array[low - 1] == element) { + return low - 1; + } else { + return low; + } + } +} diff --git a/test/utils/Arrays.test.js b/test/utils/Arrays.test.js new file mode 100644 index 00000000000..4bfc9eabacc --- /dev/null +++ b/test/utils/Arrays.test.js @@ -0,0 +1,87 @@ +const ArraysImpl = artifacts.require('ArraysImpl'); + +const BigNumber = web3.BigNumber; + +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('Arrays', function () { + context('Even number of elements', function () { + const EVEN_ELEMENTS_ARRAY = [11, 12, 13, 14, 15, 16, 17, 18, 19, 20]; + + beforeEach(async function () { + this.arrays = await ArraysImpl.new(EVEN_ELEMENTS_ARRAY); + }); + + it('should return correct index for the basic case', async function () { + (await this.arrays.findUpperBound(16)).should.be.bignumber.equal(5); + }); + + it('should return 0 for the first element', async function () { + (await this.arrays.findUpperBound(11)).should.be.bignumber.equal(0); + }); + + it('should return index of the last element', async function () { + (await this.arrays.findUpperBound(20)).should.be.bignumber.equal(9); + }); + + it('should return first index after last element if searched value is over the upper boundary', async function () { + (await this.arrays.findUpperBound(32)).should.be.bignumber.equal(10); + }); + + it('should return 0 for the element under the lower boundary', async function () { + (await this.arrays.findUpperBound(2)).should.be.bignumber.equal(0); + }); + }); + + context('Odd number of elements', function () { + const ODD_ELEMENTS_ARRAY = [11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]; + + beforeEach(async function () { + this.arrays = await ArraysImpl.new(ODD_ELEMENTS_ARRAY); + }); + + it('should return correct index for the basic case', async function () { + (await this.arrays.findUpperBound(16)).should.be.bignumber.equal(5); + }); + + it('should return 0 for the first element', async function () { + (await this.arrays.findUpperBound(11)).should.be.bignumber.equal(0); + }); + + it('should return index of the last element', async function () { + (await this.arrays.findUpperBound(21)).should.be.bignumber.equal(10); + }); + + it('should return first index after last element if searched value is over the upper boundary', async function () { + (await this.arrays.findUpperBound(32)).should.be.bignumber.equal(11); + }); + + it('should return 0 for the element under the lower boundary', async function () { + (await this.arrays.findUpperBound(2)).should.be.bignumber.equal(0); + }); + }); + + context('Array with gap', function () { + const WITH_GAP_ARRAY = [11, 12, 13, 14, 15, 20, 21, 22, 23, 24]; + + beforeEach(async function () { + this.arrays = await ArraysImpl.new(WITH_GAP_ARRAY); + }); + + it('should return index of first element in next filled range', async function () { + (await this.arrays.findUpperBound(17)).should.be.bignumber.equal(5); + }); + }); + + context('Empty array', function () { + beforeEach(async function () { + this.arrays = await ArraysImpl.new([]); + }); + + it('should always return 0 for empty array', async function () { + (await this.arrays.findUpperBound(10)).should.be.bignumber.equal(0); + }); + }); +}); From 3acc2b4216394d4d66b717f40891e64a99aed8c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Mon, 8 Oct 2018 20:01:35 -0300 Subject: [PATCH 30/33] Added a constructor to BreakInvariantBounty. (#1395) --- contracts/bounties/BreakInvariantBounty.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contracts/bounties/BreakInvariantBounty.sol b/contracts/bounties/BreakInvariantBounty.sol index 52077a5392a..6dec45a3ba8 100644 --- a/contracts/bounties/BreakInvariantBounty.sol +++ b/contracts/bounties/BreakInvariantBounty.sol @@ -8,12 +8,16 @@ import "../ownership/Ownable.sol"; * @dev This bounty will pay out to a researcher if they break invariant logic of the contract. */ contract BreakInvariantBounty is PullPayment, Ownable { - bool private _claimable = true; + bool private _claimable; mapping(address => address) private _researchers; event TargetCreated(address createdAddress); event BountyCanceled(); + constructor() public { + _claimable = true; + } + /** * @dev Fallback function allowing the contract to receive funds, if they haven't already been claimed. */ From aa6b6d054d4e050715eed26d61c5951137bafc89 Mon Sep 17 00:00:00 2001 From: Jakub Bogacz Date: Tue, 18 Sep 2018 20:51:22 +0200 Subject: [PATCH 31/33] Add ERC20 token with snapshoting mechanism (#1209) --- contracts/mocks/ERC20SnapshotMock.sol | 12 +++ contracts/token/ERC20/ERC20Snapshot.sol | 96 ++++++++++++++++++++ test/token/ERC20/ERC20Snapshot.test.js | 113 ++++++++++++++++++++++++ 3 files changed, 221 insertions(+) create mode 100644 contracts/mocks/ERC20SnapshotMock.sol create mode 100644 contracts/token/ERC20/ERC20Snapshot.sol create mode 100644 test/token/ERC20/ERC20Snapshot.test.js diff --git a/contracts/mocks/ERC20SnapshotMock.sol b/contracts/mocks/ERC20SnapshotMock.sol new file mode 100644 index 00000000000..f8c386afba6 --- /dev/null +++ b/contracts/mocks/ERC20SnapshotMock.sol @@ -0,0 +1,12 @@ +pragma solidity ^0.4.24; + +import "../token/ERC20/ERC20Snapshot.sol"; + + +contract ERC20SnapshotMock is ERC20Snapshot { + + constructor(address initialAccount, uint256 initialBalance) public { + _mint(initialAccount, initialBalance); + } + +} diff --git a/contracts/token/ERC20/ERC20Snapshot.sol b/contracts/token/ERC20/ERC20Snapshot.sol new file mode 100644 index 00000000000..1d2ca8e2689 --- /dev/null +++ b/contracts/token/ERC20/ERC20Snapshot.sol @@ -0,0 +1,96 @@ +pragma solidity ^0.4.24; + +import "./ERC20.sol"; +import "../../utils/Arrays.sol"; + + +/** + * @title ERC20Snapshot token + * @dev An ERC20 token which enables taking snapshots of accounts' balances. This can be useful to + * safely implement voting weighed by balance. + */ +contract ERC20Snapshot is ERC20 { + + using Arrays for uint256[]; + + // The 0 id represents no snapshot was taken yet. + uint256 public currentSnapshotId; + + mapping (address => uint256[]) private snapshotIds; + mapping (address => uint256[]) private snapshotBalances; + + event Snapshot(uint256 id); + + function snapshot() external returns (uint256) { + currentSnapshotId += 1; + emit Snapshot(currentSnapshotId); + return currentSnapshotId; + } + + function snapshotsLength(address account) external view returns (uint256) { + return snapshotIds[account].length; + } + + function balanceOfAt( + address account, + uint256 snapshotId + ) + external + view + returns (uint256) + { + require( + snapshotId > 0 && snapshotId <= currentSnapshotId, + "Parameter snapshotId has to be greater than 0 and lower/equal currentSnapshot"); + + uint256 idx = snapshotIds[account].findUpperBound(snapshotId); + + if (idx == snapshotIds[account].length) { + return balanceOf(account); + } else { + return snapshotBalances[account][idx]; + } + } + + function transfer( + address to, + uint256 value + ) + public + returns (bool) + { + updateSnapshot(msg.sender); + updateSnapshot(to); + return super.transfer(to, value); + } + + function transferFrom( + address from, + address to, + uint256 value + ) + public + returns (bool) + { + updateSnapshot(from); + updateSnapshot(to); + return super.transferFrom(from, to, value); + } + + function updateSnapshot(address account) internal { + if (lastSnapshotId(account) < currentSnapshotId) { + snapshotIds[account].push(currentSnapshotId); + snapshotBalances[account].push(balanceOf(account)); + } + } + + function lastSnapshotId(address account) internal view returns (uint256) { + uint256[] storage snapshots = snapshotIds[account]; + if (snapshots.length == 0) { + return 0; + } else { + return snapshots[snapshots.length - 1]; + } + } + +} diff --git a/test/token/ERC20/ERC20Snapshot.test.js b/test/token/ERC20/ERC20Snapshot.test.js new file mode 100644 index 00000000000..ba6f4f84a26 --- /dev/null +++ b/test/token/ERC20/ERC20Snapshot.test.js @@ -0,0 +1,113 @@ +const ERC20Snapshot = artifacts.require('ERC20SnapshotMock'); +const { expectThrow } = require('../../helpers/expectThrow'); +const { EVMRevert } = require('../../helpers/EVMRevert'); +const BigNumber = web3.BigNumber; + +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('ERC20Snapshot', function ([_, owner, recipient, spender]) { + beforeEach(async function () { + this.token = await ERC20Snapshot.new(owner, 100); + }); + + context('there is no snapshots yet', function () { + describe('balanceOfAt', function () { + it('rejected for snapshotId parameter equals to 0', async function () { + await expectThrow( + this.token.balanceOfAt(owner, 0), + EVMRevert, + ); + }); + + it('rejected for snapshotId parameter greather than currentSnapshotId', async function () { + await expectThrow( + this.token.balanceOfAt(owner, 1), + EVMRevert, + ); + }); + }); + + describe('after transfer', function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('balanceOf returns correct value', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(90); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(10); + }); + + it('snapshots do not exist', async function () { + (await this.token.snapshotsLength(owner)).should.be.bignumber.equal(0); + (await this.token.snapshotsLength(recipient)).should.be.bignumber.equal(0); + }); + }); + }); + + context('snapshots exist', function () { + beforeEach(async function () { + await this.token.snapshot(); + }); + + describe('accounts do not have snapshots yet', function () { + it('snapshot value of the owner account equals to his balance', async function () { + const balanceOfAt = (await this.token.balanceOfAt(owner, 1)); + const balanceOf = (await this.token.balanceOf(owner)); + balanceOfAt.should.be.bignumber.equal(balanceOf); + }); + + it('snapshot value of the recipient account equals to balance', async function () { + const balanceOfAt = (await this.token.balanceOfAt(recipient, 1)); + const balanceOf = (await this.token.balanceOf(recipient)); + balanceOfAt.should.be.bignumber.equal(balanceOf); + }); + }); + + describe('accounts have already one snapshot', function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('snapshot keeps previous balance', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + + it('account has correct balance', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(90); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(10); + }); + }); + + describe('snapshot keeps previous balance even for multiple transfers', async function () { + beforeEach(async function () { + await this.token.transfer(recipient, 10, { from: owner }); + await this.token.transfer(recipient, 10, { from: owner }); + }); + + it('snapshot has previous balance', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + + it('account has correct balance', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(80); + (await this.token.balanceOf(recipient)).should.be.bignumber.equal(20); + }); + }); + + describe('snapshot keeps correct values for transfer from action', async function () { + beforeEach(async function () { + await this.token.approve(spender, 20, { from: owner }); + await this.token.transferFrom(owner, recipient, 20, { from: spender }); + }); + + it('spender and recipient snapshot is stored', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); + }); + }); + }); +}); From 7f59f861e1966aeebc6a53a617b6857d75a293bc Mon Sep 17 00:00:00 2001 From: jbogacz Date: Sun, 23 Sep 2018 11:51:10 +0200 Subject: [PATCH 32/33] Methods visibility corrected Require reason message removed Inline documentation in NatSpec added Snapshot added to _mint and _burn method --- contracts/mocks/ERC20SnapshotMock.sol | 7 ++ contracts/token/ERC20/ERC20Snapshot.sol | 86 +++++++++++++++++-------- test/token/ERC20/ERC20Snapshot.test.js | 33 ++++++++-- 3 files changed, 95 insertions(+), 31 deletions(-) diff --git a/contracts/mocks/ERC20SnapshotMock.sol b/contracts/mocks/ERC20SnapshotMock.sol index f8c386afba6..51c67a4502e 100644 --- a/contracts/mocks/ERC20SnapshotMock.sol +++ b/contracts/mocks/ERC20SnapshotMock.sol @@ -9,4 +9,11 @@ contract ERC20SnapshotMock is ERC20Snapshot { _mint(initialAccount, initialBalance); } + function mint(address account, uint256 amount) public { + _mint(account, amount); + } + + function burn(address account, uint256 amount) public { + _burn(account, amount); + } } diff --git a/contracts/token/ERC20/ERC20Snapshot.sol b/contracts/token/ERC20/ERC20Snapshot.sol index 1d2ca8e2689..cc34e0c618e 100644 --- a/contracts/token/ERC20/ERC20Snapshot.sol +++ b/contracts/token/ERC20/ERC20Snapshot.sol @@ -6,42 +6,46 @@ import "../../utils/Arrays.sol"; /** * @title ERC20Snapshot token - * @dev An ERC20 token which enables taking snapshots of accounts' balances. This can be useful to - * safely implement voting weighed by balance. + * @dev An ERC20 token which enables taking snapshots of account balances. + * This can be useful to safely implement voting weighed by balance. */ contract ERC20Snapshot is ERC20 { using Arrays for uint256[]; // The 0 id represents no snapshot was taken yet. - uint256 public currentSnapshotId; + uint256 private currentSnapshotId; mapping (address => uint256[]) private snapshotIds; mapping (address => uint256[]) private snapshotBalances; event Snapshot(uint256 id); + /** + * @dev Increments current snapshot. Emites Snapshot event. + * @return An uint256 representing current snapshot id. + */ function snapshot() external returns (uint256) { currentSnapshotId += 1; emit Snapshot(currentSnapshotId); return currentSnapshotId; } - function snapshotsLength(address account) external view returns (uint256) { - return snapshotIds[account].length; - } - + /** + * @dev Returns account balance for specific snapshot. + * @param account address The address to query the balance of. + * @param snapshotId uint256 The snapshot id for which to query the balance of. + * @return An uint256 representing the amount owned by the passed address for specific snapshot. + */ function balanceOfAt( address account, uint256 snapshotId ) - external - view - returns (uint256) + public + view + returns (uint256) { - require( - snapshotId > 0 && snapshotId <= currentSnapshotId, - "Parameter snapshotId has to be greater than 0 and lower/equal currentSnapshot"); + require(snapshotId > 0 && snapshotId <= currentSnapshotId); uint256 idx = snapshotIds[account].findUpperBound(snapshotId); @@ -52,39 +56,69 @@ contract ERC20Snapshot is ERC20 { } } - function transfer( - address to, - uint256 value - ) - public - returns (bool) - { + /** + * @dev Transfer token for a specified address. It takes balance snapshot for the sender and recipient account + * before transfer is done. + * @param to The address to transfer to. + * @param value The amount to be transferred. + */ + function transfer(address to, uint256 value) public returns (bool) { updateSnapshot(msg.sender); updateSnapshot(to); return super.transfer(to, value); } + /** + * @dev Transfer tokens from one address to another. It takes balance snapshot of both accounts before + * the transfer is done. + * @param from address The address which you want to send tokens from + * @param to address The address which you want to transfer to + * @param value uint256 the amount of tokens to be transferred + */ function transferFrom( - address from, - address to, + address from, + address to, uint256 value ) - public - returns (bool) + public + returns (bool) { updateSnapshot(from); updateSnapshot(to); return super.transferFrom(from, to, value); } - function updateSnapshot(address account) internal { + /** + * @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 + * proper events are emitted. Takes snapshot before tokens are minted. + * @param account The account that will receive the created tokens. + * @param amount The amount that will be created. + */ + function _mint(address account, uint256 amount) internal { + updateSnapshot(account); + super._mint(account, amount); + } + + /** + * @dev Internal function that burns an amount of the token of a given + * account. Takes snapshot before tokens are burned. + * @param account The account whose tokens will be burnt. + * @param amount The amount that will be burnt. + */ + function _burn(address account, uint256 amount) internal { + updateSnapshot(account); + super._burn(account, amount); + } + + function updateSnapshot(address account) private { if (lastSnapshotId(account) < currentSnapshotId) { snapshotIds[account].push(currentSnapshotId); snapshotBalances[account].push(balanceOf(account)); } } - function lastSnapshotId(address account) internal view returns (uint256) { + function lastSnapshotId(address account) private view returns (uint256) { uint256[] storage snapshots = snapshotIds[account]; if (snapshots.length == 0) { return 0; diff --git a/test/token/ERC20/ERC20Snapshot.test.js b/test/token/ERC20/ERC20Snapshot.test.js index ba6f4f84a26..65534723791 100644 --- a/test/token/ERC20/ERC20Snapshot.test.js +++ b/test/token/ERC20/ERC20Snapshot.test.js @@ -38,11 +38,6 @@ contract('ERC20Snapshot', function ([_, owner, recipient, spender]) { (await this.token.balanceOf(owner)).should.be.bignumber.equal(90); (await this.token.balanceOf(recipient)).should.be.bignumber.equal(10); }); - - it('snapshots do not exist', async function () { - (await this.token.snapshotsLength(owner)).should.be.bignumber.equal(0); - (await this.token.snapshotsLength(recipient)).should.be.bignumber.equal(0); - }); }); }); @@ -109,5 +104,33 @@ contract('ERC20Snapshot', function ([_, owner, recipient, spender]) { (await this.token.balanceOfAt(recipient, 1)).should.be.bignumber.equal(0); }); }); + + describe('new tokens are minted', function () { + beforeEach(async function () { + await this.token.mint(owner, 50); + }); + + it('snapshot keeps balance before mint', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + }); + + it('current balance is greater after mint', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(150); + }); + }); + + describe('chunk tokens are burned', function () { + beforeEach(async function () { + await this.token.burn(owner, 30); + }); + + it('snapshot keeps balance before burn', async function () { + (await this.token.balanceOfAt(owner, 1)).should.be.bignumber.equal(100); + }); + + it('crrent balance is lower after burn', async function () { + (await this.token.balanceOf(owner)).should.be.bignumber.equal(70); + }); + }); }); }); From 360c85e0fbf30fb8c58ca80a06d8a0affe660a96 Mon Sep 17 00:00:00 2001 From: jbogacz Date: Tue, 9 Oct 2018 06:58:14 +0200 Subject: [PATCH 33/33] Use SafeMath. --- contracts/token/ERC20/ERC20Snapshot.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC20/ERC20Snapshot.sol b/contracts/token/ERC20/ERC20Snapshot.sol index cc34e0c618e..0e7672cdd8a 100644 --- a/contracts/token/ERC20/ERC20Snapshot.sol +++ b/contracts/token/ERC20/ERC20Snapshot.sol @@ -2,6 +2,7 @@ pragma solidity ^0.4.24; import "./ERC20.sol"; import "../../utils/Arrays.sol"; +import "../../math/SafeMath.sol"; /** @@ -10,7 +11,7 @@ import "../../utils/Arrays.sol"; * This can be useful to safely implement voting weighed by balance. */ contract ERC20Snapshot is ERC20 { - + using SafeMath for uint256; using Arrays for uint256[]; // The 0 id represents no snapshot was taken yet. @@ -26,7 +27,7 @@ contract ERC20Snapshot is ERC20 { * @return An uint256 representing current snapshot id. */ function snapshot() external returns (uint256) { - currentSnapshotId += 1; + currentSnapshotId = currentSnapshotId.add(1); emit Snapshot(currentSnapshotId); return currentSnapshotId; }