Merge pull request #5296 from ethereum-optimism/jm/reactivate-slither

ctb: Update slither config

.circleci/config.yml

@@ -333,12 +333,6 @@ jobs:
           command: |
             yarn lint:check || echo "export LINT_STATUS=1" >> "$BASH_ENV"
           working_directory: packages/contracts-bedrock
-      - run:
-          name: slither
-          command: |
-            slither --version
-            yarn slither || exit 0
-          working_directory: packages/contracts-bedrock
       - run:
           name: gas snapshot
           command: |
@@ -382,6 +376,26 @@ jobs:
               exit 1
             fi
 
+  contracts-bedrock-slither:
+    docker:
+      - image: ethereumoptimism/ci-builder:latest
+    resource_class: xlarge
+    steps:
+      - checkout
+      - attach_workspace: { at: "." }
+      - restore_cache:
+          name: Restore Yarn Package Cache
+          keys:
+            - yarn-packages-v2-{{ checksum "yarn.lock" }}
+      - check-changed:
+          patterns: contracts-bedrock,hardhat-deploy-config
+      - run:
+          name: slither
+          command: |
+            slither --version
+            yarn slither
+          working_directory: packages/contracts-bedrock
+
   contracts-bedrock-validate-spaces:
     docker:
       - image: ethereumoptimism/ci-builder:latest
@@ -995,6 +1009,9 @@ workflows:
       - contracts-bedrock-checks:
           requires:
             - yarn-monorepo
+      - contracts-bedrock-slither:
+          requires:
+            - yarn-monorepo
       - contracts-bedrock-validate-spaces:
           requires:
             - yarn-monorepo

packages/contracts-bedrock/package.json

@@ -32,6 +32,7 @@
     "storage-snapshot": "./scripts/storage-snapshot.sh",
     "validate-spacers": "hardhat compile && hardhat validate-spacers",
     "slither": "./scripts/slither.sh",
+    "slither:triage": "TRIAGE_MODE=1 ./scripts/slither.sh",
     "clean": "rm -rf ./dist ./artifacts ./forge-artifacts ./cache ./tsconfig.tsbuildinfo ./tsconfig.build.tsbuildinfo ./src/contract-artifacts.ts ./test-case-generator/fuzz",
     "lint:ts:check": "eslint . --max-warnings=0",
     "lint:forge-tests:check": "ts-node scripts/forge-test-names.ts",

packages/contracts-bedrock/scripts/slither.sh

@@ -2,10 +2,24 @@
 
 rm -rf artifacts forge-artifacts
 
-# Handle slither bug unable to work with the foundry tests
-TEMP=$(mktemp -d)
-mv contracts/test $TEMP/test
+# See slither.config.json for slither settings
+if [ -n "$TRIAGE_MODE" ]; then
+  # Slither's triage mode will run an 'interview' in the terminal, allowing you to review each of
+  # its findings, and specify which should be ignored in future runs of slither. This will update
+  # (or create) the slither.db.json file. This DB is a cleaner alternative to adding slither-disable
+  # comments throughout the codebase.
+  # Triage mode should only be run manually, and can be used to update the db when new findings are
+  # causing a CI failure.
+  slither . --triage-mode
 
-slither . --foundry-out-directory artifacts
-
-mv $TEMP/test contracts/test
+  # For whatever reason the slither db contains a filename_absolute property which includes the full
+  # local path to source code on the machine where it was generated. This property does not
+  # seem to be required for slither to run, so we remove it.
+  DB=slither.db.json
+  TEMP_DB=temp-slither.db.json
+  mv $DB $TEMP_DB
+  jq 'walk(if type == "object" then del(.filename_absolute) else . end)' $TEMP_DB > $DB
+  rm -f $TEMP_DB
+else
+  slither .
+fi

packages/contracts-bedrock/slither.config.json

@@ -1,12 +1,13 @@
 {
   "detectors_to_exclude": "assembly-usage,block-timestamp,naming-convention,solc-version",
-  "exclude_informational": false,
-  "exclude_low": false,
-  "exclude_medium": false,
+  "exclude_informational": true,
+  "exclude_low": true,
+  "exclude_medium": true,
   "exclude_high": false,
   "solc_disable_warnings": false,
   "hardhat_ignore_compile": false,
   "disable_color": false,
-  "exclude_dependencies": false,
-  "filter_paths": "contracts/test|lib"
+  "exclude_dependencies": true,
+  "filter_paths": "contracts/test,contracts/vendor,contracts/echidna,node_modules",
+  "foundry_out_directory": "artifacts"
 }