Add Slither reentrancy check in CI (#3047)

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>

.github/workflows/test.yml

@@ -29,7 +29,7 @@ jobs:
         env:
           FORCE_COLOR: 1
           ENABLE_GAS_REPORT: true
-      - run: npm run test:inheritance 
+      - run: npm run test:inheritance
       - name: Print gas report
         run: cat gas-report.txt
 
@@ -54,3 +54,26 @@ jobs:
         env:
           NODE_OPTIONS: --max_old_space_size=4096
       - uses: codecov/codecov-action@v2
+
+  slither:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-node@v2
+        with:
+          node-version: 12.x
+      - uses: actions/cache@v2
+        id: cache
+        with:
+          path: '**/node_modules'
+          key: npm-v2-${{ hashFiles('**/package-lock.json') }}
+          restore-keys: npm-v2-
+      - run: npm ci
+        if: steps.cache.outputs.cache-hit != 'true'
+      - name: Set up Python
+        uses: actions/setup-python@v2
+
+      - name: Install dependencies
+        run: pip3 install slither-analyzer
+      - name: Summary of static analysis
+        run: npm run slither

contracts/governance/TimelockController.sol

@@ -261,6 +261,9 @@ contract TimelockController is AccessControl {
      *
      * - the caller must have the 'executor' role.
      */
+    // This function can reenter, but it doesn't pose a risk because _afterCall checks that the proposal is pending,
+    // thus any modifications to the operation during reentrancy should be caught.
+    // slither-disable-next-line reentrancy-eth
     function execute(
         address target,
         uint256 value,

contracts/governance/extensions/GovernorTimelockControl.sol

@@ -122,6 +122,9 @@ abstract contract GovernorTimelockControl is IGovernorTimelock, Governor {
      * @dev Overriden version of the {Governor-_cancel} function to cancel the timelocked proposal if it as already
      * been queued.
      */
+    // This function can reenter through the external call to the timelock, but we assume the timelock is trusted and
+    // well behaved (according to TimelockController) and this will not happen.
+    // slither-disable-next-line reentrancy-no-eth
     function _cancel(
         address[] memory targets,
         uint256[] memory values,

contracts/token/ERC20/extensions/ERC20FlashMint.sol

@@ -56,6 +56,9 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender {
      * @param data An arbitrary datafield that is passed to the receiver.
      * @return `true` is the flash loan was successful.
      */
+    // This function can reenter, but it doesn't pose a risk because it always preserves the property that the amount
+    // minted at the beginning is always recovered and burned at the end, or else the entire function will revert.
+    // slither-disable-next-line reentrancy-no-eth
     function flashLoan(
         IERC3156FlashBorrower receiver,
         address token,

package.json

@@ -29,7 +29,8 @@
     "version": "scripts/release/version.sh",
     "test": "hardhat test",
     "test:inheritance": "node scripts/inheritanceOrdering artifacts/build-info/*",
-    "gas-report": "env ENABLE_GAS_REPORT=true npm run test"
+    "gas-report": "env ENABLE_GAS_REPORT=true npm run test",
+    "slither": "npm run clean && slither . --detect reentrancy-eth,reentrancy-no-eth,reentrancy-unlimited-gas"
   },
   "repository": {
     "type": "git",