Fix lint warnings and type errors (and check types in CI) (#145)

* Fix lint in typescript files * Update types to allow permissioned pools for activation * Fix solidity lint warnings * Add type-check script * Fix type check, add to github actions * Fix format * Compile before type-check
circlefin · Dec 9, 2022 · f458dfb · f458dfb
1 parent f7e3198
commit f458dfb
Show file tree

Hide file tree

Showing 30 changed files with 77 additions and 99 deletions.
diff --git a/.github/workflows/verify.yml b/.github/workflows/verify.yml
@@ -14,8 +14,11 @@ jobs:
       - uses: actions/checkout@v3
       - uses: actions/setup-node@v3
         with:
+          node-version: 16
           cache: "npm"
       - run: npm install
+      - run: npx hardhat compile
       - run: npm run format:check
       - run: npm run lint
+      - run: npm run type-check
       - run: npm test
diff --git a/.solhint.json b/.solhint.json
@@ -2,6 +2,10 @@
   "extends": "solhint:recommended",
   "rules": {
     "compiler-version": ["error", "^0.8.16"],
-    "func-visibility": ["warn", { "ignoreConstructors": true }]
+    "func-name-mixedcase": "off",
+    "func-visibility": ["warn", { "ignoreConstructors": true }],
+    "no-empty-blocks": "off",
+    "not-rely-on-time": "off",
+    "reason-string": ["warn", { "maxLength": 50 }]
   }
 }
diff --git a/contracts/Loan.sol b/contracts/Loan.sol
@@ -17,7 +17,6 @@ import "./upgrades/BeaconImplementation.sol";
  */
 contract Loan is ILoan, BeaconImplementation {
     using SafeMath for uint256;
-    uint256 constant RAY = 10**27;
 
     IServiceConfiguration private _serviceConfiguration;
     address private _factory;
@@ -153,10 +152,12 @@ contract Loan is ILoan, BeaconImplementation {
         uint256 paymentsTotal = settings
             .principal
             .mul(settings.apr)
-            .mul(settings.duration.mul(RAY).div(360))
-            .div(RAY)
+            .mul(settings.duration.mul(LoanLib.RAY).div(360))
+            .div(LoanLib.RAY)
             .div(10000);
-        payment = paymentsTotal.mul(RAY).div(paymentsRemaining).div(RAY);
+        payment = paymentsTotal.mul(LoanLib.RAY).div(paymentsRemaining).div(
+            LoanLib.RAY
+        );
     }
 
     /**
@@ -393,7 +394,7 @@ contract Loan is ILoan, BeaconImplementation {
             IPool(_pool).serviceFeeBps(),
             block.timestamp,
             paymentDueDate,
-            RAY
+            LoanLib.RAY
         );
 
         LoanLib.payFees(
@@ -430,7 +431,7 @@ contract Loan is ILoan, BeaconImplementation {
                 IPool(_pool).serviceFeeBps(),
                 block.timestamp,
                 paymentDueDate,
-                RAY
+                LoanLib.RAY
             );
     }
 
@@ -445,23 +446,23 @@ contract Loan is ILoan, BeaconImplementation {
         atState(ILoanLifeCycleState.Active)
         returns (uint256)
     {
-        uint256 scalingValue = RAY;
+        uint256 scalingValue = LoanLib.RAY;
 
         if (settings.loanType == ILoanType.Open) {
             // If an open term loan payment is not overdue, we will prorate the
             // payment
             if (paymentDueDate > block.timestamp) {
                 // Calculate the scaling value
-                // RAY - ((paymentDueDate - blocktimestamp) * RAY / paymentPeriod (seconds))
-                scalingValue = RAY.sub(
-                    (paymentDueDate - block.timestamp).mul(RAY).div(
+                // LoanLib.RAY - ((paymentDueDate - blocktimestamp) * LoanLib.RAY / paymentPeriod (seconds))
+                scalingValue = LoanLib.RAY.sub(
+                    (paymentDueDate - block.timestamp).mul(LoanLib.RAY).div(
                         settings.paymentPeriod * 1 days
                     )
                 );
             }
         } else {
             // Fixed term loans must pay all outstanding interest payments and fees.
-            scalingValue = RAY.mul(paymentsRemaining);
+            scalingValue = LoanLib.RAY.mul(paymentsRemaining);
         }
 
         ILoanFees memory _fees = LoanLib.previewFees(

diff --git a/contracts/libraries/LoanLib.sol b/contracts/libraries/LoanLib.sol
@@ -15,7 +15,7 @@ library LoanLib {
     using SafeERC20 for IERC20;
     using SafeMath for uint256;
 
-    uint256 constant RAY = 10**27;
+    uint256 public constant RAY = 10**27;
 
     /**
      * @dev Emitted when loan is funded.

diff --git a/package.json b/package.json
@@ -11,7 +11,8 @@
     "lint:solidity": "solhint contracts/**/*.sol",
     "test": "npx hardhat test",
     "test:coverage": "npx hardhat coverage",
-    "test:gas": "REPORT_GAS=true npx hardhat test"
+    "test:gas": "REPORT_GAS=true npx hardhat test",
+    "type-check": "tsc --pretty --noEmit"
   },
   "devDependencies": {
     "@nomicfoundation/hardhat-toolbox": "^2.0.0",

diff --git a/scripts/deploy-permissionless.ts b/scripts/deploy-permissionless.ts
@@ -1,9 +1,15 @@
 import { ethers } from "hardhat";
 import hre from "hardhat";
 
+type ExtendedHreNetworkConfig = typeof hre.network.config & {
+  usdcAddress: string | undefined;
+};
+
 async function main() {
   // The token we use for the liquidity asset must exist. If it is not defined, we'll deploy a mock token.
-  let usdcAddress = hre.network.config.usdcAddress;
+  let usdcAddress = (hre.network.config as ExtendedHreNetworkConfig)
+    .usdcAddress;
+
   if (!usdcAddress) {
     const Usdc = await ethers.getContractFactory("MockERC20");
     const usdc = await Usdc.deploy("USD Coin", "USDC", 6);

diff --git a/scripts/deploy.ts b/scripts/deploy.ts
@@ -1,9 +1,15 @@
 import { ethers } from "hardhat";
 import hre from "hardhat";
 
+type ExtendedHreNetworkConfig = typeof hre.network.config & {
+  usdcAddress: string | undefined;
+};
+
 async function main() {
   // The token we use for the liquidity asset must exist. If it is not defined, we'll deploy a mock token.
-  let usdcAddress = hre.network.config.usdcAddress;
+  let usdcAddress = (hre.network.config as ExtendedHreNetworkConfig)
+    .usdcAddress;
+
   if (!usdcAddress) {
     const Usdc = await ethers.getContractFactory("MockERC20");
     const usdc = await Usdc.deploy("USD Coin", "USDC", 6);
@@ -31,6 +37,8 @@ async function main() {
     "ToSAcceptanceRegistry"
   );
   const toSAcceptanceRegistry = await ToSAcceptanceRegistry.deploy(
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
     serviceConfiguration.address
   );
   await toSAcceptanceRegistry.deployed();
@@ -60,6 +68,8 @@ async function main() {
     "PoolAdminAccessControl"
   );
   const poolAdminAccessControl = await PoolAdminAccessControl.deploy(
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
     serviceConfiguration.address
   );
   await poolAdminAccessControl.deployed();

diff --git a/test/FirstLossVault.test.ts b/test/FirstLossVault.test.ts
@@ -1,7 +1,6 @@
 import { loadFixture } from "@nomicfoundation/hardhat-network-helpers";
 import { expect } from "chai";
 import { ethers } from "hardhat";
-import { deployServiceConfiguration } from "./support/serviceconfiguration";
 
 describe("FirstLossVault", () => {
   const VAULT_BALANCE = ethers.BigNumber.from(100);

diff --git a/test/controllers/PoolController.test.ts b/test/controllers/PoolController.test.ts
@@ -905,7 +905,7 @@ describe("PoolController", () => {
 
       // double check that the funds are now available for withdraw
       expect(await pool.maxRedeem(otherAccount.address)).to.equal(
-        redeemAmount - 2
+        redeemAmount.sub(2)
       );
 
       // check that totalAvailableAssets is dust

diff --git a/test/permissioned/PermissionedLoan.test.ts b/test/permissioned/PermissionedLoan.test.ts
@@ -19,7 +19,7 @@ import { getCommonSigners } from "../support/utils";
 
 describe("PermissionedLoan", () => {
   async function loadLoanFixture() {
-    const { operator, poolAdmin, borrower, lender } = await getCommonSigners();
+    const { poolAdmin, borrower, lender } = await getCommonSigners();
 
     const { mockERC20: liquidityAsset } = await deployMockERC20();
     const NftAsset = await ethers.getContractFactory("MockERC721");

diff --git a/test/permissioned/PermissionedLoanFactory.test.ts b/test/permissioned/PermissionedLoanFactory.test.ts
@@ -3,15 +3,13 @@ import { expect } from "chai";
 import { ethers } from "hardhat";
 import { deployMockERC20 } from "../support/erc20";
 import { deployPermissionedPool, DEFAULT_POOL_SETTINGS } from "../support/pool";
-import { DEFAULT_LOAN_SETTINGS, deployPermissionedLoan } from "../support/loan";
-import { deployPermissionedServiceConfiguration } from "../support/serviceconfiguration";
-import { deployToSAcceptanceRegistry } from "../support/tosacceptanceregistry";
+import { DEFAULT_LOAN_SETTINGS } from "../support/loan";
 import { findEventByName, getCommonSigners } from "../support/utils";
 
 describe("PermissionedLoanFactory", () => {
   async function deployFixture() {
     // Contracts are deployed using the first signer/account by default
-    const { operator, poolAdmin, borrower, otherAccounts, deployer } =
+    const { operator, poolAdmin, borrower, deployer } =
       await getCommonSigners();
 
     // Deploy the liquidity asset

diff --git a/test/permissioned/PermissionedPool.test.ts b/test/permissioned/PermissionedPool.test.ts
@@ -11,24 +11,16 @@ import {
 
 describe("PermissionedPool", () => {
   async function loadPoolFixture() {
-    const [
-      operator,
-      protocolAdmin,
-      poolAdmin,
-      otherAccount,
-      thirdAccount,
-      allowedLender
-    ] = await ethers.getSigners();
+    const [poolAdmin, otherAccount, thirdAccount, allowedLender] =
+      await ethers.getSigners();
     const {
       pool,
       liquidityAsset,
       poolAccessControl,
       tosAcceptanceRegistry,
       poolController
     } = await deployPermissionedPool({
-      operator,
-      poolAdmin: poolAdmin,
-      protocolAdmin: protocolAdmin
+      poolAdmin: poolAdmin
     });
 
     // allow allowedLender

diff --git a/test/scenarios/business/1.test.ts b/test/scenarios/business/1.test.ts
@@ -51,7 +51,7 @@ describe("Business Scenario 1", () => {
   }
 
   async function loadFixtures() {
-    const [operator, poolAdmin, lenderA, lenderB, borrowerOne, borrowerTwo] =
+    const [poolAdmin, lenderA, lenderB, borrowerOne, borrowerTwo] =
       await ethers.getSigners();
 
     const startTime = await time.latest();
@@ -66,7 +66,6 @@ describe("Business Scenario 1", () => {
       6
     );
     const { pool, serviceConfiguration, poolController } = await deployPool({
-      operator,
       poolAdmin: poolAdmin,
       settings: poolSettings,
       liquidityAsset: mockUSDC

diff --git a/test/scenarios/business/3.test.ts b/test/scenarios/business/3.test.ts
@@ -39,8 +39,7 @@ describe("Business Scenario 3", () => {
   }
 
   async function fixtures() {
-    const [operator, poolAdmin, lenderA, lenderB, borrower] =
-      await ethers.getSigners();
+    const [poolAdmin, lenderA, lenderB, borrower] = await ethers.getSigners();
     const endTime = (await time.latest()) + 5_184_000; // 60 days.
     const poolSettings = {
       endDate: endTime, // Jan 1, 2050
@@ -52,7 +51,6 @@ describe("Business Scenario 3", () => {
       6
     );
     const { pool, serviceConfiguration, poolController } = await deployPool({
-      operator,
       poolAdmin: poolAdmin,
       settings: poolSettings,
       liquidityAsset: mockUSDC

diff --git a/test/scenarios/business/4.test.ts b/test/scenarios/business/4.test.ts
@@ -39,8 +39,7 @@ describe("Business Scenario 4", () => {
   }
 
   async function fixtures() {
-    const [operator, poolAdmin, lenderA, lenderB, borrower] =
-      await ethers.getSigners();
+    const [poolAdmin, lenderA, lenderB, borrower] = await ethers.getSigners();
     const endTime = (await time.latest()) + 5_184_000; // 60 days.
     const poolSettings = {
       endDate: endTime, // Jan 1, 2050
@@ -52,7 +51,6 @@ describe("Business Scenario 4", () => {
       6
     );
     const { pool, serviceConfiguration, poolController } = await deployPool({
-      operator,
       poolAdmin: poolAdmin,
       settings: poolSettings,
       liquidityAsset: mockUSDC

diff --git a/test/scenarios/business/permissioned/1.test.ts b/test/scenarios/business/permissioned/1.test.ts
@@ -54,7 +54,7 @@ describe("Permissioned Business Scenario 1", () => {
   }
 
   async function loadFixtures() {
-    const [operator, poolAdmin, lenderA, lenderB, borrowerOne, borrowerTwo] =
+    const [poolAdmin, lenderA, lenderB, borrowerOne, borrowerTwo] =
       await ethers.getSigners();
 
     const startTime = await time.latest();
@@ -75,7 +75,6 @@ describe("Permissioned Business Scenario 1", () => {
       poolAccessControl,
       tosAcceptanceRegistry
     } = await deployPermissionedPool({
-      operator,
       poolAdmin: poolAdmin,
       settings: poolSettings,
       liquidityAsset: mockUSDC

diff --git a/test/scenarios/business/permissioned/2.test.ts b/test/scenarios/business/permissioned/2.test.ts
@@ -40,8 +40,7 @@ describe("Permissioned Business Scenario 2", () => {
   }
 
   async function fixtures() {
-    const [operator, poolAdmin, lenderA, lenderB, borrower] =
-      await ethers.getSigners();
+    const [poolAdmin, lenderA, lenderB, borrower] = await ethers.getSigners();
     const endTime = (await time.latest()) + 5_184_000; // 60 days.
     const poolSettings = {
       endDate: endTime, // Jan 1, 2050
@@ -60,7 +59,6 @@ describe("Permissioned Business Scenario 2", () => {
       poolAccessControl,
       tosAcceptanceRegistry
     } = await deployPermissionedPool({
-      operator,
       poolAdmin: poolAdmin,
       settings: poolSettings,
       liquidityAsset: mockUSDC

diff --git a/test/scenarios/business/permissioned/3.test.ts b/test/scenarios/business/permissioned/3.test.ts
@@ -40,8 +40,7 @@ describe("Permissioned Business Scenario 3", () => {
   }
 
   async function fixtures() {
-    const [operator, poolAdmin, lenderA, lenderB, borrower] =
-      await ethers.getSigners();
+    const [poolAdmin, lenderA, lenderB, borrower] = await ethers.getSigners();
     const endTime = (await time.latest()) + 5_184_000; // 60 days.
     const poolSettings = {
       endDate: endTime, // Jan 1, 2050
@@ -59,7 +58,6 @@ describe("Permissioned Business Scenario 3", () => {
       poolAccessControl,
       tosAcceptanceRegistry
     } = await deployPermissionedPool({
-      operator,
       poolAdmin: poolAdmin,
       settings: poolSettings,
       liquidityAsset: mockUSDC

diff --git a/test/scenarios/business/permissioned/4.test.ts b/test/scenarios/business/permissioned/4.test.ts
@@ -40,8 +40,7 @@ describe("Permissioned Business Scenario 4", () => {
   }
 
   async function fixtures() {
-    const [operator, poolAdmin, lenderA, lenderB, borrower] =
-      await ethers.getSigners();
+    const [poolAdmin, lenderA, lenderB, borrower] = await ethers.getSigners();
     const endTime = (await time.latest()) + 5_184_000; // 60 days.
     const poolSettings = {
       endDate: endTime, // Jan 1, 2050
@@ -59,7 +58,6 @@ describe("Permissioned Business Scenario 4", () => {
       poolAccessControl,
       tosAcceptanceRegistry
     } = await deployPermissionedPool({
-      operator,
       poolAdmin: poolAdmin,
       settings: poolSettings,
       liquidityAsset: mockUSDC

diff --git a/test/scenarios/pool/crank-variations.test.ts b/test/scenarios/pool/crank-variations.test.ts
@@ -7,12 +7,10 @@ describe("Crank Variations", () => {
   const DEPOSIT_AMOUNT = 1_000_000;
 
   async function loadPoolFixture() {
-    const [operator, poolAdmin, aliceLender, bobLender] =
-      await ethers.getSigners();
+    const [poolAdmin, aliceLender, bobLender] = await ethers.getSigners();
 
     const { pool, liquidityAsset, withdrawController, poolController } =
       await deployPool({
-        operator,
         poolAdmin: poolAdmin
       });
 
@@ -264,10 +262,8 @@ describe("Crank Variations", () => {
     const {
       pool,
       aliceLender,
-      bobLender,
       liquidityAsset,
       poolAdmin,
-      withdrawRequestPeriodDuration,
       withdrawController,
       poolController
     } = await loadFixture(loadPoolFixture);

diff --git a/test/scenarios/pool/direct-deposit.test.ts b/test/scenarios/pool/direct-deposit.test.ts
@@ -1,6 +1,5 @@
 import { loadFixture } from "@nomicfoundation/hardhat-network-helpers";
 import { expect } from "chai";
-import { ethers } from "hardhat";
 import { deployLoan, fundLoan } from "../../support/loan";
 import { activatePool, deployPool } from "../../support/pool";
 import { getCommonSigners } from "../../support/utils";