[Bug] Un-stick funds in FeeVault via mediated access through the cont…

…roller (#171)
circlefin · Feb 6, 2023 · 5622ecc · 5622ecc
1 parent 3845714
commit 5622ecc
Show file tree

Hide file tree

Showing 5 changed files with 139 additions and 0 deletions.
diff --git a/contracts/Pool.sol b/contracts/Pool.sol
@@ -426,6 +426,18 @@ contract Pool is IPool, ERC20Upgradeable, BeaconImplementation {
         IERC20Upgradeable(_liquidityAsset).safeTransfer(recipient, fixedFee);
     }
 
+    /**
+     * @inheritdoc IPool
+     */
+    function withdrawFeeVault(uint256 amount, address receiver)
+        external
+        onlyNotPaused
+        onlyPoolController
+        onlySnapshottedPool
+    {
+        _feeVault.withdrawERC20(address(_liquidityAsset), amount, receiver);
+    }
+
     /*//////////////////////////////////////////////////////////////
                 Withdraw Controller Proxy Methods
     //////////////////////////////////////////////////////////////*/

diff --git a/contracts/controllers/PoolController.sol b/contracts/controllers/PoolController.sol
@@ -532,6 +532,18 @@ contract PoolController is IPoolController, BeaconImplementation {
         );
     }
 
+    /**
+     * @inheritdoc IPoolController
+     */
+    function withdrawFeeVault(uint256 amount, address receiver)
+        external
+        onlyNotPaused
+        onlyPermittedAdmin
+        onlyAdmin
+    {
+        pool.withdrawFeeVault(amount, receiver);
+    }
+
     /*//////////////////////////////////////////////////////////////
                                 Snapshot
     //////////////////////////////////////////////////////////////*/

diff --git a/contracts/controllers/interfaces/IPoolController.sol b/contracts/controllers/interfaces/IPoolController.sol
@@ -224,6 +224,12 @@ interface IPoolController {
      */
     function claimFixedFee() external;
 
+    /**
+     * @dev Called by the pool admin, this claims fees that have accumulated
+     * in the Pool's FeeVault from ongoing borrower payments.
+     */
+    function withdrawFeeVault(uint256 amount, address receiver) external;
+
     /*//////////////////////////////////////////////////////////////
                 Snapshot
     //////////////////////////////////////////////////////////////*/

diff --git a/contracts/interfaces/IPool.sol b/contracts/interfaces/IPool.sol
@@ -189,6 +189,11 @@ interface IPool is IERC4626, IRequestWithdrawable {
         uint256
     ) external;
 
+    /**
+     * @dev Called by the Pool Controller, it withdraws from the FeeVault.
+     */
+    function withdrawFeeVault(uint256 amount, address receiver) external;
+
     /**
      * @dev Calculate the total amount of underlying assets held by the vault,
      * excluding any assets due for withdrawal.

diff --git a/test/controllers/PoolController.test.ts b/test/controllers/PoolController.test.ts
@@ -116,6 +116,36 @@ describe("PoolController", () => {
     };
   }
 
+  async function loadPoolFixtureWithServiceFee() {
+    const { poolAdmin, pauser, borrower, otherAccount } =
+      await getCommonSigners();
+    const { pool, poolController, liquidityAsset, serviceConfiguration } =
+      await deployPool({
+        poolAdmin,
+        settings: { serviceFeeBps: 500 },
+        pauser
+      });
+
+    const { loan } = await deployLoan(
+      pool.address,
+      borrower.address,
+      liquidityAsset.address,
+      serviceConfiguration
+    );
+
+    return {
+      pool,
+      poolController,
+      liquidityAsset,
+      poolAdmin,
+      borrower,
+      otherAccount,
+      loan,
+      serviceConfiguration,
+      pauser
+    };
+  }
+
   describe("setRequestFee()", () => {
     it("sets the request fee in Bps", async () => {
       const { poolController, poolAdmin } = await loadFixture(loadPoolFixture);
@@ -1382,6 +1412,80 @@ describe("PoolController", () => {
     });
   });
 
+  describe("withdraw accumulated fees", () => {
+    it("claiming fees is only available to the pool admin", async () => {
+      const { poolController, otherAccount } = await loadFixture(
+        loadPoolFixtureWithServiceFee
+      );
+
+      const tx = poolController
+        .connect(otherAccount)
+        .withdrawFeeVault(1, otherAccount.address);
+      await expect(tx).to.be.revertedWith("Pool: caller is not admin");
+    });
+
+    it("reverts if the protocol is paused", async () => {
+      const { poolController, poolAdmin, serviceConfiguration, pauser } =
+        await loadFixture(loadPoolFixtureWithServiceFee);
+
+      await serviceConfiguration.connect(pauser).setPaused(true);
+
+      const tx = poolController
+        .connect(poolAdmin)
+        .withdrawFeeVault(1, poolAdmin.address);
+      await expect(tx).to.be.revertedWith("Pool: Protocol paused");
+    });
+
+    it("can withdraw accumulated fees", async () => {
+      const {
+        pool,
+        loan,
+        borrower,
+        poolController,
+        otherAccount,
+        poolAdmin,
+        liquidityAsset
+      } = await loadFixture(loadPoolFixtureWithServiceFee);
+
+      // Make payments on a loan to accumulate fees in the feevault
+
+      // Fund loan + drawdown
+      await activatePool(pool, poolAdmin, liquidityAsset);
+      await depositToPool(pool, otherAccount, liquidityAsset, 1_000_000);
+      await fundLoan(loan, poolController, poolAdmin);
+      await loan.connect(borrower).drawdown(await loan.principal());
+
+      // Make first payment
+      await time.increase(await loan.paymentPeriod());
+
+      // 4166 interest payment
+      // with 5% service fee sliced off for the FeeVault == 208
+      await liquidityAsset.connect(borrower).approve(loan.address, 4166);
+      await loan.connect(borrower).completeNextPayment();
+
+      // Check that fees have accumulated...
+      const feeVaultAddr = await pool.feeVault();
+      const balance = await liquidityAsset.balanceOf(feeVaultAddr);
+      expect(balance).to.equal(208);
+
+      // Check that the PA can claim the fee
+      const txn = await poolController
+        .connect(poolAdmin)
+        .withdrawFeeVault(208, poolAdmin.address);
+      await expect(txn).to.not.be.reverted;
+
+      // Check that the fee moved from the vault to the PA
+      await expect(txn).to.changeTokenBalances(
+        liquidityAsset,
+        [poolAdmin.address, feeVaultAddr],
+        [+208, -208]
+      );
+
+      // Vault should now be empty
+      expect(await liquidityAsset.balanceOf(feeVaultAddr)).to.equal(0);
+    });
+  });
+
   describe("Upgrades", () => {
     it("Can be upgraded", async () => {
       const { poolController, poolLib, poolControllerFactory, deployer } =