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.
OpenZeppelin · frangio · Sep 7, 2018 · Aug 22, 2018 · Aug 22, 2018 · Aug 28, 2018
commit d8e7c25a74c571d511daa74210e5196bb5bbf01c
diff --git a/.travis.yml b/.travis.yml
@@ -38,6 +38,14 @@ jobs:
     - stage: tests
       name: "static tests"
       script: npm run lint
+    - stage: update docs
+      if: tag =~ ^v[0-9]+\.[0-9]+\.[0-9]+$
+      addons:
+        apt:
+          packages:
+            - curl
+      script:
+        - ./scripts/ci/trigger_docs_update "${TRAVIS_TAG}"
 
 notifications:
   slack:

@@ -18,8 +18,59 @@ Any exception or additions specific to our project are documented below.
 
 * Parameters must be prefixed with an underscore.
 
-```
-function test(uint256 _testParameter1, uint256 _testParameter2) {
+    ```
+    function test(uint256 _testParameter1, uint256 _testParameter2) {
     ...
-}
-```
+    }
+    ```
+
+  The exception are the parameters of events. There is no chance of ambiguity
+  with these, so they should not have underscores. Not even if they are
+  specified on an ERC with underscores; removing them doesn't change the ABI,
+  so we should be consistent with the rest of the events in this repository
+  and remove them.
+
+* Internal and private state variables should have an underscore suffix.
+
+    ```
+    contract TestContract {
+      uint256 internal internalVar_;
+      uint256 private privateVar_;
+    }
+    ```
+
+  Variables declared in a function should not follow this rule.
+
+    ```
+    function test() {
+      uint256 functionVar;
+      ...
+    }
+    ```
+
+* Internal and private functions should have an underscore prefix.
+
+    ```
+    function _testInternal() internal {
+      ...
+    }
+    ```
+
+    ```
+    function _testPrivate() private {
+      ...
+    }
+    ```
+
+* Events should be emitted immediately after the state change that they
+  represent, and consequently they should be named in past tense.
+
+    ```
+    function _burn(address _who, uint256 _value) internal {
+      super._burn(_who, _value);
+      emit TokensBurned(_who, _value);
+    }
+    ```
+
+  Some standards (e.g. ERC20) use present tense, and in those cases the
+  standard specification prevails.
diff --git a/README.md b/README.md
@@ -89,7 +89,7 @@ The following provides visibility into how OpenZeppelin's contracts are organize
 	- **ERC20** - A standard interface for fungible tokens:
 		- *Interfaces* - Includes the ERC-20 token standard basic interface. I.e., what the contract’s ABI can represent.
 		- *Implementations* - Includes ERC-20 token implementations that include all required and some optional ERC-20 functionality.
-	- **ERC721** - A standard interface for non-fungible tokens 
+	- **ERC721** - A standard interface for non-fungible tokens
 		- *Interfaces* - Includes the ERC-721 token standard basic interface. I.e., what the contract’s ABI can represent.
 		- *Implementations* - Includes ERC-721 token implementations that include all required and some optional ERC-721 functionality.
 
@@ -125,6 +125,7 @@ Interested in contributing to OpenZeppelin?
 - Framework proposal and roadmap: https://medium.com/zeppelin-blog/zeppelin-framework-proposal-and-development-roadmap-fdfa9a3a32ab#.iain47pak
 - Issue tracker: https://github.com/OpenZeppelin/openzeppelin-solidity/issues
 - Contribution guidelines: https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/CONTRIBUTING.md
+- Code-style guide: https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/CODE_STYLE.md
 - Wiki: https://github.com/OpenZeppelin/openzeppelin-solidity/wiki
 
 ## License

diff --git a/RELEASING.md b/RELEASING.md
@@ -7,7 +7,7 @@ We release a new version of OpenZeppelin monthly. Release cycles are tracked in
 Each release has at least one release candidate published first, intended for community review and any critical fixes that may come out of it. At the moment we leave 1 week between the first release candidate and the final release.
 
 Before starting make sure to verify the following items.
-* Your local `master` branch is in sync with your upstream remote.
+* Your local `master` branch is in sync with your `upstream` remote (it may have another name depending on your setup).
 * Your repo is clean, particularly with no untracked files in the contracts and tests directories. Verify with `git clean -n`.
 
 
@@ -44,8 +44,11 @@ Publish the release notes on GitHub and ask our community manager to announce th
 
 ## Creating the final release
 
+Make sure to have the latest changes from `upstream` in your local release branch.
+
 ```
 git checkout release-vX.Y.Z
+git pull upstream
 ```
 
 Change the version string in `package.json`, `package-lock.json` and `ethpm.json` removing the "-rc.R" suffix. Commit these changes and tag the commit as `vX.Y.Z`.
@@ -75,7 +78,14 @@ npm dist-tag rm --otp $2FA_CODE openzeppelin-solidity next
 
 ## Merging the release branch
 
-After the final release, the release branch should be merged back into `master`. This merge must not be squashed, because it would lose the tagged release commit, so it should be merged locally and pushed.
+After the final release, the release branch should be merged back into `master`. This merge must not be squashed because it would lose the tagged release commit. Since the GitHub repo is set up to only allow squashed merges, the merge should be done locally and pushed.
+
+Make sure to have the latest changes from `upstream` in your local release branch.
+
+```
+git checkout release-vX.Y.Z
+git pull upstream
+```
 
 ```
 git checkout master

diff --git a/contracts/LimitBalance.sol b/contracts/LimitBalance.sol
diff --git a/contracts/access/SignatureBouncer.sol b/contracts/access/SignatureBouncer.sol
@@ -2,7 +2,7 @@ pragma solidity ^0.4.24;
 
 import "../ownership/Ownable.sol";
 import "../access/rbac/RBAC.sol";
-import "../ECRecovery.sol";
+import "../cryptography/ECDSA.sol";
 
 
 /**
@@ -17,7 +17,7 @@ import "../ECRecovery.sol";
  * valid addresses on-chain, simply sign a grant of the form
  * keccak256(abi.encodePacked(`:contractAddress` + `:granteeAddress`)) using a valid bouncer address.
  * Then restrict access to your crowdsale/whitelist/airdrop using the
- * `onlyValidSignature` modifier (or implement your own using isValidSignature).
+ * `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.
@@ -30,19 +30,22 @@ import "../ECRecovery.sol";
  * much more complex. See https://ethereum.stackexchange.com/a/50616 for more details.
  */
 contract SignatureBouncer is Ownable, RBAC {
-  using ECRecovery for bytes32;
+  using ECDSA for bytes32;
 
-  string public constant ROLE_BOUNCER = "bouncer";
-  uint constant METHOD_ID_SIZE = 4;
-  // signature size is 65 bytes (tightly packed v + r + s), but gets padded to 96 bytes
-  uint constant SIGNATURE_SIZE = 96;
+  // 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;
+  // Signature size is 65 bytes (tightly packed v + r + s), but gets padded to 96 bytes
+  uint256 private constant SIGNATURE_SIZE = 96;
 
   /**
    * @dev requires that a valid signature of a bouncer was provided
    */
   modifier onlyValidSignature(bytes _signature)
   {
-    require(isValidSignature(msg.sender, _signature));
+    require(_isValidSignature(msg.sender, _signature));
     _;
   }
 
@@ -51,7 +54,7 @@ contract SignatureBouncer is Ownable, RBAC {
    */
   modifier onlyValidSignatureAndMethod(bytes _signature)
   {
-    require(isValidSignatureAndMethod(msg.sender, _signature));
+    require(_isValidSignatureAndMethod(msg.sender, _signature));
     _;
   }
 
@@ -60,10 +63,18 @@ contract SignatureBouncer is Ownable, RBAC {
    */
   modifier onlyValidSignatureAndData(bytes _signature)
   {
-    require(isValidSignatureAndData(msg.sender, _signature));
+    require(_isValidSignatureAndData(msg.sender, _signature));
     _;
   }
 
+  /**
+   * @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
    */
@@ -72,7 +83,7 @@ contract SignatureBouncer is Ownable, RBAC {
     onlyOwner
   {
     require(_bouncer != address(0));
-    addRole(_bouncer, ROLE_BOUNCER);
+    _addRole(_bouncer, ROLE_BOUNCER);
   }
 
   /**
@@ -82,19 +93,19 @@ contract SignatureBouncer is Ownable, RBAC {
     public
     onlyOwner
   {
-    removeRole(_bouncer, ROLE_BOUNCER);
+    _removeRole(_bouncer, ROLE_BOUNCER);
   }
 
   /**
    * @dev is the signature of `this + sender` from a bouncer?
    * @return bool
    */
-  function isValidSignature(address _address, bytes _signature)
+  function _isValidSignature(address _address, bytes _signature)
     internal
     view
     returns (bool)
   {
-    return isValidDataHash(
+    return _isValidDataHash(
       keccak256(abi.encodePacked(address(this), _address)),
       _signature
     );
@@ -104,7 +115,7 @@ contract SignatureBouncer is Ownable, RBAC {
    * @dev is the signature of `this + sender + methodId` from a bouncer?
    * @return bool
    */
-  function isValidSignatureAndMethod(address _address, bytes _signature)
+  function _isValidSignatureAndMethod(address _address, bytes _signature)
     internal
     view
     returns (bool)
@@ -113,7 +124,7 @@ contract SignatureBouncer is Ownable, RBAC {
     for (uint i = 0; i < data.length; i++) {
       data[i] = msg.data[i];
     }
-    return isValidDataHash(
+    return _isValidDataHash(
       keccak256(abi.encodePacked(address(this), _address, data)),
       _signature
     );
@@ -124,7 +135,7 @@ contract SignatureBouncer is Ownable, RBAC {
     * @notice the _signature parameter of the method being validated must be the "last" parameter
     * @return bool
     */
-  function isValidSignatureAndData(address _address, bytes _signature)
+  function _isValidSignatureAndData(address _address, bytes _signature)
     internal
     view
     returns (bool)
@@ -134,7 +145,7 @@ contract SignatureBouncer is Ownable, RBAC {
     for (uint i = 0; i < data.length; i++) {
       data[i] = msg.data[i];
     }
-    return isValidDataHash(
+    return _isValidDataHash(
       keccak256(abi.encodePacked(address(this), _address, data)),
       _signature
     );
@@ -145,14 +156,14 @@ contract SignatureBouncer is Ownable, RBAC {
    * and then recover the signature and check it against the bouncer role
    * @return bool
    */
-  function isValidDataHash(bytes32 _hash, bytes _signature)
+  function _isValidDataHash(bytes32 _hash, bytes _signature)
     internal
     view
     returns (bool)
   {
     address signer = _hash
       .toEthSignedMessageHash()
       .recover(_signature);
-    return hasRole(signer, ROLE_BOUNCER);
+    return isBouncer(signer);
   }
 }
diff --git a/contracts/access/Whitelist.sol b/contracts/access/Whitelist.sol
@@ -11,7 +11,9 @@ import "../access/rbac/RBAC.sol";
  * This simplifies the implementation of "user permissions".
  */
 contract Whitelist is Ownable, RBAC {
-  string public constant ROLE_WHITELISTED = "whitelist";
+
+  // Name of the whitelisted role.
+  string private constant ROLE_WHITELISTED = "whitelist";
 
   /**
    * @dev Throws if operator is not whitelisted.
@@ -31,13 +33,14 @@ contract Whitelist is Ownable, RBAC {
     public
     onlyOwner
   {
-    addRole(_operator, ROLE_WHITELISTED);
+    _addRole(_operator, ROLE_WHITELISTED);
   }
 
   /**
-   * @dev getter to determine if address is in whitelist
+   * @dev Determine if an account is whitelisted.
+   * @return true if the account is whitelisted, false otherwise.
    */
-  function whitelist(address _operator)
+  function isWhitelisted(address _operator)
     public
     view
     returns (bool)
@@ -70,7 +73,7 @@ contract Whitelist is Ownable, RBAC {
     public
     onlyOwner
   {
-    removeRole(_operator, ROLE_WHITELISTED);
+    _removeRole(_operator, ROLE_WHITELISTED);
   }
 
   /**