From 78c8da8648696cc4d13c64800723a8e05a1d8599 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 11 Jun 2025 18:55:49 +0200 Subject: [PATCH] Update pragma check: validate by actually running the compiler (#5730) Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com> --- .github/workflows/checks.yml | 4 +- package.json | 2 +- scripts/checks/pragma-consistency.js | 49 ------------------------- scripts/checks/pragma-validity.js | 45 +++++++++++++++++++++++ scripts/get-contracts-metadata.js | 55 ++++++++++++++++++++++++++++ scripts/solc-versions.js | 15 ++++++++ 6 files changed, 118 insertions(+), 52 deletions(-) delete mode 100755 scripts/checks/pragma-consistency.js create mode 100755 scripts/checks/pragma-validity.js create mode 100644 scripts/get-contracts-metadata.js create mode 100644 scripts/solc-versions.js diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 6aca7f30c..6a346d7a9 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -41,8 +41,8 @@ jobs: run: npm run test - name: Check linearisation of the inheritance graph run: npm run test:inheritance - - name: Check pragma consistency between files - run: npm run test:pragma + - name: Check pragma validity + run: npm run test:pragma -- --concurrency 1 - name: Check procedurally generated contracts are up-to-date run: npm run test:generation - name: Compare gas costs diff --git a/package.json b/package.json index 851020463..cad57a97b 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "test": ". scripts/set-max-old-space-size.sh && hardhat test", "test:generation": "scripts/checks/generation.sh", "test:inheritance": "scripts/checks/inheritance-ordering.js artifacts/build-info/*", - "test:pragma": "scripts/checks/pragma-consistency.js artifacts/build-info/*", + "test:pragma": "scripts/checks/pragma-validity.js artifacts/build-info/*", "gas-report": "env ENABLE_GAS_REPORT=true npm run test", "slither": "npm run clean && slither ." }, diff --git a/scripts/checks/pragma-consistency.js b/scripts/checks/pragma-consistency.js deleted file mode 100755 index cf74cd27a..000000000 --- a/scripts/checks/pragma-consistency.js +++ /dev/null @@ -1,49 +0,0 @@ -#!/usr/bin/env node - -const path = require('path'); -const semver = require('semver'); -const match = require('micromatch'); -const { findAll } = require('solidity-ast/utils'); -const { _: artifacts } = require('yargs').argv; - -// files to skip -const skipPatterns = ['contracts-exposed/**', 'contracts/mocks/WithInit.sol']; - -for (const artifact of artifacts) { - const { output: solcOutput } = require(path.resolve(__dirname, '../..', artifact)); - - const pragma = {}; - - // Extract pragma directive for all files - for (const source in solcOutput.contracts) { - if (match.any(source, skipPatterns)) continue; - for (const { literals } of findAll('PragmaDirective', solcOutput.sources[source].ast)) { - // There should only be one. - const [first, ...rest] = literals; - if (first === 'solidity') pragma[source] = rest.join(''); - } - } - - // Compare the pragma directive of the file, to that of the files it imports - for (const source in solcOutput.contracts) { - if (match.any(source, skipPatterns)) continue; - // minimum version of the compiler that matches source's pragma - const minVersion = semver.minVersion(pragma[source]); - // loop over all imports in source - for (const { absolutePath } of findAll('ImportDirective', solcOutput.sources[source].ast)) { - // So files that only import without declaring anything cause issues, because they don't shop in "pragma" - if (!pragma[absolutePath]) continue; - // Check that the minVersion for source satisfies the requirements of the imported files - if (!semver.satisfies(minVersion, pragma[absolutePath])) { - console.log( - `- ${source} uses ${pragma[source]} but depends on ${absolutePath} that requires ${pragma[absolutePath]}`, - ); - process.exitCode = 1; - } - } - } -} - -if (!process.exitCode) { - console.log('Pragma directives are consistent.'); -} diff --git a/scripts/checks/pragma-validity.js b/scripts/checks/pragma-validity.js new file mode 100755 index 000000000..491f650bc --- /dev/null +++ b/scripts/checks/pragma-validity.js @@ -0,0 +1,45 @@ +#!/usr/bin/env node + +const semver = require('semver'); +const pLimit = require('p-limit').default; + +const { hideBin } = require('yargs/helpers'); +const yargs = require('yargs/yargs'); + +const getContractsMetadata = require('../get-contracts-metadata'); +const { compile } = require('../solc-versions'); + +const { + argv: { pattern, skipPatterns, verbose, concurrency, _: artifacts }, +} = yargs(hideBin(process.argv)) + .env('') + .options({ + pattern: { alias: 'p', type: 'string', default: 'contracts/**/*.sol' }, + skipPatterns: { alias: 's', type: 'string', default: 'contracts/mocks/**/*.sol' }, + concurrency: { alias: 'c', type: 'number', default: 8 }, + verbose: { alias: 'v', type: 'count' }, + }); + +const limit = pLimit(concurrency); +Promise.all( + Object.entries(getContractsMetadata(pattern, skipPatterns, artifacts)).map(([source, { pragma }]) => + limit( + (file, version) => + compile(file, version).then( + () => { + verbose && console.log(`Compile ${file} using solc ${version}: ok`); + }, + error => { + console.error(`Failed to compile ${file} using solc ${version}\n${error}`); + process.exitCode = 1; + }, + ), + source, + semver.minVersion(pragma), + ), + ), +).finally(() => { + if (!process.exitCode) { + console.log('All files can be compiled with the specified pragma.'); + } +}); diff --git a/scripts/get-contracts-metadata.js b/scripts/get-contracts-metadata.js new file mode 100644 index 000000000..030ab5a3d --- /dev/null +++ b/scripts/get-contracts-metadata.js @@ -0,0 +1,55 @@ +const fs = require('fs'); +const glob = require('glob'); +const match = require('micromatch'); +const path = require('path'); +const { findAll } = require('solidity-ast/utils'); + +module.exports = function ( + pattern = 'contracts/**/*.sol', + skipPatterns = ['contracts/mocks/**/*.sol'], + artifacts = [], +) { + // Use available hardhat artifacts. They reliably identify pragmas and the contracts, libraries and interfaces + // definitions with minimal IO operations. + const metadata = Object.fromEntries( + artifacts.flatMap(artifact => { + const { output: solcOutput } = require(path.resolve(__dirname, '..', artifact)); + return Object.keys(solcOutput.contracts) + .filter(source => match.all(source, pattern) && !match.any(source, skipPatterns)) + .map(source => [ + source, + { + pragma: Array.from(findAll('PragmaDirective', solcOutput.sources[source].ast)) + .find(({ literals }) => literals.at(0) == 'solidity') + .literals.slice(1) + .join(''), + sources: Array.from(findAll('ImportDirective', solcOutput.sources[source].ast)).map( + ({ absolutePath }) => absolutePath, + ), + interface: Array.from(findAll('ContractDefinition', solcOutput.sources[source].ast)).every( + ({ contractKind }) => contractKind === 'interface', + ), + }, + ]); + }), + ); + + // Artifacts are missing files that only include imports. We have a few of these in contracts/interfaces + // We add the missing metadata entries using the foundry artifacts + glob + .sync(pattern) + .filter(file => !match.any(file, skipPatterns) && !Object.hasOwn(metadata, file)) + .forEach(file => { + const entries = glob.sync(`out/${path.basename(file)}/*`); + metadata[file] = { + pragma: fs.readFileSync(file, 'utf-8').match(/pragma solidity (?[<>=^]*[0-9]+\.[0-9]+\.[0-9]+);/) + ?.groups.pragma, + sources: entries + .flatMap(entry => Object.keys(JSON.parse(fs.readFileSync(entry)).metadata.sources)) + .filter(source => source !== file && match.all(source, pattern) && !match.any(source, skipPatterns)), + interface: entries.every(entry => path.basename(entry).match(/^I[A-Z]/)), + }; + }); + + return metadata; +}; diff --git a/scripts/solc-versions.js b/scripts/solc-versions.js new file mode 100644 index 000000000..cd27a3621 --- /dev/null +++ b/scripts/solc-versions.js @@ -0,0 +1,15 @@ +const { exec } = require('child_process'); +const semver = require('semver'); +const { range } = require('./helpers'); + +module.exports = { + versions: ['0.4.26', '0.5.16', '0.6.12', '0.7.6', '0.8.30'] + .map(semver.parse) + .flatMap(({ major, minor, patch }) => range(patch + 1).map(p => `${major}.${minor}.${p}`)), + compile: (source, version) => + new Promise((resolve, reject) => + exec(`forge build ${source} --use ${version} --out out/solc-${version}`, error => + error ? reject(error) : resolve(), + ), + ), +};