From 672ad2513004cf9b6992ac1e9e0d87f486151668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 28 Nov 2018 09:42:26 -0300 Subject: [PATCH 01/14] Added WhitelisterRole. --- contracts/access/roles/WhitelisterRole.sol | 43 ++++++++++++++++++++++ contracts/mocks/WhitelisterRoleMock.sol | 17 +++++++++ test/access/roles/WhitelisterRole.test.js | 11 ++++++ 3 files changed, 71 insertions(+) create mode 100644 contracts/access/roles/WhitelisterRole.sol create mode 100644 contracts/mocks/WhitelisterRoleMock.sol create mode 100644 test/access/roles/WhitelisterRole.test.js diff --git a/contracts/access/roles/WhitelisterRole.sol b/contracts/access/roles/WhitelisterRole.sol new file mode 100644 index 00000000000..41f0ddf421c --- /dev/null +++ b/contracts/access/roles/WhitelisterRole.sol @@ -0,0 +1,43 @@ +pragma solidity ^0.4.24; + +import "../Roles.sol"; + +contract WhitelisterRole { + using Roles for Roles.Role; + + event WhitelisterAdded(address indexed account); + event WhitelisterRemoved(address indexed account); + + Roles.Role private _whitelisters; + + constructor () internal { + _addWhitelister(msg.sender); + } + + modifier onlyWhitelister() { + require(isWhitelister(msg.sender)); + _; + } + + function isWhitelister(address account) public view returns (bool) { + return _whitelisters.has(account); + } + + function addWhitelister(address account) public onlyWhitelister { + _addWhitelister(account); + } + + function renounceWhitelister() public { + _removeWhitelister(msg.sender); + } + + function _addWhitelister(address account) internal { + _whitelisters.add(account); + emit WhitelisterAdded(account); + } + + function _removeWhitelister(address account) internal { + _whitelisters.remove(account); + emit WhitelisterRemoved(account); + } +} diff --git a/contracts/mocks/WhitelisterRoleMock.sol b/contracts/mocks/WhitelisterRoleMock.sol new file mode 100644 index 00000000000..06ea7dd63bf --- /dev/null +++ b/contracts/mocks/WhitelisterRoleMock.sol @@ -0,0 +1,17 @@ +pragma solidity ^0.4.24; + +import "../access/roles/WhitelisterRole.sol"; + +contract WhitelisterRoleMock is WhitelisterRole { + function removeWhitelister(address account) public { + _removeWhitelister(account); + } + + function onlyWhitelisterMock() public view onlyWhitelister { + } + + // Causes a compilation error if super._removeWhitelister is not internal + function _removeWhitelister(address account) internal { + super._removeWhitelister(account); + } +} diff --git a/test/access/roles/WhitelisterRole.test.js b/test/access/roles/WhitelisterRole.test.js new file mode 100644 index 00000000000..39e6644074d --- /dev/null +++ b/test/access/roles/WhitelisterRole.test.js @@ -0,0 +1,11 @@ +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); +const WhitelisterRoleMock = artifacts.require('WhitelisterRoleMock'); + +contract('WhitelisterRole', function ([_, whitelister, otherWhitelister, ...otherAccounts]) { + beforeEach(async function () { + this.contract = await WhitelisterRoleMock.new({ from: whitelister }); + await this.contract.addWhitelister(otherWhitelister, { from: whitelister }); + }); + + shouldBehaveLikePublicRole(whitelister, otherWhitelister, otherAccounts, 'whitelister'); +}); From 7575fd215f71598e9746a087d9c2df99ae7595de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 28 Nov 2018 09:58:52 -0300 Subject: [PATCH 02/14] Added WhitelisteeRole and WhitelistedCrowdsale. --- contracts/access/roles/WhitelisteeRole.sol | 40 +++++++++++++++++++ .../validation/WhitelistedCrowdsale.sol | 20 ++++++++++ 2 files changed, 60 insertions(+) create mode 100644 contracts/access/roles/WhitelisteeRole.sol create mode 100644 contracts/crowdsale/validation/WhitelistedCrowdsale.sol diff --git a/contracts/access/roles/WhitelisteeRole.sol b/contracts/access/roles/WhitelisteeRole.sol new file mode 100644 index 00000000000..804506e16d5 --- /dev/null +++ b/contracts/access/roles/WhitelisteeRole.sol @@ -0,0 +1,40 @@ +pragma solidity ^0.4.24; + +import "../Roles.sol"; +import "./WhitelisterRole.sol"; + +contract WhitelisteeRole is WhitelisterRole { + using Roles for Roles.Role; + + event WhitelisteeAdded(address indexed account); + event WhitelisteeRemoved(address indexed account); + + Roles.Role private _whitelistees; + + modifier onlyWhitelistee() { + require(isWhitelistee(msg.sender)); + _; + } + + function isWhitelistee(address account) public view returns (bool) { + return _whitelistees.has(account); + } + + function addWhitelistee(address account) public onlyWhitelister { + _addWhitelistee(account); + } + + function renounceWhitelistee() public { + _removeWhitelistee(msg.sender); + } + + function _addWhitelistee(address account) internal { + _whitelistees.add(account); + emit WhitelisteeAdded(account); + } + + function _removeWhitelistee(address account) internal { + _whitelistees.remove(account); + emit WhitelisteeRemoved(account); + } +} diff --git a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol b/contracts/crowdsale/validation/WhitelistedCrowdsale.sol new file mode 100644 index 00000000000..9d086df6abe --- /dev/null +++ b/contracts/crowdsale/validation/WhitelistedCrowdsale.sol @@ -0,0 +1,20 @@ +pragma solidity ^0.4.24; +import "../Crowdsale.sol"; +import "../../access/roles/WhitelisteeRole.sol"; + + +/** + * @title WhitelistedCrowdsale + * @dev Crowdsale in which only whitelisted users can contribute. + */ +contract WhitelistedCrowdsale is WhitelisteeRole, Crowdsale { + /** + * @dev Extend parent behavior requiring beneficiary to be whitelisted. + * @param _beneficiary Token beneficiary + * @param _weiAmount Amount of wei contributed + */ + function _preValidatePurchase(address _beneficiary, uint256 _weiAmount) internal view { + require(isWhitelistee(_beneficiary)); + super._preValidatePurchase(_beneficiary, _weiAmount); + } +} From 1165ecfcd636cba5d744088a7a0fbbac5555b9eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 28 Nov 2018 12:08:54 -0300 Subject: [PATCH 03/14] Added WhitelistedCrowdsale tests. --- contracts/mocks/WhitelistedCrowdsaleImpl.sol | 10 ++++ test/crowdsale/WhitelistedCrowdsale.test.js | 55 ++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 contracts/mocks/WhitelistedCrowdsaleImpl.sol create mode 100644 test/crowdsale/WhitelistedCrowdsale.test.js diff --git a/contracts/mocks/WhitelistedCrowdsaleImpl.sol b/contracts/mocks/WhitelistedCrowdsaleImpl.sol new file mode 100644 index 00000000000..3a52d0c3a00 --- /dev/null +++ b/contracts/mocks/WhitelistedCrowdsaleImpl.sol @@ -0,0 +1,10 @@ +pragma solidity ^0.4.24; + +import "../token/ERC20/IERC20.sol"; +import "../crowdsale/validation/WhitelistedCrowdsale.sol"; +import "../crowdsale/Crowdsale.sol"; + + +contract WhitelistedCrowdsaleImpl is Crowdsale, WhitelistedCrowdsale { + constructor (uint256 _rate, address _wallet, IERC20 _token) Crowdsale(_rate, _wallet, _token) public {} +} diff --git a/test/crowdsale/WhitelistedCrowdsale.test.js b/test/crowdsale/WhitelistedCrowdsale.test.js new file mode 100644 index 00000000000..7080a8458fd --- /dev/null +++ b/test/crowdsale/WhitelistedCrowdsale.test.js @@ -0,0 +1,55 @@ +const { ether } = require('../helpers/ether'); +const shouldFail = require('../helpers/shouldFail'); + +const BigNumber = web3.BigNumber; + +require('chai') + .should(); + +const WhitelistedCrowdsale = artifacts.require('WhitelistedCrowdsaleImpl'); +const SimpleToken = artifacts.require('SimpleToken'); + +contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelistee, otherWhitelistee, anyone]) { + const rate = 1; + const value = ether(42); + const tokenSupply = new BigNumber('1e22'); + + beforeEach(async function () { + this.token = await SimpleToken.new({ from: whitelister }); + this.crowdsale = await WhitelistedCrowdsale.new(rate, wallet, this.token.address, { from: whitelister }); + await this.token.transfer(this.crowdsale.address, tokenSupply, { from: whitelister }); + }); + + async function purchaseShouldSucceed (crowdsale, beneficiary, value) { + await crowdsale.buyTokens(beneficiary, { from: beneficiary, value }); + await crowdsale.sendTransaction({ from: beneficiary, value }); + } + + async function purchaseShouldFail (crowdsale, beneficiary, value) { + await shouldFail.reverting(crowdsale.buyTokens(beneficiary, { from: beneficiary, value })); + await shouldFail.reverting(crowdsale.sendTransaction({ from: beneficiary, value })); + } + + context('with no whitelisted addresses', function () { + it('rejects payments from all addresses', async function () { + await purchaseShouldFail(this.crowdsale, anyone, value); + await purchaseShouldFail(this.crowdsale, whitelistee, value); + }); + }); + + context('with whitelited addresses', function () { + beforeEach(async function () { + await this.crowdsale.addWhitelistee(whitelistee, { from: whitelister }); + await this.crowdsale.addWhitelistee(otherWhitelistee, { from: whitelister }); + }); + + it('accepts payments from whitelisted addresses', async function () { + await purchaseShouldSucceed(this.crowdsale, whitelistee, value); + await purchaseShouldSucceed(this.crowdsale, otherWhitelistee, value); + }); + + it('rejects payments from unwhitelisted addresses', async function () { + await purchaseShouldFail(this.crowdsale, anyone, value); + }); + }); +}); From 0415ed5ef40ee5d07fe99f16aac60fae16311bd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 28 Nov 2018 13:30:40 -0300 Subject: [PATCH 04/14] Whitelisters can now remove Whitelistees. --- contracts/access/roles/WhitelisteeRole.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/access/roles/WhitelisteeRole.sol b/contracts/access/roles/WhitelisteeRole.sol index 804506e16d5..85f26a7d141 100644 --- a/contracts/access/roles/WhitelisteeRole.sol +++ b/contracts/access/roles/WhitelisteeRole.sol @@ -24,6 +24,10 @@ contract WhitelisteeRole is WhitelisterRole { _addWhitelistee(account); } + function removeWhitelistee(address account) public onlyWhitelister { + _removeWhitelistee(account); + } + function renounceWhitelistee() public { _removeWhitelistee(msg.sender); } From adc5f71ea1298fd9130d5764069d50ddafb256b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 28 Nov 2018 13:36:34 -0300 Subject: [PATCH 05/14] PublicRole.behavior now supports a manager account, added Whitelistee tests. --- contracts/mocks/WhitelisteeRoleMock.sol | 8 +++ test/access/roles/PublicRole.behavior.js | 74 +++++++++++++---------- test/access/roles/WhitelisteeRole.test.js | 12 ++++ 3 files changed, 62 insertions(+), 32 deletions(-) create mode 100644 contracts/mocks/WhitelisteeRoleMock.sol create mode 100644 test/access/roles/WhitelisteeRole.test.js diff --git a/contracts/mocks/WhitelisteeRoleMock.sol b/contracts/mocks/WhitelisteeRoleMock.sol new file mode 100644 index 00000000000..ec1a93992e6 --- /dev/null +++ b/contracts/mocks/WhitelisteeRoleMock.sol @@ -0,0 +1,8 @@ +pragma solidity ^0.4.24; + +import "../access/roles/WhitelisteeRole.sol"; + +contract WhitelisteeRoleMock is WhitelisteeRole { + function onlyWhitelisteeMock() public view onlyWhitelistee { + } +} diff --git a/test/access/roles/PublicRole.behavior.js b/test/access/roles/PublicRole.behavior.js index 996f843d1b4..62ed8d0ed04 100644 --- a/test/access/roles/PublicRole.behavior.js +++ b/test/access/roles/PublicRole.behavior.js @@ -9,7 +9,7 @@ function capitalize (str) { return str.replace(/\b\w/g, l => l.toUpperCase()); } -function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], rolename) { +function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], rolename, manager) { rolename = capitalize(rolename); describe('should behave like public role', function () { @@ -19,11 +19,13 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role (await this.contract[`is${rolename}`](anyone)).should.equal(false); }); - it('emits events during construction', async function () { - await expectEvent.inConstruction(this.contract, `${rolename}Added`, { - account: authorized, + if (!manager) { // Managed roles are only assigned by the manager, and none are set at construction + it('emits events during construction', async function () { + await expectEvent.inConstruction(this.contract, `${rolename}Added`, { + account: authorized, + }); }); - }); + } it('reverts when querying roles for the null account', async function () { await shouldFail.reverting(this.contract[`is${rolename}`](ZERO_ADDRESS)); @@ -48,43 +50,51 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role }); describe('add', function () { - it('adds role to a new account', async function () { - await this.contract[`add${rolename}`](anyone, { from: authorized }); - (await this.contract[`is${rolename}`](anyone)).should.equal(true); - }); + const from = manager || authorized; - it(`emits a ${rolename}Added event`, async function () { - const { logs } = await this.contract[`add${rolename}`](anyone, { from: authorized }); - expectEvent.inLogs(logs, `${rolename}Added`, { account: anyone }); - }); + context(`from ${manager ? 'the manager' : 'a role-haver'} account`, function () { + it('adds role to a new account', async function () { + await this.contract[`add${rolename}`](anyone, { from }); + (await this.contract[`is${rolename}`](anyone)).should.equal(true); + }); - it('reverts when adding role to an already assigned account', async function () { - await shouldFail.reverting(this.contract[`add${rolename}`](authorized, { from: authorized })); - }); + it(`emits a ${rolename}Added event`, async function () { + const { logs } = await this.contract[`add${rolename}`](anyone, { from }); + expectEvent.inLogs(logs, `${rolename}Added`, { account: anyone }); + }); + + it('reverts when adding role to an already assigned account', async function () { + await shouldFail.reverting(this.contract[`add${rolename}`](authorized, { from })); + }); - it('reverts when adding role to the null account', async function () { - await shouldFail.reverting(this.contract[`add${rolename}`](ZERO_ADDRESS, { from: authorized })); + it('reverts when adding role to the null account', async function () { + await shouldFail.reverting(this.contract[`add${rolename}`](ZERO_ADDRESS, { from })); + }); }); }); describe('remove', function () { - it('removes role from an already assigned account', async function () { - await this.contract[`remove${rolename}`](authorized); - (await this.contract[`is${rolename}`](authorized)).should.equal(false); - (await this.contract[`is${rolename}`](otherAuthorized)).should.equal(true); - }); + const from = manager || anyone; // Non-managed roles have no restrictions on the mocked 'remove' function - it(`emits a ${rolename}Removed event`, async function () { - const { logs } = await this.contract[`remove${rolename}`](authorized); - expectEvent.inLogs(logs, `${rolename}Removed`, { account: authorized }); - }); + context(`from ${manager ? 'the manager' : 'any'} account`, function () { + it('removes role from an already assigned account', async function () { + await this.contract[`remove${rolename}`](authorized, { from }); + (await this.contract[`is${rolename}`](authorized)).should.equal(false); + (await this.contract[`is${rolename}`](otherAuthorized)).should.equal(true); + }); - it('reverts when removing from an unassigned account', async function () { - await shouldFail.reverting(this.contract[`remove${rolename}`](anyone)); - }); + it(`emits a ${rolename}Removed event`, async function () { + const { logs } = await this.contract[`remove${rolename}`](authorized, { from }); + expectEvent.inLogs(logs, `${rolename}Removed`, { account: authorized }); + }); - it('reverts when removing role from the null account', async function () { - await shouldFail.reverting(this.contract[`remove${rolename}`](ZERO_ADDRESS)); + it('reverts when removing from an unassigned account', async function () { + await shouldFail.reverting(this.contract[`remove${rolename}`](anyone), { from }); + }); + + it('reverts when removing role from the null account', async function () { + await shouldFail.reverting(this.contract[`remove${rolename}`](ZERO_ADDRESS), { from }); + }); }); }); diff --git a/test/access/roles/WhitelisteeRole.test.js b/test/access/roles/WhitelisteeRole.test.js new file mode 100644 index 00000000000..a58ea005ef7 --- /dev/null +++ b/test/access/roles/WhitelisteeRole.test.js @@ -0,0 +1,12 @@ +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); +const WhitelisteeRoleMock = artifacts.require('WhitelisteeRoleMock'); + +contract('WhitelisteeRole', function ([_, whitelistee, otherWhitelistee, whitelister, ...otherAccounts]) { + beforeEach(async function () { + this.contract = await WhitelisteeRoleMock.new({ from: whitelister }); + await this.contract.addWhitelistee(whitelistee, { from: whitelister }); + await this.contract.addWhitelistee(otherWhitelistee, { from: whitelister }); + }); + + shouldBehaveLikePublicRole(whitelistee, otherWhitelistee, otherAccounts, 'whitelistee', whitelister); +}); From 6231db7336513aa05109d2be2d515ebe03185983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 29 Nov 2018 07:17:58 -0300 Subject: [PATCH 06/14] Rephrased tests, added test for whitelistees doing invalid purchases. --- .../crowdsale/validation/WhitelistedCrowdsale.sol | 3 ++- test/crowdsale/WhitelistedCrowdsale.test.js | 10 +++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol b/contracts/crowdsale/validation/WhitelistedCrowdsale.sol index 9d086df6abe..0cd997733ec 100644 --- a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol +++ b/contracts/crowdsale/validation/WhitelistedCrowdsale.sol @@ -9,7 +9,8 @@ import "../../access/roles/WhitelisteeRole.sol"; */ contract WhitelistedCrowdsale is WhitelisteeRole, Crowdsale { /** - * @dev Extend parent behavior requiring beneficiary to be whitelisted. + * @dev Extend parent behavior requiring beneficiary to be whitelisted. Note that no + * restriction is imposed on the account sending the transaction. * @param _beneficiary Token beneficiary * @param _weiAmount Amount of wei contributed */ diff --git a/test/crowdsale/WhitelistedCrowdsale.test.js b/test/crowdsale/WhitelistedCrowdsale.test.js index 7080a8458fd..43d8d2c7a86 100644 --- a/test/crowdsale/WhitelistedCrowdsale.test.js +++ b/test/crowdsale/WhitelistedCrowdsale.test.js @@ -31,7 +31,7 @@ contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelistee, } context('with no whitelisted addresses', function () { - it('rejects payments from all addresses', async function () { + it('rejects all purchases', async function () { await purchaseShouldFail(this.crowdsale, anyone, value); await purchaseShouldFail(this.crowdsale, whitelistee, value); }); @@ -43,12 +43,16 @@ contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelistee, await this.crowdsale.addWhitelistee(otherWhitelistee, { from: whitelister }); }); - it('accepts payments from whitelisted addresses', async function () { + it('accepts purchases with whitelisted beneficiaries', async function () { await purchaseShouldSucceed(this.crowdsale, whitelistee, value); await purchaseShouldSucceed(this.crowdsale, otherWhitelistee, value); }); - it('rejects payments from unwhitelisted addresses', async function () { + it('rejects purchases from whitelisted addresses with un-whitelisted beneficiaries', async function () { + await shouldFail(this.crowdsale.buyTokens(anyone, { from: whitelistee, value} )); + }); + + it('rejects purchases with whitelisted beneficiaries', async function () { await purchaseShouldFail(this.crowdsale, anyone, value); }); }); From 0b0eec37d5edd5705cee4ea6f85fc67c3d95693b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 29 Nov 2018 09:00:33 -0300 Subject: [PATCH 07/14] Fixed linter error. --- test/crowdsale/WhitelistedCrowdsale.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/crowdsale/WhitelistedCrowdsale.test.js b/test/crowdsale/WhitelistedCrowdsale.test.js index 43d8d2c7a86..496950f351c 100644 --- a/test/crowdsale/WhitelistedCrowdsale.test.js +++ b/test/crowdsale/WhitelistedCrowdsale.test.js @@ -49,7 +49,7 @@ contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelistee, }); it('rejects purchases from whitelisted addresses with un-whitelisted beneficiaries', async function () { - await shouldFail(this.crowdsale.buyTokens(anyone, { from: whitelistee, value} )); + await shouldFail(this.crowdsale.buyTokens(anyone, { from: whitelistee, value })); }); it('rejects purchases with whitelisted beneficiaries', async function () { From 85d2ac9d07c860ad0c2f5f2efcdc2a9c42cb9863 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Tue, 11 Dec 2018 18:54:32 -0300 Subject: [PATCH 08/14] Fixed typos Co-Authored-By: nventuro --- test/crowdsale/WhitelistedCrowdsale.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/crowdsale/WhitelistedCrowdsale.test.js b/test/crowdsale/WhitelistedCrowdsale.test.js index 496950f351c..82f1f3f5961 100644 --- a/test/crowdsale/WhitelistedCrowdsale.test.js +++ b/test/crowdsale/WhitelistedCrowdsale.test.js @@ -37,7 +37,7 @@ contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelistee, }); }); - context('with whitelited addresses', function () { + context('with whitelisted addresses', function () { beforeEach(async function () { await this.crowdsale.addWhitelistee(whitelistee, { from: whitelister }); await this.crowdsale.addWhitelistee(otherWhitelistee, { from: whitelister }); @@ -48,11 +48,11 @@ contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelistee, await purchaseShouldSucceed(this.crowdsale, otherWhitelistee, value); }); - it('rejects purchases from whitelisted addresses with un-whitelisted beneficiaries', async function () { + it('rejects purchases from whitelisted addresses with non-whitelisted beneficiaries', async function () { await shouldFail(this.crowdsale.buyTokens(anyone, { from: whitelistee, value })); }); - it('rejects purchases with whitelisted beneficiaries', async function () { + it('rejects purchases with non-whitelisted beneficiaries', async function () { await purchaseShouldFail(this.crowdsale, anyone, value); }); }); From ab5aebca22c25cde38f561f0826a48dc0257006d Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Tue, 11 Dec 2018 18:55:14 -0300 Subject: [PATCH 09/14] Working around JS quirks Co-Authored-By: nventuro --- test/access/roles/PublicRole.behavior.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/access/roles/PublicRole.behavior.js b/test/access/roles/PublicRole.behavior.js index 62ed8d0ed04..22030d26a19 100644 --- a/test/access/roles/PublicRole.behavior.js +++ b/test/access/roles/PublicRole.behavior.js @@ -19,7 +19,7 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role (await this.contract[`is${rolename}`](anyone)).should.equal(false); }); - if (!manager) { // Managed roles are only assigned by the manager, and none are set at construction + if (manager === undefined) { // Managed roles are only assigned by the manager, and none are set at construction it('emits events during construction', async function () { await expectEvent.inConstruction(this.contract, `${rolename}Added`, { account: authorized, @@ -50,7 +50,7 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role }); describe('add', function () { - const from = manager || authorized; + const from = manager === undefined ? authorized : manager; context(`from ${manager ? 'the manager' : 'a role-haver'} account`, function () { it('adds role to a new account', async function () { From c9baf52432170637fa0abbe82592536577d734c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Tue, 11 Dec 2018 18:55:55 -0300 Subject: [PATCH 10/14] Update PublicRole.behavior.js --- test/access/roles/PublicRole.behavior.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/access/roles/PublicRole.behavior.js b/test/access/roles/PublicRole.behavior.js index 22030d26a19..8bda9b75af0 100644 --- a/test/access/roles/PublicRole.behavior.js +++ b/test/access/roles/PublicRole.behavior.js @@ -74,7 +74,8 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role }); describe('remove', function () { - const from = manager || anyone; // Non-managed roles have no restrictions on the mocked 'remove' function + // Non-managed roles have no restrictions on the mocked '_remove' function (exposed via 'remove'). + const from = manager || anyone; context(`from ${manager ? 'the manager' : 'any'} account`, function () { it('removes role from an already assigned account', async function () { From 3ac745e270156716050ae547c93771b7ca28b9d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 12 Dec 2018 11:53:31 -0300 Subject: [PATCH 11/14] Renamed WhitelisteeRole to WhitelistedRole. --- contracts/access/roles/WhitelistedRole.sol | 44 +++++++++++++++++++ contracts/access/roles/WhitelisteeRole.sol | 44 ------------------- .../validation/WhitelistedCrowdsale.sol | 6 +-- contracts/mocks/WhitelistedRoleMock.sol | 8 ++++ contracts/mocks/WhitelisteeRoleMock.sol | 8 ---- test/access/roles/WhitelistedRole.test.js | 12 +++++ test/access/roles/WhitelisteeRole.test.js | 12 ----- test/crowdsale/WhitelistedCrowdsale.test.js | 14 +++--- 8 files changed, 74 insertions(+), 74 deletions(-) create mode 100644 contracts/access/roles/WhitelistedRole.sol delete mode 100644 contracts/access/roles/WhitelisteeRole.sol create mode 100644 contracts/mocks/WhitelistedRoleMock.sol delete mode 100644 contracts/mocks/WhitelisteeRoleMock.sol create mode 100644 test/access/roles/WhitelistedRole.test.js delete mode 100644 test/access/roles/WhitelisteeRole.test.js diff --git a/contracts/access/roles/WhitelistedRole.sol b/contracts/access/roles/WhitelistedRole.sol new file mode 100644 index 00000000000..537633c8e9f --- /dev/null +++ b/contracts/access/roles/WhitelistedRole.sol @@ -0,0 +1,44 @@ +pragma solidity ^0.4.24; + +import "../Roles.sol"; +import "./WhitelisterRole.sol"; + +contract WhitelistedRole is WhitelisterRole { + using Roles for Roles.Role; + + event WhitelistedAdded(address indexed account); + event WhitelistedRemoved(address indexed account); + + Roles.Role private _whitelisteds; + + modifier onlyWhitelisted() { + require(isWhitelisted(msg.sender)); + _; + } + + function isWhitelisted(address account) public view returns (bool) { + return _whitelisteds.has(account); + } + + function addWhitelisted(address account) public onlyWhitelister { + _addWhitelisted(account); + } + + function removeWhitelisted(address account) public onlyWhitelister { + _removeWhitelisted(account); + } + + function renounceWhitelisted() public { + _removeWhitelisted(msg.sender); + } + + function _addWhitelisted(address account) internal { + _whitelisteds.add(account); + emit WhitelistedAdded(account); + } + + function _removeWhitelisted(address account) internal { + _whitelisteds.remove(account); + emit WhitelistedRemoved(account); + } +} diff --git a/contracts/access/roles/WhitelisteeRole.sol b/contracts/access/roles/WhitelisteeRole.sol deleted file mode 100644 index 85f26a7d141..00000000000 --- a/contracts/access/roles/WhitelisteeRole.sol +++ /dev/null @@ -1,44 +0,0 @@ -pragma solidity ^0.4.24; - -import "../Roles.sol"; -import "./WhitelisterRole.sol"; - -contract WhitelisteeRole is WhitelisterRole { - using Roles for Roles.Role; - - event WhitelisteeAdded(address indexed account); - event WhitelisteeRemoved(address indexed account); - - Roles.Role private _whitelistees; - - modifier onlyWhitelistee() { - require(isWhitelistee(msg.sender)); - _; - } - - function isWhitelistee(address account) public view returns (bool) { - return _whitelistees.has(account); - } - - function addWhitelistee(address account) public onlyWhitelister { - _addWhitelistee(account); - } - - function removeWhitelistee(address account) public onlyWhitelister { - _removeWhitelistee(account); - } - - function renounceWhitelistee() public { - _removeWhitelistee(msg.sender); - } - - function _addWhitelistee(address account) internal { - _whitelistees.add(account); - emit WhitelisteeAdded(account); - } - - function _removeWhitelistee(address account) internal { - _whitelistees.remove(account); - emit WhitelisteeRemoved(account); - } -} diff --git a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol b/contracts/crowdsale/validation/WhitelistedCrowdsale.sol index 0cd997733ec..5e6be7461f0 100644 --- a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol +++ b/contracts/crowdsale/validation/WhitelistedCrowdsale.sol @@ -1,13 +1,13 @@ pragma solidity ^0.4.24; import "../Crowdsale.sol"; -import "../../access/roles/WhitelisteeRole.sol"; +import "../../access/roles/WhitelistedRole.sol"; /** * @title WhitelistedCrowdsale * @dev Crowdsale in which only whitelisted users can contribute. */ -contract WhitelistedCrowdsale is WhitelisteeRole, Crowdsale { +contract WhitelistedCrowdsale is WhitelistedRole, Crowdsale { /** * @dev Extend parent behavior requiring beneficiary to be whitelisted. Note that no * restriction is imposed on the account sending the transaction. @@ -15,7 +15,7 @@ contract WhitelistedCrowdsale is WhitelisteeRole, Crowdsale { * @param _weiAmount Amount of wei contributed */ function _preValidatePurchase(address _beneficiary, uint256 _weiAmount) internal view { - require(isWhitelistee(_beneficiary)); + require(isWhitelisted(_beneficiary)); super._preValidatePurchase(_beneficiary, _weiAmount); } } diff --git a/contracts/mocks/WhitelistedRoleMock.sol b/contracts/mocks/WhitelistedRoleMock.sol new file mode 100644 index 00000000000..b710c38d403 --- /dev/null +++ b/contracts/mocks/WhitelistedRoleMock.sol @@ -0,0 +1,8 @@ +pragma solidity ^0.4.24; + +import "../access/roles/WhitelistedRole.sol"; + +contract WhitelistedRoleMock is WhitelistedRole { + function onlyWhitelistedMock() public view onlyWhitelisted { + } +} diff --git a/contracts/mocks/WhitelisteeRoleMock.sol b/contracts/mocks/WhitelisteeRoleMock.sol deleted file mode 100644 index ec1a93992e6..00000000000 --- a/contracts/mocks/WhitelisteeRoleMock.sol +++ /dev/null @@ -1,8 +0,0 @@ -pragma solidity ^0.4.24; - -import "../access/roles/WhitelisteeRole.sol"; - -contract WhitelisteeRoleMock is WhitelisteeRole { - function onlyWhitelisteeMock() public view onlyWhitelistee { - } -} diff --git a/test/access/roles/WhitelistedRole.test.js b/test/access/roles/WhitelistedRole.test.js new file mode 100644 index 00000000000..2290424cd5f --- /dev/null +++ b/test/access/roles/WhitelistedRole.test.js @@ -0,0 +1,12 @@ +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); +const WhitelistedRoleMock = artifacts.require('WhitelistedRoleMock'); + +contract('WhitelistedRole', function ([_, whitelisted, otherWhitelisted, whitelister, ...otherAccounts]) { + beforeEach(async function () { + this.contract = await WhitelistedRoleMock.new({ from: whitelister }); + await this.contract.addWhitelisted(whitelisted, { from: whitelister }); + await this.contract.addWhitelisted(otherWhitelisted, { from: whitelister }); + }); + + shouldBehaveLikePublicRole(whitelisted, otherWhitelisted, otherAccounts, 'whitelisted', whitelister); +}); diff --git a/test/access/roles/WhitelisteeRole.test.js b/test/access/roles/WhitelisteeRole.test.js deleted file mode 100644 index a58ea005ef7..00000000000 --- a/test/access/roles/WhitelisteeRole.test.js +++ /dev/null @@ -1,12 +0,0 @@ -const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); -const WhitelisteeRoleMock = artifacts.require('WhitelisteeRoleMock'); - -contract('WhitelisteeRole', function ([_, whitelistee, otherWhitelistee, whitelister, ...otherAccounts]) { - beforeEach(async function () { - this.contract = await WhitelisteeRoleMock.new({ from: whitelister }); - await this.contract.addWhitelistee(whitelistee, { from: whitelister }); - await this.contract.addWhitelistee(otherWhitelistee, { from: whitelister }); - }); - - shouldBehaveLikePublicRole(whitelistee, otherWhitelistee, otherAccounts, 'whitelistee', whitelister); -}); diff --git a/test/crowdsale/WhitelistedCrowdsale.test.js b/test/crowdsale/WhitelistedCrowdsale.test.js index 82f1f3f5961..7bcc5e25a61 100644 --- a/test/crowdsale/WhitelistedCrowdsale.test.js +++ b/test/crowdsale/WhitelistedCrowdsale.test.js @@ -9,7 +9,7 @@ require('chai') const WhitelistedCrowdsale = artifacts.require('WhitelistedCrowdsaleImpl'); const SimpleToken = artifacts.require('SimpleToken'); -contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelistee, otherWhitelistee, anyone]) { +contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelisted, otherWhitelisted, anyone]) { const rate = 1; const value = ether(42); const tokenSupply = new BigNumber('1e22'); @@ -33,23 +33,23 @@ contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelistee, context('with no whitelisted addresses', function () { it('rejects all purchases', async function () { await purchaseShouldFail(this.crowdsale, anyone, value); - await purchaseShouldFail(this.crowdsale, whitelistee, value); + await purchaseShouldFail(this.crowdsale, whitelisted, value); }); }); context('with whitelisted addresses', function () { beforeEach(async function () { - await this.crowdsale.addWhitelistee(whitelistee, { from: whitelister }); - await this.crowdsale.addWhitelistee(otherWhitelistee, { from: whitelister }); + await this.crowdsale.addWhitelisted(whitelisted, { from: whitelister }); + await this.crowdsale.addWhitelisted(otherWhitelisted, { from: whitelister }); }); it('accepts purchases with whitelisted beneficiaries', async function () { - await purchaseShouldSucceed(this.crowdsale, whitelistee, value); - await purchaseShouldSucceed(this.crowdsale, otherWhitelistee, value); + await purchaseShouldSucceed(this.crowdsale, whitelisted, value); + await purchaseShouldSucceed(this.crowdsale, otherWhitelisted, value); }); it('rejects purchases from whitelisted addresses with non-whitelisted beneficiaries', async function () { - await shouldFail(this.crowdsale.buyTokens(anyone, { from: whitelistee, value })); + await shouldFail(this.crowdsale.buyTokens(anyone, { from: whitelisted, value })); }); it('rejects purchases with non-whitelisted beneficiaries', async function () { From 178d149539aed13e8404b85a8cd8d1c521e96877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 12 Dec 2018 11:55:33 -0300 Subject: [PATCH 12/14] Renamed WhitelistedCrowdsale to WhitelistCrowdsale. --- .../{WhitelistedCrowdsale.sol => WhitelistCrowdsale.sol} | 4 ++-- ...telistedCrowdsaleImpl.sol => WhitelistCrowdsaleImpl.sol} | 4 ++-- ...telistedCrowdsale.test.js => WhitelistCrowdsale.test.js} | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) rename contracts/crowdsale/validation/{WhitelistedCrowdsale.sol => WhitelistCrowdsale.sol} (87%) rename contracts/mocks/{WhitelistedCrowdsaleImpl.sol => WhitelistCrowdsaleImpl.sol} (61%) rename test/crowdsale/{WhitelistedCrowdsale.test.js => WhitelistCrowdsale.test.js} (87%) diff --git a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol b/contracts/crowdsale/validation/WhitelistCrowdsale.sol similarity index 87% rename from contracts/crowdsale/validation/WhitelistedCrowdsale.sol rename to contracts/crowdsale/validation/WhitelistCrowdsale.sol index 5e6be7461f0..d93028d5913 100644 --- a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol +++ b/contracts/crowdsale/validation/WhitelistCrowdsale.sol @@ -4,10 +4,10 @@ import "../../access/roles/WhitelistedRole.sol"; /** - * @title WhitelistedCrowdsale + * @title WhitelistCrowdsale * @dev Crowdsale in which only whitelisted users can contribute. */ -contract WhitelistedCrowdsale is WhitelistedRole, Crowdsale { +contract WhitelistCrowdsale is WhitelistedRole, Crowdsale { /** * @dev Extend parent behavior requiring beneficiary to be whitelisted. Note that no * restriction is imposed on the account sending the transaction. diff --git a/contracts/mocks/WhitelistedCrowdsaleImpl.sol b/contracts/mocks/WhitelistCrowdsaleImpl.sol similarity index 61% rename from contracts/mocks/WhitelistedCrowdsaleImpl.sol rename to contracts/mocks/WhitelistCrowdsaleImpl.sol index 3a52d0c3a00..ab8bd73fa50 100644 --- a/contracts/mocks/WhitelistedCrowdsaleImpl.sol +++ b/contracts/mocks/WhitelistCrowdsaleImpl.sol @@ -1,10 +1,10 @@ pragma solidity ^0.4.24; import "../token/ERC20/IERC20.sol"; -import "../crowdsale/validation/WhitelistedCrowdsale.sol"; +import "../crowdsale/validation/WhitelistCrowdsale.sol"; import "../crowdsale/Crowdsale.sol"; -contract WhitelistedCrowdsaleImpl is Crowdsale, WhitelistedCrowdsale { +contract WhitelistCrowdsaleImpl is Crowdsale, WhitelistCrowdsale { constructor (uint256 _rate, address _wallet, IERC20 _token) Crowdsale(_rate, _wallet, _token) public {} } diff --git a/test/crowdsale/WhitelistedCrowdsale.test.js b/test/crowdsale/WhitelistCrowdsale.test.js similarity index 87% rename from test/crowdsale/WhitelistedCrowdsale.test.js rename to test/crowdsale/WhitelistCrowdsale.test.js index 7bcc5e25a61..dfdd9f9cc5f 100644 --- a/test/crowdsale/WhitelistedCrowdsale.test.js +++ b/test/crowdsale/WhitelistCrowdsale.test.js @@ -6,17 +6,17 @@ const BigNumber = web3.BigNumber; require('chai') .should(); -const WhitelistedCrowdsale = artifacts.require('WhitelistedCrowdsaleImpl'); +const WhitelistCrowdsale = artifacts.require('WhitelistCrowdsaleImpl'); const SimpleToken = artifacts.require('SimpleToken'); -contract('WhitelistedCrowdsale', function ([_, wallet, whitelister, whitelisted, otherWhitelisted, anyone]) { +contract('WhitelistCrowdsale', function ([_, wallet, whitelister, whitelisted, otherWhitelisted, anyone]) { const rate = 1; const value = ether(42); const tokenSupply = new BigNumber('1e22'); beforeEach(async function () { this.token = await SimpleToken.new({ from: whitelister }); - this.crowdsale = await WhitelistedCrowdsale.new(rate, wallet, this.token.address, { from: whitelister }); + this.crowdsale = await WhitelistCrowdsale.new(rate, wallet, this.token.address, { from: whitelister }); await this.token.transfer(this.crowdsale.address, tokenSupply, { from: whitelister }); }); From 23d66e94c3912c9b5cd4197d2cd25bcbf7bb486a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 12 Dec 2018 11:58:16 -0300 Subject: [PATCH 13/14] Now using the new test helper. --- test/crowdsale/WhitelistCrowdsale.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/crowdsale/WhitelistCrowdsale.test.js b/test/crowdsale/WhitelistCrowdsale.test.js index dfdd9f9cc5f..713de675ee4 100644 --- a/test/crowdsale/WhitelistCrowdsale.test.js +++ b/test/crowdsale/WhitelistCrowdsale.test.js @@ -1,11 +1,9 @@ +require('../helpers/setup'); const { ether } = require('../helpers/ether'); const shouldFail = require('../helpers/shouldFail'); const BigNumber = web3.BigNumber; -require('chai') - .should(); - const WhitelistCrowdsale = artifacts.require('WhitelistCrowdsaleImpl'); const SimpleToken = artifacts.require('SimpleToken'); From 5c0a3b891619ec391e9bae85d023884cbc3a2576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Wed, 12 Dec 2018 12:07:24 -0300 Subject: [PATCH 14/14] Added basic documentation. --- contracts/access/roles/WhitelistedRole.sol | 6 ++++++ contracts/access/roles/WhitelisterRole.sol | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/contracts/access/roles/WhitelistedRole.sol b/contracts/access/roles/WhitelistedRole.sol index 537633c8e9f..68a826197d6 100644 --- a/contracts/access/roles/WhitelistedRole.sol +++ b/contracts/access/roles/WhitelistedRole.sol @@ -3,6 +3,12 @@ pragma solidity ^0.4.24; import "../Roles.sol"; import "./WhitelisterRole.sol"; +/** + * @title WhitelistedRole + * @dev Whitelisted accounts have been approved by a Whitelister to perform certain actions (e.g. participate in a + * crowdsale). This role is special in that the only accounts that can add it are Whitelisters (who can also remove it), + * and not Whitelisteds themselves. + */ contract WhitelistedRole is WhitelisterRole { using Roles for Roles.Role; diff --git a/contracts/access/roles/WhitelisterRole.sol b/contracts/access/roles/WhitelisterRole.sol index 41f0ddf421c..0abdeaaf7e1 100644 --- a/contracts/access/roles/WhitelisterRole.sol +++ b/contracts/access/roles/WhitelisterRole.sol @@ -2,6 +2,10 @@ pragma solidity ^0.4.24; import "../Roles.sol"; +/** + * @title WhitelisterRole + * @dev Whitelisters are responsible for assigning and removing Whitelisted accounts. + */ contract WhitelisterRole { using Roles for Roles.Role;