[VAL-43] Update PermissionedLoan to use override modifier pattern

contracts/Loan.sol

@@ -84,6 +84,14 @@ contract Loan is ILoan {
         _;
     }
 
+    /**
+     * @dev Modifier that can be overriden by derived classes to enforce
+     * access control.
+     */
+    modifier onlyPermittedBorrower() virtual {
+        _;
+    }
+
     /**
      * @dev Modifier that requires the loan not be in a terminal state.
      */
@@ -157,6 +165,7 @@ contract Loan is ILoan {
      */
     function cancelCollateralized()
         external
+        onlyPermittedBorrower
         onlyBorrower
         atState(ILoanLifeCycleState.Collateralized)
         returns (ILoanLifeCycleState)
@@ -198,8 +207,8 @@ contract Loan is ILoan {
     }
 
     /**
-     * @dev Claims specific collateral types. Can be called by the borrower (when Canceled or Matured)
-     * or by the PA (when Defaulted)
+     * @dev Claims specific collateral types. Can be called by the borrower
+     * (when Canceled or Matured) or by the PA (when Defaulted)
      */
     function claimCollateral(
         address[] memory assets,
@@ -226,8 +235,9 @@ contract Loan is ILoan {
      * @dev Post ERC20 tokens as collateral
      */
     function postFungibleCollateral(address asset, uint256 amount)
-        public
+        external
         virtual
+        onlyPermittedBorrower
         onlyBorrower
         onlyNonTerminalState
         returns (ILoanLifeCycleState)
@@ -251,8 +261,9 @@ contract Loan is ILoan {
      * @dev Post ERC721 tokens as collateral
      */
     function postNonFungibleCollateral(address asset, uint256 tokenId)
-        public
+        external
         virtual
+        onlyPermittedBorrower
         onlyBorrower
         onlyNonTerminalState
         returns (ILoanLifeCycleState)
@@ -309,8 +320,9 @@ contract Loan is ILoan {
      * @dev Drawdown the Loan
      */
     function drawdown(uint256 amount)
-        public
+        external
         virtual
+        onlyPermittedBorrower
         onlyBorrower
         returns (uint256)
     {
@@ -331,7 +343,11 @@ contract Loan is ILoan {
      * @dev Prepay principal.
      * @dev Only callable by open term loans
      */
-    function paydownPrincipal(uint256 amount) external onlyBorrower {
+    function paydownPrincipal(uint256 amount)
+        external
+        onlyPermittedBorrower
+        onlyBorrower
+    {
         require(outstandingPrincipal >= amount, "Loan: amount too high");
         require(settings.loanType == ILoanType.Open, "Loan: invalid loan type");
         LoanLib.paydownPrincipal(liquidityAsset, amount, fundingVault);
@@ -342,7 +358,8 @@ contract Loan is ILoan {
      * @dev Complete the next payment according to loan schedule inclusive of all fees.
      */
     function completeNextPayment()
-        public
+        external
+        onlyPermittedBorrower
         onlyBorrower
         atState(ILoanLifeCycleState.Active)
         returns (uint256)
@@ -401,7 +418,8 @@ contract Loan is ILoan {
      * @dev Complete the final payment of the loan.
      */
     function completeFullPayment()
-        public
+        external
+        onlyPermittedBorrower
         onlyBorrower
         atState(ILoanLifeCycleState.Active)
         returns (uint256)

contracts/permissioned/PermissionedLoan.sol

@@ -18,10 +18,10 @@ contract PermissionedLoan is Loan {
     /**
      * @dev a modifier to only allow valid borrowers to perform an action
      */
-    modifier onlyValidBorrower() {
+    modifier onlyPermittedBorrower() override {
         require(
             poolAccessControl.isAllowed(msg.sender),
-            "caller is not a valid borrower"
+            "BORROWER_NOT_ALLOWED"
         );
         _;
     }
@@ -50,40 +50,4 @@ contract PermissionedLoan is Loan {
     {
         poolAccessControl = PermissionedPool(pool).poolAccessControl();
     }
-
-    /**
-     * @inheritdoc Loan
-     */
-    function postFungibleCollateral(address asset, uint256 amount)
-        public
-        override
-        onlyValidBorrower
-        returns (ILoanLifeCycleState)
-    {
-        return super.postFungibleCollateral(asset, amount);
-    }
-
-    /**
-     * @inheritdoc Loan
-     */
-    function postNonFungibleCollateral(address asset, uint256 tokenId)
-        public
-        override
-        onlyValidBorrower
-        returns (ILoanLifeCycleState)
-    {
-        return super.postNonFungibleCollateral(asset, tokenId);
-    }
-
-    /**
-     * @inheritdoc Loan
-     */
-    function drawdown(uint256 amount)
-        public
-        override
-        onlyValidBorrower
-        returns (uint256)
-    {
-        return super.drawdown(amount);
-    }
 }

contracts/permissioned/PermissionedPool.sol

@@ -19,10 +19,7 @@ contract PermissionedPool is Pool {
      * @dev a modifier to only allow valid lenders to perform an action
      */
     modifier onlyPermittedLender() override {
-        require(
-            poolAccessControl.isAllowed(msg.sender),
-            "caller is not a valid lender"
-        );
+        require(poolAccessControl.isAllowed(msg.sender), "LENDER_NOT_ALLOWED");
         _;
     }
 

test/permissioned/PermissionedLoan.test.ts

@@ -186,7 +186,7 @@ describe("PermissionedLoan", () => {
           loan
             .connect(borrower)
             .postFungibleCollateral(liquidityAsset.address, 10)
-        ).to.be.revertedWith("caller is not a valid borrower");
+        ).to.be.revertedWith("BORROWER_NOT_ALLOWED");
       });
     });
 
@@ -200,7 +200,7 @@ describe("PermissionedLoan", () => {
           loan
             .connect(borrower)
             .postNonFungibleCollateral(liquidityAsset.address, "123")
-        ).to.be.revertedWith("caller is not a valid borrower");
+        ).to.be.revertedWith("BORROWER_NOT_ALLOWED");
       });
     });
 
@@ -240,9 +240,49 @@ describe("PermissionedLoan", () => {
           .connect(poolAdmin)
           .removeParticipant(borrower.address);
         await expect(loan.connect(borrower).drawdown(1)).to.be.revertedWith(
-          "caller is not a valid borrower"
+          "BORROWER_NOT_ALLOWED"
         );
       });
     });
+
+    describe("cancelCollateralized()", () => {
+      it("reverts if borrower not an allowed participant", async () => {
+        const { loan, borrower } = await loadFixture(loadLoanFixture);
+
+        await expect(
+          loan.connect(borrower).cancelCollateralized()
+        ).to.be.revertedWith("BORROWER_NOT_ALLOWED");
+      });
+    });
+
+    describe("paydownPrincipal()", () => {
+      it("reverts if borrower not an allowed participant", async () => {
+        const { loan, borrower } = await loadFixture(loadLoanFixture);
+
+        await expect(
+          loan.connect(borrower).paydownPrincipal(100)
+        ).to.be.revertedWith("BORROWER_NOT_ALLOWED");
+      });
+    });
+
+    describe("completeNextPayment()", () => {
+      it("reverts if borrower not an allowed participant", async () => {
+        const { loan, borrower } = await loadFixture(loadLoanFixture);
+
+        await expect(
+          loan.connect(borrower).completeNextPayment()
+        ).to.be.revertedWith("BORROWER_NOT_ALLOWED");
+      });
+    });
+
+    describe("completeFullPayment()", () => {
+      it("reverts if borrower not an allowed participant", async () => {
+        const { loan, borrower } = await loadFixture(loadLoanFixture);
+
+        await expect(
+          loan.connect(borrower).completeFullPayment()
+        ).to.be.revertedWith("BORROWER_NOT_ALLOWED");
+      });
+    });
   });
 });

test/permissioned/PermissionedPool.test.ts

@@ -99,7 +99,7 @@ describe("PermissionedPool", () => {
 
       await expect(
         pool.connect(otherAccount).deposit(10, otherAccount.address)
-      ).to.be.revertedWith("caller is not a valid lender");
+      ).to.be.revertedWith("LENDER_NOT_ALLOWED");
     });
 
     it("allows deposits if allowed lender", async () => {
@@ -126,7 +126,7 @@ describe("PermissionedPool", () => {
 
       await expect(
         pool.connect(otherAccount).mint(10, otherAccount.address)
-      ).to.be.revertedWith("caller is not a valid lender");
+      ).to.be.revertedWith("LENDER_NOT_ALLOWED");
     });
 
     it("allows minting if allowed lender", async () => {
@@ -155,7 +155,7 @@ describe("PermissionedPool", () => {
         pool
           .connect(otherAccount)
           .redeem(10, otherAccount.address, otherAccount.address)
-      ).to.be.revertedWith("caller is not a valid lender");
+      ).to.be.revertedWith("LENDER_NOT_ALLOWED");
     });
 
     it("allows redeeming if allowed lender", async () => {
@@ -187,7 +187,7 @@ describe("PermissionedPool", () => {
         pool
           .connect(otherAccount)
           .withdraw(10, otherAccount.address, otherAccount.address)
-      ).to.be.revertedWith("caller is not a valid lender");
+      ).to.be.revertedWith("LENDER_NOT_ALLOWED");
     });
 
     it("allows withdrawing if allowed lender", async () => {