Added message string for require() (#1704)

* Error handling in ERC20 and ERC721

* Added message string for require.

* Fixed solhint errors.

* Updated PR as per issue #1709

* changes as per #1709 and openzeppelin forum.

* Changes in require statement

* Changes in require statement

* build pipeline fix

* Changes as per @nventuro's comment.

* Update revert reason strings.

* Fianal update of revert reason strings.

* WIP: Updating reason strings in test cases

* WIP: Added changes to ERC20 and ERC721

* Fixes linting errors in *.tes.js files

* Achieved 100% code coverage

* Updated the test cases with shouldFail.reverting.withMessage()

* Fix package-lock.

* address review comments

* fix linter issues

* fix remaining revert reasons

contracts/access/Roles.sol

@@ -13,17 +13,15 @@ library Roles {
      * @dev Give an account access to this role.
      */
     function add(Role storage role, address account) internal {
-        require(!has(role, account));
-
+        require(!has(role, account), "Roles: account already has role");
         role.bearer[account] = true;
     }
 
     /**
      * @dev Remove an account's access to this role.
      */
     function remove(Role storage role, address account) internal {
-        require(has(role, account));
-
+        require(has(role, account), "Roles: account does not have role");
         role.bearer[account] = false;
     }
 
@@ -32,7 +30,7 @@ library Roles {
      * @return bool
      */
     function has(Role storage role, address account) internal view returns (bool) {
-        require(account != address(0));
+        require(account != address(0), "Roles: account is the zero address");
         return role.bearer[account];
     }
 }

contracts/access/roles/CapperRole.sol

@@ -15,7 +15,7 @@ contract CapperRole {
     }
 
     modifier onlyCapper() {
-        require(isCapper(msg.sender));
+        require(isCapper(msg.sender), "CapperRole: caller does not have the Capper role");
         _;
     }
 

contracts/access/roles/MinterRole.sol

@@ -15,7 +15,7 @@ contract MinterRole {
     }
 
     modifier onlyMinter() {
-        require(isMinter(msg.sender));
+        require(isMinter(msg.sender), "MinterRole: caller does not have the Minter role");
         _;
     }
 

contracts/access/roles/PauserRole.sol

@@ -15,7 +15,7 @@ contract PauserRole {
     }
 
     modifier onlyPauser() {
-        require(isPauser(msg.sender));
+        require(isPauser(msg.sender), "PauserRole: caller does not have the Pauser role");
         _;
     }
 

contracts/access/roles/SignerRole.sol

@@ -15,7 +15,7 @@ contract SignerRole {
     }
 
     modifier onlySigner() {
-        require(isSigner(msg.sender));
+        require(isSigner(msg.sender), "SignerRole: caller does not have the Signer role");
         _;
     }
 

contracts/access/roles/WhitelistAdminRole.sol

@@ -19,7 +19,7 @@ contract WhitelistAdminRole {
     }
 
     modifier onlyWhitelistAdmin() {
-        require(isWhitelistAdmin(msg.sender));
+        require(isWhitelistAdmin(msg.sender), "WhitelistAdminRole: caller does not have the WhitelistAdmin role");
         _;
     }
 

contracts/access/roles/WhitelistedRole.sol

@@ -18,7 +18,7 @@ contract WhitelistedRole is WhitelistAdminRole {
     Roles.Role private _whitelisteds;
 
     modifier onlyWhitelisted() {
-        require(isWhitelisted(msg.sender));
+        require(isWhitelisted(msg.sender), "WhitelistedRole: caller does not have the Whitelisted role");
         _;
     }
 

contracts/crowdsale/Crowdsale.sol

@@ -54,9 +54,9 @@ contract Crowdsale is ReentrancyGuard {
      * @param token Address of the token being sold
      */
     constructor (uint256 rate, address payable wallet, IERC20 token) public {
-        require(rate > 0);
-        require(wallet != address(0));
-        require(address(token) != address(0));
+        require(rate > 0, "Crowdsale: rate is 0");
+        require(wallet != address(0), "Crowdsale: wallet is the zero address");
+        require(address(token) != address(0), "Crowdsale: token is the zero address");
 
         _rate = rate;
         _wallet = wallet;
@@ -136,8 +136,8 @@ contract Crowdsale is ReentrancyGuard {
      * @param weiAmount Value in wei involved in the purchase
      */
     function _preValidatePurchase(address beneficiary, uint256 weiAmount) internal view {
-        require(beneficiary != address(0));
-        require(weiAmount != 0);
+        require(beneficiary != address(0), "Crowdsale: beneficiary is the zero address");
+        require(weiAmount != 0, "Crowdsale: weiAmount is 0");
     }
 
     /**

contracts/crowdsale/distribution/FinalizableCrowdsale.sol

@@ -31,8 +31,8 @@ contract FinalizableCrowdsale is TimedCrowdsale {
      * work. Calls the contract's finalization function.
      */
     function finalize() public {
-        require(!_finalized);
-        require(hasClosed());
+        require(!_finalized, "FinalizableCrowdsale: already finalized");
+        require(hasClosed(), "FinalizableCrowdsale: not closed");
 
         _finalized = true;
 

contracts/crowdsale/distribution/PostDeliveryCrowdsale.sol

@@ -24,9 +24,9 @@ contract PostDeliveryCrowdsale is TimedCrowdsale {
      * @param beneficiary Whose tokens will be withdrawn.
      */
     function withdrawTokens(address beneficiary) public {
-        require(hasClosed());
+        require(hasClosed(), "PostDeliveryCrowdsale: not closed");
         uint256 amount = _balances[beneficiary];
-        require(amount > 0);
+        require(amount > 0, "PostDeliveryCrowdsale: beneficiary is not due any tokens");
 
         _balances[beneficiary] = 0;
         _vault.transfer(token(), beneficiary, amount);

contracts/crowdsale/distribution/RefundableCrowdsale.sol

@@ -28,7 +28,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
      * @param goal Funding goal
      */
     constructor (uint256 goal) public {
-        require(goal > 0);
+        require(goal > 0, "RefundableCrowdsale: goal is 0");
         _escrow = new RefundEscrow(wallet());
         _goal = goal;
     }
@@ -45,8 +45,8 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
      * @param refundee Whose refund will be claimed.
      */
     function claimRefund(address payable refundee) public {
-        require(finalized());
-        require(!goalReached());
+        require(finalized(), "RefundableCrowdsale: not finalized");
+        require(!goalReached(), "RefundableCrowdsale: goal reached");
 
         _escrow.withdraw(refundee);
     }

contracts/crowdsale/distribution/RefundablePostDeliveryCrowdsale.sol

@@ -12,8 +12,8 @@ import "./PostDeliveryCrowdsale.sol";
  */
 contract RefundablePostDeliveryCrowdsale is RefundableCrowdsale, PostDeliveryCrowdsale {
     function withdrawTokens(address beneficiary) public {
-        require(finalized());
-        require(goalReached());
+        require(finalized(), "RefundablePostDeliveryCrowdsale: not finalized");
+        require(goalReached(), "RefundablePostDeliveryCrowdsale: goal not reached");
 
         super.withdrawTokens(beneficiary);
     }

contracts/crowdsale/emission/AllowanceCrowdsale.sol

@@ -21,7 +21,7 @@ contract AllowanceCrowdsale is Crowdsale {
      * @param tokenWallet Address holding the tokens, which has approved allowance to the crowdsale.
      */
     constructor (address tokenWallet) public {
-        require(tokenWallet != address(0));
+        require(tokenWallet != address(0), "AllowanceCrowdsale: token wallet is the zero address");
         _tokenWallet = tokenWallet;
     }
 

contracts/crowdsale/emission/MintedCrowdsale.sol

@@ -16,6 +16,9 @@ contract MintedCrowdsale is Crowdsale {
      */
     function _deliverTokens(address beneficiary, uint256 tokenAmount) internal {
         // Potentially dangerous assumption about the type of the token.
-        require(ERC20Mintable(address(token())).mint(beneficiary, tokenAmount));
+        require(
+            ERC20Mintable(address(token())).mint(beneficiary, tokenAmount),
+                "MintedCrowdsale: minting failed"
+        );
     }
 }

contracts/crowdsale/price/IncreasingPriceCrowdsale.sol

@@ -21,8 +21,9 @@ contract IncreasingPriceCrowdsale is TimedCrowdsale {
      * @param finalRate Number of tokens a buyer gets per wei at the end of the crowdsale
      */
     constructor (uint256 initialRate, uint256 finalRate) public {
-        require(finalRate > 0);
-        require(initialRate > finalRate);
+        require(finalRate > 0, "IncreasingPriceCrowdsale: final rate is 0");
+        // solhint-disable-next-line max-line-length
+        require(initialRate > finalRate, "IncreasingPriceCrowdsale: initial rate is not greater than final rate");
         _initialRate = initialRate;
         _finalRate = finalRate;
     }
@@ -32,7 +33,7 @@ contract IncreasingPriceCrowdsale is TimedCrowdsale {
      * all calls to it are a mistake.
      */
     function rate() public view returns (uint256) {
-        revert();
+        revert("IncreasingPriceCrowdsale: rate() called");
     }
 
     /**

contracts/crowdsale/validation/CappedCrowdsale.sol

@@ -17,7 +17,7 @@ contract CappedCrowdsale is Crowdsale {
      * @param cap Max amount of wei to be contributed
      */
     constructor (uint256 cap) public {
-        require(cap > 0);
+        require(cap > 0, "CappedCrowdsale: cap is 0");
         _cap = cap;
     }
 
@@ -43,6 +43,6 @@ contract CappedCrowdsale is Crowdsale {
      */
     function _preValidatePurchase(address beneficiary, uint256 weiAmount) internal view {
         super._preValidatePurchase(beneficiary, weiAmount);
-        require(weiRaised().add(weiAmount) <= _cap);
+        require(weiRaised().add(weiAmount) <= _cap, "CappedCrowdsale: cap exceeded");
     }
 }

contracts/crowdsale/validation/IndividuallyCappedCrowdsale.sol

@@ -48,7 +48,8 @@ contract IndividuallyCappedCrowdsale is Crowdsale, CapperRole {
      */
     function _preValidatePurchase(address beneficiary, uint256 weiAmount) internal view {
         super._preValidatePurchase(beneficiary, weiAmount);
-        require(_contributions[beneficiary].add(weiAmount) <= _caps[beneficiary]);
+        // solhint-disable-next-line max-line-length
+        require(_contributions[beneficiary].add(weiAmount) <= _caps[beneficiary], "IndividuallyCappedCrowdsale: beneficiary's cap exceeded");
     }
 
     /**

contracts/crowdsale/validation/TimedCrowdsale.sol

@@ -24,7 +24,7 @@ contract TimedCrowdsale is Crowdsale {
      * @dev Reverts if not in crowdsale time range.
      */
     modifier onlyWhileOpen {
-        require(isOpen());
+        require(isOpen(), "TimedCrowdsale: not open");
         _;
     }
 
@@ -35,8 +35,9 @@ contract TimedCrowdsale is Crowdsale {
      */
     constructor (uint256 openingTime, uint256 closingTime) public {
         // solhint-disable-next-line not-rely-on-time
-        require(openingTime >= block.timestamp);
-        require(closingTime > openingTime);
+        require(openingTime >= block.timestamp, "TimedCrowdsale: opening time is before current time");
+        // solhint-disable-next-line max-line-length
+        require(closingTime > openingTime, "TimedCrowdsale: opening time is not before closing time");
 
         _openingTime = openingTime;
         _closingTime = closingTime;
@@ -87,8 +88,9 @@ contract TimedCrowdsale is Crowdsale {
      * @param newClosingTime Crowdsale closing time
      */
     function _extendTime(uint256 newClosingTime) internal {
-        require(!hasClosed());
-        require(newClosingTime > _closingTime);
+        require(!hasClosed(), "TimedCrowdsale: already closed");
+        // solhint-disable-next-line max-line-length
+        require(newClosingTime > _closingTime, "TimedCrowdsale: new closing time is before current closing time");
 
         emit TimedCrowdsaleExtended(_closingTime, newClosingTime);
         _closingTime = newClosingTime;

contracts/crowdsale/validation/WhitelistCrowdsale.sol

@@ -15,7 +15,7 @@ contract WhitelistCrowdsale is WhitelistedRole, Crowdsale {
      * @param _weiAmount Amount of wei contributed
      */
     function _preValidatePurchase(address _beneficiary, uint256 _weiAmount) internal view {
-        require(isWhitelisted(_beneficiary));
+        require(isWhitelisted(_beneficiary), "WhitelistCrowdsale: beneficiary doesn't have the Whitelisted role");
         super._preValidatePurchase(_beneficiary, _weiAmount);
     }
 }

contracts/drafts/ERC20Migrator.sol

@@ -44,7 +44,7 @@ contract ERC20Migrator {
      * @param legacyToken address of the old token contract
      */
     constructor (IERC20 legacyToken) public {
-        require(address(legacyToken) != address(0));
+        require(address(legacyToken) != address(0), "ERC20Migrator: legacy token is the zero address");
         _legacyToken = legacyToken;
     }
 
@@ -68,9 +68,10 @@ contract ERC20Migrator {
      * @param newToken_ the token that will be minted
      */
     function beginMigration(ERC20Mintable newToken_) public {
-        require(address(_newToken) == address(0));
-        require(address(newToken_) != address(0));
-        require(newToken_.isMinter(address(this)));
+        require(address(_newToken) == address(0), "ERC20Migrator: migration already started");
+        require(address(newToken_) != address(0), "ERC20Migrator: new token is the zero address");
+        //solhint-disable-next-line max-line-length
+        require(newToken_.isMinter(address(this)), "ERC20Migrator: not a minter for new token");
 
         _newToken = newToken_;
     }
@@ -82,7 +83,7 @@ contract ERC20Migrator {
      * @param amount amount of tokens to be migrated
      */
     function migrate(address account, uint256 amount) public {
-        require(address(_newToken) != address(0));
+        require(address(_newToken) != address(0), "ERC20Migrator: migration not started");
         _legacyToken.safeTransferFrom(account, address(this), amount);
         _newToken.mint(account, amount);
     }

contracts/drafts/ERC20Snapshot.sol

@@ -101,8 +101,9 @@ contract ERC20Snapshot is ERC20 {
     function _valueAt(uint256 snapshotId, Snapshots storage snapshots)
         private view returns (bool, uint256)
     {
-        require(snapshotId > 0);
-        require(snapshotId <= _currentSnapshotId.current());
+        require(snapshotId > 0, "ERC20Snapshot: id is 0");
+        // solhint-disable-next-line max-line-length
+        require(snapshotId <= _currentSnapshotId.current(), "ERC20Snapshot: nonexistent id");
 
         uint256 index = snapshots.ids.findUpperBound(snapshotId);
 

contracts/drafts/SignatureBouncer.sol

@@ -51,23 +51,25 @@ contract SignatureBouncer is SignerRole {
      * @dev Requires that a valid signature of a signer was provided.
      */
     modifier onlyValidSignature(bytes memory signature) {
-        require(_isValidSignature(msg.sender, signature));
+        require(_isValidSignature(msg.sender, signature), "SignatureBouncer: invalid signature for caller");
         _;
     }
 
     /**
      * @dev Requires that a valid signature with a specified method of a signer was provided.
      */
     modifier onlyValidSignatureAndMethod(bytes memory signature) {
-        require(_isValidSignatureAndMethod(msg.sender, signature));
+        // solhint-disable-next-line max-line-length
+        require(_isValidSignatureAndMethod(msg.sender, signature), "SignatureBouncer: invalid signature for caller and method");
         _;
     }
 
     /**
      * @dev Requires that a valid signature with a specified method and params of a signer was provided.
      */
     modifier onlyValidSignatureAndData(bytes memory signature) {
-        require(_isValidSignatureAndData(msg.sender, signature));
+        // solhint-disable-next-line max-line-length
+        require(_isValidSignatureAndData(msg.sender, signature), "SignatureBouncer: invalid signature for caller and data");
         _;
     }
 
@@ -97,7 +99,7 @@ contract SignatureBouncer is SignerRole {
      * @return bool
      */
     function _isValidSignatureAndData(address account, bytes memory signature) internal view returns (bool) {
-        require(msg.data.length > _SIGNATURE_SIZE);
+        require(msg.data.length > _SIGNATURE_SIZE, "SignatureBouncer: data is too short");
 
         bytes memory data = new bytes(msg.data.length - _SIGNATURE_SIZE);
         for (uint i = 0; i < data.length; i++) {

contracts/drafts/SignedSafeMath.sol

@@ -18,10 +18,10 @@ library SignedSafeMath {
             return 0;
         }
 
-        require(!(a == -1 && b == INT256_MIN)); // This is the only case of overflow not detected by the check below
+        require(!(a == -1 && b == INT256_MIN), "SignedSafeMath: multiplication overflow");
 
         int256 c = a * b;
-        require(c / a == b);
+        require(c / a == b, "SignedSafeMath: multiplication overflow");
 
         return c;
     }
@@ -30,8 +30,8 @@ library SignedSafeMath {
      * @dev Integer division of two signed integers truncating the quotient, reverts on division by zero.
      */
     function div(int256 a, int256 b) internal pure returns (int256) {
-        require(b != 0); // Solidity only automatically asserts when dividing by 0
-        require(!(b == -1 && a == INT256_MIN)); // This is the only case of overflow
+        require(b != 0, "SignedSafeMath: division by zero");
+        require(!(b == -1 && a == INT256_MIN), "SignedSafeMath: division overflow");
 
         int256 c = a / b;
 
@@ -43,7 +43,7 @@ library SignedSafeMath {
      */
     function sub(int256 a, int256 b) internal pure returns (int256) {
         int256 c = a - b;
-        require((b >= 0 && c <= a) || (b < 0 && c > a));
+        require((b >= 0 && c <= a) || (b < 0 && c > a), "SignedSafeMath: subtraction overflow");
 
         return c;
     }
@@ -53,7 +53,7 @@ library SignedSafeMath {
      */
     function add(int256 a, int256 b) internal pure returns (int256) {
         int256 c = a + b;
-        require((b >= 0 && c >= a) || (b < 0 && c < a));
+        require((b >= 0 && c >= a) || (b < 0 && c < a), "SignedSafeMath: addition overflow");
 
         return c;
     }