From 32c7c5cba77343dad73c2bd32e1bbb8705478933 Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Tue, 14 Jan 2025 16:24:53 -0500 Subject: [PATCH 01/10] Remove solidity-stringutils --- .gitmodules | 3 - CHANGELOG.md | 4 ++ README.md | 8 +-- docs/modules/pages/foundry-upgrades.adoc | 6 +- lib/forge-std | 2 +- lib/solidity-stringutils | 1 - package.json | 3 +- src/internal/Core.sol | 17 +++-- src/internal/DefenderDeploy.sol | 72 ++++++++++--------- src/internal/StringFinder.sol | 52 ++++++++++++++ src/internal/Utils.sol | 67 +++++++---------- .../build-info-v2-bad/test/Upgrades.t.sol | 8 +-- test/Upgrades.t.sol | 16 ++--- test/UpgradesUseDefenderDeploy.t.sol | 48 ++++++------- test/internal/Core.t.sol | 4 +- test/internal/DefenderDeploy.t.sol | 10 +-- test/internal/StringFinder.t.sol | 40 +++++++++++ test/internal/Utils.t.sol | 27 +++---- 18 files changed, 228 insertions(+), 160 deletions(-) delete mode 160000 lib/solidity-stringutils create mode 100644 src/internal/StringFinder.sol create mode 100644 test/internal/StringFinder.t.sol diff --git a/.gitmodules b/.gitmodules index 9b09120..888d42d 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,6 +1,3 @@ [submodule "lib/forge-std"] path = lib/forge-std url = https://github.com/foundry-rs/forge-std -[submodule "lib/solidity-stringutils"] - path = lib/solidity-stringutils - url = https://github.com/Arachnid/solidity-stringutils diff --git a/CHANGELOG.md b/CHANGELOG.md index a1295fc..fc9bb0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Remove dependency on `solidity-stringutils`. + ## 0.3.7 (2025-01-13) - Update documentation links. ([#88](https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/pull/88)) diff --git a/README.md b/README.md index c8366cf..4e59e8d 100644 --- a/README.md +++ b/README.md @@ -54,20 +54,18 @@ Follow the steps above, but instead of running `forge install OpenZeppelin/openz npm install @openzeppelin/foundry-upgrades ``` -Then add the following additional lines to `remappings.txt`, in addition to the ones described above: +Then add the following additional line to `remappings.txt`, in addition to the ones described above: ``` openzeppelin-foundry-upgrades/=node_modules/@openzeppelin/foundry-upgrades/src/ -solidity-stringutils/=node_modules/@openzeppelin/foundry-upgrades/lib/solidity-stringutils/ ``` #### Soldeer Follow the steps above, but instead of running `forge install OpenZeppelin/openzeppelin-foundry-upgrades`, use one of the install commands described in https://soldeer.xyz/project/openzeppelin-foundry-upgrades -Then add the following additional lines to `remappings.txt`, in addition to the ones described above (replace `0.3.6` with the version of the plugin that you installed): +Then add the following additional line to `remappings.txt`, in addition to the ones described above (replace `0.3.6` with the version of the plugin that you installed): ``` openzeppelin-foundry-upgrades/=dependencies/openzeppelin-foundry-upgrades-0.3.6/src/ -solidity-stringutils/=dependencies/openzeppelin-foundry-upgrades-0.3.6/lib/solidity-stringutils/ ``` ## OpenZeppelin Defender integration @@ -76,7 +74,7 @@ See [DEFENDER.md](DEFENDER.md) ## Foundry Requirements -This library requires [forge-std](https://github.com/foundry-rs/forge-std) version 1.8.0 or higher. +This library requires [forge-std](https://github.com/foundry-rs/forge-std) version 1.9.5 or higher. ## Before Running diff --git a/docs/modules/pages/foundry-upgrades.adoc b/docs/modules/pages/foundry-upgrades.adoc index 4df5055..3a5343f 100644 --- a/docs/modules/pages/foundry-upgrades.adoc +++ b/docs/modules/pages/foundry-upgrades.adoc @@ -59,22 +59,20 @@ Follow the steps above, but instead of running `forge install OpenZeppelin/openz npm install @openzeppelin/foundry-upgrades ---- -Then add the following additional lines to `remappings.txt`, in addition to the ones described above: +Then add the following additional line to `remappings.txt`, in addition to the ones described above: [source,console] ---- openzeppelin-foundry-upgrades/=node_modules/@openzeppelin/foundry-upgrades/src/ -solidity-stringutils/=node_modules/@openzeppelin/foundry-upgrades/lib/solidity-stringutils/ ---- ==== Soldeer Follow the steps above, but instead of running `forge install OpenZeppelin/openzeppelin-foundry-upgrades`, use one of the install commands described in https://soldeer.xyz/project/openzeppelin-foundry-upgrades -Then add the following additional lines to `remappings.txt`, in addition to the ones described above (replace `0.3.6` with the version of the plugin that you installed): +Then add the following additional line to `remappings.txt`, in addition to the ones described above (replace `0.3.6` with the version of the plugin that you installed): [source,console] ---- openzeppelin-foundry-upgrades/=dependencies/openzeppelin-foundry-upgrades-0.3.6/src/ -solidity-stringutils/=dependencies/openzeppelin-foundry-upgrades-0.3.6/lib/solidity-stringutils/ ---- == Foundry Requirements diff --git a/lib/forge-std b/lib/forge-std index b6a506d..b93cf4b 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit b6a506db2262cad5ff982a87789ee6d1558ec861 +Subproject commit b93cf4bc34ff214c099dc970b153f85ade8c9f66 diff --git a/lib/solidity-stringutils b/lib/solidity-stringutils deleted file mode 160000 index 4b2fcc4..0000000 --- a/lib/solidity-stringutils +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 4b2fcc43fa0426e19ce88b1f1ec16f5903a2e461 diff --git a/package.json b/package.json index 2544e05..260483e 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,7 @@ "description": "Foundry library for deploying and managing upgradeable contracts", "license": "MIT", "files": [ - "src/**/*", - "lib/solidity-stringutils/**/*" + "src/**/*" ], "repository": { "type": "git", diff --git a/src/internal/Core.sol b/src/internal/Core.sol index 23c8fc9..4b8cbec 100644 --- a/src/internal/Core.sol +++ b/src/internal/Core.sol @@ -3,7 +3,8 @@ pragma solidity ^0.8.0; import {Vm} from "forge-std/Vm.sol"; import {console} from "forge-std/console.sol"; -import {strings} from "solidity-stringutils/src/strings.sol"; + +import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; import {Options} from "../Options.sol"; import {Versions} from "./Versions.sol"; @@ -61,6 +62,8 @@ library Core { upgradeProxy(proxy, contractName, data, opts); } + using Strings for *; + /** * @dev Upgrades a proxy to a new implementation contract. Only supported for UUPS or transparent proxies. * @@ -74,7 +77,7 @@ library Core { bytes32 adminSlot = vm.load(proxy, ADMIN_SLOT); if (adminSlot == bytes32(0)) { string memory upgradeInterfaceVersion = getUpgradeInterfaceVersion(proxy); - if (upgradeInterfaceVersion.toSlice().equals("5.0.0".toSlice()) || data.length > 0) { + if (upgradeInterfaceVersion.equal("5.0.0") || data.length > 0) { IUpgradeableProxy(proxy).upgradeToAndCall(newImpl, data); } else { IUpgradeableProxy(proxy).upgradeTo(newImpl); @@ -82,7 +85,7 @@ library Core { } else { address admin = address(uint160(uint256(adminSlot))); string memory upgradeInterfaceVersion = getUpgradeInterfaceVersion(admin); - if (upgradeInterfaceVersion.toSlice().equals("5.0.0".toSlice()) || data.length > 0) { + if (upgradeInterfaceVersion.equal("5.0.0") || data.length > 0) { IProxyAdmin(admin).upgradeAndCall(proxy, newImpl, data); } else { IProxyAdmin(admin).upgrade(proxy, newImpl); @@ -300,8 +303,6 @@ library Core { */ bytes32 private constant BEACON_SLOT = 0xa3f0ad74e5423aebfd80d3ef4346578335a9a72aeaee59ff6cb3582b35133d50; - using strings for *; - /** * @dev Gets the upgrade interface version string from a proxy or admin contract using the `UPGRADE_INTERFACE_VERSION()` getter. * If the contract does not have the getter or the return data does not look like a string, this function returns an empty string. @@ -346,7 +347,8 @@ library Core { // CLI validate command uses exit code to indicate if the validation passed or failed. // As an extra precaution, we also check stdout for "SUCCESS" to ensure it actually ran. - if (result.exitCode == 0 && stdout.toSlice().contains("SUCCESS".toSlice())) { + Vm vm = Vm(Utils.CHEATCODE_ADDRESS); + if (result.exitCode == 0 && vm.contains(stdout, "SUCCESS")) { return; } else if (result.stderr.length > 0) { // Validations failed to run @@ -361,7 +363,7 @@ library Core { string memory contractName, Options memory opts, bool requireReference - ) internal view returns (string[] memory) { + ) internal returns (string[] memory) { string memory outDir = Utils.getOutDir(); string[] memory inputBuilder = new string[](2 ** 16); @@ -456,6 +458,7 @@ library Core { function _deployFromBytecode(bytes memory bytecode) private returns (address) { address addr; + /// @solidity memory-safe-assembly assembly { addr := create(0, add(bytecode, 32), mload(bytecode)) } diff --git a/src/internal/DefenderDeploy.sol b/src/internal/DefenderDeploy.sol index f2c00b5..281385b 100644 --- a/src/internal/DefenderDeploy.sol +++ b/src/internal/DefenderDeploy.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.0; import {Vm} from "forge-std/Vm.sol"; import {console} from "forge-std/console.sol"; -import {strings} from "solidity-stringutils/src/strings.sol"; import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; @@ -18,8 +17,6 @@ import {ProposeUpgradeResponse, ApprovalProcessResponse} from "../Defender.sol"; * WARNING: DO NOT USE DIRECTLY. Use Defender.sol instead. */ library DefenderDeploy { - using strings for *; - function deploy( string memory contractName, bytes memory constructorData, @@ -54,7 +51,7 @@ library DefenderDeploy { ) internal view returns (string[] memory) { Vm vm = Vm(Utils.CHEATCODE_ADDRESS); - if (!(defenderOpts.licenseType).toSlice().empty()) { + if (bytes(defenderOpts.licenseType).length != 0) { if (defenderOpts.skipVerifySourceCode) { revert("The `licenseType` option cannot be used when the `skipVerifySourceCode` option is `true`"); } else if (defenderOpts.skipLicenseType) { @@ -86,14 +83,14 @@ library DefenderDeploy { if (defenderOpts.skipVerifySourceCode) { inputBuilder[i++] = "--verifySourceCode"; inputBuilder[i++] = "false"; - } else if (!(defenderOpts.licenseType).toSlice().empty()) { + } else if (bytes(defenderOpts.licenseType).length != 0) { inputBuilder[i++] = "--licenseType"; inputBuilder[i++] = string(abi.encodePacked('"', defenderOpts.licenseType, '"')); - } else if (!defenderOpts.skipLicenseType && !(contractInfo.license).toSlice().empty()) { + } else if (!defenderOpts.skipLicenseType && bytes(contractInfo.license).length != 0) { inputBuilder[i++] = "--licenseType"; inputBuilder[i++] = string(abi.encodePacked('"', _toLicenseType(contractInfo), '"')); } - if (!(defenderOpts.relayerId).toSlice().empty()) { + if (bytes(defenderOpts.relayerId).length != 0) { inputBuilder[i++] = "--relayerId"; inputBuilder[i++] = defenderOpts.relayerId; } @@ -117,7 +114,7 @@ library DefenderDeploy { inputBuilder[i++] = "--maxPriorityFeePerGas"; inputBuilder[i++] = Strings.toString(defenderOpts.txOverrides.maxPriorityFeePerGas); } - if (!(defenderOpts.metadata).toSlice().empty()) { + if (bytes(defenderOpts.metadata).length != 0) { inputBuilder[i++] = "--metadata"; inputBuilder[i++] = string(abi.encodePacked('"', vm.replace(defenderOpts.metadata, '"', '\\"'), '"')); } @@ -133,35 +130,37 @@ library DefenderDeploy { return inputs; } + using Strings for string; + function _toLicenseType(ContractInfo memory contractInfo) private pure returns (string memory) { - strings.slice memory id = contractInfo.license.toSlice(); - if (id.equals("UNLICENSED".toSlice())) { + string memory id = contractInfo.license; + if (id.equal("UNLICENSED")) { return "None"; - } else if (id.equals("Unlicense".toSlice())) { + } else if (id.equal("Unlicense")) { return "Unlicense"; - } else if (id.equals("MIT".toSlice())) { + } else if (id.equal("MIT")) { return "MIT"; - } else if (id.equals("GPL-2.0-only".toSlice()) || id.equals("GPL-2.0-or-later".toSlice())) { + } else if (id.equal("GPL-2.0-only") || id.equal("GPL-2.0-or-later")) { return "GNU GPLv2"; - } else if (id.equals("GPL-3.0-only".toSlice()) || id.equals("GPL-3.0-or-later".toSlice())) { + } else if (id.equal("GPL-3.0-only") || id.equal("GPL-3.0-or-later")) { return "GNU GPLv3"; - } else if (id.equals("LGPL-2.1-only".toSlice()) || id.equals("LGPL-2.1-or-later".toSlice())) { + } else if (id.equal("LGPL-2.1-only") || id.equal("LGPL-2.1-or-later")) { return "GNU LGPLv2.1"; - } else if (id.equals("LGPL-3.0-only".toSlice()) || id.equals("LGPL-3.0-or-later".toSlice())) { + } else if (id.equal("LGPL-3.0-only") || id.equal("LGPL-3.0-or-later")) { return "GNU LGPLv3"; - } else if (id.equals("BSD-2-Clause".toSlice())) { + } else if (id.equal("BSD-2-Clause")) { return "BSD-2-Clause"; - } else if (id.equals("BSD-3-Clause".toSlice())) { + } else if (id.equal("BSD-3-Clause")) { return "BSD-3-Clause"; - } else if (id.equals("MPL-2.0".toSlice())) { + } else if (id.equal("MPL-2.0")) { return "MPL-2.0"; - } else if (id.equals("OSL-3.0".toSlice())) { + } else if (id.equal("OSL-3.0")) { return "OSL-3.0"; - } else if (id.equals("Apache-2.0".toSlice())) { + } else if (id.equal("Apache-2.0")) { return "Apache-2.0"; - } else if (id.equals("AGPL-3.0-only".toSlice()) || id.equals("AGPL-3.0-or-later".toSlice())) { + } else if (id.equal("AGPL-3.0-only") || id.equal("AGPL-3.0-or-later")) { return "GNU AGPLv3"; - } else if (id.equals("BUSL-1.1".toSlice())) { + } else if (id.equal("BUSL-1.1")) { return "BSL 1.1"; } else { revert( @@ -217,7 +216,7 @@ library DefenderDeploy { return parseProposeUpgradeResponse(stdout); } - function parseProposeUpgradeResponse(string memory stdout) internal pure returns (ProposeUpgradeResponse memory) { + function parseProposeUpgradeResponse(string memory stdout) internal returns (ProposeUpgradeResponse memory) { ProposeUpgradeResponse memory response; response.proposalId = _parseLine("Proposal ID: ", stdout, true); response.url = _parseLine("Proposal URL: ", stdout, false); @@ -228,15 +227,20 @@ library DefenderDeploy { string memory expectedPrefix, string memory stdout, bool required - ) private pure returns (string memory) { - strings.slice memory delim = expectedPrefix.toSlice(); - if (stdout.toSlice().contains(delim)) { - strings.slice memory slice = stdout.toSlice().copy().find(delim).beyond(delim); + ) private returns (string memory) { + Vm vm = Vm(Utils.CHEATCODE_ADDRESS); + if (vm.contains(stdout, expectedPrefix)) { + // Get the substring after the prefix + string[] memory segments = vm.split(stdout, expectedPrefix); + string memory suffix = ""; + for (uint256 i = 1; i < segments.length; i++) { + suffix = string(abi.encodePacked(suffix, segments[i])); + } // Remove any following lines - if (slice.contains("\n".toSlice())) { - slice = slice.split("\n".toSlice()); + if (vm.contains(suffix, "\n")) { + suffix = vm.split(suffix, "\n")[0]; } - return slice.toString(); + return suffix; } else if (required) { revert( string(abi.encodePacked("Failed to find line with prefix '", expectedPrefix, "' in output: ", stdout)) @@ -276,7 +280,7 @@ library DefenderDeploy { inputBuilder[i++] = "--proxyAdminAddress"; inputBuilder[i++] = vm.toString(proxyAdminAddress); } - if (!(opts.defender.upgradeApprovalProcessId).toSlice().empty()) { + if (bytes(opts.defender.upgradeApprovalProcessId).length != 0) { inputBuilder[i++] = "--approvalProcessId"; inputBuilder[i++] = opts.defender.upgradeApprovalProcessId; } @@ -303,7 +307,7 @@ library DefenderDeploy { return parseApprovalProcessResponse(stdout); } - function parseApprovalProcessResponse(string memory stdout) internal pure returns (ApprovalProcessResponse memory) { + function parseApprovalProcessResponse(string memory stdout) internal returns (ApprovalProcessResponse memory) { Vm vm = Vm(Utils.CHEATCODE_ADDRESS); ApprovalProcessResponse memory response; @@ -311,7 +315,7 @@ library DefenderDeploy { response.approvalProcessId = _parseLine("Approval process ID: ", stdout, true); string memory viaString = _parseLine("Via: ", stdout, false); - if (viaString.toSlice().len() != 0) { + if (bytes(viaString).length != 0) { response.via = vm.parseAddress(viaString); } diff --git a/src/internal/StringFinder.sol b/src/internal/StringFinder.sol new file mode 100644 index 0000000..c4f4573 --- /dev/null +++ b/src/internal/StringFinder.sol @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {Vm} from "forge-std/Vm.sol"; +import {Utils} from "./Utils.sol"; + +/** + * String finder functions using Forge's string cheatcodes. + * For internal use only. + */ +library StringFinder { + /** + * Returns whether the subject string contains the search string. + */ + function contains(string memory subject, string memory search) internal returns (bool) { + Vm vm = Vm(Utils.CHEATCODE_ADDRESS); + return vm.contains(subject, search); + } + + /** + * Returns whether the subject string starts with the search string. + */ + function startsWith(string memory subject, string memory search) internal pure returns (bool) { + Vm vm = Vm(Utils.CHEATCODE_ADDRESS); + uint256 index = vm.indexOf(subject, search); + return index == 0; + } + + /** + * Returns whether the subject string ends with the search string. + */ + function endsWith(string memory subject, string memory search) internal returns (bool) { + Vm vm = Vm(Utils.CHEATCODE_ADDRESS); + if (!vm.contains(subject, search)) { + return false; + } + string[] memory tokens = vm.split(subject, search); + return bytes(tokens[tokens.length - 1]).length == 0; + } + + /** + * Returns the number of occurrences of the search string in the subject string. + */ + function count(string memory subject, string memory search) internal returns (uint256) { + Vm vm = Vm(Utils.CHEATCODE_ADDRESS); + if (!vm.contains(subject, search)) { + return 0; + } + string[] memory tokens = vm.split(subject, search); + return tokens.length - 1; + } +} diff --git a/src/internal/Utils.sol b/src/internal/Utils.sol index b3c149d..51a7cdc 100644 --- a/src/internal/Utils.sol +++ b/src/internal/Utils.sol @@ -3,7 +3,8 @@ pragma solidity ^0.8.0; import {Vm} from "forge-std/Vm.sol"; import {console} from "forge-std/console.sol"; -import {strings} from "solidity-stringutils/src/strings.sol"; + +import {StringFinder} from "./StringFinder.sol"; struct ContractInfo { /* @@ -41,10 +42,7 @@ library Utils { * @param outDir Foundry output directory to search in if contractName is not an artifact path * @return Fully qualified name of the contract, e.g. "src/MyContract.sol:MyContract" */ - function getFullyQualifiedName( - string memory contractName, - string memory outDir - ) internal view returns (string memory) { + function getFullyQualifiedName(string memory contractName, string memory outDir) internal returns (string memory) { ContractInfo memory info = getContractInfo(contractName, outDir); return string(abi.encodePacked(info.contractPath, ":", info.shortName)); } @@ -56,10 +54,7 @@ library Utils { * @param outDir Foundry output directory to search in if contractName is not an artifact path * @return ContractInfo struct containing information about the contract */ - function getContractInfo( - string memory contractName, - string memory outDir - ) internal view returns (ContractInfo memory) { + function getContractInfo(string memory contractName, string memory outDir) internal returns (ContractInfo memory) { Vm vm = Vm(CHEATCODE_ADDRESS); ContractInfo memory info; @@ -97,7 +92,7 @@ library Utils { return info; } - using strings for *; + using StringFinder for string; /** * Gets the path to the build-info file that contains the given bytecode. @@ -121,7 +116,7 @@ library Utils { Vm.FfiResult memory result = runAsBashCommand(inputs); string memory stdout = string(result.stdout); - if (!stdout.toSlice().endsWith(".json".toSlice())) { + if (!stdout.endsWith(".json")) { revert( string( abi.encodePacked( @@ -145,26 +140,15 @@ library Utils { return vm.envOr("FOUNDRY_OUT", defaultOutDir); } - function _split( - strings.slice memory inputSlice, - strings.slice memory delimSlice - ) private pure returns (string[] memory) { - string[] memory parts = new string[](inputSlice.count(delimSlice) + 1); - for (uint i = 0; i < parts.length; i++) { - parts[i] = inputSlice.split(delimSlice).toString(); - } - return parts; - } - - function _toFileName(string memory contractName) private pure returns (string memory) { - strings.slice memory name = contractName.toSlice(); - if (name.endsWith(".sol".toSlice())) { - return name.toString(); - } else if (name.count(":".toSlice()) == 1) { - return name.split(":".toSlice()).toString(); + function _toFileName(string memory name) private returns (string memory) { + Vm vm = Vm(CHEATCODE_ADDRESS); + if (name.endsWith(".sol")) { + return name; + } else if (name.count(":") == 1) { + return vm.split(name, ":")[0]; } else { - if (name.endsWith(".json".toSlice())) { - string[] memory parts = _split(name, "/".toSlice()); + if (name.endsWith(".json")) { + string[] memory parts = vm.split(name, "/"); if (parts.length > 1) { return parts[parts.length - 2]; } @@ -174,7 +158,7 @@ library Utils { string( abi.encodePacked( "Contract name ", - contractName, + name, " must be in the format MyContract.sol:MyContract or MyContract.sol or out/MyContract.sol/MyContract.json" ) ) @@ -182,23 +166,22 @@ library Utils { } } - function _toShortName(string memory contractName) private pure returns (string memory) { - strings.slice memory name = contractName.toSlice(); - if (name.endsWith(".sol".toSlice())) { - return name.until(".sol".toSlice()).toString(); - } else if (name.count(":".toSlice()) == 1) { - name.split(":".toSlice()); - return name.split(":".toSlice()).toString(); - } else if (name.endsWith(".json".toSlice())) { - string[] memory parts = _split(name, "/".toSlice()); + function _toShortName(string memory name) private returns (string memory) { + Vm vm = Vm(CHEATCODE_ADDRESS); + if (name.endsWith(".sol") && name.count(".sol") == 1) { + return vm.replace(name, ".sol", ""); + } else if (name.count(":") == 1) { + return vm.split(name, ":")[1]; + } else if (name.endsWith(".json") && name.count(".json") == 1) { + string[] memory parts = vm.split(name, "/"); string memory jsonName = parts[parts.length - 1]; - return jsonName.toSlice().until(".json".toSlice()).toString(); + return vm.replace(jsonName, ".json", ""); } else { revert( string( abi.encodePacked( "Contract name ", - contractName, + name, " must be in the format MyContract.sol:MyContract or MyContract.sol or out/MyContract.sol/MyContract.json" ) ) diff --git a/test-profiles/build-info-v2-bad/test/Upgrades.t.sol b/test-profiles/build-info-v2-bad/test/Upgrades.t.sol index 83f102c..6cd55fd 100644 --- a/test-profiles/build-info-v2-bad/test/Upgrades.t.sol +++ b/test-profiles/build-info-v2-bad/test/Upgrades.t.sol @@ -4,12 +4,13 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; import {Upgrades, Options} from "openzeppelin-foundry-upgrades/Upgrades.sol"; -import {strings} from "solidity-stringutils/src/strings.sol"; + +import {StringFinder} from "openzeppelin-foundry-upgrades/internal/StringFinder.sol"; import {MyContract} from "./contracts/MyContract.sol"; contract UpgradesTest is Test { - using strings for *; + using StringFinder for string; function testValidateWithReferenceBuildInfo_Bad() public { Options memory opts; @@ -19,8 +20,7 @@ contract UpgradesTest is Test { try v.validateUpgrade("MyContract.sol", opts) { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("Deleted `x`".toSlice())); + assertTrue(reason.contains("Deleted `x`")); } } } diff --git a/test/Upgrades.t.sol b/test/Upgrades.t.sol index c60f8e6..477caec 100644 --- a/test/Upgrades.t.sol +++ b/test/Upgrades.t.sol @@ -5,6 +5,8 @@ import {Test} from "forge-std/Test.sol"; import {Upgrades, Options} from "openzeppelin-foundry-upgrades/Upgrades.sol"; +import {StringFinder} from "openzeppelin-foundry-upgrades/internal/StringFinder.sol"; + import {IBeacon} from "@openzeppelin/contracts/proxy/beacon/IBeacon.sol"; import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; import {ITransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; @@ -16,8 +18,6 @@ import {GreeterV2Proxiable} from "./contracts/GreeterV2Proxiable.sol"; import {WithConstructor, NoInitializer} from "./contracts/WithConstructor.sol"; import {HasOwner} from "./contracts/HasOwner.sol"; -import {strings} from "solidity-stringutils/src/strings.sol"; - // Import additional contracts to include them for compilation import "./contracts/Validations.sol"; @@ -25,7 +25,7 @@ import "./contracts/Validations.sol"; * @dev Tests for the Upgrades library. */ contract UpgradesTest is Test { - using strings for *; + using StringFinder for string; function testUUPS() public { address proxy = Upgrades.deployUUPSProxy( @@ -281,9 +281,8 @@ contract UpgradesTest is Test { { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("`initialOwner` must not be a ProxyAdmin contract.".toSlice())); - assertTrue(slice.contains(vm.toString(address(admin)).toSlice())); + assertTrue(reason.contains("`initialOwner` must not be a ProxyAdmin contract.")); + assertTrue(reason.contains(vm.toString(address(admin)))); } } @@ -302,9 +301,8 @@ contract UpgradesTest is Test { { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("`initialOwner` must not be a ProxyAdmin contract.".toSlice())); - assertTrue(slice.contains(vm.toString(address(hasOwner)).toSlice())); + assertTrue(reason.contains("`initialOwner` must not be a ProxyAdmin contract.")); + assertTrue(reason.contains(vm.toString(address(hasOwner)))); } } diff --git a/test/UpgradesUseDefenderDeploy.t.sol b/test/UpgradesUseDefenderDeploy.t.sol index 847076d..122c16a 100644 --- a/test/UpgradesUseDefenderDeploy.t.sol +++ b/test/UpgradesUseDefenderDeploy.t.sol @@ -5,19 +5,19 @@ import {Test} from "forge-std/Test.sol"; import {Upgrades, Options} from "openzeppelin-foundry-upgrades/Upgrades.sol"; +import {StringFinder} from "openzeppelin-foundry-upgrades/internal/StringFinder.sol"; + import {Greeter} from "./contracts/Greeter.sol"; import {GreeterProxiable} from "./contracts/GreeterProxiable.sol"; import {GreeterV2} from "./contracts/GreeterV2.sol"; import {GreeterV2Proxiable} from "./contracts/GreeterV2Proxiable.sol"; -import {strings} from "solidity-stringutils/src/strings.sol"; - /** * @dev Tests that the `defender.useDefenderDeploy` flag is recognized in the Upgrades library. * These do not perform any actual deployments, but just checks that the Defender CLI is invoked and catches its error message since we are using a dev network. */ contract UpgradesUseDefenderDeployTest is Test { - using strings for *; + using StringFinder for string; Deployer d; @@ -25,11 +25,10 @@ contract UpgradesUseDefenderDeployTest is Test { d = new Deployer(); } - function _assertDefenderNotAvailable(strings.slice memory slice) private pure { + function _assertDefenderNotAvailable(string memory str) private { assertTrue( - slice.contains( - "The current network with chainId 31337 is not supported by OpenZeppelin Defender".toSlice() - ) || slice.contains("DEFENDER_KEY and DEFENDER_SECRET must be set in environment variables".toSlice()) + str.contains("The current network with chainId 31337 is not supported by OpenZeppelin Defender") || + str.contains("DEFENDER_KEY and DEFENDER_SECRET must be set in environment variables") ); } @@ -46,9 +45,8 @@ contract UpgradesUseDefenderDeployTest is Test { { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("Failed to deploy contract GreeterProxiable.sol".toSlice())); - _assertDefenderNotAvailable(slice); + assertTrue(reason.contains("Failed to deploy contract GreeterProxiable.sol")); + _assertDefenderNotAvailable(reason); } } @@ -66,9 +64,8 @@ contract UpgradesUseDefenderDeployTest is Test { { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("Failed to deploy contract Greeter.sol".toSlice())); - _assertDefenderNotAvailable(slice); + assertTrue(reason.contains("Failed to deploy contract Greeter.sol")); + _assertDefenderNotAvailable(reason); } } @@ -86,9 +83,8 @@ contract UpgradesUseDefenderDeployTest is Test { { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("Failed to deploy contract GreeterV2Proxiable.sol".toSlice())); - _assertDefenderNotAvailable(slice); + assertTrue(reason.contains("Failed to deploy contract GreeterV2Proxiable.sol")); + _assertDefenderNotAvailable(reason); } } @@ -99,9 +95,8 @@ contract UpgradesUseDefenderDeployTest is Test { try d.deployBeacon("Greeter.sol", msg.sender, opts) { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("Failed to deploy contract Greeter.sol".toSlice())); - _assertDefenderNotAvailable(slice); + assertTrue(reason.contains("Failed to deploy contract Greeter.sol")); + _assertDefenderNotAvailable(reason); } } @@ -114,10 +109,9 @@ contract UpgradesUseDefenderDeployTest is Test { try d.deployBeaconProxy(beacon, abi.encodeCall(Greeter.initialize, (msg.sender, "hello")), opts) { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); // Note the below is not the implementation contract, because this function only deploys the BeaconProxy contract - assertTrue(slice.contains("Failed to deploy contract BeaconProxy.sol".toSlice())); - _assertDefenderNotAvailable(slice); + assertTrue(reason.contains("Failed to deploy contract BeaconProxy.sol")); + _assertDefenderNotAvailable(reason); } } @@ -130,9 +124,8 @@ contract UpgradesUseDefenderDeployTest is Test { try d.upgradeBeacon(beacon, "GreeterV2.sol", opts) { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("Failed to deploy contract GreeterV2.sol".toSlice())); - _assertDefenderNotAvailable(slice); + assertTrue(reason.contains("Failed to deploy contract GreeterV2.sol")); + _assertDefenderNotAvailable(reason); } } @@ -143,9 +136,8 @@ contract UpgradesUseDefenderDeployTest is Test { try d.prepareUpgrade("GreeterV2.sol", opts) { fail(); } catch Error(string memory reason) { - strings.slice memory slice = reason.toSlice(); - assertTrue(slice.contains("Failed to deploy contract GreeterV2.sol".toSlice())); - _assertDefenderNotAvailable(slice); + assertTrue(reason.contains("Failed to deploy contract GreeterV2.sol")); + _assertDefenderNotAvailable(reason); } } diff --git a/test/internal/Core.t.sol b/test/internal/Core.t.sol index 3f855f6..dc2af4a 100644 --- a/test/internal/Core.t.sol +++ b/test/internal/Core.t.sol @@ -42,7 +42,7 @@ contract CoreTest is Test { assertEq(Core.getUpgradeInterfaceVersion(address(u)), ""); } - function testBuildValidateCommand() public view { + function testBuildValidateCommand() public { Options memory opts; string memory commandString = StringHelper.join(Core.buildValidateCommand("Greeter.sol", opts, false)); @@ -56,7 +56,7 @@ contract CoreTest is Test { ); } - function testBuildValidateCommandExcludes() public view { + function testBuildValidateCommandExcludes() public { Options memory opts; opts.exclude = new string[](2); opts.exclude[0] = "test/contracts/**/{Foo,Bar}.sol"; diff --git a/test/internal/DefenderDeploy.t.sol b/test/internal/DefenderDeploy.t.sol index 56e8ea3..14b38a8 100644 --- a/test/internal/DefenderDeploy.t.sol +++ b/test/internal/DefenderDeploy.t.sol @@ -287,7 +287,7 @@ contract DefenderDeployTest is Test { ); } - function testBuildProposeUpgradeCommand() public view { + function testBuildProposeUpgradeCommand() public { ContractInfo memory contractInfo = Utils.getContractInfo("MyContractFile.sol:MyContractName", "out"); Options memory opts; @@ -313,7 +313,7 @@ contract DefenderDeployTest is Test { ); } - function testParseProposeUpgradeResponse() public pure { + function testParseProposeUpgradeResponse() public { string memory output = "Upgrade proposal created.\nProposal ID: 123\nProposal URL: https://my.url/my-tx"; ProposeUpgradeResponse memory response = DefenderDeploy.parseProposeUpgradeResponse(output); @@ -322,7 +322,7 @@ contract DefenderDeployTest is Test { assertEq(response.url, "https://my.url/my-tx"); } - function testParseProposeUpgradeResponseNoUrl() public pure { + function testParseProposeUpgradeResponseNoUrl() public { string memory output = "Upgrade proposal created.\nProposal ID: 123"; ProposeUpgradeResponse memory response = DefenderDeploy.parseProposeUpgradeResponse(output); @@ -346,7 +346,7 @@ contract DefenderDeployTest is Test { ); } - function testParseApprovalProcessResponse() public pure { + function testParseApprovalProcessResponse() public { string memory output = "Approval process ID: abc\nVia: 0x1230000000000000000000000000000000000456\nVia type: Relayer"; @@ -357,7 +357,7 @@ contract DefenderDeployTest is Test { assertEq(response.viaType, "Relayer"); } - function testParseApprovalProcessResponseIdOnly() public pure { + function testParseApprovalProcessResponseIdOnly() public { string memory output = "Approval process ID: abc"; ApprovalProcessResponse memory response = DefenderDeploy.parseApprovalProcessResponse(output); diff --git a/test/internal/StringFinder.t.sol b/test/internal/StringFinder.t.sol new file mode 100644 index 0000000..ccdf458 --- /dev/null +++ b/test/internal/StringFinder.t.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; + +import {StringFinder} from "openzeppelin-foundry-upgrades/internal/StringFinder.sol"; + +/** + * @dev Tests the StringFinder internal library. + */ +contract StringFinderTest is Test { + using StringFinder for string; + + function testContains() public { + string memory str = "hello world"; + assertTrue(str.contains("ello")); + assertFalse(str.contains("Ello")); + } + + function testStartsWith() public { + string memory str = "hello world"; + assertTrue(str.startsWith("hello")); + assertFalse(str.startsWith("ello")); + assertFalse(str.startsWith("Hello")); + } + + function testEndsWith() public { + string memory str = "hello world"; + assertTrue(str.endsWith("world")); + assertFalse(str.endsWith("worl")); + assertFalse(str.endsWith("World")); + } + + function testCount() public { + string memory str = "hello world"; + assertEq(str.count("l"), 3); + assertEq(str.count("ll"), 1); + assertEq(str.count("a"), 0); + } +} diff --git a/test/internal/Utils.t.sol b/test/internal/Utils.t.sol index 6ae8921..a726d26 100644 --- a/test/internal/Utils.t.sol +++ b/test/internal/Utils.t.sol @@ -2,17 +2,18 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; -import {strings} from "solidity-stringutils/src/strings.sol"; import {Utils, ContractInfo} from "openzeppelin-foundry-upgrades/internal/Utils.sol"; +import {StringFinder} from "openzeppelin-foundry-upgrades/internal/StringFinder.sol"; + import {MyContractName} from "../contracts/MyContractFile.sol"; /** * @dev Tests the Utils internal library. */ contract UtilsTest is Test { - function testGetContractInfo_from_file() public view { + function testGetContractInfo_from_file() public { ContractInfo memory info = Utils.getContractInfo("Greeter.sol", "out"); assertEq(info.shortName, "Greeter"); @@ -22,14 +23,14 @@ contract UtilsTest is Test { assertEq(info.sourceCodeHash, "0x9564e0245350d0eb5e42a8fed97d87518dbfbddf7668ed383f97a8558b2a9c39"); // source code hash of Greeter.sol } - function testGetContractInfo_from_fileAndName() public view { + function testGetContractInfo_from_fileAndName() public { ContractInfo memory info = Utils.getContractInfo("MyContractFile.sol:MyContractName", "out"); assertEq(info.shortName, "MyContractName"); assertEq(info.contractPath, "test/contracts/MyContractFile.sol"); } - function testGetContractInfo_from_artifact() public view { + function testGetContractInfo_from_artifact() public { ContractInfo memory info = Utils.getContractInfo("out/MyContractFile.sol/MyContractName.json", "out"); assertEq(info.shortName, "MyContractName"); @@ -48,7 +49,7 @@ contract UtilsTest is Test { } } - function testGetContractInfo_outDirTrailingSlash() public view { + function testGetContractInfo_outDirTrailingSlash() public { ContractInfo memory info = Utils.getContractInfo("Greeter.sol", "out/"); assertEq(info.shortName, "Greeter"); @@ -62,19 +63,19 @@ contract UtilsTest is Test { } catch {} } - function testGetFullyQualifiedName_from_file() public view { + function testGetFullyQualifiedName_from_file() public { string memory fqName = Utils.getFullyQualifiedName("Greeter.sol", "out"); assertEq(fqName, "test/contracts/Greeter.sol:Greeter"); } - function testGetFullyQualifiedName_from_fileAndName() public view { + function testGetFullyQualifiedName_from_fileAndName() public { string memory fqName = Utils.getFullyQualifiedName("MyContractFile.sol:MyContractName", "out"); assertEq(fqName, "test/contracts/MyContractFile.sol:MyContractName"); } - function testGetFullyQualifiedName_from_artifact() public view { + function testGetFullyQualifiedName_from_artifact() public { string memory fqName = Utils.getFullyQualifiedName("out/MyContractFile.sol/MyContractName.json", "out"); assertEq(fqName, "test/contracts/MyContractFile.sol:MyContractName"); @@ -103,7 +104,7 @@ contract UtilsTest is Test { assertEq(Utils.getOutDir(), "out"); } - using strings for *; + using StringFinder for string; function testGetBuildInfoFile() public { ContractInfo memory contractInfo = Utils.getContractInfo("Greeter.sol", "out"); @@ -113,8 +114,8 @@ contract UtilsTest is Test { "out" ); - assertTrue(buildInfoFile.toSlice().startsWith("out/build-info".toSlice())); - assertTrue(buildInfoFile.toSlice().endsWith(".json".toSlice())); + assertTrue(buildInfoFile.startsWith("out/build-info")); + assertTrue(buildInfoFile.endsWith(".json")); } function testToBashCommand() public pure { @@ -133,11 +134,11 @@ contract UtilsTest is Test { } contract Invoker { - function getContractInfo(string memory contractName, string memory outDir) public view { + function getContractInfo(string memory contractName, string memory outDir) public { Utils.getContractInfo(contractName, outDir); } - function getFullyQualifiedName(string memory contractName, string memory outDir) public view { + function getFullyQualifiedName(string memory contractName, string memory outDir) public { Utils.getFullyQualifiedName(contractName, outDir); } } From 6dbf3588e2868fc09db12bcb6d7afde3004ced80 Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Fri, 24 Jan 2025 16:35:40 -0500 Subject: [PATCH 02/10] Update lockfile --- yarn.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/yarn.lock b/yarn.lock index ed5b644..aafb3a0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -951,9 +951,9 @@ integrity sha512-m4iHazOsOCv1DgM7eD7GupTJ+NFVujRZt1wzddDPSVGpWdKq1SKkla5htKG7+IS4d2XOCtzkUNwRZ7Vq5aEUMA== "@openzeppelin/contracts-upgradeable@^5.0.2": - version "5.0.2" - resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-5.0.2.tgz#3e5321a2ecdd0b206064356798c21225b6ec7105" - integrity sha512-0MmkHSHiW2NRFiT9/r5Lu4eJq5UJ4/tzlOgYXNAIj/ONkQTVnz22pLxDvp4C4uZ9he7ZFvGn3Driptn1/iU7tQ== + version "5.2.0" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-5.2.0.tgz#caf9a6eaf4f16d7f90f9b45a6db4e7b125f4b13b" + integrity sha512-mZIu9oa4tQTlGiOJHk6D3LdJlqFqF6oNOSn6S6UVJtzfs9UsY9/dhMEbAVTwElxUtJnjpf6yA062+oBp+eOyPg== "@openzeppelin/contracts-v4@npm:@openzeppelin/contracts@^v4.9.6": version "4.9.6" @@ -961,9 +961,9 @@ integrity sha512-xSmezSupL+y9VkHZJGDoCBpmnB2ogM13ccaYDWqJTfS3dbuHkgjuwDFUmaFauBCboQMGB/S5UqUl2y54X99BmA== "@openzeppelin/contracts@^5.0.2": - version "5.0.2" - resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.2.tgz#b1d03075e49290d06570b2fd42154d76c2a5d210" - integrity sha512-ytPc6eLGcHHnapAZ9S+5qsdomhjo6QBHTDRRBFfTxXIpsicMhVPouPgmUPebZZZGX7vt9USA+Z+0M0dSVtSUEA== + version "5.2.0" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.2.0.tgz#bd020694218202b811b0ea3eec07277814c658da" + integrity sha512-bxjNie5z89W1Ea0NZLZluFh8PrFNn9DH8DQlujEok2yjsOlraUPKID5p1Wk3qdNbf6XkQ1Os2RvfiHrrXLHWKA== "@openzeppelin/defender-deploy-client-cli@0.0.1-alpha.10": version "0.0.1-alpha.10" From d2dbf4811008ca7054fc79ca82f340d01725d802 Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Fri, 24 Jan 2025 16:38:58 -0500 Subject: [PATCH 03/10] Simplify substring handling --- src/internal/DefenderDeploy.sol | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/internal/DefenderDeploy.sol b/src/internal/DefenderDeploy.sol index 281385b..e571808 100644 --- a/src/internal/DefenderDeploy.sol +++ b/src/internal/DefenderDeploy.sol @@ -232,10 +232,19 @@ library DefenderDeploy { if (vm.contains(stdout, expectedPrefix)) { // Get the substring after the prefix string[] memory segments = vm.split(stdout, expectedPrefix); - string memory suffix = ""; - for (uint256 i = 1; i < segments.length; i++) { - suffix = string(abi.encodePacked(suffix, segments[i])); + if (segments.length > 2) { + revert( + string( + abi.encodePacked( + "Found multiple occurrences of prefix '", + expectedPrefix, + "' in output: ", + stdout + ) + ) + ); } + string memory suffix = segments[1]; // Remove any following lines if (vm.contains(suffix, "\n")) { suffix = vm.split(suffix, "\n")[0]; From d2c841903b37d8b9621208523182231feb38b23b Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Fri, 24 Jan 2025 17:18:44 -0500 Subject: [PATCH 04/10] Remove unneeded contains --- src/internal/StringFinder.sol | 14 ++++---------- test/internal/StringFinder.t.sol | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/internal/StringFinder.sol b/src/internal/StringFinder.sol index c4f4573..5c4d9b2 100644 --- a/src/internal/StringFinder.sol +++ b/src/internal/StringFinder.sol @@ -29,23 +29,17 @@ library StringFinder { /** * Returns whether the subject string ends with the search string. */ - function endsWith(string memory subject, string memory search) internal returns (bool) { + function endsWith(string memory subject, string memory search) internal pure returns (bool) { Vm vm = Vm(Utils.CHEATCODE_ADDRESS); - if (!vm.contains(subject, search)) { - return false; - } string[] memory tokens = vm.split(subject, search); - return bytes(tokens[tokens.length - 1]).length == 0; + return tokens.length > 1 && bytes(tokens[tokens.length - 1]).length == 0; } /** - * Returns the number of occurrences of the search string in the subject string. + * Returns the number of non-overlapping occurrences of the search string in the subject string. */ - function count(string memory subject, string memory search) internal returns (uint256) { + function count(string memory subject, string memory search) internal pure returns (uint256) { Vm vm = Vm(Utils.CHEATCODE_ADDRESS); - if (!vm.contains(subject, search)) { - return 0; - } string[] memory tokens = vm.split(subject, search); return tokens.length - 1; } diff --git a/test/internal/StringFinder.t.sol b/test/internal/StringFinder.t.sol index ccdf458..e590b0d 100644 --- a/test/internal/StringFinder.t.sol +++ b/test/internal/StringFinder.t.sol @@ -22,6 +22,10 @@ contract StringFinderTest is Test { assertTrue(str.startsWith("hello")); assertFalse(str.startsWith("ello")); assertFalse(str.startsWith("Hello")); + assertTrue(str.startsWith("")); + + string memory empty = ""; + assertFalse(empty.startsWith("a")); } function testEndsWith() public { @@ -29,6 +33,10 @@ contract StringFinderTest is Test { assertTrue(str.endsWith("world")); assertFalse(str.endsWith("worl")); assertFalse(str.endsWith("World")); + assertTrue(str.endsWith("")); + + string memory empty = ""; + assertFalse(empty.endsWith("a")); } function testCount() public { @@ -36,5 +44,13 @@ contract StringFinderTest is Test { assertEq(str.count("l"), 3); assertEq(str.count("ll"), 1); assertEq(str.count("a"), 0); + assertEq(str.count(""), 12); + + string memory overlap = "aaa"; + assertEq(overlap.count("aa"), 1); // does not count overlapping occurrences + + string memory empty = ""; + assertEq(empty.count("a"), 0); + assertEq(empty.count(""), 1); } } From 6baa0e4d91c0b5a7f46705cf29ef1b06897a487f Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Fri, 24 Jan 2025 17:26:19 -0500 Subject: [PATCH 05/10] Revert mutability --- src/internal/Core.sol | 2 +- src/internal/Utils.sol | 14 ++++++++++---- test/internal/Core.t.sol | 4 ++-- test/internal/DefenderDeploy.t.sol | 2 +- test/internal/StringFinder.t.sol | 6 +++--- test/internal/Utils.t.sol | 18 +++++++++--------- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/internal/Core.sol b/src/internal/Core.sol index 8c49718..389329f 100644 --- a/src/internal/Core.sol +++ b/src/internal/Core.sol @@ -377,7 +377,7 @@ library Core { string memory contractName, Options memory opts, bool requireReference - ) internal returns (string[] memory) { + ) internal view returns (string[] memory) { string memory outDir = Utils.getOutDir(); string[] memory inputBuilder = new string[](2 ** 16); diff --git a/src/internal/Utils.sol b/src/internal/Utils.sol index 51a7cdc..e8aa531 100644 --- a/src/internal/Utils.sol +++ b/src/internal/Utils.sol @@ -42,7 +42,10 @@ library Utils { * @param outDir Foundry output directory to search in if contractName is not an artifact path * @return Fully qualified name of the contract, e.g. "src/MyContract.sol:MyContract" */ - function getFullyQualifiedName(string memory contractName, string memory outDir) internal returns (string memory) { + function getFullyQualifiedName( + string memory contractName, + string memory outDir + ) internal view returns (string memory) { ContractInfo memory info = getContractInfo(contractName, outDir); return string(abi.encodePacked(info.contractPath, ":", info.shortName)); } @@ -54,7 +57,10 @@ library Utils { * @param outDir Foundry output directory to search in if contractName is not an artifact path * @return ContractInfo struct containing information about the contract */ - function getContractInfo(string memory contractName, string memory outDir) internal returns (ContractInfo memory) { + function getContractInfo( + string memory contractName, + string memory outDir + ) internal view returns (ContractInfo memory) { Vm vm = Vm(CHEATCODE_ADDRESS); ContractInfo memory info; @@ -140,7 +146,7 @@ library Utils { return vm.envOr("FOUNDRY_OUT", defaultOutDir); } - function _toFileName(string memory name) private returns (string memory) { + function _toFileName(string memory name) private pure returns (string memory) { Vm vm = Vm(CHEATCODE_ADDRESS); if (name.endsWith(".sol")) { return name; @@ -166,7 +172,7 @@ library Utils { } } - function _toShortName(string memory name) private returns (string memory) { + function _toShortName(string memory name) private pure returns (string memory) { Vm vm = Vm(CHEATCODE_ADDRESS); if (name.endsWith(".sol") && name.count(".sol") == 1) { return vm.replace(name, ".sol", ""); diff --git a/test/internal/Core.t.sol b/test/internal/Core.t.sol index dc2af4a..3f855f6 100644 --- a/test/internal/Core.t.sol +++ b/test/internal/Core.t.sol @@ -42,7 +42,7 @@ contract CoreTest is Test { assertEq(Core.getUpgradeInterfaceVersion(address(u)), ""); } - function testBuildValidateCommand() public { + function testBuildValidateCommand() public view { Options memory opts; string memory commandString = StringHelper.join(Core.buildValidateCommand("Greeter.sol", opts, false)); @@ -56,7 +56,7 @@ contract CoreTest is Test { ); } - function testBuildValidateCommandExcludes() public { + function testBuildValidateCommandExcludes() public view { Options memory opts; opts.exclude = new string[](2); opts.exclude[0] = "test/contracts/**/{Foo,Bar}.sol"; diff --git a/test/internal/DefenderDeploy.t.sol b/test/internal/DefenderDeploy.t.sol index 14b38a8..e73a08f 100644 --- a/test/internal/DefenderDeploy.t.sol +++ b/test/internal/DefenderDeploy.t.sol @@ -287,7 +287,7 @@ contract DefenderDeployTest is Test { ); } - function testBuildProposeUpgradeCommand() public { + function testBuildProposeUpgradeCommand() public view { ContractInfo memory contractInfo = Utils.getContractInfo("MyContractFile.sol:MyContractName", "out"); Options memory opts; diff --git a/test/internal/StringFinder.t.sol b/test/internal/StringFinder.t.sol index e590b0d..44c67f6 100644 --- a/test/internal/StringFinder.t.sol +++ b/test/internal/StringFinder.t.sol @@ -17,7 +17,7 @@ contract StringFinderTest is Test { assertFalse(str.contains("Ello")); } - function testStartsWith() public { + function testStartsWith() public pure { string memory str = "hello world"; assertTrue(str.startsWith("hello")); assertFalse(str.startsWith("ello")); @@ -28,7 +28,7 @@ contract StringFinderTest is Test { assertFalse(empty.startsWith("a")); } - function testEndsWith() public { + function testEndsWith() public pure { string memory str = "hello world"; assertTrue(str.endsWith("world")); assertFalse(str.endsWith("worl")); @@ -39,7 +39,7 @@ contract StringFinderTest is Test { assertFalse(empty.endsWith("a")); } - function testCount() public { + function testCount() public pure { string memory str = "hello world"; assertEq(str.count("l"), 3); assertEq(str.count("ll"), 1); diff --git a/test/internal/Utils.t.sol b/test/internal/Utils.t.sol index a726d26..deb89fb 100644 --- a/test/internal/Utils.t.sol +++ b/test/internal/Utils.t.sol @@ -13,7 +13,7 @@ import {MyContractName} from "../contracts/MyContractFile.sol"; * @dev Tests the Utils internal library. */ contract UtilsTest is Test { - function testGetContractInfo_from_file() public { + function testGetContractInfo_from_file() public view { ContractInfo memory info = Utils.getContractInfo("Greeter.sol", "out"); assertEq(info.shortName, "Greeter"); @@ -23,14 +23,14 @@ contract UtilsTest is Test { assertEq(info.sourceCodeHash, "0x9564e0245350d0eb5e42a8fed97d87518dbfbddf7668ed383f97a8558b2a9c39"); // source code hash of Greeter.sol } - function testGetContractInfo_from_fileAndName() public { + function testGetContractInfo_from_fileAndName() public view { ContractInfo memory info = Utils.getContractInfo("MyContractFile.sol:MyContractName", "out"); assertEq(info.shortName, "MyContractName"); assertEq(info.contractPath, "test/contracts/MyContractFile.sol"); } - function testGetContractInfo_from_artifact() public { + function testGetContractInfo_from_artifact() public view { ContractInfo memory info = Utils.getContractInfo("out/MyContractFile.sol/MyContractName.json", "out"); assertEq(info.shortName, "MyContractName"); @@ -49,7 +49,7 @@ contract UtilsTest is Test { } } - function testGetContractInfo_outDirTrailingSlash() public { + function testGetContractInfo_outDirTrailingSlash() public view { ContractInfo memory info = Utils.getContractInfo("Greeter.sol", "out/"); assertEq(info.shortName, "Greeter"); @@ -63,19 +63,19 @@ contract UtilsTest is Test { } catch {} } - function testGetFullyQualifiedName_from_file() public { + function testGetFullyQualifiedName_from_file() public view { string memory fqName = Utils.getFullyQualifiedName("Greeter.sol", "out"); assertEq(fqName, "test/contracts/Greeter.sol:Greeter"); } - function testGetFullyQualifiedName_from_fileAndName() public { + function testGetFullyQualifiedName_from_fileAndName() public view { string memory fqName = Utils.getFullyQualifiedName("MyContractFile.sol:MyContractName", "out"); assertEq(fqName, "test/contracts/MyContractFile.sol:MyContractName"); } - function testGetFullyQualifiedName_from_artifact() public { + function testGetFullyQualifiedName_from_artifact() public view { string memory fqName = Utils.getFullyQualifiedName("out/MyContractFile.sol/MyContractName.json", "out"); assertEq(fqName, "test/contracts/MyContractFile.sol:MyContractName"); @@ -134,11 +134,11 @@ contract UtilsTest is Test { } contract Invoker { - function getContractInfo(string memory contractName, string memory outDir) public { + function getContractInfo(string memory contractName, string memory outDir) public view { Utils.getContractInfo(contractName, outDir); } - function getFullyQualifiedName(string memory contractName, string memory outDir) public { + function getFullyQualifiedName(string memory contractName, string memory outDir) public view { Utils.getFullyQualifiedName(contractName, outDir); } } From 13493cd8493387f92ac38179299985300972b4bf Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Mon, 27 Jan 2025 11:49:52 -0500 Subject: [PATCH 06/10] Apply review comment --- src/internal/DefenderDeploy.sol | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/internal/DefenderDeploy.sol b/src/internal/DefenderDeploy.sol index e571808..bc6bab1 100644 --- a/src/internal/DefenderDeploy.sol +++ b/src/internal/DefenderDeploy.sol @@ -245,11 +245,8 @@ library DefenderDeploy { ); } string memory suffix = segments[1]; - // Remove any following lines - if (vm.contains(suffix, "\n")) { - suffix = vm.split(suffix, "\n")[0]; - } - return suffix; + // Keep only the first line + return vm.split(suffix, "\n")[0]; } else if (required) { revert( string(abi.encodePacked("Failed to find line with prefix '", expectedPrefix, "' in output: ", stdout)) From c4e81091f23f80b731712fbc18a70482479259bf Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Mon, 27 Jan 2025 11:54:24 -0500 Subject: [PATCH 07/10] Update changelog and version --- CHANGELOG.md | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32771d5..f51ddbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## Unreleased +## 0.3.9 (2025-01-27) - Remove dependency on `solidity-stringutils`. ([#91](https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/pull/91)) diff --git a/package.json b/package.json index 4c47410..547ba92 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@openzeppelin/foundry-upgrades", - "version": "0.3.8", + "version": "0.3.9", "description": "Foundry library for deploying and managing upgradeable contracts", "license": "MIT", "files": [ From 3347ec922bb8c4bcd78644446256045c576c418e Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Mon, 27 Jan 2025 11:57:50 -0500 Subject: [PATCH 08/10] Update doc --- docs/modules/pages/foundry-upgrades.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/pages/foundry-upgrades.adoc b/docs/modules/pages/foundry-upgrades.adoc index 3a5343f..94fe4dc 100644 --- a/docs/modules/pages/foundry-upgrades.adoc +++ b/docs/modules/pages/foundry-upgrades.adoc @@ -77,7 +77,7 @@ openzeppelin-foundry-upgrades/=dependencies/openzeppelin-foundry-upgrades-0.3.6/ == Foundry Requirements -This library requires https://github.com/foundry-rs/forge-std[forge-std] version 1.8.0 or higher. +This library requires https://github.com/foundry-rs/forge-std[forge-std] version 1.9.5 or higher. == Before Running From 102b64947b68872ba75d470db0a7c7fd80580034 Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Mon, 27 Jan 2025 12:04:13 -0500 Subject: [PATCH 09/10] Bump version to indicate breaking changes --- CHANGELOG.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f51ddbb..811262a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,12 @@ # Changelog -## 0.3.9 (2025-01-27) +## 0.4.0 (2025-01-27) - Remove dependency on `solidity-stringutils`. ([#91](https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/pull/91)) +### Breaking changes +- Requires forge-std version v1.9.5 or later. + ## 0.3.8 (2025-01-24) - Fix error conditions when warnings occur in validation output. ([#94](https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/pull/94)) diff --git a/package.json b/package.json index 547ba92..dcbefc3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@openzeppelin/foundry-upgrades", - "version": "0.3.9", + "version": "0.4.0", "description": "Foundry library for deploying and managing upgradeable contracts", "license": "MIT", "files": [ From 2e02bf61886d9f6e66642e235fa0349fbbaac023 Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Mon, 27 Jan 2025 12:07:02 -0500 Subject: [PATCH 10/10] Changelog formatting --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 811262a..a634f45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ - Remove dependency on `solidity-stringutils`. ([#91](https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/pull/91)) ### Breaking changes -- Requires forge-std version v1.9.5 or later. +- Requires `forge-std` version v1.9.5 or higher. ## 0.3.8 (2025-01-24)