Pool fees setting should be per pool, not service level (#56)

* Move pool fee to pool * Add parameterized fixture
circlefin · Oct 21, 2022 · 56eaa58 · 56eaa58
1 parent 18dc192
commit 56eaa58
Show file tree

Hide file tree

Showing 11 changed files with 50 additions and 30 deletions.
diff --git a/contracts/Loan.sol b/contracts/Loan.sol
@@ -285,7 +285,7 @@ contract Loan is ILoan {
             .previewFees(
                 payment,
                 _serviceConfiguration.firstLossFeeBps(),
-                _serviceConfiguration.poolFeePercentOfInterest()
+                IPool(_pool).poolFeePercentOfInterest()
             );
 
         LoanLib.payFees(
@@ -313,7 +313,7 @@ contract Loan is ILoan {
             .previewFees(
                 amount,
                 _serviceConfiguration.firstLossFeeBps(),
-                _serviceConfiguration.poolFeePercentOfInterest()
+                IPool(_pool).poolFeePercentOfInterest()
             );
 
         LoanLib.payFees(

diff --git a/contracts/Pool.sol b/contracts/Pool.sol
@@ -250,6 +250,13 @@ contract Pool is IPool, ERC20 {
         return _accountings;
     }
 
+    /**
+     * @dev The fee
+     */
+    function poolFeePercentOfInterest() external view returns (uint256) {
+        return _poolSettings.poolFeePercentOfInterest;
+    }
+
     /**
      * @dev Supplies first-loss to the pool. Can only be called by the Pool Manager.
      */

diff --git a/contracts/PoolFactory.sol b/contracts/PoolFactory.sol
@@ -32,7 +32,8 @@ contract PoolFactory {
         uint256 endDate,
         uint256 requestFeeBps,
         uint256 withdrawGateBps,
-        uint256 withdrawRequestPeriodDuration
+        uint256 withdrawRequestPeriodDuration,
+        uint256 poolFeePercentOfInterest
     ) public virtual returns (address poolAddress) {
         require(
             _serviceConfiguration.paused() == false,
@@ -50,7 +51,8 @@ contract PoolFactory {
             requestFeeBps,
             withdrawGateBps,
             firstLossInitialMinimum,
-            withdrawRequestPeriodDuration
+            withdrawRequestPeriodDuration,
+            poolFeePercentOfInterest
         );
         Pool pool = new Pool(
             liquidityAsset,

diff --git a/contracts/ServiceConfiguration.sol b/contracts/ServiceConfiguration.sol
@@ -23,8 +23,6 @@ contract ServiceConfiguration is AccessControl, IServiceConfiguration {
 
     uint256 public firstLossFeeBps = 500;
 
-    uint256 public poolFeePercentOfInterest = 0;
-
     /**
      * @dev Holds a reference to valid LoanFactories
      */
@@ -91,11 +89,6 @@ contract ServiceConfiguration is AccessControl, IServiceConfiguration {
         emit ProtocolPaused(paused);
     }
 
-    function setPoolFeePercentOfInterest(uint256 amount) public onlyOperator {
-        poolFeePercentOfInterest = amount;
-        emit ParameterSet("pooFeePercentOfInterest", amount);
-    }
-
     /**
      * @dev Check that `msg.sender` is an Operator.
      */

diff --git a/contracts/interfaces/IPool.sol b/contracts/interfaces/IPool.sol
@@ -31,6 +31,7 @@ struct IPoolConfigurableSettings {
     uint256 withdrawGateBps; // Percent of liquidity pool available to withdraw, represented in BPS
     uint256 firstLossInitialMinimum; // amount
     uint256 withdrawRequestPeriodDuration; // seconds (e.g. 30 days)
+    uint256 poolFeePercentOfInterest; // bips
 }
 
 /**
@@ -134,6 +135,11 @@ interface IPool is IERC4626 {
      */
     function accountings() external view returns (IPoolAccountings memory);
 
+    /**
+     * @dev The pool fee, in bps, taken from each interest payment
+     */
+    function poolFeePercentOfInterest() external view returns (uint256);
+
     /**
      * @dev Deposits first-loss to the pool. Can only be called by the Pool Manager.
      */

diff --git a/contracts/interfaces/IServiceConfiguration.sol b/contracts/interfaces/IServiceConfiguration.sol
@@ -14,8 +14,6 @@ interface IServiceConfiguration is IAccessControl {
 
     function paused() external view returns (bool);
 
-    function poolFeePercentOfInterest() external view returns (uint256);
-
     function firstLossFeeBps() external view returns (uint256);
 
     function isLiquidityAsset(address addr) external view returns (bool);

diff --git a/contracts/permissioned/PermissionedPoolFactory.sol b/contracts/permissioned/PermissionedPoolFactory.sol
@@ -45,7 +45,8 @@ contract PermissionedPoolFactory is PoolFactory {
         uint256 endDate,
         uint256 requestFeeBps,
         uint256 withdrawGateBps,
-        uint256 withdrawRequestPeriodDuration
+        uint256 withdrawRequestPeriodDuration,
+        uint256 poolFeePercentOfInterest
     ) public override onlyVerifiedPoolManager returns (address poolAddress) {
         require(
             withdrawRequestPeriodDuration > 0,
@@ -60,7 +61,8 @@ contract PermissionedPoolFactory is PoolFactory {
             requestFeeBps,
             withdrawGateBps,
             firstLossInitialMinimum,
-            withdrawRequestPeriodDuration
+            withdrawRequestPeriodDuration,
+            poolFeePercentOfInterest
         );
         PermissionedPool pool = new PermissionedPool(
             liquidityAsset,

diff --git a/test/Loan.test.ts b/test/Loan.test.ts
@@ -14,7 +14,7 @@ describe("Loan", () => {
   const SEVEN_DAYS = 6 * 60 * 60 * 24;
   const THIRTY_DAYS = 30 * 60 * 60 * 24;
 
-  async function deployFixture() {
+  async function deployFixture(poolSettings = DEFAULT_POOL_SETTINGS) {
     // Contracts are deployed using the first signer/account by default
     const [operator, poolManager, borrower, lender, other] =
       await ethers.getSigners();
@@ -62,11 +62,12 @@ describe("Loan", () => {
       .connect(poolManager)
       .createPool(
         liquidityAsset.address,
-        DEFAULT_POOL_SETTINGS.maxCapacity,
-        DEFAULT_POOL_SETTINGS.endDate,
-        DEFAULT_POOL_SETTINGS.requestFeeBps,
-        DEFAULT_POOL_SETTINGS.withdrawGateBps,
-        DEFAULT_POOL_SETTINGS.withdrawRequestPeriodDuration
+        poolSettings.maxCapacity,
+        poolSettings.endDate,
+        poolSettings.requestFeeBps,
+        poolSettings.withdrawGateBps,
+        poolSettings.withdrawRequestPeriodDuration,
+        poolSettings.poolFeePercentOfInterest
       );
     const tx1Receipt = await tx1.wait();
 
@@ -147,6 +148,13 @@ describe("Loan", () => {
     };
   }
 
+  async function deployFixturePoolFees() {
+    const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, {
+      poolFeePercentOfInterest: 100
+    });
+    return deployFixture(poolSettings);
+  }
+
   describe("after initialization", () => {
     it("is initialized!", async () => {
       const { loan, pool, borrower, loanFactory } = await loadFixture(
@@ -889,15 +897,14 @@ describe("Loan", () => {
     });
 
     it("can collect pool fees from the next payment", async () => {
-      const fixture = await loadFixture(deployFixture);
+      const fixture = await loadFixture(deployFixturePoolFees);
       const {
         borrower,
         collateralAsset,
         liquidityAsset,
         loan,
         pool,
-        poolManager,
-        serviceConfiguration
+        poolManager
       } = fixture;
 
       // Setup
@@ -907,11 +914,11 @@ describe("Loan", () => {
         .postFungibleCollateral(collateralAsset.address, 100);
       await pool.connect(poolManager).fundLoan(loan.address);
       await loan.connect(borrower).drawdown();
+      expect(await pool.poolFeePercentOfInterest()).to.equal(100);
 
       // Make payment
       const firstLoss = await pool.firstLossVault();
       const feeVault = await pool.feeVault();
-      await serviceConfiguration.setPoolFeePercentOfInterest(100);
       const dueDate = await loan.paymentDueDate();
       expect(await loan.paymentsRemaining()).to.equal(6);
       await liquidityAsset.connect(borrower).approve(loan.address, 2083);

diff --git a/test/PoolFactory.test.ts b/test/PoolFactory.test.ts
@@ -46,7 +46,8 @@ describe("PoolFactory", () => {
         /* endDate */ 0,
         /* requestFeeBps */ 0,
         /* withdrawGateBps */ 0,
-        /* withdrawRequestPeriodDuration: */ 0
+        /* withdrawRequestPeriodDuration: */ 0,
+        /* poolFeePercentOfInterest */ 0
       )
     ).to.be.revertedWith("PoolFactory: Invalid duration");
   });
@@ -61,7 +62,8 @@ describe("PoolFactory", () => {
         /* endDate */ 0,
         /* requestFeeBps */ 0,
         /* withdrawGateBps */ 0,
-        /* withdrawRequestPeriodDuration: */ 1
+        /* withdrawRequestPeriodDuration: */ 1,
+        /* poolFeePercentOfInterest */ 0
       )
     ).to.emit(poolFactory, "PoolCreated");
   });

diff --git a/test/permissioned/PermissionedPoolFactory.test.ts b/test/permissioned/PermissionedPoolFactory.test.ts
@@ -82,7 +82,8 @@ describe("PermissionedPoolFactory", () => {
           /* endDate */ 0,
           /* requestFeeBps */ 0,
           /* withdrawGateBps */ 0,
-          /* withdrawRequestPeriodDuration: */ 1
+          /* withdrawRequestPeriodDuration: */ 1,
+          /* poolFeePercentOfInterest */ 0
         )
     ).to.emit(poolFactory, "PoolCreated");
   });
@@ -104,7 +105,8 @@ describe("PermissionedPoolFactory", () => {
         /* endDate */ 0,
         /* requestFeeBps */ 0,
         /* withdrawGateBps */ 0,
-        /* withdrawRequestPeriodDuration: */ 1
+        /* withdrawRequestPeriodDuration: */ 1,
+        /* poolFeePercentOfInterest */ 0
       )
     ).to.be.revertedWith("caller is not a pool manager");
   });

diff --git a/test/support/pool.ts b/test/support/pool.ts
@@ -9,7 +9,8 @@ export const DEFAULT_POOL_SETTINGS = {
   requestFeeBps: 500, // bps (1%)
   withdrawGateBps: 10_000, // bps (100%)
   firstLossInitialMinimum: 100_000,
-  withdrawRequestPeriodDuration: 30 * 24 * 60 * 60 // 30 days
+  withdrawRequestPeriodDuration: 30 * 24 * 60 * 60, // 30 days
+  poolFeePercentOfInterest: 0 // bps (0%)
 };
 
 /**