Add ABI diff logic (#598)

README.md

@@ -84,8 +84,9 @@ module.exports = {
 | measureStatementCoverage | *boolean* | `true` | Computes statement (in addition to line) coverage. [More...][34] |
 | measureFunctionCoverage | *boolean* | `true` | Computes function coverage. [More...][34] |
 | measureModifierCoverage | *boolean* | `true` | Computes each modifier invocation as a code branch. [More...][34] |
-| matrixOutputPath | *String* | `./testMatrix.json` | Relative path to write test matrix JSON object to. [More...][39] |
 | modifierWhitelist | *String[]* | `[]` | List of modifier names (ex: "onlyOwner") to exclude from branch measurement. (Useful for modifiers which prepare something instead of acting as a gate.)) |
+| matrixOutputPath | *String* | `./testMatrix.json` | Relative path to write test matrix JSON object to. [More...][39] |
+| abiOutputPath | *String* | `./humanReadableAbis.json` | Relative path to write diff-able ABI data to |
 | istanbulFolder | *String* | `./coverage` |  Folder location for Istanbul coverage reports. |
 | istanbulReporter | *Array* | `['html', 'lcov', 'text', 'json']` | [Istanbul coverage reporters][2]  |
 | mocha | *Object* | `{ }` | [Mocha options][3] to merge into existing mocha config. `grep` and `invert` are useful for skipping certain tests under coverage using tags in the test descriptions.|

lib/abi.js

@@ -0,0 +1,99 @@
+const ethersABI = require("@ethersproject/abi");
+const difflib = require('difflib');
+
+class AbiUtils {
+
+  diff(orig={}, cur={}){
+    let plus = 0;
+    let minus = 0;
+
+    const unifiedDiff = difflib.unifiedDiff(
+      orig.humanReadableAbiList,
+      cur.humanReadableAbiList,
+      {
+        fromfile: orig.contractName,
+        tofile: cur.contractName,
+        fromfiledate: `sha: ${orig.sha}`,
+        tofiledate: `sha: ${cur.sha}`,
+        lineterm: ''
+      }
+    );
+
+    // Count changes (unified diff always has a plus & minus in header);
+    if (unifiedDiff.length){
+      plus = -1;
+      minus = -1;
+    }
+
+    unifiedDiff.forEach(line => {
+      if (line[0] === `+`) plus++;
+      if (line[0] === `-`) minus++;
+    })
+
+    return {
+      plus,
+      minus,
+      unifiedDiff
+    }
+  }
+
+  toHumanReadableFunctions(contract){
+    const human = [];
+    const ethersOutput = new ethersABI.Interface(contract.abi).functions;
+    const signatures = Object.keys(ethersOutput);
+
+    for (const sig of signatures){
+      const method = ethersOutput[sig];
+      let returns = '';
+
+      method.outputs.forEach(output => {
+        (returns.length)
+          ? returns += `, ${output.type}`
+          : returns += output.type;
+      });
+
+      let readable = `${method.type} ${sig} ${method.stateMutability}`;
+
+      if (returns.length){
+        readable += ` returns (${returns})`
+      }
+
+      human.push(readable);
+    }
+
+    return human;
+  }
+
+  toHumanReadableEvents(contract){
+    const human = [];
+    const ethersOutput = new ethersABI.Interface(contract.abi).events;
+    const signatures = Object.keys(ethersOutput);
+
+    for (const sig of signatures){
+      const method = ethersOutput[sig];
+      const readable = `${ethersOutput[sig].type} ${sig}`;
+      human.push(readable);
+    }
+
+    return human;
+  }
+
+  generateHumanReadableAbiList(_artifacts, sha){
+    const list = [];
+    if (_artifacts.length){
+      for (const item of _artifacts){
+        const fns = this.toHumanReadableFunctions(item);
+        const evts = this.toHumanReadableEvents(item);
+        const all = fns.concat(evts);
+        list.push({
+          contractName: item.contractName,
+          sha: sha,
+          humanReadableAbiList: all
+        })
+      }
+    }
+    return list;
+  }
+}
+
+module.exports = AbiUtils;

lib/api.js

@@ -12,13 +12,15 @@ const Instrumenter = require('./instrumenter');
 const Coverage = require('./coverage');
 const DataCollector = require('./collector');
 const AppUI = require('./ui').AppUI;
+const AbiUtils = require('./abi');
 
 /**
  * Coverage Runner
  */
 class API {
   constructor(config={}) {
-    this.validator = new ConfigValidator()
+    this.validator = new ConfigValidator();
+    this.abiUtils = new AbiUtils();
     this.config = config || {};
     this.testMatrix = {};
 
@@ -31,6 +33,7 @@ class API {
     this.testsErrored = false;
 
     this.cwd = config.cwd || process.cwd();
+    this.abiOutputPath = config.abiOutputPath || "humanReadableAbis.json";
     this.matrixOutputPath = config.matrixOutputPath || "testMatrix.json";
     this.matrixReporterPath = config.matrixReporterPath || "solidity-coverage/plugins/resources/matrix.js"
 
@@ -360,6 +363,11 @@ class API {
     fs.writeFileSync(matrixPath, JSON.stringify(mapping, null, ' '));
   }
 
+  saveHumanReadableAbis(data){
+    const abiPath = path.join(this.cwd, this.abiOutputPath);
+    fs.writeFileSync(abiPath, JSON.stringify(data, null, ' '));
+  }
+
   // =====
   // Paths
   // =====

lib/validator.js

@@ -14,7 +14,9 @@ const configSchema = {
     client: {type: "object"},
     cwd:    {type: "string"},
     host:   {type: "string"},
-
+    abiOutputPath:      {type: "string"},
+    matrixOutputPath:   {type: "string"},
+    matrixReporterPath: {type: "string"},
     port:                 {type: "number"},
     providerOptions:      {type: "object"},
     silent:               {type: "boolean"},

package.json

@@ -25,10 +25,12 @@
   "license": "ISC",
   "dependencies": {
     "@solidity-parser/parser": "^0.14.0",
+    "@ethersproject/abi": "^5.0.9",
     "@truffle/provider": "^0.2.24",
     "chalk": "^2.4.2",
     "death": "^1.1.0",
     "detect-port": "^1.3.0",
+    "difflib": "^0.2.4",
     "fs-extra": "^8.1.0",
     "ghost-testrpc": "^0.0.2",
     "global-modules": "^2.0.0",

plugins/resources/nomiclabs.utils.js

@@ -181,6 +181,35 @@ function collectTestMatrixData(args, env, api){
   }
 }
 
+/**
+ * Returns all Hardhat artifacts.
+ * @param  {HRE} env
+ * @return {Artifact[]}
+ */
+async function getAllArtifacts(env){
+  const all = [];
+  const qualifiedNames = await env.artifacts.getArtifactPaths();
+  for (const name of qualifiedNames){
+    all.push(await env.artifacts.readArtifact(name));
+  }
+  return all;
+}
+
+/**
+ * Compiles project
+ * Collects all artifacts from Hardhat project,
+ * Converts them to a format that can be consumed by api.abiUtils.diff
+ * Saves them to `api.abiOutputPath`
+ * @param  {HRE}    env
+ * @param  {SolidityCoverageAPI} api
+ */
+async function generateHumanReadableAbiList(env, api){
+  await env.run(TASK_COMPILE);
+  const _artifacts = getAllArtifacts(env);
+  const list = api.abiUtils.generateHumanReadableAbiList(_artifacts)
+  api.saveHumanReadableAbis(list);
+}
+
 /**
  * Sets the default `from` account field in the network that will be used.
  * This needs to be done after accounts are fetched from the launched client.
@@ -233,6 +262,7 @@ module.exports = {
   setupHardhatNetwork,
   getTestFilePaths,
   setNetworkFrom,
-  collectTestMatrixData
+  collectTestMatrixData,
+  getAllArtifacts
 }
 

plugins/resources/truffle.utils.js

@@ -35,6 +35,39 @@ async function getTestFilePaths(config){
   return target.filter(f => f.match(testregex) != null);
 }
 
+/**
+ * Returns all Truffle artifacts.
+ * @param  {TruffleConfig} config
+ * @return {Artifact[]}
+ */
+function getAllArtifacts(config){
+  const all = [];
+  const artifactsGlob = path.join(config.artifactsDir, '/**/*.json');
+  const files = globby.sync([artifactsGlob])
+  for (const file of files){
+    const candidate = require(file);
+    if (candidate.contractName && candidate.abi){
+      all.push(candidate);
+    }
+  }
+  return all;
+}
+
+/**
+ * Compiles project
+ * Collects all artifacts from Truffle project,
+ * Converts them to a format that can be consumed by api.abiUtils.diff
+ * Saves them to `api.abiOutputPath`
+ * @param  {TruffleConfig} config
+ * @param  {TruffleAPI}    truffle
+ * @param  {SolidityCoverageAPI} api
+ */
+async function generateHumanReadableAbiList(config, truffle, api){
+  await truffle.compile(config);
+  const _artifacts = getAllArtifacts(config);
+  const list = api.abiUtils.generateHumanReadableAbiList(_artifacts)
+  api.saveHumanReadableAbis(list);
+}
 
 /**
  * Configures the network. Runs before the server is launched.

test/sources/solidity/contracts/diff/addition.sol

@@ -0,0 +1,38 @@
+pragma solidity ^0.7.0;
+
+contract Old {
+  uint public y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+
+  function c() external {
+    bool x = true;
+  }
+}
+
+contract New {
+  uint public y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+
+  function c() external {
+    bool x = true;
+  }
+
+  function d() external {
+    bool x = true;
+  }
+}
+

test/sources/solidity/contracts/diff/events.sol

@@ -0,0 +1,23 @@
+pragma solidity ^0.7.0;
+
+contract Old {
+  uint y;
+
+  event Evt(uint x, bytes8 y);
+
+  function a() public {
+    bool x = true;
+  }
+}
+
+contract New {
+  uint y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  event aEvt(bytes8);
+  event _Evt(bytes8 x, bytes8 y);
+}
+

test/sources/solidity/contracts/diff/no-change.sol

@@ -0,0 +1,34 @@
+pragma solidity ^0.7.0;
+
+contract Old {
+  uint y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+
+  function c() external {
+    bool x = true;
+  }
+}
+
+contract New {
+  uint y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+
+  function c() external {
+    bool x = true;
+  }
+}
+

test/sources/solidity/contracts/diff/param-change.sol

@@ -0,0 +1,33 @@
+pragma solidity ^0.7.0;
+
+contract Old {
+  uint y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+
+  function c() external {
+    bool x = true;
+  }
+}
+
+contract New {
+  uint y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b(bytes8 z) external {
+    bool x = true;
+  }
+
+  function c(uint q, uint r) external {
+    bool x = true;
+  }
+}

test/sources/solidity/contracts/diff/removal.sol

@@ -0,0 +1,29 @@
+pragma solidity ^0.7.0;
+
+contract Old {
+  uint y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+
+  function c() external {
+    bool x = true;
+  }
+}
+
+contract New {
+  uint y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+}

test/sources/solidity/contracts/diff/reorder.sol

@@ -0,0 +1,33 @@
+pragma solidity ^0.7.0;
+
+contract Old {
+  uint y;
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+
+  function c() external {
+    bool x = true;
+  }
+}
+
+contract New {
+  uint y;
+
+  function c() external {
+    bool x = true;
+  }
+
+  function a() public {
+    bool x = true;
+  }
+
+  function b() external {
+    bool x = true;
+  }
+}

test/sources/solidity/contracts/diff/return-sig.sol

@@ -0,0 +1,25 @@
+pragma solidity ^0.7.0;
+
+contract Old {
+  function a() public view returns (uint) {
+    return 1;
+  }
+}
+
+contract New {
+  function a() public view returns (bool) {
+    return true;
+  }
+
+  function e() public view returns (uint8[2] memory) {
+    return [5,7];
+  }
+
+  function f() public view returns (uint8[2] memory, uint) {
+    return ([5,7], 7);
+  }
+
+  function g() public view returns (uint8[3] memory) {
+    return [5,7,8];
+  }
+}

test/sources/solidity/contracts/diff/state-mod-change.sol

@@ -0,0 +1,13 @@
+pragma solidity ^0.7.0;
+
+contract Old {
+  function a() public {
+    bool x = true;
+  }
+}
+
+contract New {
+  function a() public view returns (bool) {
+    return true;
+  }
+}

test/units/diff.js

@@ -0,0 +1,128 @@
+const assert = require('assert');
+const util = require('./../util/util.js');
+const Api = require('./../../lib/api')
+
+describe('abi diffs', function(){
+  const api = new Api();
+
+  function setUp(source){
+    const abis = util.getDiffABIs(source);
+    const orig = api.abiUtils.generateHumanReadableAbiList([abis.original], abis.original.sha);
+    const cur = api.abiUtils.generateHumanReadableAbiList([abis.current], abis.current.sha);
+    return api.abiUtils.diff(orig[0], cur[0]);
+  }
+
+  function validate(result, expectPlus, expectMinus, expectDiff){
+    assert.equal(result.plus, expectPlus);
+    assert.equal(result.minus, expectMinus);
+    assert.deepEqual(result.unifiedDiff, expectDiff);
+  }
+
+  it('when methods are added', function() {
+    const expectPlus = 1;
+    const expectMinus = 0;
+    const expectDiff = [
+      "--- Test\tsha: d8b26d8",
+      "+++ Test\tsha: e77e29d",
+      "@@ -1,4 +1,5 @@",
+      " function a() nonpayable",
+      " function b() nonpayable",
+      " function c() nonpayable",
+      "+function d() nonpayable",
+      " function y() view returns (uint256)"
+    ];
+
+    validate(setUp('diff/addition'), expectPlus, expectMinus, expectDiff);
+  });
+
+  it('when there are events', function() {
+    const expectPlus = 2;
+    const expectMinus = 1;
+    const expectDiff = [
+      "--- Test\tsha: d8b26d8",
+      "+++ Test\tsha: e77e29d",
+      "@@ -1,2 +1,3 @@",
+      " function a() nonpayable",
+      "-event Evt(uint256,bytes8)",
+      "+event _Evt(bytes8,bytes8)",
+      "+event aEvt(bytes8)"
+    ];
+
+    validate(setUp('diff/events'), expectPlus, expectMinus, expectDiff);
+  });
+
+  it('when parameters change', function() {
+    const expectPlus = 2;
+    const expectMinus = 2;
+    const expectDiff = [
+      "--- Test\tsha: d8b26d8",
+      "+++ Test\tsha: e77e29d",
+      "@@ -1,3 +1,3 @@",
+      " function a() nonpayable",
+      "-function b() nonpayable",
+      "-function c() nonpayable",
+      "+function b(bytes8) nonpayable",
+      "+function c(uint256,uint256) nonpayable"
+    ];
+
+    validate(setUp('diff/param-change'), expectPlus, expectMinus, expectDiff);
+  });
+
+  it('when there is no change', function() {
+    const expectPlus = 0;
+    const expectMinus = 0;
+    const expectDiff = [];
+    validate(setUp('diff/no-change'), expectPlus, expectMinus, expectDiff);
+  });
+
+  it('when methods are removed', function() {
+    const expectPlus = 0;
+    const expectMinus = 1;
+    const expectDiff = [
+      '--- Test\tsha: d8b26d8',
+      '+++ Test\tsha: e77e29d',
+      '@@ -1,3 +1,2 @@',
+      ' function a() nonpayable',
+      ' function b() nonpayable',
+      '-function c() nonpayable'
+    ];
+
+    validate(setUp('diff/removal'), expectPlus, expectMinus, expectDiff);
+  });
+
+  it('when methods are reordered', function() {
+    const expectPlus = 0;
+    const expectMinus = 0;
+    const expectDiff = [];
+    validate(setUp('diff/reorder'), expectPlus, expectMinus, expectDiff);
+  });
+
+  it('when return signatures change', function() {
+    const expectPlus = 4;
+    const expectMinus = 1;
+    const expectDiff = [
+      '--- Test\tsha: d8b26d8',
+      '+++ Test\tsha: e77e29d',
+      '@@ -1 +1,4 @@',
+      '-function a() view returns (uint256)',
+      '+function a() view returns (bool)',
+      '+function e() view returns (uint8[2])',
+      '+function f() view returns (uint8[2], uint256)',
+      '+function g() view returns (uint8[3])'
+    ];
+    validate(setUp('diff/return-sig'), expectPlus, expectMinus, expectDiff);
+  });
+
+  it('when state modifiablility changes', function() {
+    const expectPlus = 1;
+    const expectMinus = 1;
+    const expectDiff = [
+      '--- Test\tsha: d8b26d8',
+      '+++ Test\tsha: e77e29d',
+      '@@ -1 +1 @@',
+      '-function a() nonpayable',
+      '+function a() view returns (bool)'
+    ];
+    validate(setUp('diff/state-mod-change'), expectPlus, expectMinus, expectDiff);
+  });
+});

test/units/validator.js

@@ -22,7 +22,9 @@ describe('config validation', () => {
     const options =  [
       "cwd",
       "host",
-      "istanbulFolder"
+      "istanbulFolder",
+      "abiOutputPath",
+      "matrixOutputPath",
     ]
 
     options.forEach(name => {

test/util/integration.js

@@ -50,7 +50,6 @@ function decacheConfigs(){
 
 function clean() {
   shell.config.silent = true;
-
   shell.rm('-Rf', temp);
   shell.rm('-Rf', 'coverage');
   shell.rm('coverage.json');

test/util/util.js

@@ -68,6 +68,26 @@ function codeToCompilerInput(code) {
   });
 }
 
+// ===========
+// Diff tests
+// ===========
+function getDiffABIs(sourceName, testFile="test.sol", original="Old", current="New"){
+  const contract = getCode(`${sourceName}.sol`)
+  const solcOutput = compile(contract)
+  return {
+    original: {
+      contractName: "Test",
+      sha: "d8b26d8",
+      abi: solcOutput.contracts[testFile][original].abi,
+    },
+    current: {
+      contractName: "Test",
+      sha: "e77e29d",
+      abi: solcOutput.contracts[testFile][current].abi,
+    }
+  }
+}
+
 // ============================
 // Instrumentation Correctness
 // ============================
@@ -118,11 +138,12 @@ function initializeProvider(ganache){
 }
 
 module.exports = {
-  getCode: getCode,
-  pathPrefix: pathPrefix,
-  filePath: filePath,
-  report: report,
-  instrumentAndCompile: instrumentAndCompile,
-  bootstrapCoverage: bootstrapCoverage,
-  initializeProvider: initializeProvider,
+  getCode,
+  pathPrefix,
+  filePath,
+  report,
+  instrumentAndCompile,
+  bootstrapCoverage,
+  initializeProvider,
+  getDiffABIs
 }