diff --git a/lib/@openzeppelin/contracts b/lib/@openzeppelin/contracts index 91df66c..0a25c19 160000 --- a/lib/@openzeppelin/contracts +++ b/lib/@openzeppelin/contracts @@ -1 +1 @@ -Subproject commit 91df66c4a9dfd0425ff923cbeb3a20155f1355ea +Subproject commit 0a25c1940ca220686588c4af3ec526f725fe2582 diff --git a/src/BobVault.sol b/src/BobVault.sol index 0e50506..93c6d70 100644 --- a/src/BobVault.sol +++ b/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); } diff --git a/src/minters/BalancedMinter.sol b/src/minters/BalancedMinter.sol index 2e10ce5..65a9986 100644 --- a/src/minters/BalancedMinter.sol +++ b/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); } } diff --git a/src/minters/BaseMinter.sol b/src/minters/BaseMinter.sol index f92fe70..5075eba 100644 --- a/src/minters/BaseMinter.sol +++ b/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); } /** diff --git a/src/minters/FlashMinter.sol b/src/minters/FlashMinter.sol index d601abf..9d70949 100644 --- a/src/minters/FlashMinter.sol +++ b/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; } diff --git a/src/minters/SurplusMinter.sol b/src/minters/SurplusMinter.sol index 8f3cd3b..d1b1d67 100644 --- a/src/minters/SurplusMinter.sol +++ b/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); } /** diff --git a/src/token/BaseERC20.sol b/src/token/BaseERC20.sol index 8d7abfc..9ba44f5 100644 --- a/src/token/BaseERC20.sol +++ b/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,7 +110,7 @@ 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; } @@ -115,7 +118,7 @@ abstract contract BaseERC20 is IERC20, IERC20Metadata { 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; } } diff --git a/src/token/ERC20Recovery.sol b/src/token/ERC20Recovery.sol index 6222c09..1fc663c 100644 --- a/src/token/ERC20Recovery.sol +++ b/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; diff --git a/src/utils/UniswapV3Seller.sol b/src/utils/UniswapV3Seller.sol index 16e6908..11087f3 100644 --- a/src/utils/UniswapV3Seller.sol +++ b/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); diff --git a/src/zkbob/ZkBobDirectDepositQueue.sol b/src/zkbob/ZkBobDirectDepositQueue.sol index 3583f42..819330b 100644 --- a/src/zkbob/ZkBobDirectDepositQueue.sol +++ b/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); diff --git a/src/zkbob/ZkBobPool.sol b/src/zkbob/ZkBobPool.sol index a53a844..7875786 100644 --- a/src/zkbob/ZkBobPool.sol +++ b/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; diff --git a/src/zkbob/utils/ZkBobAccounting.sol b/src/zkbob/utils/ZkBobAccounting.sol index 2c9b8a5..fc9db45 100644 --- a/src/zkbob/utils/ZkBobAccounting.sol +++ b/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); diff --git a/test/BobVault.t.sol b/test/BobVault.t.sol index f0fbd91..8c5ae5a 100644 --- a/test/BobVault.t.sol +++ b/test/BobVault.t.sol @@ -45,10 +45,8 @@ contract BobVaultTest is AbstractBobVaultTest { function setUp() public { _setUpBobVault(); - tokenA = IERC20(new ERC20Mock()); - ERC20Mock(address(tokenA)).mint(address(this), 1_000_000 ether); - tokenB = IERC20(new ERC20Mock()); - ERC20Mock(address(tokenB)).mint(address(this), 1_000_000 ether); + tokenA = IERC20(new ERC20Mock("Mock Token A", "MA", address(this), 1_000_000 ether)); + tokenB = IERC20(new ERC20Mock("Mock Token B", "MB", address(this), 1_000_000 ether)); } function testAuthRights() public { @@ -373,7 +371,6 @@ abstract contract AbstractBobVault3poolTest is AbstractBobVaultTest, AbstractFor vault.reclaim(user1, 1000 ether); assertEq(bob.balanceOf(address(vault)), 0); assertGt(bob.balanceOf(user1), 700 ether); - vm.stopPrank(); } function testMaxBalance() public { @@ -611,7 +608,6 @@ abstract contract AbstractBobVaultAAVETest is AbstractBobVaultTest, AbstractFork assertEq(stat.required, 9_990_000 * 1e6); assertGt(stat.farmed, 10_001 * 1e6); - vm.stopPrank(); vm.startPrank(user2); deal(address(usdc), user2, 0); diff --git a/test/minters/BalancedMinter.t.sol b/test/minters/BalancedMinter.t.sol index 731d924..e2ef5ec 100644 --- a/test/minters/BalancedMinter.t.sol +++ b/test/minters/BalancedMinter.t.sol @@ -66,6 +66,20 @@ contract BalancedMinterTest is Test { assertEq(minter.burnQuota(), 105 ether); } + function testOverflowQuotas() public { + minter.setMinter(address(this), true); + + vm.expectRevert("BalancedMinter: exceeds minting quota"); + minter.mint(user1, 2 ** 127); + vm.expectRevert("BalancedMinter: exceeds minting quota"); + minter.mint(user1, 2 ** 200); + + vm.expectRevert("BalancedMinter: exceeds burning quota"); + minter.burn(2 ** 127); + vm.expectRevert("BalancedMinter: exceeds burning quota"); + minter.burn(2 ** 200); + } + function testExceedingQuotas() public { bob.mint(address(this), 200 ether); minter.setMinter(address(this), true);