Use our new non-invasive hardhat-cover plugin for coverage (compound-…

…finance#284)

.gitattributes

@@ -0,0 +1 @@
+yarn.lock binary

.github/workflows/run-coverage.yaml

@@ -7,6 +7,7 @@ jobs:
     name: Run coverage
     runs-on: ubuntu-latest
     env:
+      OPTIMIZER_DISABLED: true
       ETHERSCAN_KEY: ${{ secrets.ETHERSCAN_KEY }}
       SNOWTRACE_KEY: ${{ secrets.SNOWTRACE_KEY }}
       INFURA_KEY: ${{ secrets.INFURA_KEY }}
@@ -23,7 +24,7 @@ jobs:
         run: yarn install --non-interactive --frozen-lockfile && yarn build
 
       - name: Run tests
-        run: yarn test:coverage
+        run: yarn cover
 
       - name: Upload the coverage reports to Coveralls
         uses: coverallsapp/github-action@master

contracts/CometProxyAdmin.sol

@@ -9,14 +9,12 @@ interface Deployable {
 
 contract CometProxyAdmin is ProxyAdmin {
     /**
-     * @dev Deploy a new Comet and upgrade the implementation of the Comet proxy.
-     *
-     * Requirements:
-     *
-     * - This contract must be the admin of `CometProxy`.
+     * @dev Deploy a new Comet and upgrade the implementation of the Comet proxy
+     *  Requirements:
+     *   - This contract must be the admin of `CometProxy`
      */
     function deployAndUpgradeTo(Deployable configuratorProxy, TransparentUpgradeableProxy cometProxy) public virtual onlyOwner {
         address newCometImpl = configuratorProxy.deploy();
         upgrade(cometProxy, newCometImpl);
     }
-} 
+}

contracts/ConfiguratorStorage.sol

@@ -21,7 +21,7 @@ contract ConfiguratorStorage is CometConfiguration {
     Configuration internal configuratorParams; // XXX can create a public getter for this
 
     /// @notice The admin of the protocol
-    address public admin; 
+    address public admin;
 
     /// @notice Address for the Comet factory contract
     address public factory;

hardhat.config.ts

@@ -6,7 +6,7 @@ import '@nomiclabs/hardhat-waffle';
 import '@nomiclabs/hardhat-ethers';
 import '@nomiclabs/hardhat-etherscan';
 import '@typechain/hardhat';
-import 'solidity-coverage';
+import 'hardhat-cover';
 import 'hardhat-contract-sizer';
 import 'hardhat-gas-reporter';
 
@@ -100,10 +100,24 @@ const config: HardhatUserConfig = {
   solidity: {
     version: '0.8.11',
     settings: {
-      optimizer: {
-        enabled: true,
-        runs: 1,
+      optimizer: (
+        process.env['OPTIMIZER_DISABLED'] ? { enabled: false } : {
+          enabled: true,
+          runs: 1,
+          details: {
+            yulDetails: {
+              optimizerSteps: 'dhfoDgvulfnTUtnIf [xa[r]scLM cCTUtTOntnfDIul Lcul Vcul [j] Tpeul xa[rul] xa[r]cL gvif CTUca[r]LsTOtfDnca[r]Iulc] jmul[jul] VcTOcul jmul'
+            },
+          },
+        }
+      ),
+      outputSelection: {
+        "*": {
+          "*": ["evm.deployedBytecode.sourceMap"]
+        },
       },
+      // XXX for when we enable viaIR in a future merge:
+      //viaIR: process.env['OPTIMIZER_DISABLED'] ? false : true,
     },
   },
 

package.json

@@ -6,6 +6,7 @@
   "scripts": {
     "build": "hardhat compile",
     "clean": "hardhat clean && rm -rf build/ cache/ coverage* dist/",
+    "cover": "hardhat cover && npx istanbul report --include coverage.json html lcov",
     "deploy": "npx hardhat run ./scripts/deploy-comet.ts",
     "gas": "REPORT_GAS=true yarn test",
     "lint-contracts": "solhint 'contracts/**/*.sol'",
@@ -14,7 +15,7 @@
     "scenario": "hardhat scenario",
     "spider": "hardhat spider",
     "test": "hardhat test",
-    "test:coverage": "hardhat coverage",
+    "test:coverage": "hardhat cover --no-compile",
     "audit:vendor": "vendoza contracts/vendor/manifest.json"
   },
   "keywords": [],
@@ -38,16 +39,17 @@
     "ethereum-waffle": "^3.4.0",
     "ethers": "^5.5.1",
     "fast-glob": "^3.2.7",
-    "hardhat": "^2.8.3",
+    "hardhat": "2.8.3",
     "hardhat-contract-sizer": "^2.4.0",
+    "hardhat-cover": "compound-finance/hardhat-cover",
     "hardhat-gas-reporter": "^1.0.7",
     "jest-diff": "^27.4.2",
     "mocha-junit-reporter": "^2.0.2",
     "mocha-multi-reporters": "hayesgm/mocha-multi-reporters#hayesgm/reporter-options-to-option",
     "nock": "^13.2.2",
     "prettier": "2.5.1",
+    "sc-istanbul": "^0.4.5",
     "solhint": "^3.3.6",
-    "solidity-coverage": "^0.7.20",
     "ts-node": "^10.4.0",
     "typechain": "^6.0.2",
     "typescript": "^4.4.4",

plugins/deployment_manager/test/ContractMapTest.ts

@@ -8,7 +8,7 @@ import { tempDir } from './TestHelpers';
 import { Cache } from '../Cache';
 import { getContracts, getContractsFromAliases, storeBuildFile } from '../ContractMap';
 import { deploy } from '../Deploy';
-import { faucetTokenBuildFile, tokenArgs } from './DeployTest';
+import { faucetTokenBuildFile, tokenArgs } from './DeployHelpers';
 import { objectFromMap } from '../Utils';
 import { BuildFile } from '../Types';
 

plugins/deployment_manager/test/DeploymentManagerTest.ts

@@ -26,7 +26,7 @@ import { getRoots } from '../Roots';
 import { Address } from '../Types';
 import { objectFromMap } from '../Utils';
 import { deploy } from '../Deploy';
-import { faucetTokenBuildFile, tokenArgs } from './DeployTest';
+import { faucetTokenBuildFile, tokenArgs } from './DeployHelpers';
 import { tempDir } from './TestHelpers';
 
 export interface TestContracts {

plugins/deployment_manager/test/VerifyTest.ts

@@ -8,7 +8,7 @@ import * as path from 'path';
 import { Cache } from '../Cache';
 import { verifyContract } from '../Verify';
 import { deployBuild } from '../Deploy';
-import { buildToken, faucetTokenBuildFile, tokenArgs } from './DeployTest';
+import { buildToken, faucetTokenBuildFile, tokenArgs } from './DeployHelpers';
 
 export function mockVerifySuccess(hre: HardhatRuntimeEnvironment) {
   let solcList = JSON.parse(fs.readFileSync(path.join(__dirname, './SolcList.json'), 'utf8'));

scenario/constraints/TokenBalanceConstraint.ts

@@ -45,7 +45,7 @@ export class TokenBalanceConstraint<T extends CometContext, R extends Requiremen
                 toTransfer = max(exp(amount.val, decimals) - balance, 0);
                 break;
               case ComparisonOp.LTE:
-                // `toTransfer` should not be positive        
+                // `toTransfer` should not be positive
                 toTransfer = min(exp(amount.val, decimals) - balance, 0);
                 break;
               case ComparisonOp.GT:
@@ -83,7 +83,7 @@ export class TokenBalanceConstraint<T extends CometContext, R extends Requiremen
             case ComparisonOp.GTE:
               expect(balance).to.be.at.least(exp(amount.val, decimals));
               break;
-            case ComparisonOp.LTE:        
+            case ComparisonOp.LTE:
               expect(balance).to.be.at.most(exp(amount.val, decimals));
               break;
             case ComparisonOp.GT:

test/accrue-test.ts

@@ -1,4 +1,4 @@
-import { Comet, ethers, expect, exp, getBlock, inCoverage, makeProtocol, wait } from './helpers';
+import { Comet, ethers, expect, exp, getBlock, makeProtocol, wait } from './helpers';
 
 function projectBaseIndex(index, rate, time, factorScale = exp(1, 18)) {
   return index.add(index.mul(rate.mul(time)).div(factorScale));
@@ -35,8 +35,7 @@ describe('accrue', function () {
     expect(await t0.totalSupplyBase).to.be.equal(0);
     expect(await t0.totalBorrowBase).to.be.equal(0);
 
-    const tol = inCoverage() ? 100 : 50;
-    expect(await t0.lastAccrualTime).to.be.approximately(Date.now() / 1000, tol);
+    expect(await t0.lastAccrualTime).to.be.approximately(Date.now() / 1000, 80);
 
     const a0 = await wait(comet.accrue());
     expect(await comet.baseMinForRewards()).to.be.equal(params.baseMinForRewards);

test/base-tracking-accrued-test.ts

@@ -1,4 +1,4 @@
-import { ethers, expect, exp, makeProtocol } from './helpers';
+import { ethers, expect, exp, fastForward, makeProtocol } from './helpers';
 
 describe('baseTrackingAccrued', function() {
   it('supply updates baseTrackingAccrued to 6 decimal value', async () => {
@@ -114,7 +114,7 @@ describe('baseTrackingAccrued', function() {
     expect(userBasic1.baseTrackingAccrued).to.eq(0);
 
     // allow 10 seconds to pass
-    await ethers.provider.send('evm_increaseTime', [10]);
+    await fastForward(10);
 
     // supply again
     await comet.connect(alice).supply(USDC.address, 1e6);

test/helpers.ts

@@ -29,7 +29,7 @@ import {
 import { BigNumber } from 'ethers';
 import { TransactionReceipt, TransactionResponse } from '@ethersproject/abstract-provider';
 
-export { Comet, ethers, expect };
+export { Comet, ethers, expect, hre };
 
 export type Numeric = number | bigint;
 
@@ -174,6 +174,12 @@ export async function getBlock(n?: number): Promise<Block> {
   return ethers.provider.getBlock(blockNumber);
 }
 
+export async function fastForward(seconds: number): Promise<Block> {
+  const block = await getBlock();
+  await ethers.provider.send('evm_setNextBlockTimestamp', [block.timestamp + seconds]);
+  return block;
+}
+
 export async function makeProtocol(opts: ProtocolOpts = {}): Promise<Protocol> {
   const signers = await ethers.getSigners();
 
@@ -326,8 +332,6 @@ export async function makeConfigurator(opts: ProtocolOpts = {}): Promise<Configu
 
   const { tokens, comet, extensionDelegate } = await makeProtocol(opts);
 
-  if (opts.start) await ethers.provider.send('evm_setNextBlockTimestamp', [opts.start]);
-
   // Deploy CometFactory
   const CometFactoryFactory = (await ethers.getContractFactory('CometFactory')) as CometFactory__factory;
   const cometFactory = await CometFactoryFactory.deploy();
@@ -377,11 +381,12 @@ export async function makeConfigurator(opts: ProtocolOpts = {}): Promise<Configu
   await proxyAdmin.deployed();
 
   // Deploy Configurator proxy
+  const initializeCalldata = (await configurator.populateTransaction.initialize(governor.address, cometFactory.address, configuration)).data;
   const ConfiguratorProxy = (await ethers.getContractFactory('TransparentUpgradeableConfiguratorProxy')) as TransparentUpgradeableConfiguratorProxy__factory;
   const configuratorProxy = await ConfiguratorProxy.deploy(
     configurator.address,
     proxyAdmin.address,
-    (await configurator.populateTransaction.initialize(governor.address, cometFactory.address, configuration)).data,
+    initializeCalldata,
   );
   await configuratorProxy.deployed();
 
@@ -453,7 +458,3 @@ export function event(tx, index) {
   }
   return { [ev.event]: args };
 }
-
-export function inCoverage() {
-  return hre['__SOLIDITY_COVERAGE_RUNNING'];
-}

test/sanity-test.ts

@@ -15,3 +15,4 @@ describe('updateBaseBalance', function () {
     // XXX
   });
 });
+

test/supply-test.ts

@@ -1,4 +1,4 @@
-import { Comet, ethers, event, expect, exp, makeProtocol, portfolio, wait } from './helpers';
+import { Comet, ethers, event, expect, exp, hre, makeProtocol, portfolio, wait } from './helpers';
 
 describe('supplyTo', function () {
   it('supplies base from sender if the asset is base', async () => {
@@ -51,8 +51,7 @@ describe('supplyTo', function () {
     expect(q1.external).to.be.deep.equal({USDC: 0n, COMP: 0n, WETH: 0n, WBTC: 0n});
     expect(t1.totalSupplyBase).to.be.equal(t0.totalSupplyBase.add(100e6));
     expect(t1.totalBorrowBase).to.be.equal(t0.totalBorrowBase);
-    // XXX disable during coverage? more ideally coverage would not modify gas costs
-    //expect(Number(s0.receipt.gasUsed)).to.be.lessThan(100000);
+    expect(Number(s0.receipt.gasUsed)).to.be.lessThan(120000);
   });
 
   it('supplies collateral from sender if the asset is collateral', async () => {
@@ -98,8 +97,7 @@ describe('supplyTo', function () {
     expect(q1.internal).to.be.deep.equal({USDC: 0n, COMP: 0n, WETH: 0n, WBTC: 0n});
     expect(q1.external).to.be.deep.equal({USDC: 0n, COMP: 0n, WETH: 0n, WBTC: 0n});
     expect(t1.totalSupplyAsset).to.be.equal(t0.totalSupplyAsset.add(8e8));
-    // XXX disable during coverage? more ideally coverage would not modify gas costs
-    //expect(Number(s0.receipt.gasUsed)).to.be.lessThan(125000);
+    expect(Number(s0.receipt.gasUsed)).to.be.lessThan(140000);
   });
 
   it('calculates base principal correctly', async () => {

test/tracking-index-bounds-test.ts

@@ -1,4 +1,4 @@
-import { Comet, ethers, expect, exp, factor, defaultAssets, makeProtocol, portfolio, wait, toYears } from './helpers';
+import { Comet, ethers, expect, exp, factor, defaultAssets, fastForward, makeProtocol, portfolio, wait, toYears } from './helpers';
 import { BigNumber } from 'ethers';
 
 describe('total tracking index bounds', function () {
@@ -26,7 +26,7 @@ describe('total tracking index bounds', function () {
     });
     await wait(comet.setTotalsBasic(t0));
 
-    await ethers.provider.send('evm_increaseTime', [secondsUntilOverflow-1]);
+    await fastForward(secondsUntilOverflow-1);
 
     // First accrue is successful without overflow
     await comet.accrue();
@@ -59,7 +59,7 @@ describe('total tracking index bounds', function () {
     });
     await wait(comet.setTotalsBasic(t0));
 
-    await ethers.provider.send('evm_increaseTime', [secondsUntilOverflow-1]);
+    await fastForward(secondsUntilOverflow-1);
 
     // First accrue is successful without overflow
     await comet.accrue();
@@ -92,7 +92,7 @@ describe('total tracking index bounds', function () {
       totalSupplyBase: BigNumber.from(exp(1, 15)).mul(await comet.baseScale()).mul(3), // 3e15 base units
     });
     await wait(comet.setTotalsBasic(t2));
-    
+
     await comet.accrue();
     const t3 = await comet.totalsBasic();
 
@@ -124,7 +124,7 @@ describe('total tracking index bounds', function () {
       totalBorrowBase: BigNumber.from(exp(1, 15)).mul(await comet.baseScale()).mul(3), // 3e15 base units
     });
     await wait(comet.setTotalsBasic(t2));
-    
+
     await comet.accrue();
     const t3 = await comet.totalsBasic();
 

test/transfer-test.ts

@@ -36,8 +36,7 @@ describe('transfer', function () {
     expect(q1.internal).to.be.deep.equal({ USDC: 0n, COMP: 0n, WETH: 0n, WBTC: 0n });
     expect(t1.totalSupplyBase).to.be.equal(t0.totalSupplyBase);
     expect(t1.totalBorrowBase).to.be.equal(t0.totalBorrowBase);
-    // XXX disable during coverage? more ideally coverage would not modify gas costs
-    //expect(Number(s0.receipt.gasUsed)).to.be.lessThan(80000);
+    expect(Number(s0.receipt.gasUsed)).to.be.lessThan(90000);
   });
 
   it('transfers collateral from sender if the asset is collateral', async () => {
@@ -74,8 +73,7 @@ describe('transfer', function () {
     expect(p1.internal).to.be.deep.equal({ USDC: 0n, COMP: exp(8, 8), WETH: 0n, WBTC: 0n });
     expect(q1.internal).to.be.deep.equal({ USDC: 0n, COMP: 0n, WETH: 0n, WBTC: 0n });
     expect(t1.totalSupplyAsset).to.be.equal(t0.totalSupplyAsset);
-    // XXX disable during coverage? more ideally coverage would not modify gas costs
-    //expect(Number(s0.receipt.gasUsed)).to.be.lessThan(50000);
+    expect(Number(s0.receipt.gasUsed)).to.be.lessThan(95000);
   });
 
   it('calculates base principal correctly', async () => {

test/withdraw-test.ts

@@ -56,8 +56,7 @@ describe('withdrawTo', function () {
     expect(q1.external).to.be.deep.equal({USDC: 0n, COMP: 0n, WETH: 0n, WBTC: 0n});
     expect(t1.totalSupplyBase).to.be.equal(0n);
     expect(t1.totalBorrowBase).to.be.equal(0n);
-    // XXX disable during coverage? more ideally coverage would not modify gas costs
-    //expect(Number(s0.receipt.gasUsed)).to.be.lessThan(80000);
+    expect(Number(s0.receipt.gasUsed)).to.be.lessThan(100000);
   });
 
   it('withdraws collateral from sender if the asset is collateral', async () => {
@@ -106,8 +105,7 @@ describe('withdrawTo', function () {
     expect(q1.internal).to.be.deep.equal({USDC: 0n, COMP: 0n, WETH: 0n, WBTC: 0n});
     expect(q1.external).to.be.deep.equal({USDC: 0n, COMP: 0n, WETH: 0n, WBTC: 0n});
     expect(t1.totalSupplyAsset).to.be.equal(0n);
-    // XXX disable during coverage? more ideally coverage would not modify gas costs
-    //expect(Number(s0.receipt.gasUsed)).to.be.lessThan(60000);
+    expect(Number(s0.receipt.gasUsed)).to.be.lessThan(80000);
   });
 
   it('calculates base principal correctly', async () => {