Remove double sload when doing math checks in tokens (#2506)

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
OpenZeppelin · Feb 9, 2021 · a44303c · a44303c
1 parent f49e9ee
commit a44303c
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 24 deletions.
diff --git a/contracts/token/ERC1155/ERC1155.sol b/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);
 
-        require(_balances[id][from] >= amount, "ERC1155: insufficient balance for transfer");
-        _balances[id][from] -= amount;
+        uint256 fromBalance = _balances[id][from];
+        require(fromBalance >= amount, "ERC1155: insufficient balance for transfer");
+        _balances[id][from] = fromBalance - amount;
         _balances[id][to] += amount;
 
         emit TransferSingle(operator, from, to, id, amount);
@@ -175,8 +176,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
             uint256 id = ids[i];
             uint256 amount = amounts[i];
 
-            require(_balances[id][from] >= amount, "ERC1155: insufficient balance for transfer");
-            _balances[id][from] -= amount;
+            uint256 fromBalance = _balances[id][from];
+            require(fromBalance >= amount, "ERC1155: insufficient balance for transfer");
+            _balances[id][from] = fromBalance - amount;
             _balances[id][to] += amount;
         }
 
@@ -273,8 +275,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
 
         _beforeTokenTransfer(operator, account, address(0), _asSingletonArray(id), _asSingletonArray(amount), "");
 
-        require(_balances[id][account] >= amount, "ERC1155: burn amount exceeds balance");
-        _balances[id][account] -= amount;
+        uint256 accountBalance = _balances[id][account];
+        require(accountBalance >= amount, "ERC1155: burn amount exceeds balance");
+        _balances[id][account] = accountBalance - amount;
 
         emit TransferSingle(operator, account, address(0), id, amount);
     }
@@ -298,8 +301,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
             uint256 id = ids[i];
             uint256 amount = amounts[i];
 
-            require(_balances[id][account] >= amount, "ERC1155: burn amount exceeds balance");
-            _balances[id][account] -= amount;
+            uint256 accountBalance = _balances[id][account];
+            require(accountBalance >= amount, "ERC1155: burn amount exceeds balance");
+            _balances[id][account] = accountBalance - amount;
         }
 
         emit TransferBatch(operator, account, address(0), ids, amounts);

diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol
@@ -147,8 +147,9 @@ 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);
+        uint256 currentAllowance = _allowances[sender][_msgSender()];
+        require(currentAllowance >= amount, "ERC20: transfer amount exceeds allowance");
+        _approve(sender, _msgSender(), currentAllowance - amount);
 
         return true;
     }
@@ -185,8 +186,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);
+        uint256 currentAllowance = _allowances[_msgSender()][spender];
+        require(currentAllowance >= subtractedValue, "ERC20: decreased allowance below zero");
+        _approve(_msgSender(), spender, currentAllowance - subtractedValue);
 
         return true;
     }
@@ -211,8 +213,9 @@ contract ERC20 is Context, IERC20 {
 
         _beforeTokenTransfer(sender, recipient, amount);
 
-        require(_balances[sender] >= amount, "ERC20: transfer amount exceeds balance");
-        _balances[sender] -= amount;
+        uint256 senderBalance = _balances[sender];
+        require(senderBalance >= amount, "ERC20: transfer amount exceeds balance");
+        _balances[sender] = senderBalance - amount;
         _balances[recipient] += amount;
 
         emit Transfer(sender, recipient, amount);
@@ -253,8 +256,9 @@ contract ERC20 is Context, IERC20 {
 
         _beforeTokenTransfer(account, address(0), amount);
 
-        require(_balances[account] >= amount, "ERC20: burn amount exceeds balance");
-        _balances[account] -= amount;
+        uint256 accountBalance = _balances[account];
+        require(accountBalance >= amount, "ERC20: burn amount exceeds balance");
+        _balances[account] = accountBalance - amount;
         _totalSupply -= amount;
 
         emit Transfer(account, address(0), amount);

diff --git a/contracts/token/ERC20/ERC20Burnable.sol b/contracts/token/ERC20/ERC20Burnable.sol
@@ -32,8 +32,9 @@ abstract contract ERC20Burnable is Context, ERC20 {
      * `amount`.
      */
     function burnFrom(address account, uint256 amount) public virtual {
-        require(allowance(account, _msgSender()) >= amount, "ERC20: burn amount exceeds allowance");
-        _approve(account, _msgSender(), allowance(account, _msgSender()) - amount);
+        uint256 currentAllowance = allowance(account, _msgSender());
+        require(currentAllowance >= amount, "ERC20: burn amount exceeds allowance");
+        _approve(account, _msgSender(), currentAllowance - amount);
         _burn(account, amount);
     }
 }
diff --git a/contracts/token/ERC777/ERC777.sol b/contracts/token/ERC777/ERC777.sol
@@ -278,8 +278,9 @@ contract ERC777 is Context, IERC777, IERC20 {
 
         _move(spender, holder, recipient, amount, "", "");
 
-        require(_allowances[holder][spender] >= amount, "ERC777: transfer amount exceeds allowance");
-        _approve(holder, spender, _allowances[holder][spender] - amount);
+        uint256 currentAllowance = _allowances[holder][spender];
+        require(currentAllowance >= amount, "ERC777: transfer amount exceeds allowance");
+        _approve(holder, spender, currentAllowance - amount);
 
         _callTokensReceived(spender, holder, recipient, amount, "", "", false);
 
@@ -385,8 +386,9 @@ contract ERC777 is Context, IERC777, IERC20 {
         _beforeTokenTransfer(operator, from, address(0), amount);
 
         // Update state variables
-        require(_balances[from] >= amount, "ERC777: burn amount exceeds balance");
-        _balances[from] -= amount;
+        uint256 fromBalance = _balances[from];
+        require(fromBalance >= amount, "ERC777: burn amount exceeds balance");
+        _balances[from] = fromBalance - amount;
         _totalSupply -= amount;
 
         emit Burned(operator, from, amount, data, operatorData);
@@ -405,8 +407,9 @@ contract ERC777 is Context, IERC777, IERC20 {
     {
         _beforeTokenTransfer(operator, from, to, amount);
 
-        require(_balances[from] >= amount, "ERC777: transfer amount exceeds balance");
-        _balances[from] -= amount;
+        uint256 fromBalance = _balances[from];
+        require(fromBalance >= amount, "ERC777: transfer amount exceeds balance");
+        _balances[from] = fromBalance - amount;
         _balances[to] += amount;
 
         emit Sent(operator, from, to, amount, userData, operatorData);