[RFP12] CToken Cleanup (#152)

* Upgrade Contracts to 0.8.6 * Remove CToken SafeMath, use Custom Errors * EnterExitMarkets.scen fixes * Borrow.scen fixes * Update CI and Dockerfile Solidity version * Increase memory available for CI * gasCosts * Remove *LocalVar structs * clean up proposalID * pull in punitive accounting * comments * audit remediations * further remediations * compilation fixes after master merge * Partial Fix: L03 - Gas inefficiencies * Partial Fix: L05 - Missing or erroneous docstrings and comments * Partial Fix: L08 - Unreachable code * var type * remove unused accrue interest custom errors * Partial Fix: N13 - Typos * Fix L07: Outdated Solidity versions * use solhint over solium * debug CI Co-authored-by: Kevin Britz <[email protected]>
compound-finance · Jun 7, 2022 · a3214f6 · a3214f6
1 parent 3affca8
commit a3214f6
Show file tree

Hide file tree

Showing 157 changed files with 4,043 additions and 11,223 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -11,7 +11,7 @@ jobs:
     steps:
       - run:
         |
-          sudo wget https://github.com/ethereum/solidity/releases/download/v0.5.16/solc-static-linux -O /usr/local/bin/solc
+          sudo wget https://github.com/ethereum/solidity/releases/download/v0.8.10/solc-static-linux -O /usr/local/bin/solc
           sudo chmod +x /usr/local/bin/solc
       - checkout
       - restore_cache:
@@ -43,7 +43,7 @@ jobs:
       - run:
           shell: /bin/bash -eox pipefail -O globstar
           name: yarn test
-          command: JEST_JUNIT_OUTPUT_DIR=~/junit JEST_JUNIT_OUTPUT_NAME=test-results.xml yarn test $(circleci tests glob 'tests/**/**Test.js' | circleci tests split --split-by=timings)
+          command: NODE_OPTIONS="--max-old-space-size=12228" JEST_JUNIT_OUTPUT_DIR=~/junit JEST_JUNIT_OUTPUT_NAME=test-results.xml yarn test $(circleci tests glob 'tests/**/**Test.js' | circleci tests split --split-by=timings)
       - save_cache:
           paths:
             - .build
@@ -79,7 +79,7 @@ jobs:
     steps:
       - run:
         |
-          sudo wget https://github.com/ethereum/solidity/releases/download/v0.5.16/solc-static-linux -O /usr/local/bin/solc
+          sudo wget https://github.com/ethereum/solidity/releases/download/v0.8.10/solc-static-linux -O /usr/local/bin/solc
           sudo chmod +x /usr/local/bin/solc
       - checkout
       - restore_cache:
@@ -112,7 +112,7 @@ jobs:
           shell: /bin/bash -eox pipefail -O globstar
           name: yarn test
           no_output_timeout: 30m
-          command: JEST_JUNIT_OUTPUT_DIR=~/junit JEST_JUNIT_OUTPUT_NAME=test-results.xml script/coverage $(circleci tests glob 'tests/**/**Test.js' | circleci tests split --split-by=timings) -- --maxWorkers=4
+          command: NODE_OPTIONS="--max-old-space-size=12228" JEST_JUNIT_OUTPUT_DIR=~/junit JEST_JUNIT_OUTPUT_NAME=test-results.xml script/coverage $(circleci tests glob 'tests/**/**Test.js' | circleci tests split --split-by=timings) -- --maxWorkers=4
       - save_cache:
           paths:
             - .build

diff --git a/.gitignore b/.gitignore
@@ -36,8 +36,10 @@ scenario/build/webpack.js
 scenario/.tscache
 script/certora
 tests/scenarios/
+tests/Scenarios/
 junit.xml
 .build
 .last_confs
 .saddle_history
-node_modules_tmp
+node_modules_tmp
+.idea
diff --git a/.solhint.json b/.solhint.json
@@ -0,0 +1,15 @@
+{
+  "extends": "solhint:recommended",
+  "rules": {
+    "quotes": ["error", "double"],
+    "max-line-length": ["error", 200],
+    "compiler-version": ["error", "^0.8.0"],
+    "reason-string": ["warn", { "maxLength": 128 }],
+    "not-rely-on-block-hash": "off",
+    "not-rely-on-time": "off",
+    "no-inline-assembly": "off",
+    "avoid-low-level-calls": "off",
+    "avoid-tx-origin": "off",
+    "imports-on-top": "off"
+  }
+}
diff --git a/.soliumignore → .solhintignore b/.soliumignore → .solhintignore
diff --git a/.soliumrc.json b/.soliumrc.json
diff --git a/Dockerfile b/Dockerfile
@@ -1,7 +1,7 @@
 FROM mhart/alpine-node:13.8.0
 
 RUN apk update && apk add --no-cache --virtual build-dependencies git python g++ make
-RUN wget https://github.com/ethereum/solidity/releases/download/v0.5.16/solc-static-linux -O /bin/solc && chmod +x /bin/solc
+RUN wget https://github.com/ethereum/solidity/releases/download/v0.8.10/solc-static-linux -O /bin/solc && chmod +x /bin/solc
 
 RUN mkdir -p /compound-protocol
 WORKDIR /compound-protocol

diff --git a/contracts/BaseJumpRateModelV2.sol b/contracts/BaseJumpRateModelV2.sol
@@ -1,17 +1,18 @@
-pragma solidity ^0.5.16;
+// SPDX-License-Identifier: BSD-3-Clause
+pragma solidity ^0.8.10;
 
-import "./SafeMath.sol";
+import "./InterestRateModel.sol";
 
 /**
   * @title Logic for Compound's JumpRateModel Contract V2.
   * @author Compound (modified by Dharma Labs, refactored by Arr00)
   * @notice Version 2 modifies Version 1 by enabling updateable parameters.
   */
-contract BaseJumpRateModelV2 {
-    using SafeMath for uint;
-
+abstract contract BaseJumpRateModelV2 is InterestRateModel {
     event NewInterestParams(uint baseRatePerBlock, uint multiplierPerBlock, uint jumpMultiplierPerBlock, uint kink);
 
+    uint256 private constant BASE = 1e18;
+
     /**
      * @notice The address of the owner, i.e. the Timelock contract, which can update parameters directly
      */
@@ -44,8 +45,8 @@ contract BaseJumpRateModelV2 {
 
     /**
      * @notice Construct an interest rate model
-     * @param baseRatePerYear The approximate target base APR, as a mantissa (scaled by 1e18)
-     * @param multiplierPerYear The rate of increase in interest rate wrt utilization (scaled by 1e18)
+     * @param baseRatePerYear The approximate target base APR, as a mantissa (scaled by BASE)
+     * @param multiplierPerYear The rate of increase in interest rate wrt utilization (scaled by BASE)
      * @param jumpMultiplierPerYear The multiplierPerBlock after hitting a specified utilization point
      * @param kink_ The utilization point at which the jump multiplier is applied
      * @param owner_ The address of the owner, i.e. the Timelock contract (which has the ability to update parameters directly)
@@ -58,12 +59,12 @@ contract BaseJumpRateModelV2 {
 
     /**
      * @notice Update the parameters of the interest rate model (only callable by owner, i.e. Timelock)
-     * @param baseRatePerYear The approximate target base APR, as a mantissa (scaled by 1e18)
-     * @param multiplierPerYear The rate of increase in interest rate wrt utilization (scaled by 1e18)
+     * @param baseRatePerYear The approximate target base APR, as a mantissa (scaled by BASE)
+     * @param multiplierPerYear The rate of increase in interest rate wrt utilization (scaled by BASE)
      * @param jumpMultiplierPerYear The multiplierPerBlock after hitting a specified utilization point
      * @param kink_ The utilization point at which the jump multiplier is applied
      */
-    function updateJumpRateModel(uint baseRatePerYear, uint multiplierPerYear, uint jumpMultiplierPerYear, uint kink_) external {
+    function updateJumpRateModel(uint baseRatePerYear, uint multiplierPerYear, uint jumpMultiplierPerYear, uint kink_) virtual external {
         require(msg.sender == owner, "only the owner may call this function.");
 
         updateJumpRateModelInternal(baseRatePerYear, multiplierPerYear, jumpMultiplierPerYear, kink_);
@@ -74,33 +75,33 @@ contract BaseJumpRateModelV2 {
      * @param cash The amount of cash in the market
      * @param borrows The amount of borrows in the market
      * @param reserves The amount of reserves in the market (currently unused)
-     * @return The utilization rate as a mantissa between [0, 1e18]
+     * @return The utilization rate as a mantissa between [0, BASE]
      */
     function utilizationRate(uint cash, uint borrows, uint reserves) public pure returns (uint) {
         // Utilization rate is 0 when there are no borrows
         if (borrows == 0) {
             return 0;
         }
 
-        return borrows.mul(1e18).div(cash.add(borrows).sub(reserves));
+        return borrows * BASE / (cash + borrows - reserves);
     }
 
     /**
      * @notice Calculates the current borrow rate per block, with the error code expected by the market
      * @param cash The amount of cash in the market
      * @param borrows The amount of borrows in the market
      * @param reserves The amount of reserves in the market
-     * @return The borrow rate percentage per block as a mantissa (scaled by 1e18)
+     * @return The borrow rate percentage per block as a mantissa (scaled by BASE)
      */
     function getBorrowRateInternal(uint cash, uint borrows, uint reserves) internal view returns (uint) {
         uint util = utilizationRate(cash, borrows, reserves);
 
         if (util <= kink) {
-            return util.mul(multiplierPerBlock).div(1e18).add(baseRatePerBlock);
+            return ((util * multiplierPerBlock) / BASE) + baseRatePerBlock;
         } else {
-            uint normalRate = kink.mul(multiplierPerBlock).div(1e18).add(baseRatePerBlock);
-            uint excessUtil = util.sub(kink);
-            return excessUtil.mul(jumpMultiplierPerBlock).div(1e18).add(normalRate);
+            uint normalRate = ((kink * multiplierPerBlock) / BASE) + baseRatePerBlock;
+            uint excessUtil = util - kink;
+            return ((excessUtil * jumpMultiplierPerBlock) / BASE) + normalRate;
         }
     }
 
@@ -110,26 +111,26 @@ contract BaseJumpRateModelV2 {
      * @param borrows The amount of borrows in the market
      * @param reserves The amount of reserves in the market
      * @param reserveFactorMantissa The current reserve factor for the market
-     * @return The supply rate percentage per block as a mantissa (scaled by 1e18)
+     * @return The supply rate percentage per block as a mantissa (scaled by BASE)
      */
-    function getSupplyRate(uint cash, uint borrows, uint reserves, uint reserveFactorMantissa) public view returns (uint) {
-        uint oneMinusReserveFactor = uint(1e18).sub(reserveFactorMantissa);
+    function getSupplyRate(uint cash, uint borrows, uint reserves, uint reserveFactorMantissa) virtual override public view returns (uint) {
+        uint oneMinusReserveFactor = BASE - reserveFactorMantissa;
         uint borrowRate = getBorrowRateInternal(cash, borrows, reserves);
-        uint rateToPool = borrowRate.mul(oneMinusReserveFactor).div(1e18);
-        return utilizationRate(cash, borrows, reserves).mul(rateToPool).div(1e18);
+        uint rateToPool = borrowRate * oneMinusReserveFactor / BASE;
+        return utilizationRate(cash, borrows, reserves) * rateToPool / BASE;
     }
 
     /**
      * @notice Internal function to update the parameters of the interest rate model
-     * @param baseRatePerYear The approximate target base APR, as a mantissa (scaled by 1e18)
-     * @param multiplierPerYear The rate of increase in interest rate wrt utilization (scaled by 1e18)
+     * @param baseRatePerYear The approximate target base APR, as a mantissa (scaled by BASE)
+     * @param multiplierPerYear The rate of increase in interest rate wrt utilization (scaled by BASE)
      * @param jumpMultiplierPerYear The multiplierPerBlock after hitting a specified utilization point
      * @param kink_ The utilization point at which the jump multiplier is applied
      */
     function updateJumpRateModelInternal(uint baseRatePerYear, uint multiplierPerYear, uint jumpMultiplierPerYear, uint kink_) internal {
-        baseRatePerBlock = baseRatePerYear.div(blocksPerYear);
-        multiplierPerBlock = (multiplierPerYear.mul(1e18)).div(blocksPerYear.mul(kink_));
-        jumpMultiplierPerBlock = jumpMultiplierPerYear.div(blocksPerYear);
+        baseRatePerBlock = baseRatePerYear / blocksPerYear;
+        multiplierPerBlock = (multiplierPerYear * BASE) / (blocksPerYear * kink_);
+        jumpMultiplierPerBlock = jumpMultiplierPerYear / blocksPerYear;
         kink = kink_;
 
         emit NewInterestParams(baseRatePerBlock, multiplierPerBlock, jumpMultiplierPerBlock, kink);

diff --git a/contracts/CDaiDelegate.sol b/contracts/CDaiDelegate.sol
@@ -1,4 +1,5 @@
-pragma solidity ^0.5.16;
+// SPDX-License-Identifier: BSD-3-Clause
+pragma solidity ^0.8.10;
 
 import "./CErc20Delegate.sol";
 
@@ -27,7 +28,7 @@ contract CDaiDelegate is CErc20Delegate {
      * @notice Delegate interface to become the implementation
      * @param data The encoded arguments for becoming
      */
-    function _becomeImplementation(bytes memory data) public {
+    function _becomeImplementation(bytes memory data) override public {
         require(msg.sender == admin, "only the admin may initialize the implementation");
 
         (address daiJoinAddress_, address potAddress_) = abi.decode(data, (address, address));
@@ -53,7 +54,7 @@ contract CDaiDelegate is CErc20Delegate {
         vatAddress = address(vat);
 
         // Approve moving our DAI into the vat through daiJoin
-        dai.approve(daiJoinAddress, uint(-1));
+        dai.approve(daiJoinAddress, type(uint).max);
 
         // Approve the pot to transfer our funds within the vat
         vat.hope(potAddress);
@@ -69,7 +70,7 @@ contract CDaiDelegate is CErc20Delegate {
     /**
      * @notice Delegate interface to resign the implementation
      */
-    function _resignImplementation() public {
+    function _resignImplementation() override public {
         require(msg.sender == admin, "only the admin may abandon the implementation");
 
         // Transfer all cash out of the DSR - note that this relies on self-transfer
@@ -98,7 +99,7 @@ contract CDaiDelegate is CErc20Delegate {
       * @dev This calculates interest accrued from the last checkpointed block
       *      up to the current block and writes new checkpoint to storage.
       */
-    function accrueInterest() public returns (uint) {
+    function accrueInterest() override public returns (uint) {
         // Accumulate DSR interest
         PotLike(potAddress).drip();
 
@@ -113,7 +114,7 @@ contract CDaiDelegate is CErc20Delegate {
      * @dev This excludes the value of the current message, if any
      * @return The quantity of underlying tokens owned by this contract
      */
-    function getCashPrior() internal view returns (uint) {
+    function getCashPrior() override internal view returns (uint) {
         PotLike pot = PotLike(potAddress);
         uint pie = pot.pie(address(this));
         return mul(pot.chi(), pie) / RAY;
@@ -125,13 +126,15 @@ contract CDaiDelegate is CErc20Delegate {
      * @param amount Amount of underlying to transfer
      * @return The actual amount that is transferred
      */
-    function doTransferIn(address from, uint amount) internal returns (uint) {
+    function doTransferIn(address from, uint amount) override internal returns (uint) {
+        // Read from storage once
+        address underlying_ = underlying;
         // Perform the EIP-20 transfer in
-        EIP20Interface token = EIP20Interface(underlying);
+        EIP20Interface token = EIP20Interface(underlying_);
         require(token.transferFrom(from, address(this), amount), "unexpected EIP-20 transfer in return");
 
         DaiJoinLike daiJoin = DaiJoinLike(daiJoinAddress);
-        GemLike dai = GemLike(underlying);
+        GemLike dai = GemLike(underlying_);
         PotLike pot = PotLike(potAddress);
         VatLike vat = VatLike(vatAddress);
 
@@ -154,7 +157,7 @@ contract CDaiDelegate is CErc20Delegate {
      * @param to Address to transfer funds to
      * @param amount Amount of underlying to transfer
      */
-    function doTransferOut(address payable to, uint amount) internal {
+    function doTransferOut(address payable to, uint amount) override internal {
         DaiJoinLike daiJoin = DaiJoinLike(daiJoinAddress);
         PotLike pot = PotLike(potAddress);