C4 token fixes

brownie-config.yaml

@@ -10,7 +10,7 @@ compiler:
   solc:
     version: 0.8.7
     remappings:
-      - "@openzeppelin=OpenZeppelin/[email protected].2"
+      - "@openzeppelin=OpenZeppelin/[email protected].3"
     optimizer:
       enabled: true
       runs: 400
@@ -21,4 +21,4 @@ compiler:
 
 autofetch_sources: True
 dependencies:
-  - OpenZeppelin/[email protected].2
+  - OpenZeppelin/[email protected].3

contracts/OverlayToken.sol

@@ -20,20 +20,30 @@ contract OverlayToken is AccessControlEnumerable, ERC20("Overlay", "OVL") {
   }
 
   modifier onlyMinter() {
-    require(hasRole(MINTER_ROLE, msg.sender), "only minter");
+    require(hasRole(MINTER_ROLE, msg.sender), "ERC20: !minter");
     _;
   }
 
   modifier onlyBurner() {
-    require(hasRole(BURNER_ROLE, msg.sender), "only burner");
+    require(hasRole(BURNER_ROLE, msg.sender), "ERC20: !burner");
     _;
   }
 
   function mint(address _recipient, uint256 _amount) external onlyMinter {
       _mint(_recipient, _amount);
   }
 
-  function burn(address _account, uint256 _amount) external onlyBurner {
+  function burn(uint256 _amount) external onlyBurner {
+      _burn(msg.sender, _amount);
+  }
+
+  // See: OpenZeppelin Contracts v4.4.0 (token/ERC20/extensions/ERC20Burnable.sol)
+  function burnFrom(address _account, uint256 _amount) external onlyBurner {
+      uint256 _currentAllowance = allowance(_account, msg.sender);
+      require(_currentAllowance >= _amount, "ERC20: burn amount exceeds allowance");
+      unchecked {
+          _approve(_account, msg.sender, _currentAllowance - _amount);
+      }
       _burn(_account, _amount);
   }
 }

contracts/collateral/OverlayV1OVLCollateral.sol

@@ -8,7 +8,6 @@ import "@openzeppelin/contracts/token/ERC1155/extensions/ERC1155Supply.sol";
 import "../interfaces/IOverlayV1Market.sol";
 import "../interfaces/IOverlayV1Mothership.sol";
 import "../interfaces/IOverlayToken.sol";
-import "../interfaces/IOverlayTokenNew.sol";
 
 contract OverlayV1OVLCollateral is ERC1155Supply {
 
@@ -32,7 +31,7 @@ contract OverlayV1OVLCollateral is ERC1155Supply {
     Position.Info[] public positions;
 
     IOverlayV1Mothership public immutable mothership;
-    IOverlayTokenNew immutable public ovl;
+    IOverlayToken immutable public ovl;
 
     uint256 public fees;
     uint256 public liquidations;
@@ -178,7 +177,7 @@ contract OverlayV1OVLCollateral is ERC1155Supply {
             _liqBurn
         );
 
-        ovl.burn(address(this), _feeBurn + _liqBurn);
+        ovl.burn(_feeBurn + _liqBurn);
         ovl.transfer(_feeTo, _feeForward + _liqForward);
 
     }
@@ -236,7 +235,7 @@ contract OverlayV1OVLCollateral is ERC1155Supply {
 
     /**
       @notice Build a position on Overlay with OVL collateral
-      @dev This interacts with an Overlay Market to register oi and hold 
+      @dev This interacts with an Overlay Market to register oi and hold
       positions on behalf of users.
       @dev Build event emitted
       @param _market The address of the desired market to interact with
@@ -289,9 +288,9 @@ contract OverlayV1OVLCollateral is ERC1155Supply {
 
         emit Build(_market, _positionId, _oiAdjusted, _debtAdjusted);
 
-        ovl.transferFromBurn(msg.sender, address(this), _collateralAdjusted + _fee, _impact);
+        ovl.transferFrom(msg.sender, address(this), _collateralAdjusted + _impact + _fee);
 
-        // ovl.burn(msg.sender, _impact);
+        ovl.burn(_impact);
 
         _mint(msg.sender, _positionId, _oiAdjusted, ""); // WARNING: last b/c erc1155 callback
 
@@ -326,7 +325,7 @@ contract OverlayV1OVLCollateral is ERC1155Supply {
                     pos.pricePoint
                 );
 
-        uint _totalPosShares = totalSupply(_positionId);
+        uint _totalPosShares = pos.oiShares;
 
         uint _userOiShares = _shares;
         uint _userNotional = _shares * pos.notional(_oi, _oiShares, _priceFrame) / _totalPosShares;
@@ -349,35 +348,26 @@ contract OverlayV1OVLCollateral is ERC1155Supply {
         pos.cost -= _userCost;
         pos.oiShares -= _userOiShares;
 
-        // ovl.transfer(msg.sender, _userCost);
+        IOverlayV1Market(pos.market).exitOI(
+            pos.isLong,
+            _userOi,
+            _userOiShares,
+            _userCost < _userValueAdjusted ? _userValueAdjusted - _userCost : 0,
+            _userCost < _userValueAdjusted ? 0 : _userCost - _userValueAdjusted
+        );
 
         // mint/burn excess PnL = valueAdjusted - cost
         if (_userCost < _userValueAdjusted) {
 
-            ovl.transferMint(
-                msg.sender, 
-                _userCost, 
-                _userValueAdjusted - _userCost
-            );
+            ovl.mint(address(this), _userValueAdjusted - _userCost);
 
         } else {
 
-            ovl.transferBurn(
-                msg.sender, 
-                _userValueAdjusted, 
-                _userCost - _userValueAdjusted
-            );
+            ovl.burn(_userCost - _userValueAdjusted);
 
         }
 
-
-        IOverlayV1Market(pos.market).exitOI(
-            pos.isLong,
-            _userOi,
-            _userOiShares,
-            _userCost < _userValueAdjusted ? _userValueAdjusted - _userCost : 0,
-            _userCost < _userValueAdjusted ? 0 : _userCost - _userValueAdjusted
-        );
+        ovl.transfer(msg.sender, _userValueAdjusted);
 
         }
 
@@ -446,8 +436,8 @@ contract OverlayV1OVLCollateral is ERC1155Supply {
             _rewardsTo
         );
 
-        // ovl.burn(address(this), pos.cost - _value);
-        ovl.transferBurn(_rewardsTo, _toReward, pos.cost - _value);
+        ovl.burn(pos.cost - _value);
+        ovl.transfer(_rewardsTo, _toReward);
 
     }
 

contracts/interfaces/IOverlayToken.sol

@@ -3,10 +3,12 @@ pragma solidity ^0.8.7;
 
 interface IOverlayToken {
 
-    function burn(address account, uint256 amount) external;
+    function burn(uint256 amount) external;
 
     function mint(address account, uint256 amount) external;
 
+    function burnFrom(address account, uint256 amount) external;
+
     function totalSupply() external view returns (uint256);
 
     function balanceOf(address account) external view returns (uint256);
@@ -20,7 +22,7 @@ interface IOverlayToken {
     function decimals() external view returns (uint);
 
     function symbol () external view returns (string memory);
-    
+
     function name () external view returns (string memory);
 
     function transferFrom(

contracts/interfaces/IOverlayV1Mothership.sol

@@ -2,12 +2,11 @@
 pragma solidity ^0.8.7;
 
 import "./IOverlayToken.sol";
-import "./IOverlayTokenNew.sol";
 
 interface IOverlayV1Mothership {
 
     function ovl () external view returns (
-        IOverlayTokenNew ovl_
+        IOverlayToken ovl_
     );
 
     function marketActive(

contracts/market/OverlayV1Governance.sol

@@ -2,8 +2,6 @@
 pragma solidity ^0.8.7;
 
 import "../interfaces/IOverlayV1Mothership.sol";
-import "../interfaces/IOverlayToken.sol";
-import "../interfaces/IOverlayTokenNew.sol";
 import "./OverlayV1Comptroller.sol";
 import "./OverlayV1OI.sol";
 import "./OverlayV1PricePoint.sol";

contracts/market/OverlayV1Market.sol

@@ -7,7 +7,6 @@ import "../interfaces/IOverlayV1Mothership.sol";
 import "./OverlayV1Governance.sol";
 import "./OverlayV1OI.sol";
 import "./OverlayV1PricePoint.sol";
-import "../OverlayToken.sol";
 import "./OverlayV1Comptroller.sol";
 
 abstract contract OverlayV1Market is OverlayV1Governance {
@@ -89,7 +88,7 @@ abstract contract OverlayV1Market is OverlayV1Governance {
       @notice First part of the flow to remove OI from the system
       @dev This is called by the collateral managers to retrieve the necessary
       @dev information to calculate the specifics of each position, for
-      @dev instance the PnL or if it is liquidatable. 
+      @dev instance the PnL or if it is liquidatable.
       @param _isLong Whether the data is being retrieved for a long or short
       @param _pricePoint Index of the initial price point
       @param oi_ Total outstanding open interest on that side of the market
@@ -173,7 +172,7 @@ abstract contract OverlayV1Market is OverlayV1Governance {
 
             updated = _now;
 
-        } 
+        }
 
         // Call to `OverlayV1OI` contract
         (   uint _compoundings,

tests/markets/conftest.py

@@ -3,7 +3,7 @@
 import os
 import json
 from brownie import (
-    OverlayTokenNew,
+    OverlayToken,
     ComptrollerShim,
     chain,
     interface,
@@ -17,6 +17,7 @@
 PRICE_POINTS_START = 50
 PRICE_POINTS_END = 100
 
+
 PRICE_WINDOW_MACRO = 3600
 PRICE_WINDOW_MICRO = 600
 
@@ -143,7 +144,7 @@ def create_token(gov, alice, bob):
     sup = TOKEN_TOTAL_SUPPLY
 
     def create_token(supply=sup):
-        tok = gov.deploy(OverlayTokenNew)
+        tok = gov.deploy(OverlayToken)
         tok.mint(gov, supply, {"from": gov})
         tok.transfer(bob, supply/2, {"from": gov})
         tok.transfer(alice, supply/2, {"from": gov})
@@ -165,12 +166,12 @@ def feed_infos():
     depth_path = '../../feeds/univ3_axs_weth'
 
     raw_uni_framed_market_path = os.path.join(base,
-                                              market_path +
-                                              '_raw_uni_framed.json')
+                                              market_path
+                                              + '_raw_uni_framed.json')
     reflected_market_path = os.path.join(base, market_path + '_reflected.json')
     raw_uni_framed_depth_path = os.path.join(base,
-                                             depth_path +
-                                             '_raw_uni_framed.json')
+                                             depth_path
+                                             + '_raw_uni_framed.json')
     reflected_depth_path = os.path.join(base, depth_path + '_reflected.json')
 
     with open(os.path.normpath(raw_uni_framed_market_path)) as f:

tests/token/conftest.py

@@ -29,7 +29,8 @@ def create_token(gov, alice, bob, request):
     def create_token(supply=sup):
         tok = gov.deploy(OverlayToken)
         tok.mint(gov, supply * 10 ** tok.decimals(), {"from": gov})
-        tok.transfer(bob, supply * 10 ** tok.decimals(), {"from": gov})
+        tok.transfer(alice, (supply/2) * 10 ** tok.decimals(), {"from": gov})
+        tok.transfer(bob, (supply/2) * 10 ** tok.decimals(), {"from": gov})
         return tok
 
     yield create_token

tests/token/test_config.py

@@ -1,7 +1,7 @@
 def test_balances(token, gov, alice, bob, minter, burner, admin):
-    assert token.totalSupply() == token.balanceOf(bob)
+    assert token.totalSupply() == 2 * token.balanceOf(alice) == \
+        2 * token.balanceOf(bob)
     assert token.balanceOf(gov) == 0
-    assert token.balanceOf(alice) == 0
     assert token.balanceOf(minter) == 0
     assert token.balanceOf(burner) == 0
     assert token.balanceOf(admin) == 0

tests/token/test_mint_burn.py

@@ -1,16 +1,28 @@
 import brownie
 
 
-def test_only_minter(token, alice):
-    EXPECTED_ERROR_MSG = 'only minter'
+def test_only_minter_on_mint(token, alice, bob):
+    EXPECTED_ERROR_MSG = 'ERC20: !minter'
     with brownie.reverts(EXPECTED_ERROR_MSG):
-        token.mint(alice, 1 * 10 ** token.decimals(), {"from": alice})
+        token.mint(bob, 1 * 10 ** token.decimals(), {"from": alice})
 
 
-def test_only_burner(token, bob):
-    EXPECTED_ERROR_MSG = 'only burner'
+def test_only_burner_on_burn_from(token, alice, bob):
+    EXPECTED_ERROR_MSG = 'ERC20: !burner'
     with brownie.reverts(EXPECTED_ERROR_MSG):
-        token.burn(bob, 1 * 10 ** token.decimals(), {"from": bob})
+        token.burnFrom(alice, 1 * 10 ** token.decimals(), {"from": bob})
+
+
+def test_only_burner_on_burn(token, bob):
+    EXPECTED_ERROR_MSG = 'ERC20: !burner'
+    with brownie.reverts(EXPECTED_ERROR_MSG):
+        token.burn(1 * 10 ** token.decimals(), {"from": bob})
+
+
+def test_burn_from_exceeds_allowance(token, burner, bob):
+    EXPECTED_ERROR_MSG = 'ERC20: burn amount exceeds allowance'
+    with brownie.reverts(EXPECTED_ERROR_MSG):
+        token.burnFrom(bob, 1 * 10 ** token.decimals(), {"from": burner})
 
 
 def test_mint(token, minter, alice):
@@ -20,17 +32,43 @@ def test_mint(token, minter, alice):
     assert token.balanceOf(alice) == before + amount
 
 
-def test_burn(token, burner, bob):
+def test_burn(token, burner, minter):
+    amount = 1 * 10 ** token.decimals()
+    token.mint(burner, amount, {"from": minter})
+    before = token.balanceOf(burner)
+
+    token.burn(amount, {"from": burner})
+    assert token.balanceOf(burner) == before - amount
+
+
+def test_burn_from(token, burner, bob):
     before = token.balanceOf(bob)
     amount = 1 * 10 ** token.decimals()
-    token.burn(bob, amount, {"from": burner})
+
+    token.approve(burner, amount, {"from": bob})
+    token.burnFrom(bob, amount, {"from": burner})
+
     assert token.balanceOf(bob) == before - amount
 
 
 def test_mint_then_burn(token, market, alice):
     before = token.balanceOf(alice)
+
     token.mint(alice, 20 * 10 ** token.decimals(), {"from": market})
     mid = before + 20 * 10 ** token.decimals()
     assert token.balanceOf(alice) == mid
-    token.burn(alice, 15 * 10 ** token.decimals(), {"from": market})
+
+    token.approve(market, 20 * 10 ** token.decimals(), {"from": alice})
+    token.burnFrom(alice, 15 * 10 ** token.decimals(), {"from": market})
     assert token.balanceOf(alice) == mid - 15 * 10 ** token.decimals()
+
+    before_market = token.balanceOf(market)
+    token.transfer(market, 5 * 10 ** token.decimals(), {"from": alice})
+    assert token.balanceOf(market) == before_market + \
+        5 * 10 ** token.decimals()
+    assert token.balanceOf(alice) == mid - 20 * 10 ** token.decimals()
+
+    before_market += 5 * 10 ** token.decimals()
+    token.burn(5 * 10 ** token.decimals(), {"from": market})
+    assert token.balanceOf(market) == before_market - \
+        5 * 10 ** token.decimals()