Pārlūkot izejas kodu

re-enabling safemath revert reasons in ERC20, ERC777 and ERC1155 (#2491)

* re-enabling safemath revert reasons in ERC20 and ERC1155

* adding revert messages to ERC777

* removing uncheck block
Hadrien Croubois 4 gadi atpakaļ
vecāks
revīzija
5db7413827

+ 15 - 8
contracts/token/ERC1155/ERC1155.sol

@@ -137,8 +137,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
 
         _beforeTokenTransfer(operator, from, to, _asSingletonArray(id), _asSingletonArray(amount), data);
 
-        _balances[id][from] = _balances[id][from] - amount;
-        _balances[id][to] = _balances[id][to] + amount;
+        require(_balances[id][from] >= amount, "ERC1155: insufficient balance for transfer");
+        _balances[id][from] -= amount;
+        _balances[id][to] += amount;
 
         emit TransferSingle(operator, from, to, id, amount);
 
@@ -174,8 +175,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
             uint256 id = ids[i];
             uint256 amount = amounts[i];
 
-            _balances[id][from] = _balances[id][from] - amount;
-            _balances[id][to] = _balances[id][to] + amount;
+            require(_balances[id][from] >= amount, "ERC1155: insufficient balance for transfer");
+            _balances[id][from] -= amount;
+            _balances[id][to] += amount;
         }
 
         emit TransferBatch(operator, from, to, ids, amounts);
@@ -224,7 +226,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
 
         _beforeTokenTransfer(operator, address(0), account, _asSingletonArray(id), _asSingletonArray(amount), data);
 
-        _balances[id][account] = _balances[id][account] + amount;
+        _balances[id][account] += amount;
         emit TransferSingle(operator, address(0), account, id, amount);
 
         _doSafeTransferAcceptanceCheck(operator, address(0), account, id, amount, data);
@@ -248,7 +250,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
         _beforeTokenTransfer(operator, address(0), to, ids, amounts, data);
 
         for (uint i = 0; i < ids.length; i++) {
-            _balances[ids[i]][to] = amounts[i] + _balances[ids[i]][to];
+            _balances[ids[i]][to] += amounts[i];
         }
 
         emit TransferBatch(operator, address(0), to, ids, amounts);
@@ -271,7 +273,8 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
 
         _beforeTokenTransfer(operator, account, address(0), _asSingletonArray(id), _asSingletonArray(amount), "");
 
-        _balances[id][account] = _balances[id][account] - amount;
+        require(_balances[id][account] >= amount, "ERC1155: burn amount exceeds balance");
+        _balances[id][account] -= amount;
 
         emit TransferSingle(operator, account, address(0), id, amount);
     }
@@ -292,7 +295,11 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
         _beforeTokenTransfer(operator, account, address(0), ids, amounts, "");
 
         for (uint i = 0; i < ids.length; i++) {
-            _balances[ids[i]][account] = _balances[ids[i]][account] - amounts[i];
+            uint256 id = ids[i];
+            uint256 amount = amounts[i];
+
+            require(_balances[id][account] >= amount, "ERC1155: burn amount exceeds balance");
+            _balances[id][account] -= amount;
         }
 
         emit TransferBatch(operator, account, address(0), ids, amounts);

+ 15 - 6
contracts/token/ERC20/ERC20.sol

@@ -148,7 +148,10 @@ contract ERC20 is Context, IERC20 {
      */
     function transferFrom(address sender, address recipient, uint256 amount) public virtual override returns (bool) {
         _transfer(sender, recipient, amount);
+
+        require(_allowances[sender][_msgSender()] >= amount, "ERC20: transfer amount exceeds allowance");
         _approve(sender, _msgSender(), _allowances[sender][_msgSender()] - amount);
+
         return true;
     }
 
@@ -184,7 +187,9 @@ contract ERC20 is Context, IERC20 {
      * `subtractedValue`.
      */
     function decreaseAllowance(address spender, uint256 subtractedValue) public virtual returns (bool) {
+        require(_allowances[_msgSender()][spender] >= subtractedValue, "ERC20: decreased allowance below zero");
         _approve(_msgSender(), spender, _allowances[_msgSender()][spender] - subtractedValue);
+
         return true;
     }
 
@@ -208,8 +213,10 @@ contract ERC20 is Context, IERC20 {
 
         _beforeTokenTransfer(sender, recipient, amount);
 
-        _balances[sender] = _balances[sender] - amount;
-        _balances[recipient] = _balances[recipient] + amount;
+        require(_balances[sender] >= amount, "ERC20: transfer amount exceeds balance");
+        _balances[sender] -= amount;
+        _balances[recipient] += amount;
+
         emit Transfer(sender, recipient, amount);
     }
 
@@ -227,8 +234,8 @@ contract ERC20 is Context, IERC20 {
 
         _beforeTokenTransfer(address(0), account, amount);
 
-        _totalSupply = _totalSupply + amount;
-        _balances[account] = _balances[account] + amount;
+        _totalSupply += amount;
+        _balances[account] += amount;
         emit Transfer(address(0), account, amount);
     }
 
@@ -248,8 +255,10 @@ contract ERC20 is Context, IERC20 {
 
         _beforeTokenTransfer(account, address(0), amount);
 
-        _balances[account] = _balances[account] - amount;
-        _totalSupply = _totalSupply - amount;
+        require(_balances[account] >= amount, "ERC20: burn amount exceeds balance");
+        _balances[account] -= amount;
+        _totalSupply -= amount;
+
         emit Transfer(account, address(0), amount);
     }
 

+ 2 - 3
contracts/token/ERC20/ERC20Burnable.sol

@@ -32,9 +32,8 @@ abstract contract ERC20Burnable is Context, ERC20 {
      * `amount`.
      */
     function burnFrom(address account, uint256 amount) public virtual {
-        uint256 decreasedAllowance = allowance(account, _msgSender()) - amount;
-
-        _approve(account, _msgSender(), decreasedAllowance);
+        require(allowance(account, _msgSender()) >= amount, "ERC20: burn amount exceeds allowance");
+        _approve(account, _msgSender(), allowance(account, _msgSender()) - amount);
         _burn(account, amount);
     }
 }

+ 6 - 2
contracts/token/ERC20/SafeERC20.sol

@@ -49,8 +49,12 @@ library SafeERC20 {
     }
 
     function safeDecreaseAllowance(IERC20 token, address spender, uint256 value) internal {
-        uint256 newAllowance = token.allowance(address(this), spender) - value;
-        _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
+        unchecked {
+            uint256 oldAllowance = token.allowance(address(this), spender);
+            require(oldAllowance >= value, "SafeERC20: decreased allowance below zero");
+            uint256 newAllowance = oldAllowance - value;
+            _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance));
+        }
     }
 
     /**

+ 10 - 6
contracts/token/ERC777/ERC777.sol

@@ -277,6 +277,8 @@ contract ERC777 is Context, IERC777, IERC20 {
         _callTokensToSend(spender, holder, recipient, amount, "", "");
 
         _move(spender, holder, recipient, amount, "", "");
+
+        require(_allowances[holder][spender] >= amount, "ERC777: transfer amount exceeds allowance");
         _approve(holder, spender, _allowances[holder][spender] - amount);
 
         _callTokensReceived(spender, holder, recipient, amount, "", "", false);
@@ -317,8 +319,8 @@ contract ERC777 is Context, IERC777, IERC20 {
         _beforeTokenTransfer(operator, address(0), account, amount);
 
         // Update state variables
-        _totalSupply = _totalSupply + amount;
-        _balances[account] = _balances[account] + amount;
+        _totalSupply += amount;
+        _balances[account] += amount;
 
         _callTokensReceived(operator, address(0), account, amount, userData, operatorData, true);
 
@@ -383,8 +385,9 @@ contract ERC777 is Context, IERC777, IERC20 {
         _beforeTokenTransfer(operator, from, address(0), amount);
 
         // Update state variables
-        _balances[from] = _balances[from] - amount;
-        _totalSupply = _totalSupply - amount;
+        require(_balances[from] >= amount, "ERC777: burn amount exceeds balance");
+        _balances[from] -= amount;
+        _totalSupply -= amount;
 
         emit Burned(operator, from, amount, data, operatorData);
         emit Transfer(from, address(0), amount);
@@ -402,8 +405,9 @@ contract ERC777 is Context, IERC777, IERC20 {
     {
         _beforeTokenTransfer(operator, from, to, amount);
 
-        _balances[from] = _balances[from] - amount;
-        _balances[to] = _balances[to] + amount;
+        require(_balances[from] >= amount, "ERC777: transfer amount exceeds balance");
+        _balances[from] -= amount;
+        _balances[to] += amount;
 
         emit Sent(operator, from, to, amount, userData, operatorData);
         emit Transfer(from, to, amount);

+ 4 - 2
test/token/ERC1155/ERC1155.behavior.js

@@ -208,7 +208,7 @@ function shouldBehaveLikeERC1155 ([minter, firstTokenHolder, secondTokenHolder,
       });
 
       it('reverts when transferring more than balance', async function () {
-        await expectRevert.unspecified(
+        await expectRevert(
           this.token.safeTransferFrom(
             multiTokenHolder,
             recipient,
@@ -217,6 +217,7 @@ function shouldBehaveLikeERC1155 ([minter, firstTokenHolder, secondTokenHolder,
             '0x',
             { from: multiTokenHolder },
           ),
+          'ERC1155: insufficient balance for transfer',
         );
       });
 
@@ -463,13 +464,14 @@ function shouldBehaveLikeERC1155 ([minter, firstTokenHolder, secondTokenHolder,
       });
 
       it('reverts when transferring amount more than any of balances', async function () {
-        await expectRevert.unspecified(
+        await expectRevert(
           this.token.safeBatchTransferFrom(
             multiTokenHolder, recipient,
             [firstTokenId, secondTokenId],
             [firstAmount, secondAmount.addn(1)],
             '0x', { from: multiTokenHolder },
           ),
+          'ERC1155: insufficient balance for transfer',
         );
       });
 

+ 6 - 3
test/token/ERC1155/ERC1155.test.js

@@ -118,8 +118,9 @@ contract('ERC1155', function (accounts) {
       });
 
       it('reverts when burning a non-existent token id', async function () {
-        await expectRevert.unspecified(
+        await expectRevert(
           this.token.burn(tokenHolder, tokenId, mintAmount),
+          'ERC1155: burn amount exceeds balance',
         );
       });
 
@@ -132,8 +133,9 @@ contract('ERC1155', function (accounts) {
           { from: operator },
         );
 
-        await expectRevert.unspecified(
+        await expectRevert(
           this.token.burn(tokenHolder, tokenId, mintAmount.addn(1)),
+          'ERC1155: burn amount exceeds balance',
         );
       });
 
@@ -188,8 +190,9 @@ contract('ERC1155', function (accounts) {
       });
 
       it('reverts when burning a non-existent token id', async function () {
-        await expectRevert.unspecified(
+        await expectRevert(
           this.token.burnBatch(tokenBatchHolder, tokenBatchIds, burnAmounts),
+          'ERC1155: burn amount exceeds balance',
         );
       });
 

+ 8 - 8
test/token/ERC20/ERC20.behavior.js

@@ -87,8 +87,8 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
             const amount = initialSupply.addn(1);
 
             it('reverts', async function () {
-              await expectRevert.unspecified(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }),
+              await expectRevert(this.token.transferFrom(
+                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds balance`,
               );
             });
           });
@@ -103,8 +103,8 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
             const amount = initialSupply;
 
             it('reverts', async function () {
-              await expectRevert.unspecified(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }),
+              await expectRevert(this.token.transferFrom(
+                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds allowance`,
               );
             });
           });
@@ -113,8 +113,8 @@ function shouldBehaveLikeERC20 (errorPrefix, initialSupply, initialHolder, recip
             const amount = initialSupply.addn(1);
 
             it('reverts', async function () {
-              await expectRevert.unspecified(this.token.transferFrom(
-                tokenOwner, to, amount, { from: spender }),
+              await expectRevert(this.token.transferFrom(
+                tokenOwner, to, amount, { from: spender }), `${errorPrefix}: transfer amount exceeds balance`,
               );
             });
           });
@@ -165,8 +165,8 @@ function shouldBehaveLikeERC20Transfer (errorPrefix, from, to, balance, transfer
       const amount = balance.addn(1);
 
       it('reverts', async function () {
-        await expectRevert.unspecified(transfer.call(this, from, to, amount),
-
+        await expectRevert(transfer.call(this, from, to, amount),
+          `${errorPrefix}: transfer amount exceeds balance`,
         );
       });
     });

+ 8 - 8
test/token/ERC20/ERC20.test.js

@@ -53,8 +53,8 @@ contract('ERC20', function (accounts) {
       function shouldDecreaseApproval (amount) {
         describe('when there was no approved amount before', function () {
           it('reverts', async function () {
-            await expectRevert.unspecified(this.token.decreaseAllowance(
-              spender, amount, { from: initialHolder }),
+            await expectRevert(this.token.decreaseAllowance(
+              spender, amount, { from: initialHolder }), 'ERC20: decreased allowance below zero',
             );
           });
         });
@@ -88,9 +88,9 @@ contract('ERC20', function (accounts) {
           });
 
           it('reverts when more than the full allowance is removed', async function () {
-            await expectRevert.unspecified(
+            await expectRevert(
               this.token.decreaseAllowance(spender, approvedAmount.addn(1), { from: initialHolder }),
-
+              'ERC20: decreased allowance below zero',
             );
           });
         });
@@ -114,8 +114,8 @@ contract('ERC20', function (accounts) {
       const spender = ZERO_ADDRESS;
 
       it('reverts', async function () {
-        await expectRevert.unspecified(this.token.decreaseAllowance(
-          spender, amount, { from: initialHolder }),
+        await expectRevert(this.token.decreaseAllowance(
+          spender, amount, { from: initialHolder }), 'ERC20: decreased allowance below zero',
         );
       });
     });
@@ -247,8 +247,8 @@ contract('ERC20', function (accounts) {
 
     describe('for a non zero account', function () {
       it('rejects burning more than balance', async function () {
-        await expectRevert.unspecified(this.token.burn(
-          initialHolder, initialSupply.addn(1)),
+        await expectRevert(this.token.burn(
+          initialHolder, initialSupply.addn(1)), 'ERC20: burn amount exceeds balance',
         );
       });
 

+ 4 - 3
test/token/ERC20/SafeERC20.test.js

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

+ 6 - 6
test/token/ERC20/behaviors/ERC20Burnable.behavior.js

@@ -37,8 +37,8 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) {
       const amount = initialBalance.addn(1);
 
       it('reverts', async function () {
-        await expectRevert.unspecified(this.token.burn(amount, { from: owner }),
-
+        await expectRevert(this.token.burn(amount, { from: owner }),
+          'ERC20: burn amount exceeds balance',
         );
       });
     });
@@ -86,8 +86,8 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) {
 
       it('reverts', async function () {
         await this.token.approve(burner, amount, { from: owner });
-        await expectRevert.unspecified(this.token.burnFrom(owner, amount, { from: burner }),
-
+        await expectRevert(this.token.burnFrom(owner, amount, { from: burner }),
+          'ERC20: burn amount exceeds balance',
         );
       });
     });
@@ -97,8 +97,8 @@ function shouldBehaveLikeERC20Burnable (owner, initialBalance, [burner]) {
 
       it('reverts', async function () {
         await this.token.approve(burner, allowance, { from: owner });
-        await expectRevert.unspecified(this.token.burnFrom(owner, allowance.addn(1), { from: burner }),
-
+        await expectRevert(this.token.burnFrom(owner, allowance.addn(1), { from: burner }),
+          'ERC20: burn amount exceeds allowance',
         );
       });
     });