From f12817e446fcd3d7212fe307bb17424ceb28f0b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 7 Sep 2018 07:16:51 -0300 Subject: [PATCH] RBAC and Ownable migration towards Roles (#1291) * Role tests (#1228) * Moved RBAC tests to access. * Added Roles.addMany and tests. * Fixed linter error. * Now using uint256 indexes. * Removed RBAC tokens (#1229) * Deleted RBACCappedTokenMock. * Removed RBACMintableToken. * Removed RBACMintableToken from the MintedCrowdsale tests. * Roles can now be transfered. (#1235) * Roles can now be transfered. * Now explicitly checking support for the null address. * Now rejecting transfer to a role-haver. * Added renounce, roles can no longer be transfered to 0. * Fixed linter errors. * Fixed a Roles test. * True Ownership (#1247) * Added barebones Secondary. * Added transferPrimary * Escrow is now Secondary instead of Ownable. * Now reverting on transfers to 0. * The Secondary's primary is now private. * MintableToken using Roles (#1236) * Minor test style improvements (#1219) * Changed .eq to .equal * Changed equal(bool) to .to.be.bool * Changed be.bool to equal(bool), disallowed unused expressions. * Add ERC165Query library (#1086) * Add ERC165Query library * Address PR Comments * Add tests and mocks from #1024 and refactor code slightly * Fix javascript and solidity linting errors * Split supportsInterface into three methods as discussed in #1086 * Change InterfaceId_ERC165 comment to match style in the rest of the repo * Fix max-len lint issue on ERC165Checker.sol * Conditionally ignore the asserts during solidity-coverage test * Switch to abi.encodeWithSelector and add test for account addresses * Switch to supportsInterfaces API as suggested by @frangio * Adding ERC165InterfacesSupported.sol * Fix style issues * Add test for supportsInterfaces returning false * Add ERC165Checker.sol newline * feat: fix coverage implementation * fix: solidity linting error * fix: revert to using boolean tests instead of require statements * fix: make supportsERC165Interface private again * rename SupportsInterfaceWithLookupMock to avoid name clashing * Added mint and burn tests for zero amounts. (#1230) * Changed .eq to .equal. (#1231) * ERC721 pausable token (#1154) * ERC721 pausable token * Reuse of ERC721 Basic behavior for Pausable, split view checks in paused state & style fixes * [~] paused token behavior * Add some detail to releasing steps (#1190) * add note about pulling upstream changes to release branch * add comment about upstream changes in merging section * Increase test coverage (#1237) * Fixed a SplitPayment test * Deleted unnecessary function. * Improved PostDeliveryCrowdsale tests. * Improved RefundableCrowdsale tests. * Improved MintedCrowdsale tests. * Improved IncreasingPriceCrowdsale tests. * Fixed a CappedCrowdsale test. * Improved TimedCrowdsale tests. * Improved descriptions of added tests. * ci: trigger docs update on tag (#1186) * MintableToken now uses Roles. * Fixed FinalizableCrowdsale test. * Roles can now be transfered. * Fixed tests related to MintableToken. * Removed Roles.check. * Renamed transferMintPermission. * Moved MinterRole * Fixed RBAC. * Adressed review comments. * Addressed review comments * Fixed linter errors. * Added Events tests of Pausable contract (#1207) * Fixed roles tests. * Rename events to past-tense (#1181) * fix: refactor sign.js and related tests (#1243) * fix: refactor sign.js and related tests * fix: remove unused dep * fix: update package.json correctly * Added "_" sufix to internal variables (#1171) * Added PublicRole test. * Fixed crowdsale tests. * Rename ERC interfaces to I prefix (#1252) * rename ERC20 to IERC20 * move ERC20.sol to IERC20.sol * rename StandardToken to ERC20 * rename StandardTokenMock to ERC20Mock * move StandardToken.sol to ERC20.sol, likewise test and mock files * rename MintableToken to ERC20Mintable * move MintableToken.sol to ERC20Mintable.sol, likewise test and mock files * rename BurnableToken to ERC20Burnable * move BurnableToken.sol to ERC20Burnable.sol, likewise for related files * rename CappedToken to ERC20Capped * move CappedToken.sol to ERC20Capped.sol, likewise for related files * rename PausableToken to ERC20Pausable * move PausableToken.sol to ERC20Pausable.sol, likewise for related files * rename DetailedERC20 to ERC20Detailed * move DetailedERC20.sol to ERC20Detailed.sol, likewise for related files * rename ERC721 to IERC721, and likewise for other related interfaces * move ERC721.sol to IERC721.sol, likewise for other 721 interfaces * rename ERC721Token to ERC721 * move ERC721Token.sol to ERC721.sol, likewise for related files * rename ERC721BasicToken to ERC721Basic * move ERC721BasicToken.sol to ERC721Basic.sol, likewise for related files * rename ERC721PausableToken to ERC721Pausable * move ERC721PausableToken.sol to ERC721Pausable.sol * rename ERC165 to IERC165 * move ERC165.sol to IERC165.sol * amend comment that ERC20 is based on FirstBlood * fix comments mentioning IERC721Receiver * added explicit visibility (#1261) * Remove underscores from event parameters. (#1258) * Remove underscores from event parameters. Fixes #1175 * Add comment about ERC * Move contracts to subdirectories (#1253) * Move contracts to subdirectories Fixes #1177. This Change also removes the LimitBalance contract. * fix import * move MerkleProof to cryptography * Fix import * Remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner (#1254) * remove HasNoEther, HasNoTokens, HasNoContracts, and NoOwner * remove unused ERC223TokenMock * remove Contactable * remove TokenDestructible * remove DeprecatedERC721 * inline Destructible#destroy in Bounty * remove Destructible * Functions in interfaces changed to "external" (#1263) * Add a leading underscore to internal and private functions. (#1257) * Add a leading underscore to internal and private functions. Fixes #1176 * Remove super * update the ERC721 changes * add missing underscore after merge * Fix mock * Improve encapsulation on SignatureBouncer, Whitelist and RBAC example (#1265) * Improve encapsulation on Whitelist * remove only * update whitelisted crowdsale test * Improve encapsulation on SignatureBouncer * fix missing test * Improve encapsulation on RBAC example * Improve encapsulation on RBAC example * Remove extra visibility * Improve encapsulation on ERC20 Mintable * Improve encapsulation on Superuser * fix lint * add missing constant * Addressed review comments. * Fixed build error. * Improved Roles API. (#1280) * Improved Roles API. * fix linter error * Added PauserRole. (#1283) * Remove Claimable, DelayedClaimable, Heritable (#1274) * remove Claimable, DelayedClaimable, Heritable * remove SimpleSavingsWallet example which used Heritable (cherry picked from commit 0dc711732a297e70af63f23a9b52e4b3712eac40) * Role behavior tests (#1285) * Added role tests. * Added PauserRole tests to contracts that have that role. * Added MinterRole tests to contracts that have that role. * Fixed linter errors. * Migrate Ownable to Roles (#1287) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Remove RBAC, SignatureBouncer refactor (#1289) * Added CapperRole. * RefundEscrow is now Secondary. * FinalizableCrowdsale is no longer Ownable. * Removed Whitelist and WhitelistedCrowdsale, redesign needed. * Fixed linter errors, disabled lbrace due to it being buggy. * Moved SignatureBouncer tests. * Deleted RBAC and Superuser. * Deleted rbac directory. * Updated readme. * SignatureBouncer now uses SignerRole, renamed bouncer to signer. * feat: implement ERC721Mintable and ERC721Burnable (#1276) * feat: implement ERC721Mintable and ERC721Burnable * fix: linting errors * fix: remove unused mintable mock for ERC721BasicMock * fix: add finishMinting tests * fix: catch MintFinished typo * inline ERC721Full behavior * undo pretty formatting * fix lint errors * rename canMint to onlyBeforeMintingFinished for consistency with ERC20Mintable * Fix the merge with the privatization branch * remove duplicate CapperRole test --- .soliumrc.json | 1 + README.md | 3 +- contracts/access/{rbac => }/Roles.sol | 21 +- contracts/access/Whitelist.sol | 94 ------- contracts/access/rbac/RBAC.sol | 106 -------- contracts/access/roles/CapperRole.sol | 35 +++ contracts/access/roles/MinterRole.sol | 35 +++ contracts/access/roles/PauserRole.sol | 35 +++ contracts/access/roles/SignerRole.sol | 35 +++ contracts/bounties/BreakInvariantBounty.sol | 1 + contracts/crowdsale/Crowdsale.sol | 3 + .../distribution/FinalizableCrowdsale.sol | 11 +- .../distribution/RefundableCrowdsale.sol | 2 +- .../IndividuallyCappedCrowdsale.sol | 6 +- .../validation/WhitelistedCrowdsale.sol | 27 -- contracts/drafts/SignatureBouncer.sol | 64 ++--- contracts/examples/RBACWithAdmin.sol | 73 ------ contracts/examples/SampleCrowdsale.sol | 1 - contracts/lifecycle/Pausable.sol | 9 +- contracts/mocks/CapperRoleMock.sol | 18 ++ contracts/mocks/ERC20MintableMock.sol | 8 + contracts/mocks/ERC20PausableMock.sol | 3 +- contracts/mocks/ERC721BasicMock.sol | 4 +- .../mocks/ERC721MintableBurnableImpl.sol | 18 ++ contracts/mocks/ERC721Mock.sol | 19 +- contracts/mocks/ERC721PausableMock.sol | 3 +- contracts/mocks/FinalizableCrowdsaleImpl.sol | 4 +- .../mocks/IndividuallyCappedCrowdsaleImpl.sol | 7 +- contracts/mocks/MinterRoleMock.sol | 18 ++ contracts/mocks/PausableMock.sol | 3 +- contracts/mocks/PauserRoleMock.sol | 18 ++ contracts/mocks/RBACCappedTokenMock.sol | 12 - contracts/mocks/RBACMock.sol | 69 ----- contracts/mocks/RolesMock.sol | 22 ++ contracts/mocks/SecondaryMock.sol | 9 + contracts/mocks/SignatureBouncerMock.sol | 3 +- contracts/mocks/SignerRoleMock.sol | 18 ++ contracts/mocks/WhitelistMock.sol | 14 - contracts/mocks/WhitelistedCrowdsaleImpl.sol | 19 -- contracts/ownership/Secondary.sol | 35 +++ contracts/ownership/Superuser.sol | 62 ----- contracts/payment/Escrow.sol | 10 +- contracts/payment/RefundEscrow.sol | 14 +- contracts/token/ERC20/ERC20Capped.sol | 4 +- contracts/token/ERC20/ERC20Detailed.sol | 2 - contracts/token/ERC20/ERC20Mintable.sol | 25 +- contracts/token/ERC20/RBACMintableToken.sol | 48 ---- contracts/token/ERC721/ERC721Burnable.sol | 13 + contracts/token/ERC721/ERC721Mintable.sol | 78 ++++++ test/access/Roles.test.js | 61 +++++ test/access/SignatureBouncer.test.js | 246 ------------------ test/access/Whitelist.test.js | 65 ----- test/access/roles/CapperRole.test.js | 11 + test/access/roles/MinterRole.test.js | 11 + test/access/roles/PauserRole.test.js | 11 + test/access/roles/PublicRole.behavior.js | 66 +++++ test/access/roles/SignerRole.test.js | 11 + test/crowdsale/FinalizableCrowdsale.test.js | 26 +- .../IndividuallyCappedCrowdsale.test.js | 93 ++++--- test/crowdsale/MintedCrowdsale.test.js | 23 +- test/crowdsale/RefundableCrowdsale.test.js | 10 +- test/crowdsale/WhitelistedCrowdsale.test.js | 89 ------- test/drafts/SignatureBouncer.test.js | 212 +++++++++++++++ test/examples/SampleCrowdsale.test.js | 8 +- test/helpers/sign.js | 4 +- test/lifecycle/Pausable.test.js | 124 ++++++--- test/ownership/Secondary.test.js | 57 ++++ test/ownership/Superuser.test.js | 69 ----- test/ownership/rbac/RBAC.test.js | 73 ------ test/payment/Escrow.behavior.js | 30 +-- test/payment/Escrow.test.js | 6 +- test/payment/RefundEscrow.test.js | 42 +-- test/token/ERC20/ERC20Capped.behavior.js | 2 +- test/token/ERC20/ERC20Capped.test.js | 10 +- test/token/ERC20/ERC20Mintable.behavior.js | 57 ++-- test/token/ERC20/ERC20Mintable.test.js | 18 +- test/token/ERC20/ERC20Pausable.test.js | 115 ++++---- test/token/ERC20/RBACCappedToken.test.js | 19 -- .../token/ERC20/RBACMintableToken.behavior.js | 28 -- test/token/ERC20/RBACMintableToken.test.js | 14 - test/token/ERC20/TokenTimelock.test.js | 6 +- test/token/ERC721/ERC721.test.js | 65 ++--- test/token/ERC721/ERC721Basic.behavior.js | 159 ++++++----- test/token/ERC721/ERC721Basic.test.js | 8 +- test/token/ERC721/ERC721Burnable.test.js | 22 ++ test/token/ERC721/ERC721MintBurn.behavior.js | 97 +++++-- test/token/ERC721/ERC721Mintable.test.js | 24 ++ test/token/ERC721/ERC721Pausable.test.js | 33 ++- 88 files changed, 1499 insertions(+), 1668 deletions(-) rename contracts/access/{rbac => }/Roles.sol (59%) delete mode 100644 contracts/access/Whitelist.sol delete mode 100644 contracts/access/rbac/RBAC.sol create mode 100644 contracts/access/roles/CapperRole.sol create mode 100644 contracts/access/roles/MinterRole.sol create mode 100644 contracts/access/roles/PauserRole.sol create mode 100644 contracts/access/roles/SignerRole.sol delete mode 100644 contracts/crowdsale/validation/WhitelistedCrowdsale.sol delete mode 100644 contracts/examples/RBACWithAdmin.sol create mode 100644 contracts/mocks/CapperRoleMock.sol create mode 100644 contracts/mocks/ERC20MintableMock.sol create mode 100644 contracts/mocks/ERC721MintableBurnableImpl.sol create mode 100644 contracts/mocks/MinterRoleMock.sol create mode 100644 contracts/mocks/PauserRoleMock.sol delete mode 100644 contracts/mocks/RBACCappedTokenMock.sol delete mode 100644 contracts/mocks/RBACMock.sol create mode 100644 contracts/mocks/RolesMock.sol create mode 100644 contracts/mocks/SecondaryMock.sol create mode 100644 contracts/mocks/SignerRoleMock.sol delete mode 100644 contracts/mocks/WhitelistMock.sol delete mode 100644 contracts/mocks/WhitelistedCrowdsaleImpl.sol create mode 100644 contracts/ownership/Secondary.sol delete mode 100644 contracts/ownership/Superuser.sol delete mode 100644 contracts/token/ERC20/RBACMintableToken.sol create mode 100644 contracts/token/ERC721/ERC721Burnable.sol create mode 100644 contracts/token/ERC721/ERC721Mintable.sol create mode 100644 test/access/Roles.test.js delete mode 100644 test/access/SignatureBouncer.test.js delete mode 100644 test/access/Whitelist.test.js create mode 100644 test/access/roles/CapperRole.test.js create mode 100644 test/access/roles/MinterRole.test.js create mode 100644 test/access/roles/PauserRole.test.js create mode 100644 test/access/roles/PublicRole.behavior.js create mode 100644 test/access/roles/SignerRole.test.js delete mode 100644 test/crowdsale/WhitelistedCrowdsale.test.js create mode 100644 test/drafts/SignatureBouncer.test.js create mode 100644 test/ownership/Secondary.test.js delete mode 100644 test/ownership/Superuser.test.js delete mode 100644 test/ownership/rbac/RBAC.test.js delete mode 100644 test/token/ERC20/RBACCappedToken.test.js delete mode 100644 test/token/ERC20/RBACMintableToken.behavior.js delete mode 100644 test/token/ERC20/RBACMintableToken.test.js create mode 100644 test/token/ERC721/ERC721Burnable.test.js create mode 100644 test/token/ERC721/ERC721Mintable.test.js diff --git a/.soliumrc.json b/.soliumrc.json index 6a08c646678..be4afce1095 100644 --- a/.soliumrc.json +++ b/.soliumrc.json @@ -4,6 +4,7 @@ "rules": { "error-reason": "off", "indentation": ["error", 2], + "lbrace": "off", "linebreak-style": ["error", "unix"], "max-len": ["error", 79], "no-constant": ["error"], diff --git a/README.md b/README.md index f866b881e0c..887083c2e92 100644 --- a/README.md +++ b/README.md @@ -69,8 +69,7 @@ contract MyContract is Ownable { ## Architecture The following provides visibility into how OpenZeppelin's contracts are organized: -- **access** - Smart contracts that enable functionality that can be used for selective restrictions and basic authorization control functions. Includes address whitelisting and signature-based permissions management. - - **rbac** - A library used to manage addresses assigned to different user roles and an example Role-Based Access Control (RBAC) interface that demonstrates how to handle setters and getters for roles and addresses. +- **access** - Smart contracts that enable functionality that can be used for selective restrictions and basic authorization control functions. - **crowdsale** - A collection of smart contracts used to manage token crowdsales that allow investors to purchase tokens with ETH. Includes a base contract which implements fundamental crowdsale functionality in its simplest form. The base contract can be extended in order to satisfy your crowdsale’s specific requirements. - **distribution** - Includes extensions of the base crowdsale contract which can be used to customize the completion of a crowdsale. - **emission** - Includes extensions of the base crowdsale contract which can be used to mint and manage how tokens are issued to purchasers. diff --git a/contracts/access/rbac/Roles.sol b/contracts/access/Roles.sol similarity index 59% rename from contracts/access/rbac/Roles.sol rename to contracts/access/Roles.sol index de46894095f..f5523a3e1b1 100644 --- a/contracts/access/rbac/Roles.sol +++ b/contracts/access/Roles.sol @@ -3,9 +3,7 @@ pragma solidity ^0.4.24; /** * @title Roles - * @author Francisco Giordano (@frangio) * @dev Library for managing addresses assigned to a Role. - * See RBAC.sol for example usage. */ library Roles { struct Role { @@ -15,32 +13,17 @@ library Roles { /** * @dev give an account access to this role */ - function add(Role storage _role, address _account) - internal - { + function add(Role storage _role, address _account) internal { _role.bearer[_account] = true; } /** * @dev remove an account's access to this role */ - function remove(Role storage _role, address _account) - internal - { + function remove(Role storage _role, address _account) internal { _role.bearer[_account] = false; } - /** - * @dev check if an account has this role - * // reverts - */ - function check(Role storage _role, address _account) - internal - view - { - require(has(_role, _account)); - } - /** * @dev check if an account has this role * @return bool diff --git a/contracts/access/Whitelist.sol b/contracts/access/Whitelist.sol deleted file mode 100644 index 3d9b3da5c6f..00000000000 --- a/contracts/access/Whitelist.sol +++ /dev/null @@ -1,94 +0,0 @@ -pragma solidity ^0.4.24; - - -import "../ownership/Ownable.sol"; -import "../access/rbac/RBAC.sol"; - - -/** - * @title Whitelist - * @dev The Whitelist contract has a whitelist of addresses, and provides basic authorization control functions. - * This simplifies the implementation of "user permissions". - */ -contract Whitelist is Ownable, RBAC { - - // Name of the whitelisted role. - string private constant ROLE_WHITELISTED = "whitelist"; - - /** - * @dev Throws if operator is not whitelisted. - * @param _operator address - */ - modifier onlyIfWhitelisted(address _operator) { - checkRole(_operator, ROLE_WHITELISTED); - _; - } - - /** - * @dev add an address to the whitelist - * @param _operator address - * @return true if the address was added to the whitelist, false if the address was already in the whitelist - */ - function addAddressToWhitelist(address _operator) - public - onlyOwner - { - _addRole(_operator, ROLE_WHITELISTED); - } - - /** - * @dev Determine if an account is whitelisted. - * @return true if the account is whitelisted, false otherwise. - */ - function isWhitelisted(address _operator) - public - view - returns (bool) - { - return hasRole(_operator, ROLE_WHITELISTED); - } - - /** - * @dev add addresses to the whitelist - * @param _operators addresses - * @return true if at least one address was added to the whitelist, - * false if all addresses were already in the whitelist - */ - function addAddressesToWhitelist(address[] _operators) - public - onlyOwner - { - for (uint256 i = 0; i < _operators.length; i++) { - addAddressToWhitelist(_operators[i]); - } - } - - /** - * @dev remove an address from the whitelist - * @param _operator address - * @return true if the address was removed from the whitelist, - * false if the address wasn't in the whitelist in the first place - */ - function removeAddressFromWhitelist(address _operator) - public - onlyOwner - { - _removeRole(_operator, ROLE_WHITELISTED); - } - - /** - * @dev remove addresses from the whitelist - * @param _operators addresses - * @return true if at least one address was removed from the whitelist, - * false if all addresses weren't in the whitelist in the first place - */ - function removeAddressesFromWhitelist(address[] _operators) - public - onlyOwner - { - for (uint256 i = 0; i < _operators.length; i++) { - removeAddressFromWhitelist(_operators[i]); - } - } - -} diff --git a/contracts/access/rbac/RBAC.sol b/contracts/access/rbac/RBAC.sol deleted file mode 100644 index 8ecefaa99f0..00000000000 --- a/contracts/access/rbac/RBAC.sol +++ /dev/null @@ -1,106 +0,0 @@ -pragma solidity ^0.4.24; - -import "./Roles.sol"; - - -/** - * @title RBAC (Role-Based Access Control) - * @author Matt Condon (@Shrugs) - * @dev Stores and provides setters and getters for roles and addresses. - * Supports unlimited numbers of roles and addresses. - * See //contracts/mocks/RBACMock.sol for an example of usage. - * This RBAC method uses strings to key roles. It may be beneficial - * for you to write your own implementation of this interface using Enums or similar. - */ -contract RBAC { - using Roles for Roles.Role; - - mapping (string => Roles.Role) private roles; - - event RoleAdded(address indexed operator, string role); - event RoleRemoved(address indexed operator, string role); - - /** - * @dev reverts if addr does not have role - * @param _operator address - * @param _role the name of the role - * // reverts - */ - function checkRole(address _operator, string _role) - public - view - { - roles[_role].check(_operator); - } - - /** - * @dev determine if addr has role - * @param _operator address - * @param _role the name of the role - * @return bool - */ - function hasRole(address _operator, string _role) - public - view - returns (bool) - { - return roles[_role].has(_operator); - } - - /** - * @dev add a role to an address - * @param _operator address - * @param _role the name of the role - */ - function _addRole(address _operator, string _role) - internal - { - roles[_role].add(_operator); - emit RoleAdded(_operator, _role); - } - - /** - * @dev remove a role from an address - * @param _operator address - * @param _role the name of the role - */ - function _removeRole(address _operator, string _role) - internal - { - roles[_role].remove(_operator); - emit RoleRemoved(_operator, _role); - } - - /** - * @dev modifier to scope access to a single role (uses msg.sender as addr) - * @param _role the name of the role - * // reverts - */ - modifier onlyRole(string _role) - { - checkRole(msg.sender, _role); - _; - } - - /** - * @dev modifier to scope access to a set of roles (uses msg.sender as addr) - * @param _roles the names of the roles to scope access to - * // reverts - * - * @TODO - when solidity supports dynamic arrays as arguments to modifiers, provide this - * see: https://github.com/ethereum/solidity/issues/2467 - */ - // modifier onlyRoles(string[] _roles) { - // bool hasAnyRole = false; - // for (uint8 i = 0; i < _roles.length; i++) { - // if (hasRole(msg.sender, _roles[i])) { - // hasAnyRole = true; - // break; - // } - // } - - // require(hasAnyRole); - - // _; - // } -} diff --git a/contracts/access/roles/CapperRole.sol b/contracts/access/roles/CapperRole.sol new file mode 100644 index 00000000000..bfa28712c8e --- /dev/null +++ b/contracts/access/roles/CapperRole.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.4.24; + +import "../Roles.sol"; + + +contract CapperRole { + using Roles for Roles.Role; + + Roles.Role private cappers; + + constructor() public { + cappers.add(msg.sender); + } + + modifier onlyCapper() { + require(isCapper(msg.sender)); + _; + } + + function isCapper(address _account) public view returns (bool) { + return cappers.has(_account); + } + + function addCapper(address _account) public onlyCapper { + cappers.add(_account); + } + + function renounceCapper() public { + cappers.remove(msg.sender); + } + + function _removeCapper(address _account) internal { + cappers.remove(_account); + } +} diff --git a/contracts/access/roles/MinterRole.sol b/contracts/access/roles/MinterRole.sol new file mode 100644 index 00000000000..4e21b40472e --- /dev/null +++ b/contracts/access/roles/MinterRole.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.4.24; + +import "../Roles.sol"; + + +contract MinterRole { + using Roles for Roles.Role; + + Roles.Role private minters; + + constructor() public { + minters.add(msg.sender); + } + + modifier onlyMinter() { + require(isMinter(msg.sender)); + _; + } + + function isMinter(address _account) public view returns (bool) { + return minters.has(_account); + } + + function addMinter(address _account) public onlyMinter { + minters.add(_account); + } + + function renounceMinter() public { + minters.remove(msg.sender); + } + + function _removeMinter(address _account) internal { + minters.remove(_account); + } +} diff --git a/contracts/access/roles/PauserRole.sol b/contracts/access/roles/PauserRole.sol new file mode 100644 index 00000000000..ed7fb2727a2 --- /dev/null +++ b/contracts/access/roles/PauserRole.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.4.24; + +import "../Roles.sol"; + + +contract PauserRole { + using Roles for Roles.Role; + + Roles.Role private pausers; + + constructor() public { + pausers.add(msg.sender); + } + + modifier onlyPauser() { + require(isPauser(msg.sender)); + _; + } + + function isPauser(address _account) public view returns (bool) { + return pausers.has(_account); + } + + function addPauser(address _account) public onlyPauser { + pausers.add(_account); + } + + function renouncePauser() public { + pausers.remove(msg.sender); + } + + function _removePauser(address _account) internal { + pausers.remove(_account); + } +} diff --git a/contracts/access/roles/SignerRole.sol b/contracts/access/roles/SignerRole.sol new file mode 100644 index 00000000000..5c01a611eec --- /dev/null +++ b/contracts/access/roles/SignerRole.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.4.24; + +import "../Roles.sol"; + + +contract SignerRole { + using Roles for Roles.Role; + + Roles.Role private signers; + + constructor() public { + signers.add(msg.sender); + } + + modifier onlySigner() { + require(isSigner(msg.sender)); + _; + } + + function isSigner(address _account) public view returns (bool) { + return signers.has(_account); + } + + function addSigner(address _account) public onlySigner { + signers.add(_account); + } + + function renounceSigner() public { + signers.remove(msg.sender); + } + + function _removeSigner(address _account) internal { + signers.remove(_account); + } +} diff --git a/contracts/bounties/BreakInvariantBounty.sol b/contracts/bounties/BreakInvariantBounty.sol index 05a19045cd2..f41bfb77548 100644 --- a/contracts/bounties/BreakInvariantBounty.sol +++ b/contracts/bounties/BreakInvariantBounty.sol @@ -2,6 +2,7 @@ pragma solidity ^0.4.24; import "../payment/PullPayment.sol"; +import "../ownership/Ownable.sol"; /** diff --git a/contracts/crowdsale/Crowdsale.sol b/contracts/crowdsale/Crowdsale.sol index 0b133c0aa28..f600174d019 100644 --- a/contracts/crowdsale/Crowdsale.sol +++ b/contracts/crowdsale/Crowdsale.sol @@ -28,6 +28,9 @@ contract Crowdsale { address private wallet_; // How many token units a buyer gets per wei. + // The rate is the conversion between wei and the smallest and indivisible token unit. + // So, if you are using a rate of 1 with a ERC20Detailed token with 3 decimals called TOK + // 1 wei will give you 1 unit, or 0.001 TOK. uint256 private rate_; // Amount of wei raised diff --git a/contracts/crowdsale/distribution/FinalizableCrowdsale.sol b/contracts/crowdsale/distribution/FinalizableCrowdsale.sol index 693e0b498e1..d8aa1eeabae 100644 --- a/contracts/crowdsale/distribution/FinalizableCrowdsale.sol +++ b/contracts/crowdsale/distribution/FinalizableCrowdsale.sol @@ -1,16 +1,15 @@ pragma solidity ^0.4.24; import "../../math/SafeMath.sol"; -import "../../ownership/Ownable.sol"; import "../validation/TimedCrowdsale.sol"; /** * @title FinalizableCrowdsale - * @dev Extension of Crowdsale where an owner can do extra work - * after finishing. + * @dev Extension of Crowdsale with a one-off finalization action, where one + * can do extra work after finishing. */ -contract FinalizableCrowdsale is Ownable, TimedCrowdsale { +contract FinalizableCrowdsale is TimedCrowdsale { using SafeMath for uint256; bool private finalized_ = false; @@ -20,7 +19,7 @@ contract FinalizableCrowdsale is Ownable, TimedCrowdsale { /** * @return true if the crowdsale is finalized, false otherwise. */ - function finalized() public view returns(bool) { + function finalized() public view returns (bool) { return finalized_; } @@ -28,7 +27,7 @@ contract FinalizableCrowdsale is Ownable, TimedCrowdsale { * @dev Must be called after crowdsale ends, to do some extra finalization * work. Calls the contract's finalization function. */ - function finalize() public onlyOwner { + function finalize() public { require(!finalized_); require(hasClosed()); diff --git a/contracts/crowdsale/distribution/RefundableCrowdsale.sol b/contracts/crowdsale/distribution/RefundableCrowdsale.sol index 6279fbbdeb9..6e8f368238f 100644 --- a/contracts/crowdsale/distribution/RefundableCrowdsale.sol +++ b/contracts/crowdsale/distribution/RefundableCrowdsale.sol @@ -57,7 +57,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale { } /** - * @dev escrow finalization task, called when owner calls finalize() + * @dev escrow finalization task, called when finalize() is called */ function _finalization() internal { if (goalReached()) { diff --git a/contracts/crowdsale/validation/IndividuallyCappedCrowdsale.sol b/contracts/crowdsale/validation/IndividuallyCappedCrowdsale.sol index 7d139edb886..139099e50a2 100644 --- a/contracts/crowdsale/validation/IndividuallyCappedCrowdsale.sol +++ b/contracts/crowdsale/validation/IndividuallyCappedCrowdsale.sol @@ -2,14 +2,14 @@ pragma solidity ^0.4.24; import "../../math/SafeMath.sol"; import "../Crowdsale.sol"; -import "../../ownership/Ownable.sol"; +import "../../access/roles/CapperRole.sol"; /** * @title IndividuallyCappedCrowdsale * @dev Crowdsale with per-beneficiary caps. */ -contract IndividuallyCappedCrowdsale is Ownable, Crowdsale { +contract IndividuallyCappedCrowdsale is Crowdsale, CapperRole { using SafeMath for uint256; mapping(address => uint256) private contributions_; @@ -20,7 +20,7 @@ contract IndividuallyCappedCrowdsale is Ownable, Crowdsale { * @param _beneficiary Address to be capped * @param _cap Wei limit for individual contribution */ - function setCap(address _beneficiary, uint256 _cap) external onlyOwner { + function setCap(address _beneficiary, uint256 _cap) external onlyCapper { caps_[_beneficiary] = _cap; } diff --git a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol b/contracts/crowdsale/validation/WhitelistedCrowdsale.sol deleted file mode 100644 index e3d1de08e7f..00000000000 --- a/contracts/crowdsale/validation/WhitelistedCrowdsale.sol +++ /dev/null @@ -1,27 +0,0 @@ -pragma solidity ^0.4.24; - -import "../Crowdsale.sol"; -import "../../access/Whitelist.sol"; - - -/** - * @title WhitelistedCrowdsale - * @dev Crowdsale in which only whitelisted users can contribute. - */ -contract WhitelistedCrowdsale is Whitelist, Crowdsale { - /** - * @dev Extend parent behavior requiring beneficiary to be in whitelist. - * @param _beneficiary Token beneficiary - * @param _weiAmount Amount of wei contributed - */ - function _preValidatePurchase( - address _beneficiary, - uint256 _weiAmount - ) - internal - onlyIfWhitelisted(_beneficiary) - { - super._preValidatePurchase(_beneficiary, _weiAmount); - } - -} diff --git a/contracts/drafts/SignatureBouncer.sol b/contracts/drafts/SignatureBouncer.sol index 0b91dcdb3fb..024cfd75c46 100644 --- a/contracts/drafts/SignatureBouncer.sol +++ b/contracts/drafts/SignatureBouncer.sol @@ -1,27 +1,26 @@ pragma solidity ^0.4.24; -import "../ownership/Ownable.sol"; -import "../access/rbac/RBAC.sol"; +import "../access/roles/SignerRole.sol"; import "../cryptography/ECDSA.sol"; /** * @title SignatureBouncer * @author PhABC, Shrugs and aflesher - * @dev Bouncer allows users to submit a signature as a permission to do an action. - * If the signature is from one of the authorized bouncer addresses, the signature - * is valid. The owner of the contract adds/removes bouncers. - * Bouncer addresses can be individual servers signing grants or different + * @dev SignatureBouncer allows users to submit a signature as a permission to do an action. + * If the signature is from one of the authorized signer addresses, the signature + * is valid. + * Signer addresses can be individual servers signing grants or different * users within a decentralized club that have permission to invite other members. * This technique is useful for whitelists and airdrops; instead of putting all * valid addresses on-chain, simply sign a grant of the form - * keccak256(abi.encodePacked(`:contractAddress` + `:granteeAddress`)) using a valid bouncer address. + * keccak256(abi.encodePacked(`:contractAddress` + `:granteeAddress`)) using a valid signer address. * Then restrict access to your crowdsale/whitelist/airdrop using the * `onlyValidSignature` modifier (or implement your own using _isValidSignature). * In addition to `onlyValidSignature`, `onlyValidSignatureAndMethod` and * `onlyValidSignatureAndData` can be used to restrict access to only a given method * or a given method with given parameters respectively. - * See the tests Bouncer.test.js for specific usage examples. + * See the tests in SignatureBouncer.test.js for specific usage examples. * @notice A method that uses the `onlyValidSignatureAndData` modifier must make the _signature * parameter the "last" parameter. You cannot sign a message that has its own * signature in it so the last 128 bytes of msg.data (which represents the @@ -29,11 +28,9 @@ import "../cryptography/ECDSA.sol"; * Also non fixed sized parameters make constructing the data in the signature * much more complex. See https://ethereum.stackexchange.com/a/50616 for more details. */ -contract SignatureBouncer is Ownable, RBAC { +contract SignatureBouncer is SignerRole { using ECDSA for bytes32; - // Name of the bouncer role. - string private constant ROLE_BOUNCER = "bouncer"; // Function selectors are 4 bytes long, as documented in // https://solidity.readthedocs.io/en/v0.4.24/abi-spec.html#function-selector uint256 private constant METHOD_ID_SIZE = 4; @@ -41,7 +38,7 @@ contract SignatureBouncer is Ownable, RBAC { uint256 private constant SIGNATURE_SIZE = 96; /** - * @dev requires that a valid signature of a bouncer was provided + * @dev requires that a valid signature of a signer was provided */ modifier onlyValidSignature(bytes _signature) { @@ -50,7 +47,7 @@ contract SignatureBouncer is Ownable, RBAC { } /** - * @dev requires that a valid signature with a specifed method of a bouncer was provided + * @dev requires that a valid signature with a specifed method of a signer was provided */ modifier onlyValidSignatureAndMethod(bytes _signature) { @@ -59,7 +56,7 @@ contract SignatureBouncer is Ownable, RBAC { } /** - * @dev requires that a valid signature with a specifed method and params of a bouncer was provided + * @dev requires that a valid signature with a specifed method and params of a signer was provided */ modifier onlyValidSignatureAndData(bytes _signature) { @@ -68,36 +65,7 @@ contract SignatureBouncer is Ownable, RBAC { } /** - * @dev Determine if an account has the bouncer role. - * @return true if the account is a bouncer, false otherwise. - */ - function isBouncer(address _account) public view returns(bool) { - return hasRole(_account, ROLE_BOUNCER); - } - - /** - * @dev allows the owner to add additional bouncer addresses - */ - function addBouncer(address _bouncer) - public - onlyOwner - { - require(_bouncer != address(0)); - _addRole(_bouncer, ROLE_BOUNCER); - } - - /** - * @dev allows the owner to remove bouncer addresses - */ - function removeBouncer(address _bouncer) - public - onlyOwner - { - _removeRole(_bouncer, ROLE_BOUNCER); - } - - /** - * @dev is the signature of `this + sender` from a bouncer? + * @dev is the signature of `this + sender` from a signer? * @return bool */ function _isValidSignature(address _address, bytes _signature) @@ -112,7 +80,7 @@ contract SignatureBouncer is Ownable, RBAC { } /** - * @dev is the signature of `this + sender + methodId` from a bouncer? + * @dev is the signature of `this + sender + methodId` from a signer? * @return bool */ function _isValidSignatureAndMethod(address _address, bytes _signature) @@ -131,7 +99,7 @@ contract SignatureBouncer is Ownable, RBAC { } /** - * @dev is the signature of `this + sender + methodId + params(s)` from a bouncer? + * @dev is the signature of `this + sender + methodId + params(s)` from a signer? * @notice the _signature parameter of the method being validated must be the "last" parameter * @return bool */ @@ -153,7 +121,7 @@ contract SignatureBouncer is Ownable, RBAC { /** * @dev internal function to convert a hash to an eth signed message - * and then recover the signature and check it against the bouncer role + * and then recover the signature and check it against the signer role * @return bool */ function _isValidDataHash(bytes32 _hash, bytes _signature) @@ -164,6 +132,6 @@ contract SignatureBouncer is Ownable, RBAC { address signer = _hash .toEthSignedMessageHash() .recover(_signature); - return isBouncer(signer); + return isSigner(signer); } } diff --git a/contracts/examples/RBACWithAdmin.sol b/contracts/examples/RBACWithAdmin.sol deleted file mode 100644 index 83c2dcef7a8..00000000000 --- a/contracts/examples/RBACWithAdmin.sol +++ /dev/null @@ -1,73 +0,0 @@ -pragma solidity ^0.4.24; - -import "../access/rbac/RBAC.sol"; - - -/** - * @title RBACWithAdmin - * @author Matt Condon (@Shrugs) - * @dev It's recommended that you define constants in the contract, - * like ROLE_ADMIN below, to avoid typos. - * @notice RBACWithAdmin is probably too expansive and powerful for your - * application; an admin is actually able to change any address to any role - * which is a very large API surface. It's recommended that you follow a strategy - * of strictly defining the abilities of your roles - * and the API-surface of your contract. - * This is just an example for example's sake. - */ -contract RBACWithAdmin is RBAC { - /** - * A constant role name for indicating admins. - */ - string private constant ROLE_ADMIN = "admin"; - - /** - * @dev modifier to scope access to admins - * // reverts - */ - modifier onlyAdmin() - { - checkRole(msg.sender, ROLE_ADMIN); - _; - } - - /** - * @dev constructor. Sets msg.sender as admin by default - */ - constructor() - public - { - _addRole(msg.sender, ROLE_ADMIN); - } - - /** - * @return true if the account is admin, false otherwise. - */ - function isAdmin(address _account) public view returns(bool) { - return hasRole(_account, ROLE_ADMIN); - } - - /** - * @dev add a role to an account - * @param _account the account that will have the role - * @param _roleName the name of the role - */ - function adminAddRole(address _account, string _roleName) - public - onlyAdmin - { - _addRole(_account, _roleName); - } - - /** - * @dev remove a role from an account - * @param _account the account that will no longer have the role - * @param _roleName the name of the role - */ - function adminRemoveRole(address _account, string _roleName) - public - onlyAdmin - { - _removeRole(_account, _roleName); - } -} diff --git a/contracts/examples/SampleCrowdsale.sol b/contracts/examples/SampleCrowdsale.sol index 564c6a0873e..5773f5d8646 100644 --- a/contracts/examples/SampleCrowdsale.sol +++ b/contracts/examples/SampleCrowdsale.sol @@ -16,7 +16,6 @@ contract SampleCrowdsaleToken is ERC20Mintable { string public constant name = "Sample Crowdsale Token"; string public constant symbol = "SCT"; uint8 public constant decimals = 18; - } diff --git a/contracts/lifecycle/Pausable.sol b/contracts/lifecycle/Pausable.sol index 37caf1e0a52..28bcac96682 100644 --- a/contracts/lifecycle/Pausable.sol +++ b/contracts/lifecycle/Pausable.sol @@ -1,14 +1,13 @@ pragma solidity ^0.4.24; - -import "../ownership/Ownable.sol"; +import "../access/roles/PauserRole.sol"; /** * @title Pausable * @dev Base contract which allows children to implement an emergency stop mechanism. */ -contract Pausable is Ownable { +contract Pausable is PauserRole { event Paused(); event Unpaused(); @@ -41,7 +40,7 @@ contract Pausable is Ownable { /** * @dev called by the owner to pause, triggers stopped state */ - function pause() public onlyOwner whenNotPaused { + function pause() public onlyPauser whenNotPaused { paused_ = true; emit Paused(); } @@ -49,7 +48,7 @@ contract Pausable is Ownable { /** * @dev called by the owner to unpause, returns to normal state */ - function unpause() public onlyOwner whenPaused { + function unpause() public onlyPauser whenPaused { paused_ = false; emit Unpaused(); } diff --git a/contracts/mocks/CapperRoleMock.sol b/contracts/mocks/CapperRoleMock.sol new file mode 100644 index 00000000000..f4f58a05fdb --- /dev/null +++ b/contracts/mocks/CapperRoleMock.sol @@ -0,0 +1,18 @@ +pragma solidity ^0.4.24; + +import "../access/roles/CapperRole.sol"; + + +contract CapperRoleMock is CapperRole { + function removeCapper(address _account) public { + _removeCapper(_account); + } + + function onlyCapperMock() public view onlyCapper { + } + + // Causes a compilation error if super._removeCapper is not internal + function _removeCapper(address _account) internal { + super._removeCapper(_account); + } +} diff --git a/contracts/mocks/ERC20MintableMock.sol b/contracts/mocks/ERC20MintableMock.sol new file mode 100644 index 00000000000..a2056548f74 --- /dev/null +++ b/contracts/mocks/ERC20MintableMock.sol @@ -0,0 +1,8 @@ +pragma solidity ^0.4.24; + +import "../token/ERC20/ERC20Mintable.sol"; +import "./MinterRoleMock.sol"; + + +contract ERC20MintableMock is ERC20Mintable, MinterRoleMock { +} diff --git a/contracts/mocks/ERC20PausableMock.sol b/contracts/mocks/ERC20PausableMock.sol index a60f8a12da6..8dad33cd40f 100644 --- a/contracts/mocks/ERC20PausableMock.sol +++ b/contracts/mocks/ERC20PausableMock.sol @@ -1,10 +1,11 @@ pragma solidity ^0.4.24; import "../token/ERC20/ERC20Pausable.sol"; +import "./PauserRoleMock.sol"; // mock class using ERC20Pausable -contract ERC20PausableMock is ERC20Pausable { +contract ERC20PausableMock is ERC20Pausable, PauserRoleMock { constructor(address _initialAccount, uint _initialBalance) public { _mint(_initialAccount, _initialBalance); diff --git a/contracts/mocks/ERC721BasicMock.sol b/contracts/mocks/ERC721BasicMock.sol index 87add3adb7e..5b8417fd5fb 100644 --- a/contracts/mocks/ERC721BasicMock.sol +++ b/contracts/mocks/ERC721BasicMock.sol @@ -9,10 +9,10 @@ import "../token/ERC721/ERC721Basic.sol"; */ contract ERC721BasicMock is ERC721Basic { function mint(address _to, uint256 _tokenId) public { - super._mint(_to, _tokenId); + _mint(_to, _tokenId); } function burn(uint256 _tokenId) public { - super._burn(ownerOf(_tokenId), _tokenId); + _burn(ownerOf(_tokenId), _tokenId); } } diff --git a/contracts/mocks/ERC721MintableBurnableImpl.sol b/contracts/mocks/ERC721MintableBurnableImpl.sol new file mode 100644 index 00000000000..d0eae1fb353 --- /dev/null +++ b/contracts/mocks/ERC721MintableBurnableImpl.sol @@ -0,0 +1,18 @@ +pragma solidity ^0.4.24; + +import "../token/ERC721/ERC721.sol"; +import "../token/ERC721/ERC721Mintable.sol"; +import "../token/ERC721/ERC721Burnable.sol"; + + +/** + * @title ERC721MintableBurnableImpl + */ +contract ERC721MintableBurnableImpl is ERC721, ERC721Mintable, ERC721Burnable { + constructor() + ERC721Mintable() + ERC721("Test", "TEST") + public + { + } +} diff --git a/contracts/mocks/ERC721Mock.sol b/contracts/mocks/ERC721Mock.sol index b6c138fd2cd..88ac56a5682 100644 --- a/contracts/mocks/ERC721Mock.sol +++ b/contracts/mocks/ERC721Mock.sol @@ -1,6 +1,8 @@ pragma solidity ^0.4.24; import "../token/ERC721/ERC721.sol"; +import "../token/ERC721/ERC721Mintable.sol"; +import "../token/ERC721/ERC721Burnable.sol"; /** @@ -8,18 +10,11 @@ import "../token/ERC721/ERC721.sol"; * This mock just provides a public mint and burn functions for testing purposes, * and a public setter for metadata URI */ -contract ERC721Mock is ERC721 { - constructor(string name, string symbol) public - ERC721(name, symbol) - { } - - function mint(address _to, uint256 _tokenId) public { - _mint(_to, _tokenId); - } - - function burn(uint256 _tokenId) public { - _burn(ownerOf(_tokenId), _tokenId); - } +contract ERC721Mock is ERC721, ERC721Mintable, ERC721Burnable { + constructor(string _name, string _symbol) public + ERC721Mintable() + ERC721(_name, _symbol) + {} function exists(uint256 _tokenId) public view returns (bool) { return _exists(_tokenId); diff --git a/contracts/mocks/ERC721PausableMock.sol b/contracts/mocks/ERC721PausableMock.sol index ef2722f4ace..8382518468c 100644 --- a/contracts/mocks/ERC721PausableMock.sol +++ b/contracts/mocks/ERC721PausableMock.sol @@ -1,13 +1,14 @@ pragma solidity ^0.4.24; import "../token/ERC721/ERC721Pausable.sol"; +import "./PauserRoleMock.sol"; /** * @title ERC721PausableMock * This mock just provides a public mint, burn and exists functions for testing purposes */ -contract ERC721PausableMock is ERC721Pausable { +contract ERC721PausableMock is ERC721Pausable, PauserRoleMock { function mint(address _to, uint256 _tokenId) public { super._mint(_to, _tokenId); } diff --git a/contracts/mocks/FinalizableCrowdsaleImpl.sol b/contracts/mocks/FinalizableCrowdsaleImpl.sol index aa419b82211..def19e6bf24 100644 --- a/contracts/mocks/FinalizableCrowdsaleImpl.sol +++ b/contracts/mocks/FinalizableCrowdsaleImpl.sol @@ -1,6 +1,6 @@ pragma solidity ^0.4.24; -import "../token/ERC20/ERC20Mintable.sol"; +import "../token/ERC20/IERC20.sol"; import "../crowdsale/distribution/FinalizableCrowdsale.sol"; @@ -11,7 +11,7 @@ contract FinalizableCrowdsaleImpl is FinalizableCrowdsale { uint256 _closingTime, uint256 _rate, address _wallet, - ERC20Mintable _token + IERC20 _token ) public Crowdsale(_rate, _wallet, _token) diff --git a/contracts/mocks/IndividuallyCappedCrowdsaleImpl.sol b/contracts/mocks/IndividuallyCappedCrowdsaleImpl.sol index b4b470f7ca6..552e9f8c5ca 100644 --- a/contracts/mocks/IndividuallyCappedCrowdsaleImpl.sol +++ b/contracts/mocks/IndividuallyCappedCrowdsaleImpl.sol @@ -2,11 +2,13 @@ pragma solidity ^0.4.24; import "../token/ERC20/IERC20.sol"; import "../crowdsale/validation/IndividuallyCappedCrowdsale.sol"; +import "./CapperRoleMock.sol"; -contract IndividuallyCappedCrowdsaleImpl is IndividuallyCappedCrowdsale { +contract IndividuallyCappedCrowdsaleImpl + is IndividuallyCappedCrowdsale, CapperRoleMock { - constructor ( + constructor( uint256 _rate, address _wallet, IERC20 _token @@ -15,5 +17,4 @@ contract IndividuallyCappedCrowdsaleImpl is IndividuallyCappedCrowdsale { Crowdsale(_rate, _wallet, _token) { } - } diff --git a/contracts/mocks/MinterRoleMock.sol b/contracts/mocks/MinterRoleMock.sol new file mode 100644 index 00000000000..232cbc19b20 --- /dev/null +++ b/contracts/mocks/MinterRoleMock.sol @@ -0,0 +1,18 @@ +pragma solidity ^0.4.24; + +import "../access/roles/MinterRole.sol"; + + +contract MinterRoleMock is MinterRole { + function removeMinter(address _account) public { + _removeMinter(_account); + } + + function onlyMinterMock() public view onlyMinter { + } + + // Causes a compilation error if super._removeMinter is not internal + function _removeMinter(address _account) internal { + super._removeMinter(_account); + } +} diff --git a/contracts/mocks/PausableMock.sol b/contracts/mocks/PausableMock.sol index 11802687aa5..a1c24c0877e 100644 --- a/contracts/mocks/PausableMock.sol +++ b/contracts/mocks/PausableMock.sol @@ -2,10 +2,11 @@ pragma solidity ^0.4.24; import "../lifecycle/Pausable.sol"; +import "./PauserRoleMock.sol"; // mock class using Pausable -contract PausableMock is Pausable { +contract PausableMock is Pausable, PauserRoleMock { bool public drasticMeasureTaken; uint256 public count; diff --git a/contracts/mocks/PauserRoleMock.sol b/contracts/mocks/PauserRoleMock.sol new file mode 100644 index 00000000000..21b0b0c0346 --- /dev/null +++ b/contracts/mocks/PauserRoleMock.sol @@ -0,0 +1,18 @@ +pragma solidity ^0.4.24; + +import "../access/roles/PauserRole.sol"; + + +contract PauserRoleMock is PauserRole { + function removePauser(address _account) public { + _removePauser(_account); + } + + function onlyPauserMock() public view onlyPauser { + } + + // Causes a compilation error if super._removePauser is not internal + function _removePauser(address _account) internal { + super._removePauser(_account); + } +} diff --git a/contracts/mocks/RBACCappedTokenMock.sol b/contracts/mocks/RBACCappedTokenMock.sol deleted file mode 100644 index 8d182c6e5be..00000000000 --- a/contracts/mocks/RBACCappedTokenMock.sol +++ /dev/null @@ -1,12 +0,0 @@ -pragma solidity ^0.4.24; - -import "../token/ERC20/RBACMintableToken.sol"; -import "../token/ERC20/ERC20Capped.sol"; - - -contract RBACCappedTokenMock is ERC20Capped, RBACMintableToken { - constructor(uint256 _cap) - ERC20Capped(_cap) - public - {} -} diff --git a/contracts/mocks/RBACMock.sol b/contracts/mocks/RBACMock.sol deleted file mode 100644 index f4acc927915..00000000000 --- a/contracts/mocks/RBACMock.sol +++ /dev/null @@ -1,69 +0,0 @@ -pragma solidity ^0.4.24; - -import "../examples/RBACWithAdmin.sol"; - - -contract RBACMock is RBACWithAdmin { - - string internal constant ROLE_ADVISOR = "advisor"; - - modifier onlyAdminOrAdvisor() - { - require( - isAdmin(msg.sender) || - hasRole(msg.sender, ROLE_ADVISOR) - ); - _; - } - - constructor(address[] _advisors) - public - { - _addRole(msg.sender, ROLE_ADVISOR); - - for (uint256 i = 0; i < _advisors.length; i++) { - _addRole(_advisors[i], ROLE_ADVISOR); - } - } - - function onlyAdminsCanDoThis() - external - onlyAdmin - view - { - } - - function onlyAdvisorsCanDoThis() - external - onlyRole(ROLE_ADVISOR) - view - { - } - - function eitherAdminOrAdvisorCanDoThis() - external - onlyAdminOrAdvisor - view - { - } - - function nobodyCanDoThis() - external - onlyRole("unknown") - view - { - } - - // admins can remove advisor's role - function removeAdvisor(address _account) - public - onlyAdmin - { - // revert if the user isn't an advisor - // (perhaps you want to soft-fail here instead?) - checkRole(_account, ROLE_ADVISOR); - - // remove the advisor's role - _removeRole(_account, ROLE_ADVISOR); - } -} diff --git a/contracts/mocks/RolesMock.sol b/contracts/mocks/RolesMock.sol new file mode 100644 index 00000000000..6baf873988e --- /dev/null +++ b/contracts/mocks/RolesMock.sol @@ -0,0 +1,22 @@ +pragma solidity ^0.4.24; + +import "../access/Roles.sol"; + + +contract RolesMock { + using Roles for Roles.Role; + + Roles.Role private dummyRole; + + function add(address _account) public { + dummyRole.add(_account); + } + + function remove(address _account) public { + dummyRole.remove(_account); + } + + function has(address _account) public view returns (bool) { + return dummyRole.has(_account); + } +} diff --git a/contracts/mocks/SecondaryMock.sol b/contracts/mocks/SecondaryMock.sol new file mode 100644 index 00000000000..6863112fa8c --- /dev/null +++ b/contracts/mocks/SecondaryMock.sol @@ -0,0 +1,9 @@ +pragma solidity ^0.4.24; + +import "../ownership/Secondary.sol"; + + +contract SecondaryMock is Secondary { + function onlyPrimaryMock() public view onlyPrimary { + } +} diff --git a/contracts/mocks/SignatureBouncerMock.sol b/contracts/mocks/SignatureBouncerMock.sol index d9fea81f402..d2663d2c527 100644 --- a/contracts/mocks/SignatureBouncerMock.sol +++ b/contracts/mocks/SignatureBouncerMock.sol @@ -1,9 +1,10 @@ pragma solidity ^0.4.24; import "../drafts/SignatureBouncer.sol"; +import "./SignerRoleMock.sol"; -contract SignatureBouncerMock is SignatureBouncer { +contract SignatureBouncerMock is SignatureBouncer, SignerRoleMock { function checkValidSignature(address _address, bytes _signature) public view diff --git a/contracts/mocks/SignerRoleMock.sol b/contracts/mocks/SignerRoleMock.sol new file mode 100644 index 00000000000..0157884e41c --- /dev/null +++ b/contracts/mocks/SignerRoleMock.sol @@ -0,0 +1,18 @@ +pragma solidity ^0.4.24; + +import "../access/roles/SignerRole.sol"; + + +contract SignerRoleMock is SignerRole { + function removeSigner(address _account) public { + _removeSigner(_account); + } + + function onlySignerMock() public view onlySigner { + } + + // Causes a compilation error if super._removeSigner is not internal + function _removeSigner(address _account) internal { + super._removeSigner(_account); + } +} diff --git a/contracts/mocks/WhitelistMock.sol b/contracts/mocks/WhitelistMock.sol deleted file mode 100644 index 081b6631bb2..00000000000 --- a/contracts/mocks/WhitelistMock.sol +++ /dev/null @@ -1,14 +0,0 @@ -pragma solidity ^0.4.24; - -import "../access/Whitelist.sol"; - - -contract WhitelistMock is Whitelist { - - function onlyWhitelistedCanDoThis() - external - onlyIfWhitelisted(msg.sender) - view - { - } -} diff --git a/contracts/mocks/WhitelistedCrowdsaleImpl.sol b/contracts/mocks/WhitelistedCrowdsaleImpl.sol deleted file mode 100644 index 25cd5118aea..00000000000 --- a/contracts/mocks/WhitelistedCrowdsaleImpl.sol +++ /dev/null @@ -1,19 +0,0 @@ -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/contracts/ownership/Secondary.sol b/contracts/ownership/Secondary.sol new file mode 100644 index 00000000000..e0397216cd1 --- /dev/null +++ b/contracts/ownership/Secondary.sol @@ -0,0 +1,35 @@ +pragma solidity ^0.4.24; + + +/** + * @title Secondary + * @dev A Secondary contract can only be used by its primary account (the one that created it) + */ +contract Secondary { + address private primary_; + + /** + * @dev Sets the primary account to the one that is creating the Secondary contract. + */ + constructor() public { + primary_ = msg.sender; + } + + /** + * @dev Reverts if called from any account other than the primary. + */ + modifier onlyPrimary() { + require(msg.sender == primary_); + _; + } + + function primary() public view returns (address) { + return primary_; + } + + function transferPrimary(address _recipient) public onlyPrimary { + require(_recipient != address(0)); + + primary_ = _recipient; + } +} diff --git a/contracts/ownership/Superuser.sol b/contracts/ownership/Superuser.sol deleted file mode 100644 index b1f1fa4b52d..00000000000 --- a/contracts/ownership/Superuser.sol +++ /dev/null @@ -1,62 +0,0 @@ -pragma solidity ^0.4.24; - - -import "./Ownable.sol"; -import "../access/rbac/RBAC.sol"; - - -/** - * @title Superuser - * @dev The Superuser contract defines a single superuser who can transfer the ownership - * of a contract to a new address, even if he is not the owner. - * A superuser can transfer his role to a new address. - */ -contract Superuser is Ownable, RBAC { - string private constant ROLE_SUPERUSER = "superuser"; - - constructor () public { - _addRole(msg.sender, ROLE_SUPERUSER); - } - - /** - * @dev Throws if called by any account that's not a superuser. - */ - modifier onlySuperuser() { - checkRole(msg.sender, ROLE_SUPERUSER); - _; - } - - modifier onlyOwnerOrSuperuser() { - require(msg.sender == owner() || isSuperuser(msg.sender)); - _; - } - - /** - * @dev getter to determine if an account has superuser role - */ - function isSuperuser(address _account) - public - view - returns (bool) - { - return hasRole(_account, ROLE_SUPERUSER); - } - - /** - * @dev Allows the current superuser to transfer his role to a newSuperuser. - * @param _newSuperuser The address to transfer ownership to. - */ - function transferSuperuser(address _newSuperuser) public onlySuperuser { - require(_newSuperuser != address(0)); - _removeRole(msg.sender, ROLE_SUPERUSER); - _addRole(_newSuperuser, ROLE_SUPERUSER); - } - - /** - * @dev Allows the current superuser or owner to transfer control of the contract to a newOwner. - * @param _newOwner The address to transfer ownership to. - */ - function transferOwnership(address _newOwner) public onlyOwnerOrSuperuser { - _transferOwnership(_newOwner); - } -} diff --git a/contracts/payment/Escrow.sol b/contracts/payment/Escrow.sol index ca0c124d773..0c6bf7fdca2 100644 --- a/contracts/payment/Escrow.sol +++ b/contracts/payment/Escrow.sol @@ -1,17 +1,17 @@ pragma solidity ^0.4.24; import "../math/SafeMath.sol"; -import "../ownership/Ownable.sol"; +import "../ownership/Secondary.sol"; /** * @title Escrow * @dev Base escrow contract, holds funds destinated to a payee until they * withdraw them. The contract that uses the escrow as its payment method - * should be its owner, and provide public methods redirecting to the escrow's + * should be its primary, and provide public methods redirecting to the escrow's * deposit and withdraw. */ -contract Escrow is Ownable { +contract Escrow is Secondary { using SafeMath for uint256; event Deposited(address indexed payee, uint256 weiAmount); @@ -27,7 +27,7 @@ contract Escrow is Ownable { * @dev Stores the sent amount as credit to be withdrawn. * @param _payee The destination address of the funds. */ - function deposit(address _payee) public onlyOwner payable { + function deposit(address _payee) public onlyPrimary payable { uint256 amount = msg.value; deposits_[_payee] = deposits_[_payee].add(amount); @@ -38,7 +38,7 @@ contract Escrow is Ownable { * @dev Withdraw accumulated balance for a payee. * @param _payee The address whose funds will be withdrawn and transferred to. */ - function withdraw(address _payee) public onlyOwner { + function withdraw(address _payee) public onlyPrimary { uint256 payment = deposits_[_payee]; assert(address(this).balance >= payment); diff --git a/contracts/payment/RefundEscrow.sol b/contracts/payment/RefundEscrow.sol index 884494f1a8e..1031a952f24 100644 --- a/contracts/payment/RefundEscrow.sol +++ b/contracts/payment/RefundEscrow.sol @@ -1,16 +1,16 @@ pragma solidity ^0.4.24; import "./ConditionalEscrow.sol"; -import "../ownership/Ownable.sol"; +import "../ownership/Secondary.sol"; /** * @title RefundEscrow * @dev Escrow that holds funds for a beneficiary, deposited from multiple parties. - * The contract owner may close the deposit period, and allow for either withdrawal + * The primary account may close the deposit period, and allow for either withdrawal * by the beneficiary, or refunds to the depositors. */ -contract RefundEscrow is Ownable, ConditionalEscrow { +contract RefundEscrow is Secondary, ConditionalEscrow { enum State { Active, Refunding, Closed } event Closed(); @@ -32,14 +32,14 @@ contract RefundEscrow is Ownable, ConditionalEscrow { /** * @return the current state of the escrow. */ - function state() public view returns(State) { + function state() public view returns (State) { return state_; } /** * @return the beneficiary of the escrow. */ - function beneficiary() public view returns(address) { + function beneficiary() public view returns (address) { return beneficiary_; } @@ -56,7 +56,7 @@ contract RefundEscrow is Ownable, ConditionalEscrow { * @dev Allows for the beneficiary to withdraw their funds, rejecting * further deposits. */ - function close() public onlyOwner { + function close() public onlyPrimary { require(state_ == State.Active); state_ = State.Closed; emit Closed(); @@ -65,7 +65,7 @@ contract RefundEscrow is Ownable, ConditionalEscrow { /** * @dev Allows for refunds to take place, rejecting further deposits. */ - function enableRefunds() public onlyOwner { + function enableRefunds() public onlyPrimary { require(state_ == State.Active); state_ = State.Refunding; emit RefundsEnabled(); diff --git a/contracts/token/ERC20/ERC20Capped.sol b/contracts/token/ERC20/ERC20Capped.sol index 23631f9049c..bc1e8eee04e 100644 --- a/contracts/token/ERC20/ERC20Capped.sol +++ b/contracts/token/ERC20/ERC20Capped.sol @@ -11,7 +11,9 @@ contract ERC20Capped is ERC20Mintable { uint256 private cap_; - constructor(uint256 _cap) public { + constructor(uint256 _cap) + public + { require(_cap > 0); cap_ = _cap; } diff --git a/contracts/token/ERC20/ERC20Detailed.sol b/contracts/token/ERC20/ERC20Detailed.sol index 6becdee1086..b6d50fca2f8 100644 --- a/contracts/token/ERC20/ERC20Detailed.sol +++ b/contracts/token/ERC20/ERC20Detailed.sol @@ -26,14 +26,12 @@ contract ERC20Detailed is IERC20 { function name() public view returns(string) { return name_; } - /** * @return the symbol of the token. */ function symbol() public view returns(string) { return symbol_; } - /** * @return the number of decimals of the token. */ diff --git a/contracts/token/ERC20/ERC20Mintable.sol b/contracts/token/ERC20/ERC20Mintable.sol index 8b80dd80ae5..430a3cdf795 100644 --- a/contracts/token/ERC20/ERC20Mintable.sol +++ b/contracts/token/ERC20/ERC20Mintable.sol @@ -1,31 +1,24 @@ pragma solidity ^0.4.24; import "./ERC20.sol"; -import "../../ownership/Ownable.sol"; +import "../../access/roles/MinterRole.sol"; /** - * @title Mintable token - * @dev Simple ERC20 Token example, with mintable token creation - * Based on code by TokenMarketNet: https://github.com/TokenMarketNet/ico/blob/master/contracts/MintableToken.sol + * @title ERC20Mintable + * @dev ERC20 minting logic */ -contract ERC20Mintable is ERC20, Ownable { - event Mint(address indexed to, uint256 amount); - event MintFinished(); +contract ERC20Mintable is ERC20, MinterRole { + event Minted(address indexed to, uint256 amount); + event MintingFinished(); bool private mintingFinished_ = false; - modifier onlyBeforeMintingFinished() { require(!mintingFinished_); _; } - modifier onlyMinter() { - require(isOwner()); - _; - } - /** * @return true if the minting is finished. */ @@ -49,7 +42,7 @@ contract ERC20Mintable is ERC20, Ownable { returns (bool) { _mint(_to, _amount); - emit Mint(_to, _amount); + emit Minted(_to, _amount); return true; } @@ -59,12 +52,12 @@ contract ERC20Mintable is ERC20, Ownable { */ function finishMinting() public - onlyOwner + onlyMinter onlyBeforeMintingFinished returns (bool) { mintingFinished_ = true; - emit MintFinished(); + emit MintingFinished(); return true; } } diff --git a/contracts/token/ERC20/RBACMintableToken.sol b/contracts/token/ERC20/RBACMintableToken.sol deleted file mode 100644 index 65e16f564f8..00000000000 --- a/contracts/token/ERC20/RBACMintableToken.sol +++ /dev/null @@ -1,48 +0,0 @@ -pragma solidity ^0.4.24; - -import "./ERC20Mintable.sol"; -import "../../access/rbac/RBAC.sol"; - - -/** - * @title RBACMintableToken - * @author Vittorio Minacori (@vittominacori) - * @dev Mintable Token, with RBAC minter permissions - */ -contract RBACMintableToken is ERC20Mintable, RBAC { - /** - * A constant role name for indicating minters. - */ - string private constant ROLE_MINTER = "minter"; - - /** - * @dev override the Mintable token modifier to add role based logic - */ - modifier onlyMinter() { - checkRole(msg.sender, ROLE_MINTER); - _; - } - - /** - * @return true if the account is a minter, false otherwise. - */ - function isMinter(address _account) public view returns(bool) { - return hasRole(_account, ROLE_MINTER); - } - - /** - * @dev add a minter role to an address - * @param _minter address - */ - function addMinter(address _minter) public onlyOwner { - _addRole(_minter, ROLE_MINTER); - } - - /** - * @dev remove a minter role from an address - * @param _minter address - */ - function removeMinter(address _minter) public onlyOwner { - _removeRole(_minter, ROLE_MINTER); - } -} diff --git a/contracts/token/ERC721/ERC721Burnable.sol b/contracts/token/ERC721/ERC721Burnable.sol new file mode 100644 index 00000000000..ea42dab0528 --- /dev/null +++ b/contracts/token/ERC721/ERC721Burnable.sol @@ -0,0 +1,13 @@ +pragma solidity ^0.4.24; + +import "./ERC721.sol"; + + +contract ERC721Burnable is ERC721 { + function burn(uint256 _tokenId) + public + { + require(_isApprovedOrOwner(msg.sender, _tokenId)); + _burn(ownerOf(_tokenId), _tokenId); + } +} diff --git a/contracts/token/ERC721/ERC721Mintable.sol b/contracts/token/ERC721/ERC721Mintable.sol new file mode 100644 index 00000000000..adee3595200 --- /dev/null +++ b/contracts/token/ERC721/ERC721Mintable.sol @@ -0,0 +1,78 @@ +pragma solidity ^0.4.24; + +import "./ERC721.sol"; +import "../../access/roles/MinterRole.sol"; + + +/** + * @title ERC721Mintable + * @dev ERC721 minting logic + */ +contract ERC721Mintable is ERC721, MinterRole { + event Minted(address indexed to, uint256 tokenId); + 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. + * @param _tokenId The token id to mint. + * @return A boolean that indicates if the operation was successful. + */ + function mint( + address _to, + uint256 _tokenId + ) + public + onlyMinter + onlyBeforeMintingFinished + returns (bool) + { + _mint(_to, _tokenId); + emit Minted(_to, _tokenId); + return true; + } + + function mintWithTokenURI( + address _to, + uint256 _tokenId, + string _tokenURI + ) + 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/access/Roles.test.js b/test/access/Roles.test.js new file mode 100644 index 00000000000..be13be072d1 --- /dev/null +++ b/test/access/Roles.test.js @@ -0,0 +1,61 @@ +const RolesMock = artifacts.require('RolesMock'); + +require('chai') + .should(); + +contract('Roles', function ([_, authorized, otherAuthorized, anyone]) { + const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; + + beforeEach(async function () { + this.roles = await RolesMock.new(); + }); + + context('initially', function () { + it('doesn\'t pre-assign roles', async function () { + (await this.roles.has(authorized)).should.equal(false); + (await this.roles.has(otherAuthorized)).should.equal(false); + (await this.roles.has(anyone)).should.equal(false); + }); + + describe('adding roles', function () { + it('adds roles to a single account', async function () { + await this.roles.add(authorized); + (await this.roles.has(authorized)).should.equal(true); + (await this.roles.has(anyone)).should.equal(false); + }); + + it('adds roles to an already-assigned account', async function () { + await this.roles.add(authorized); + await this.roles.add(authorized); + (await this.roles.has(authorized)).should.equal(true); + }); + + it('doesn\'t revert when adding roles to the null account', async function () { + await this.roles.add(ZERO_ADDRESS); + }); + }); + }); + + context('with added roles', function () { + beforeEach(async function () { + await this.roles.add(authorized); + await this.roles.add(otherAuthorized); + }); + + describe('removing roles', function () { + it('removes a single role', async function () { + await this.roles.remove(authorized); + (await this.roles.has(authorized)).should.equal(false); + (await this.roles.has(otherAuthorized)).should.equal(true); + }); + + it('doesn\'t revert when removing unassigned roles', async function () { + await this.roles.remove(anyone); + }); + + it('doesn\'t revert when removing roles from the null account', async function () { + await this.roles.remove(ZERO_ADDRESS); + }); + }); + }); +}); diff --git a/test/access/SignatureBouncer.test.js b/test/access/SignatureBouncer.test.js deleted file mode 100644 index 798ab0ac9fa..00000000000 --- a/test/access/SignatureBouncer.test.js +++ /dev/null @@ -1,246 +0,0 @@ -const { assertRevert } = require('../helpers/assertRevert'); -const { getBouncerSigner } = require('../helpers/sign'); - -const Bouncer = artifacts.require('SignatureBouncerMock'); - -const BigNumber = web3.BigNumber; - -require('chai') - .use(require('chai-bignumber')(BigNumber)) - .should(); - -const UINT_VALUE = 23; -const BYTES_VALUE = web3.toHex('test'); -const INVALID_SIGNATURE = '0xabcd'; - -contract('Bouncer', function ([_, owner, anyone, bouncerAddress, authorizedUser]) { - beforeEach(async function () { - this.bouncer = await Bouncer.new({ from: owner }); - }); - - context('management', function () { - it('has a default owner of self', async function () { - (await this.bouncer.owner()).should.equal(owner); - }); - - it('allows the owner to add a bouncer', async function () { - await this.bouncer.addBouncer(bouncerAddress, { from: owner }); - (await this.bouncer.isBouncer(bouncerAddress)).should.equal(true); - }); - - it('does not allow adding an invalid address', async function () { - await assertRevert( - this.bouncer.addBouncer('0x0', { from: owner }) - ); - }); - - it('allows the owner to remove a bouncer', async function () { - await this.bouncer.addBouncer(bouncerAddress, { from: owner }); - - await this.bouncer.removeBouncer(bouncerAddress, { from: owner }); - (await this.bouncer.isBouncer(bouncerAddress)).should.equal(false); - }); - - it('does not allow anyone to add a bouncer', async function () { - await assertRevert( - this.bouncer.addBouncer(bouncerAddress, { from: anyone }) - ); - }); - - it('does not allow anyone to remove a bouncer', async function () { - await this.bouncer.addBouncer(bouncerAddress, { from: owner }); - - await assertRevert( - this.bouncer.removeBouncer(bouncerAddress, { from: anyone }) - ); - }); - }); - - context('with bouncer address', function () { - beforeEach(async function () { - await this.bouncer.addBouncer(bouncerAddress, { from: owner }); - this.signFor = getBouncerSigner(this.bouncer, bouncerAddress); - }); - - describe('modifiers', function () { - context('plain signature', function () { - it('allows valid signature for sender', async function () { - await this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser), { from: authorizedUser }); - }); - - it('does not allow invalid signature for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignature(INVALID_SIGNATURE, { from: authorizedUser }) - ); - }); - - it('does not allow valid signature for other sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser), { from: anyone }) - ); - }); - - it('does not allow valid signature for method for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignature(this.signFor(authorizedUser, 'onlyWithValidSignature'), - { from: authorizedUser }) - ); - }); - }); - - context('method signature', function () { - it('allows valid signature with correct method for sender', async function () { - await this.bouncer.onlyWithValidSignatureAndMethod( - this.signFor(authorizedUser, 'onlyWithValidSignatureAndMethod'), { from: authorizedUser } - ); - }); - - it('does not allow invalid signature with correct method for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndMethod(INVALID_SIGNATURE, { from: authorizedUser }) - ); - }); - - it('does not allow valid signature with correct method for other sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndMethod( - this.signFor(authorizedUser, 'onlyWithValidSignatureAndMethod'), { from: anyone } - ) - ); - }); - - it('does not allow valid method signature with incorrect method for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndMethod(this.signFor(authorizedUser, 'theWrongMethod'), - { from: authorizedUser }) - ); - }); - - it('does not allow valid non-method signature method for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndMethod(this.signFor(authorizedUser), { from: authorizedUser }) - ); - }); - }); - - context('method and data signature', function () { - it('allows valid signature with correct method and data for sender', async function () { - await this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, - this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), { from: authorizedUser } - ); - }); - - it('does not allow invalid signature with correct method and data for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, INVALID_SIGNATURE, { from: authorizedUser }) - ); - }); - - it('does not allow valid signature with correct method and incorrect data for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE + 10, - this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), - { from: authorizedUser } - ) - ); - }); - - it('does not allow valid signature with correct method and data for other sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, - this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), - { from: anyone } - ) - ); - }); - - it('does not allow valid non-method signature for sender', async function () { - await assertRevert( - this.bouncer.onlyWithValidSignatureAndData(UINT_VALUE, - this.signFor(authorizedUser), { from: authorizedUser } - ) - ); - }); - }); - }); - - context('signature validation', function () { - context('plain signature', function () { - it('validates valid signature for valid user', async function () { - (await this.bouncer.checkValidSignature(authorizedUser, this.signFor(authorizedUser))).should.equal(true); - }); - - it('does not validate invalid signature for valid user', async function () { - (await this.bouncer.checkValidSignature(authorizedUser, INVALID_SIGNATURE)).should.equal(false); - }); - - it('does not validate valid signature for anyone', async function () { - (await this.bouncer.checkValidSignature(anyone, this.signFor(authorizedUser))).should.equal(false); - }); - - it('does not validate valid signature for method for valid user', async function () { - (await this.bouncer.checkValidSignature(authorizedUser, this.signFor(authorizedUser, 'checkValidSignature')) - ).should.equal(false); - }); - }); - - context('method signature', function () { - it('validates valid signature with correct method for valid user', async function () { - (await this.bouncer.checkValidSignatureAndMethod(authorizedUser, - this.signFor(authorizedUser, 'checkValidSignatureAndMethod')) - ).should.equal(true); - }); - - it('does not validate invalid signature with correct method for valid user', async function () { - (await this.bouncer.checkValidSignatureAndMethod(authorizedUser, INVALID_SIGNATURE)).should.equal(false); - }); - - it('does not validate valid signature with correct method for anyone', async function () { - (await this.bouncer.checkValidSignatureAndMethod(anyone, - this.signFor(authorizedUser, 'checkValidSignatureAndMethod')) - ).should.equal(false); - }); - - it('does not validate valid non-method signature with correct method for valid user', async function () { - (await this.bouncer.checkValidSignatureAndMethod(authorizedUser, this.signFor(authorizedUser)) - ).should.equal(false); - }); - }); - - context('method and data signature', function () { - it('validates valid signature with correct method and data for valid user', async function () { - (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, - this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) - ).should.equal(true); - }); - - it('does not validate invalid signature with correct method and data for valid user', async function () { - (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, INVALID_SIGNATURE) - ).should.equal(false); - }); - - it('does not validate valid signature with correct method and incorrect data for valid user', - async function () { - (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE + 10, - this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) - ).should.equal(false); - } - ); - - it('does not validate valid signature with correct method and data for anyone', async function () { - (await this.bouncer.checkValidSignatureAndData(anyone, BYTES_VALUE, UINT_VALUE, - this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) - ).should.equal(false); - }); - - it('does not validate valid non-method-data signature with correct method and data for valid user', - async function () { - (await this.bouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, - this.signFor(authorizedUser, 'checkValidSignatureAndData')) - ).should.equal(false); - } - ); - }); - }); - }); -}); diff --git a/test/access/Whitelist.test.js b/test/access/Whitelist.test.js deleted file mode 100644 index f1113ffc3c9..00000000000 --- a/test/access/Whitelist.test.js +++ /dev/null @@ -1,65 +0,0 @@ -const { expectThrow } = require('../helpers/expectThrow'); - -const WhitelistMock = artifacts.require('WhitelistMock'); - -require('chai') - .should(); - -contract('Whitelist', function ([_, owner, whitelistedAddress1, whitelistedAddress2, anyone]) { - const whitelistedAddresses = [whitelistedAddress1, whitelistedAddress2]; - - beforeEach(async function () { - this.mock = await WhitelistMock.new({ from: owner }); - }); - - context('in normal conditions', function () { - it('should add address to the whitelist', async function () { - await this.mock.addAddressToWhitelist(whitelistedAddress1, { from: owner }); - (await this.mock.isWhitelisted(whitelistedAddress1)).should.equal(true); - }); - - it('should add addresses to the whitelist', async function () { - await this.mock.addAddressesToWhitelist(whitelistedAddresses, { from: owner }); - for (const addr of whitelistedAddresses) { - (await this.mock.isWhitelisted(addr)).should.equal(true); - } - }); - - it('should remove address from the whitelist', async function () { - await this.mock.removeAddressFromWhitelist(whitelistedAddress1, { from: owner }); - (await this.mock.isWhitelisted(whitelistedAddress1)).should.equal(false); - }); - - it('should remove addresses from the the whitelist', async function () { - await this.mock.removeAddressesFromWhitelist(whitelistedAddresses, { from: owner }); - for (const addr of whitelistedAddresses) { - (await this.mock.isWhitelisted(addr)).should.equal(false); - } - }); - - it('should allow whitelisted address to call #onlyWhitelistedCanDoThis', async function () { - await this.mock.addAddressToWhitelist(whitelistedAddress1, { from: owner }); - await this.mock.onlyWhitelistedCanDoThis({ from: whitelistedAddress1 }); - }); - }); - - context('in adversarial conditions', function () { - it('should not allow "anyone" to add to the whitelist', async function () { - await expectThrow( - this.mock.addAddressToWhitelist(whitelistedAddress1, { from: anyone }) - ); - }); - - it('should not allow "anyone" to remove from the whitelist', async function () { - await expectThrow( - this.mock.removeAddressFromWhitelist(whitelistedAddress1, { from: anyone }) - ); - }); - - it('should not allow "anyone" to call #onlyWhitelistedCanDoThis', async function () { - await expectThrow( - this.mock.onlyWhitelistedCanDoThis({ from: anyone }) - ); - }); - }); -}); diff --git a/test/access/roles/CapperRole.test.js b/test/access/roles/CapperRole.test.js new file mode 100644 index 00000000000..a79944a2778 --- /dev/null +++ b/test/access/roles/CapperRole.test.js @@ -0,0 +1,11 @@ +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); +const CapperRoleMock = artifacts.require('CapperRoleMock'); + +contract('CapperRole', function ([_, capper, otherCapper, ...otherAccounts]) { + beforeEach(async function () { + this.contract = await CapperRoleMock.new({ from: capper }); + await this.contract.addCapper(otherCapper, { from: capper }); + }); + + shouldBehaveLikePublicRole(capper, otherCapper, otherAccounts, 'capper'); +}); diff --git a/test/access/roles/MinterRole.test.js b/test/access/roles/MinterRole.test.js new file mode 100644 index 00000000000..b2e70b53a45 --- /dev/null +++ b/test/access/roles/MinterRole.test.js @@ -0,0 +1,11 @@ +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); +const MinterRoleMock = artifacts.require('MinterRoleMock'); + +contract('MinterRole', function ([_, minter, otherMinter, ...otherAccounts]) { + beforeEach(async function () { + this.contract = await MinterRoleMock.new({ from: minter }); + await this.contract.addMinter(otherMinter, { from: minter }); + }); + + shouldBehaveLikePublicRole(minter, otherMinter, otherAccounts, 'minter'); +}); diff --git a/test/access/roles/PauserRole.test.js b/test/access/roles/PauserRole.test.js new file mode 100644 index 00000000000..927e46c0ba8 --- /dev/null +++ b/test/access/roles/PauserRole.test.js @@ -0,0 +1,11 @@ +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); +const PauserRoleMock = artifacts.require('PauserRoleMock'); + +contract('PauserRole', function ([_, pauser, otherPauser, ...otherAccounts]) { + beforeEach(async function () { + this.contract = await PauserRoleMock.new({ from: pauser }); + await this.contract.addPauser(otherPauser, { from: pauser }); + }); + + shouldBehaveLikePublicRole(pauser, otherPauser, otherAccounts, 'pauser'); +}); diff --git a/test/access/roles/PublicRole.behavior.js b/test/access/roles/PublicRole.behavior.js new file mode 100644 index 00000000000..3181b705165 --- /dev/null +++ b/test/access/roles/PublicRole.behavior.js @@ -0,0 +1,66 @@ +require('chai') + .should(); + +function capitalize (str) { + return str.replace(/\b\w/g, l => l.toUpperCase()); +} + +function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], rolename) { + rolename = capitalize(rolename); + const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; + + describe('should behave like public role', function () { + beforeEach('check preconditions', async function () { + (await this.contract[`is${rolename}`](authorized)).should.equal(true); + (await this.contract[`is${rolename}`](otherAuthorized)).should.equal(true); + (await this.contract[`is${rolename}`](anyone)).should.equal(false); + }); + + 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); + }); + + it('adds role to an already-assigned account', async function () { + await this.contract[`add${rolename}`](authorized, { from: authorized }); + (await this.contract[`is${rolename}`](authorized)).should.equal(true); + }); + + it('doesn\'t revert when adding role to the null account', async function () { + await this.contract[`add${rolename}`](ZERO_ADDRESS, { from: authorized }); + }); + }); + + 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); + }); + + it('doesn\'t revert when removing from an unassigned account', async function () { + await this.contract[`remove${rolename}`](anyone); + }); + + it('doesn\'t revert when removing role from the null account', async function () { + await this.contract[`remove${rolename}`](ZERO_ADDRESS); + }); + }); + + describe('renouncing roles', function () { + it('renounces an assigned role', async function () { + await this.contract[`renounce${rolename}`]({ from: authorized }); + (await this.contract[`is${rolename}`](authorized)).should.equal(false); + }); + + it('doesn\'t revert when renouncing unassigned role', async function () { + await this.contract[`renounce${rolename}`]({ from: anyone }); + }); + }); + }); +} + +module.exports = { + shouldBehaveLikePublicRole, +}; diff --git a/test/access/roles/SignerRole.test.js b/test/access/roles/SignerRole.test.js new file mode 100644 index 00000000000..317c100d7ed --- /dev/null +++ b/test/access/roles/SignerRole.test.js @@ -0,0 +1,11 @@ +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); +const SignerRoleMock = artifacts.require('SignerRoleMock'); + +contract('SignerRole', function ([_, signer, otherSigner, ...otherAccounts]) { + beforeEach(async function () { + this.contract = await SignerRoleMock.new({ from: signer }); + await this.contract.addSigner(otherSigner, { from: signer }); + }); + + shouldBehaveLikePublicRole(signer, otherSigner, otherAccounts, 'signer'); +}); diff --git a/test/crowdsale/FinalizableCrowdsale.test.js b/test/crowdsale/FinalizableCrowdsale.test.js index ed2d5314157..df7a743dd20 100644 --- a/test/crowdsale/FinalizableCrowdsale.test.js +++ b/test/crowdsale/FinalizableCrowdsale.test.js @@ -11,9 +11,9 @@ const should = require('chai') .should(); const FinalizableCrowdsale = artifacts.require('FinalizableCrowdsaleImpl'); -const ERC20Mintable = artifacts.require('ERC20Mintable'); +const ERC20 = artifacts.require('ERC20'); -contract('FinalizableCrowdsale', function ([_, owner, wallet, thirdparty]) { +contract('FinalizableCrowdsale', function ([_, wallet, anyone]) { const rate = new BigNumber(1000); before(async function () { @@ -26,36 +26,30 @@ contract('FinalizableCrowdsale', function ([_, owner, wallet, thirdparty]) { this.closingTime = this.openingTime + duration.weeks(1); this.afterClosingTime = this.closingTime + duration.seconds(1); - this.token = await ERC20Mintable.new(); + this.token = await ERC20.new(); this.crowdsale = await FinalizableCrowdsale.new( - this.openingTime, this.closingTime, rate, wallet, this.token.address, { from: owner } + this.openingTime, this.closingTime, rate, wallet, this.token.address ); - await this.token.transferOwnership(this.crowdsale.address); }); it('cannot be finalized before ending', async function () { - await expectThrow(this.crowdsale.finalize({ from: owner }), EVMRevert); + await expectThrow(this.crowdsale.finalize({ from: anyone }), EVMRevert); }); - it('cannot be finalized by third party after ending', async function () { + it('can be finalized by anyone after ending', async function () { await increaseTimeTo(this.afterClosingTime); - await expectThrow(this.crowdsale.finalize({ from: thirdparty }), EVMRevert); - }); - - it('can be finalized by owner after ending', async function () { - await increaseTimeTo(this.afterClosingTime); - await this.crowdsale.finalize({ from: owner }); + await this.crowdsale.finalize({ from: anyone }); }); it('cannot be finalized twice', async function () { await increaseTimeTo(this.afterClosingTime); - await this.crowdsale.finalize({ from: owner }); - await expectThrow(this.crowdsale.finalize({ from: owner }), EVMRevert); + await this.crowdsale.finalize({ from: anyone }); + await expectThrow(this.crowdsale.finalize({ from: anyone }), EVMRevert); }); it('logs finalized', async function () { await increaseTimeTo(this.afterClosingTime); - const { logs } = await this.crowdsale.finalize({ from: owner }); + const { logs } = await this.crowdsale.finalize({ from: anyone }); const event = logs.find(e => e.event === 'CrowdsaleFinalized'); should.exist(event); }); diff --git a/test/crowdsale/IndividuallyCappedCrowdsale.test.js b/test/crowdsale/IndividuallyCappedCrowdsale.test.js index b174c81bb62..70c01a6b9b6 100644 --- a/test/crowdsale/IndividuallyCappedCrowdsale.test.js +++ b/test/crowdsale/IndividuallyCappedCrowdsale.test.js @@ -8,10 +8,12 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -const CappedCrowdsale = artifacts.require('IndividuallyCappedCrowdsaleImpl'); +const IndividuallyCappedCrowdsaleImpl = artifacts.require('IndividuallyCappedCrowdsaleImpl'); const SimpleToken = artifacts.require('SimpleToken'); +const { shouldBehaveLikePublicRole } = require('../access/roles/PublicRole.behavior'); -contract('IndividuallyCappedCrowdsale', function ([_, wallet, alice, bob, charlie]) { +contract('IndividuallyCappedCrowdsale', function ( + [_, capper, otherCapper, wallet, alice, bob, charlie, anyone, ...otherAccounts]) { const rate = new BigNumber(1); const capAlice = ether(10); const capBob = ether(2); @@ -19,49 +21,72 @@ contract('IndividuallyCappedCrowdsale', function ([_, wallet, alice, bob, charli const lessThanCapBoth = ether(1); const tokenSupply = new BigNumber('1e22'); - describe('individual capping', function () { + beforeEach(async function () { + this.token = await SimpleToken.new(); + this.crowdsale = await IndividuallyCappedCrowdsaleImpl.new(rate, wallet, this.token.address, { from: capper }); + }); + + describe('capper role', function () { beforeEach(async function () { - this.token = await SimpleToken.new(); - this.crowdsale = await CappedCrowdsale.new(rate, wallet, this.token.address); - await this.crowdsale.setCap(alice, capAlice); - await this.crowdsale.setCap(bob, capBob); - await this.token.transfer(this.crowdsale.address, tokenSupply); + this.contract = this.crowdsale; + await this.contract.addCapper(otherCapper, { from: capper }); }); - describe('accepting payments', function () { - it('should accept payments within cap', async function () { - await this.crowdsale.buyTokens(alice, { value: lessThanCapAlice }); - await this.crowdsale.buyTokens(bob, { value: lessThanCapBoth }); - }); + shouldBehaveLikePublicRole(capper, otherCapper, otherAccounts, 'capper'); + }); - it('should reject payments outside cap', async function () { - await this.crowdsale.buyTokens(alice, { value: capAlice }); - await expectThrow(this.crowdsale.buyTokens(alice, { value: 1 }), EVMRevert); - }); + describe('individual caps', function () { + it('sets a cap when the sender is a capper', async function () { + await this.crowdsale.setCap(alice, capAlice, { from: capper }); + (await this.crowdsale.getCap(alice)).should.be.bignumber.equal(capAlice); + }); - it('should reject payments that exceed cap', async function () { - await expectThrow(this.crowdsale.buyTokens(alice, { value: capAlice.plus(1) }), EVMRevert); - await expectThrow(this.crowdsale.buyTokens(bob, { value: capBob.plus(1) }), EVMRevert); - }); + it('reverts when a non-capper sets a cap', async function () { + await expectThrow(this.crowdsale.setCap(alice, capAlice, { from: anyone }), EVMRevert); + }); - it('should manage independent caps', async function () { - await this.crowdsale.buyTokens(alice, { value: lessThanCapAlice }); - await expectThrow(this.crowdsale.buyTokens(bob, { value: lessThanCapAlice }), EVMRevert); + context('with individual caps', function () { + beforeEach(async function () { + await this.crowdsale.setCap(alice, capAlice, { from: capper }); + await this.crowdsale.setCap(bob, capBob, { from: capper }); + await this.token.transfer(this.crowdsale.address, tokenSupply); }); - it('should default to a cap of zero', async function () { - await expectThrow(this.crowdsale.buyTokens(charlie, { value: lessThanCapBoth }), EVMRevert); - }); - }); + describe('accepting payments', function () { + it('should accept payments within cap', async function () { + await this.crowdsale.buyTokens(alice, { value: lessThanCapAlice }); + await this.crowdsale.buyTokens(bob, { value: lessThanCapBoth }); + }); + + it('should reject payments outside cap', async function () { + await this.crowdsale.buyTokens(alice, { value: capAlice }); + await expectThrow(this.crowdsale.buyTokens(alice, { value: 1 }), EVMRevert); + }); - describe('reporting state', function () { - it('should report correct cap', async function () { - (await this.crowdsale.getCap(alice)).should.be.bignumber.equal(capAlice); + it('should reject payments that exceed cap', async function () { + await expectThrow(this.crowdsale.buyTokens(alice, { value: capAlice.plus(1) }), EVMRevert); + await expectThrow(this.crowdsale.buyTokens(bob, { value: capBob.plus(1) }), EVMRevert); + }); + + it('should manage independent caps', async function () { + await this.crowdsale.buyTokens(alice, { value: lessThanCapAlice }); + await expectThrow(this.crowdsale.buyTokens(bob, { value: lessThanCapAlice }), EVMRevert); + }); + + it('should default to a cap of zero', async function () { + await expectThrow(this.crowdsale.buyTokens(charlie, { value: lessThanCapBoth }), EVMRevert); + }); }); - it('should report actual contribution', async function () { - await this.crowdsale.buyTokens(alice, { value: lessThanCapAlice }); - (await this.crowdsale.getContribution(alice)).should.be.bignumber.equal(lessThanCapAlice); + describe('reporting state', function () { + it('should report correct cap', async function () { + (await this.crowdsale.getCap(alice)).should.be.bignumber.equal(capAlice); + }); + + it('should report actual contribution', async function () { + await this.crowdsale.buyTokens(alice, { value: lessThanCapAlice }); + (await this.crowdsale.getContribution(alice)).should.be.bignumber.equal(lessThanCapAlice); + }); }); }); }); diff --git a/test/crowdsale/MintedCrowdsale.test.js b/test/crowdsale/MintedCrowdsale.test.js index 50a37eaef72..e254ddf1b93 100644 --- a/test/crowdsale/MintedCrowdsale.test.js +++ b/test/crowdsale/MintedCrowdsale.test.js @@ -6,35 +6,22 @@ const BigNumber = web3.BigNumber; const MintedCrowdsale = artifacts.require('MintedCrowdsaleImpl'); const ERC20Mintable = artifacts.require('ERC20Mintable'); -const RBACMintableToken = artifacts.require('RBACMintableToken'); const ERC20 = artifacts.require('ERC20'); -contract('MintedCrowdsale', function ([_, investor, wallet, purchaser]) { +contract('MintedCrowdsale', function ([_, deployer, investor, wallet, purchaser]) { const rate = new BigNumber(1000); const value = ether(5); describe('using ERC20Mintable', function () { beforeEach(async function () { - this.token = await ERC20Mintable.new(); + this.token = await ERC20Mintable.new({ from: deployer }); this.crowdsale = await MintedCrowdsale.new(rate, wallet, this.token.address); - await this.token.transferOwnership(this.crowdsale.address); - }); - - it('should be token owner', async function () { - (await this.token.owner()).should.equal(this.crowdsale.address); - }); - - shouldBehaveLikeMintedCrowdsale([_, investor, wallet, purchaser], rate, value); - }); - describe('using RBACMintableToken', function () { - beforeEach(async function () { - this.token = await RBACMintableToken.new(); - this.crowdsale = await MintedCrowdsale.new(rate, wallet, this.token.address); - await this.token.addMinter(this.crowdsale.address); + await this.token.addMinter(this.crowdsale.address, { from: deployer }); + await this.token.renounceMinter({ from: deployer }); }); - it('should have minter role on token', async function () { + it('crowdsale should be minter', async function () { (await this.token.isMinter(this.crowdsale.address)).should.equal(true); }); diff --git a/test/crowdsale/RefundableCrowdsale.test.js b/test/crowdsale/RefundableCrowdsale.test.js index c031073353f..3a5305ceb5d 100644 --- a/test/crowdsale/RefundableCrowdsale.test.js +++ b/test/crowdsale/RefundableCrowdsale.test.js @@ -15,7 +15,7 @@ require('chai') const RefundableCrowdsale = artifacts.require('RefundableCrowdsaleImpl'); const SimpleToken = artifacts.require('SimpleToken'); -contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser]) { +contract('RefundableCrowdsale', function ([_, wallet, investor, purchaser, anyone]) { const rate = new BigNumber(1); const goal = ether(50); const lessThanGoal = ether(45); @@ -38,7 +38,7 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser it('rejects a goal of zero', async function () { await expectThrow( RefundableCrowdsale.new( - this.openingTime, this.closingTime, rate, wallet, this.token.address, 0, { from: owner } + this.openingTime, this.closingTime, rate, wallet, this.token.address, 0, ), EVMRevert, ); @@ -47,7 +47,7 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser context('with crowdsale', function () { beforeEach(async function () { this.crowdsale = await RefundableCrowdsale.new( - this.openingTime, this.closingTime, rate, wallet, this.token.address, goal, { from: owner } + this.openingTime, this.closingTime, rate, wallet, this.token.address, goal ); await this.token.transfer(this.crowdsale.address, tokenSupply); @@ -76,7 +76,7 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser context('after closing time and finalization', function () { beforeEach(async function () { await increaseTimeTo(this.afterClosingTime); - await this.crowdsale.finalize({ from: owner }); + await this.crowdsale.finalize({ from: anyone }); }); it('refunds', async function () { @@ -96,7 +96,7 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser context('after closing time and finalization', function () { beforeEach(async function () { await increaseTimeTo(this.afterClosingTime); - await this.crowdsale.finalize({ from: owner }); + await this.crowdsale.finalize({ from: anyone }); }); it('denies refunds', async function () { diff --git a/test/crowdsale/WhitelistedCrowdsale.test.js b/test/crowdsale/WhitelistedCrowdsale.test.js deleted file mode 100644 index 3cf9b3206b9..00000000000 --- a/test/crowdsale/WhitelistedCrowdsale.test.js +++ /dev/null @@ -1,89 +0,0 @@ -const { ether } = require('../helpers/ether'); -const { expectThrow } = require('../helpers/expectThrow'); - -const BigNumber = web3.BigNumber; - -require('chai') - .should(); - -const WhitelistedCrowdsale = artifacts.require('WhitelistedCrowdsaleImpl'); -const SimpleToken = artifacts.require('SimpleToken'); - -contract('WhitelistedCrowdsale', function ([_, wallet, authorized, unauthorized, anotherAuthorized]) { - const rate = 1; - const value = ether(42); - const tokenSupply = new BigNumber('1e22'); - - describe('single user whitelisting', function () { - beforeEach(async function () { - this.token = await SimpleToken.new(); - this.crowdsale = await WhitelistedCrowdsale.new(rate, wallet, this.token.address); - await this.token.transfer(this.crowdsale.address, tokenSupply); - await this.crowdsale.addAddressToWhitelist(authorized); - }); - - describe('accepting payments', function () { - it('should accept payments to whitelisted (from whichever buyers)', async function () { - await this.crowdsale.sendTransaction({ value, from: authorized }); - await this.crowdsale.buyTokens(authorized, { value: value, from: authorized }); - await this.crowdsale.buyTokens(authorized, { value: value, from: unauthorized }); - }); - - it('should reject payments to not whitelisted (from whichever buyers)', async function () { - await expectThrow(this.crowdsale.sendTransaction({ value, from: unauthorized })); - await expectThrow(this.crowdsale.buyTokens(unauthorized, { value: value, from: unauthorized })); - await expectThrow(this.crowdsale.buyTokens(unauthorized, { value: value, from: authorized })); - }); - - it('should reject payments to addresses removed from whitelist', async function () { - await this.crowdsale.removeAddressFromWhitelist(authorized); - await expectThrow(this.crowdsale.buyTokens(authorized, { value: value, from: authorized })); - }); - }); - - describe('reporting whitelisted', function () { - it('should correctly report whitelisted addresses', async function () { - (await this.crowdsale.isWhitelisted(authorized)).should.equal(true); - (await this.crowdsale.isWhitelisted(unauthorized)).should.equal(false); - }); - }); - }); - - describe('many user whitelisting', function () { - beforeEach(async function () { - this.token = await SimpleToken.new(); - this.crowdsale = await WhitelistedCrowdsale.new(rate, wallet, this.token.address); - await this.token.transfer(this.crowdsale.address, tokenSupply); - await this.crowdsale.addAddressesToWhitelist([authorized, anotherAuthorized]); - }); - - describe('accepting payments', function () { - it('should accept payments to whitelisted (from whichever buyers)', async function () { - await this.crowdsale.buyTokens(authorized, { value: value, from: authorized }); - await this.crowdsale.buyTokens(authorized, { value: value, from: unauthorized }); - await this.crowdsale.buyTokens(anotherAuthorized, { value: value, from: authorized }); - await this.crowdsale.buyTokens(anotherAuthorized, { value: value, from: unauthorized }); - }); - - it('should reject payments to not whitelisted (with whichever buyers)', async function () { - await expectThrow(this.crowdsale.send(value)); - await expectThrow(this.crowdsale.buyTokens(unauthorized, { value: value, from: unauthorized })); - await expectThrow(this.crowdsale.buyTokens(unauthorized, { value: value, from: authorized })); - }); - - it('should reject payments to addresses removed from whitelist', async function () { - await this.crowdsale.removeAddressFromWhitelist(anotherAuthorized); - await this.crowdsale.buyTokens(authorized, { value: value, from: authorized }); - await expectThrow(this.crowdsale.buyTokens(anotherAuthorized, { value: value, from: authorized })); - }); - }); - - describe('reporting whitelisted', function () { - it('should correctly report whitelisted addresses', async function () { - (await this.crowdsale.isWhitelisted(authorized)).should.equal(true); - (await this.crowdsale.isWhitelisted(anotherAuthorized)).should.equal(true); - (await this.crowdsale.isWhitelisted(unauthorized)).should.equal(false); - }); - }); - }); -}); diff --git a/test/drafts/SignatureBouncer.test.js b/test/drafts/SignatureBouncer.test.js new file mode 100644 index 00000000000..b320ca98c32 --- /dev/null +++ b/test/drafts/SignatureBouncer.test.js @@ -0,0 +1,212 @@ +const { assertRevert } = require('../helpers/assertRevert'); +const { getSignFor } = require('../helpers/sign'); +const { shouldBehaveLikePublicRole } = require('../access/roles/PublicRole.behavior'); + +const SignatureBouncerMock = artifacts.require('SignatureBouncerMock'); + +const BigNumber = web3.BigNumber; + +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); + +const UINT_VALUE = 23; +const BYTES_VALUE = web3.toHex('test'); +const INVALID_SIGNATURE = '0xabcd'; + +contract('SignatureBouncer', function ([_, signer, otherSigner, anyone, authorizedUser, ...otherAccounts]) { + beforeEach(async function () { + this.sigBouncer = await SignatureBouncerMock.new({ from: signer }); + this.signFor = getSignFor(this.sigBouncer, signer); + }); + + describe('signer role', function () { + beforeEach(async function () { + this.contract = this.sigBouncer; + await this.contract.addSigner(otherSigner, { from: signer }); + }); + + shouldBehaveLikePublicRole(signer, otherSigner, otherAccounts, 'signer'); + }); + + describe('modifiers', function () { + context('plain signature', function () { + it('allows valid signature for sender', async function () { + await this.sigBouncer.onlyWithValidSignature(this.signFor(authorizedUser), { from: authorizedUser }); + }); + + it('does not allow invalid signature for sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignature(INVALID_SIGNATURE, { from: authorizedUser }) + ); + }); + + it('does not allow valid signature for other sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignature(this.signFor(authorizedUser), { from: anyone }) + ); + }); + + it('does not allow valid signature for method for sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignature(this.signFor(authorizedUser, 'onlyWithValidSignature'), + { from: authorizedUser }) + ); + }); + }); + + context('method signature', function () { + it('allows valid signature with correct method for sender', async function () { + await this.sigBouncer.onlyWithValidSignatureAndMethod( + this.signFor(authorizedUser, 'onlyWithValidSignatureAndMethod'), { from: authorizedUser } + ); + }); + + it('does not allow invalid signature with correct method for sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignatureAndMethod(INVALID_SIGNATURE, { from: authorizedUser }) + ); + }); + + it('does not allow valid signature with correct method for other sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignatureAndMethod( + this.signFor(authorizedUser, 'onlyWithValidSignatureAndMethod'), { from: anyone } + ) + ); + }); + + it('does not allow valid method signature with incorrect method for sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignatureAndMethod(this.signFor(authorizedUser, 'theWrongMethod'), + { from: authorizedUser }) + ); + }); + + it('does not allow valid non-method signature method for sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignatureAndMethod(this.signFor(authorizedUser), { from: authorizedUser }) + ); + }); + }); + + context('method and data signature', function () { + it('allows valid signature with correct method and data for sender', async function () { + await this.sigBouncer.onlyWithValidSignatureAndData(UINT_VALUE, + this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), { from: authorizedUser } + ); + }); + + it('does not allow invalid signature with correct method and data for sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignatureAndData(UINT_VALUE, INVALID_SIGNATURE, { from: authorizedUser }) + ); + }); + + it('does not allow valid signature with correct method and incorrect data for sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignatureAndData(UINT_VALUE + 10, + this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), + { from: authorizedUser } + ) + ); + }); + + it('does not allow valid signature with correct method and data for other sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignatureAndData(UINT_VALUE, + this.signFor(authorizedUser, 'onlyWithValidSignatureAndData', [UINT_VALUE]), + { from: anyone } + ) + ); + }); + + it('does not allow valid non-method signature for sender', async function () { + await assertRevert( + this.sigBouncer.onlyWithValidSignatureAndData(UINT_VALUE, + this.signFor(authorizedUser), { from: authorizedUser } + ) + ); + }); + }); + }); + + context('signature validation', function () { + context('plain signature', function () { + it('validates valid signature for valid user', async function () { + (await this.sigBouncer.checkValidSignature(authorizedUser, this.signFor(authorizedUser))).should.equal(true); + }); + + it('does not validate invalid signature for valid user', async function () { + (await this.sigBouncer.checkValidSignature(authorizedUser, INVALID_SIGNATURE)).should.equal(false); + }); + + it('does not validate valid signature for anyone', async function () { + (await this.sigBouncer.checkValidSignature(anyone, this.signFor(authorizedUser))).should.equal(false); + }); + + it('does not validate valid signature for method for valid user', async function () { + (await this.sigBouncer.checkValidSignature(authorizedUser, this.signFor(authorizedUser, 'checkValidSignature')) + ).should.equal(false); + }); + }); + + context('method signature', function () { + it('validates valid signature with correct method for valid user', async function () { + (await this.sigBouncer.checkValidSignatureAndMethod(authorizedUser, + this.signFor(authorizedUser, 'checkValidSignatureAndMethod')) + ).should.equal(true); + }); + + it('does not validate invalid signature with correct method for valid user', async function () { + (await this.sigBouncer.checkValidSignatureAndMethod(authorizedUser, INVALID_SIGNATURE)).should.equal(false); + }); + + it('does not validate valid signature with correct method for anyone', async function () { + (await this.sigBouncer.checkValidSignatureAndMethod(anyone, + this.signFor(authorizedUser, 'checkValidSignatureAndMethod')) + ).should.equal(false); + }); + + it('does not validate valid non-method signature with correct method for valid user', async function () { + (await this.sigBouncer.checkValidSignatureAndMethod(authorizedUser, this.signFor(authorizedUser)) + ).should.equal(false); + }); + }); + + context('method and data signature', function () { + it('validates valid signature with correct method and data for valid user', async function () { + (await this.sigBouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, + this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) + ).should.equal(true); + }); + + it('does not validate invalid signature with correct method and data for valid user', async function () { + (await this.sigBouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, INVALID_SIGNATURE) + ).should.equal(false); + }); + + it('does not validate valid signature with correct method and incorrect data for valid user', + async function () { + (await this.sigBouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE + 10, + this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) + ).should.equal(false); + } + ); + + it('does not validate valid signature with correct method and data for anyone', async function () { + (await this.sigBouncer.checkValidSignatureAndData(anyone, BYTES_VALUE, UINT_VALUE, + this.signFor(authorizedUser, 'checkValidSignatureAndData', [authorizedUser, BYTES_VALUE, UINT_VALUE])) + ).should.equal(false); + }); + + it('does not validate valid non-method-data signature with correct method and data for valid user', + async function () { + (await this.sigBouncer.checkValidSignatureAndData(authorizedUser, BYTES_VALUE, UINT_VALUE, + this.signFor(authorizedUser, 'checkValidSignatureAndData')) + ).should.equal(false); + } + ); + }); + }); +}); diff --git a/test/examples/SampleCrowdsale.test.js b/test/examples/SampleCrowdsale.test.js index aad435689d9..e4714353536 100644 --- a/test/examples/SampleCrowdsale.test.js +++ b/test/examples/SampleCrowdsale.test.js @@ -16,7 +16,7 @@ const should = require('chai') const SampleCrowdsale = artifacts.require('SampleCrowdsale'); const SampleCrowdsaleToken = artifacts.require('SampleCrowdsaleToken'); -contract('SampleCrowdsale', function ([_, owner, wallet, investor]) { +contract('SampleCrowdsale', function ([_, deployer, owner, wallet, investor]) { const RATE = new BigNumber(10); const GOAL = ether(10); const CAP = ether(20); @@ -31,12 +31,14 @@ contract('SampleCrowdsale', function ([_, owner, wallet, investor]) { this.closingTime = this.openingTime + duration.weeks(1); this.afterClosingTime = this.closingTime + duration.seconds(1); - this.token = await SampleCrowdsaleToken.new({ from: owner }); + this.token = await SampleCrowdsaleToken.new({ from: deployer }); this.crowdsale = await SampleCrowdsale.new( this.openingTime, this.closingTime, RATE, wallet, CAP, this.token.address, GOAL, { from: owner } ); - await this.token.transferOwnership(this.crowdsale.address, { from: owner }); + + await this.token.addMinter(this.crowdsale.address, { from: deployer }); + await this.token.renounceMinter({ from: deployer }); }); it('should create crowdsale with correct parameters', async function () { diff --git a/test/helpers/sign.js b/test/helpers/sign.js index a05238ad4fa..9164a9cd1a2 100644 --- a/test/helpers/sign.js +++ b/test/helpers/sign.js @@ -39,7 +39,7 @@ const transformToFullName = function (json) { * @param methodName string * @param methodArgs any[] */ -const getBouncerSigner = (contract, signer) => (redeemer, methodName, methodArgs = []) => { +const getSignFor = (contract, signer) => (redeemer, methodName, methodArgs = []) => { const parts = [ contract.address, redeemer, @@ -70,5 +70,5 @@ const getBouncerSigner = (contract, signer) => (redeemer, methodName, methodArgs module.exports = { signMessage, toEthSignedMessageHash, - getBouncerSigner, + getSignFor, }; diff --git a/test/lifecycle/Pausable.test.js b/test/lifecycle/Pausable.test.js index da37b6e6a01..2ae4c90d7da 100644 --- a/test/lifecycle/Pausable.test.js +++ b/test/lifecycle/Pausable.test.js @@ -1,6 +1,8 @@ const { assertRevert } = require('../helpers/assertRevert'); const expectEvent = require('../helpers/expectEvent'); + const PausableMock = artifacts.require('PausableMock'); +const { shouldBehaveLikePublicRole } = require('../access/roles/PublicRole.behavior'); const BigNumber = web3.BigNumber; @@ -8,58 +10,104 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -contract('Pausable', function () { +contract('Pausable', function ([_, pauser, otherPauser, anyone, ...otherAccounts]) { beforeEach(async function () { - this.Pausable = await PausableMock.new(); + this.pausable = await PausableMock.new({ from: pauser }); }); - it('can perform normal process in non-pause', async function () { - (await this.Pausable.count()).should.be.bignumber.equal(0); + describe('pauser role', function () { + beforeEach(async function () { + this.contract = this.pausable; + await this.contract.addPauser(otherPauser, { from: pauser }); + }); - await this.Pausable.normalProcess(); - (await this.Pausable.count()).should.be.bignumber.equal(1); + shouldBehaveLikePublicRole(pauser, otherPauser, otherAccounts, 'pauser'); }); - it('can not perform normal process in pause', async function () { - await this.Pausable.pause(); - (await this.Pausable.count()).should.be.bignumber.equal(0); + context('when unapused', function () { + beforeEach(async function () { + (await this.pausable.paused()).should.equal(false); + }); - await assertRevert(this.Pausable.normalProcess()); - (await this.Pausable.count()).should.be.bignumber.equal(0); - }); + it('can perform normal process in non-pause', async function () { + (await this.pausable.count()).should.be.bignumber.equal(0); - it('can not take drastic measure in non-pause', async function () { - await assertRevert(this.Pausable.drasticMeasure()); - (await this.Pausable.drasticMeasureTaken()).should.equal(false); - }); + await this.pausable.normalProcess({ from: anyone }); + (await this.pausable.count()).should.be.bignumber.equal(1); + }); - it('can take a drastic measure in a pause', async function () { - await this.Pausable.pause(); - await this.Pausable.drasticMeasure(); - (await this.Pausable.drasticMeasureTaken()).should.equal(true); - }); + it('cannot take drastic measure in non-pause', async function () { + await assertRevert(this.pausable.drasticMeasure({ from: anyone })); + (await this.pausable.drasticMeasureTaken()).should.equal(false); + }); - it('should resume allowing normal process after pause is over', async function () { - await this.Pausable.pause(); - await this.Pausable.unpause(); - await this.Pausable.normalProcess(); - (await this.Pausable.count()).should.be.bignumber.equal(1); - }); + describe('pausing', function () { + it('is pausable by the pauser', async function () { + await this.pausable.pause({ from: pauser }); + (await this.pausable.paused()).should.equal(true); + }); - it('should prevent drastic measure after pause is over', async function () { - await this.Pausable.pause(); - await this.Pausable.unpause(); + it('reverts when pausing from non-pauser', async function () { + await assertRevert(this.pausable.pause({ from: anyone })); + }); - await assertRevert(this.Pausable.drasticMeasure()); + context('when paused', function () { + beforeEach(async function () { + ({ logs: this.logs } = await this.pausable.pause({ from: pauser })); + }); - (await this.Pausable.drasticMeasureTaken()).should.equal(false); - }); + it('emits a Paused event', function () { + expectEvent.inLogs(this.logs, 'Paused'); + }); + + it('cannot perform normal process in pause', async function () { + await assertRevert(this.pausable.normalProcess({ from: anyone })); + }); + + it('can take a drastic measure in a pause', async function () { + await this.pausable.drasticMeasure({ from: anyone }); + (await this.pausable.drasticMeasureTaken()).should.equal(true); + }); + + it('reverts when re-pausing', async function () { + await assertRevert(this.pausable.pause({ from: pauser })); + }); + + describe('unpausing', function () { + it('is unpausable by the pauser', async function () { + await this.pausable.unpause({ from: pauser }); + (await this.pausable.paused()).should.equal(false); + }); + + it('reverts when unpausing from non-pauser', async function () { + await assertRevert(this.pausable.unpause({ from: anyone })); + }); + + context('when unpaused', function () { + beforeEach(async function () { + ({ logs: this.logs } = await this.pausable.unpause({ from: pauser })); + }); + + it('emits an Unpaused event', function () { + expectEvent.inLogs(this.logs, 'Unpaused'); + }); + + it('should resume allowing normal process', async function () { + (await this.pausable.count()).should.be.bignumber.equal(0); + await this.pausable.normalProcess({ from: anyone }); + (await this.pausable.count()).should.be.bignumber.equal(1); + }); - it('should log Pause and Unpause events appropriately', async function () { - const setPauseLogs = (await this.Pausable.pause()).logs; - expectEvent.inLogs(setPauseLogs, 'Paused'); + it('should prevent drastic measure', async function () { + await assertRevert(this.pausable.drasticMeasure({ from: anyone })); + }); - const setUnPauseLogs = (await this.Pausable.unpause()).logs; - expectEvent.inLogs(setUnPauseLogs, 'Unpaused'); + it('reverts when re-unpausing', async function () { + await assertRevert(this.pausable.unpause({ from: pauser })); + }); + }); + }); + }); + }); }); }); diff --git a/test/ownership/Secondary.test.js b/test/ownership/Secondary.test.js new file mode 100644 index 00000000000..000d0af11ba --- /dev/null +++ b/test/ownership/Secondary.test.js @@ -0,0 +1,57 @@ +const { assertRevert } = require('../helpers/assertRevert'); + +const SecondaryMock = artifacts.require('SecondaryMock'); + +require('chai') + .should(); + +contract('Secondary', function ([_, primary, newPrimary, anyone]) { + const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; + + beforeEach(async function () { + this.secondary = await SecondaryMock.new({ from: primary }); + }); + + it('stores the primary\'s address', async function () { + (await this.secondary.primary()).should.equal(primary); + }); + + describe('onlyPrimary', function () { + it('allows the primary account to call onlyPrimary functions', async function () { + await this.secondary.onlyPrimaryMock({ from: primary }); + }); + + it('reverts when anyone calls onlyPrimary functions', async function () { + await assertRevert(this.secondary.onlyPrimaryMock({ from: anyone })); + }); + }); + + describe('transferPrimary', function () { + it('makes the recipient the new primary', async function () { + await this.secondary.transferPrimary(newPrimary, { from: primary }); + (await this.secondary.primary()).should.equal(newPrimary); + }); + + it('reverts when transfering to the null address', async function () { + await assertRevert(this.secondary.transferPrimary(ZERO_ADDRESS, { from: primary })); + }); + + it('reverts when called by anyone', async function () { + await assertRevert(this.secondary.transferPrimary(newPrimary, { from: anyone })); + }); + + context('with new primary', function () { + beforeEach(async function () { + await this.secondary.transferPrimary(newPrimary, { from: primary }); + }); + + it('allows the new primary account to call onlyPrimary functions', async function () { + await this.secondary.onlyPrimaryMock({ from: newPrimary }); + }); + + it('reverts when the old primary account calls onlyPrimary functions', async function () { + await assertRevert(this.secondary.onlyPrimaryMock({ from: primary })); + }); + }); + }); +}); diff --git a/test/ownership/Superuser.test.js b/test/ownership/Superuser.test.js deleted file mode 100644 index 5e4d2c32456..00000000000 --- a/test/ownership/Superuser.test.js +++ /dev/null @@ -1,69 +0,0 @@ -const { expectThrow } = require('../helpers/expectThrow'); -const expectEvent = require('../helpers/expectEvent'); - -const Superuser = artifacts.require('Superuser'); - -require('chai') - .should(); - -contract('Superuser', function ([_, firstOwner, newSuperuser, newOwner, anyone]) { - const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; - - beforeEach(async function () { - this.superuser = await Superuser.new({ from: firstOwner }); - }); - - context('in normal conditions', function () { - it('should set the owner as the default superuser', async function () { - (await this.superuser.isSuperuser(firstOwner)).should.equal(true); - }); - - it('should change superuser after transferring', async function () { - await this.superuser.transferSuperuser(newSuperuser, { from: firstOwner }); - - (await this.superuser.isSuperuser(firstOwner)).should.equal(false); - - (await this.superuser.isSuperuser(newSuperuser)).should.equal(true); - }); - - it('should prevent changing to a null superuser', async function () { - await expectThrow( - this.superuser.transferSuperuser(ZERO_ADDRESS, { from: firstOwner }) - ); - }); - - it('should change owner after the superuser transfers the ownership', async function () { - await this.superuser.transferSuperuser(newSuperuser, { from: firstOwner }); - - await expectEvent.inTransaction( - this.superuser.transferOwnership(newOwner, { from: newSuperuser }), - 'OwnershipTransferred' - ); - - (await this.superuser.owner()).should.equal(newOwner); - }); - - it('should change owner after the owner transfers the ownership', async function () { - await expectEvent.inTransaction( - this.superuser.transferOwnership(newOwner, { from: firstOwner }), - 'OwnershipTransferred' - ); - - (await this.superuser.owner()).should.equal(newOwner); - }); - }); - - context('in adversarial conditions', function () { - it('should prevent non-superusers from transfering the superuser role', async function () { - await expectThrow( - this.superuser.transferSuperuser(newOwner, { from: anyone }) - ); - }); - - it('should prevent users that are not superuser nor owner from setting a new owner', async function () { - await expectThrow( - this.superuser.transferOwnership(newOwner, { from: anyone }) - ); - }); - }); -}); diff --git a/test/ownership/rbac/RBAC.test.js b/test/ownership/rbac/RBAC.test.js deleted file mode 100644 index 32859993d1d..00000000000 --- a/test/ownership/rbac/RBAC.test.js +++ /dev/null @@ -1,73 +0,0 @@ -const { expectThrow } = require('../../helpers/expectThrow'); -const expectEvent = require('../../helpers/expectEvent'); - -const RBACMock = artifacts.require('RBACMock'); - -require('chai') - .should(); - -const ROLE_ADVISOR = 'advisor'; - -contract('RBAC', function ([_, admin, anyone, advisor, otherAdvisor, futureAdvisor]) { - let mock; - - beforeEach(async function () { - mock = await RBACMock.new([advisor, otherAdvisor], { from: admin }); - }); - - context('in normal conditions', function () { - it('allows admin to call #onlyAdminsCanDoThis', async function () { - await mock.onlyAdminsCanDoThis({ from: admin }); - }); - it('allows admin to call #onlyAdvisorsCanDoThis', async function () { - await mock.onlyAdvisorsCanDoThis({ from: admin }); - }); - it('allows advisors to call #onlyAdvisorsCanDoThis', async function () { - await mock.onlyAdvisorsCanDoThis({ from: advisor }); - }); - it('allows admin to call #eitherAdminOrAdvisorCanDoThis', async function () { - await mock.eitherAdminOrAdvisorCanDoThis({ from: admin }); - }); - it('allows advisors to call #eitherAdminOrAdvisorCanDoThis', async function () { - await mock.eitherAdminOrAdvisorCanDoThis({ from: advisor }); - }); - it('does not allow admins to call #nobodyCanDoThis', async function () { - await expectThrow(mock.nobodyCanDoThis({ from: admin })); - }); - it('does not allow advisors to call #nobodyCanDoThis', async function () { - await expectThrow(mock.nobodyCanDoThis({ from: advisor })); - }); - it('does not allow anyone to call #nobodyCanDoThis', async function () { - await expectThrow(mock.nobodyCanDoThis({ from: anyone })); - }); - it('allows an admin to remove an advisor\'s role', async function () { - await mock.removeAdvisor(advisor, { from: admin }); - }); - it('allows admins to #adminRemoveRole', async function () { - await mock.adminRemoveRole(advisor, ROLE_ADVISOR, { from: admin }); - }); - - it('announces a RoleAdded event on addRole', async function () { - await expectEvent.inTransaction( - mock.adminAddRole(futureAdvisor, ROLE_ADVISOR, { from: admin }), - 'RoleAdded' - ); - }); - - it('announces a RoleRemoved event on removeRole', async function () { - await expectEvent.inTransaction( - mock.adminRemoveRole(futureAdvisor, ROLE_ADVISOR, { from: admin }), - 'RoleRemoved' - ); - }); - }); - - context('in adversarial conditions', function () { - it('does not allow an advisor to remove another advisor', async function () { - await expectThrow(mock.removeAdvisor(otherAdvisor, { from: advisor })); - }); - it('does not allow "anyone" to remove an advisor', async function () { - await expectThrow(mock.removeAdvisor(advisor, { from: anyone })); - }); - }); -}); diff --git a/test/payment/Escrow.behavior.js b/test/payment/Escrow.behavior.js index 64e857fa401..761aecd890d 100644 --- a/test/payment/Escrow.behavior.js +++ b/test/payment/Escrow.behavior.js @@ -9,13 +9,13 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -function shouldBehaveLikeEscrow (owner, [payee1, payee2]) { +function shouldBehaveLikeEscrow (primary, [payee1, payee2]) { const amount = web3.toWei(42.0, 'ether'); describe('as an escrow', function () { describe('deposits', function () { it('can accept a single deposit', async function () { - await this.escrow.deposit(payee1, { from: owner, value: amount }); + await this.escrow.deposit(payee1, { from: primary, value: amount }); (await ethGetBalance(this.escrow.address)).should.be.bignumber.equal(amount); @@ -23,23 +23,23 @@ function shouldBehaveLikeEscrow (owner, [payee1, payee2]) { }); it('can accept an empty deposit', async function () { - await this.escrow.deposit(payee1, { from: owner, value: 0 }); + await this.escrow.deposit(payee1, { from: primary, value: 0 }); }); - it('only the owner can deposit', async function () { + it('only the primary account can deposit', async function () { await expectThrow(this.escrow.deposit(payee1, { from: payee2 }), EVMRevert); }); it('emits a deposited event', async function () { - const receipt = await this.escrow.deposit(payee1, { from: owner, value: amount }); + 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); }); it('can add multiple deposits on a single account', async function () { - await this.escrow.deposit(payee1, { from: owner, value: amount }); - await this.escrow.deposit(payee1, { from: owner, value: amount * 2 }); + await this.escrow.deposit(payee1, { from: primary, value: amount }); + await this.escrow.deposit(payee1, { from: primary, value: amount * 2 }); (await ethGetBalance(this.escrow.address)).should.be.bignumber.equal(amount * 3); @@ -47,8 +47,8 @@ function shouldBehaveLikeEscrow (owner, [payee1, payee2]) { }); it('can track deposits to multiple accounts', async function () { - await this.escrow.deposit(payee1, { from: owner, value: amount }); - await this.escrow.deposit(payee2, { from: owner, value: amount * 2 }); + await this.escrow.deposit(payee1, { from: primary, value: amount }); + await this.escrow.deposit(payee2, { from: primary, value: amount * 2 }); (await ethGetBalance(this.escrow.address)).should.be.bignumber.equal(amount * 3); @@ -62,8 +62,8 @@ function shouldBehaveLikeEscrow (owner, [payee1, payee2]) { it('can withdraw payments', async function () { const payeeInitialBalance = await ethGetBalance(payee1); - await this.escrow.deposit(payee1, { from: owner, value: amount }); - await this.escrow.withdraw(payee1, { from: owner }); + await this.escrow.deposit(payee1, { from: primary, value: amount }); + await this.escrow.withdraw(payee1, { from: primary }); (await ethGetBalance(this.escrow.address)).should.be.bignumber.equal(0); @@ -74,16 +74,16 @@ function shouldBehaveLikeEscrow (owner, [payee1, payee2]) { }); it('can do an empty withdrawal', async function () { - await this.escrow.withdraw(payee1, { from: owner }); + await this.escrow.withdraw(payee1, { from: primary }); }); - it('only the owner can withdraw', async function () { + it('only the primary account can withdraw', async function () { await expectThrow(this.escrow.withdraw(payee1, { from: payee1 }), EVMRevert); }); it('emits a withdrawn event', async function () { - await this.escrow.deposit(payee1, { from: owner, value: amount }); - const receipt = await this.escrow.withdraw(payee1, { from: owner }); + 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); diff --git a/test/payment/Escrow.test.js b/test/payment/Escrow.test.js index a67b26417d0..6af510d659f 100644 --- a/test/payment/Escrow.test.js +++ b/test/payment/Escrow.test.js @@ -2,10 +2,10 @@ const { shouldBehaveLikeEscrow } = require('./Escrow.behavior'); const Escrow = artifacts.require('Escrow'); -contract('Escrow', function ([_, owner, ...otherAccounts]) { +contract('Escrow', function ([_, primary, ...otherAccounts]) { beforeEach(async function () { - this.escrow = await Escrow.new({ from: owner }); + this.escrow = await Escrow.new({ from: primary }); }); - shouldBehaveLikeEscrow(owner, otherAccounts); + shouldBehaveLikeEscrow(primary, otherAccounts); }); diff --git a/test/payment/RefundEscrow.test.js b/test/payment/RefundEscrow.test.js index 7730d8242a0..e412816c405 100644 --- a/test/payment/RefundEscrow.test.js +++ b/test/payment/RefundEscrow.test.js @@ -11,20 +11,20 @@ require('chai') const RefundEscrow = artifacts.require('RefundEscrow'); -contract('RefundEscrow', function ([_, owner, beneficiary, refundee1, refundee2]) { +contract('RefundEscrow', function ([_, primary, beneficiary, refundee1, refundee2]) { const amount = web3.toWei(54.0, 'ether'); const refundees = [refundee1, refundee2]; const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; it('requires a non-null beneficiary', async function () { await expectThrow( - RefundEscrow.new(ZERO_ADDRESS, { from: owner }) + RefundEscrow.new(ZERO_ADDRESS, { from: primary }) ); }); context('once deployed', function () { beforeEach(async function () { - this.escrow = await RefundEscrow.new(beneficiary, { from: owner }); + this.escrow = await RefundEscrow.new(beneficiary, { from: primary }); }); context('active state', function () { @@ -34,39 +34,39 @@ contract('RefundEscrow', function ([_, owner, beneficiary, refundee1, refundee2] }); it('accepts deposits', async function () { - await this.escrow.deposit(refundee1, { from: owner, value: amount }); + await this.escrow.deposit(refundee1, { from: primary, value: amount }); (await this.escrow.depositsOf(refundee1)).should.be.bignumber.equal(amount); }); it('does not refund refundees', async function () { - await this.escrow.deposit(refundee1, { from: owner, value: amount }); + await this.escrow.deposit(refundee1, { from: primary, value: amount }); await expectThrow(this.escrow.withdraw(refundee1), EVMRevert); }); it('does not allow beneficiary withdrawal', async function () { - await this.escrow.deposit(refundee1, { from: owner, value: amount }); + await this.escrow.deposit(refundee1, { from: primary, value: amount }); await expectThrow(this.escrow.beneficiaryWithdraw(), EVMRevert); }); }); - it('only owner can enter closed state', async function () { + 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: owner }); + const receipt = await this.escrow.close({ from: primary }); expectEvent.inLogs(receipt.logs, 'Closed'); }); context('closed state', function () { beforeEach(async function () { - await Promise.all(refundees.map(refundee => this.escrow.deposit(refundee, { from: owner, value: amount }))); + await Promise.all(refundees.map(refundee => this.escrow.deposit(refundee, { from: primary, value: amount }))); - await this.escrow.close({ from: owner }); + await this.escrow.close({ from: primary }); }); it('rejects deposits', async function () { - await expectThrow(this.escrow.deposit(refundee1, { from: owner, value: amount }), EVMRevert); + await expectThrow(this.escrow.deposit(refundee1, { from: primary, value: amount }), EVMRevert); }); it('does not refund refundees', async function () { @@ -82,37 +82,37 @@ contract('RefundEscrow', function ([_, owner, beneficiary, refundee1, refundee2] }); it('prevents entering the refund state', async function () { - await expectThrow(this.escrow.enableRefunds({ from: owner }), EVMRevert); + await expectThrow(this.escrow.enableRefunds({ from: primary }), EVMRevert); }); it('prevents re-entering the closed state', async function () { - await expectThrow(this.escrow.close({ from: owner }), EVMRevert); + await expectThrow(this.escrow.close({ from: primary }), EVMRevert); }); }); - it('only owner can enter refund state', async function () { + 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: owner }); + const receipt = await this.escrow.enableRefunds({ from: primary }); expectEvent.inLogs(receipt.logs, 'RefundsEnabled'); }); context('refund state', function () { beforeEach(async function () { - await Promise.all(refundees.map(refundee => this.escrow.deposit(refundee, { from: owner, value: amount }))); + await Promise.all(refundees.map(refundee => this.escrow.deposit(refundee, { from: primary, value: amount }))); - await this.escrow.enableRefunds({ from: owner }); + await this.escrow.enableRefunds({ from: primary }); }); it('rejects deposits', async function () { - await expectThrow(this.escrow.deposit(refundee1, { from: owner, value: amount }), EVMRevert); + await expectThrow(this.escrow.deposit(refundee1, { from: primary, value: amount }), EVMRevert); }); it('refunds refundees', async function () { for (const refundee of [refundee1, refundee2]) { const refundeeInitialBalance = await ethGetBalance(refundee); - await this.escrow.withdraw(refundee, { from: owner }); + await this.escrow.withdraw(refundee, { from: primary }); const refundeeFinalBalance = await ethGetBalance(refundee); refundeeFinalBalance.sub(refundeeInitialBalance).should.be.bignumber.equal(amount); @@ -124,11 +124,11 @@ contract('RefundEscrow', function ([_, owner, beneficiary, refundee1, refundee2] }); it('prevents entering the closed state', async function () { - await expectThrow(this.escrow.close({ from: owner }), EVMRevert); + await expectThrow(this.escrow.close({ from: primary }), EVMRevert); }); it('prevents re-entering the refund state', async function () { - await expectThrow(this.escrow.enableRefunds({ from: owner }), EVMRevert); + await expectThrow(this.escrow.enableRefunds({ from: primary }), EVMRevert); }); }); }); diff --git a/test/token/ERC20/ERC20Capped.behavior.js b/test/token/ERC20/ERC20Capped.behavior.js index 004d512c042..3bec1930256 100644 --- a/test/token/ERC20/ERC20Capped.behavior.js +++ b/test/token/ERC20/ERC20Capped.behavior.js @@ -17,7 +17,7 @@ function shouldBehaveLikeERC20Capped (minter, [anyone], cap) { it('should mint when amount is less than cap', async function () { const { logs } = await this.token.mint(anyone, cap.sub(1), { from }); - expectEvent.inLogs(logs, 'Mint'); + expectEvent.inLogs(logs, 'Minted'); }); it('should fail to mint if the ammount exceeds the cap', async function () { diff --git a/test/token/ERC20/ERC20Capped.test.js b/test/token/ERC20/ERC20Capped.test.js index 820a37be872..7e0c35c13c6 100644 --- a/test/token/ERC20/ERC20Capped.test.js +++ b/test/token/ERC20/ERC20Capped.test.js @@ -5,21 +5,21 @@ const { shouldBehaveLikeERC20Capped } = require('./ERC20Capped.behavior'); const ERC20Capped = artifacts.require('ERC20Capped'); -contract('ERC20Capped', function ([_, owner, ...otherAccounts]) { +contract('ERC20Capped', function ([_, minter, ...otherAccounts]) { const cap = ether(1000); it('requires a non-zero cap', async function () { await assertRevert( - ERC20Capped.new(0, { from: owner }) + ERC20Capped.new(0, { from: minter }) ); }); context('once deployed', async function () { beforeEach(async function () { - this.token = await ERC20Capped.new(cap, { from: owner }); + this.token = await ERC20Capped.new(cap, { from: minter }); }); - shouldBehaveLikeERC20Capped(owner, otherAccounts, cap); - shouldBehaveLikeERC20Mintable(owner, owner, otherAccounts); + shouldBehaveLikeERC20Capped(minter, otherAccounts, cap); + shouldBehaveLikeERC20Mintable(minter, otherAccounts); }); }); diff --git a/test/token/ERC20/ERC20Mintable.behavior.js b/test/token/ERC20/ERC20Mintable.behavior.js index 63fb861b551..debca80975a 100644 --- a/test/token/ERC20/ERC20Mintable.behavior.js +++ b/test/token/ERC20/ERC20Mintable.behavior.js @@ -7,26 +7,20 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) { +function shouldBehaveLikeERC20Mintable (minter, [anyone]) { const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; - describe('as a basic mintable token', function () { - describe('after token creation', function () { - it('sender should be token owner', async function () { - (await this.token.owner({ from: owner })).should.equal(owner); - }); - }); - - describe('minting finished', function () { - describe('when the token minting is not finished', function () { + 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); }); }); - describe('when the token is minting finished', function () { + context('when token minting is finished', function () { beforeEach(async function () { - await this.token.finishMinting({ from: owner }); + await this.token.finishMinting({ from: minter }); }); it('returns true', async function () { @@ -35,11 +29,11 @@ function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) { }); }); - describe('finish minting', function () { - describe('when the sender is the token owner', function () { - const from = owner; + describe('finishMinting', function () { + context('when the sender has minting permission', function () { + const from = minter; - describe('when the token minting was not finished', function () { + context('when token minting was not finished', function () { it('finishes token minting', async function () { await this.token.finishMinting({ from }); @@ -49,12 +43,11 @@ function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) { it('emits a mint finished event', async function () { const { logs } = await this.token.finishMinting({ from }); - logs.length.should.be.equal(1); - logs[0].event.should.equal('MintFinished'); + await expectEvent.inLogs(logs, 'MintingFinished'); }); }); - describe('when the token minting was already finished', function () { + context('when token minting was already finished', function () { beforeEach(async function () { await this.token.finishMinting({ from }); }); @@ -65,18 +58,18 @@ function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) { }); }); - describe('when the sender is not the token owner', function () { + context('when the sender doesn\'t have minting permission', function () { const from = anyone; - describe('when the token minting was not finished', function () { + context('when token minting was not finished', function () { it('reverts', async function () { await assertRevert(this.token.finishMinting({ from })); }); }); - describe('when the token minting was already finished', function () { + context('when token minting was already finished', function () { beforeEach(async function () { - await this.token.finishMinting({ from: owner }); + await this.token.finishMinting({ from: minter }); }); it('reverts', async function () { @@ -89,10 +82,10 @@ function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) { describe('mint', function () { const amount = 100; - describe('when the sender has the minting permission', function () { + context('when the sender has minting permission', function () { const from = minter; - describe('when the token minting is not finished', function () { + context('when token minting is not finished', function () { context('for a zero amount', function () { shouldMint(0); }); @@ -111,7 +104,7 @@ function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) { }); it('emits a mint and a transfer event', async function () { - const mintEvent = expectEvent.inLogs(this.logs, 'Mint', { + const mintEvent = expectEvent.inLogs(this.logs, 'Minted', { to: anyone, }); mintEvent.args.amount.should.be.bignumber.equal(amount); @@ -125,9 +118,9 @@ function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) { } }); - describe('when the token minting is finished', function () { + context('when token minting is finished', function () { beforeEach(async function () { - await this.token.finishMinting({ from: owner }); + await this.token.finishMinting({ from: minter }); }); it('reverts', async function () { @@ -136,18 +129,18 @@ function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) { }); }); - describe('when the sender has not the minting permission', function () { + context('when the sender doesn\'t have minting permission', function () { const from = anyone; - describe('when the token minting is not finished', function () { + context('when token minting is not finished', function () { it('reverts', async function () { await assertRevert(this.token.mint(anyone, amount, { from })); }); }); - describe('when the token minting is already finished', function () { + context('when token minting is already finished', function () { beforeEach(async function () { - await this.token.finishMinting({ from: owner }); + await this.token.finishMinting({ from: minter }); }); it('reverts', async function () { diff --git a/test/token/ERC20/ERC20Mintable.test.js b/test/token/ERC20/ERC20Mintable.test.js index 20727dafd9a..9dca0e55890 100644 --- a/test/token/ERC20/ERC20Mintable.test.js +++ b/test/token/ERC20/ERC20Mintable.test.js @@ -1,10 +1,20 @@ const { shouldBehaveLikeERC20Mintable } = require('./ERC20Mintable.behavior'); -const ERC20Mintable = artifacts.require('ERC20Mintable'); +const ERC20MintableMock = artifacts.require('ERC20MintableMock'); +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); -contract('ERC20Mintable', function ([_, owner, ...otherAccounts]) { +contract('ERC20Mintable', function ([_, minter, otherMinter, ...otherAccounts]) { beforeEach(async function () { - this.token = await ERC20Mintable.new({ from: owner }); + this.token = await ERC20MintableMock.new({ from: minter }); }); - shouldBehaveLikeERC20Mintable(owner, owner, otherAccounts); + describe('minter role', function () { + beforeEach(async function () { + this.contract = this.token; + await this.contract.addMinter(otherMinter, { from: minter }); + }); + + shouldBehaveLikePublicRole(minter, otherMinter, otherAccounts, 'minter'); + }); + + shouldBehaveLikeERC20Mintable(minter, otherAccounts); }); diff --git a/test/token/ERC20/ERC20Pausable.test.js b/test/token/ERC20/ERC20Pausable.test.js index 8fbe4f70876..59eb5ce3877 100644 --- a/test/token/ERC20/ERC20Pausable.test.js +++ b/test/token/ERC20/ERC20Pausable.test.js @@ -1,14 +1,25 @@ const { assertRevert } = require('../../helpers/assertRevert'); + const ERC20Pausable = artifacts.require('ERC20PausableMock'); +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); -contract('ERC20Pausable', function ([_, owner, recipient, anotherAccount]) { +contract('ERC20Pausable', function ([_, pauser, otherPauser, recipient, anotherAccount, ...otherAccounts]) { beforeEach(async function () { - this.token = await ERC20Pausable.new(owner, 100, { from: owner }); + this.token = await ERC20Pausable.new(pauser, 100, { from: pauser }); + }); + + describe('pauser role', function () { + beforeEach(async function () { + this.contract = this.token; + await this.contract.addPauser(otherPauser, { from: pauser }); + }); + + shouldBehaveLikePublicRole(pauser, otherPauser, otherAccounts, 'pauser'); }); describe('pause', function () { - describe('when the sender is the token owner', function () { - const from = owner; + describe('when the sender is the token pauser', function () { + const from = pauser; describe('when the token is unpaused', function () { it('pauses the token', async function () { @@ -35,7 +46,7 @@ contract('ERC20Pausable', function ([_, owner, recipient, anotherAccount]) { }); }); - describe('when the sender is not the token owner', function () { + describe('when the sender is not the token pauser', function () { const from = anotherAccount; it('reverts', async function () { @@ -45,8 +56,8 @@ contract('ERC20Pausable', function ([_, owner, recipient, anotherAccount]) { }); describe('unpause', function () { - describe('when the sender is the token owner', function () { - const from = owner; + describe('when the sender is the token pauser', function () { + const from = pauser; describe('when the token is paused', function () { beforeEach(async function () { @@ -73,7 +84,7 @@ contract('ERC20Pausable', function ([_, owner, recipient, anotherAccount]) { }); }); - describe('when the sender is not the token owner', function () { + describe('when the sender is not the token pauser', function () { const from = anotherAccount; it('reverts', async function () { @@ -83,7 +94,7 @@ contract('ERC20Pausable', function ([_, owner, recipient, anotherAccount]) { }); describe('pausable token', function () { - const from = owner; + const from = pauser; describe('paused', function () { it('is not paused by default', async function () { @@ -104,132 +115,132 @@ contract('ERC20Pausable', function ([_, owner, recipient, anotherAccount]) { describe('transfer', function () { it('allows to transfer when unpaused', async function () { - await this.token.transfer(recipient, 100, { from: owner }); + await this.token.transfer(recipient, 100, { from: pauser }); - (await this.token.balanceOf(owner)).should.be.bignumber.equal(0); + (await this.token.balanceOf(pauser)).should.be.bignumber.equal(0); (await this.token.balanceOf(recipient)).should.be.bignumber.equal(100); }); it('allows to transfer when paused and then unpaused', async function () { - await this.token.pause({ from: owner }); - await this.token.unpause({ from: owner }); + await this.token.pause({ from: pauser }); + await this.token.unpause({ from: pauser }); - await this.token.transfer(recipient, 100, { from: owner }); + await this.token.transfer(recipient, 100, { from: pauser }); - (await this.token.balanceOf(owner)).should.be.bignumber.equal(0); + (await this.token.balanceOf(pauser)).should.be.bignumber.equal(0); (await this.token.balanceOf(recipient)).should.be.bignumber.equal(100); }); it('reverts when trying to transfer when paused', async function () { - await this.token.pause({ from: owner }); + await this.token.pause({ from: pauser }); - await assertRevert(this.token.transfer(recipient, 100, { from: owner })); + await assertRevert(this.token.transfer(recipient, 100, { from: pauser })); }); }); describe('approve', function () { it('allows to approve when unpaused', async function () { - await this.token.approve(anotherAccount, 40, { from: owner }); + await this.token.approve(anotherAccount, 40, { from: pauser }); - (await this.token.allowance(owner, anotherAccount)).should.be.bignumber.equal(40); + (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(40); }); it('allows to transfer when paused and then unpaused', async function () { - await this.token.pause({ from: owner }); - await this.token.unpause({ from: owner }); + await this.token.pause({ from: pauser }); + await this.token.unpause({ from: pauser }); - await this.token.approve(anotherAccount, 40, { from: owner }); + await this.token.approve(anotherAccount, 40, { from: pauser }); - (await this.token.allowance(owner, anotherAccount)).should.be.bignumber.equal(40); + (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(40); }); it('reverts when trying to transfer when paused', async function () { - await this.token.pause({ from: owner }); + await this.token.pause({ from: pauser }); - await assertRevert(this.token.approve(anotherAccount, 40, { from: owner })); + await assertRevert(this.token.approve(anotherAccount, 40, { from: pauser })); }); }); describe('transfer from', function () { beforeEach(async function () { - await this.token.approve(anotherAccount, 50, { from: owner }); + await this.token.approve(anotherAccount, 50, { from: pauser }); }); it('allows to transfer from when unpaused', async function () { - await this.token.transferFrom(owner, recipient, 40, { from: anotherAccount }); + await this.token.transferFrom(pauser, recipient, 40, { from: anotherAccount }); - (await this.token.balanceOf(owner)).should.be.bignumber.equal(60); + (await this.token.balanceOf(pauser)).should.be.bignumber.equal(60); (await this.token.balanceOf(recipient)).should.be.bignumber.equal(40); }); it('allows to transfer when paused and then unpaused', async function () { - await this.token.pause({ from: owner }); - await this.token.unpause({ from: owner }); + await this.token.pause({ from: pauser }); + await this.token.unpause({ from: pauser }); - await this.token.transferFrom(owner, recipient, 40, { from: anotherAccount }); + await this.token.transferFrom(pauser, recipient, 40, { from: anotherAccount }); - (await this.token.balanceOf(owner)).should.be.bignumber.equal(60); + (await this.token.balanceOf(pauser)).should.be.bignumber.equal(60); (await this.token.balanceOf(recipient)).should.be.bignumber.equal(40); }); it('reverts when trying to transfer from when paused', async function () { - await this.token.pause({ from: owner }); + await this.token.pause({ from: pauser }); - await assertRevert(this.token.transferFrom(owner, recipient, 40, { from: anotherAccount })); + await assertRevert(this.token.transferFrom(pauser, recipient, 40, { from: anotherAccount })); }); }); describe('decrease approval', function () { beforeEach(async function () { - await this.token.approve(anotherAccount, 100, { from: owner }); + await this.token.approve(anotherAccount, 100, { from: pauser }); }); it('allows to decrease approval when unpaused', async function () { - await this.token.decreaseApproval(anotherAccount, 40, { from: owner }); + await this.token.decreaseApproval(anotherAccount, 40, { from: pauser }); - (await this.token.allowance(owner, anotherAccount)).should.be.bignumber.equal(60); + (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(60); }); it('allows to decrease approval when paused and then unpaused', async function () { - await this.token.pause({ from: owner }); - await this.token.unpause({ from: owner }); + await this.token.pause({ from: pauser }); + await this.token.unpause({ from: pauser }); - await this.token.decreaseApproval(anotherAccount, 40, { from: owner }); + await this.token.decreaseApproval(anotherAccount, 40, { from: pauser }); - (await this.token.allowance(owner, anotherAccount)).should.be.bignumber.equal(60); + (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(60); }); it('reverts when trying to transfer when paused', async function () { - await this.token.pause({ from: owner }); + await this.token.pause({ from: pauser }); - await assertRevert(this.token.decreaseApproval(anotherAccount, 40, { from: owner })); + await assertRevert(this.token.decreaseApproval(anotherAccount, 40, { from: pauser })); }); }); describe('increase approval', function () { beforeEach(async function () { - await this.token.approve(anotherAccount, 100, { from: owner }); + await this.token.approve(anotherAccount, 100, { from: pauser }); }); it('allows to increase approval when unpaused', async function () { - await this.token.increaseApproval(anotherAccount, 40, { from: owner }); + await this.token.increaseApproval(anotherAccount, 40, { from: pauser }); - (await this.token.allowance(owner, anotherAccount)).should.be.bignumber.equal(140); + (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(140); }); it('allows to increase approval when paused and then unpaused', async function () { - await this.token.pause({ from: owner }); - await this.token.unpause({ from: owner }); + await this.token.pause({ from: pauser }); + await this.token.unpause({ from: pauser }); - await this.token.increaseApproval(anotherAccount, 40, { from: owner }); + await this.token.increaseApproval(anotherAccount, 40, { from: pauser }); - (await this.token.allowance(owner, anotherAccount)).should.be.bignumber.equal(140); + (await this.token.allowance(pauser, anotherAccount)).should.be.bignumber.equal(140); }); it('reverts when trying to increase approval when paused', async function () { - await this.token.pause({ from: owner }); + await this.token.pause({ from: pauser }); - await assertRevert(this.token.increaseApproval(anotherAccount, 40, { from: owner })); + await assertRevert(this.token.increaseApproval(anotherAccount, 40, { from: pauser })); }); }); }); diff --git a/test/token/ERC20/RBACCappedToken.test.js b/test/token/ERC20/RBACCappedToken.test.js deleted file mode 100644 index 988a415d450..00000000000 --- a/test/token/ERC20/RBACCappedToken.test.js +++ /dev/null @@ -1,19 +0,0 @@ -const { ether } = require('../../helpers/ether'); -const { shouldBehaveLikeRBACMintableToken } = require('./RBACMintableToken.behavior'); -const { shouldBehaveLikeERC20Mintable } = require('./ERC20Mintable.behavior'); -const { shouldBehaveLikeERC20Capped } = require('./ERC20Capped.behavior'); - -const RBACCappedTokenMock = artifacts.require('RBACCappedTokenMock'); - -contract('RBACCappedToken', function ([_, owner, minter, ...otherAccounts]) { - const cap = ether(1000); - - beforeEach(async function () { - this.token = await RBACCappedTokenMock.new(cap, { from: owner }); - await this.token.addMinter(minter, { from: owner }); - }); - - shouldBehaveLikeERC20Mintable(owner, minter, otherAccounts); - shouldBehaveLikeRBACMintableToken(owner, otherAccounts); - shouldBehaveLikeERC20Capped(minter, otherAccounts, cap); -}); diff --git a/test/token/ERC20/RBACMintableToken.behavior.js b/test/token/ERC20/RBACMintableToken.behavior.js deleted file mode 100644 index 8906c58dbe1..00000000000 --- a/test/token/ERC20/RBACMintableToken.behavior.js +++ /dev/null @@ -1,28 +0,0 @@ -const { expectThrow } = require('../../helpers/expectThrow'); - -function shouldBehaveLikeRBACMintableToken (owner, [anyone]) { - describe('handle roles', function () { - it('owner can add and remove a minter role', async function () { - await this.token.addMinter(anyone, { from: owner }); - (await this.token.isMinter(anyone)).should.equal(true); - - await this.token.removeMinter(anyone, { from: owner }); - (await this.token.isMinter(anyone)).should.equal(false); - }); - - it('anyone can\'t add or remove a minter role', async function () { - await expectThrow( - this.token.addMinter(anyone, { from: anyone }) - ); - - await this.token.addMinter(anyone, { from: owner }); - await expectThrow( - this.token.removeMinter(anyone, { from: anyone }) - ); - }); - }); -} - -module.exports = { - shouldBehaveLikeRBACMintableToken, -}; diff --git a/test/token/ERC20/RBACMintableToken.test.js b/test/token/ERC20/RBACMintableToken.test.js deleted file mode 100644 index 2caf4bface8..00000000000 --- a/test/token/ERC20/RBACMintableToken.test.js +++ /dev/null @@ -1,14 +0,0 @@ -const { shouldBehaveLikeRBACMintableToken } = require('./RBACMintableToken.behavior'); -const { shouldBehaveLikeERC20Mintable } = require('./ERC20Mintable.behavior'); - -const RBACMintableToken = artifacts.require('RBACMintableToken'); - -contract('RBACMintableToken', function ([_, owner, minter, ...otherAccounts]) { - beforeEach(async function () { - this.token = await RBACMintableToken.new({ from: owner }); - await this.token.addMinter(minter, { from: owner }); - }); - - shouldBehaveLikeRBACMintableToken(owner, otherAccounts); - shouldBehaveLikeERC20Mintable(owner, minter, otherAccounts); -}); diff --git a/test/token/ERC20/TokenTimelock.test.js b/test/token/ERC20/TokenTimelock.test.js index 70eaae8f54d..c10b1b82137 100644 --- a/test/token/ERC20/TokenTimelock.test.js +++ b/test/token/ERC20/TokenTimelock.test.js @@ -11,12 +11,12 @@ require('chai') const ERC20Mintable = artifacts.require('ERC20Mintable'); const TokenTimelock = artifacts.require('TokenTimelock'); -contract('TokenTimelock', function ([_, owner, beneficiary]) { +contract('TokenTimelock', function ([_, minter, beneficiary]) { const amount = new BigNumber(100); context('with token', function () { beforeEach(async function () { - this.token = await ERC20Mintable.new({ from: owner }); + this.token = await ERC20Mintable.new({ from: minter }); }); it('rejects a release time in the past', async function () { @@ -30,7 +30,7 @@ contract('TokenTimelock', function ([_, owner, beneficiary]) { beforeEach(async function () { this.releaseTime = (await latestTime()) + duration.years(1); this.timelock = await TokenTimelock.new(this.token.address, beneficiary, this.releaseTime); - await this.token.mint(this.timelock.address, amount, { from: owner }); + await this.token.mint(this.timelock.address, amount, { from: minter }); }); it('can get state', async function () { diff --git a/test/token/ERC721/ERC721.test.js b/test/token/ERC721/ERC721.test.js index 5469c87cf28..081d8e86f9e 100644 --- a/test/token/ERC721/ERC721.test.js +++ b/test/token/ERC721/ERC721.test.js @@ -11,55 +11,57 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -contract('ERC721', function (accounts) { +contract('ERC721', function ([ + creator, + ...accounts +]) { const name = 'Non Fungible Token'; const symbol = 'NFT'; const firstTokenId = 100; const secondTokenId = 200; + const thirdTokenId = 300; const nonExistentTokenId = 999; - const creator = accounts[0]; - const anyone = accounts[9]; + + const minter = creator; + + const [ + owner, + newOwner, + another, + anyone, + ] = accounts; beforeEach(async function () { this.token = await ERC721.new(name, symbol, { from: creator }); }); - shouldBehaveLikeERC721Basic(accounts); - shouldBehaveLikeMintAndBurnERC721(accounts); - describe('like a full ERC721', function () { beforeEach(async function () { - await this.token.mint(creator, firstTokenId, { from: creator }); - await this.token.mint(creator, secondTokenId, { from: creator }); + await this.token.mint(owner, firstTokenId, { from: minter }); + await this.token.mint(owner, secondTokenId, { from: minter }); }); describe('mint', function () { - const to = accounts[1]; - const tokenId = 3; - beforeEach(async function () { - await this.token.mint(to, tokenId); + await this.token.mint(newOwner, thirdTokenId, { from: minter }); }); it('adjusts owner tokens by index', async function () { - (await this.token.tokenOfOwnerByIndex(to, 0)).toNumber().should.be.equal(tokenId); + (await this.token.tokenOfOwnerByIndex(newOwner, 0)).toNumber().should.be.equal(thirdTokenId); }); it('adjusts all tokens list', async function () { - (await this.token.tokenByIndex(2)).toNumber().should.be.equal(tokenId); + (await this.token.tokenByIndex(2)).toNumber().should.be.equal(thirdTokenId); }); }); describe('burn', function () { - const tokenId = firstTokenId; - const sender = creator; - beforeEach(async function () { - await this.token.burn(tokenId, { from: sender }); + await this.token.burn(firstTokenId, { from: owner }); }); it('removes that token from the token list of the owner', async function () { - (await this.token.tokenOfOwnerByIndex(sender, 0)).toNumber().should.be.equal(secondTokenId); + (await this.token.tokenOfOwnerByIndex(owner, 0)).toNumber().should.be.equal(secondTokenId); }); it('adjusts all tokens list', async function () { @@ -67,7 +69,7 @@ contract('ERC721', function (accounts) { }); it('burns all tokens', async function () { - await this.token.burn(secondTokenId, { from: sender }); + await this.token.burn(secondTokenId, { from: owner }); (await this.token.totalSupply()).toNumber().should.be.equal(0); await assertRevert(this.token.tokenByIndex(0)); }); @@ -76,25 +78,25 @@ contract('ERC721', function (accounts) { describe('removeTokenFrom', function () { it('reverts if the correct owner is not passed', async function () { await assertRevert( - this.token.removeTokenFrom(anyone, firstTokenId, { from: creator }) + this.token.removeTokenFrom(anyone, firstTokenId, { from: owner }) ); }); context('once removed', function () { beforeEach(async function () { - await this.token.removeTokenFrom(creator, firstTokenId, { from: creator }); + await this.token.removeTokenFrom(owner, firstTokenId, { from: owner }); }); it('has been removed', async function () { - await assertRevert(this.token.tokenOfOwnerByIndex(creator, 1)); + await assertRevert(this.token.tokenOfOwnerByIndex(owner, 1)); }); it('adjusts token list', async function () { - (await this.token.tokenOfOwnerByIndex(creator, 0)).toNumber().should.be.equal(secondTokenId); + (await this.token.tokenOfOwnerByIndex(owner, 0)).toNumber().should.be.equal(secondTokenId); }); it('adjusts owner count', async function () { - (await this.token.balanceOf(creator)).toNumber().should.be.equal(1); + (await this.token.balanceOf(owner)).toNumber().should.be.equal(1); }); it('does not adjust supply', async function () { @@ -125,7 +127,7 @@ contract('ERC721', function (accounts) { it('can burn token with metadata', async function () { await this.token.setTokenURI(firstTokenId, sampleUri); - await this.token.burn(firstTokenId); + await this.token.burn(firstTokenId, { from: owner }); (await this.token.exists(firstTokenId)).should.equal(false); }); @@ -145,9 +147,6 @@ contract('ERC721', function (accounts) { }); describe('tokenOfOwnerByIndex', function () { - const owner = creator; - const another = accounts[1]; - describe('when the given index is lower than the amount of tokens owned by the given address', function () { it('returns the token ID placed at the given index', async function () { (await this.token.tokenOfOwnerByIndex(owner, 0)).should.be.bignumber.equal(firstTokenId); @@ -197,13 +196,12 @@ contract('ERC721', function (accounts) { [firstTokenId, secondTokenId].forEach(function (tokenId) { it(`should return all tokens after burning token ${tokenId} and minting new tokens`, async function () { - const owner = accounts[0]; const newTokenId = 300; const anotherNewTokenId = 400; await this.token.burn(tokenId, { from: owner }); - await this.token.mint(owner, newTokenId, { from: owner }); - await this.token.mint(owner, anotherNewTokenId, { from: owner }); + await this.token.mint(newOwner, newTokenId, { from: minter }); + await this.token.mint(newOwner, anotherNewTokenId, { from: minter }); (await this.token.totalSupply()).toNumber().should.be.equal(3); @@ -218,6 +216,9 @@ contract('ERC721', function (accounts) { }); }); + shouldBehaveLikeERC721Basic(creator, minter, accounts); + shouldBehaveLikeMintAndBurnERC721(creator, minter, accounts); + shouldSupportInterfaces([ 'ERC165', 'ERC721', diff --git a/test/token/ERC721/ERC721Basic.behavior.js b/test/token/ERC721/ERC721Basic.behavior.js index 765614afce7..b3be95ed020 100644 --- a/test/token/ERC721/ERC721Basic.behavior.js +++ b/test/token/ERC721/ERC721Basic.behavior.js @@ -11,30 +11,34 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -function shouldBehaveLikeERC721Basic (accounts) { +function shouldBehaveLikeERC721Basic ( + creator, + minter, + [owner, approved, anotherApproved, operator, anyone] +) { const firstTokenId = 1; const secondTokenId = 2; const unknownTokenId = 3; - const creator = accounts[0]; const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; const RECEIVER_MAGIC_VALUE = '0x150b7a02'; describe('like an ERC721Basic', function () { beforeEach(async function () { - await this.token.mint(creator, firstTokenId, { from: creator }); - await this.token.mint(creator, secondTokenId, { from: creator }); + await this.token.mint(owner, firstTokenId, { from: minter }); + await this.token.mint(owner, secondTokenId, { from: minter }); + this.toWhom = anyone; // default to anyone for toWhom in context-dependent tests }); describe('balanceOf', function () { context('when the given address owns some tokens', function () { it('returns the amount of tokens owned by the given address', async function () { - (await this.token.balanceOf(creator)).should.be.bignumber.equal(2); + (await this.token.balanceOf(owner)).should.be.bignumber.equal(2); }); }); context('when the given address does not own any tokens', function () { it('returns 0', async function () { - (await this.token.balanceOf(accounts[1])).should.be.bignumber.equal(0); + (await this.token.balanceOf(anyone)).should.be.bignumber.equal(0); }); }); @@ -50,7 +54,7 @@ function shouldBehaveLikeERC721Basic (accounts) { const tokenId = firstTokenId; it('returns the owner of the given token ID', async function () { - (await this.token.ownerOf(tokenId)).should.be.equal(creator); + (await this.token.ownerOf(tokenId)).should.be.equal(owner); }); }); @@ -64,24 +68,19 @@ function shouldBehaveLikeERC721Basic (accounts) { }); describe('transfers', function () { - const owner = accounts[0]; - const approved = accounts[2]; - const operator = accounts[3]; - const unauthorized = accounts[4]; const tokenId = firstTokenId; const data = '0x42'; let logs = null; beforeEach(async function () { - this.to = accounts[1]; await this.token.approve(approved, tokenId, { from: owner }); await this.token.setApprovalForAll(operator, true, { from: owner }); }); const transferWasSuccessful = function ({ owner, tokenId, approved }) { it('transfers the ownership of the given token ID to the given address', async function () { - (await this.token.ownerOf(tokenId)).should.be.equal(this.to); + (await this.token.ownerOf(tokenId)).should.be.equal(this.toWhom); }); it('clears the approval for the token ID', async function () { @@ -93,7 +92,7 @@ function shouldBehaveLikeERC721Basic (accounts) { 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.to); + logs[0].args.to.should.be.equal(this.toWhom); logs[0].args.tokenId.should.be.bignumber.equal(tokenId); }); } else { @@ -101,21 +100,19 @@ function shouldBehaveLikeERC721Basic (accounts) { 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.to); + logs[0].args.to.should.be.equal(this.toWhom); logs[0].args.tokenId.should.be.bignumber.equal(tokenId); }); } it('adjusts owners balances', async function () { - (await this.token.balanceOf(this.to)).should.be.bignumber.equal(1); - (await this.token.balanceOf(owner)).should.be.bignumber.equal(1); }); it('adjusts owners tokens by index', async function () { if (!this.token.tokenOfOwnerByIndex) return; - (await this.token.tokenOfOwnerByIndex(this.to, 0)).toNumber().should.be.equal(tokenId); + (await this.token.tokenOfOwnerByIndex(this.toWhom, 0)).toNumber().should.be.equal(tokenId); (await this.token.tokenOfOwnerByIndex(owner, 0)).toNumber().should.not.be.equal(tokenId); }); @@ -124,21 +121,21 @@ function shouldBehaveLikeERC721Basic (accounts) { const shouldTransferTokensByUsers = function (transferFunction) { context('when called by the owner', function () { beforeEach(async function () { - ({ logs } = await transferFunction.call(this, owner, this.to, tokenId, { from: owner })); + ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: owner })); }); transferWasSuccessful({ owner, tokenId, approved }); }); context('when called by the approved individual', function () { beforeEach(async function () { - ({ logs } = await transferFunction.call(this, owner, this.to, tokenId, { from: approved })); + ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: approved })); }); transferWasSuccessful({ owner, tokenId, approved }); }); context('when called by the operator', function () { beforeEach(async function () { - ({ logs } = await transferFunction.call(this, owner, this.to, tokenId, { from: operator })); + ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: operator })); }); transferWasSuccessful({ owner, tokenId, approved }); }); @@ -146,7 +143,7 @@ function shouldBehaveLikeERC721Basic (accounts) { context('when called by the owner without an approved user', function () { beforeEach(async function () { await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner }); - ({ logs } = await transferFunction.call(this, owner, this.to, tokenId, { from: operator })); + ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: operator })); }); transferWasSuccessful({ owner, tokenId, approved: null }); }); @@ -185,19 +182,22 @@ function shouldBehaveLikeERC721Basic (accounts) { context('when the address of the previous owner is incorrect', function () { it('reverts', async function () { - await assertRevert(transferFunction.call(this, unauthorized, this.to, tokenId, { from: owner })); + await assertRevert(transferFunction.call(this, anyone, anyone, tokenId, { from: owner }) + ); }); }); context('when the sender is not authorized for the token id', function () { it('reverts', async function () { - await assertRevert(transferFunction.call(this, owner, this.to, tokenId, { from: unauthorized })); + await assertRevert(transferFunction.call(this, owner, anyone, tokenId, { from: anyone }) + ); }); }); context('when the given token ID does not exist', function () { it('reverts', async function () { - await assertRevert(transferFunction.call(this, owner, this.to, unknownTokenId, { from: owner })); + await assertRevert(transferFunction.call(this, owner, anyone, unknownTokenId, { from: owner }) + ); }); }); @@ -237,13 +237,13 @@ function shouldBehaveLikeERC721Basic (accounts) { describe('to a valid receiver contract', function () { beforeEach(async function () { this.receiver = await ERC721Receiver.new(RECEIVER_MAGIC_VALUE, false); - this.to = this.receiver.address; + this.toWhom = this.receiver.address; }); shouldTransferTokensByUsers(transferFun); it('should call onERC721Received', async function () { - const result = await transferFun.call(this, owner, this.to, tokenId, { from: owner }); + const result = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: owner }); result.receipt.logs.length.should.be.equal(2); const [log] = decodeLogs([result.receipt.logs[1]], ERC721Receiver, this.receiver.address); log.event.should.be.equal('Received'); @@ -254,9 +254,15 @@ function shouldBehaveLikeERC721Basic (accounts) { }); it('should call onERC721Received from approved', async function () { - const result = await transferFun.call(this, owner, this.to, tokenId, { from: approved }); + const result = await transferFun.call(this, owner, this.receiver.address, tokenId, { + from: approved, + }); result.receipt.logs.length.should.be.equal(2); - const [log] = decodeLogs([result.receipt.logs[1]], ERC721Receiver, this.receiver.address); + const [log] = decodeLogs( + [result.receipt.logs[1]], + ERC721Receiver, + this.receiver.address + ); log.event.should.be.equal('Received'); log.args.operator.should.be.equal(approved); log.args.from.should.be.equal(owner); @@ -270,7 +276,7 @@ function shouldBehaveLikeERC721Basic (accounts) { transferFun.call( this, owner, - this.to, + this.receiver.address, unknownTokenId, { from: owner }, ) @@ -313,8 +319,6 @@ function shouldBehaveLikeERC721Basic (accounts) { describe('approve', function () { const tokenId = firstTokenId; - const sender = creator; - const to = accounts[1]; let logs = null; @@ -334,7 +338,7 @@ function shouldBehaveLikeERC721Basic (accounts) { 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(sender); + 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); }); @@ -343,7 +347,7 @@ function shouldBehaveLikeERC721Basic (accounts) { context('when clearing approval', function () { context('when there was no prior approval', function () { beforeEach(async function () { - ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: sender })); + ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner })); }); itClearsApproval(); @@ -352,8 +356,8 @@ function shouldBehaveLikeERC721Basic (accounts) { context('when there was a prior approval', function () { beforeEach(async function () { - await this.token.approve(to, tokenId, { from: sender }); - ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: sender })); + await this.token.approve(approved, tokenId, { from: owner }); + ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner })); }); itClearsApproval(); @@ -364,90 +368,87 @@ function shouldBehaveLikeERC721Basic (accounts) { context('when approving a non-zero address', function () { context('when there was no prior approval', function () { beforeEach(async function () { - ({ logs } = await this.token.approve(to, tokenId, { from: sender })); + ({ logs } = await this.token.approve(approved, tokenId, { from: owner })); }); - itApproves(to); - itEmitsApprovalEvent(to); + itApproves(approved); + itEmitsApprovalEvent(approved); }); context('when there was a prior approval to the same address', function () { beforeEach(async function () { - await this.token.approve(to, tokenId, { from: sender }); - ({ logs } = await this.token.approve(to, tokenId, { from: sender })); + await this.token.approve(approved, tokenId, { from: owner }); + ({ logs } = await this.token.approve(approved, tokenId, { from: owner })); }); - itApproves(to); - itEmitsApprovalEvent(to); + itApproves(approved); + itEmitsApprovalEvent(approved); }); context('when there was a prior approval to a different address', function () { beforeEach(async function () { - await this.token.approve(accounts[2], tokenId, { from: sender }); - ({ logs } = await this.token.approve(to, tokenId, { from: sender })); + await this.token.approve(anotherApproved, tokenId, { from: owner }); + ({ logs } = await this.token.approve(anotherApproved, tokenId, { from: owner })); }); - itApproves(to); - itEmitsApprovalEvent(to); + itApproves(anotherApproved); + itEmitsApprovalEvent(anotherApproved); }); }); context('when the address that receives the approval is the owner', function () { it('reverts', async function () { - await assertRevert(this.token.approve(sender, tokenId, { from: sender })); + await assertRevert( + this.token.approve(owner, tokenId, { from: owner }) + ); }); }); context('when the sender does not own the given token ID', function () { it('reverts', async function () { - await assertRevert(this.token.approve(to, tokenId, { from: accounts[2] })); + await assertRevert(this.token.approve(approved, tokenId, { from: anyone })); }); }); context('when the sender is approved for the given token ID', function () { it('reverts', async function () { - await this.token.approve(accounts[2], tokenId, { from: sender }); - await assertRevert(this.token.approve(to, tokenId, { from: accounts[2] })); + await this.token.approve(approved, tokenId, { from: owner }); + await assertRevert(this.token.approve(anotherApproved, tokenId, { from: approved })); }); }); context('when the sender is an operator', function () { - const operator = accounts[2]; beforeEach(async function () { - await this.token.setApprovalForAll(operator, true, { from: sender }); - ({ logs } = await this.token.approve(to, tokenId, { from: operator })); + await this.token.setApprovalForAll(operator, true, { from: owner }); + ({ logs } = await this.token.approve(approved, tokenId, { from: operator })); }); - itApproves(to); - itEmitsApprovalEvent(to); + itApproves(approved); + itEmitsApprovalEvent(approved); }); context('when the given token ID does not exist', function () { it('reverts', async function () { - await assertRevert(this.token.approve(to, unknownTokenId, { from: sender })); + await assertRevert(this.token.approve(approved, unknownTokenId, { from: operator })); }); }); }); describe('setApprovalForAll', function () { - const sender = creator; - context('when the operator willing to approve is not the owner', function () { - const operator = accounts[1]; - context('when there is no operator approval set by the sender', function () { it('approves the operator', async function () { - await this.token.setApprovalForAll(operator, true, { from: sender }); + await this.token.setApprovalForAll(operator, true, { from: owner }); - (await this.token.isApprovedForAll(sender, operator)).should.equal(true); + (await this.token.isApprovedForAll(owner, operator)).should.equal(true); }); it('emits an approval event', async function () { - const { logs } = await this.token.setApprovalForAll(operator, true, { from: sender }); + 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(sender); + logs[0].args.owner.should.be.equal(owner); logs[0].args.operator.should.be.equal(operator); logs[0].args.approved.should.equal(true); }); @@ -455,49 +456,49 @@ function shouldBehaveLikeERC721Basic (accounts) { context('when the operator was set as not approved', function () { beforeEach(async function () { - await this.token.setApprovalForAll(operator, false, { from: sender }); + await this.token.setApprovalForAll(operator, false, { from: owner }); }); it('approves the operator', async function () { - await this.token.setApprovalForAll(operator, true, { from: sender }); + await this.token.setApprovalForAll(operator, true, { from: owner }); - (await this.token.isApprovedForAll(sender, operator)).should.equal(true); + (await this.token.isApprovedForAll(owner, operator)).should.equal(true); }); it('emits an approval event', async function () { - const { logs } = await this.token.setApprovalForAll(operator, true, { from: sender }); + 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(sender); + logs[0].args.owner.should.be.equal(owner); logs[0].args.operator.should.be.equal(operator); logs[0].args.approved.should.equal(true); }); it('can unset the operator approval', async function () { - await this.token.setApprovalForAll(operator, false, { from: sender }); + await this.token.setApprovalForAll(operator, false, { from: owner }); - (await this.token.isApprovedForAll(sender, operator)).should.equal(false); + (await this.token.isApprovedForAll(owner, operator)).should.equal(false); }); }); context('when the operator was already approved', function () { beforeEach(async function () { - await this.token.setApprovalForAll(operator, true, { from: sender }); + await this.token.setApprovalForAll(operator, true, { from: owner }); }); it('keeps the approval to the given address', async function () { - await this.token.setApprovalForAll(operator, true, { from: sender }); + await this.token.setApprovalForAll(operator, true, { from: owner }); - (await this.token.isApprovedForAll(sender, operator)).should.equal(true); + (await this.token.isApprovedForAll(owner, operator)).should.equal(true); }); it('emits an approval event', async function () { - const { logs } = await this.token.setApprovalForAll(operator, true, { from: sender }); + 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(sender); + logs[0].args.owner.should.be.equal(owner); logs[0].args.operator.should.be.equal(operator); logs[0].args.approved.should.equal(true); }); @@ -505,10 +506,8 @@ function shouldBehaveLikeERC721Basic (accounts) { }); context('when the operator is the owner', function () { - const operator = creator; - it('reverts', async function () { - await assertRevert(this.token.setApprovalForAll(operator, true, { from: sender })); + await assertRevert(this.token.setApprovalForAll(owner, true, { from: owner })); }); }); }); diff --git a/test/token/ERC721/ERC721Basic.test.js b/test/token/ERC721/ERC721Basic.test.js index f86a97f5b68..708523ed228 100644 --- a/test/token/ERC721/ERC721Basic.test.js +++ b/test/token/ERC721/ERC721Basic.test.js @@ -1,5 +1,4 @@ const { shouldBehaveLikeERC721Basic } = require('./ERC721Basic.behavior'); -const { shouldBehaveLikeMintAndBurnERC721 } = require('./ERC721MintBurn.behavior'); const BigNumber = web3.BigNumber; const ERC721Basic = artifacts.require('ERC721BasicMock.sol'); @@ -8,11 +7,10 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -contract('ERC721Basic', function (accounts) { +contract('ERC721Basic', function ([_, creator, ...accounts]) { beforeEach(async function () { - this.token = await ERC721Basic.new({ from: accounts[0] }); + this.token = await ERC721Basic.new({ from: creator }); }); - shouldBehaveLikeERC721Basic(accounts); - shouldBehaveLikeMintAndBurnERC721(accounts); + shouldBehaveLikeERC721Basic(creator, creator, accounts); }); diff --git a/test/token/ERC721/ERC721Burnable.test.js b/test/token/ERC721/ERC721Burnable.test.js new file mode 100644 index 00000000000..34605028ddb --- /dev/null +++ b/test/token/ERC721/ERC721Burnable.test.js @@ -0,0 +1,22 @@ +const { shouldBehaveLikeERC721Basic } = require('./ERC721Basic.behavior'); +const { + shouldBehaveLikeMintAndBurnERC721, +} = require('./ERC721MintBurn.behavior'); + +const BigNumber = web3.BigNumber; +const ERC721Burnable = artifacts.require('ERC721MintableBurnableImpl.sol'); + +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('ERC721Burnable', function ([_, creator, ...accounts]) { + const minter = creator; + + beforeEach(async function () { + this.token = await ERC721Burnable.new({ from: creator }); + }); + + shouldBehaveLikeERC721Basic(creator, minter, accounts); + shouldBehaveLikeMintAndBurnERC721(creator, minter, accounts); +}); diff --git a/test/token/ERC721/ERC721MintBurn.behavior.js b/test/token/ERC721/ERC721MintBurn.behavior.js index ad7d207363d..a86e0c4a562 100644 --- a/test/token/ERC721/ERC721MintBurn.behavior.js +++ b/test/token/ERC721/ERC721MintBurn.behavior.js @@ -1,84 +1,100 @@ const { assertRevert } = require('../../helpers/assertRevert'); +const expectEvent = require('../../helpers/expectEvent'); const BigNumber = web3.BigNumber; require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -function shouldBehaveLikeMintAndBurnERC721 (accounts) { +function shouldBehaveLikeMintAndBurnERC721 ( + creator, + minter, + [owner, newOwner, approved, anyone] +) { const firstTokenId = 1; const secondTokenId = 2; - const unknownTokenId = 3; - const creator = accounts[0]; + const thirdTokenId = 3; + const unknownTokenId = 4; const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; + const MOCK_URI = 'https://example.com'; describe('like a mintable and burnable ERC721', function () { beforeEach(async function () { - await this.token.mint(creator, firstTokenId, { from: creator }); - await this.token.mint(creator, secondTokenId, { from: creator }); + await this.token.mint(owner, firstTokenId, { from: minter }); + await this.token.mint(owner, secondTokenId, { from: minter }); }); describe('mint', function () { - const to = accounts[1]; - const tokenId = unknownTokenId; let logs = null; describe('when successful', function () { beforeEach(async function () { - const result = await this.token.mint(to, tokenId); + const result = await this.token.mint(newOwner, thirdTokenId, { from: minter }); logs = result.logs; }); it('assigns the token to the new owner', async function () { - (await this.token.ownerOf(tokenId)).should.be.equal(to); + (await this.token.ownerOf(thirdTokenId)).should.be.equal(newOwner); }); it('increases the balance of its owner', async function () { - (await this.token.balanceOf(to)).should.be.bignumber.equal(1); + (await this.token.balanceOf(newOwner)).should.be.bignumber.equal(1); }); - it('emits 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(ZERO_ADDRESS); - logs[0].args.to.should.be.equal(to); - logs[0].args.tokenId.should.be.bignumber.equal(tokenId); + it('emits a transfer and minted event', async function () { + await expectEvent.inLogs(logs, 'Transfer', { + from: ZERO_ADDRESS, + to: newOwner, + }); + logs[0].args.tokenId.should.be.bignumber.equal(thirdTokenId); + + await expectEvent.inLogs(logs, 'Minted', { + to: newOwner, + }); + logs[1].args.tokenId.should.be.bignumber.equal(thirdTokenId); }); }); describe('when the given owner address is the zero address', function () { it('reverts', async function () { - await assertRevert(this.token.mint(ZERO_ADDRESS, tokenId)); + await assertRevert(this.token.mint(ZERO_ADDRESS, thirdTokenId)); }); }); describe('when the given token ID was already tracked by this contract', function () { it('reverts', async function () { - await assertRevert(this.token.mint(accounts[1], firstTokenId)); + await assertRevert(this.token.mint(owner, firstTokenId)); + }); + }); + }); + + describe('mintWithTokenURI', function () { + it('can mint with a tokenUri', async function () { + await this.token.mintWithTokenURI(newOwner, thirdTokenId, MOCK_URI, { + from: minter, }); }); }); describe('burn', function () { const tokenId = firstTokenId; - const sender = creator; let logs = null; describe('when successful', function () { beforeEach(async function () { - const result = await this.token.burn(tokenId, { from: sender }); + const result = await this.token.burn(tokenId, { from: owner }); logs = result.logs; }); it('burns the given token ID and adjusts the balance of the owner', async function () { await assertRevert(this.token.ownerOf(tokenId)); - (await this.token.balanceOf(sender)).should.be.bignumber.equal(1); + (await this.token.balanceOf(owner)).should.be.bignumber.equal(1); }); 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(sender); + 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); }); @@ -86,8 +102,8 @@ function shouldBehaveLikeMintAndBurnERC721 (accounts) { describe('when there is a previous approval burned', function () { beforeEach(async function () { - await this.token.approve(accounts[1], tokenId, { from: sender }); - const result = await this.token.burn(tokenId, { from: sender }); + await this.token.approve(approved, tokenId, { from: owner }); + const result = await this.token.burn(tokenId, { from: owner }); logs = result.logs; }); @@ -100,7 +116,38 @@ function shouldBehaveLikeMintAndBurnERC721 (accounts) { describe('when the given token ID was not tracked by this contract', function () { it('reverts', async function () { - await assertRevert(this.token.burn(unknownTokenId, { from: creator })); + await assertRevert( + this.token.burn(unknownTokenId, { from: creator }) + ); + }); + }); + }); + + 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 }) + ); }); }); }); diff --git a/test/token/ERC721/ERC721Mintable.test.js b/test/token/ERC721/ERC721Mintable.test.js new file mode 100644 index 00000000000..b53ddb59301 --- /dev/null +++ b/test/token/ERC721/ERC721Mintable.test.js @@ -0,0 +1,24 @@ +const { shouldBehaveLikeERC721Basic } = require('./ERC721Basic.behavior'); +const { + shouldBehaveLikeMintAndBurnERC721, +} = require('./ERC721MintBurn.behavior'); + +const BigNumber = web3.BigNumber; +const ERC721Mintable = artifacts.require('ERC721MintableBurnableImpl.sol'); + +require('chai') + .use(require('chai-bignumber')(BigNumber)) + .should(); + +contract('ERC721Mintable', function ([_, creator, ...accounts]) { + const minter = creator; + + beforeEach(async function () { + this.token = await ERC721Mintable.new({ + from: creator, + }); + }); + + shouldBehaveLikeERC721Basic(creator, minter, accounts); + shouldBehaveLikeMintAndBurnERC721(creator, minter, accounts); +}); diff --git a/test/token/ERC721/ERC721Pausable.test.js b/test/token/ERC721/ERC721Pausable.test.js index 7c03c888ab5..240e478e8a3 100644 --- a/test/token/ERC721/ERC721Pausable.test.js +++ b/test/token/ERC721/ERC721Pausable.test.js @@ -1,5 +1,6 @@ const { shouldBehaveLikeERC721PausedToken } = require('./ERC721PausedToken.behavior'); const { shouldBehaveLikeERC721Basic } = require('./ERC721Basic.behavior'); +const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior'); const BigNumber = web3.BigNumber; const ERC721Pausable = artifacts.require('ERC721PausableMock.sol'); @@ -8,29 +9,45 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -contract('ERC721Pausable', function ([_, owner, recipient, operator, ...otherAccounts]) { +contract('ERC721Pausable', function ([ + _, + creator, + owner, + operator, + otherPauser, + ...accounts +]) { beforeEach(async function () { - this.token = await ERC721Pausable.new({ from: owner }); + this.token = await ERC721Pausable.new({ from: creator }); + }); + + describe('pauser role', function () { + beforeEach(async function () { + this.contract = this.token; + await this.contract.addPauser(otherPauser, { from: creator }); + }); + + shouldBehaveLikePublicRole(creator, otherPauser, accounts, 'pauser'); }); context('when token is paused', function () { beforeEach(async function () { - await this.token.pause({ from: owner }); + await this.token.pause({ from: creator }); }); - shouldBehaveLikeERC721PausedToken(owner, [...otherAccounts]); + shouldBehaveLikeERC721PausedToken(creator, accounts); }); context('when token is not paused yet', function () { - shouldBehaveLikeERC721Basic([owner, ...otherAccounts]); + shouldBehaveLikeERC721Basic(creator, creator, accounts); }); context('when token is paused and then unpaused', function () { beforeEach(async function () { - await this.token.pause({ from: owner }); - await this.token.unpause({ from: owner }); + await this.token.pause({ from: creator }); + await this.token.unpause({ from: creator }); }); - shouldBehaveLikeERC721Basic([owner, ...otherAccounts]); + shouldBehaveLikeERC721Basic(creator, creator, accounts); }); });