test: add FeeCollector unit and integration tests (#8)

* feat: make FEE_COLLECTOR an immutable variable

* test: add FeeCollector unit tests

* test: add collectFees unit tests

* test: address requested changes

README.md

@@ -14,28 +14,30 @@ On-chain shorting via Aave and Uniswap.
 
 ## To-Do
 
--   [x] Impelemnt protocol fee
--   [x] Implement frontend incentive
--   [x] Account for case where there is no client (if no frontend, they would be able to pass their own address...)
--   [x] Should not repay more to Aave than what is owed when closing a position
-
-Unit Tests:
-
--   [x] Mock feel collector in tests
--   [x] Should not repay more to Aave than what is owed when closing a position
--   [ ] Test that the client's collected fee balance increased by the correct amount
--   [ ] Test that the totalClientBalance increased by the correct amount
--   [ ] Test that an admin you can set a clientRate
--   [ ] Test that a non-admin cannot set a clientRate
--   [ ] Test that a client can withdraw their collected fees
--   [ ] Test that extractERC20 works correctly on FeeCollector (it has different logic than the other contracts)
--   [ ] Test that a non-admin cannot extractERC20
--   [ ] Test that an admin can withdraw native
--   [ ] Test that a non-admin cannot withdraw native
--   [ ] Test that FeeCollector can recieve native
--   [ ] Test Fallback on FeeCollector
--   [ ] Test that the correct protocol fee is collected during a short (proabably separate test from Position.t.sol)
+Logic:
+
+-   [x] Make `FEE_COLLECTOR` an immutable variable in Position.sol and PositionFactory.sol
+-   [x] Add `owner()` and `clientRate()` to IFeeCollector
+
+Tests:
+
+-   [ ] Separate integration tests from unit tests (separate PR)
+-   [x] testFuzz_CollectFeesWithClient
+-   [x] testFuzz_CollectFeesNoClient
+-   [x] testFuzz_CollectFeesWithClientIntegrated
+-   [x] testFuzz_CollectFeesNoClientIntegrated
+-   [x] testFuzz_ClientWithdraw
+-   [x] testFuzz_SetClientRate
+-   [x] testFuzz_CannotSetClientRateOutOfRange
+-   [x] testFuzz_CannotSetClientRateUnauthorized
+-   [x] testFuzz_ExtractNative
+-   [x] testFuzz_CannotExtractNative
+-   [x] testFuzz_ExtractERC20
+-   [x] testFuzz_CannotExtractERC20
+-   [x] testFuzz_Receive
+-   [x] testFuzz_Fallback
 
 Considerations:
 
--   [ ] Consider emitting events through position factory
+-   [ ] Consider emitting Position events through another contract
+-   [ ] Consider adding a function to short with signatures, via `ERCRecover`.

src/Position.sol

@@ -14,18 +14,19 @@ import { IFeeCollector } from "src/interfaces/IFeeCollector.sol";
 contract Position is DebtService, SwapService {
     // Constants: no SLOAD to save gas
     uint256 public constant PROTOCOL_FEE = 3;
-    address private constant FEE_COLLECTOR = 0x2cD6D948263F20C3c27f181f14647840fC64b488;
 
     // Immutables: no SLOAD to save gas
+    address public immutable FEE_COLLECTOR;
     address public immutable B_TOKEN;
 
     // Events
     event Short(uint256 cAmt, uint256 dAmt, uint256 bAmt);
     event Close(uint256 gains);
 
-    constructor(address _owner, address _cToken, address _dToken, address _bToken)
+    constructor(address _owner, address _feeCollector, address _cToken, address _dToken, address _bToken)
         DebtService(_owner, _cToken, _dToken)
     {
+        FEE_COLLECTOR = _feeCollector;
         B_TOKEN = _bToken;
     }
 

src/PositionFactory.sol

@@ -14,6 +14,9 @@ contract PositionFactory is Ownable {
     // Constants: no SLOAD to save gas
     address private constant CONTRACT_DEPLOYER = 0x0a5B347509621337cDDf44CBCf6B6E7C9C908CD2;
 
+    // Immutables: no SLOAD to save gas
+    address public immutable FEE_COLLECTOR;
+
     // Factory Storage
     /// @dev Mapping from owner to cToken to dToken to bToken to position
     mapping(address => mapping(address => mapping(address => mapping(address => address)))) public positions;
@@ -23,8 +26,9 @@ contract PositionFactory is Ownable {
     error Unauthorized();
     error PositionExists();
 
-    constructor(address _owner) Ownable(_owner) {
+    constructor(address _owner, address _feeCollector) Ownable(_owner) {
         if (msg.sender != CONTRACT_DEPLOYER) revert Unauthorized();
+        FEE_COLLECTOR = _feeCollector;
     }
 
     /**
@@ -40,7 +44,7 @@ contract PositionFactory is Ownable {
     {
         if (positions[msg.sender][_cToken][_dToken][_bToken] != address(0)) revert PositionExists();
 
-        position = address(new Position(msg.sender, _cToken, _dToken, _bToken));
+        position = address(new Position(msg.sender, FEE_COLLECTOR, _cToken, _dToken, _bToken));
 
         positionsLookup[msg.sender].push(position);
         positions[msg.sender][_cToken][_dToken][_bToken] = position;

src/interfaces/IFeeCollector.sol

@@ -8,6 +8,16 @@ interface IFeeCollector {
     **  CORE FUNCTIONS
     **
     ******************************************************************************/
+    /**
+     * @notice Returns the owner of this contract.
+     */
+    function owner() external returns (address);
+
+    /**
+     * @notice Returns the current client rate.
+     */
+    function clientRate() external returns (uint256);
+
     /**
      * @notice Returns the total balance for the specified token across all client operators.
      * @param _token The token address to check.

src/interfaces/IPositionFactory.sol

@@ -12,7 +12,7 @@ interface IPositionFactory {
     /**
      * @notice Returns the owner of this contract.
      */
-    function OWNER() external returns (address);
+    function owner() external returns (address);
 
     /**
      * @notice Returns the address of an owner's specified Position contract.