Jelajahi Sumber

Generate comparative gas repports on PR (#3532)

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
Hadrien Croubois 3 tahun lalu
induk
melakukan
cfc9f840a0

+ 49 - 0
.github/actions/gas-compare/action.yml

@@ -0,0 +1,49 @@
+name: Compare gas costs
+inputs:
+  token:
+    description: github token
+    required: true
+  report:
+    description: report to read from
+    required: false
+    default: gasReporterOutput.json
+  out_report:
+    description: report to read 
+    required: false
+    default: ${{ github.ref_name }}.gasreport.json
+  ref_report:
+    description: report to read from
+    required: false
+    default: ${{ github.base_ref }}.gasreport.json
+
+runs:
+  using: composite
+  steps:
+    - name: Download reference report
+      if: github.event_name == 'pull_request'
+      run: |
+        RUN_ID=`gh run list --repo ${{ github.repository }} --branch ${{ github.base_ref }} --workflow ${{ github.workflow }} --json 'conclusion,databaseId' --jq 'map(select(.conclusion=="success"))[0].databaseId'`
+        gh run download ${RUN_ID} --repo ${{ github.repository }} -n gasreport
+      env:
+        GITHUB_TOKEN: ${{ inputs.token }}
+      shell: bash
+      continue-on-error: true
+      id: reference
+    - name: Compare reports
+      if: steps.reference.outcome == 'success' && github.event_name == 'pull_request'
+      run: |
+        node scripts/checks/compareGasReports.js ${{ inputs.report }} ${{ inputs.ref_report }} >> $GITHUB_STEP_SUMMARY
+      env:
+        STYLE: markdown
+      shell: bash
+    - name: Rename report for upload
+      if: github.event_name != 'pull_request'
+      run: |
+        mv ${{ inputs.report }} ${{ inputs.out_report }}
+      shell: bash
+    - name: Save report
+      if: github.event_name != 'pull_request'
+      uses: actions/upload-artifact@v3
+      with:
+        name: gasreport
+        path: ${{ inputs.out_report }}

+ 13 - 6
.github/workflows/checks.yml

@@ -24,17 +24,24 @@ jobs:
 
   tests:
     runs-on: ubuntu-latest
+    env:
+      FORCE_COLOR: 1
+      GAS: true
     steps:
       - uses: actions/checkout@v3
       - name: Set up environment
         uses: ./.github/actions/setup
-      - run: npm run test
-        env:
-          FORCE_COLOR: 1
-          ENABLE_GAS_REPORT: true
-      - run: npm run test:inheritance
-      - run: npm run test:generation
+      - name: Run tests and generate gas report
+        run: npm run test
+      - name: Check linearisation of the inheritance graph
+        run: npm run test:inheritance
+      - name: Check proceduraly generated contracts are up-to-date
         if: github.repository != 'OpenZeppelin/openzeppelin-contracts-upgradeable'
+        run: npm run test:generation
+      - name: Compare gas costs
+        uses: ./.github/actions/gas-compare
+        with:
+          token: ${{ github.token }}
 
   coverage:
     if: github.repository != 'OpenZeppelin/openzeppelin-contracts-upgradeable'

+ 10 - 7
hardhat.config.js

@@ -11,10 +11,6 @@ const path = require('path');
 const argv = require('yargs/yargs')()
   .env('')
   .options({
-    ci: {
-      type: 'boolean',
-      default: false,
-    },
     coverage: {
       type: 'boolean',
       default: false,
@@ -24,6 +20,12 @@ const argv = require('yargs/yargs')()
       type: 'boolean',
       default: false,
     },
+    gasReport: {
+      alias: 'enableGasReportPath',
+      type: 'string',
+      implies: 'gas',
+      default: undefined,
+    },
     mode: {
       alias: 'compileMode',
       type: 'string',
@@ -49,7 +51,7 @@ const argv = require('yargs/yargs')()
 
 require('@nomiclabs/hardhat-truffle5');
 
-if (argv.enableGasReport) {
+if (argv.gas) {
   require('hardhat-gas-reporter');
 }
 
@@ -57,7 +59,7 @@ for (const f of fs.readdirSync(path.join(__dirname, 'hardhat'))) {
   require(path.join(__dirname, 'hardhat', f));
 }
 
-const withOptimizations = argv.enableGasReport || argv.compileMode === 'production';
+const withOptimizations = argv.gas || argv.compileMode === 'production';
 
 /**
  * @type import('hardhat/config').HardhatUserConfig
@@ -80,8 +82,9 @@ module.exports = {
     },
   },
   gasReporter: {
+    showMethodSig: true,
     currency: 'USD',
-    outputFile: argv.ci ? 'gas-report.txt' : undefined,
+    outputFile: argv.gasReport,
     coinmarketcap: argv.coinmarketcap,
   },
 };

+ 189 - 0
scripts/checks/compareGasReports.js

@@ -0,0 +1,189 @@
+#!/usr/bin/env node
+
+const fs = require('fs');
+const chalk = require('chalk');
+const { argv } = require('yargs')
+  .env()
+  .options({
+    style: {
+      type: 'string',
+      choices: [ 'shell', 'markdown' ],
+      default: 'shell',
+    },
+  });
+
+// Deduce base tx cost from the percentage denominator
+const BASE_TX_COST = 21000;
+
+// Utilities
+function sum (...args) {
+  return args.reduce((a, b) => a + b, 0);
+}
+
+function average (...args) {
+  return sum(...args) / args.length;
+}
+
+function variation (current, previous) {
+  return {
+    value: current,
+    delta: current - previous,
+    prcnt: 100 * (current - previous) / (previous - BASE_TX_COST),
+  };
+}
+
+// Report class
+class Report {
+  // Read report file
+  static load (filepath) {
+    return JSON.parse(fs.readFileSync(filepath, 'utf8'));
+  }
+
+  // Compare two reports
+  static compare (update, ref, opts = { hideEqual: true }) {
+    if (JSON.stringify(update.config.metadata) !== JSON.stringify(ref.config.metadata)) {
+      throw new Error('Reports produced with non matching metadata');
+    }
+    return Object.keys(update.info.methods)
+      .filter(key => ref.info.methods[key])
+      .filter(key => update.info.methods[key].numberOfCalls > 0)
+      .filter(key => update.info.methods[key].numberOfCalls === ref.info.methods[key].numberOfCalls)
+      .map(key => ({
+        contract: ref.info.methods[key].contract,
+        method: ref.info.methods[key].fnSig,
+        min: variation(...[update, ref].map(x => ~~Math.min(...x.info.methods[key].gasData))),
+        max: variation(...[update, ref].map(x => ~~Math.max(...x.info.methods[key].gasData))),
+        avg: variation(...[update, ref].map(x => ~~average(...x.info.methods[key].gasData))),
+      }))
+      .filter(row => !opts.hideEqual || (row.min.delta && row.max.delta && row.avg.delta))
+      .sort((a, b) => `${a.contract}:${a.method}` < `${b.contract}:${b.method}` ? -1 : 1);
+  }
+}
+
+// Display
+function center (text, length) {
+  return text.padStart((text.length + length) / 2).padEnd(length);
+}
+
+function plusSign (num) {
+  return num > 0 ? '+' : '';
+}
+
+function formatCellShell (cell) {
+  const format = chalk[cell.delta > 0 ? 'red' : cell.delta < 0 ? 'green' : 'reset'];
+  return [
+    format((isNaN(cell.value) ? '-' : cell.value.toString()).padStart(8)),
+    format((isNaN(cell.delta) ? '-' : plusSign(cell.delta) + cell.delta.toString()).padStart(8)),
+    format((isNaN(cell.prcnt) ? '-' : plusSign(cell.prcnt) + cell.prcnt.toFixed(2) + '%').padStart(8)),
+  ];
+}
+
+function formatCmpShell (rows) {
+  const contractLength = Math.max(8, ...rows.map(({ contract }) => contract.length));
+  const methodLength = Math.max(7, ...rows.map(({ method }) => method.length));
+
+  const COLS = [
+    { txt: '', length: 0 },
+    { txt: 'Contract', length: contractLength },
+    { txt: 'Method', length: methodLength },
+    { txt: 'Min', length: 30 },
+    { txt: 'Avg', length: 30 },
+    { txt: 'Max', length: 30 },
+    { txt: '', length: 0 },
+  ];
+  const HEADER = COLS.map(entry => chalk.bold(center(entry.txt, entry.length || 0))).join(' | ').trim();
+  const SEPARATOR = COLS.map(({ length }) => length > 0 ? '-'.repeat(length + 2) : '').join('|').trim();
+
+  return [
+    '',
+    HEADER,
+    ...rows.map(entry => [
+      '',
+      chalk.grey(entry.contract.padEnd(contractLength)),
+      entry.method.padEnd(methodLength),
+      ...formatCellShell(entry.min),
+      ...formatCellShell(entry.avg),
+      ...formatCellShell(entry.max),
+      '',
+    ].join(' | ').trim()),
+    '',
+  ].join(`\n${SEPARATOR}\n`).trim();
+}
+
+function alignPattern (align) {
+  switch (align) {
+  case 'left':
+  case undefined:
+    return ':-';
+  case 'right':
+    return '-:';
+  case 'center':
+    return ':-:';
+  }
+}
+
+function trend (value) {
+  return value > 0
+    ? ':x:'
+    : value < 0
+      ? ':heavy_check_mark:'
+      : ':heavy_minus_sign:';
+}
+
+function formatCellMarkdown (cell) {
+  return [
+    (isNaN(cell.value) ? '-' : cell.value.toString()),
+    (isNaN(cell.delta) ? '-' : plusSign(cell.delta) + cell.delta.toString()),
+    (isNaN(cell.prcnt) ? '-' : plusSign(cell.prcnt) + cell.prcnt.toFixed(2) + '%') + trend(cell.delta),
+  ];
+}
+
+function formatCmpMarkdown (rows) {
+  const COLS = [
+    { txt: '' },
+    { txt: 'Contract', align: 'left' },
+    { txt: 'Method', align: 'left' },
+    { txt: 'Min', align: 'right' },
+    { txt: '(+/-)', align: 'right' },
+    { txt: '%', align: 'right' },
+    { txt: 'Avg', align: 'right' },
+    { txt: '(+/-)', align: 'right' },
+    { txt: '%', align: 'right' },
+    { txt: 'Max', align: 'right' },
+    { txt: '(+/-)', align: 'right' },
+    { txt: '%', align: 'right' },
+    { txt: '' },
+  ];
+  const HEADER = COLS.map(entry => entry.txt).join(' | ').trim();
+  const SEPARATOR = COLS.map(entry => entry.txt ? alignPattern(entry.align) : '').join('|').trim();
+
+  return [
+    '# Changes to gas costs',
+    '',
+    HEADER,
+    SEPARATOR,
+    rows.map(entry => [
+      '',
+      entry.contract,
+      entry.method,
+      ...formatCellMarkdown(entry.min),
+      ...formatCellMarkdown(entry.avg),
+      ...formatCellMarkdown(entry.max),
+      '',
+    ].join(' | ').trim()).join('\n'),
+    '',
+  ].join('\n').trim();
+}
+
+// MAIN
+const report = Report.compare(Report.load(argv._[0]), Report.load(argv._[1]));
+
+switch (argv.style) {
+case 'markdown':
+  console.log(formatCmpMarkdown(report));
+  break;
+case 'shell':
+default:
+  console.log(formatCmpShell(report));
+  break;
+}