From 5162fc01a0d52dbc3054b09eedb0a9f70b629437 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 1 May 2024 12:53:03 +0100 Subject: [PATCH 1/7] chore: forge-std v1.8.1, remove ds-test dep --- packages/solidity-devops/package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/solidity-devops/package.json b/packages/solidity-devops/package.json index dced824a53..8fda9001aa 100644 --- a/packages/solidity-devops/package.json +++ b/packages/solidity-devops/package.json @@ -29,8 +29,7 @@ "dependencies": { "chalk": "^4.1.2", "dotenv": "^16.4.5", - "ds-test": "github:dapphub/ds-test#e282159", - "forge-std": "github:foundry-rs/forge-std#v1.7.6" + "forge-std": "github:foundry-rs/forge-std#v1.8.1" }, "bin": { "fsr": "js/forgeScriptRun.js", From 2fc5649809ce2ccb3e8341940f725f484627e3ef Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 1 May 2024 13:15:17 +0100 Subject: [PATCH 2/7] chore: remove ds-test from remappings --- packages/solidity-devops/remappings.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/solidity-devops/remappings.txt b/packages/solidity-devops/remappings.txt index 4c87d16572..8b405e72ec 100644 --- a/packages/solidity-devops/remappings.txt +++ b/packages/solidity-devops/remappings.txt @@ -1,2 +1 @@ -forge-std/=node_modules/forge-std/src/ -ds-test/=node_modules/ds-test/src/ \ No newline at end of file +forge-std/=node_modules/forge-std/src/ \ No newline at end of file From 1b54e03c33db76657630b4794737f35da4d9f62f Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 1 May 2024 13:20:20 +0100 Subject: [PATCH 3/7] refactor: state mutability --- .../src/base/ChainAwareness.sol | 2 ++ .../solidity-devops/src/base/PathFinder.sol | 2 +- .../src/base/SynapseBaseScript.sol | 3 ++- .../test/libs/StringUtils.t.sol | 26 +++++++++---------- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/solidity-devops/src/base/ChainAwareness.sol b/packages/solidity-devops/src/base/ChainAwareness.sol index e6571356f1..407dd29502 100644 --- a/packages/solidity-devops/src/base/ChainAwareness.sol +++ b/packages/solidity-devops/src/base/ChainAwareness.sol @@ -111,6 +111,8 @@ abstract contract ChainAwareness is PathFinder { /// @notice Wrapper for block.chainid, which only exists in Solidity 0.8+ function blockChainId() internal view returns (uint256 chainId) { + // silence the state mutability warning + this; // solhint-disable-next-line no-inline-assembly assembly { chainId := chainid() diff --git a/packages/solidity-devops/src/base/PathFinder.sol b/packages/solidity-devops/src/base/PathFinder.sol index fbf065f426..62a7c5aabb 100644 --- a/packages/solidity-devops/src/base/PathFinder.sol +++ b/packages/solidity-devops/src/base/PathFinder.sol @@ -32,7 +32,7 @@ abstract contract PathFinder is CommonBase { // ═══════════════════════════════════════════════ PATH GETTERS ════════════════════════════════════════════════════ /// @dev Path to the devops.json file. Could be overridden if the location of the file is different. - function getDevopsConfigPath() internal view virtual returns (string memory) { + function getDevopsConfigPath() internal pure virtual returns (string memory) { return DEFAULT_DEVOPS_CONFIG; } diff --git a/packages/solidity-devops/src/base/SynapseBaseScript.sol b/packages/solidity-devops/src/base/SynapseBaseScript.sol index 97a5e418a1..d9820aa235 100644 --- a/packages/solidity-devops/src/base/SynapseBaseScript.sol +++ b/packages/solidity-devops/src/base/SynapseBaseScript.sol @@ -10,6 +10,7 @@ import {Script} from "forge-std/Script.sol"; abstract contract SynapseBaseScript is Script, Deployer, DeploymentSaver { uint256 private initialDeployerNonce; + uint256 private finalDeployerNonce; /// @notice Common pattern for running a script. modifier broadcastWithHooks() { @@ -42,7 +43,7 @@ abstract contract SynapseBaseScript is Script, Deployer, DeploymentSaver { /// Make sure to call `super.afterExecution()` in the overridden function. function afterExecution() internal virtual { // Check if the deployer address has the same nonce as before the script was executed - uint256 finalDeployerNonce = vm.getNonce(msg.sender); + finalDeployerNonce = vm.getNonce(msg.sender); printInfo( StringUtils.concat( "Total amount of broadcasted txs: ", vm.toString(finalDeployerNonce - initialDeployerNonce) diff --git a/packages/solidity-devops/test/libs/StringUtils.t.sol b/packages/solidity-devops/test/libs/StringUtils.t.sol index 940c8eefe8..a0176e8fd6 100644 --- a/packages/solidity-devops/test/libs/StringUtils.t.sol +++ b/packages/solidity-devops/test/libs/StringUtils.t.sol @@ -15,7 +15,7 @@ contract StringUtilsTest is Test { libHarness = new StringUtilsHarness(); } - function testLength() public { + function testLength() public view { assertEq(libHarness.length(""), 0); assertEq(libHarness.length("0"), 1); assertEq(libHarness.length("0123456789"), 10); @@ -23,7 +23,7 @@ contract StringUtilsTest is Test { // ════════════════════════════════════════════════ SLICING ══════════════════════════════════════════════════════ - function testSubstring() public { + function testSubstring() public view { string memory str = "0123456789"; // Empty string assertEq(libHarness.substring(str, 0, 0), ""); @@ -59,7 +59,7 @@ contract StringUtilsTest is Test { libHarness.substring(str, 5, 4); } - function testSuffix() public { + function testSuffix() public view { string memory str = "0123456789"; assertEq(libHarness.suffix(str, 10), ""); assertEq(libHarness.suffix(str, 9), "9"); @@ -73,7 +73,7 @@ contract StringUtilsTest is Test { libHarness.suffix(str, 11); } - function testPrefix() public { + function testPrefix() public view { string memory str = "0123456789"; assertEq(libHarness.prefix(str, 0), ""); assertEq(libHarness.prefix(str, 1), "0"); @@ -163,7 +163,7 @@ contract StringUtilsTest is Test { // ════════════════════════════════════════════════ COMPARISON ═════════════════════════════════════════════════════ - function testEquals(string memory a, string memory b) public { + function testEquals(string memory a, string memory b) public view { if (libHarness.equals(a, b)) { assertEq(a, b); } else { @@ -171,7 +171,7 @@ contract StringUtilsTest is Test { } } - function testEqualsToClone(string memory a) public { + function testEqualsToClone(string memory a) public view { bytes memory clone = new bytes(bytes(a).length); for (uint256 i = 0; i < bytes(a).length; i++) { clone[i] = bytes(a)[i]; @@ -179,7 +179,7 @@ contract StringUtilsTest is Test { assertTrue(libHarness.equals(a, string(clone))); } - function testIndexOf() public { + function testIndexOf() public view { string memory str = "01201234012345"; assertEq(libHarness.indexOf("", ""), 0); assertEq(libHarness.indexOf("", "0"), type(uint256).max); @@ -201,7 +201,7 @@ contract StringUtilsTest is Test { assertEq(libHarness.indexOf(str, "01230"), type(uint256).max); } - function testLastIndexOf() public { + function testLastIndexOf() public view { string memory str = "01201234012345"; assertEq(libHarness.lastIndexOf("", ""), 0); assertEq(libHarness.lastIndexOf("", "0"), type(uint256).max); @@ -225,7 +225,7 @@ contract StringUtilsTest is Test { // ════════════════════════════════════════════ INTEGER CONVERSION ═════════════════════════════════════════════════ - function testToUint() public { + function testToUint() public view { assertEq(libHarness.toUint("0"), 0); assertEq(libHarness.toUint("1"), 1); assertEq(libHarness.toUint("42"), 42); @@ -241,7 +241,7 @@ contract StringUtilsTest is Test { libHarness.toUint("0a"); } - function testFromUint() public { + function testFromUint() public view { assertEq(libHarness.fromUint(0), "0"); assertEq(libHarness.fromUint(1), "1"); assertEq(libHarness.fromUint(42), "42"); @@ -252,13 +252,13 @@ contract StringUtilsTest is Test { ); } - function testUintRoundtrip(uint256 val) public { + function testUintRoundtrip(uint256 val) public view { assertEq(libHarness.toUint(libHarness.fromUint(val)), val); } // ═════════════════════════════════════════════ FLOAT CONVERSION ══════════════════════════════════════════════════ - function testFromFloat() public { + function testFromFloat() public view { // Zero decimals assertEq(libHarness.fromFloat(0, 0), "0.0"); assertEq(libHarness.fromFloat(1, 0), "1.0"); @@ -288,7 +288,7 @@ contract StringUtilsTest is Test { ); } - function testFromWei() public { + function testFromWei() public view { assertEq(libHarness.fromWei(0), "0.000000000000000000"); assertEq(libHarness.fromWei(1), "0.000000000000000001"); assertEq(libHarness.fromWei(42), "0.000000000000000042"); From 6b8a7c309a4f7f9f19e24765e6a493c505e715db Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 1 May 2024 13:33:28 +0100 Subject: [PATCH 4/7] chore: add solhint --- packages/solidity-devops/.solhint.json | 18 ++++++++++++++++++ packages/solidity-devops/package.json | 7 +++++-- 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 packages/solidity-devops/.solhint.json diff --git a/packages/solidity-devops/.solhint.json b/packages/solidity-devops/.solhint.json new file mode 100644 index 0000000000..a4de2755de --- /dev/null +++ b/packages/solidity-devops/.solhint.json @@ -0,0 +1,18 @@ +{ + "extends": "solhint:recommended", + "rules": { + "compiler-version": "off", + "code-complexity": ["warn", 5], + "func-param-name-mixedcase": "error", + "func-visibility": ["warn", { "ignoreConstructors": true }], + "gas-custom-errors": "off", + "max-line-length": ["warn", 120], + "modifier-name-mixedcase": "error", + "no-complex-fallback": "off", + "no-console": "off", + "not-rely-on-block-hash": "off", + "not-rely-on-time": "off", + "reason-string": "off", + "reentrancy": "off" + } +} diff --git a/packages/solidity-devops/package.json b/packages/solidity-devops/package.json index 8fda9001aa..c724c9d8ed 100644 --- a/packages/solidity-devops/package.json +++ b/packages/solidity-devops/package.json @@ -22,8 +22,8 @@ "build:go": " ", "build:slither": " ", "ci:lint": "npm run lint:check", - "lint": "forge fmt && eslint . --fix --ext js", - "lint:check": "forge fmt --check && eslint . --max-warnings=0 --ext js", + "lint": "forge fmt && eslint . --fix --ext js && solhint --fix -c .solhint.json '{src,script,test}/**/*.sol'", + "lint:check": "forge fmt --check && eslint . --max-warnings=0 --ext js && solhint -c .solhint.json '{src,script,test}/**/*.sol'", "test:coverage": "echo 'TODO: Add test coverage'" }, "dependencies": { @@ -37,5 +37,8 @@ "fvc": "js/forgeVerifyContract.js", "sd": "js/saveDeployment.js", "vp": "js/verifyProxy.js" + }, + "devDependencies": { + "solhint": "^4.5.4" } } From 77ed06659b10b3f678df6fadc7f13cb5f937e20c Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 1 May 2024 13:14:25 +0100 Subject: [PATCH 5/7] chore: yarn --- yarn.lock | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/yarn.lock b/yarn.lock index 15568e8a75..376bb95a2b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6616,6 +6616,11 @@ resolved "https://registry.yarnpkg.com/@solidity-parser/parser/-/parser-0.17.0.tgz#52a2fcc97ff609f72011014e4c5b485ec52243ef" integrity sha512-Nko8R0/kUo391jsEHHxrGM07QFdnPGvlmox4rmH0kNiNAashItAilhy4Mv4pK5gQmW5f4sXAF58fwJbmlkGcVw== +"@solidity-parser/parser@^0.18.0": + version "0.18.0" + resolved "https://registry.yarnpkg.com/@solidity-parser/parser/-/parser-0.18.0.tgz#8e77a02a09ecce957255a2f48c9a7178ec191908" + integrity sha512-yfORGUIPgLck41qyN7nbwJRAx17/jAIXCTanHOJZhB6PJ1iAk/84b/xlsVKFSyNyLXIj0dhppoE0+CRws7wlzA== + "@stablelib/aead@^1.0.1": version "1.0.1" resolved "https://registry.yarnpkg.com/@stablelib/aead/-/aead-1.0.1.tgz#c4b1106df9c23d1b867eb9b276d8f42d5fc4c0c3" @@ -11426,6 +11431,11 @@ antlr4@^4.11.0: resolved "https://registry.yarnpkg.com/antlr4/-/antlr4-4.13.1.tgz#1e0a1830a08faeb86217cb2e6c34716004e4253d" integrity sha512-kiXTspaRYvnIArgE97z5YVVf/cDVQABr3abFRR6mE7yesLMkgu4ujuyV/sgxafQ8wgve0DJQUJ38Z8tkgA2izA== +antlr4@^4.13.1-patch-1: + version "4.13.1-patch-1" + resolved "https://registry.yarnpkg.com/antlr4/-/antlr4-4.13.1-patch-1.tgz#946176f863f890964a050c4f18c47fd6f7e57602" + integrity sha512-OjFLWWLzDMV9rdFhpvroCWR4ooktNg9/nvVYSA5z28wuVpU36QUNuioR1XLnQtcjVlf8npjyz593PxnU/f/Cow== + antlr4ts@^0.5.0-alpha.4: version "0.5.0-alpha.4" resolved "https://registry.yarnpkg.com/antlr4ts/-/antlr4ts-0.5.0-alpha.4.tgz#71702865a87478ed0b40c0709f422cf14d51652a" @@ -15776,10 +15786,6 @@ download@^7.1.0: p-event "^2.1.0" pify "^3.0.0" -"ds-test@github:dapphub/ds-test#e282159": - version "1.0.0" - resolved "https://codeload.github.com/dapphub/ds-test/tar.gz/e282159d5170298eb2455a6c05280ab5a73a4ef0" - dset@^3.1.1, dset@^3.1.2: version "3.1.3" resolved "https://registry.yarnpkg.com/dset/-/dset-3.1.3.tgz#c194147f159841148e8e34ca41f638556d9542d2" @@ -18254,9 +18260,9 @@ forever-agent@~0.6.1: resolved "https://registry.yarnpkg.com/forever-agent/-/forever-agent-0.6.1.tgz#fbc71f0c41adeb37f96c577ad1ed42d8fdacca91" integrity sha512-j0KLYPhm6zeac4lz3oJ3o65qvgQCcPubiyotZrXqEaG4hNagNYO8qdlUrX5vwqv9ohqeT/Z3j6+yW067yWWdUw== -"forge-std@github:foundry-rs/forge-std#v1.7.6": +"forge-std@github:foundry-rs/forge-std#v1.8.1": version "1.7.6" - resolved "https://codeload.github.com/foundry-rs/forge-std/tar.gz/ae570fec082bfe1c1f45b0acca4a2b4f84d345ce" + resolved "https://codeload.github.com/foundry-rs/forge-std/tar.gz/bb4ceea94d6f10eeb5b41dc2391c6c8bf8e734ef" fork-ts-checker-webpack-plugin@^4.1.6: version "4.1.6" @@ -29422,6 +29428,32 @@ solhint@^4.1.1: optionalDependencies: prettier "^2.8.3" +solhint@^4.5.4: + version "4.5.4" + resolved "https://registry.yarnpkg.com/solhint/-/solhint-4.5.4.tgz#171cf33f46c36b8499efe60c0e425f6883a54e50" + integrity sha512-Cu1XiJXub2q1eCr9kkJ9VPv1sGcmj3V7Zb76B0CoezDOB9bu3DxKIFFH7ggCl9fWpEPD6xBmRLfZrYijkVmujQ== + dependencies: + "@solidity-parser/parser" "^0.18.0" + ajv "^6.12.6" + antlr4 "^4.13.1-patch-1" + ast-parents "^0.0.1" + chalk "^4.1.2" + commander "^10.0.0" + cosmiconfig "^8.0.0" + fast-diff "^1.2.0" + glob "^8.0.3" + ignore "^5.2.4" + js-yaml "^4.1.0" + latest-version "^7.0.0" + lodash "^4.17.21" + pluralize "^8.0.0" + semver "^7.5.2" + strip-ansi "^6.0.1" + table "^6.8.1" + text-table "^0.2.0" + optionalDependencies: + prettier "^2.8.3" + solidity-comments-extractor@^0.0.8: version "0.0.8" resolved "https://registry.yarnpkg.com/solidity-comments-extractor/-/solidity-comments-extractor-0.0.8.tgz#f6e148ab0c49f30c1abcbecb8b8df01ed8e879f8" From d453550bc12f54b9a1523d2a4d7dc3d8e3ad6bba Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 1 May 2024 13:38:30 +0100 Subject: [PATCH 6/7] fix: unused imports --- packages/solidity-devops/src/SynapseScript.sol | 1 + packages/solidity-devops/src/SynapseScript06.sol | 1 + packages/solidity-devops/src/deploy/DeploymentSaver.sol | 2 +- packages/solidity-devops/test/libs/StringUtils.t.sol | 2 +- 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/solidity-devops/src/SynapseScript.sol b/packages/solidity-devops/src/SynapseScript.sol index ce12bea633..705410ace8 100644 --- a/packages/solidity-devops/src/SynapseScript.sol +++ b/packages/solidity-devops/src/SynapseScript.sol @@ -5,6 +5,7 @@ pragma experimental ABIEncoderV2; import {SynapseBaseScript} from "./base/SynapseBaseScript.sol"; // Imports for external consumption +// solhint-disable no-unused-import import {StringUtils} from "./libs/StringUtils.sol"; import {stdJson} from "forge-std/Script.sol"; diff --git a/packages/solidity-devops/src/SynapseScript06.sol b/packages/solidity-devops/src/SynapseScript06.sol index 95c0d092f8..da68fd3c68 100644 --- a/packages/solidity-devops/src/SynapseScript06.sol +++ b/packages/solidity-devops/src/SynapseScript06.sol @@ -5,6 +5,7 @@ pragma experimental ABIEncoderV2; import {SynapseBaseScript} from "./base/SynapseBaseScript.sol"; // Imports for external consumption +// solhint-disable no-unused-import import {StringUtils} from "./libs/StringUtils.sol"; import {stdJson} from "forge-std/Script.sol"; diff --git a/packages/solidity-devops/src/deploy/DeploymentSaver.sol b/packages/solidity-devops/src/deploy/DeploymentSaver.sol index 1394bf86c9..b263f0710b 100644 --- a/packages/solidity-devops/src/deploy/DeploymentSaver.sol +++ b/packages/solidity-devops/src/deploy/DeploymentSaver.sol @@ -6,7 +6,7 @@ import {StringUtils} from "../libs/StringUtils.sol"; import {ChainAwareReader} from "../reader/ChainAwareReader.sol"; import {ChainAwareWriter} from "../writer/ChainAwareWriter.sol"; -import {console2, stdJson} from "forge-std/Script.sol"; +import {stdJson} from "forge-std/Script.sol"; abstract contract DeploymentSaver is ChainAwareReader, ChainAwareWriter { using stdJson for string; diff --git a/packages/solidity-devops/test/libs/StringUtils.t.sol b/packages/solidity-devops/test/libs/StringUtils.t.sol index a0176e8fd6..a808839d34 100644 --- a/packages/solidity-devops/test/libs/StringUtils.t.sol +++ b/packages/solidity-devops/test/libs/StringUtils.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.23; -import {StringUtilsHarness, StringUtils} from "../harnesses/StringUtilsHarness.sol"; +import {StringUtilsHarness} from "../harnesses/StringUtilsHarness.sol"; import {Test} from "forge-std/Test.sol"; From a6e5658fb0d81486ace6e76c7ee6020fbe3850dc Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 1 May 2024 13:38:46 +0100 Subject: [PATCH 7/7] fix: max line length --- packages/solidity-devops/src/deploy/DeploymentSaver.sol | 3 ++- packages/solidity-devops/src/writer/ChainAwareWriter.sol | 3 ++- packages/solidity-devops/src/writer/DataWriter.sol | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/solidity-devops/src/deploy/DeploymentSaver.sol b/packages/solidity-devops/src/deploy/DeploymentSaver.sol index b263f0710b..9dbc5b5eca 100644 --- a/packages/solidity-devops/src/deploy/DeploymentSaver.sol +++ b/packages/solidity-devops/src/deploy/DeploymentSaver.sol @@ -136,7 +136,8 @@ abstract contract DeploymentSaver is ChainAwareReader, ChainAwareWriter { /// @notice Saves the deployment JSON for a contract on a given chain under the specified alias. /// Example: contractName = "LinkedPool", contractAlias = "LinkedPool.USDC" - /// Note: writes to the FRESH deployment path, which is moved to the correct location after the contract is deployed. + /// Note: writes the JSON file to the FRESH deployments directory. The written file needs to be moved + /// to the correct location outside of the deployment script. /// Note: will not include the ABI in the output JSON. function saveDeployment( string memory contractAlias, diff --git a/packages/solidity-devops/src/writer/ChainAwareWriter.sol b/packages/solidity-devops/src/writer/ChainAwareWriter.sol index dda122a7e3..6182dc4c59 100644 --- a/packages/solidity-devops/src/writer/ChainAwareWriter.sol +++ b/packages/solidity-devops/src/writer/ChainAwareWriter.sol @@ -11,7 +11,8 @@ abstract contract ChainAwareWriter is ChainAwareness, DataWriter { /// @notice Writes the deployment JSON for a contract on the active chain under the specified alias. /// Example: contractName = "LinkedPool", contractAlias = "LinkedPool.USDC" - /// Note: writes to the FRESH deployment path, which is moved to the correct location after the contract is deployed. + /// Note: writes the JSON file to the FRESH deployments directory. The written file needs to be moved + /// to the correct location outside of the deployment script. /// Note: will not include the ABI in the output JSON. function writeDeploymentArtifact(string memory contractAlias, string memory artifact) internal { writeDeploymentArtifact(activeChain, contractAlias, artifact); diff --git a/packages/solidity-devops/src/writer/DataWriter.sol b/packages/solidity-devops/src/writer/DataWriter.sol index ba53fc5daa..a987ea8f61 100644 --- a/packages/solidity-devops/src/writer/DataWriter.sol +++ b/packages/solidity-devops/src/writer/DataWriter.sol @@ -21,7 +21,8 @@ abstract contract DataWriter is PathFinder, Logger { /// @notice Writes the deployment JSON for a contract on a given chain under the specified alias. /// Example: contractName = "LinkedPool", contractAlias = "LinkedPool.USDC" - /// Note: writes to the FRESH deployment path, which is moved to the correct location after the contract is deployed. + /// Note: writes the JSON file to the FRESH deployments directory. The written file needs to be moved + /// to the correct location outside of the deployment script. /// Note: will not include the ABI in the output JSON. function writeDeploymentArtifact( string memory chain,