test: add protocol fee invariant test and client fee integration test

README.md

@@ -18,14 +18,15 @@ The following outlines principles for core protocol funcitonality.
 
 Logic:
 
--   Move PositionAdmin to services, rename
+-   [ ] Move PositionAdmin to services, rename
 -   [ ] Consider changing short() to add()
 -   [ ] Emit event when a position is created (get clear on whether or not an implicit event is emitted when creating a contract)
+-   [ ] Add comment in permit functions specifying that only ERC-2612 compliant tokens can use this functionality.
 
 Tests:
 
--   [ ] Invariant: the clientTakeRate + userTakeRate = clientRate
--   [ ] Invariant: the totalTokenAmt - sum(clientFeesToken) = (1 - clientRate) \* totalTokenAmt
+-   [x] Invariant: netProtocolFees = 1 - clientRate _ sum(clientTakeRate_i _ maxFee_i) ➡️ Test that netProtocolFees >= (1 - clientRate) \* totalBal
+-   [x] Integration: Test that the sum of all calculated client fees is equal to totalClientBalances
 -   [x] Unit test setClientTakeRate()
 -   [x] Unit test getClientAllocations()
 -   [x] Unit test FeeLib via Test Harness

src/FeeCollector.sol

@@ -62,12 +62,10 @@ contract FeeCollector is Ownable {
     /**
      * @notice Allows clients to set the percentage of the clientRate they will receive each revenue-generating tx.
      *         Amounts less than 100 will give the calling client's users a protocol fee discount:
-     *         clientTakeRateOfProtocolFee = clientRate * _clientTakeRate
-     *              ex: _clientTakeRate = 50% -> clientTakeRate = clientRate * 0.5
-     *         userTakeRateOfProtocolFee =  clientRate * (1 - _clientTakeRate)
-     *              ex: _clientTakeRate = 50% -> userTakeRate = clientRate * (1 - 0.5)
-     *         clientFee = protocolFee * clientTakeRateOfProtocolFee
-     *         userSavings = protocolFee * userTakeRateOfProtocolFee
+     *         clientPercentOfProtocolFee = clientRate * _clientTakeRate
+     *         userPercentOfProtocolFee =  clientRate * (1 - _clientTakeRate)
+     *         clientFee = protocolFee * clientPercentOfProtocolFee
+     *         userSavings = protocolFee * userPercentOfProtocolFee
      * @param _clientTakeRate The percentage of the clientRate the client will receive each revenue-generating tx (100 = 100%).
      */
     function setClientTakeRate(uint256 _clientTakeRate) public payable {
@@ -76,9 +74,12 @@ contract FeeCollector is Ownable {
     }
 
     /**
-     * @notice Returns the amount discounted from the protocol fee by using the provided client.
+     * @notice Returns the amount discounted from the protocol fee for using the provided client,
+     *         and the amount of fees the client will receive.
      * @param _client The address where a client operator will receive protocols fees.
      * @param _maxFee The maximum amount of fees the protocol will collect.
+     * @return userSavings The amount of fees discounted from the protocol fee.
+     * @return clientFee The amount of fees the client will receive.
      */
     function getClientAllocations(address _client, uint256 _maxFee)
         public
@@ -87,8 +88,8 @@ contract FeeCollector is Ownable {
     {
         // 1. Calculate user savings
         uint256 userTakeRate = 100 - clientTakeRates[_client];
-        uint256 userPercentOfProtocolFee = (userTakeRate * clientRate) / 100;
-        userSavings = (userPercentOfProtocolFee * _maxFee) / 100;
+        uint256 userPercentOfProtocolFee = (userTakeRate * clientRate);
+        userSavings = (userPercentOfProtocolFee * _maxFee) / 1e4;
 
         // 2. Calculate client fee
         uint256 maxClientFee = (_maxFee * clientRate) / 100;

src/Position.sol

@@ -28,11 +28,11 @@ contract Position is DebtService, SwapService {
 
     /**
      * @notice Adds to this contract's short position.
-     * @param _cAmt The amount of collateral to be supplied for this transaction-specific loan (units: C_DECIMALS).
+     * @param _cAmt The amount of collateral token to be supplied for this transaction-specific loan (units: C_DECIMALS).
      * @param _ltv The desired loan-to-value ratio for this transaction-specific loan (ex: 75 is 75%).
      * @param _swapAmtOutMin The minimum amount of output tokens from swap for the tx to go through.
      * @param _poolFee The fee of the Uniswap pool.
-     * @param _client The address where a client operator will receive protocols fees. (use address(0) if no client).
+     * @param _client The address of the client operator. Use address(0) if not using a client.
      */
     function short(uint256 _cAmt, uint256 _ltv, uint256 _swapAmtOutMin, uint24 _poolFee, address _client)
         public
@@ -57,15 +57,15 @@ contract Position is DebtService, SwapService {
 
     /**
      * @notice Adds to this contract's short position with permit, obviating the need for a separate approve tx.
-     * @param _cAmt The amount of collateral to be supplied for this transaction-specific loan (units: C_DECIMALS).
+     * @param _cAmt The amount of collateral token to be supplied for this transaction-specific loan (units: C_DECIMALS).
      * @param _ltv The desired loan-to-value ratio for this transaction-specific loan (ex: 75 is 75%).
      * @param _swapAmtOutMin The minimum amount of output tokens from swap for the tx to go through.
      * @param _poolFee The fee of the Uniswap pool.
-     * @param _client The address where a client operator will receive protocols fees. (use address(0) if no client).
-     * @param _deadline The deadline timestamp that the permit is valid.
-     * @param _v The V parameter of ERC712 permit signature.
-     * @param _r The R parameter of ERC712 permit signature.
-     * @param _s The S parameter of ERC712 permit signature.
+     * @param _client The address of the client operator. Use address(0) if not using a client.
+     * @param _deadline The expiration timestamp of the permit.
+     * @param _v The V parameter of ERC712 signature for the permit.
+     * @param _r The R parameter of ERC712 signature for the permit.
+     * @param _s The S parameter of ERC712 signature for the permit.
      */
     function shortWithPermit(
         uint256 _cAmt,

src/interfaces/IFeeCollector.sol

@@ -49,22 +49,23 @@ interface IFeeCollector {
     /**
      * @notice Allows clients to set the percentage of the clientRate they will receive each revenue-generating tx.
      *         Amounts less than 100 will give the calling client's users a protocol fee discount:
-     *         clientTakeRateOfProtocolFee = clientRate * _clientTakeRate
-     *              ex: _clientTakeRate = 50% -> clientTakeRate = clientRate * 0.5
-     *         userTakeRateOfProtocolFee =  clientRate * (1 - _clientTakeRate)
-     *              ex: _clientTakeRate = 50% -> userTakeRate = clientRate * (1 - 0.5)
-     *         clientFee = protocolFee * clientTakeRateOfProtocolFee
-     *         userSavings = protocolFee * userTakeRateOfProtocolFee
+     *         clientPercentOfProtocolFee = clientRate * _clientTakeRate
+     *         userPercentOfProtocolFee =  clientRate * (1 - _clientTakeRate)
+     *         clientFee = protocolFee * clientPercentOfProtocolFee
+     *         userSavings = protocolFee * userPercentOfProtocolFee
      * @param _clientTakeRate The percentage of the clientRate the client will receive each revenue-generating tx (100 = 100%).
      */
     function setClientTakeRate(uint256 _clientTakeRate) external payable;
 
     /**
-     * @notice Returns the amount discounted from the protocol fee by using the provided client.
+     * @notice Returns the amount discounted from the protocol fee for using the provided client,
+     *         and the amount of fees the client will receive.
      * @param _client The address where a client operator will receive protocols fees.
-     * @param _protocolFee The maximum amount of fees the protocol will collect.
+     * @param _maxFee The maximum amount of fees the protocol will collect.
+     * @return userSavings The amount of fees discounted from the protocol fee.
+     * @return clientFee The amount of fees the client will receive.
      */
-    function getClientAllocations(address _client, uint256 _protocolFee)
+    function getClientAllocations(address _client, uint256 _maxFee)
         external
         view
         returns (uint256 userSavings, uint256 clientFee);

src/interfaces/IPosition.sol

@@ -32,27 +32,27 @@ interface IPosition {
     ******************************************************************************/
     /**
      * @notice Adds to this contract's short position.
-     * @param _cAmt The amount of collateral to be supplied for this transaction-specific loan (units: C_DECIMALS).
+     * @param _cAmt The amount of collateral token to be supplied for this transaction-specific loan (units: C_DECIMALS).
      * @param _ltv The desired loan-to-value ratio for this transaction-specific loan (ex: 75 is 75%).
      * @param _swapAmtOutMin The minimum amount of output tokens from swap for the tx to go through.
      * @param _poolFee The fee of the Uniswap pool.
-     * @param _client The address where a client operator will receive protocols fees. (use address(0) if no client).
+     * @param _client The address of the client operator. Use address(0) if not using a client.
      */
     function short(uint256 _cAmt, uint256 _ltv, uint256 _swapAmtOutMin, uint24 _poolFee, address _client)
         external
         payable;
 
     /**
      * @notice Adds to this contract's short position with permit, obviating the need for a separate approve tx.
-     * @param _cAmt The amount of collateral to be supplied for this transaction-specific loan (units: C_DECIMALS).
+     * @param _cAmt The amount of collateral token to be supplied for this transaction-specific loan (units: C_DECIMALS).
      * @param _ltv The desired loan-to-value ratio for this transaction-specific loan (ex: 75 is 75%).
      * @param _swapAmtOutMin The minimum amount of output tokens from swap for the tx to go through.
      * @param _poolFee The fee of the Uniswap pool.
-     * @param _client The address where a client operator will receive protocols fees. (use address(0) if no client).
-     * @param _deadline The deadline timestamp that the permit is valid.
-     * @param _v The V parameter of ERC712 permit signature.
-     * @param _r The R parameter of ERC712 permit signature.
-     * @param _s The S parameter of ERC712 permit signature.
+     * @param _client The address of the client operator. Use address(0) if not using a client.
+     * @param _deadline The expiration timestamp of the permit.
+     * @param _v The V parameter of ERC712 signature for the permit.
+     * @param _r The R parameter of ERC712 signature for the permit.
+     * @param _s The S parameter of ERC712 signature for the permit.
      */
     function shortWithPermit(
         uint256 _cAmt,
@@ -78,18 +78,18 @@ interface IPosition {
         payable;
 
     /**
-     * @notice Increases the collateral amount for this contract's loan.
+     * @notice Increases the collateral amount backing this contract's loan.
      * @param _cAmt The amount of collateral to be supplied (units: C_DECIMALS).
      */
     function addCollateral(uint256 _cAmt) external payable;
 
     /**
      * @notice Increases the collateral amount for this contract's loan with permit, obviating the need for a separate approve tx.
      * @param _cAmt The amount of collateral to be supplied (units: C_DECIMALS).
-     * @param _deadline The deadline timestamp that the permit is valid.
-     * @param _v The V parameter of ERC712 permit signature.
-     * @param _r The R parameter of ERC712 permit signature.
-     * @param _s The S parameter of ERC712 permit signature.
+     * @param _deadline The expiration timestamp of the permit.
+     * @param _v The V parameter of ERC712 signature for the permit.
+     * @param _r The R parameter of ERC712 signature for the permit.
+     * @param _s The S parameter of ERC712 signature for the permit.
      */
     function addCollateralWithPermit(uint256 _cAmt, uint256 _deadline, uint8 _v, bytes32 _r, bytes32 _s)
         external
@@ -109,10 +109,10 @@ interface IPosition {
      * @param _dAmt The amount of debt token to repay to Aave (units: D_DECIMALS).
      *              To pay off entire debt, _dAmt = debtOwed + smallBuffer (to account for interest).
      * @param _withdrawBuffer The amount of collateral left as safety buffer for tx to go through (default = 100_000, units: 8 decimals).
-     * @param _deadline The deadline timestamp that the permit is valid.
-     * @param _v The V parameter of ERC712 permit signature.
-     * @param _r The R parameter of ERC712 permit signature.
-     * @param _s The S parameter of ERC712 permit signature.
+     * @param _deadline The expiration timestamp of the permit.
+     * @param _v The V parameter of ERC712 signature for the permit.
+     * @param _r The R parameter of ERC712 signature for the permit.
+     * @param _s The S parameter of ERC712 signature for the permit.
      */
     function repayAfterCloseWithPermit(
         uint256 _dAmt,

src/interfaces/aave/IPool.sol

@@ -254,12 +254,12 @@ interface IPool {
      * @param onBehalfOf The address that will receive the aTokens, same as msg.sender if the user
      *   wants to receive them on his own wallet, or a different address if the beneficiary of aTokens
      *   is a different wallet
-     * @param deadline The deadline timestamp that the permit is valid.
+     * @param deadline The expiration timestamp of the permit.
      * @param referralCode Code used to register the integrator originating the operation, for potential rewards.
      *   0 if the action is executed directly by the user, without any middle-man
-     * @param permitV The V parameter of ERC712 permit signature.
-     * @param permitR The R parameter of ERC712 permit signature.
-     * @param permitS The S parameter of ERC712 permit signature.
+     * @param permitV The V parameter of ERC712 signature for the permit.
+     * @param permitR The R parameter of ERC712 signature for the permit.
+     * @param permitS The S parameter of ERC712 signature for the permit.
      *
      */
     function supplyWithPermit(
@@ -333,10 +333,10 @@ interface IPool {
      * @param onBehalfOf Address of the user who will get his debt reduced/removed. Should be the address of the
      * user calling the function if he wants to reduce/remove his own debt, or the address of any other
      * other borrower whose debt should be removed
-     * @param deadline The deadline timestamp that the permit is valid.
-     * @param permitV The V parameter of ERC712 permit signature.
-     * @param permitR The R parameter of ERC712 permit signature.
-     * @param permitS The S parameter of ERC712 permit signature.
+     * @param deadline The expiration timestamp of the permit.
+     * @param permitV The V parameter of ERC712 signature for the permit.
+     * @param permitR The R parameter of ERC712 signature for the permit.
+     * @param permitS The S parameter of ERC712 signature for the permit.
      * @return The final amount repaid
      *
      */

src/services/DebtService.sol

@@ -112,7 +112,7 @@ contract DebtService is PositionAdmin {
     }
 
     /**
-     * @notice Increases the collateral amount for this contract's loan.
+     * @notice Increases the collateral amount backing this contract's loan.
      * @param _cAmt The amount of collateral to be supplied (units: C_DECIMALS).
      */
     function addCollateral(uint256 _cAmt) public payable onlyOwner {
@@ -129,10 +129,10 @@ contract DebtService is PositionAdmin {
     /**
      * @notice Increases the collateral amount for this contract's loan with permit, obviating the need for a separate approve tx.
      * @param _cAmt The amount of collateral to be supplied (units: C_DECIMALS).
-     * @param _deadline The deadline timestamp that the permit is valid.
-     * @param _v The V parameter of ERC712 permit signature.
-     * @param _r The R parameter of ERC712 permit signature.
-     * @param _s The S parameter of ERC712 permit signature.
+     * @param _deadline The expiration timestamp of the permit.
+     * @param _v The V parameter of ERC712 signature for the permit.
+     * @param _r The R parameter of ERC712 signature for the permit.
+     * @param _s The S parameter of ERC712 signature for the permit.
      */
     function addCollateralWithPermit(uint256 _cAmt, uint256 _deadline, uint8 _v, bytes32 _r, bytes32 _s)
         public
@@ -166,10 +166,10 @@ contract DebtService is PositionAdmin {
      * @param _dAmt The amount of debt token to repay to Aave (units: D_DECIMALS).
      *              To pay off entire debt, _dAmt = debtOwed + smallBuffer (to account for interest).
      * @param _withdrawBuffer The amount of collateral left as safety buffer for tx to go through (default = 100_000, units: 8 decimals).
-     * @param _deadline The deadline timestamp that the permit is valid.
-     * @param _v The V parameter of ERC712 permit signature.
-     * @param _r The R parameter of ERC712 permit signature.
-     * @param _s The S parameter of ERC712 permit signature.
+     * @param _deadline The expiration timestamp of the permit.
+     * @param _v The V parameter of ERC712 signature for the permit.
+     * @param _r The R parameter of ERC712 signature for the permit.
+     * @param _s The S parameter of ERC712 signature for the permit.
      */
     function repayAfterCloseWithPermit(
         uint256 _dAmt,

test/FeeCollector.t.sol

@@ -295,15 +295,16 @@ contract FeeCollectorTest is Test, TokenUtils, FeeUtils {
 
             // Expectations
             uint256 maxClientFee = (CLIENT_RATE * _maxFee) / 100;
-            (uint256 expectedUserSavings, uint256 expectedClientFee) =
-                _getExpectedClientAllocations(_maxFee, _clientTakeRate);
+            uint256 userTakeRate = 100 - _clientTakeRate;
+            uint256 expectedClientFee = (_clientTakeRate * CLIENT_RATE * _maxFee) / 1e4;
+            uint256 expectedUserSavings = (userTakeRate * CLIENT_RATE * _maxFee) / 1e4;
 
             // Act
             (uint256 userSavings, uint256 clientFee) = feeCollector.getClientAllocations(TEST_CLIENT, _maxFee);
 
             // Assertions
-            assertEq(userSavings, expectedUserSavings);
-            assertEq(clientFee, expectedClientFee);
+            assertApproxEqAbs(userSavings, expectedUserSavings, 1);
+            assertApproxEqAbs(clientFee, expectedClientFee, 1);
             assertEq(userSavings + clientFee, maxClientFee);
             assertLe(userSavings, maxClientFee);
             assertLe(clientFee, maxClientFee);

test/common/utils/FeeUtils.t.sol

@@ -14,8 +14,8 @@ contract FeeUtils is Test {
         returns (uint256 userSavings, uint256 clientFee)
     {
         uint256 userTakeRate = 100 - _clientTakeRate;
-        uint256 userPercentOfProtocolFee = (userTakeRate * CLIENT_RATE) / 100;
-        userSavings = (userPercentOfProtocolFee * _maxFee) / 100;
+        uint256 userPercentOfProtocolFee = (userTakeRate * CLIENT_RATE);
+        userSavings = (userPercentOfProtocolFee * _maxFee) / 1e4;
 
         // 2. Calculate client fee
         uint256 maxClientFee = (_maxFee * CLIENT_RATE) / 100;