From 0f6bf7b462d9653afa381288c2d4c3b3b990ee8b Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Mon, 22 Apr 2024 10:45:57 -0400 Subject: [PATCH] Defender: Improve license type handling (#1013) --- .eslintrc.js | 1 + .../api/pages/api-foundry-upgrades.adoc | 2 + packages/plugin-hardhat/CHANGELOG.md | 8 + .../plugin-hardhat/contracts/NoLicense.sol | 3 + .../plugin-hardhat/contracts/Unlicensed.sol | 4 + .../contracts/UnrecognizedLicense.sol | 4 + packages/plugin-hardhat/package.json | 2 +- .../plugin-hardhat/src/defender/deploy.ts | 93 ++++++-- packages/plugin-hardhat/src/utils/options.ts | 3 + .../plugin-hardhat/test/defender-deploy.js | 217 +++++++++++++++++- submodules/openzeppelin-foundry-upgrades | 2 +- 11 files changed, 311 insertions(+), 28 deletions(-) create mode 100644 packages/plugin-hardhat/contracts/NoLicense.sol create mode 100644 packages/plugin-hardhat/contracts/Unlicensed.sol create mode 100644 packages/plugin-hardhat/contracts/UnrecognizedLicense.sol diff --git a/.eslintrc.js b/.eslintrc.js index bf5705a95..038626645 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -15,6 +15,7 @@ module.exports = { 'unicorn/no-array-reduce': 'warn', 'no-plusplus': ['warn', { allowForLoopAfterthoughts: true }], }, + ignorePatterns: ['submodules'], overrides: [ { files: ['*.ts'], diff --git a/docs/modules/ROOT/pages/foundry/api/pages/api-foundry-upgrades.adoc b/docs/modules/ROOT/pages/foundry/api/pages/api-foundry-upgrades.adoc index 7e51b1f11..54e24eab3 100644 --- a/docs/modules/ROOT/pages/foundry/api/pages/api-foundry-upgrades.adoc +++ b/docs/modules/ROOT/pages/foundry/api/pages/api-foundry-upgrades.adoc @@ -72,6 +72,8 @@ struct DefenderOptions { string relayerId; bytes32 salt; string upgradeApprovalProcessId; + string licenseType; + bool skipLicenseType; } ``` diff --git a/packages/plugin-hardhat/CHANGELOG.md b/packages/plugin-hardhat/CHANGELOG.md index 9705d6ec6..c4333fc27 100644 --- a/packages/plugin-hardhat/CHANGELOG.md +++ b/packages/plugin-hardhat/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## 3.1.0 (2024-04-22) + +- Defender: Fix handling of license types for block explorer verification, support `licenseType` and `skipLicenseType` options. ([#1013](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1013)) + +### Breaking changes +- When deploying through Defender, if your contract does not have an SPDX license identifier, the verified source code on Etherscan will no longer show any license type. + - If you want the license type to appear as "None", either set your contract to have `// SPDX-License-Identifier: UNLICENSED` according to [Solidity docs](https://docs.soliditylang.org/en/latest/layout-of-source-files.html#spdx-license-identifier), or set the `licenseType` option to `"None"`. + ## 3.0.5 (2024-03-08) - Simplify console logging for `admin.transferProxyAdminOwnership`. ([#978](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/978)) diff --git a/packages/plugin-hardhat/contracts/NoLicense.sol b/packages/plugin-hardhat/contracts/NoLicense.sol new file mode 100644 index 000000000..ebef766cc --- /dev/null +++ b/packages/plugin-hardhat/contracts/NoLicense.sol @@ -0,0 +1,3 @@ +pragma solidity ^0.8.20; + +contract NoLicense {} diff --git a/packages/plugin-hardhat/contracts/Unlicensed.sol b/packages/plugin-hardhat/contracts/Unlicensed.sol new file mode 100644 index 000000000..f73bb7396 --- /dev/null +++ b/packages/plugin-hardhat/contracts/Unlicensed.sol @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +contract Unlicensed {} diff --git a/packages/plugin-hardhat/contracts/UnrecognizedLicense.sol b/packages/plugin-hardhat/contracts/UnrecognizedLicense.sol new file mode 100644 index 000000000..34768fe65 --- /dev/null +++ b/packages/plugin-hardhat/contracts/UnrecognizedLicense.sol @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: UnrecognizedId +pragma solidity ^0.8.20; + +contract UnrecognizedLicense {} diff --git a/packages/plugin-hardhat/package.json b/packages/plugin-hardhat/package.json index 0213f4ddd..a68d4fa8e 100644 --- a/packages/plugin-hardhat/package.json +++ b/packages/plugin-hardhat/package.json @@ -1,6 +1,6 @@ { "name": "@openzeppelin/hardhat-upgrades", - "version": "3.0.5", + "version": "3.1.0", "description": "", "repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/plugin-hardhat", "license": "MIT", diff --git a/packages/plugin-hardhat/src/defender/deploy.ts b/packages/plugin-hardhat/src/defender/deploy.ts index dce505d4f..b18c60ef2 100644 --- a/packages/plugin-hardhat/src/defender/deploy.ts +++ b/packages/plugin-hardhat/src/defender/deploy.ts @@ -70,22 +70,39 @@ export async function defenderDeploy( const verifySourceCode = opts.verifySourceCode ?? true; debug(`Verify source code: ${verifySourceCode}`); - let license: string | undefined = undefined; - if (verifySourceCode) { - license = getLicenseFromMetadata(contractInfo); - debug(`License type: ${license}`); - } - if (opts.salt !== undefined) { debug(`Salt: ${opts.salt}`); } + if (opts.licenseType !== undefined) { + if (opts.verifySourceCode === false) { + throw new UpgradesError('The `licenseType` option cannot be used when the `verifySourceCode` option is `false`'); + } else if (opts.skipLicenseType) { + throw new UpgradesError('The `licenseType` option cannot be used when the `skipLicenseType` option is `true`'); + } + } + + let licenseType: SourceCodeLicense | undefined = undefined; + if (verifySourceCode) { + if (opts.licenseType !== undefined) { + licenseType = opts.licenseType; + debug(`licenseType option: ${licenseType}`); + } else if (!opts.skipLicenseType) { + const spdxIdentifier = getSpdxLicenseIdentifier(contractInfo); + debug(`SPDX license identifier from metadata: ${spdxIdentifier}`); + if (spdxIdentifier !== undefined) { + licenseType = toLicenseType(spdxIdentifier, contractInfo); + debug(`licenseType inferred: ${licenseType}`); + } + } + } + const deploymentRequest: DeployContractRequest = { contractName: contractInfo.contractName, contractPath: contractInfo.sourceName, network: network, artifactPayload: JSON.stringify(contractInfo.buildInfo), - licenseType: license as SourceCodeLicense | undefined, // cast without validation but catch error from API below + licenseType: licenseType, constructorInputs: constructorArgs, verifySourceCode: verifySourceCode, relayerId: opts.relayerId, @@ -101,8 +118,9 @@ export async function defenderDeploy( } catch (e: any) { if (e.response?.data?.message?.includes('licenseType should be equal to one of the allowed values')) { throw new UpgradesError( - `License type ${license} is not a valid SPDX license identifier for block explorer verification.`, - () => 'Specify a valid SPDX-License-Identifier in your contract.', + `The licenseType option "${licenseType}" is not valid for block explorer verification.`, + () => + 'See https://etherscan.io/contract-license-types for supported values and use the string found in brackets, e.g. "MIT"', ); } else { throw e; @@ -197,9 +215,9 @@ async function getContractInfo( } /** - * Get the license type from the contract metadata without validating its validity, except converts undefined or UNLICENSED to None. + * Get the SPDX license identifier from the contract metadata without validating it. */ -function getLicenseFromMetadata(contractInfo: ContractInfo): string { +function getSpdxLicenseIdentifier(contractInfo: ContractInfo): string | undefined { const compilerOutput: CompilerOutputWithMetadata = contractInfo.buildInfo.output.contracts[contractInfo.sourceName][contractInfo.contractName]; @@ -213,11 +231,52 @@ function getLicenseFromMetadata(contractInfo: ContractInfo): string { const metadata = JSON.parse(metadataString); - const license: string = metadata.sources[contractInfo.sourceName].license; - if (license === undefined || license === 'UNLICENSED') { - // UNLICENSED means no license according to solidity docs - return 'None'; - } else { - return license; + return metadata.sources[contractInfo.sourceName].license; +} + +/** + * Infers a SourceCodeLicense from an SPDX license identifier. + */ +function toLicenseType(spdxIdentifier: string, contractInfo: ContractInfo): SourceCodeLicense { + switch (spdxIdentifier) { + case 'UNLICENSED': + return 'None'; + case 'Unlicense': + return 'Unlicense'; + case 'MIT': + return 'MIT'; + case 'GPL-2.0-only': + case 'GPL-2.0-or-later': + return 'GNU GPLv2'; + case 'GPL-3.0-only': + case 'GPL-3.0-or-later': + return 'GNU GPLv3'; + case 'LGPL-2.1-only': + case 'LGPL-2.1-or-later': + return 'GNU LGPLv2.1'; + case 'LGPL-3.0-only': + case 'LGPL-3.0-or-later': + return 'GNU LGPLv3'; + case 'BSD-2-Clause': + return 'BSD-2-Clause'; + case 'BSD-3-Clause': + return 'BSD-3-Clause'; + case 'MPL-2.0': + return 'MPL-2.0'; + case 'OSL-3.0': + return 'OSL-3.0'; + case 'Apache-2.0': + return 'Apache-2.0'; + case 'AGPL-3.0-only': + case 'AGPL-3.0-or-later': + return 'GNU AGPLv3'; + case 'BUSL-1.1': + return 'BSL 1.1'; + default: + throw new UpgradesError( + `SPDX license identifier ${spdxIdentifier} in ${contractInfo.sourceName} does not look like a supported license for block explorer verification.`, + () => + `Use the \`licenseType\` option to specify a license type, or set the \`skipLicenseType\` option to \`true\` to skip.`, + ); } } diff --git a/packages/plugin-hardhat/src/utils/options.ts b/packages/plugin-hardhat/src/utils/options.ts index db2f56c17..318416fdf 100644 --- a/packages/plugin-hardhat/src/utils/options.ts +++ b/packages/plugin-hardhat/src/utils/options.ts @@ -1,3 +1,4 @@ +import { SourceCodeLicense } from '@openzeppelin/defender-sdk-deploy-client'; import { DeployOpts, ProxyKindOption, @@ -64,6 +65,8 @@ export type DefenderDeployOptions = DefenderDeploy & { relayerId?: string; salt?: string; createFactoryAddress?: string; + licenseType?: SourceCodeLicense; + skipLicenseType?: boolean; }; /** diff --git a/packages/plugin-hardhat/test/defender-deploy.js b/packages/plugin-hardhat/test/defender-deploy.js index 29f2fb3d6..e288d2604 100644 --- a/packages/plugin-hardhat/test/defender-deploy.js +++ b/packages/plugin-hardhat/test/defender-deploy.js @@ -99,7 +99,7 @@ test('calls defender deploy', async t => { contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: undefined, @@ -127,7 +127,7 @@ test('calls defender deploy with relayerId', async t => { contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: RELAYER_ID, @@ -155,7 +155,7 @@ test('calls defender deploy with salt', async t => { contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: undefined, @@ -183,7 +183,7 @@ test('calls defender deploy with createFactoryAddress', async t => { contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: undefined, @@ -224,6 +224,205 @@ test('calls defender deploy with license', async t => { assertResult(t, result); }); +test('calls defender deploy - licenseType', async t => { + const { spy, deploy, fakeHre, fakeChainId } = t.context; + + const contractPath = 'contracts/WithLicense.sol'; + const contractName = 'WithLicense'; + + const factory = await ethers.getContractFactory(contractName); + const result = await deploy.defenderDeploy(fakeHre, factory, { + licenseType: 'My License Type', // not a valid type, but this just sets the option + }); + + const buildInfo = await hre.artifacts.getBuildInfo(`${contractPath}:${contractName}`); + sinon.assert.calledWithExactly(spy, { + contractName: contractName, + contractPath: contractPath, + network: fakeChainId, + artifactPayload: JSON.stringify(buildInfo), + licenseType: 'My License Type', + constructorInputs: [], + verifySourceCode: true, + relayerId: undefined, + salt: undefined, + createFactoryAddress: undefined, + txOverrides: undefined, + libraries: undefined, + }); + + assertResult(t, result); +}); + +test('calls defender deploy - verifySourceCode false', async t => { + const { spy, deploy, fakeHre, fakeChainId } = t.context; + + const contractPath = 'contracts/WithLicense.sol'; + const contractName = 'WithLicense'; + + const factory = await ethers.getContractFactory(contractName); + const result = await deploy.defenderDeploy(fakeHre, factory, { + verifySourceCode: false, + }); + + const buildInfo = await hre.artifacts.getBuildInfo(`${contractPath}:${contractName}`); + sinon.assert.calledWithExactly(spy, { + contractName: contractName, + contractPath: contractPath, + network: fakeChainId, + artifactPayload: JSON.stringify(buildInfo), + licenseType: undefined, + constructorInputs: [], + verifySourceCode: false, + relayerId: undefined, + salt: undefined, + createFactoryAddress: undefined, + txOverrides: undefined, + libraries: undefined, + }); + + assertResult(t, result); +}); + +test('calls defender deploy - skipLicenseType', async t => { + const { spy, deploy, fakeHre, fakeChainId } = t.context; + + const contractPath = 'contracts/WithLicense.sol'; + const contractName = 'WithLicense'; + + const factory = await ethers.getContractFactory(contractName); + const result = await deploy.defenderDeploy(fakeHre, factory, { + skipLicenseType: true, + }); + + const buildInfo = await hre.artifacts.getBuildInfo(`${contractPath}:${contractName}`); + sinon.assert.calledWithExactly(spy, { + contractName: contractName, + contractPath: contractPath, + network: fakeChainId, + artifactPayload: JSON.stringify(buildInfo), + licenseType: undefined, + constructorInputs: [], + verifySourceCode: true, + relayerId: undefined, + salt: undefined, + createFactoryAddress: undefined, + txOverrides: undefined, + libraries: undefined, + }); + + assertResult(t, result); +}); + +test('calls defender deploy - error - licenseType with skipLicenseType true', async t => { + const { deploy, fakeHre } = t.context; + + const contractName = 'WithLicense'; + + const factory = await ethers.getContractFactory(contractName); + const error = await t.throwsAsync(() => + deploy.defenderDeploy(fakeHre, factory, { + licenseType: 'MIT', + skipLicenseType: true, + }), + ); + t.true( + error?.message.includes('The `licenseType` option cannot be used when the `skipLicenseType` option is `true`'), + ); +}); + +test('calls defender deploy - error - licenseType with verifySourceCode false', async t => { + const { deploy, fakeHre } = t.context; + + const contractName = 'WithLicense'; + + const factory = await ethers.getContractFactory(contractName); + const error = await t.throwsAsync(() => + deploy.defenderDeploy(fakeHre, factory, { + licenseType: 'MIT', + verifySourceCode: false, + }), + ); + t.true( + error?.message.includes('The `licenseType` option cannot be used when the `verifySourceCode` option is `false`'), + ); +}); + +test('calls defender deploy - error - unrecognized license', async t => { + const { deploy, fakeHre } = t.context; + + const contractName = 'UnrecognizedLicense'; + + const factory = await ethers.getContractFactory(contractName); + const error = await t.throwsAsync(() => deploy.defenderDeploy(fakeHre, factory, {})); + t.true( + error?.message.includes( + 'SPDX license identifier UnrecognizedId in contracts/UnrecognizedLicense.sol does not look like a supported license for block explorer verification.', + ), + ); + t.true( + error?.message.includes( + 'Use the `licenseType` option to specify a license type, or set the `skipLicenseType` option to `true` to skip.', + ), + ); +}); + +test('calls defender deploy - no contract license', async t => { + const { spy, deploy, fakeHre, fakeChainId } = t.context; + + const contractPath = 'contracts/NoLicense.sol'; + const contractName = 'NoLicense'; + + const factory = await ethers.getContractFactory(contractName); + const result = await deploy.defenderDeploy(fakeHre, factory, {}); + + const buildInfo = await hre.artifacts.getBuildInfo(`${contractPath}:${contractName}`); + sinon.assert.calledWithExactly(spy, { + contractName: contractName, + contractPath: contractPath, + network: fakeChainId, + artifactPayload: JSON.stringify(buildInfo), + licenseType: undefined, + constructorInputs: [], + verifySourceCode: true, + relayerId: undefined, + salt: undefined, + createFactoryAddress: undefined, + txOverrides: undefined, + libraries: undefined, + }); + + assertResult(t, result); +}); + +test('calls defender deploy - unlicensed', async t => { + const { spy, deploy, fakeHre, fakeChainId } = t.context; + + const contractPath = 'contracts/Unlicensed.sol'; + const contractName = 'Unlicensed'; + + const factory = await ethers.getContractFactory(contractName); + const result = await deploy.defenderDeploy(fakeHre, factory, {}); + + const buildInfo = await hre.artifacts.getBuildInfo(`${contractPath}:${contractName}`); + sinon.assert.calledWithExactly(spy, { + contractName: contractName, + contractPath: contractPath, + network: fakeChainId, + artifactPayload: JSON.stringify(buildInfo), + licenseType: 'None', + constructorInputs: [], + verifySourceCode: true, + relayerId: undefined, + salt: undefined, + createFactoryAddress: undefined, + txOverrides: undefined, + libraries: undefined, + }); + + assertResult(t, result); +}); + test('calls defender deploy with constructor args', async t => { const { spy, deploy, fakeHre, fakeChainId } = t.context; @@ -373,7 +572,7 @@ test('calls defender deploy with txOverrides.gasLimit', async t => { contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: undefined, @@ -406,7 +605,7 @@ test('calls defender deploy with txOverrides.gasPrice', async t => { contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: undefined, @@ -441,7 +640,7 @@ test('calls defender deploy with txOverrides.maxFeePerGas and txOverrides.maxPri contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: undefined, @@ -478,7 +677,7 @@ test('calls defender deploy with external library', async t => { contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: undefined, @@ -513,7 +712,7 @@ test('calls defender deploy with multiple external libraries', async t => { contractPath: contractPath, network: fakeChainId, artifactPayload: JSON.stringify(buildInfo), - licenseType: 'None', + licenseType: undefined, constructorInputs: [], verifySourceCode: true, relayerId: undefined, diff --git a/submodules/openzeppelin-foundry-upgrades b/submodules/openzeppelin-foundry-upgrades index 8a7c35fca..332bd3306 160000 --- a/submodules/openzeppelin-foundry-upgrades +++ b/submodules/openzeppelin-foundry-upgrades @@ -1 +1 @@ -Subproject commit 8a7c35fca2a1fbe9be290ffde251491456ee3c4d +Subproject commit 332bd3306242e09520df2685b2edb99ebd7f5d37