Merge branch 'release-v2.0.0' of github.com:OpenZeppelin/openzeppelin-solidity into release-v2.0.0

contracts/access/roles/CapperRole.sol

@@ -13,7 +13,7 @@ contract CapperRole is Initializable {
   Roles.Role private cappers;
 
   function initialize() public initializer {
-    cappers.add(msg.sender);
+    _addCapper(msg.sender);
   }
 
   modifier onlyCapper() {
@@ -26,12 +26,16 @@ contract CapperRole is Initializable {
   }
 
   function addCapper(address account) public onlyCapper {
-    cappers.add(account);
-    emit CapperAdded(account);
+    _addCapper(account);
   }
 
   function renounceCapper() public {
-    cappers.remove(msg.sender);
+    _removeCapper(msg.sender);
+  }
+
+  function _addCapper(address account) internal {
+    cappers.add(account);
+    emit CapperAdded(account);
   }
 
   function _removeCapper(address account) internal {

contracts/access/roles/MinterRole.sol

@@ -13,7 +13,7 @@ contract MinterRole is Initializable {
   Roles.Role private minters;
 
   function initialize() public initializer {
-    minters.add(msg.sender);
+    _addMinter(msg.sender);
   }
 
   modifier onlyMinter() {
@@ -26,12 +26,16 @@ contract MinterRole is Initializable {
   }
 
   function addMinter(address account) public onlyMinter {
-    minters.add(account);
-    emit MinterAdded(account);
+    _addMinter(account);
   }
 
   function renounceMinter() public {
-    minters.remove(msg.sender);
+    _removeMinter(msg.sender);
+  }
+
+  function _addMinter(address account) internal {
+    minters.add(account);
+    emit MinterAdded(account);
   }
 
   function _removeMinter(address account) internal {

contracts/access/roles/PauserRole.sol

@@ -13,7 +13,7 @@ contract PauserRole is Initializable {
   Roles.Role private pausers;
 
   function initialize() public initializer {
-    pausers.add(msg.sender);
+    _addPauser(msg.sender);
   }
 
   modifier onlyPauser() {
@@ -26,12 +26,16 @@ contract PauserRole is Initializable {
   }
 
   function addPauser(address account) public onlyPauser {
-    pausers.add(account);
-    emit PauserAdded(account);
+    _addPauser(account);
   }
 
   function renouncePauser() public {
-    pausers.remove(msg.sender);
+    _removePauser(msg.sender);
+  }
+
+  function _addPauser(address account) internal {
+    pausers.add(account);
+    emit PauserAdded(account);
   }
 
   function _removePauser(address account) internal {

contracts/access/roles/SignerRole.sol

@@ -13,7 +13,7 @@ contract SignerRole is Initializable {
   Roles.Role private signers;
 
   function initialize() public initializer {
-    signers.add(msg.sender);
+    _addSigner(msg.sender);
   }
 
   modifier onlySigner() {
@@ -26,12 +26,16 @@ contract SignerRole is Initializable {
   }
 
   function addSigner(address account) public onlySigner {
-    signers.add(account);
-    emit SignerAdded(account);
+    _addSigner(account);
   }
 
   function renounceSigner() public {
-    signers.remove(msg.sender);
+    _removeSigner(msg.sender);
+  }
+
+  function _addSigner(address account) internal {
+    signers.add(account);
+    emit SignerAdded(account);
   }
 
   function _removeSigner(address account) internal {

contracts/mocks/InsecureInvariantTargetBounty.sol → contracts/mocks/BreakInvariantBountyMock.sol

@@ -6,15 +6,24 @@ pragma solidity ^0.4.24;
 import {BreakInvariantBounty, Target} from "../drafts/BreakInvariantBounty.sol";
 
 
-contract InsecureInvariantTargetMock is Target {
-  function checkInvariant() public returns(bool) {
-    return false;
+contract TargetMock is Target {
+  bool private exploited;
+
+  function exploitVulnerability() public {
+    exploited = true;
   }
-}
 
+  function checkInvariant() public returns (bool) {
+    if (exploited) {
+      return false;
+    }
+
+    return true;
+  }
+}
 
-contract InsecureInvariantTargetBounty is BreakInvariantBounty {
+contract BreakInvariantBountyMock is BreakInvariantBounty {
   function _deployContract() internal returns (address) {
-    return new InsecureInvariantTargetMock();
+    return new TargetMock();
   }
 }

contracts/mocks/SecureInvariantTargetBounty.sol

-pragma solidity ^0.4.24;
-
-// When this line is split, truffle parsing fails.
-// See: https://github.com/ethereum/solidity/issues/4871
-// solium-disable-next-line max-len
-import {BreakInvariantBounty, Target} from "../drafts/BreakInvariantBounty.sol";
-
-
-contract SecureInvariantTargetMock is Target {
-  function checkInvariant() public returns(bool) {
-    return true;
-  }
-}
-
-
-contract SecureInvariantTargetBounty is BreakInvariantBounty {
-  function _deployContract() internal returns (address) {
-    return new SecureInvariantTargetMock();
-  }
-}

contracts/payment/Escrow.sol

@@ -45,7 +45,6 @@ contract Escrow is Initializable, Secondary {
   */
   function withdraw(address payee) public onlyPrimary {
     uint256 payment = _deposits[payee];
-    assert(address(this).balance >= payment);
 
     _deposits[payee] = 0;
 

contracts/payment/SplitPayment.sol

@@ -86,7 +86,6 @@ contract SplitPayment is Initializable {
     );
 
     require(payment != 0);
-    assert(address(this).balance >= payment);
 
     _released[account] = _released[account].add(payment);
     _totalReleased = _totalReleased.add(payment);

contracts/token/ERC20/ERC20Mintable.sol

@@ -10,27 +10,11 @@ import "../../access/roles/MinterRole.sol";
  * @dev ERC20 minting logic
  */
 contract ERC20Mintable is Initializable, ERC20, MinterRole {
-  event MintingFinished();
-
-  bool private _mintingFinished = false;
-
-  modifier onlyBeforeMintingFinished() {
-    require(!_mintingFinished);
-    _;
-  }
-
   function initialize() public initializer {
     MinterRole.initialize();
   }
 
   /**
-   * @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 amount The amount of tokens to mint.
@@ -42,25 +26,9 @@ contract ERC20Mintable is Initializable, ERC20, MinterRole {
   )
     public
     onlyMinter
-    onlyBeforeMintingFinished
     returns (bool)
   {
     _mint(to, amount);
     return true;
   }
-
-  /**
-   * @dev Function to stop minting new tokens.
-   * @return True if the operation was successful.
-   */
-  function finishMinting()
-    public
-    onlyMinter
-    onlyBeforeMintingFinished
-    returns (bool)
-  {
-    _mintingFinished = true;
-    emit MintingFinished();
-    return true;
-  }
 }

contracts/token/ERC721/ERC721Mintable.sol

@@ -10,27 +10,11 @@ import "../../access/roles/MinterRole.sol";
  * @dev ERC721 minting logic
  */
 contract ERC721Mintable is Initializable, ERC721Full, MinterRole {
-  event MintingFinished();
-
-  bool private _mintingFinished = false;
-
-  modifier onlyBeforeMintingFinished() {
-    require(!_mintingFinished);
-    _;
-  }
-
   function initialize() public initializer {
     MinterRole.initialize();
   }
 
   /**
-   * @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.
@@ -42,7 +26,6 @@ contract ERC721Mintable is Initializable, ERC721Full, MinterRole {
   )
     public
     onlyMinter
-    onlyBeforeMintingFinished
     returns (bool)
   {
     _mint(to, tokenId);
@@ -56,26 +39,10 @@ contract ERC721Mintable is Initializable, ERC721Full, MinterRole {
   )
     public
     onlyMinter
-    onlyBeforeMintingFinished
     returns (bool)
   {
     mint(to, tokenId);
     _setTokenURI(tokenId, tokenURI);
     return true;
   }
-
-  /**
-   * @dev Function to stop minting new tokens.
-   * @return True if the operation was successful.
-   */
-  function finishMinting()
-    public
-    onlyMinter
-    onlyBeforeMintingFinished
-    returns (bool)
-  {
-    _mintingFinished = true;
-    emit MintingFinished();
-    return true;
-  }
 }

test/access/roles/PublicRole.behavior.js

@@ -89,6 +89,11 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role
         (await this.contract[`is${rolename}`](authorized)).should.equal(false);
       });
 
+      it(`emits a ${rolename}Removed event`, async function () {
+        const { logs } = await this.contract[`renounce${rolename}`]({ from: authorized });
+        expectEvent.inLogs(logs, `${rolename}Removed`, { account: authorized });
+      });
+
       it('doesn\'t revert when renouncing unassigned role', async function () {
         await this.contract[`renounce${rolename}`]({ from: anyone });
       });

test/drafts/BreakInvariantBounty.test.js

@@ -2,97 +2,104 @@ const { ethGetBalance, ethSendTransaction } = require('../helpers/web3');
 const expectEvent = require('../helpers/expectEvent');
 const { assertRevert } = require('../helpers/assertRevert');
 
-const SecureInvariantTargetBounty = artifacts.require('SecureInvariantTargetBounty');
-const InsecureInvariantTargetBounty = artifacts.require('InsecureInvariantTargetBounty');
+const BreakInvariantBountyMock = artifacts.require('BreakInvariantBountyMock');
+const TargetMock = artifacts.require('TargetMock');
 
 require('chai')
   .use(require('chai-bignumber')(web3.BigNumber))
   .should();
 
-const sendReward = async (from, to, value) => ethSendTransaction({
-  from,
-  to,
-  value,
-});
-
 const reward = new web3.BigNumber(web3.toWei(1, 'ether'));
 
-contract('BreakInvariantBounty', function ([_, owner, researcher, nonTarget]) {
-  context('against secure contract', function () {
-    beforeEach(async function () {
-      this.bounty = await SecureInvariantTargetBounty.new({ from: owner });
-    });
+contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTarget]) {
+  beforeEach(async function () {
+    this.bounty = await BreakInvariantBountyMock.new({ from: owner });
+  });
 
-    it('can set reward', async function () {
-      await sendReward(owner, this.bounty.address, reward);
+  it('can set reward', async function () {
+    await ethSendTransaction({ from: owner, to: this.bounty.address, value: reward });
+    (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(reward);
+  });
 
-      const balance = await ethGetBalance(this.bounty.address);
-      balance.should.be.bignumber.equal(reward);
+  context('with reward', function () {
+    beforeEach(async function () {
+      await ethSendTransaction({ from: owner, to: this.bounty.address, value: reward });
     });
 
-    context('with reward', function () {
-      beforeEach(async function () {
-        const result = await this.bounty.createTarget({ from: researcher });
-        const event = expectEvent.inLogs(result.logs, 'TargetCreated');
-
-        this.targetAddress = event.args.createdAddress;
-
-        await sendReward(owner, this.bounty.address, reward);
+    describe('destroy', function () {
+      it('returns all balance to the owner', async function () {
+        const ownerPreBalance = await ethGetBalance(owner);
+        await this.bounty.destroy({ from: owner, gasPrice: 0 });
+        const ownerPostBalance = await ethGetBalance(owner);
 
-        const balance = await ethGetBalance(this.bounty.address);
-        balance.should.be.bignumber.equal(reward);
+        (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0);
+        ownerPostBalance.sub(ownerPreBalance).should.be.bignumber.equal(reward);
       });
 
-      it('cannot claim reward', async function () {
-        await assertRevert(
-          this.bounty.claim(this.targetAddress, { from: researcher }),
-        );
+      it('reverts when called by anyone', async function () {
+        await assertRevert(this.bounty.destroy({ from: anyone }));
       });
     });
-  });
-
-  context('against broken contract', function () {
-    beforeEach(async function () {
-      this.bounty = await InsecureInvariantTargetBounty.new();
 
-      const result = await this.bounty.createTarget({ from: researcher });
-      const event = expectEvent.inLogs(result.logs, 'TargetCreated');
-
-      this.targetAddress = event.args.createdAddress;
-      await sendReward(owner, this.bounty.address, reward);
-    });
-
-    it('can claim reward', async function () {
-      await this.bounty.claim(this.targetAddress, { from: researcher });
-      const claim = await this.bounty.claimed();
-
-      claim.should.equal(true);
-
-      const researcherPrevBalance = await ethGetBalance(researcher);
-
-      await this.bounty.withdrawPayments(researcher, { gasPrice: 0 });
-      const updatedBalance = await ethGetBalance(this.bounty.address);
-      updatedBalance.should.be.bignumber.equal(0);
-
-      const researcherCurrBalance = await ethGetBalance(researcher);
-      researcherCurrBalance.sub(researcherPrevBalance).should.be.bignumber.equal(reward);
-    });
+    describe('claim', function () {
+      it('is initially unclaimed', async function () {
+        (await this.bounty.claimed()).should.equal(false);
+      });
 
-    it('cannot claim reward from non-target', async function () {
-      await assertRevert(
-        this.bounty.claim(nonTarget, { from: researcher })
-      );
-    });
+      it('can create claimable target', async function () {
+        const { logs } = await this.bounty.createTarget({ from: researcher });
+        expectEvent.inLogs(logs, 'TargetCreated');
+      });
 
-    context('reward claimed', function () {
-      beforeEach(async function () {
-        await this.bounty.claim(this.targetAddress, { from: researcher });
+      context('with target', async function () {
+        beforeEach(async function () {
+          const { logs } = await this.bounty.createTarget({ from: researcher });
+          const event = expectEvent.inLogs(logs, 'TargetCreated');
+          this.target = TargetMock.at(event.args.createdAddress);
+        });
+
+        context('before exploiting vulnerability', async function () {
+          it('reverts when claiming reward', async function () {
+            await assertRevert(this.bounty.claim(this.target.address, { from: researcher }));
+          });
+        });
+
+        context('after exploiting vulnerability', async function () {
+          beforeEach(async function () {
+            await this.target.exploitVulnerability({ from: researcher });
+          });
+
+          it('sends the reward to the researcher', async function () {
+            await this.bounty.claim(this.target.address, { from: anyone });
+
+            const researcherPreBalance = await ethGetBalance(researcher);
+            await this.bounty.withdrawPayments(researcher);
+            const researcherPostBalance = await ethGetBalance(researcher);
+
+            researcherPostBalance.sub(researcherPreBalance).should.be.bignumber.equal(reward);
+            (await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0);
+          });
+
+          context('after claiming', async function () {
+            beforeEach(async function () {
+              await this.bounty.claim(this.target.address, { from: researcher });
+            });
+
+            it('is claimed', async function () {
+              (await this.bounty.claimed()).should.equal(true);
+            });
+
+            it('no longer accepts rewards', async function () {
+              await assertRevert(ethSendTransaction({ from: owner, to: this.bounty.address, value: reward }));
+            });
+          });
+        });
       });
 
-      it('should no longer be payable', async function () {
-        await assertRevert(
-          sendReward(owner, this.bounty.address, reward)
-        );
+      context('with non-target', function () {
+        it('reverts when claiming reward', async function () {
+          await assertRevert(this.bounty.claim(nonTarget, { from: researcher }));
+        });
       });
     });
   });

test/token/ERC20/behaviors/ERC20Mintable.behavior.js

@@ -11,136 +11,44 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) {
   const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000';
 
   describe('as a mintable token', function () {
-    describe('mintingFinished', function () {
-      context('when token minting is not finished', function () {
-        it('returns false', async function () {
-          (await this.token.mintingFinished()).should.equal(false);
-        });
-      });
-
-      context('when token minting is finished', function () {
-        beforeEach(async function () {
-          await this.token.finishMinting({ from: minter });
-        });
-
-        it('returns true', async function () {
-          (await this.token.mintingFinished()).should.equal(true);
-        });
-      });
-    });
+    describe('mint', function () {
+      const amount = 100;
 
-    describe('finishMinting', function () {
       context('when the sender has minting permission', function () {
         const from = minter;
 
-        context('when token minting was not finished', function () {
-          it('finishes token minting', async function () {
-            await this.token.finishMinting({ from });
-
-            (await this.token.mintingFinished()).should.equal(true);
-          });
-
-          it('emits a mint finished event', async function () {
-            const { logs } = await this.token.finishMinting({ from });
-
-            await expectEvent.inLogs(logs, 'MintingFinished');
-          });
-        });
-
-        context('when token minting was already finished', function () {
-          beforeEach(async function () {
-            await this.token.finishMinting({ from });
-          });
-
-          it('reverts', async function () {
-            await assertRevert(this.token.finishMinting({ from }));
-          });
+        context('for a zero amount', function () {
+          shouldMint(0);
         });
-      });
-
-      context('when the sender doesn\'t have minting permission', function () {
-        const from = anyone;
 
-        context('when token minting was not finished', function () {
-          it('reverts', async function () {
-            await assertRevert(this.token.finishMinting({ from }));
-          });
+        context('for a non-zero amount', function () {
+          shouldMint(amount);
         });
 
-        context('when token minting was already finished', function () {
+        function shouldMint (amount) {
           beforeEach(async function () {
-            await this.token.finishMinting({ from: minter });
-          });
-
-          it('reverts', async function () {
-            await assertRevert(this.token.finishMinting({ from }));
-          });
-        });
-      });
-    });
-
-    describe('mint', function () {
-      const amount = 100;
-
-      context('when the sender has minting permission', function () {
-        const from = minter;
-
-        context('when token minting is not finished', function () {
-          context('for a zero amount', function () {
-            shouldMint(0);
+            ({ logs: this.logs } = await this.token.mint(anyone, amount, { from }));
           });
 
-          context('for a non-zero amount', function () {
-            shouldMint(amount);
+          it('mints the requested amount', async function () {
+            (await this.token.balanceOf(anyone)).should.be.bignumber.equal(amount);
           });
 
-          function shouldMint (amount) {
-            beforeEach(async function () {
-              ({ logs: this.logs } = await this.token.mint(anyone, amount, { from }));
-            });
-
-            it('mints the requested amount', async function () {
-              (await this.token.balanceOf(anyone)).should.be.bignumber.equal(amount);
-            });
-
-            it('emits a mint and a transfer event', async function () {
-              const transferEvent = expectEvent.inLogs(this.logs, 'Transfer', {
-                from: ZERO_ADDRESS,
-                to: anyone,
-              });
-              transferEvent.args.value.should.be.bignumber.equal(amount);
+          it('emits a mint and a transfer event', async function () {
+            const transferEvent = expectEvent.inLogs(this.logs, 'Transfer', {
+              from: ZERO_ADDRESS,
+              to: anyone,
             });
-          }
-        });
-
-        context('when token minting is finished', function () {
-          beforeEach(async function () {
-            await this.token.finishMinting({ from: minter });
+            transferEvent.args.value.should.be.bignumber.equal(amount);
           });
-
-          it('reverts', async function () {
-            await assertRevert(this.token.mint(anyone, amount, { from }));
-          });
-        });
+        }
       });
 
       context('when the sender doesn\'t have minting permission', function () {
         const from = anyone;
 
-        context('when token minting is not finished', function () {
-          it('reverts', async function () {
-            await assertRevert(this.token.mint(anyone, amount, { from }));
-          });
-        });
-
-        context('when token minting is already finished', function () {
-          beforeEach(async function () {
-            await this.token.finishMinting({ from: minter });
-          });
-
-          it('reverts', async function () {
-            await assertRevert(this.token.mint(anyone, amount, { from }));
-          });
+        it('reverts', async function () {
+          await assertRevert(this.token.mint(anyone, amount, { from }));
         });
       });
     });

test/token/ERC721/ERC721Holder.test.js

+const ERC721Holder = artifacts.require('ERC721Holder.sol');
+const ERC721Mintable = artifacts.require('ERC721MintableBurnableImpl.sol');
+
+require('chai')
+  .should();
+
+contract('ERC721Holder', function ([creator]) {
+  it('receives an ERC721 token', async function () {
+    const token = await ERC721Mintable.new({ from: creator });
+    const tokenId = 1;
+    await token.mint(creator, tokenId, { from: creator });
+
+    const receiver = await ERC721Holder.new();
+    await token.approve(receiver.address, tokenId, { from: creator });
+    await token.safeTransferFrom(creator, receiver.address, tokenId);
+
+    (await token.ownerOf(tokenId)).should.be.equal(receiver.address);
+  });
+});

test/token/ERC721/ERC721MintBurn.behavior.js

@@ -117,35 +117,6 @@ function shouldBehaveLikeMintAndBurnERC721 (
         });
       });
     });
-
-    describe('finishMinting', function () {
-      it('allows the minter to finish minting', async function () {
-        const { logs } = await this.token.finishMinting({ from: minter });
-        expectEvent.inLogs(logs, 'MintingFinished');
-      });
-    });
-
-    context('mintingFinished', function () {
-      beforeEach(async function () {
-        await this.token.finishMinting({ from: minter });
-      });
-
-      describe('mint', function () {
-        it('reverts', async function () {
-          await assertRevert(
-            this.token.mint(owner, thirdTokenId, { from: minter })
-          );
-        });
-      });
-
-      describe('mintWithTokenURI', function () {
-        it('reverts', async function () {
-          await assertRevert(
-            this.token.mintWithTokenURI(owner, thirdTokenId, MOCK_URI, { from: minter })
-          );
-        });
-      });
-    });
   });
 }