pull lesson22 to main

src/AccessManager.sol

@@ -9,7 +9,19 @@ contract AccessManager is AccessControl {
 
     constructor() {
         _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
+
+        // Set role admin relationships
+        _setRoleAdmin(Roles.ADMIN_ROLE, DEFAULT_ADMIN_ROLE);
         _setRoleAdmin(Roles.MULTISIG_ADMIN_ROLE, DEFAULT_ADMIN_ROLE);
+        _setRoleAdmin(Roles.ALLOWED_EIP712_SWAP_ROLE, DEFAULT_ADMIN_ROLE);
+    }
+
+    function addAdmin(address _admin) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        _grantRole(Roles.ADMIN_ROLE, _admin);
+    }
+
+    function removeAdmin(address _admin) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        _revokeRole(Roles.ADMIN_ROLE, _admin);
     }
 
     function addMultisigAdmin(address _multisigAdmin) external onlyRole(DEFAULT_ADMIN_ROLE) {
@@ -20,7 +32,23 @@ contract AccessManager is AccessControl {
         _revokeRole(Roles.MULTISIG_ADMIN_ROLE, _multisigAdmin);
     }
 
+    function addEIP712Swapper(address _swapper) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        _grantRole(Roles.ALLOWED_EIP712_SWAP_ROLE, _swapper);
+    }
+
+    function removeEIP712Swapper(address _swapper) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        _revokeRole(Roles.ALLOWED_EIP712_SWAP_ROLE, _swapper);
+    }
+
+    function isAdmin(address _address) external view returns (bool) {
+        return hasRole(Roles.ADMIN_ROLE, _address);
+    }
+
     function isMultisigAdmin(address _address) external view returns (bool) {
         return hasRole(Roles.MULTISIG_ADMIN_ROLE, _address);
     }
+
+    function isEIP712Swapper(address _address) external view returns (bool) {
+        return hasRole(Roles.ALLOWED_EIP712_SWAP_ROLE, _address);
+    }
 }

src/EIP712Swap.sol

@@ -55,13 +55,14 @@ contract EIP712Swap is EIP712 {
         if (_swapRequest.nonce != _nonces[_swapRequest.sender]) revert InvalidNonce();
 
         _nonces[_swapRequest.sender]++;
-        LiquidityPool(_swapRequest.pool).swap(
-            _swapRequest.sender,
-            _swapRequest.tokenIn,
-            _swapRequest.tokenOut,
-            _swapRequest.amountIn,
-            _swapRequest.minAmountOut
-        );
+        LiquidityPool(_swapRequest.pool)
+            .swap(
+                _swapRequest.sender,
+                _swapRequest.tokenIn,
+                _swapRequest.tokenOut,
+                _swapRequest.amountIn,
+                _swapRequest.minAmountOut
+            );
 
         return true;
     }

src/FeeManager.sol

@@ -4,16 +4,15 @@ pragma solidity ^0.8.30;
 import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
-
 import "./ISwap.sol";
 
 contract FeeManager is Initializable, AccessControlUpgradeable, UUPSUpgradeable {
     uint256 public constant FEE_DENOMINATOR = 10000;
-
-    uint256 public fee;
+    uint256 public fee; // Fee in basis points (e.g., 250 = 2.5%)
 
     function initialize(uint256 _fee) external initializer {
-        _disableInitializers();
+        __AccessControl_init();
+        __UUPSUpgradeable_init();
         _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
         fee = _fee;
     }
@@ -24,7 +23,13 @@ contract FeeManager is Initializable, AccessControlUpgradeable, UUPSUpgradeable
         fee = _fee;
     }
 
+    /// @notice Calculate absolute fee amount for a swap
+    /// @param swapParams The swap parameters
+    /// @return Absolute fee amount in output token units
     function getFee(ISwap.SwapParams memory swapParams) external view returns (uint256) {
-        return (swapParams.amount0 * fee) / FEE_DENOMINATOR;
+        // Calculate fee based on input amount and convert to output token equivalent
+        uint256 amountOut =
+            (swapParams.amount0 * swapParams.reserveToken1) / (swapParams.reserveToken0 + swapParams.amount0);
+        return (amountOut * fee) / FEE_DENOMINATOR;
     }
 }

src/LiquidityPool.sol

@@ -2,11 +2,15 @@
 pragma solidity ^0.8.30;
 
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts/access/AccessControl.sol";
 import "./ISwap.sol";
 import "./FeeManager.sol";
 import "./EIP712Swap.sol";
+import "./Roles.sol";
+
+contract LiquidityPool is ISwap, AccessControl {
+    using Roles for bytes32;
 
-contract LiquidityPool is ISwap, FeeManager {
     address public token0;
     uint256 public token0Decimals;
     address public token1;
@@ -16,24 +20,22 @@ contract LiquidityPool is ISwap, FeeManager {
     FeeManager public feeManager;
     EIP712Swap public eip712Swap;
 
-    // @notice Emitted when liquidity is added to the pool
-    /// @param _token The token that was added
-    /// @param _amount The amount of tokens that were added
-    event LiquidityAdded(address indexed _token, uint256 _amount);
+    modifier onlyAdminOrEIP712Swap() {
+        require(
+            hasRole(Roles.ADMIN_ROLE, msg.sender) || hasRole(Roles.ALLOWED_EIP712_SWAP_ROLE, msg.sender),
+            "Not authorized for swap operations"
+        );
+        _;
+    }
 
-    // @notice Emitted when a swap is executed
-    /// @param _tokenIn The token that was swapped in
-    /// @param _tokenOut The token that was swapped out
-    /// @param _amountIn The amount of tokens that were swapped in
-    /// @param _amountOut The amount of tokens that were swapped out
+    event LiquidityAdded(address indexed _token, uint256 _amount);
     event Swap(address indexed _tokenIn, address indexed _tokenOut, uint256 _amountIn, uint256 _amountOut);
 
     error InsufficientTokenBalance();
     error InvalidTokenAddress(address _token);
     error InvalidTokenPair(address _tokenIn, address _tokenOut);
     error InsufficientLiquidity();
     error InsufficientOutputAmount(uint256 expected, uint256 actual);
-    error ExcessiveInputAmount(uint256 expected, uint256 actual);
     error InsufficientAllowance();
 
     constructor(
@@ -50,61 +52,88 @@ contract LiquidityPool is ISwap, FeeManager {
         token1Decimals = _token1Decimals;
         feeManager = FeeManager(_feeManager);
         eip712Swap = EIP712Swap(_eip712Swap);
+
+        // Setup roles
+        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
+        _grantRole(Roles.ADMIN_ROLE, msg.sender);
+
+        // Grant EIP712Swap contract permission to execute swaps
+        _grantRole(Roles.ALLOWED_EIP712_SWAP_ROLE, _eip712Swap);
+
+        // Set role admin relationships
+        _setRoleAdmin(Roles.ADMIN_ROLE, DEFAULT_ADMIN_ROLE);
+        _setRoleAdmin(Roles.ALLOWED_EIP712_SWAP_ROLE, DEFAULT_ADMIN_ROLE);
     }
 
-    // @notice Add liquidity to the pool
-    /// @param _token The token to add liquidity for
-    /// @param _amount The amount of tokens to add
-    function addLiquidity(address _token, uint256 _amount) external onlyRole(DEFAULT_ADMIN_ROLE) {
-        if (_token == token0 || _token == token1) {
+    /// @notice Add liquidity to the pool (admin only)
+    function addLiquidity(address _token, uint256 _amount) external onlyRole(Roles.ADMIN_ROLE) {
+        if (_token != token0 && _token != token1) {
             revert InvalidTokenAddress(_token);
         }
 
-        if (IERC20(_token).balanceOf(address(msg.sender)) < _amount) {
+        if (IERC20(_token).balanceOf(msg.sender) < _amount) {
             revert InsufficientTokenBalance();
         }
 
         require(IERC20(_token).transferFrom(msg.sender, address(this), _amount));
 
         if (_token == token0) {
             reserveToken0 += _amount;
-        } else if (_token == token1) {
+        } else {
             reserveToken1 += _amount;
         }
 
         emit LiquidityAdded(_token, _amount);
     }
 
-    // @notice Get the reserves of the pool
-    /// @return _reserveToken0 The reserve of token0
-    /// @return _reserveToken1 The reserve of token1
+    /// @notice Remove liquidity from the pool (admin only)
+    function removeLiquidity(address _token, uint256 _amount) external onlyRole(Roles.ADMIN_ROLE) {
+        if (_token != token0 && _token != token1) {
+            revert InvalidTokenAddress(_token);
+        }
+
+        uint256 currentReserve = _token == token0 ? reserveToken0 : reserveToken1;
+        require(_amount <= currentReserve, "Insufficient reserves");
+
+        require(IERC20(_token).transfer(msg.sender, _amount));
+
+        if (_token == token0) {
+            reserveToken0 -= _amount;
+        } else {
+            reserveToken1 -= _amount;
+        }
+    }
+
+    /// @notice Grant EIP712 swap permission to an address
+    function grantSwapRole(address _swapper) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        _grantRole(Roles.ALLOWED_EIP712_SWAP_ROLE, _swapper);
+    }
+
+    /// @notice Revoke EIP712 swap permission from an address
+    function revokeSwapRole(address _swapper) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        _revokeRole(Roles.ALLOWED_EIP712_SWAP_ROLE, _swapper);
+    }
+
     function getReserves() external view returns (uint256 _reserveToken0, uint256 _reserveToken1) {
         _reserveToken0 = reserveToken0;
         _reserveToken1 = reserveToken1;
     }
 
-    /// @notice Get the price of the token in the pool
-    /// @param _tokenIn The token to get the price of
-    /// @param _tokenOut The token to get the price of
-    /// @return _price The price of the token in the pool
     function getPrice(address _tokenIn, address _tokenOut) external view returns (uint256 _price) {
         uint256 _reserveTokenIn = _tokenIn == token0 ? reserveToken0 : reserveToken1;
         uint256 _reserveTokenOut = _tokenOut == token0 ? reserveToken0 : reserveToken1;
-        uint256 _tokenInDecimals = _tokenIn == token0 ? token0Decimals : token1Decimals;
-        uint256 _tokenOutDecimals = _tokenOut == token0 ? token0Decimals : token1Decimals;
 
-        _price = (_reserveTokenIn * 10 ** _tokenInDecimals) / (_reserveTokenOut * 10 ** _tokenOutDecimals);
+        if (_reserveTokenIn == 0 || _reserveTokenOut == 0) {
+            return 0;
+        }
+
+        _price = (_reserveTokenOut * 1e18) / _reserveTokenIn;
     }
 
-    /// @notice Swap tokens in the pool
-    /// @param _sender The address of the sender
-    /// @param _tokenIn The token to swap in
-    /// @param _tokenOut The token to swap out
-    /// @param _amountIn The amount of tokens to swap in
-    /// @param _minAmountOut The minimum amount of tokens to swap out
+    /// @notice Execute a swap (admin or authorized EIP712 contract only)
     function swap(address _sender, address _tokenIn, address _tokenOut, uint256 _amountIn, uint256 _minAmountOut)
         external
-        onlyRole(DEFAULT_ADMIN_ROLE)
+        onlyAdminOrEIP712Swap
     {
         if (
             _tokenIn != token0 && _tokenIn != token1 || _tokenOut != token0 && _tokenOut != token1
@@ -113,25 +142,35 @@ contract LiquidityPool is ISwap, FeeManager {
             revert InvalidTokenPair(_tokenIn, _tokenOut);
         }
 
-        address _msgSender = msg.sender == address(eip712Swap) ? _sender : msg.sender;
+        address tokenHolder = _sender;
 
-        if (IERC20(_tokenIn).allowance(_msgSender, address(this)) < _amountIn) revert InsufficientAllowance();
+        if (IERC20(_tokenIn).allowance(tokenHolder, address(this)) < _amountIn) revert InsufficientAllowance();
 
         uint256 _reserveTokenIn = _tokenIn == token0 ? reserveToken0 : reserveToken1;
         uint256 _reserveTokenOut = _tokenOut == token0 ? reserveToken0 : reserveToken1;
-        uint256 _tokenInDecimals = _tokenIn == token0 ? token0Decimals : token1Decimals;
-        uint256 _tokenOutDecimals = _tokenOut == token0 ? token0Decimals : token1Decimals;
 
-        uint256 amountOut = (_amountIn * (10 ** _tokenOutDecimals) * _reserveTokenOut)
-            / (_reserveTokenIn + _amountIn * (10 ** _tokenOutDecimals));
-        uint256 _fee = feeManager.getFee(SwapParams(_tokenIn, _tokenOut, _amountIn, _reserveTokenIn, _reserveTokenOut));
-        amountOut -= (_fee * (10 ** _tokenOutDecimals)) / (10 ** _tokenInDecimals);
+        if (_reserveTokenIn == 0 || _reserveTokenOut == 0) revert InsufficientLiquidity();
+        if (_amountIn >= _reserveTokenIn) revert InsufficientLiquidity();
+
+        // AMM calculation
+        uint256 amountOut = (_amountIn * _reserveTokenOut) / (_reserveTokenIn + _amountIn);
+
+        // Apply fee using FeeManager
+        ISwap.SwapParams memory swapParams = ISwap.SwapParams({
+            token0: _tokenIn,
+            token1: _tokenOut,
+            amount0: _amountIn,
+            reserveToken0: _reserveTokenIn,
+            reserveToken1: _reserveTokenOut
+        });
+
+        uint256 feeAmount = feeManager.getFee(swapParams);
+        amountOut = amountOut > feeAmount ? amountOut - feeAmount : 0;
 
         if (amountOut < _minAmountOut) revert InsufficientOutputAmount(_minAmountOut, amountOut);
-        if (amountOut > _reserveTokenOut) revert InsufficientLiquidity();
 
-        require(IERC20(_tokenIn).transferFrom(_msgSender, address(this), _amountIn));
-        require(IERC20(_tokenOut).transfer(_msgSender, amountOut));
+        require(IERC20(_tokenIn).transferFrom(tokenHolder, address(this), _amountIn));
+        require(IERC20(_tokenOut).transfer(tokenHolder, amountOut));
 
         if (_tokenIn == token0) {
             reserveToken0 += _amountIn;

src/Roles.sol

@@ -4,4 +4,5 @@ pragma solidity ^0.8.30;
 library Roles {
     bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
     bytes32 public constant MULTISIG_ADMIN_ROLE = keccak256("MULTISIG_ADMIN_ROLE");
+    bytes32 public constant ALLOWED_EIP712_SWAP_ROLE = keccak256("ALLOWED_EIP712_SWAP_ROLE");
 }