Merge pull request #306 from sablierhq/prb/invariants

test: add invariant tests

.github/workflows/ci.yml

@@ -10,12 +10,11 @@ env:
   INFURA_API_KEY: ${{ secrets.INFURA_API_KEY }}
 
 on:
+  workflow_dispatch:
   pull_request:
-    branches:
-      - "main"
   push:
     branches:
-      - "main"
+      - main
 
 jobs:
   lint:
@@ -58,6 +57,12 @@ jobs:
       - name: "Produce an optimized build with --via-ir"
         run: "FOUNDRY_PROFILE=optimized forge build"
 
+      - name: "Cache the build so that it can be re-used by the other jobs"
+        uses: "actions/cache/save@v3"
+        with:
+          path: "optimized-out"
+          key: "foundry-build-${{ github.sha }}"
+
       - name: "Store contract ABIs as artifacts in GitHub Actions"
         uses: "actions/upload-artifact@v3"
         with:
@@ -100,8 +105,12 @@ jobs:
       - name: "Install Foundry"
         uses: "foundry-rs/foundry-toolchain@v1"
 
-      - name: "Produce an optimized build with --via-ir"
-        run: "FOUNDRY_PROFILE=optimized forge build"
+      - name: "Restore the cached build"
+        uses: "actions/cache/restore@v3"
+        with:
+          fail-on-cache-miss: true
+          key: "foundry-build-${{ github.sha }}"
+          path: "optimized-out"
 
       - name: "Run the unit tests against the optimized build"
         run: "FOUNDRY_PROFILE=test-optimized forge test --match-path \"test/unit/**/*.sol\""
@@ -113,7 +122,7 @@ jobs:
 
   test-fuzz:
     env:
-      FOUNDRY_FUZZ_RUNS: "20000"
+      FOUNDRY_FUZZ_RUNS: "50000"
     needs: ["lint", "build"]
     runs-on: "ubuntu-latest"
     steps:
@@ -125,8 +134,12 @@ jobs:
       - name: "Install Foundry"
         uses: "foundry-rs/foundry-toolchain@v1"
 
-      - name: "Produce an optimized build with --via-ir"
-        run: "FOUNDRY_PROFILE=optimized forge build"
+      - name: "Restore the cached build"
+        uses: "actions/cache/restore@v3"
+        with:
+          fail-on-cache-miss: true
+          key: "foundry-build-${{ github.sha }}"
+          path: "optimized-out"
 
       - name: "Run the fuzz tests against the optimized build"
         run: "FOUNDRY_PROFILE=test-optimized forge test --match-path \"test/fuzz/**/*.sol\""
@@ -136,6 +149,32 @@ jobs:
           echo "## Fuzz tests result" >> $GITHUB_STEP_SUMMARY
           echo "✅ Passed" >> $GITHUB_STEP_SUMMARY
 
+  test-invariant:
+    needs: ["lint", "build"]
+    runs-on: "ubuntu-latest"
+    steps:
+      - name: "Check out the repo"
+        uses: "actions/checkout@v3"
+        with:
+          submodules: "recursive"
+
+      - name: "Install Foundry"
+        uses: "foundry-rs/foundry-toolchain@v1"
+
+      - name: "Restore the cached build"
+        uses: "actions/cache/restore@v3"
+        with:
+          fail-on-cache-miss: true
+          key: "foundry-build-${{ github.sha }}"
+          path: "optimized-out"
+
+      - name: "Run the invariant tests against the optimized build"
+        run: "FOUNDRY_PROFILE=test-optimized forge test --match-path \"test/invariant/**/*.sol\""
+
+      - name: "Add test summary"
+        run: |
+          echo "## Invariant tests result" >> $GITHUB_STEP_SUMMARY
+          echo "✅ Passed" >> $GITHUB_STEP_SUMMARY
 
   test-e2e:
     env:
@@ -151,8 +190,12 @@ jobs:
       - name: "Install Foundry"
         uses: "foundry-rs/foundry-toolchain@v1"
 
-      - name: "Produce an optimized build with --via-ir"
-        run: "FOUNDRY_PROFILE=optimized forge build"
+      - name: "Restore the cached build"
+        uses: "actions/cache/restore@v3"
+        with:
+          fail-on-cache-miss: true
+          key: "foundry-build-${{ github.sha }}"
+          path: "optimized-out"
 
       - name: "Generate fuzz seed that changes weekly to avoid burning through RPC allowance"
         run: >

.gitmodules

@@ -3,7 +3,7 @@
   path = "lib/ERC3156"
   url = "https://github.com/alcueca/ERC3156"
 [submodule "lib/forge-std"]
-  branch = "master"
+  branch = "v1"
   path = "lib/forge-std"
   url = "https://github.com/foundry-rs/forge-std"
 [submodule "lib/openzeppelin-contracts"]

.vscode/settings.json

@@ -1,10 +1,10 @@
 {
-  "[toml]": {
-    "editor.defaultFormatter": "tamasfe.even-better-toml"
-  },
   "[solidity]": {
     "editor.defaultFormatter": "esbenp.prettier-vscode"
   },
+  "[toml]": {
+    "editor.defaultFormatter": "tamasfe.even-better-toml"
+  },
   "solidity.formatter": "prettier",
   "solidity.packageDefaultDependenciesContractsDirectory": "src",
   "solidity.packageDefaultDependenciesDirectory": "lib"

foundry.toml

@@ -1,7 +1,6 @@
 [profile.default]
   auto_detect_solc = false
   fs_permissions = [{ access = "read", path = "./optimized-out" }]
-  fuzz = { max_test_rejects = 1_000_000, runs = 1_000 }
   libs = ["lib"]
   gas_reports = [
     "SablierV2Comptroller",
@@ -16,9 +15,19 @@
   src = "src"
   test = "test"
 
+[profile.default.fuzz]
+  max_test_rejects = 1_000_000 # Number of times `vm.assume` can fail
+  runs = 1_000
+
+[profile.default.invariant]
+  call_override = false # Override unsafe external calls to perform reentrancy checks
+  fail_on_revert = true
+  depth = 200           # Number of calls executed in one run
+
 # Speed up compilation and tests during development
 [profile.lite]
   fuzz = { runs = 50 }
+  invariant = { depth = 50, runs = 50 }
   optimizer = false
 
 # Compile only the production code with IR
@@ -30,6 +39,7 @@
 # Test the optimized contracts without re-compiling them
 [profile.test-optimized]
   fuzz = { runs = 5_000 }
+  invariant = { depth = 250, runs = 200 }
   src = "test"
   verbosity = 4
 
@@ -39,5 +49,5 @@
 
 [rpc_endpoints]
   ethereum = "https://eth-mainnet.g.alchemy.com/v2/${ALCHEMY_API_KEY}"
-  localhost = "http://localhost:8545"
   goerli = "https://goerli.infura.io/v3/${INFURA_API_KEY}"
+  localhost = "http://localhost:8545"

remappings.txt

@@ -2,7 +2,7 @@
 @prb/contracts/=lib/prb-contracts/src/
 @prb/math/=lib/prb-math/src/
 @prb/test/=lib/prb-test/src/
-erc3156/=lib/ERC3156/
+erc3156/=lib/ERC3156/contracts/
 forge-std/=lib/forge-std/src/
 solarray/=lib/solarray/src/
 src/=src/

src/abstracts/SablierV2FlashLoan.sol

@@ -4,8 +4,8 @@ pragma solidity >=0.8.13;
 import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol";
 import { SafeERC20 } from "@openzeppelin/token/ERC20/utils/SafeERC20.sol";
 import { ud } from "@prb/math/UD60x18.sol";
-import { IERC3156FlashBorrower } from "erc3156/contracts/interfaces/IERC3156FlashBorrower.sol";
-import { IERC3156FlashLender } from "erc3156/contracts/interfaces/IERC3156FlashLender.sol";
+import { IERC3156FlashBorrower } from "erc3156/interfaces/IERC3156FlashBorrower.sol";
+import { IERC3156FlashLender } from "erc3156/interfaces/IERC3156FlashLender.sol";
 
 import { Errors } from "../libraries/Errors.sol";
 import { Events } from "../libraries/Events.sol";

src/abstracts/SablierV2Lockup.sol

@@ -99,7 +99,9 @@ abstract contract SablierV2Lockup is
             revert Errors.SablierV2Lockup_StreamNotCanceledOrDepleted(streamId);
         }
 
-        // Checks: the `msg.sender` is either the owner of the NFT or an approved operator.
+        // Checks:
+        // 1. NFT exists (see `getApproved`).
+        // 2. `msg.sender` is either the owner of the NFT or an approved operator.
         if (!_isApprovedOrOwner(streamId, msg.sender)) {
             revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
         }
@@ -142,12 +144,12 @@ abstract contract SablierV2Lockup is
 
     /// @inheritdoc ISablierV2Lockup
     function renounce(uint256 streamId) external override isActiveStream(streamId) {
-        // Checks: the `msg.sender` is the sender of the stream.
+        // Checks: `msg.sender` is the sender of the stream.
         if (!_isCallerStreamSender(streamId)) {
             revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
         }
 
-        // Checks: the stream is cancelable.
+        // Checks: the stream is not already non-cancelable.
         if (!isCancelable(streamId)) {
             revert Errors.SablierV2Lockup_RenounceNonCancelableStream(streamId);
         }
@@ -202,7 +204,7 @@ abstract contract SablierV2Lockup is
 
             // If the `streamId` does not point to an active stream, simply skip it.
             if (getStatus(streamId) == Lockup.Status.ACTIVE) {
-                // Checks: the `msg.sender` is an approved operator or the owner of the NFT (also known as the recipient
+                // Checks: `msg.sender` is an approved operator or the owner of the NFT (also known as the recipient
                 // of the stream).
                 if (!_isApprovedOrOwner(streamId, msg.sender)) {
                     revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
@@ -232,9 +234,9 @@ abstract contract SablierV2Lockup is
         address spender
     ) internal view virtual returns (bool isApprovedOrOwner);
 
-    /// @notice Checks whether the `msg.sender` is the sender of the stream or not.
+    /// @notice Checks whether `msg.sender` is the sender of the stream or not.
     /// @param streamId The id of the stream to make the query for.
-    /// @return result Whether the `msg.sender` is the sender of the stream or not.
+    /// @return result Whether `msg.sender` is the sender of the stream or not.
     function _isCallerStreamSender(uint256 streamId) internal view virtual returns (bool result);
 
     /*//////////////////////////////////////////////////////////////////////////

src/libraries/Events.sol

@@ -3,7 +3,7 @@ pragma solidity >=0.8.13;
 
 import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol";
 import { UD60x18 } from "@prb/math/UD60x18.sol";
-import { IERC3156FlashBorrower } from "erc3156/contracts/interfaces/IERC3156FlashBorrower.sol";
+import { IERC3156FlashBorrower } from "erc3156/interfaces/IERC3156FlashBorrower.sol";
 
 import { ISablierV2Comptroller } from "../interfaces/ISablierV2Comptroller.sol";
 import { Lockup, LockupLinear, LockupPro } from "../types/DataTypes.sol";

test/Base.t.sol

@@ -21,7 +21,7 @@ import { Constants } from "./shared/helpers/Constants.t.sol";
 import { Utils } from "./shared/helpers/Utils.t.sol";
 
 /// @title Base_Test
-/// @notice Base test contract that contains common logic needed by all test contracts.
+/// @notice Base test contract with common logic needed by all test contracts.
 abstract contract Base_Test is Assertions, Constants, Calculations, Utils, StdCheats {
     /*//////////////////////////////////////////////////////////////////////////
                                        STRUCTS
@@ -66,8 +66,8 @@ abstract contract Base_Test is Assertions, Constants, Calculations, Utils, StdCh
     ISablierV2Comptroller internal comptroller;
     IERC20 internal dai = new ERC20("Dai Stablecoin", "DAI");
     ISablierV2LockupLinear internal linear;
-    NonCompliantERC20 internal nonCompliantAsset = new NonCompliantERC20("Non-Compliant ERC-20 Asset", "NCT", 18);
     ISablierV2LockupPro internal pro;
+    NonCompliantERC20 internal nonCompliantAsset = new NonCompliantERC20("Non-Compliant ERC-20 Asset", "NCT", 18);
 
     /*//////////////////////////////////////////////////////////////////////////
                                     CONSTRUCTOR
@@ -83,6 +83,18 @@ abstract contract Base_Test is Assertions, Constants, Calculations, Utils, StdCh
         vm.label({ account: address(nonCompliantAsset), newLabel: "Non-Compliant ERC-20 Asset" });
     }
 
+    /*//////////////////////////////////////////////////////////////////////////
+                                     MODIFIERS
+    //////////////////////////////////////////////////////////////////////////*/
+
+    /// @dev Modifier that runs the function only in a CI environment.
+    modifier onlyInCI() {
+        string memory ci = vm.envOr("CI", string(""));
+        if (eqString(ci, "true")) {
+            _;
+        }
+    }
+
     /*//////////////////////////////////////////////////////////////////////////
                                   SET-UP FUNCTION
     //////////////////////////////////////////////////////////////////////////*/
@@ -109,12 +121,6 @@ abstract contract Base_Test is Assertions, Constants, Calculations, Utils, StdCh
         blockTimestamp = uint40(block.timestamp);
     }
 
-    /// @dev Checks if the Foundry profile is "test-optimized".
-    function isTestOptimizedProfile() internal returns (bool result) {
-        string memory profile = vm.envOr("FOUNDRY_PROFILE", string(""));
-        result = eqString(profile, "test-optimized");
-    }
-
     /*//////////////////////////////////////////////////////////////////////////
                            INTERNAL NON-CONSTANT FUNCTIONS
     //////////////////////////////////////////////////////////////////////////*/
@@ -190,8 +196,14 @@ abstract contract Base_Test is Assertions, Constants, Calculations, Utils, StdCh
         }
 
         // Finally, label all the contracts just deployed.
-        vm.label({ account: address(comptroller), newLabel: "SablierV2Comptroller" });
-        vm.label({ account: address(linear), newLabel: "SablierV2LockupLinear" });
-        vm.label({ account: address(pro), newLabel: "SablierV2LockupPro" });
+        vm.label({ account: address(comptroller), newLabel: "Comptroller" });
+        vm.label({ account: address(linear), newLabel: "LockupLinear" });
+        vm.label({ account: address(pro), newLabel: "LockupPro" });
+    }
+
+    /// @dev Checks if the Foundry profile is "test-optimized".
+    function isTestOptimizedProfile() internal returns (bool result) {
+        string memory profile = vm.envOr("FOUNDRY_PROFILE", string(""));
+        result = eqString(profile, "test-optimized");
     }
 }

test/e2e/E2eTest.t.sol

@@ -35,7 +35,7 @@ abstract contract E2eTest is Base_Test {
     }
 
     /*//////////////////////////////////////////////////////////////////////////
-                                   SETUP FUNCTION
+                                  SET-UP FUNCTION
     //////////////////////////////////////////////////////////////////////////*/
 
     function setUp() public virtual override {

test/e2e/lockup/linear/Linear.t.sol

@@ -18,7 +18,7 @@ abstract contract Linear_E2e_Test is E2eTest {
     constructor(IERC20 asset_, address holder_) E2eTest(asset_, holder_) {}
 
     /*//////////////////////////////////////////////////////////////////////////
-                                   SETUP FUNCTION
+                                  SET-UP FUNCTION
     //////////////////////////////////////////////////////////////////////////*/
 
     function setUp() public virtual override {
@@ -109,6 +109,7 @@ abstract contract Linear_E2e_Test is E2eTest {
     /// - Multiple values for the protocol fee, including zero.
     /// - Multiple values for the withdraw amount, including zero.
     function testForkFuzz_Linear_CreateWithdrawCancel(Params memory params) external {
+        vm.assume(params.range.start <= params.range.cliff && params.range.cliff <= params.range.end);
         vm.assume(params.sender != address(0) && params.recipient != address(0) && params.broker.addr != address(0));
         vm.assume(
             params.sender != params.recipient &&
@@ -122,6 +123,7 @@ abstract contract Linear_E2e_Test is E2eTest {
                 params.broker.addr != address(linear)
         );
         vm.assume(params.range.start <= params.range.cliff && params.range.cliff < params.range.end);
+        vm.assume(params.totalAmount != 0 && params.totalAmount <= initialHolderBalance);
         params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE);
         params.protocolFee = bound(params.protocolFee, 0, DEFAULT_MAX_FEE);
         params.totalAmount = boundUint128(params.totalAmount, 1, uint128(initialHolderBalance));

test/e2e/lockup/pro/Pro.t.sol

@@ -18,7 +18,7 @@ abstract contract Pro_E2e_Test is E2eTest {
     constructor(IERC20 asset_, address holder_) E2eTest(asset_, holder_) {}
 
     /*//////////////////////////////////////////////////////////////////////////
-                                   SETUP FUNCTION
+                                  SET-UP FUNCTION
     //////////////////////////////////////////////////////////////////////////*/
 
     function setUp() public virtual override {

test/fuzz/Fuzz.t.sol

@@ -4,7 +4,7 @@ pragma solidity >=0.8.13 <0.9.0;
 import { Base_Test } from "../Base.t.sol";
 
 /// @title Fuzz_Test
-/// @notice Base fuzz test contract that contains common logic needed by all fuzz test contracts.
+/// @notice Base test contract with common logic needed by all fuzz test contracts.
 abstract contract Fuzz_Test is Base_Test {
     /*//////////////////////////////////////////////////////////////////////////
                                   SET-UP FUNCTION

test/fuzz/adminable/Adminable.t.sol

@@ -21,7 +21,7 @@ abstract contract Adminable_Fuzz_Test is Fuzz_Test {
     function setUp() public virtual override {
         Fuzz_Test.setUp();
 
-        // Cast the linear contract as the `ISablierV2Adminable` contract.
+        // Cast the linear contract as {ISablierV2Adminable}.
         adminable = ISablierV2Adminable(address(linear));
     }
 }