fix: update forge-std to 1.8.1, remove ds-test, use solhint for…

… linting (#2545)

* chore: forge-std  v1.8.1, remove ds-test dep

* chore: remove ds-test from remappings

* refactor: state mutability

* chore: add solhint

* chore: yarn

* fix: unused imports

* fix: max line length

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"
+  }
+}

packages/solidity-devops/package.json

@@ -22,21 +22,23 @@
     "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": {
     "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",
     "fsr-str": "js/forgeScriptRunString.js",
     "fvc": "js/forgeVerifyContract.js",
     "sd": "js/saveDeployment.js",
     "vp": "js/verifyProxy.js"
+  },
+  "devDependencies": {
+    "solhint": "^4.5.4"
   }
 }

packages/solidity-devops/remappings.txt

@@ -1,2 +1 @@
-forge-std/=node_modules/forge-std/src/
-ds-test/=node_modules/ds-test/src/
+forge-std/=node_modules/forge-std/src/

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";
 

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";
 

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()

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;
     }
 

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)

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;
@@ -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,

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);

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,

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";
 
@@ -15,15 +15,15 @@ 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);
     }
 
     // ════════════════════════════════════════════════ 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,23 +163,23 @@ 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 {
             assertNotEq(a, b);
         }
     }
 
-    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];
         }
         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");

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"