Merge branch 'master' into remove-safe-math

CHANGELOG.md

@@ -3,8 +3,9 @@
 ## 2.2.0 (unreleased)
 
 ### New features:
+ * `ERC20`: added internal `_approve(address owner, address spender, uint256 value)`, allowing derived contracts to set the allowance of arbitrary accounts.
 
-### Improvements::
+### Improvements:
  * Upgraded the minimum compiler version to v0.5.2: this removes many Solidity warnings that were false positives.
  * `Counter`'s API has been improved, and is now used by `ERC721` (though it is still in `drafts`).
  * `ERC721`'s transfers are now more gas efficient due to removal of unnecessary `SafeMath` calls.
@@ -14,6 +15,9 @@
 
 ### Breaking changes:
 
+## 2.1.2 (2019-17-01)
+ * Removed most of the test suite from the npm package, except `PublicRole.behavior.js`, which may be useful to users testing their own `Roles`.
+
 ## 2.1.1 (2019-04-01)
  * Version bump to avoid conflict in the npm registry.
 

contracts/mocks/ERC20Mock.sol

@@ -19,4 +19,8 @@ contract ERC20Mock is ERC20 {
     function burnFrom(address account, uint256 amount) public {
         _burnFrom(account, amount);
     }
+
+    function approveInternal(address owner, address spender, uint256 value) public {
+        _approve(owner, spender, value);
+    }
 }

contracts/token/ERC20/ERC20.sol

@@ -70,10 +70,7 @@ contract ERC20 is IERC20 {
      * @param value The amount of tokens to be spent.
      */
     function approve(address spender, uint256 value) public returns (bool) {
-        require(spender != address(0));
-
-        _allowed[msg.sender][spender] = value;
-        emit Approval(msg.sender, spender, value);
+        _approve(msg.sender, spender, value);
         return true;
     }
 
@@ -86,9 +83,8 @@ contract ERC20 is IERC20 {
      * @param value uint256 the amount of tokens to be transferred
      */
     function transferFrom(address from, address to, uint256 value) public returns (bool) {
-        _allowed[from][msg.sender] = _allowed[from][msg.sender].sub(value);
         _transfer(from, to, value);
-        emit Approval(from, msg.sender, _allowed[from][msg.sender]);
+        _approve(from, msg.sender, _allowed[from][msg.sender].sub(value));
         return true;
     }
 
@@ -103,10 +99,7 @@ contract ERC20 is IERC20 {
      * @param addedValue The amount of tokens to increase the allowance by.
      */
     function increaseAllowance(address spender, uint256 addedValue) public returns (bool) {
-        require(spender != address(0));
-
-        _allowed[msg.sender][spender] = _allowed[msg.sender][spender].add(addedValue);
-        emit Approval(msg.sender, spender, _allowed[msg.sender][spender]);
+        _approve(msg.sender, spender, _allowed[msg.sender][spender].add(addedValue));
         return true;
     }
 
@@ -121,10 +114,7 @@ contract ERC20 is IERC20 {
      * @param subtractedValue The amount of tokens to decrease the allowance by.
      */
     function decreaseAllowance(address spender, uint256 subtractedValue) public returns (bool) {
-        require(spender != address(0));
-
-        _allowed[msg.sender][spender] = _allowed[msg.sender][spender].sub(subtractedValue);
-        emit Approval(msg.sender, spender, _allowed[msg.sender][spender]);
+        _approve(msg.sender, spender, _allowed[msg.sender][spender].sub(subtractedValue));
         return true;
     }
 
@@ -171,6 +161,20 @@ contract ERC20 is IERC20 {
         emit Transfer(account, address(0), value);
     }
 
+    /**
+     * @dev Approve an address to spend another addresses' tokens.
+     * @param owner The address that owns the tokens.
+     * @param spender The address that will spend the tokens.
+     * @param value The number of tokens that can be spent.
+     */
+    function _approve(address owner, address spender, uint256 value) internal {
+        require(spender != address(0));
+        require(owner != address(0));
+
+        _allowed[owner][spender] = value;
+        emit Approval(owner, spender, value);
+    }
+
     /**
      * @dev Internal function that burns an amount of the token of a given
      * account, deducting from the sender's allowance for said account. Uses the
@@ -180,8 +184,7 @@ contract ERC20 is IERC20 {
      * @param value The amount that will be burnt.
      */
     function _burnFrom(address account, uint256 value) internal {
-        _allowed[account][msg.sender] = _allowed[account][msg.sender].sub(value);
         _burn(account, value);
-        emit Approval(account, msg.sender, _allowed[account][msg.sender]);
+        _approve(account, msg.sender, _allowed[account][msg.sender].sub(value));
     }
 }

ethpm.json

@@ -1,6 +1,6 @@
 {
   "package_name": "zeppelin",
-  "version": "2.1.1",
+  "version": "2.1.2",
   "description": "Secure Smart Contract library for Solidity",
   "authors": [
     "OpenZeppelin Community <maintainers@openzeppelin.org>"

package.json

@@ -1,11 +1,11 @@
 {
   "name": "openzeppelin-solidity",
-  "version": "2.1.1",
+  "version": "2.1.2",
   "description": "Secure Smart Contract library for Solidity",
   "files": [
     "build",
     "contracts",
-    "test"
+    "test/behaviors"
   ],
   "scripts": {
     "build": "scripts/build.sh",

test/access/roles/CapperRole.test.js

@@ -1,4 +1,4 @@
-const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../../behaviors/access/roles/PublicRole.behavior');
 const CapperRoleMock = artifacts.require('CapperRoleMock');
 
 contract('CapperRole', function ([_, capper, otherCapper, ...otherAccounts]) {

test/access/roles/MinterRole.test.js

@@ -1,4 +1,4 @@
-const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../../behaviors/access/roles/PublicRole.behavior');
 const MinterRoleMock = artifacts.require('MinterRoleMock');
 
 contract('MinterRole', function ([_, minter, otherMinter, ...otherAccounts]) {

test/access/roles/PauserRole.test.js

@@ -1,4 +1,4 @@
-const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../../behaviors/access/roles/PublicRole.behavior');
 const PauserRoleMock = artifacts.require('PauserRoleMock');
 
 contract('PauserRole', function ([_, pauser, otherPauser, ...otherAccounts]) {

test/access/roles/SignerRole.test.js

@@ -1,4 +1,4 @@
-const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../../behaviors/access/roles/PublicRole.behavior');
 const SignerRoleMock = artifacts.require('SignerRoleMock');
 
 contract('SignerRole', function ([_, signer, otherSigner, ...otherAccounts]) {

test/access/roles/WhitelistAdminRole.test.js

@@ -1,4 +1,4 @@
-const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../../behaviors/access/roles/PublicRole.behavior');
 const WhitelistAdminRoleMock = artifacts.require('WhitelistAdminRoleMock');
 
 contract('WhitelistAdminRole', function ([_, whitelistAdmin, otherWhitelistAdmin, ...otherAccounts]) {

test/access/roles/WhitelistedRole.test.js

@@ -1,4 +1,4 @@
-const { shouldBehaveLikePublicRole } = require('../../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../../behaviors/access/roles/PublicRole.behavior');
 const WhitelistedRoleMock = artifacts.require('WhitelistedRoleMock');
 
 contract('WhitelistedRole', function ([_, whitelisted, otherWhitelisted, whitelistAdmin, ...otherAccounts]) {

test/access/roles/PublicRole.behavior.js → test/behaviors/access/roles/PublicRole.behavior.js

@@ -5,6 +5,21 @@ function capitalize (str) {
   return str.replace(/\b\w/g, l => l.toUpperCase());
 }
 
+// Tests that a role complies with the standard role interface, that is:
+//  * an onlyRole modifier
+//  * an isRole function
+//  * an addRole function, accessible to role havers
+//  * a renounceRole function
+//  * roleAdded and roleRemoved events
+// Both the modifier and an additional internal remove function are tested through a mock contract that exposes them.
+// This mock contract should be stored in this.contract.
+//
+// @param authorized an account that has the role
+// @param otherAuthorized another account that also has the role
+// @param anyone an account that doesn't have the role, passed inside an array for ergonomics
+// @param rolename a string with the name of the role
+// @param manager undefined for regular roles, or a manager account for managed roles. In these, only the manager
+// account can create and remove new role bearers.
 function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], rolename, manager) {
   rolename = capitalize(rolename);
 

test/crowdsale/IndividuallyCappedCrowdsale.test.js

@@ -2,7 +2,7 @@ const { BN, ether, shouldFail } = require('openzeppelin-test-helpers');
 
 const IndividuallyCappedCrowdsaleImpl = artifacts.require('IndividuallyCappedCrowdsaleImpl');
 const SimpleToken = artifacts.require('SimpleToken');
-const { shouldBehaveLikePublicRole } = require('../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../behaviors/access/roles/PublicRole.behavior');
 
 contract('IndividuallyCappedCrowdsale', function (
   [_, capper, otherCapper, wallet, alice, bob, charlie, anyone, ...otherAccounts]) {

test/drafts/SignatureBouncer.test.js

@@ -1,6 +1,6 @@
 const { shouldFail } = require('openzeppelin-test-helpers');
 const { getSignFor } = require('../helpers/sign');
-const { shouldBehaveLikePublicRole } = require('../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../behaviors/access/roles/PublicRole.behavior');
 
 const SignatureBouncerMock = artifacts.require('SignatureBouncerMock');
 

test/lifecycle/Pausable.test.js

@@ -1,5 +1,5 @@
 const { expectEvent, shouldFail } = require('openzeppelin-test-helpers');
-const { shouldBehaveLikePublicRole } = require('../access/roles/PublicRole.behavior');
+const { shouldBehaveLikePublicRole } = require('../behaviors/access/roles/PublicRole.behavior');
 
 const PausableMock = artifacts.require('PausableMock');