From 1b36add0bbc8277e24f5bd337a0eeef13068813e Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 21 Oct 2024 16:39:05 +0200 Subject: [PATCH 1/8] Bump pragma to 0.8.22 for all contract that depend on ERC1967Utils --- contracts/mocks/DummyImplementation.sol | 2 +- contracts/mocks/Stateless.sol | 2 +- contracts/mocks/proxy/UUPSUpgradeableMock.sol | 2 +- contracts/proxy/ERC1967/ERC1967Proxy.sol | 2 +- contracts/proxy/ERC1967/ERC1967Utils.sol | 2 +- contracts/proxy/beacon/BeaconProxy.sol | 2 +- contracts/proxy/transparent/TransparentUpgradeableProxy.sol | 2 +- contracts/proxy/utils/UUPSUpgradeable.sol | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/mocks/DummyImplementation.sol b/contracts/mocks/DummyImplementation.sol index 4925c89df4b..0f1147407f5 100644 --- a/contracts/mocks/DummyImplementation.sol +++ b/contracts/mocks/DummyImplementation.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.22; import {ERC1967Utils} from "../proxy/ERC1967/ERC1967Utils.sol"; import {StorageSlot} from "../utils/StorageSlot.sol"; diff --git a/contracts/mocks/Stateless.sol b/contracts/mocks/Stateless.sol index 98e7eaf7443..9e43232d587 100644 --- a/contracts/mocks/Stateless.sol +++ b/contracts/mocks/Stateless.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.22; // We keep these imports and a dummy contract just to we can run the test suite after transpilation. diff --git a/contracts/mocks/proxy/UUPSUpgradeableMock.sol b/contracts/mocks/proxy/UUPSUpgradeableMock.sol index a5f2d4a25d8..8c5641e6ca3 100644 --- a/contracts/mocks/proxy/UUPSUpgradeableMock.sol +++ b/contracts/mocks/proxy/UUPSUpgradeableMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.20; +pragma solidity ^0.8.22; import {UUPSUpgradeable} from "../../proxy/utils/UUPSUpgradeable.sol"; import {ERC1967Utils} from "../../proxy/ERC1967/ERC1967Utils.sol"; diff --git a/contracts/proxy/ERC1967/ERC1967Proxy.sol b/contracts/proxy/ERC1967/ERC1967Proxy.sol index 4f51cd9578b..cad9eb5ab7e 100644 --- a/contracts/proxy/ERC1967/ERC1967Proxy.sol +++ b/contracts/proxy/ERC1967/ERC1967Proxy.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v5.1.0) (proxy/ERC1967/ERC1967Proxy.sol) -pragma solidity ^0.8.20; +pragma solidity ^0.8.22; import {Proxy} from "../Proxy.sol"; import {ERC1967Utils} from "./ERC1967Utils.sol"; diff --git a/contracts/proxy/ERC1967/ERC1967Utils.sol b/contracts/proxy/ERC1967/ERC1967Utils.sol index 1f320135277..287bb6beee2 100644 --- a/contracts/proxy/ERC1967/ERC1967Utils.sol +++ b/contracts/proxy/ERC1967/ERC1967Utils.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v5.1.0) (proxy/ERC1967/ERC1967Utils.sol) -pragma solidity ^0.8.21; +pragma solidity ^0.8.22; import {IBeacon} from "../beacon/IBeacon.sol"; import {IERC1967} from "../../interfaces/IERC1967.sol"; diff --git a/contracts/proxy/beacon/BeaconProxy.sol b/contracts/proxy/beacon/BeaconProxy.sol index 2606f21db08..e38b9d891cf 100644 --- a/contracts/proxy/beacon/BeaconProxy.sol +++ b/contracts/proxy/beacon/BeaconProxy.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v5.1.0) (proxy/beacon/BeaconProxy.sol) -pragma solidity ^0.8.20; +pragma solidity ^0.8.22; import {IBeacon} from "./IBeacon.sol"; import {Proxy} from "../Proxy.sol"; diff --git a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol index a35a725f2b3..7342d9f8f0a 100644 --- a/contracts/proxy/transparent/TransparentUpgradeableProxy.sol +++ b/contracts/proxy/transparent/TransparentUpgradeableProxy.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v5.1.0) (proxy/transparent/TransparentUpgradeableProxy.sol) -pragma solidity ^0.8.20; +pragma solidity ^0.8.22; import {ERC1967Utils} from "../ERC1967/ERC1967Utils.sol"; import {ERC1967Proxy} from "../ERC1967/ERC1967Proxy.sol"; diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol index dc799962cb3..745c56fa5d2 100644 --- a/contracts/proxy/utils/UUPSUpgradeable.sol +++ b/contracts/proxy/utils/UUPSUpgradeable.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v5.1.0) (proxy/utils/UUPSUpgradeable.sol) -pragma solidity ^0.8.20; +pragma solidity ^0.8.22; import {IERC1822Proxiable} from "../../interfaces/draft-IERC1822.sol"; import {ERC1967Utils} from "../ERC1967/ERC1967Utils.sol"; From dd32af26b6a836140a8507b5d023cb46a20983ae Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 10:02:36 +0200 Subject: [PATCH 2/8] Add automated test for pragma consistency --- .github/workflows/checks.yml | 4 +++ package.json | 3 +- scripts/checks/pragma-consistency.js | 46 ++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100755 scripts/checks/pragma-consistency.js diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 9d338bb642f..a4d08c1da51 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -41,6 +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 proceduraly generated contracts are up-to-date run: npm run test:generation - name: Compare gas costs @@ -68,6 +70,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 storage layout uses: ./.github/actions/storage-layout continue-on-error: ${{ contains(github.event.pull_request.labels.*.name, 'breaking change') }} diff --git a/package.json b/package.json index f9b88273362..618a7efb868 100644 --- a/package.json +++ b/package.json @@ -26,8 +26,9 @@ "generate": "scripts/generate/run.js", "version": "scripts/release/version.sh", "test": "hardhat test", - "test:inheritance": "scripts/checks/inheritance-ordering.js artifacts/build-info/*", "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/*", "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 new file mode 100755 index 00000000000..f2c0448f8aa --- /dev/null +++ b/scripts/checks/pragma-consistency.js @@ -0,0 +1,46 @@ +#!/usr/bin/env node + +const path = require('path'); +const semver = require('semver'); +const { findAll } = require('solidity-ast/utils'); +const { _: artifacts } = require('yargs').argv; + +// files to skip +const skipPatterns = ['contracts-exposed/']; +const skip = source => skipPatterns.some(pattern => source.startsWith(pattern)); + +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 (skip(source)) continue; + for (const {literals} of findAll('PragmaDirective', solcOutput.sources[source].ast)) { + // There should only be one. + pragma[source] = literals.slice(1).join('') + } + } + + // Compare the pragma directive of the file, to that of the files it imports + for (const source in solcOutput.contracts) { + if (skip(source)) 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 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.'); +} From 9bf0ab397c77105961bd756ed77606b1fb06b3af Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 10:02:47 +0200 Subject: [PATCH 3/8] more fixes --- contracts/mocks/MerkleTreeMock.sol | 2 +- contracts/mocks/governance/GovernorStorageMock.sol | 2 +- contracts/proxy/transparent/ProxyAdmin.sol | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/mocks/MerkleTreeMock.sol b/contracts/mocks/MerkleTreeMock.sol index 2454efa2f91..dcde6b65884 100644 --- a/contracts/mocks/MerkleTreeMock.sol +++ b/contracts/mocks/MerkleTreeMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity ^0.8.20; import {MerkleTree} from "../utils/structs/MerkleTree.sol"; diff --git a/contracts/mocks/governance/GovernorStorageMock.sol b/contracts/mocks/governance/GovernorStorageMock.sol index 88c6bf906bf..26e0e10b57a 100644 --- a/contracts/mocks/governance/GovernorStorageMock.sol +++ b/contracts/mocks/governance/GovernorStorageMock.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; +pragma solidity ^0.8.20; import {IGovernor, Governor} from "../../governance/Governor.sol"; import {GovernorTimelockControl} from "../../governance/extensions/GovernorTimelockControl.sol"; diff --git a/contracts/proxy/transparent/ProxyAdmin.sol b/contracts/proxy/transparent/ProxyAdmin.sol index 31772350392..2a60edfe987 100644 --- a/contracts/proxy/transparent/ProxyAdmin.sol +++ b/contracts/proxy/transparent/ProxyAdmin.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // OpenZeppelin Contracts (last updated v5.1.0) (proxy/transparent/ProxyAdmin.sol) -pragma solidity ^0.8.20; +pragma solidity ^0.8.22; import {ITransparentUpgradeableProxy} from "./TransparentUpgradeableProxy.sol"; import {Ownable} from "../../access/Ownable.sol"; From bda9c4ecdfe37f35254788e34b826882b6408e97 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 10:03:41 +0200 Subject: [PATCH 4/8] add changeset --- .changeset/seven-donkeys-tap.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/seven-donkeys-tap.md diff --git a/.changeset/seven-donkeys-tap.md b/.changeset/seven-donkeys-tap.md new file mode 100644 index 00000000000..25d2305b9b8 --- /dev/null +++ b/.changeset/seven-donkeys-tap.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +Update some pragma directives to ensure that all file requirements match that of the files they import. From ef9b8c13c2ef9de32e3bee4ce7e69524c11ede40 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 10:04:15 +0200 Subject: [PATCH 5/8] fix lint --- scripts/checks/pragma-consistency.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/checks/pragma-consistency.js b/scripts/checks/pragma-consistency.js index f2c0448f8aa..97d2cd83fe3 100755 --- a/scripts/checks/pragma-consistency.js +++ b/scripts/checks/pragma-consistency.js @@ -12,14 +12,14 @@ const skip = source => skipPatterns.some(pattern => source.startsWith(pattern)); for (const artifact of artifacts) { const { output: solcOutput } = require(path.resolve(__dirname, '../..', artifact)); - const pragma = {} + const pragma = {}; // Extract pragma directive for all files for (const source in solcOutput.contracts) { if (skip(source)) continue; - for (const {literals} of findAll('PragmaDirective', solcOutput.sources[source].ast)) { + for (const { literals } of findAll('PragmaDirective', solcOutput.sources[source].ast)) { // There should only be one. - pragma[source] = literals.slice(1).join('') + pragma[source] = literals.slice(1).join(''); } } @@ -29,12 +29,14 @@ for (const artifact of artifacts) { // 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)) { + 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 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]}`); + console.log( + `- ${source} uses ${pragma[source]} but depends on ${absolutePath} that requires ${pragma[absolutePath]}`, + ); process.exitCode = 1; } } From 00d914ac6427fad4f950985b253f1b6554cdca29 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 10:13:34 +0200 Subject: [PATCH 6/8] fix script --- scripts/checks/pragma-consistency.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/checks/pragma-consistency.js b/scripts/checks/pragma-consistency.js index 97d2cd83fe3..bbf8d95e189 100755 --- a/scripts/checks/pragma-consistency.js +++ b/scripts/checks/pragma-consistency.js @@ -19,7 +19,8 @@ for (const artifact of artifacts) { if (skip(source)) continue; for (const { literals } of findAll('PragmaDirective', solcOutput.sources[source].ast)) { // There should only be one. - pragma[source] = literals.slice(1).join(''); + const [first, ...rest] = literals; + if (first === 'solidity') pragma[source] = rest.join(''); } } From 6a41c62565cc7ae2793171d9801931f498c230e7 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 10:35:46 +0200 Subject: [PATCH 7/8] skip WithInit.sol (from the upgradeable contracts) --- scripts/checks/pragma-consistency.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checks/pragma-consistency.js b/scripts/checks/pragma-consistency.js index bbf8d95e189..06130f45dc9 100755 --- a/scripts/checks/pragma-consistency.js +++ b/scripts/checks/pragma-consistency.js @@ -6,7 +6,7 @@ const { findAll } = require('solidity-ast/utils'); const { _: artifacts } = require('yargs').argv; // files to skip -const skipPatterns = ['contracts-exposed/']; +const skipPatterns = ['contracts-exposed/', 'contracts/mocks/WithInit.sol']; const skip = source => skipPatterns.some(pattern => source.startsWith(pattern)); for (const artifact of artifacts) { From 281c3228be435f0946a1a392a09c6fd94b80c709 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 22 Oct 2024 15:16:48 +0200 Subject: [PATCH 8/8] use micromatch --- scripts/checks/inheritance-ordering.js | 9 +++++---- scripts/checks/pragma-consistency.js | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/scripts/checks/inheritance-ordering.js b/scripts/checks/inheritance-ordering.js index 72aa37ef7b3..4ed2deec454 100755 --- a/scripts/checks/inheritance-ordering.js +++ b/scripts/checks/inheritance-ordering.js @@ -2,9 +2,13 @@ const path = require('path'); const graphlib = require('graphlib'); +const match = require('micromatch'); const { findAll } = require('solidity-ast/utils'); const { _: artifacts } = require('yargs').argv; +// files to skip +const skipPatterns = ['contracts-exposed/**', 'contracts/mocks/**']; + for (const artifact of artifacts) { const { output: solcOutput } = require(path.resolve(__dirname, '../..', artifact)); @@ -13,10 +17,7 @@ for (const artifact of artifacts) { const linearized = []; for (const source in solcOutput.contracts) { - if (['contracts-exposed/', 'contracts/mocks/'].some(pattern => source.startsWith(pattern))) { - continue; - } - + if (match.any(source, skipPatterns)) continue; for (const contractDef of findAll('ContractDefinition', solcOutput.sources[source].ast)) { names[contractDef.id] = contractDef.name; linearized.push(contractDef.linearizedBaseContracts); diff --git a/scripts/checks/pragma-consistency.js b/scripts/checks/pragma-consistency.js index 06130f45dc9..f2f3c548f59 100755 --- a/scripts/checks/pragma-consistency.js +++ b/scripts/checks/pragma-consistency.js @@ -2,12 +2,12 @@ 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']; -const skip = source => skipPatterns.some(pattern => source.startsWith(pattern)); +const skipPatterns = ['contracts-exposed/**', 'contracts/mocks/WithInit.sol']; for (const artifact of artifacts) { const { output: solcOutput } = require(path.resolve(__dirname, '../..', artifact)); @@ -16,7 +16,7 @@ for (const artifact of artifacts) { // Extract pragma directive for all files for (const source in solcOutput.contracts) { - if (skip(source)) continue; + 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; @@ -26,7 +26,7 @@ for (const artifact of artifacts) { // Compare the pragma directive of the file, to that of the files it imports for (const source in solcOutput.contracts) { - if (skip(source)) continue; + 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