feat: slither added

.github/workflows/forge.yaml

@@ -1,10 +1,13 @@
-on: [push]
+on:
+  push:
+    paths:
+    - 'src/**/*.sol'
 
 env:
   FOUNDRY_PROFILE: ci
   RPC_MAINNET: ${{ secrets.RPC_MAINNET }}
 
-name: test
+name: Foundry test
 
 jobs:
   check:

.github/workflows/slither.yaml

@@ -0,0 +1,33 @@
+name: Slither analysis
+
+# Controls when the action will run. 
+on:
+  push:
+    paths:
+    - 'src/**/*.sol'
+
+# ensure multiple CI processes are not running analysis on contracts
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event_name == 'pull_request_target' && github.head_ref || github.ref }}
+  cancel-in-progress: true
+
+env:
+  COMMIT_SHA: ${{ github.event.pull_request.head.sha }}
+  PULL_NUMBER: ${{ github.event.pull_request.number }}
+  RUN_ID: ${{ github.run_id }}
+
+jobs:
+  slither:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+        with:
+          submodules: recursive
+
+      - name: Run Slither
+        uses: crytic/[email protected]
+        id: slither
+        with:
+          fail-on: all
+          slither-args: --checklist

README.md

@@ -35,6 +35,13 @@ Fuzz test all methods on `MevEthRouter` produce better results than Deposit / Re
 forge test -vvv
 ```
 
+## Slither audit
+
+```bash
+poetry install
+
+poetry run slither .
+```
 
 ## Test deploy
 Ethereum mainnet:
@@ -57,6 +64,7 @@ Fill in `PRIVATE_KEY` and `ETHERSCAN_KEY` in `.env`.
 - [x] Testing
 - [x] Deployment scripts
 - [x] Redeem route with queue toggle and slippage tolerance
+- [x] Slither self audit workflow
 - [ ] Documentation of derived math and code
 - [ ] Gas optimization
   - [x] Abstract route finder for off-chain call

pyproject.toml

@@ -0,0 +1,16 @@
+[tool.poetry]
+name = "MevEthRouter"
+version = "0.1.0"
+description = ""
+authors = ["Your Name <[email protected]>"]
+readme = "README.md"
+packages = [{include = "MevEthRouter"}]
+
+[tool.poetry.dependencies]
+python = "^3.11"
+slither-analyzer = "^0.10.0"
+
+
+[build-system]
+requires = ["poetry-core"]
+build-backend = "poetry.core.masonry.api"

slither.config.json

@@ -0,0 +1,15 @@
+{
+    "detectors_to_exclude": "calls-loop,unused-return,too-many-digits,similar-names,cyclomatic-complexity,low-level-calls,assembly,uninitialized-local,incorrect-equality,arbitrary-send-eth,block-timestamp,solc-version,naming-convention,reentrancy-events",
+    "exclude_informational": false,
+    "exclude_low": false,
+    "exclude_medium": false,
+    "exclude_high": false,
+    "disable_color": false,
+    "filter_paths": "(lib/|mocks/|test/)",
+    "legacy_ast": false,
+    "solc_remaps": [
+        "ds-test/=lib/ds-test/src/",
+        "forge-std/=lib/forge-std/src/",
+        "solmate/=lib/solmate/src/"
+    ]
+}

src/MevEthRouter.sol

@@ -66,10 +66,9 @@ contract MevEthRouter is IUniswapV3SwapCallback, IMevEthRouter {
     /// @dev Balancer pool id
     bytes32 internal poolId = 0xb3b675a9a3cb0df8f66caf08549371bfb76a9867000200000000000000000611;
     /// @dev Gyro pool
-    IGyro internal gyro = IGyro(0xb3b675a9A3CB0DF8F66Caf08549371BfB76A9867);
+    IGyro internal constant gyro = IGyro(0xb3b675a9A3CB0DF8F66Caf08549371BfB76A9867);
 
-    IRateProvider internal rateProvider0 = IRateProvider(0xf518f2EbeA5df8Ca2B5E9C7996a2A25e8010014b);
-    IRateProvider internal rateProvider1 = IRateProvider(address(0));
+    IRateProvider internal constant rateProvider0 = IRateProvider(0xf518f2EbeA5df8Ca2B5E9C7996a2A25e8010014b);
 
     uint256[3] internal uniV3Caps = [0, 0, 15 ether];
 
@@ -547,93 +546,6 @@ contract MevEthRouter is IUniswapV3SwapCallback, IMevEthRouter {
         }
     }
 
-    /// @notice assigns optimal route for maximum amount out, given pool reserves
-    function _splitSwapOutRough(
-        bool isDeposit,
-        uint256 amountIn,
-        uint256[8] memory amountsOutSingleSwap,
-        uint256[8] memory amountsOutSingleEth,
-        Reserve[8] memory reserves
-    )
-        internal
-        returns (uint256[8] memory amountsIn, uint256[8] memory amountsOut)
-    {
-        uint256[8] memory index = MevEthLibrary._sortArray(amountsOutSingleSwap); // sorts in ascending order (i.e. best price is last)
-        // First check best single swap price and return if no split is needed
-        if (_isNonZero(amountsOutSingleSwap[index[7]]) && amountIn <= MIN_LIQUIDITY) {
-            amountsIn[index[7]] = amountIn; // set best price as default, before splitting
-            amountsOut[index[7]] = amountsOutSingleSwap[index[7]];
-            return (amountsIn, amountsOut);
-        }
-
-        // check split estimate
-        uint256 numPools;
-        for (uint256 i; i < 8; i = _inc(i)) {
-            if (_isZero(amountsOutSingleEth[i])) continue;
-            unchecked {
-                ++numPools;
-            }
-        }
-
-        if (numPools < 2) {
-            amountsIn[index[7]] = amountIn; // set best price as default, before splitting
-            amountsOut[index[7]] = amountsOutSingleSwap[index[7]];
-            return (amountsIn, amountsOut);
-        }
-        // optimistically initialise with equal amounts in
-        for (uint256 i; i < 8; i = _inc(i)) {
-            if (_isZero(amountsOutSingleEth[i])) continue;
-            amountsIn[i] = amountIn / numPools;
-            amountsOut[i] = amountOutCall(isDeposit, i, amountsIn[i], reserves[i].reserveIn, reserves[i].reserveOut);
-            if (amountsOut[i] == 0) {
-                amountsIn[i] = 0;
-                amountsOutSingleEth[i] = 0;
-                --numPools;
-                if (numPools < 2) {
-                    delete amountsIn;
-                    delete amountsOut;
-                    amountsIn[index[7]] = amountIn; // set best price as default, before splitting
-                    amountsOut[index[7]] = amountsOutSingleSwap[index[7]];
-                    return (amountsIn, amountsOut);
-                }
-            }
-        }
-
-        // Converge on optimal split
-        for (uint256 iteration = 0; iteration < 10; iteration = _inc(iteration)) {
-            uint256 totalAmountOut = 0;
-
-            // Calculate total output for the current split
-            for (uint256 i = 0; i < 8; i = _inc(i)) {
-                totalAmountOut += amountsOut[i];
-            }
-
-            // Adjust amounts based on the difference between target and actual output
-            for (uint256 i = 0; i < 8; i++) {
-                if (amountsIn[i] == 0) continue;
-                uint256 targetAmountOut = (amountsIn[i] * totalAmountOut) / amountIn;
-                if (amountsOut[i] > targetAmountOut) {
-                    // performing better so assign more amount in
-                    amountsIn[i] = amountsIn[i] + (amountsIn[i] * 2 * (amountsOut[i] - targetAmountOut)) / targetAmountOut;
-                } else {
-                    amountsIn[i] = amountsIn[i] - (amountsIn[i] * 2 * (targetAmountOut - amountsOut[i])) / targetAmountOut;
-                }
-            }
-
-            // Normalize amounts to maintain the total sum
-            uint256 currentTotalAmountIn = 0;
-            for (uint256 i = 0; i < 8; i++) {
-                if (amountsIn[i] == 0) continue;
-                currentTotalAmountIn += amountsIn[i];
-            }
-            for (uint256 i = 0; i < 8; i++) {
-                if (amountsIn[i] == 0) continue;
-                amountsIn[i] = (amountsIn[i] * amountIn) / currentTotalAmountIn;
-                amountsOut[i] = amountOutCall(isDeposit, i, amountsIn[i], reserves[i].reserveIn, reserves[i].reserveOut);
-            }
-        }
-    }
-
     /// @notice assigns optimal route for maximum amount out, given pool reserves
     function _splitSwapOut(
         bool isDeposit,
@@ -1044,6 +956,7 @@ contract MevEthRouter is IUniswapV3SwapCallback, IMevEthRouter {
 
     function changeGov(address newGov) external {
         if (msg.sender != gov) revert ExecuteNotAuthorized();
+        if (newGov == address(0)) revert ZeroAddress();
         gov = newGov;
     }
 
@@ -1054,6 +967,7 @@ contract MevEthRouter is IUniswapV3SwapCallback, IMevEthRouter {
 
     function changeCurvePool(address newCurvePool) external {
         if (msg.sender != gov) revert ExecuteNotAuthorized();
+        if (newCurvePool == address(0)) revert ZeroAddress();
         curveV2Pool = newCurvePool;
     }
 

src/libraries/MevEthLibrary.sol

@@ -4,7 +4,6 @@ pragma solidity ^0.8.13;
 import "../interfaces/IUniswapV3Pool.sol";
 import "../interfaces/IUniswapV2Pair.sol";
 import "../interfaces/IUniswapV2Factory.sol";
-import "./Babylonian.sol";
 
 /// @title MevEthLibrary
 /// @author Manifold FInance
@@ -79,17 +78,6 @@ library MevEthLibrary {
         if (isZeroAddress) revert ZeroAddress();
     }
 
-    /// @notice Calculates the CREATE2 address for a pair without making any external calls
-    /// @dev Factory passed in directly because we have multiple factories. Format changes for new solidity spec.
-    /// @param factory Factory address for dex
-    /// @param tokenA Pool token
-    /// @param tokenB Pool token
-    /// @return pair Pair pool address
-    function pairFor(address factory, address tokenA, address tokenB, bytes32 factoryHash) internal pure returns (address pair) {
-        (address token0, address token1) = sortTokens(tokenA, tokenB);
-        pair = _asmPairFor(factory, token0, token1, factoryHash);
-    }
-
     /// @custom:assembly Calculates the CREATE2 address for a pair without making any external calls from pre-sorted tokens
     /// @notice Calculates the CREATE2 address for a pair without making any external calls from pre-sorted tokens
     /// @dev Factory passed in directly because we have multiple factories. Format changes for new solidity spec.
@@ -121,18 +109,6 @@ library MevEthLibrary {
         }
     }
 
-    /// @notice Fetches and sorts the reserves for a pair
-    /// @param factory Factory address for dex
-    /// @param tokenA Pool token
-    /// @param tokenB Pool token
-    /// @return reserveA Reserves for tokenA
-    /// @return reserveB Reserves for tokenB
-    function getReserves(address factory, address tokenA, address tokenB, bytes32 factoryHash) internal view returns (uint256 reserveA, uint256 reserveB) {
-        (address token0, address token1) = sortTokens(tokenA, tokenB);
-        (uint256 reserve0, uint256 reserve1,) = IUniswapV2Pair(_asmPairFor(factory, token0, token1, factoryHash)).getReserves();
-        (reserveA, reserveB) = tokenA == token0 ? (reserve0, reserve1) : (reserve1, reserve0);
-    }
-
     /// @notice Given an input asset amount, returns the maximum output amount of the other asset (accounting for fees) given reserves
     /// @dev Require replaced with revert custom error
     /// @param amountIn Amount of token in
@@ -213,19 +189,6 @@ library MevEthLibrary {
         }
     }
 
-    /// @dev returns amount In of pool 1 required to sync prices with pool 2
-    /// @param x1 reserveIn pool 1
-    /// @param y1 reserveOut pool 1
-    /// @param x2 reserveIn pool 2
-    /// @param y2 reserveOut pool 2
-    /// @param fee pool 1 fee
-    function _amountToSyncPricesFee(uint256 x1, uint256 y1, uint256 x2, uint256 y2, uint256 fee) internal pure returns (uint256) {
-        unchecked {
-            return (x1 * (Babylonian.sqrt((fee * fee + (x2 * y1 * (4_000_000_000_000 - 4_000_000 * fee)) / (x1 * y2))) - (2_000_000 - fee)))
-                / (2 * (1_000_000 - fee));
-        }
-    }
-
     /// @custom:gas Uint256 zero check gas saver
     /// @dev Uint256 zero check gas saver
     /// @param value Number to check
@@ -244,15 +207,6 @@ library MevEthLibrary {
         }
     }
 
-    /// @custom:gas Unchecked increment gas saver
-    /// @dev Unchecked increment gas saver for loops
-    /// @param i Number to increment
-    function _inc(uint256 i) internal pure returns (uint256) {
-        unchecked {
-            return i + 1;
-        }
-    }
-
     /// @custom:gas Unchecked decrement gas saver
     /// @dev Unchecked decrement gas saver for loops
     /// @param i Number to decrement