Audit fixes (#67)

src/BobVault.sol

@@ -86,9 +86,10 @@ contract BobVault is EIP1967Admin, Ownable, YieldConnector {
 
         res.total = IERC20(_token).balanceOf(address(this));
         res.required = token.balance;
-        if (token.yield != address(0)) {
-            res.total += _delegateInvestedAmount(token.yield, _token);
-            res.required += token.dust;
+        (uint96 dust, address yield) = (token.dust, token.yield);
+        if (yield != address(0)) {
+            res.total += _delegateInvestedAmount(yield, _token);
+            res.required += dust;
         }
         res.farmed = res.total - res.required;
     }
@@ -578,9 +579,10 @@ contract BobVault is EIP1967Admin, Ownable, YieldConnector {
         uint256 currentBalance = IERC20(_token).balanceOf(address(this));
         uint256 requiredBalance = token.balance;
 
-        if (token.yield != address(0)) {
-            currentBalance += _delegateInvestedAmount(token.yield, _token);
-            requiredBalance += token.dust;
+        (uint96 dust, address yield) = (token.dust, token.yield);
+        if (yield != address(0)) {
+            currentBalance += _delegateInvestedAmount(yield, _token);
+            requiredBalance += dust;
         }
 
         if (requiredBalance >= currentBalance) {
@@ -589,7 +591,7 @@ contract BobVault is EIP1967Admin, Ownable, YieldConnector {
 
         uint256 value = currentBalance - requiredBalance;
         _transferOut(_token, msg.sender, value);
-        emit Farm(_token, token.yield, value);
+        emit Farm(_token, yield, value);
 
         return value;
     }
@@ -606,9 +608,11 @@ contract BobVault is EIP1967Admin, Ownable, YieldConnector {
         Collateral memory token = collateral[_token];
         require(token.price > 0, "BobVault: unsupported collateral");
 
-        returnData = _delegateFarmExtra(token.yield, _token, msg.sender, _data);
+        address yield = token.yield;
+        require(yield != address(0), "BobVault: yield not enabled");
+        returnData = _delegateFarmExtra(yield, _token, msg.sender, _data);
 
-        emit FarmExtra(_token, token.yield);
+        emit FarmExtra(_token, yield);
     }
 
     /**
@@ -668,7 +672,7 @@ contract BobVault is EIP1967Admin, Ownable, YieldConnector {
             if (invested < withdrawValue) {
                 withdrawValue = invested;
             }
-            _delegateWithdraw(token.yield, _token, withdrawValue);
+            _delegateWithdraw(yield, _token, withdrawValue);
             emit Withdraw(_token, yield, withdrawValue);
         }
 

src/minters/BalancedMinter.sol

@@ -9,6 +9,8 @@ import "./BaseMinter.sol";
  * BOB minting/burning middleware with simple usage quotas.
  */
 contract BalancedMinter is BaseMinter {
+    uint256 private constant MINT_BURN_VALUE_LIMIT = 1 << 127;
+
     int128 public mintQuota; // remaining minting quota
     int128 public burnQuota; // remaining burning quota
 
@@ -29,7 +31,7 @@ contract BalancedMinter is BaseMinter {
         (int128 newMintQuota, int128 newBurnQuota) = (mintQuota + _dMint, burnQuota + _dBurn);
         (mintQuota, burnQuota) = (newMintQuota, newBurnQuota);
 
-        emit UpdateQuotas(newBurnQuota, newBurnQuota);
+        emit UpdateQuotas(newMintQuota, newBurnQuota);
     }
 
     /**
@@ -39,7 +41,7 @@ contract BalancedMinter is BaseMinter {
     function _beforeMint(uint256 _amount) internal override {
         int128 amount = int128(uint128(_amount));
         unchecked {
-            require(mintQuota >= amount, "BalancedMinter: exceeds minting quota");
+            require(mintQuota >= amount && _amount < MINT_BURN_VALUE_LIMIT, "BalancedMinter: exceeds minting quota");
             (mintQuota, burnQuota) = (mintQuota - amount, burnQuota + amount);
         }
     }
@@ -51,7 +53,7 @@ contract BalancedMinter is BaseMinter {
     function _beforeBurn(uint256 _amount) internal override {
         int128 amount = int128(uint128(_amount));
         unchecked {
-            require(burnQuota >= amount, "BalancedMinter: exceeds burning quota");
+            require(burnQuota >= amount && _amount < MINT_BURN_VALUE_LIMIT, "BalancedMinter: exceeds burning quota");
             (mintQuota, burnQuota) = (mintQuota + amount, burnQuota - amount);
         }
     }

src/minters/BaseMinter.sol

@@ -16,6 +16,7 @@ abstract contract BaseMinter is IMintableERC20, IBurnableERC20, IERC677Receiver,
 
     mapping(address => bool) public isMinter;
 
+    event UpdateMinter(address indexed minter, bool enabled);
     event Mint(address minter, address to, uint256 amount);
     event Burn(address burner, address from, uint256 amount);
 
@@ -31,6 +32,8 @@ abstract contract BaseMinter is IMintableERC20, IBurnableERC20, IERC677Receiver,
      */
     function setMinter(address _account, bool _enabled) external onlyOwner {
         isMinter[_account] = _enabled;
+
+        emit UpdateMinter(_account, _enabled);
     }
 
     /**

src/minters/FlashMinter.sol

@@ -24,21 +24,25 @@ contract FlashMinter is IERC3156FlashLender, ReentrancyGuard, Ownable {
     uint64 public fee; // fee percentage * 1 ether
     uint96 public maxFee; // max fee in absolute values
 
+    event UpdateConfig(uint96 limit, address treasury, uint64 fee, uint96 maxFee);
     event FlashMint(address indexed _receiver, uint256 _amount, uint256 _fee);
 
     constructor(address _token, uint96 _limit, address _treasury, uint64 _fee, uint96 _maxFee) {
-        require(_treasury != address(0) || _fee == 0, "FlashMinter: invalid fee config");
         token = _token;
-        limit = _limit;
-        treasury = _treasury;
-        _setFees(_fee, _maxFee);
+        _updateConfig(_limit, _treasury, _fee, _maxFee);
     }
 
     function updateConfig(uint96 _limit, address _treasury, uint64 _fee, uint96 _maxFee) external onlyOwner {
+        _updateConfig(_limit, _treasury, _fee, _maxFee);
+    }
+
+    function _updateConfig(uint96 _limit, address _treasury, uint64 _fee, uint96 _maxFee) internal {
         require(_treasury != address(0) || _fee == 0, "FlashMinter: invalid fee config");
         limit = _limit;
         treasury = _treasury;
         _setFees(_fee, _maxFee);
+
+        emit UpdateConfig(_limit, _treasury, _fee, _maxFee);
     }
 
     function _setFees(uint64 _fee, uint96 _maxFee) internal {
@@ -104,18 +108,18 @@ contract FlashMinter is IERC3156FlashLender, ReentrancyGuard, Ownable {
     {
         require(token == _token, "FlashMinter: wrong token");
         require(_amount <= limit, "FlashMinter: amount exceeds maxFlashLoan");
-        uint256 fee = _flashFee(_amount);
+        uint256 flashFee = _flashFee(_amount);
         IMintableERC20(_token).mint(address(_receiver), _amount);
         require(
-            _receiver.onFlashLoan(msg.sender, _token, _amount, fee, _data) == _RETURN_VALUE,
+            _receiver.onFlashLoan(msg.sender, _token, _amount, flashFee, _data) == _RETURN_VALUE,
             "FlashMinter: invalid return value"
         );
         IBurnableERC20(_token).burnFrom(address(_receiver), _amount);
-        if (fee > 0) {
-            IERC20(_token).transferFrom(address(_receiver), treasury, fee);
+        if (flashFee > 0) {
+            IERC20(_token).transferFrom(address(_receiver), treasury, flashFee);
         }
 
-        emit FlashMint(address(_receiver), _amount, fee);
+        emit FlashMint(address(_receiver), _amount, flashFee);
 
         return true;
     }

src/minters/SurplusMinter.sol

@@ -19,6 +19,7 @@ contract SurplusMinter is IERC677Receiver, Ownable {
 
     uint256 public surplus; // unrealized surplus
 
+    event UpdateMinter(address indexed minter, bool enabled);
     event WithdrawSurplus(address indexed to, uint256 realized, uint256 unrealized);
     event AddSurplus(address indexed from, uint256 unrealized);
 
@@ -34,6 +35,8 @@ contract SurplusMinter is IERC677Receiver, Ownable {
      */
     function setMinter(address _account, bool _enabled) external onlyOwner {
         isMinter[_account] = _enabled;
+
+        emit UpdateMinter(_account, _enabled);
     }
 
     /**

src/token/BaseERC20.sol

@@ -9,6 +9,9 @@ import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
  * @title BaseERC20
  */
 abstract contract BaseERC20 is IERC20, IERC20Metadata {
+    uint256 private constant FROZEN_MASK = 0x8000000000000000000000000000000000000000000000000000000000000000;
+    uint256 private constant BALANCE_MASK = 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
+
     mapping(address => uint256) internal _balances;
     mapping(address => mapping(address => uint256)) public allowance;
     uint256 public totalSupply;
@@ -24,7 +27,7 @@ abstract contract BaseERC20 is IERC20, IERC20Metadata {
     function balanceOf(address account) public view virtual override returns (uint256 _balance) {
         _balance = _balances[account];
         assembly {
-            _balance := and(_balance, 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
+            _balance := and(_balance, BALANCE_MASK)
         }
     }
 
@@ -107,15 +110,15 @@ abstract contract BaseERC20 is IERC20, IERC20Metadata {
 
     function _increaseBalance(address _account, uint256 _amount) internal {
         uint256 balance = _balances[_account];
-        require(balance < 1 << 255, "ERC20: account frozen");
+        require(balance < FROZEN_MASK, "ERC20: account frozen");
         unchecked {
             _balances[_account] = balance + _amount;
         }
     }
 
     function _decreaseBalance(address _account, uint256 _amount) internal {
         uint256 balance = _balances[_account];
-        require(balance < 1 << 255, "ERC20: account frozen");
+        require(balance < FROZEN_MASK, "ERC20: account frozen");
         require(balance >= _amount, "ERC20: amount exceeds balance");
         unchecked {
             _balances[_account] = balance - _amount;
@@ -130,14 +133,14 @@ abstract contract BaseERC20 is IERC20, IERC20Metadata {
     }
 
     function _isFrozen(address _account) internal view returns (bool) {
-        return _balances[_account] >= 1 << 255;
+        return _balances[_account] >= FROZEN_MASK;
     }
 
     function _freezeBalance(address _account) internal {
-        _balances[_account] |= 1 << 255;
+        _balances[_account] |= FROZEN_MASK;
     }
 
     function _unfreezeBalance(address _account) internal {
-        _balances[_account] &= (1 << 255) - 1;
+        _balances[_account] &= BALANCE_MASK;
     }
 }

src/token/ERC20Recovery.sol

@@ -127,7 +127,7 @@ abstract contract ERC20Recovery is Ownable, BaseERC20 {
         uint256[] memory values = new uint256[](_values.length);
 
         uint256 total = 0;
-        for (uint256 i = 0; i < _values.length; i++) {
+        for (uint256 i = 0; i < _values.length; ++i) {
             uint256 balance = balanceOf(_accounts[i]);
             uint256 value = balance < _values[i] ? balance : _values[i];
             values[i] = value;
@@ -190,7 +190,7 @@ abstract contract ERC20Recovery is Ownable, BaseERC20 {
         uint256 total = 0;
         address receiver = recoveredFundsReceiver;
 
-        for (uint256 i = 0; i < _accounts.length; i++) {
+        for (uint256 i = 0; i < _accounts.length; ++i) {
             uint256 balance = balanceOf(_accounts[i]);
             uint256 value = balance < _values[i] ? balance : _values[i];
             total += value;

src/utils/UniswapV3Seller.sol

@@ -2,6 +2,7 @@
 
 pragma solidity 0.8.15;
 
+import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import "@uniswap/v3-periphery/contracts/interfaces/ISwapRouter.sol";
 import "@uniswap/v3-periphery/contracts/interfaces/IPeripheryImmutableState.sol";
 import "@uniswap/v3-periphery/contracts/interfaces/IQuoter.sol";
@@ -14,6 +15,8 @@ import "./Sacrifice.sol";
  * Helper for selling some token for ETH through a 2-hop UniswapV3 exchange.
  */
 contract UniswapV3Seller is ITokenSeller {
+    using SafeERC20 for IERC20;
+
     ISwapRouter immutable swapRouter;
     IQuoter immutable quoter;
     IWETH9 immutable WETH;
@@ -67,7 +70,7 @@ contract UniswapV3Seller is ITokenSeller {
         uint256 remainingBalance = IERC20(token0).balanceOf(address(this));
         if (remainingBalance + _amount > balance) {
             uint256 refund = remainingBalance + _amount - balance;
-            IERC20(token0).transfer(msg.sender, refund);
+            IERC20(token0).safeTransfer(msg.sender, refund);
             return (amountOut, refund);
         }
         return (amountOut, 0);

src/zkbob/ZkBobDirectDepositQueue.sol

@@ -21,7 +21,6 @@ contract ZkBobDirectDepositQueue is IZkBobDirectDeposits, IZkBobDirectDepositQue
     using SafeERC20 for IERC20;
 
     uint256 internal constant R = 21888242871839275222246405745257275088548364400416034343698204186575808495617;
-    uint256 internal constant MAX_POOL_ID = 0xffffff;
     uint256 internal constant TOKEN_DENOMINATOR = 1_000_000_000;
     uint256 internal constant MAX_NUMBER_OF_DIRECT_DEPOSITS = 16;
     bytes4 internal constant MESSAGE_PREFIX_DIRECT_DEPOSIT_V1 = 0x00000001;
@@ -58,16 +57,10 @@ contract ZkBobDirectDepositQueue is IZkBobDirectDeposits, IZkBobDirectDepositQue
         pool_id = uint24(IZkBobPool(_pool).pool_id());
     }
 
-    /**
-     * @dev Throws if called by any account other than the current relayer operator.
-     */
-    modifier onlyOperator() {
-        require(operatorManager.isOperator(_msgSender()), "ZkBobDirectDepositQueue: not an operator");
-        _;
-    }
-
     /**
      * @dev Updates used operator manager contract.
+     * Operator manager in this contract is only responsible for fast-track processing of refunds.
+     * Usage of fully permissionless operator managers is not recommended, due to existence of front-running DoS attacks.
      * Callable only by the contract owner / proxy admin.
      * @param _operatorManager new operator manager implementation.
      */
@@ -125,7 +118,7 @@ contract ZkBobDirectDepositQueue is IZkBobDirectDeposits, IZkBobDirectDepositQue
         }
         total = 0;
         totalFee = 0;
-        for (uint256 i = 0; i < count; i++) {
+        for (uint256 i = 0; i < count; ++i) {
             uint256 index = _indices[i];
             DirectDeposit storage dd = directDeposits[index];
             (bytes32 pk, bytes10 diversifier, uint64 deposit, uint64 fee, DirectDepositStatus status) =
@@ -213,12 +206,13 @@ contract ZkBobDirectDepositQueue is IZkBobDirectDeposits, IZkBobDirectDepositQue
     function refundDirectDeposit(uint256[] calldata _indices) external {
         bool isOperator = operatorManager.isOperator(msg.sender);
 
-        for (uint256 i = 0; i < _indices.length; i++) {
+        uint256 timeout = directDepositTimeout;
+        for (uint256 i = 0; i < _indices.length; ++i) {
             DirectDeposit storage dd = directDeposits[_indices[i]];
 
             if (dd.status == DirectDepositStatus.Pending) {
                 require(
-                    isOperator || dd.timestamp + directDepositTimeout < block.timestamp,
+                    isOperator || dd.timestamp + timeout < block.timestamp,
                     "ZkBobDirectDepositQueue: direct deposit timeout not passed"
                 );
                 _refundDirectDeposit(_indices[i], dd);

src/zkbob/ZkBobPool.sol

@@ -65,6 +65,7 @@ abstract contract ZkBobPool is IZkBobPool, EIP1967Admin, Ownable, Parameters, Zk
         require(Address.isContract(_token), "ZkBobPool: not a contract");
         require(Address.isContract(address(_transfer_verifier)), "ZkBobPool: not a contract");
         require(Address.isContract(address(_tree_verifier)), "ZkBobPool: not a contract");
+        require(Address.isContract(address(_batch_deposit_verifier)), "ZkBobPool: not a contract");
         require(Address.isContract(_direct_deposit_queue), "ZkBobPool: not a contract");
         pool_id = __pool_id;
         token = _token;

src/zkbob/utils/ZkBobAccounting.sol

@@ -189,7 +189,9 @@ contract ZkBobAccounting is KycProvidersManagerStorage {
         s0.cumTvl += s1.tvl / uint72(PRECISION);
         s0.txCount++;
 
-        _processTVLChange(s1, _user, _txAmount);
+        if (_txAmount != 0) {
+            _processTVLChange(s1, _user, _txAmount);
+        }
 
         slot0 = s0;
         return (s0.maxWeeklyAvgTvl, s0.maxWeeklyTxCount, txCount);
@@ -198,10 +200,6 @@ contract ZkBobAccounting is KycProvidersManagerStorage {
     function _processTVLChange(Slot1 memory s1, address _user, int256 _txAmount) internal {
         uint16 curDay = uint16(block.timestamp / SLOT_DURATION / DAY_SLOTS);
 
-        if (_txAmount == 0) {
-            return;
-        }
-
         UserStats memory us = userStats[_user];
         Tier storage t = tiers[_adjustConfiguredTierForUser(_user, us.tier)];
         TierLimits memory tl = t.limits;
@@ -348,7 +346,7 @@ contract ZkBobAccounting is KycProvidersManagerStorage {
         require(
             _tier == 255 || tiers[uint256(_tier)].limits.tvlCap > 0, "ZkBobAccounting: non-existing pool limits tier"
         );
-        for (uint256 i = 0; i < _users.length; i++) {
+        for (uint256 i = 0; i < _users.length; ++i) {
             address user = _users[i];
             userStats[user].tier = _tier;
             emit UpdateTier(user, _tier);