From 332bd3306242e09520df2685b2edb99ebd7f5d37 Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Wed, 17 Apr 2024 11:28:53 -0400 Subject: [PATCH] Defender: Improve license type handling (#43) --- CHANGELOG.md | 4 + .../api/pages/api-foundry-upgrades.adoc | 2 + package.json | 2 +- src/Options.sol | 13 ++ src/internal/DefenderDeploy.sol | 59 +++++- src/internal/Utils.sol | 6 +- src/internal/Versions.sol | 2 +- test/contracts/NoLicense.sol | 3 + test/contracts/Unlicensed.sol | 4 + test/contracts/UnrecognizedLicense.sol | 4 + test/internal/DefenderDeploy.t.sol | 197 +++++++++++++++++- yarn.lock | 8 +- 12 files changed, 290 insertions(+), 14 deletions(-) create mode 100644 test/contracts/NoLicense.sol create mode 100644 test/contracts/Unlicensed.sol create mode 100644 test/contracts/UnrecognizedLicense.sol diff --git a/CHANGELOG.md b/CHANGELOG.md index aa10bfb..f76702b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.2.2 (2024-04-17) + +- Defender: Fix handling of license types for block explorer verification, support `licenseType` and `skipLicenseType` options. ([#43](https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/pull/43)) + ## 0.2.1 (2024-03-20) - Throw helpful error message if AST not found in contract artifacts. ([#28](https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/pull/28)) diff --git a/docs/modules/api/pages/api-foundry-upgrades.adoc b/docs/modules/api/pages/api-foundry-upgrades.adoc index 7e51b1f..54e24ea 100644 --- a/docs/modules/api/pages/api-foundry-upgrades.adoc +++ b/docs/modules/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/package.json b/package.json index af75472..14f4c1d 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ "devDependencies": { "@nomicfoundation/hardhat-foundry": "^1.1.1", "@openzeppelin/contracts": "^5.0.2", - "@openzeppelin/defender-deploy-client-cli": "0.0.1-alpha.5", + "@openzeppelin/defender-deploy-client-cli": "0.0.1-alpha.6", "@openzeppelin/upgrades-core": "^1.32.3", "hardhat": "^2.21.0", "prettier": "^3.0.0", diff --git a/src/Options.sol b/src/Options.sol index e689e95..5aa4826 100644 --- a/src/Options.sol +++ b/src/Options.sol @@ -66,4 +66,17 @@ struct DefenderOptions { * Defaults to the upgrade approval process configured for your deployment environment on Defender. */ string upgradeApprovalProcessId; + /** + * License type to display on block explorers for verified source code. + * See https://etherscan.io/contract-license-types for supported values and use the string found in brackets, e.g. MIT. + * If not set, infers the license type by using the SPDX license identifier from the contract's Solidity file. + * Cannot be set if `skipLicenseType` or `skipVerifySourceCode` is `true`. + */ + string licenseType; + /** + * If set to `true`, does not set the license type on block explorers for verified source code. + * Use this if your contract's license type is not supported by block explorers. + * Defaults to `false`. + */ + bool skipLicenseType; } diff --git a/src/internal/DefenderDeploy.sol b/src/internal/DefenderDeploy.sol index b57669e..de46b75 100644 --- a/src/internal/DefenderDeploy.sol +++ b/src/internal/DefenderDeploy.sol @@ -54,6 +54,14 @@ library DefenderDeploy { ) internal view returns (string[] memory) { Vm vm = Vm(Utils.CHEATCODE_ADDRESS); + if (!(defenderOpts.licenseType).toSlice().empty()) { + if (defenderOpts.skipVerifySourceCode) { + revert("The `licenseType` option cannot be used when the `skipVerifySourceCode` option is `true`"); + } else if (defenderOpts.skipLicenseType) { + revert("The `licenseType` option cannot be used when the `skipLicenseType` option is `true`"); + } + } + string[] memory inputBuilder = new string[](255); uint8 i = 0; @@ -72,8 +80,6 @@ library DefenderDeploy { inputBuilder[i++] = Strings.toString(block.chainid); inputBuilder[i++] = "--buildInfoFile"; inputBuilder[i++] = buildInfoFile; - inputBuilder[i++] = "--licenseType"; - inputBuilder[i++] = contractInfo.license; if (constructorData.length > 0) { inputBuilder[i++] = "--constructorBytecode"; inputBuilder[i++] = vm.toString(constructorData); @@ -81,6 +87,12 @@ library DefenderDeploy { if (defenderOpts.skipVerifySourceCode) { inputBuilder[i++] = "--verifySourceCode"; inputBuilder[i++] = "false"; + } else if (!(defenderOpts.licenseType).toSlice().empty()) { + inputBuilder[i++] = "--licenseType"; + inputBuilder[i++] = string.concat('"', defenderOpts.licenseType, '"'); + } else if (!defenderOpts.skipLicenseType && !(contractInfo.license).toSlice().empty()) { + inputBuilder[i++] = "--licenseType"; + inputBuilder[i++] = string.concat('"', _toLicenseType(contractInfo), '"'); } if (!(defenderOpts.relayerId).toSlice().empty()) { inputBuilder[i++] = "--relayerId"; @@ -100,6 +112,49 @@ library DefenderDeploy { return inputs; } + function _toLicenseType(ContractInfo memory contractInfo) private pure returns (string memory) { + strings.slice memory id = contractInfo.license.toSlice(); + if (id.equals("UNLICENSED".toSlice())) { + return "None"; + } else if (id.equals("Unlicense".toSlice())) { + return "Unlicense"; + } else if (id.equals("MIT".toSlice())) { + return "MIT"; + } else if (id.equals("GPL-2.0-only".toSlice()) || id.equals("GPL-2.0-or-later".toSlice())) { + return "GNU GPLv2"; + } else if (id.equals("GPL-3.0-only".toSlice()) || id.equals("GPL-3.0-or-later".toSlice())) { + return "GNU GPLv3"; + } else if (id.equals("LGPL-2.1-only".toSlice()) || id.equals("LGPL-2.1-or-later".toSlice())) { + return "GNU LGPLv2.1"; + } else if (id.equals("LGPL-3.0-only".toSlice()) || id.equals("LGPL-3.0-or-later".toSlice())) { + return "GNU LGPLv3"; + } else if (id.equals("BSD-2-Clause".toSlice())) { + return "BSD-2-Clause"; + } else if (id.equals("BSD-3-Clause".toSlice())) { + return "BSD-3-Clause"; + } else if (id.equals("MPL-2.0".toSlice())) { + return "MPL-2.0"; + } else if (id.equals("OSL-3.0".toSlice())) { + return "OSL-3.0"; + } else if (id.equals("Apache-2.0".toSlice())) { + return "Apache-2.0"; + } else if (id.equals("AGPL-3.0-only".toSlice()) || id.equals("AGPL-3.0-or-later".toSlice())) { + return "GNU AGPLv3"; + } else if (id.equals("BUSL-1.1".toSlice())) { + return "BSL 1.1"; + } else { + revert( + string.concat( + "SPDX license identifier ", + contractInfo.license, + " in ", + contractInfo.contractPath, + " 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." + ) + ); + } + } + function proposeUpgrade( address proxyAddress, address proxyAdminAddress, diff --git a/src/internal/Utils.sol b/src/internal/Utils.sol index 92718e8..7837a6c 100644 --- a/src/internal/Utils.sol +++ b/src/internal/Utils.sol @@ -15,7 +15,7 @@ struct ContractInfo { */ string shortName; /** - * License identifier from the compiled artifact + * License identifier from the compiled artifact. Empty if not found. */ string license; /** @@ -86,7 +86,9 @@ library Utils { ); } info.contractPath = vm.parseJsonString(artifactJson, ".ast.absolutePath"); - info.license = vm.parseJsonString(artifactJson, ".ast.license"); + if (vm.keyExistsJson(artifactJson, ".ast.license")) { + info.license = vm.parseJsonString(artifactJson, ".ast.license"); + } info.sourceCodeHash = vm.parseJsonString( artifactJson, string.concat(".metadata.sources.['", info.contractPath, "'].keccak256") diff --git a/src/internal/Versions.sol b/src/internal/Versions.sol index 6281f6b..77183ac 100644 --- a/src/internal/Versions.sol +++ b/src/internal/Versions.sol @@ -4,5 +4,5 @@ pragma solidity ^0.8.20; library Versions { // TODO add a workflow to update this automatically based on package.json string constant UPGRADES_CORE = "^1.32.3"; - string constant DEFENDER_DEPLOY_CLIENT_CLI = "0.0.1-alpha.5"; + string constant DEFENDER_DEPLOY_CLIENT_CLI = "0.0.1-alpha.6"; } diff --git a/test/contracts/NoLicense.sol b/test/contracts/NoLicense.sol new file mode 100644 index 0000000..ebef766 --- /dev/null +++ b/test/contracts/NoLicense.sol @@ -0,0 +1,3 @@ +pragma solidity ^0.8.20; + +contract NoLicense {} diff --git a/test/contracts/Unlicensed.sol b/test/contracts/Unlicensed.sol new file mode 100644 index 0000000..f73bb73 --- /dev/null +++ b/test/contracts/Unlicensed.sol @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.20; + +contract Unlicensed {} diff --git a/test/contracts/UnrecognizedLicense.sol b/test/contracts/UnrecognizedLicense.sol new file mode 100644 index 0000000..34768fe --- /dev/null +++ b/test/contracts/UnrecognizedLicense.sol @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: UnrecognizedId +pragma solidity ^0.8.20; + +contract UnrecognizedLicense {} diff --git a/test/internal/DefenderDeploy.t.sol b/test/internal/DefenderDeploy.t.sol index 10b1b3e..d9cc08b 100644 --- a/test/internal/DefenderDeploy.t.sol +++ b/test/internal/DefenderDeploy.t.sol @@ -9,6 +9,9 @@ import {Versions} from "openzeppelin-foundry-upgrades/internal/Versions.sol"; import {Options, DefenderOptions} from "openzeppelin-foundry-upgrades/Options.sol"; import {ProposeUpgradeResponse, ApprovalProcessResponse} from "openzeppelin-foundry-upgrades/Defender.sol"; import {WithConstructor} from "../contracts/WithConstructor.sol"; +import {UnrecognizedLicense} from "../contracts/UnrecognizedLicense.sol"; +import {NoLicense} from "../contracts/NoLicense.sol"; +import {Unlicensed} from "../contracts/Unlicensed.sol"; /** * @dev Tests the DefenderDeploy internal library. @@ -45,7 +48,7 @@ contract DefenderDeployTest is Test { Versions.DEFENDER_DEPLOY_CLIENT_CLI, " deploy --contractName MyContractName --contractPath test/contracts/MyContractFile.sol --chainId 31337 --buildInfoFile ", buildInfoFile, - " --licenseType MIT" + ' --licenseType "MIT"' ) ); } @@ -72,7 +75,7 @@ contract DefenderDeployTest is Test { Versions.DEFENDER_DEPLOY_CLIENT_CLI, " deploy --contractName WithConstructor --contractPath test/contracts/WithConstructor.sol --chainId 31337 --buildInfoFile ", buildInfoFile, - " --licenseType MIT --constructorBytecode 0x000000000000000000000000000000000000000000000000000000000000007b" + ' --constructorBytecode 0x000000000000000000000000000000000000000000000000000000000000007b --licenseType "MIT"' ) ); } @@ -89,9 +92,67 @@ contract DefenderDeployTest is Test { DefenderOptions memory opts; opts.useDefenderDeploy = true; - opts.skipVerifySourceCode = true; opts.relayerId = "my-relayer-id"; opts.salt = 0xabc0000000000000000000000000000000000000000000000000000000000123; + opts.licenseType = "My License Type"; // not a valid type, but this just sets the option + + string memory commandString = _toString( + DefenderDeploy.buildDeployCommand(contractInfo, buildInfoFile, constructorData, opts) + ); + + assertEq( + commandString, + string.concat( + "npx @openzeppelin/defender-deploy-client-cli@", + Versions.DEFENDER_DEPLOY_CLIENT_CLI, + " deploy --contractName WithConstructor --contractPath test/contracts/WithConstructor.sol --chainId 31337 --buildInfoFile ", + buildInfoFile, + ' --constructorBytecode 0x000000000000000000000000000000000000000000000000000000000000007b --licenseType "My License Type" --relayerId my-relayer-id --salt 0xabc0000000000000000000000000000000000000000000000000000000000123' + ) + ); + } + + function testBuildDeployCommandSkipVerifySourceCode() public { + ContractInfo memory contractInfo = Utils.getContractInfo("WithConstructor.sol:WithConstructor", "out"); + string memory buildInfoFile = Utils.getBuildInfoFile( + contractInfo.sourceCodeHash, + contractInfo.shortName, + "out" + ); + + bytes memory constructorData = abi.encode(123); + + DefenderOptions memory opts; + opts.skipVerifySourceCode = true; + + string memory commandString = _toString( + DefenderDeploy.buildDeployCommand(contractInfo, buildInfoFile, constructorData, opts) + ); + + assertEq( + commandString, + string.concat( + "npx @openzeppelin/defender-deploy-client-cli@", + Versions.DEFENDER_DEPLOY_CLIENT_CLI, + " deploy --contractName WithConstructor --contractPath test/contracts/WithConstructor.sol --chainId 31337 --buildInfoFile ", + buildInfoFile, + " --constructorBytecode 0x000000000000000000000000000000000000000000000000000000000000007b --verifySourceCode false" + ) + ); + } + + function testBuildDeployCommandSkipLicenseType() public { + ContractInfo memory contractInfo = Utils.getContractInfo("WithConstructor.sol:WithConstructor", "out"); + string memory buildInfoFile = Utils.getBuildInfoFile( + contractInfo.sourceCodeHash, + contractInfo.shortName, + "out" + ); + + bytes memory constructorData = abi.encode(123); + + DefenderOptions memory opts; + opts.skipLicenseType = true; string memory commandString = _toString( DefenderDeploy.buildDeployCommand(contractInfo, buildInfoFile, constructorData, opts) @@ -104,7 +165,124 @@ contract DefenderDeployTest is Test { Versions.DEFENDER_DEPLOY_CLIENT_CLI, " deploy --contractName WithConstructor --contractPath test/contracts/WithConstructor.sol --chainId 31337 --buildInfoFile ", buildInfoFile, - " --licenseType MIT --constructorBytecode 0x000000000000000000000000000000000000000000000000000000000000007b --verifySourceCode false --relayerId my-relayer-id --salt 0xabc0000000000000000000000000000000000000000000000000000000000123" + " --constructorBytecode 0x000000000000000000000000000000000000000000000000000000000000007b" + ) + ); + } + + function testBuildDeployCommand_error_licenseType_skipLicenseType() public { + ContractInfo memory contractInfo = Utils.getContractInfo("WithConstructor.sol:WithConstructor", "out"); + string memory buildInfoFile = Utils.getBuildInfoFile( + contractInfo.sourceCodeHash, + contractInfo.shortName, + "out" + ); + + bytes memory constructorData = abi.encode(123); + + DefenderOptions memory opts; + opts.skipLicenseType = true; + opts.licenseType = "MyLicenseType"; + + Invoker i = new Invoker(); + try i.buildDeployCommand(contractInfo, buildInfoFile, constructorData, opts) { + fail(); + } catch Error(string memory reason) { + assertEq(reason, "The `licenseType` option cannot be used when the `skipLicenseType` option is `true`"); + } + } + + function testBuildDeployCommand_error_licenseType_skipVerifySourceCode() public { + ContractInfo memory contractInfo = Utils.getContractInfo("WithConstructor.sol:WithConstructor", "out"); + string memory buildInfoFile = Utils.getBuildInfoFile( + contractInfo.sourceCodeHash, + contractInfo.shortName, + "out" + ); + + bytes memory constructorData = abi.encode(123); + + DefenderOptions memory opts; + opts.skipVerifySourceCode = true; + opts.licenseType = "MyLicenseType"; + + Invoker i = new Invoker(); + try i.buildDeployCommand(contractInfo, buildInfoFile, constructorData, opts) { + fail(); + } catch Error(string memory reason) { + assertEq( + reason, + "The `licenseType` option cannot be used when the `skipVerifySourceCode` option is `true`" + ); + } + } + + function testBuildDeployCommand_error_unrecognizedLicense() public { + ContractInfo memory contractInfo = Utils.getContractInfo("UnrecognizedLicense.sol:UnrecognizedLicense", "out"); + string memory buildInfoFile = Utils.getBuildInfoFile( + contractInfo.sourceCodeHash, + contractInfo.shortName, + "out" + ); + + DefenderOptions memory opts; + + Invoker i = new Invoker(); + try i.buildDeployCommand(contractInfo, buildInfoFile, "", opts) { + fail(); + } catch Error(string memory reason) { + assertEq( + reason, + "SPDX license identifier UnrecognizedId in test/contracts/UnrecognizedLicense.sol 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." + ); + } + } + + function testBuildDeployCommandNoContractLicense() public { + ContractInfo memory contractInfo = Utils.getContractInfo("NoLicense.sol:NoLicense", "out"); + string memory buildInfoFile = Utils.getBuildInfoFile( + contractInfo.sourceCodeHash, + contractInfo.shortName, + "out" + ); + + DefenderOptions memory opts; + string memory commandString = _toString( + DefenderDeploy.buildDeployCommand(contractInfo, buildInfoFile, "", opts) + ); + + assertEq( + commandString, + string.concat( + "npx @openzeppelin/defender-deploy-client-cli@", + Versions.DEFENDER_DEPLOY_CLIENT_CLI, + " deploy --contractName NoLicense --contractPath test/contracts/NoLicense.sol --chainId 31337 --buildInfoFile ", + buildInfoFile + ) + ); + } + + function testBuildDeployCommandUnlicensed() public { + ContractInfo memory contractInfo = Utils.getContractInfo("Unlicensed.sol:Unlicensed", "out"); + string memory buildInfoFile = Utils.getBuildInfoFile( + contractInfo.sourceCodeHash, + contractInfo.shortName, + "out" + ); + + DefenderOptions memory opts; + string memory commandString = _toString( + DefenderDeploy.buildDeployCommand(contractInfo, buildInfoFile, "", opts) + ); + + assertEq( + commandString, + string.concat( + "npx @openzeppelin/defender-deploy-client-cli@", + Versions.DEFENDER_DEPLOY_CLIENT_CLI, + " deploy --contractName Unlicensed --contractPath test/contracts/Unlicensed.sol --chainId 31337 --buildInfoFile ", + buildInfoFile, + ' --licenseType "None"' ) ); } @@ -188,3 +366,14 @@ contract DefenderDeployTest is Test { assertEq(response.viaType, ""); } } + +contract Invoker { + function buildDeployCommand( + ContractInfo memory contractInfo, + string memory buildInfoFile, + bytes memory constructorData, + DefenderOptions memory defenderOpts + ) public view { + DefenderDeploy.buildDeployCommand(contractInfo, buildInfoFile, constructorData, defenderOpts); + } +} diff --git a/yarn.lock b/yarn.lock index 5bf9ccc..9086f9a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -428,10 +428,10 @@ resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.2.tgz#b1d03075e49290d06570b2fd42154d76c2a5d210" integrity sha512-ytPc6eLGcHHnapAZ9S+5qsdomhjo6QBHTDRRBFfTxXIpsicMhVPouPgmUPebZZZGX7vt9USA+Z+0M0dSVtSUEA== -"@openzeppelin/defender-deploy-client-cli@0.0.1-alpha.5": - version "0.0.1-alpha.5" - resolved "https://registry.yarnpkg.com/@openzeppelin/defender-deploy-client-cli/-/defender-deploy-client-cli-0.0.1-alpha.5.tgz#088643b9574c6ede1fab538b52c772bfe6cc7465" - integrity sha512-Ict4BAiV6Xa6MjGaXFoHYif+LG9MqZ4IihDkKswbceoxAx1QosMA36tRV4CV/24ZLt2FZKH7QJjZtWhjVX7CZQ== +"@openzeppelin/defender-deploy-client-cli@0.0.1-alpha.6": + version "0.0.1-alpha.6" + resolved "https://registry.yarnpkg.com/@openzeppelin/defender-deploy-client-cli/-/defender-deploy-client-cli-0.0.1-alpha.6.tgz#73a7671cc8343883ad8b462784e3ce5a41d9d150" + integrity sha512-yye4SGdaRFRo3BVSJEsJQzguBAYFIX7x9O6KRrCTKR8pcPsNn+tSBokk4qDMKKYmdMcAfZVCvGxwG3F3ZjXcmg== dependencies: "@openzeppelin/defender-sdk-base-client" "^1.10.0" "@openzeppelin/defender-sdk-deploy-client" "^1.10.0"