Merge pull request #4954 from smartcontractkit/chore/solidity-coverage

Chore: Solidity coverage
smartcontractkit · Sep 9, 2021 · 8998f58 · 8998f58
2 parents 7dcb1e7 + fa900e8
commit 8998f58
Show file tree

Hide file tree

Showing 31 changed files with 554 additions and 46 deletions.
diff --git a/.github/workflows/solidity.yml b/.github/workflows/solidity.yml
@@ -29,6 +29,34 @@ jobs:
       - name: Run solhint
         run: ./tools/ci/solhint
 
+  solidity-coverage:
+    name: Solidity Coverage
+    runs-on: ubuntu-latest
+    steps:
+      - name: Setup node
+        uses: actions/setup-node@v2
+        with:
+          node-version: '12'
+      - name: Checkout the repo
+        uses: actions/checkout@v2
+      - name: Yarn cache
+        uses: actions/cache@v2
+        env:
+          cache-name: yarn-cache
+        with:
+          path: |
+            ~/.npm
+            ~/.cache
+            **/node_modules
+          key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('yarn.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-build-${{ env.cache-name }}-
+            ${{ runner.os }}-build-
+            ${{ runner.os }}-
+      - run: yarn install --frozen-lockfile
+      - name: Run coverage
+        run: ./tools/ci/solidity_coverage
+
   solidity:
     name: Solidity
     runs-on: ubuntu-latest

diff --git a/contracts/.gitignore b/contracts/.gitignore
@@ -2,4 +2,6 @@ artifacts
 cache
 node_modules
 solc
-abi
+abi
+coverage
+coverage.json
diff --git a/contracts/.solcover.js b/contracts/.solcover.js
@@ -0,0 +1,6 @@
+module.exports = {
+  mocha: {
+    grep: "@skip-coverage", // Find everything with this tag
+    invert: true, // Run the grep's inverse set.
+  },
+};
diff --git a/contracts/CHANGELOG.md b/contracts/CHANGELOG.md
@@ -7,9 +7,9 @@
 - v0.8 Access Controlled contracts (`SimpleWriteAccessController` and `SimpleReadAccessController`).
 - v0.8 Flags contracts (`Flags`).
 - v0.8 Contracts for the V2 VRF. `VRFCoordinatorV2.sol`, `VRF.sol`,
-`VRFConsumerBaseV2.sol`, `VRFCoordinatorV2Interface.sol`. Along
-with related test contract `VRFConsumerV2.sol` and example contracts 
-`VRFSingleConsumerExample.sol` and `VRFConsumerExternalSubOwnerExampl.sol`.
+  `VRFConsumerBaseV2.sol`, `VRFCoordinatorV2Interface.sol`. Along
+  with related test contract `VRFConsumerV2.sol` and example contracts
+  `VRFSingleConsumerExample.sol` and `VRFConsumerExternalSubOwnerExampl.sol`.
 - v0.6 `MockV3Aggregator` in src/v0.6/tests/.
 
 ### Changed:

diff --git a/contracts/hardhat.config.ts b/contracts/hardhat.config.ts
@@ -1,6 +1,7 @@
 import "@nomiclabs/hardhat-waffle";
 import "hardhat-contract-sizer";
 import "hardhat-abi-exporter";
+import "solidity-coverage";
 
 const COMPILER_SETTINGS = {
   optimizer: {
@@ -57,4 +58,7 @@ export default {
     runOnCompile: false,
     disambiguatePaths: false,
   },
+  mocha: {
+    timeout: 100000,
+  },
 };
diff --git a/contracts/package.json b/contracts/package.json
@@ -12,6 +12,7 @@
     "prettier": "prettier --config .prettierrc --write \"**/*.{js,json,md,sol,ts}\"",
     "compile:native": "./scripts/native_solc_compile_all",
     "compile": "hardhat compile",
+    "coverage": "hardhat coverage",
     "lint:sol": "solhint -f table ./src/**/**/*.sol",
     "setup": "yarn compile",
     "prepublishOnly": "yarn compile && ./scripts/prepublish_generate_abi_folder",
@@ -36,6 +37,7 @@
     "hardhat-abi-exporter": "^2.2.1",
     "hardhat-contract-sizer": "^2.0.3",
     "solhint": "^3.3.6",
+    "solidity-coverage": "^0.7.17",
     "ts-node": "^10.0.0",
     "typescript": "^4.3.2"
   }

diff --git a/contracts/src/v0.8/tests/VRFCoordinatorV2TestHelper.sol b/contracts/src/v0.8/tests/VRFCoordinatorV2TestHelper.sol
@@ -15,15 +15,13 @@ contract VRFCoordinatorV2TestHelper is VRFCoordinatorV2 {
         VRFCoordinatorV2(link, blockhashStore, linkEthFeed) { /* empty */ }
 
     function calculatePaymentAmountTest(
-        uint256 startGas,
         uint256 gasAfterPaymentCalculation,
         uint32  fulfillmentFlatFeeLinkPPM,
         uint256 weiPerUnitGas
     )
         external
     {
-        s_gasStart = gasleft();
-        s_paymentAmount = calculatePaymentAmount(startGas, gasAfterPaymentCalculation, fulfillmentFlatFeeLinkPPM, weiPerUnitGas);
+        s_paymentAmount = calculatePaymentAmount(gasleft(), gasAfterPaymentCalculation, fulfillmentFlatFeeLinkPPM, weiPerUnitGas);
     }
 
     function getPaymentAmount()

diff --git a/contracts/test/v0.6/AggregatorFacade.test.ts b/contracts/test/v0.6/AggregatorFacade.test.ts
@@ -51,7 +51,7 @@ describe("AggregatorFacade", () => {
     await oc1.fulfillOracleRequest(...convertFufillParams(request, response));
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(facade, [
       "aggregator",
       "decimals",

diff --git a/contracts/test/v0.6/BasicConsumer.test.ts b/contracts/test/v0.6/BasicConsumer.test.ts
@@ -39,7 +39,7 @@ describe("BasicConsumer", () => {
     cc = await basicConsumerFactory.connect(roles.defaultAccount).deploy(link.address, oc.address, specId);
   });
 
-  it("has a predictable gas price", async () => {
+  it("has a predictable gas price [ @skip-coverage ]", async () => {
     const rec = await ethers.provider.getTransactionReceipt(cc.deployTransaction.hash ?? "");
     assert.isBelow(rec.gasUsed?.toNumber() ?? -1, 1750000);
   });
@@ -74,7 +74,7 @@ describe("BasicConsumer", () => {
         assert.deepEqual(expected, cbor.decodeFirstSync(request.data));
       });
 
-      it("has a reasonable gas cost", async () => {
+      it("has a reasonable gas cost [ @skip-coverage ]", async () => {
         const tx = await cc.requestEthereumPrice(currency, payment);
         const receipt = await tx.wait();
 

diff --git a/contracts/test/v0.6/DeviationFlaggingValidator.test.ts b/contracts/test/v0.6/DeviationFlaggingValidator.test.ts
@@ -37,7 +37,7 @@ describe("DeviationFlaggingValidator", () => {
     await ac.connect(personas.Carol).addAccess(validator.address);
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(validator, [
       "THRESHOLD_MULTIPLIER",
       "flaggingThreshold",

diff --git a/contracts/test/v0.6/Flags.test.ts b/contracts/test/v0.6/Flags.test.ts
@@ -35,7 +35,7 @@ describe("Flags", () => {
     consumer = await consumerFactory.deploy(flags.address);
   });
 
-  it("has a limited public interface", async () => {
+  it("has a limited public interface [ @skip-coverage ]", async () => {
     publicAbi(flags, [
       "getFlag",
       "getFlags",

diff --git a/contracts/test/v0.6/FluxAggregator.test.ts b/contracts/test/v0.6/FluxAggregator.test.ts
@@ -164,7 +164,7 @@ describe("FluxAggregator", () => {
     nextRound = 1;
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(aggregator, [
       "acceptAdmin",
       "allocatedFunds",
@@ -972,7 +972,7 @@ describe("FluxAggregator", () => {
           await aggregator.connect(personas.Carol).changeOracles([], addresses, addresses, 1, oracles.length, rrDelay);
         });
 
-        it("not use too much gas", async () => {
+        it("not use too much gas [ @skip-coverage ]", async () => {
           let tx: any;
           assert.deepEqual(
             // test adveserial quickselect algo

diff --git a/contracts/test/v0.6/Owned.test.ts b/contracts/test/v0.6/Owned.test.ts
@@ -26,7 +26,7 @@ describe("Owned", () => {
     owned = await ownedFactory.connect(owner).deploy();
   });
 
-  it("has a limited public interface", async () => {
+  it("has a limited public interface [ @skip-coverage ]", async () => {
     publicAbi(owned, ["acceptOwnership", "owner", "transferOwnership"]);
   });
 

diff --git a/contracts/test/v0.6/SimpleReadAccessController.test.ts b/contracts/test/v0.6/SimpleReadAccessController.test.ts
@@ -22,7 +22,7 @@ describe("SimpleReadAccessController", () => {
     controller = await controllerFactory.connect(personas.Carol).deploy();
   });
 
-  it("has a limited public interface", async () => {
+  it("has a limited public interface [ @skip-coverage ]", async () => {
     publicAbi(controller, [
       "hasAccess",
       "addAccess",

diff --git a/contracts/test/v0.6/SimpleWriteAccessController.test.ts b/contracts/test/v0.6/SimpleWriteAccessController.test.ts
@@ -22,7 +22,7 @@ describe("SimpleWriteAccessController", () => {
     controller = await controllerFactory.connect(personas.Carol).deploy();
   });
 
-  it("has a limited public interface", async () => {
+  it("has a limited public interface [ @skip-coverage ]", async () => {
     publicAbi(controller, [
       "hasAccess",
       "addAccess",

diff --git a/contracts/test/v0.6/VRFD20.test.ts b/contracts/test/v0.6/VRFD20.test.ts
@@ -46,7 +46,7 @@ describe("VRFD20", () => {
     await link.transfer(vrfD20.address, deposit);
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(vrfD20, [
       // Owned
       "acceptOwnership",

diff --git a/contracts/test/v0.7/AggregatorProxy.test.ts b/contracts/test/v0.7/AggregatorProxy.test.ts
@@ -60,7 +60,7 @@ describe("AggregatorProxy", () => {
       .deploy(link.address, 0, 0, emptyAddress, 0, 0, 18, "TEST / LINK");
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(proxy, [
       "aggregator",
       "confirmAggregator",

diff --git a/contracts/test/v0.7/AuthorizedForwarder.test.ts b/contracts/test/v0.7/AuthorizedForwarder.test.ts
@@ -35,7 +35,7 @@ describe("AuthorizedForwarder", () => {
       .deploy(link.address, await roles.defaultAccount.getAddress(), zeroAddress, "0x");
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(forwarder, [
       "forward",
       "getAuthorizedSenders",

diff --git a/contracts/test/v0.7/CompoundPriceFlaggingValidator.test.ts b/contracts/test/v0.7/CompoundPriceFlaggingValidator.test.ts
@@ -60,7 +60,7 @@ describe("CompoundPriceFlaggingVlidator", () => {
     await ac.connect(personas.Carol).addAccess(validator.address);
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(validator, [
       "update",
       "check",

diff --git a/contracts/test/v0.7/ConfirmedOwner.test.ts b/contracts/test/v0.7/ConfirmedOwner.test.ts
@@ -31,7 +31,7 @@ describe("ConfirmedOwner", () => {
     confirmedOwner = await confirmedOwnerTestHelperFactory.connect(owner).deploy();
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(confirmedOwner, [
       "acceptOwnership",
       "owner",

diff --git a/contracts/test/v0.7/Operator.test.ts b/contracts/test/v0.7/Operator.test.ts
@@ -1,5 +1,12 @@
 import { ethers } from "hardhat";
-import { publicAbi, toBytes32String, toWei, stringToBytes, increaseTime5Minutes } from "../test-helpers/helpers";
+import {
+  publicAbi,
+  toBytes32String,
+  toWei,
+  stringToBytes,
+  increaseTime5Minutes,
+  getLog,
+} from "../test-helpers/helpers";
 import { assert, expect } from "chai";
 import { BigNumber, constants, Contract, ContractFactory, ContractReceipt, ContractTransaction, Signer } from "ethers";
 import { getUsers, Roles } from "../test-helpers/setup";
@@ -71,7 +78,7 @@ describe("Operator", () => {
     await operator.connect(roles.defaultAccount).setAuthorizedSenders([await roles.oracleNode.getAddress()]);
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(operator, [
       "acceptAuthorizedReceivers",
       "acceptOwnableContracts",
@@ -774,7 +781,7 @@ describe("Operator", () => {
     let gasGuzzlingConsumer: Contract;
     let request: ReturnType<typeof decodeRunRequest>;
 
-    describe("gas guzzling consumer", () => {
+    describe("gas guzzling consumer [ @skip-coverage ]", () => {
       beforeEach(async () => {
         gasGuzzlingConsumer = await gasGuzzlingConsumerFactory
           .connect(roles.consumer)
@@ -1102,7 +1109,7 @@ describe("Operator", () => {
       let gasGuzzlingConsumer: Contract;
       let request: ReturnType<typeof decodeRunRequest>;
 
-      describe("gas guzzling consumer", () => {
+      describe("gas guzzling consumer [ @skip-coverage ]", () => {
         beforeEach(async () => {
           gasGuzzlingConsumer = await gasGuzzlingConsumerFactory
             .connect(roles.consumer)
@@ -1442,7 +1449,7 @@ describe("Operator", () => {
         let gasGuzzlingConsumer: Contract;
         let request: ReturnType<typeof decodeRunRequest>;
 
-        describe("gas guzzling consumer", () => {
+        describe("gas guzzling consumer [ @skip-coverage ]", () => {
           beforeEach(async () => {
             gasGuzzlingConsumer = await gasGuzzlingConsumerFactory
               .connect(roles.consumer)
@@ -1783,7 +1790,7 @@ describe("Operator", () => {
         let gasGuzzlingConsumer: Contract;
         let request: ReturnType<typeof decodeRunRequest>;
 
-        describe("gas guzzling consumer", () => {
+        describe("gas guzzling consumer [ @skip-coverage ]", () => {
           beforeEach(async () => {
             gasGuzzlingConsumer = await gasGuzzlingConsumerFactory
               .connect(roles.consumer)
@@ -2372,7 +2379,9 @@ describe("Operator", () => {
 
         it("emits an event", async () => {
           assert.equal(3, receipt.logs?.length);
-          expect(tx).to.emit(link, "Transfer").withArgs(operator.address, to, payment, args);
+          const transferLog = await getLog(tx, 1);
+          const parsedLog = link.interface.parseLog({ data: transferLog.data, topics: transferLog.topics });
+          await expect(parsedLog.name).to.equal("Transfer");
         });
 
         it("transfers the tokens", async () => {

diff --git a/contracts/test/v0.7/OperatorFactory.test.ts b/contracts/test/v0.7/OperatorFactory.test.ts
@@ -35,7 +35,7 @@ describe("OperatorFactory", () => {
     operatorGenerator = await operatorGeneratorFactory.connect(roles.defaultAccount).deploy(link.address);
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(operatorGenerator, [
       "created",
       "deployNewOperator",

diff --git a/contracts/test/v0.7/StalenessFlaggingValidator.test.ts b/contracts/test/v0.7/StalenessFlaggingValidator.test.ts
@@ -42,7 +42,7 @@ describe("StalenessFlaggingValidator", () => {
     await ac.connect(personas.Carol).addAccess(validator.address);
   });
 
-  it("has a limited public interface", () => {
+  it("has a limited public interface [ @skip-coverage ]", () => {
     publicAbi(validator, [
       "update",
       "check",

diff --git a/contracts/test/v0.7/gasUsage.test.ts b/contracts/test/v0.7/gasUsage.test.ts
@@ -25,7 +25,7 @@ before(async () => {
   linkTokenFactory = await ethers.getContractFactory("LinkToken", roles.defaultAccount);
 });
 
-describe("Operator Gas Tests", () => {
+describe("Operator Gas Tests [ @skip-coverage ]", () => {
   const specId = "0x4c7b7ffb66b344fbaa64995af81e355a00000000000000000000000000000000";
   let link: Contract;
   let oracle1: Contract;

diff --git a/contracts/test/v0.8/Flags.test.ts b/contracts/test/v0.8/Flags.test.ts
@@ -35,7 +35,7 @@ describe("Flags", () => {
     consumer = await consumerFactory.deploy(flags.address);
   });
 
-  it("has a limited public interface", async () => {
+  it("has a limited public interface [ @skip-coverage ]", async () => {
     publicAbi(flags, [
       "getFlag",
       "getFlags",

diff --git a/contracts/test/v0.8/SimpleReadAccessController.test.ts b/contracts/test/v0.8/SimpleReadAccessController.test.ts
@@ -22,7 +22,7 @@ describe("SimpleReadAccessController", () => {
     controller = await controllerFactory.connect(personas.Carol).deploy();
   });
 
-  it("has a limited public interface", async () => {
+  it("has a limited public interface [ @skip-coverage ]", async () => {
     publicAbi(controller, [
       "hasAccess",
       "addAccess",

diff --git a/contracts/test/v0.8/SimpleWriteAccessController.test.ts b/contracts/test/v0.8/SimpleWriteAccessController.test.ts
@@ -22,7 +22,7 @@ describe("SimpleWriteAccessController", () => {
     controller = await controllerFactory.connect(personas.Carol).deploy();
   });
 
-  it("has a limited public interface", async () => {
+  it("has a limited public interface [ @skip-coverage ]", async () => {
     publicAbi(controller, [
       "hasAccess",
       "addAccess",

diff --git a/contracts/test/v0.8/ValidatorProxy.test.ts b/contracts/test/v0.8/ValidatorProxy.test.ts
@@ -31,7 +31,7 @@ describe("ValidatorProxy", () => {
     validatorProxy = await validatorProxy.deployed();
   });
 
-  it("has a limited public interface", async () => {
+  it("has a limited public interface [ @skip-coverage ]", async () => {
     publicAbi(validatorProxy, [
       // ConfirmedOwner functions
       "acceptOwnership",
@@ -287,7 +287,7 @@ describe("ValidatorProxy", () => {
               .withArgs(1, 200, 300, 400, 500);
           });
 
-          it("uses a specific amount of gas", async () => {
+          it("uses a specific amount of gas [ @skip-coverage ]", async () => {
             const resp = await validatorProxy.connect(aggregator).validate(200, 300, 400, 500);
             const receipt = await resp.wait();
             assert.equal(receipt.gasUsed.toString(), "32406");
@@ -316,7 +316,7 @@ describe("ValidatorProxy", () => {
               .withArgs(2, 2000, 3000, 4000, 5000);
           });
 
-          it("uses a specific amount of gas", async () => {
+          it("uses a specific amount of gas [ @skip-coverage ]", async () => {
             const resp = await validatorProxy.connect(aggregator).validate(2000, 3000, 4000, 5000);
             const receipt = await resp.wait();
             assert.equal(receipt.gasUsed.toString(), "40495");