Functions in SafeMath contract overloaded to accept custom error messages (#1828)

* Imporvement: functions in SafeMath contract overloaded to accept custom error messages.

* CHANGELOG updated, custom error messages added to ERC20, ERC721 and ERC777 for subtraction related exceptions.

* SafeMath overloads for 'add' and 'mul' removed.

* Error messages modified.

* Update CHANGELOG.md

CHANGELOG.md

@@ -7,6 +7,7 @@
 
 ### Improvements:
  * `Address.isContract`: switched from `extcodesize` to `extcodehash` for less gas usage. ([#1802](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1802))
+ * `SafeMath`: added custom error messages support for `sub`, `div` and `mod` functions. `ERC20` and `ERC777` updated to throw custom errors on subtraction overflows. ([#1828](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/1828)) 
 
 ### Bugfixes
 

contracts/math/SafeMath.sol

@@ -40,7 +40,20 @@ library SafeMath {
      * - Subtraction cannot overflow.
      */
     function sub(uint256 a, uint256 b) internal pure returns (uint256) {
-        require(b <= a, "SafeMath: subtraction overflow");
+        return sub(a, b, "SafeMath: subtraction overflow");
+    }
+
+    /**
+     * @dev Returns the subtraction of two unsigned integers, reverting with custom message on
+     * overflow (when the result is negative).
+     *
+     * Counterpart to Solidity's `-` operator.
+     *
+     * Requirements:
+     * - Subtraction cannot overflow.
+     */
+    function sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        require(b <= a, errorMessage);
         uint256 c = a - b;
 
         return c;
@@ -81,8 +94,23 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function div(uint256 a, uint256 b) internal pure returns (uint256) {
+        return div(a, b, "SafeMath: division by zero");
+    }
+
+    /**
+     * @dev Returns the integer division of two unsigned integers. Reverts with custom message on
+     * division by zero. The result is rounded towards zero.
+     *
+     * Counterpart to Solidity's `/` operator. Note: this function uses a
+     * `revert` opcode (which leaves remaining gas untouched) while Solidity
+     * uses an invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     * - The divisor cannot be zero.
+     */
+    function div(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
         // Solidity only automatically asserts when dividing by 0
-        require(b > 0, "SafeMath: division by zero");
+        require(b > 0, errorMessage);
         uint256 c = a / b;
         // assert(a == b * c + a % b); // There is no case in which this doesn't hold
 
@@ -101,7 +129,22 @@ library SafeMath {
      * - The divisor cannot be zero.
      */
     function mod(uint256 a, uint256 b) internal pure returns (uint256) {
-        require(b != 0, "SafeMath: modulo by zero");
+        return mod(a, b, "SafeMath: modulo by zero");
+    }
+
+    /**
+     * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
+     * Reverts with custom message when dividing by zero.
+     *
+     * Counterpart to Solidity's `%` operator. This function uses a `revert`
+     * opcode (which leaves remaining gas untouched) while Solidity uses an
+     * invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     * - The divisor cannot be zero.
+     */
+    function mod(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        require(b != 0, errorMessage);
         return a % b;
     }
 }

contracts/token/ERC20/ERC20.sol

@@ -96,7 +96,7 @@ contract ERC20 is IERC20 {
      */
     function transferFrom(address sender, address recipient, uint256 amount) public returns (bool) {
         _transfer(sender, recipient, amount);
-        _approve(sender, msg.sender, _allowances[sender][msg.sender].sub(amount));
+        _approve(sender, msg.sender, _allowances[sender][msg.sender].sub(amount, "ERC20: transfer amount exceeds allowance"));
         return true;
     }
 
@@ -132,7 +132,7 @@ contract ERC20 is IERC20 {
      * `subtractedValue`.
      */
     function decreaseAllowance(address spender, uint256 subtractedValue) public returns (bool) {
-        _approve(msg.sender, spender, _allowances[msg.sender][spender].sub(subtractedValue));
+        _approve(msg.sender, spender, _allowances[msg.sender][spender].sub(subtractedValue, "ERC20: decreased allowance below zero"));
         return true;
     }
 
@@ -154,7 +154,7 @@ contract ERC20 is IERC20 {
         require(sender != address(0), "ERC20: transfer from the zero address");
         require(recipient != address(0), "ERC20: transfer to the zero address");
 
-        _balances[sender] = _balances[sender].sub(amount);
+        _balances[sender] = _balances[sender].sub(amount, "ERC20: transfer amount exceeds balance");
         _balances[recipient] = _balances[recipient].add(amount);
         emit Transfer(sender, recipient, amount);
     }
@@ -190,8 +190,8 @@ contract ERC20 is IERC20 {
     function _burn(address account, uint256 value) internal {
         require(account != address(0), "ERC20: burn from the zero address");
 
+        _balances[account] = _balances[account].sub(value, "ERC20: burn amount exceeds balance");
         _totalSupply = _totalSupply.sub(value);
-        _balances[account] = _balances[account].sub(value);
         emit Transfer(account, address(0), value);
     }
 
@@ -224,6 +224,6 @@ contract ERC20 is IERC20 {
      */
     function _burnFrom(address account, uint256 amount) internal {
         _burn(account, amount);
-        _approve(account, msg.sender, _allowances[account][msg.sender].sub(amount));
+        _approve(account, msg.sender, _allowances[account][msg.sender].sub(amount, "ERC20: burn amount exceeds allowance"));
     }
 }

contracts/token/ERC20/SafeERC20.sol

@@ -42,7 +42,7 @@ library SafeERC20 {
     }
 
     function safeDecreaseAllowance(IERC20 token, address spender, uint256 value) internal {
-        uint256 newAllowance = token.allowance(address(this), spender).sub(value);
+        uint256 newAllowance = token.allowance(address(this), spender).sub(value, "SafeERC20: decreased allowance below zero");
         callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
     }
 

contracts/token/ERC777/ERC777.sol

@@ -285,7 +285,7 @@ contract ERC777 is IERC777, IERC20 {
         _callTokensToSend(spender, holder, recipient, amount, "", "");
 
         _move(spender, holder, recipient, amount, "", "");
-        _approve(holder, spender, _allowances[holder][spender].sub(amount));
+        _approve(holder, spender, _allowances[holder][spender].sub(amount, "ERC777: transfer amount exceeds allowance"));
 
         _callTokensReceived(spender, holder, recipient, amount, "", "", false);
 
@@ -383,8 +383,8 @@ contract ERC777 is IERC777, IERC20 {
         _callTokensToSend(operator, from, address(0), amount, data, operatorData);
 
         // Update state variables
+        _balances[from] = _balances[from].sub(amount, "ERC777: burn amount exceeds balance");
         _totalSupply = _totalSupply.sub(amount);
-        _balances[from] = _balances[from].sub(amount);
 
         emit Burned(operator, from, amount, data, operatorData);
         emit Transfer(from, address(0), amount);
@@ -400,7 +400,7 @@ contract ERC777 is IERC777, IERC20 {
     )
         private
     {
-        _balances[from] = _balances[from].sub(amount);
+        _balances[from] = _balances[from].sub(amount, "ERC777: transfer amount exceeds balance");
         _balances[to] = _balances[to].add(amount);
 
         emit Sent(operator, from, to, amount, userData, operatorData);

test/token/ERC20/ERC20.behavior.js

@@ -88,7 +88,7 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
 
             it('reverts', async function () {
               await expectRevert(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
+                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds balance`
               );
             });
           });
@@ -104,7 +104,7 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
 
             it('reverts', async function () {
               await expectRevert(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
+                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds allowance`
               );
             });
           });
@@ -114,7 +114,7 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
 
             it('reverts', async function () {
               await expectRevert(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }), 'SafeMath: subtraction overflow'
+                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds balance`
               );
             });
           });
@@ -166,7 +166,7 @@ function shouldBehaveLikeERC20Transfer (errorPrefix, from, to, balance, transfer
 
       it('reverts', async function () {
         await expectRevert(transfer.call(this, from, to, amount),
-          'SafeMath: subtraction overflow'
+          `${errorPrefix}: transfer amount exceeds balance`
         );
       });
     });

test/token/ERC20/ERC20.test.js

@@ -27,7 +27,7 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) {
         describe('when there was no approved amount before', function () {
           it('reverts', async function () {
             await expectRevert(this.token.decreaseAllowance(
-              spender, amount, { from: initialHolder }), 'SafeMath: subtraction overflow'
+              spender, amount, { from: initialHolder }), 'ERC20: decreased allowance below zero'
             );
           });
         });
@@ -63,7 +63,7 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) {
           it('reverts when more than the full allowance is removed', async function () {
             await expectRevert(
               this.token.decreaseAllowance(spender, approvedAmount.addn(1), { from: initialHolder }),
-              'SafeMath: subtraction overflow'
+              'ERC20: decreased allowance below zero'
             );
           });
         });
@@ -88,7 +88,7 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) {
 
       it('reverts', async function () {
         await expectRevert(this.token.decreaseAllowance(
-          spender, amount, { from: initialHolder }), 'SafeMath: subtraction overflow'
+          spender, amount, { from: initialHolder }), 'ERC20: decreased allowance below zero'
         );
       });
     });
@@ -221,7 +221,7 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) {
     describe('for a non zero account', function () {
       it('rejects burning more than balance', async function () {
         await expectRevert(this.token.burn(
-          initialHolder, initialSupply.addn(1)), 'SafeMath: subtraction overflow'
+          initialHolder, initialSupply.addn(1)), 'ERC20: burn amount exceeds balance'
         );
       });
 
@@ -276,13 +276,13 @@ contract('ERC20', function ([_, initialHolder, recipient, anotherAccount]) {
     describe('for a non zero account', function () {
       it('rejects burning more than allowance', async function () {
         await expectRevert(this.token.burnFrom(initialHolder, allowance.addn(1)),
-          'SafeMath: subtraction overflow'
+          'ERC20: burn amount exceeds allowance'
         );
       });
 
       it('rejects burning more than balance', async function () {
         await expectRevert(this.token.burnFrom(initialHolder, initialSupply.addn(1)),
-          'SafeMath: subtraction overflow'
+          'ERC20: burn amount exceeds balance'
         );
       });
 

test/token/ERC20/SafeERC20.test.js

@@ -93,7 +93,7 @@ function shouldOnlyRevertOnErrors () {
       it('reverts when decreasing the allowance', async function () {
         await expectRevert(
           this.wrapper.decreaseAllowance(10),
-          'SafeMath: subtraction overflow'
+          'SafeERC20: decreased allowance below zero'
         );
       });
     });
@@ -125,7 +125,7 @@ function shouldOnlyRevertOnErrors () {
       it('reverts when decreasing the allowance to a negative value', async function () {
         await expectRevert(
           this.wrapper.decreaseAllowance(200),
-          'SafeMath: subtraction overflow'
+          'SafeERC20: decreased allowance below zero'
         );
       });
     });

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

@@ -38,7 +38,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) {
 
       it('reverts', async function () {
         await expectRevert(this.token.burn(amount, { from: owner }),
-          'SafeMath: subtraction overflow'
+          'ERC20: burn amount exceeds balance'
         );
       });
     });
@@ -87,7 +87,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) {
       it('reverts', async function () {
         await this.token.approve(burner, amount, { from: owner });
         await expectRevert(this.token.burnFrom(owner, amount, { from: burner }),
-          'SafeMath: subtraction overflow'
+          'ERC20: burn amount exceeds balance'
         );
       });
     });
@@ -98,7 +98,7 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) {
       it('reverts', async function () {
         await this.token.approve(burner, allowance, { from: owner });
         await expectRevert(this.token.burnFrom(owner, allowance.addn(1), { from: burner }),
-          'SafeMath: subtraction overflow'
+          'ERC20: burn amount exceeds allowance'
         );
       });
     });