From ff201966a4dd5ef6702237783cc94ca6f71744d0 Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Tue, 24 Jan 2023 22:33:21 +0200 Subject: [PATCH 01/11] ci: simplify syntax for "on:" ci: add "workflow_syntax" in "on" for "main" branch --- .github/workflows/ci.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d0de63822..1cf5fc03f 100644 --- a/.github/workflows/ci.yml +++ b/.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: From 3ad58bbf5aef15b77e20e79820fa9e806f0e57fd Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Thu, 26 Jan 2023 12:51:16 +0200 Subject: [PATCH 02/11] build: set version of "forge-std" to "v1" --- .gitmodules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitmodules b/.gitmodules index b593dcfb1..a82b3aae4 100644 --- a/.gitmodules +++ b/.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"] From b1c34ec88e1e78efb720cac68cee61ce5c9a4ae8 Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Thu, 26 Jan 2023 18:54:53 +0200 Subject: [PATCH 03/11] test: add NFT non-existent unit test for "burn" chore: document all checks in "burn" function with code comments --- src/abstracts/SablierV2Lockup.sol | 4 +++- test/unit/lockup/shared/burn/burn.t.sol | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/abstracts/SablierV2Lockup.sol b/src/abstracts/SablierV2Lockup.sol index 9c64870bf..0b7316b48 100644 --- a/src/abstracts/SablierV2Lockup.sol +++ b/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); } diff --git a/test/unit/lockup/shared/burn/burn.t.sol b/test/unit/lockup/shared/burn/burn.t.sol index 358b0c691..4a0b3a0a0 100644 --- a/test/unit/lockup/shared/burn/burn.t.sol +++ b/test/unit/lockup/shared/burn/burn.t.sol @@ -57,8 +57,22 @@ abstract contract Burn_Unit_Test is Unit_Test, Lockup_Shared_Test { _; } + /// @dev it should revert. + function test_RevertWhen_NFTNonExistent() external streamCanceledOrDepleted callerAuthorized { + // Burn the NFT so that it no longer exists. + lockup.burn(streamId); + + // Run the test. + vm.expectRevert("ERC721: invalid token ID"); + lockup.burn(streamId); + } + + modifier nftExistent() { + _; + } + /// @dev it should burn the NFT. - function test_Burn_CallerApprovedOperator() external streamCanceledOrDepleted callerAuthorized { + function test_Burn_CallerApprovedOperator() external streamCanceledOrDepleted callerAuthorized nftExistent { // Approve the operator to handle the stream. lockup.approve({ to: users.operator, tokenId: streamId }); @@ -75,7 +89,7 @@ abstract contract Burn_Unit_Test is Unit_Test, Lockup_Shared_Test { } /// @dev it should burn the NFT. - function test_Burn_CallerNFTOwner() external streamCanceledOrDepleted callerAuthorized { + function test_Burn_CallerNFTOwner() external streamCanceledOrDepleted callerAuthorized nftExistent { lockup.burn(streamId); address actualNFTOwner = lockup.getRecipient(streamId); address expectedNFTOwner = address(0); From 2a3b0a82d078cb46a5681a38d55ca6e26d2391d1 Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Fri, 27 Jan 2023 23:53:27 +0200 Subject: [PATCH 04/11] test: invariant tests chore: improve formatting and wording in code comments docs: improve wording in NatSpec comments refactor: change remappings from "erc3156/contracts" to simply "erc3156" test: add "MAX_UNIX_TIMESTAMP" constant test: add invariant handlers test: add "calculateDepositAmount" helper test: bound unix timestamps to "MAX_UNIX_TIMESTAMP" instead of "UINT40_MAX" --- .vscode/settings.json | 6 +- foundry.toml | 14 +- remappings.txt | 2 +- src/abstracts/SablierV2FlashLoan.sol | 4 +- src/abstracts/SablierV2Lockup.sol | 10 +- src/libraries/Events.sol | 2 +- test/Base.t.sol | 8 +- test/e2e/E2eTest.t.sol | 2 +- test/e2e/lockup/linear/Linear.t.sol | 4 +- test/e2e/lockup/pro/Pro.t.sol | 2 +- test/fuzz/Fuzz.t.sol | 2 +- test/fuzz/adminable/Adminable.t.sol | 2 +- .../flash-loan/flash-loan/flashLoan.t.sol | 8 +- test/fuzz/lockup/linear/Linear.t.sol | 6 +- .../createWithDurations.t.sol | 2 +- .../create-with-range/createWithRange.t.sol | 4 +- .../streamed-amount-of/streamedAmountOf.sol | 2 +- .../withdrawableAmountOf.t.sol | 4 +- test/fuzz/lockup/pro/Pro.t.sol | 6 +- .../create-with-deltas/createWithDeltas.t.sol | 4 +- .../createWithMilestones.t.sol | 13 +- .../withdraw-multiple/withdrawMultiple.t.sol | 2 +- test/invariant/Invariant.t.sol | 41 +++ test/invariant/handlers/BaseHandler.t.sol | 48 ++++ .../handlers/ComptrollerHandler.t.sol | 46 ++++ .../invariant/handlers/FlashLoanHandler.t.sol | 77 ++++++ test/invariant/handlers/LockupHandler.t.sol | 233 ++++++++++++++++++ .../handlers/LockupLinearHandler.t.sol | 161 ++++++++++++ .../invariant/handlers/LockupProHandler.t.sol | 203 +++++++++++++++ test/invariant/lockup/Lockup.t.sol | 161 ++++++++++++ test/invariant/lockup/linear/Linear.t.sol | 160 ++++++++++++ test/invariant/lockup/pro/Pro.t.sol | 161 ++++++++++++ test/shared/helpers/Calculations.t.sol | 14 +- test/shared/helpers/Constants.t.sol | 1 + test/shared/lockup/Lockup.t.sol | 2 +- test/shared/lockup/linear/Linear.t.sol | 2 +- test/shared/lockup/pro/Pro.t.sol | 2 +- .../flash-loan/FaultyFlashLoanReceiver.t.sol | 2 +- .../flash-loan/GoodFlashLoanReceiver.t.sol | 2 +- .../ReentrantFlashLoanReceiver.t.sol | 4 +- test/unit/Unit.t.sol | 11 +- test/unit/adminable/Adminable.t.sol | 2 +- test/unit/flash-loan/FlashLoan.t.sol | 2 +- .../flash-loan/flash-loan/flashLoan.t.sol | 6 +- test/unit/lockup/linear/Linear.t.sol | 6 +- test/unit/lockup/pro/Pro.t.sol | 8 +- 46 files changed, 1391 insertions(+), 73 deletions(-) create mode 100644 test/invariant/Invariant.t.sol create mode 100644 test/invariant/handlers/BaseHandler.t.sol create mode 100644 test/invariant/handlers/ComptrollerHandler.t.sol create mode 100644 test/invariant/handlers/FlashLoanHandler.t.sol create mode 100644 test/invariant/handlers/LockupHandler.t.sol create mode 100644 test/invariant/handlers/LockupLinearHandler.t.sol create mode 100644 test/invariant/handlers/LockupProHandler.t.sol create mode 100644 test/invariant/lockup/Lockup.t.sol create mode 100644 test/invariant/lockup/linear/Linear.t.sol create mode 100644 test/invariant/lockup/pro/Pro.t.sol diff --git a/.vscode/settings.json b/.vscode/settings.json index bced5b437..9686f2b28 100644 --- a/.vscode/settings.json +++ b/.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" diff --git a/foundry.toml b/foundry.toml index 0bc229f40..236291f54 100644 --- a/foundry.toml +++ b/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 = 1_000, runs = 5_000 } 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" diff --git a/remappings.txt b/remappings.txt index 622ac221f..2d6a1af14 100644 --- a/remappings.txt +++ b/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/ diff --git a/src/abstracts/SablierV2FlashLoan.sol b/src/abstracts/SablierV2FlashLoan.sol index e78a9deb0..dfba8ad4f 100644 --- a/src/abstracts/SablierV2FlashLoan.sol +++ b/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"; diff --git a/src/abstracts/SablierV2Lockup.sol b/src/abstracts/SablierV2Lockup.sol index 0b7316b48..150939c8e 100644 --- a/src/abstracts/SablierV2Lockup.sol +++ b/src/abstracts/SablierV2Lockup.sol @@ -144,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); } @@ -204,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); @@ -234,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); /*////////////////////////////////////////////////////////////////////////// diff --git a/src/libraries/Events.sol b/src/libraries/Events.sol index ef9ee5d58..3d0721fde 100644 --- a/src/libraries/Events.sol +++ b/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"; diff --git a/test/Base.t.sol b/test/Base.t.sol index 947dc7a54..a9e2bbc9b 100644 --- a/test/Base.t.sol +++ b/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 @@ -190,8 +190,8 @@ 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" }); } } diff --git a/test/e2e/E2eTest.t.sol b/test/e2e/E2eTest.t.sol index 78e793676..4baa69b0a 100644 --- a/test/e2e/E2eTest.t.sol +++ b/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 { diff --git a/test/e2e/lockup/linear/Linear.t.sol b/test/e2e/lockup/linear/Linear.t.sol index 12ffe7896..49ec876e5 100644 --- a/test/e2e/lockup/linear/Linear.t.sol +++ b/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 { @@ -108,6 +108,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 && @@ -121,6 +122,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)); diff --git a/test/e2e/lockup/pro/Pro.t.sol b/test/e2e/lockup/pro/Pro.t.sol index 6d6d8e2bb..bdd1522d0 100644 --- a/test/e2e/lockup/pro/Pro.t.sol +++ b/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 { diff --git a/test/fuzz/Fuzz.t.sol b/test/fuzz/Fuzz.t.sol index de9ca077e..9025bd573 100644 --- a/test/fuzz/Fuzz.t.sol +++ b/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 diff --git a/test/fuzz/adminable/Adminable.t.sol b/test/fuzz/adminable/Adminable.t.sol index 91cfcff21..97781a4f8 100644 --- a/test/fuzz/adminable/Adminable.t.sol +++ b/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)); } } diff --git a/test/fuzz/flash-loan/flash-loan/flashLoan.t.sol b/test/fuzz/flash-loan/flash-loan/flashLoan.t.sol index 1bb936289..8b8769b7f 100644 --- a/test/fuzz/flash-loan/flash-loan/flashLoan.t.sol +++ b/test/fuzz/flash-loan/flash-loan/flashLoan.t.sol @@ -3,7 +3,7 @@ pragma solidity >=0.8.13; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; import { MAX_UD60x18, UD60x18, ud } from "@prb/math/UD60x18.sol"; -import { IERC3156FlashBorrower } from "erc3156/contracts/interfaces/IERC3156FlashBorrower.sol"; +import { IERC3156FlashBorrower } from "erc3156/interfaces/IERC3156FlashBorrower.sol"; import { Errors } from "src/libraries/Errors.sol"; import { Events } from "src/libraries/Events.sol"; @@ -56,7 +56,7 @@ contract FlashLoanFunction_Fuzz_Test is FlashLoan_Fuzz_Test { /// - Multiple values for the comptroller flash fee, including zero. /// - Multiple values for the flash loan amount, including zero. /// - Multiple values for the data bytes array, including zero length. - function testFuzz_FlashLoan( + function testFuzz_FlashLoanFunction( UD60x18 comptrollerFlashFee, uint128 amount, bytes calldata data @@ -70,10 +70,10 @@ contract FlashLoanFunction_Fuzz_Test is FlashLoan_Fuzz_Test { // Load the flash fee. uint256 fee = flashLoan.flashFee({ asset: address(DEFAULT_ASSET), amount: amount }); - // Deal the flash loan amount to the contract. + // Mint the flash loan amount to the contract. deal({ token: address(DEFAULT_ASSET), to: address(flashLoan), give: amount }); - // Deal the flash fee to the receiver so that they can repay the flash loan. + // Mint the flash fee to the receiver so that they can repay the flash loan. deal({ token: address(DEFAULT_ASSET), to: address(goodFlashLoanReceiver), give: fee }); // Expect `amount` of ERC-20 assets to be transferred from the {SablierV2FlashLoan} contract to the receiver. diff --git a/test/fuzz/lockup/linear/Linear.t.sol b/test/fuzz/lockup/linear/Linear.t.sol index a0fd61f97..8cfe1661c 100644 --- a/test/fuzz/lockup/linear/Linear.t.sol +++ b/test/fuzz/lockup/linear/Linear.t.sol @@ -23,12 +23,12 @@ import { WithdrawMultiple_Fuzz_Test } from "../shared/withdraw-multiple/withdraw /// @notice Common testing logic needed across {SablierV2LockupLinear} fuzz tests. abstract contract Linear_Fuzz_Test is Fuzz_Test, Linear_Shared_Test { function setUp() public virtual override(Fuzz_Test, Linear_Shared_Test) { - // Both of these contracts inherit from `Base_Test`, which is fine because multiple inheritance is - // allowed in Solidity, and `Base_Test.setUp` will only be called once. + // Both of these contracts inherit from {Base_Test}, but this is fine because multiple inheritance is + // allowed in Solidity, and {Base_Test-setUp} will only be called once. Fuzz_Test.setUp(); Linear_Shared_Test.setUp(); - // Cast the linear contract as `ISablierV2Config` and `ISablierV2Lockup`. + // Cast the linear contract as {ISablierV2Config} and {ISablierV2Lockup}. config = ISablierV2Config(linear); lockup = ISablierV2Lockup(linear); diff --git a/test/fuzz/lockup/linear/create-with-durations/createWithDurations.t.sol b/test/fuzz/lockup/linear/create-with-durations/createWithDurations.t.sol index 0257d2a53..3075c1c91 100644 --- a/test/fuzz/lockup/linear/create-with-durations/createWithDurations.t.sol +++ b/test/fuzz/lockup/linear/create-with-durations/createWithDurations.t.sol @@ -86,7 +86,7 @@ contract CreateWithDurations_Linear_Fuzz_Test is Linear_Fuzz_Test { /// @dev it should perform the ERC-20 transfers, create the stream, bump the next stream id, record the /// protocol fee, mint the NFT, and emit a {CreateLockupLinearStream} event. function testFuzz_CreateWithDurations(LockupLinear.Durations memory durations) external { - durations.total = boundUint40(durations.total, 0, UINT40_MAX - getBlockTimestamp()); + durations.total = boundUint40(durations.total, 0, MAX_UNIX_TIMESTAMP); vm.assume(durations.cliff < durations.total); // Make the sender the funder in this test. diff --git a/test/fuzz/lockup/linear/create-with-range/createWithRange.t.sol b/test/fuzz/lockup/linear/create-with-range/createWithRange.t.sol index e60225ff0..d7dffb257 100644 --- a/test/fuzz/lockup/linear/create-with-range/createWithRange.t.sol +++ b/test/fuzz/lockup/linear/create-with-range/createWithRange.t.sol @@ -32,7 +32,7 @@ contract CreateWithRange_Linear_Fuzz_Test is Linear_Fuzz_Test { function testFuzz_RevertWhen_StartTimeGreaterThanCliffTime( uint40 startTime ) external recipientNonZeroAddress depositAmountNotZero { - startTime = boundUint40(startTime, defaultParams.createWithRange.range.cliff + 1, UINT40_MAX); + startTime = boundUint40(startTime, defaultParams.createWithRange.range.cliff + 1, MAX_UNIX_TIMESTAMP); vm.expectRevert( abi.encodeWithSelector( Errors.SablierV2LockupLinear_StartTimeGreaterThanCliffTime.selector, @@ -209,7 +209,7 @@ contract CreateWithRange_Linear_Fuzz_Test is Linear_Fuzz_Test { // Make the fuzzed funder the caller in this test. changePrank(params.funder); - // Mint enough assets to the funder. + // Mint enough ERC-20 assets to the funder. deal({ token: address(DEFAULT_ASSET), to: params.funder, give: params.totalAmount }); // Approve the {SablierV2LockupLinear} contract to transfer the assets from the fuzzed funder. diff --git a/test/fuzz/lockup/linear/streamed-amount-of/streamedAmountOf.sol b/test/fuzz/lockup/linear/streamed-amount-of/streamedAmountOf.sol index f8d9f9a61..a7f388d55 100644 --- a/test/fuzz/lockup/linear/streamed-amount-of/streamedAmountOf.sol +++ b/test/fuzz/lockup/linear/streamed-amount-of/streamedAmountOf.sol @@ -53,7 +53,7 @@ contract StreamedAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test { uint40 currentTime = DEFAULT_START_TIME + timeWarp; vm.warp({ timestamp: currentTime }); - // Mint enough assets to the sender. + // Mint enough ERC-20 assets to the sender. deal({ token: address(DEFAULT_ASSET), to: users.sender, give: depositAmount }); // Create the stream. diff --git a/test/fuzz/lockup/linear/withdrawable-amount-of/withdrawableAmountOf.t.sol b/test/fuzz/lockup/linear/withdrawable-amount-of/withdrawableAmountOf.t.sol index 5acc27fa5..73114722e 100644 --- a/test/fuzz/lockup/linear/withdrawable-amount-of/withdrawableAmountOf.t.sol +++ b/test/fuzz/lockup/linear/withdrawable-amount-of/withdrawableAmountOf.t.sol @@ -52,7 +52,7 @@ contract WithdrawableAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test { uint40 currentTime = DEFAULT_START_TIME + timeWarp; vm.warp({ timestamp: currentTime }); - // Mint enough assets to the sender. + // Mint enough ERC-20 assets to the sender. deal({ token: address(DEFAULT_ASSET), to: users.sender, give: depositAmount }); // Create the stream. The broker fee is disabled so that it doesn't interfere with the calculations. @@ -97,7 +97,7 @@ contract WithdrawableAmountOf_Linear_Fuzz_Test is Linear_Fuzz_Test { uint128 streamedAmount = calculateStreamedAmount(currentTime, depositAmount); withdrawAmount = boundUint128(withdrawAmount, 1, streamedAmount); - // Mint enough assets to the sender. + // Mint enough ERC-20 assets to the sender. deal({ token: address(DEFAULT_ASSET), to: users.sender, give: depositAmount }); // Create the stream. The broker fee is disabled so that it doesn't interfere with the calculations. diff --git a/test/fuzz/lockup/pro/Pro.t.sol b/test/fuzz/lockup/pro/Pro.t.sol index 8e5641815..f58d82582 100644 --- a/test/fuzz/lockup/pro/Pro.t.sol +++ b/test/fuzz/lockup/pro/Pro.t.sol @@ -23,12 +23,12 @@ import { WithdrawMultiple_Fuzz_Test } from "../shared/withdraw-multiple/withdraw /// @notice Common testing logic needed across {SablierV2LockupPro} fuzz tests. abstract contract Pro_Fuzz_Test is Fuzz_Test, Pro_Shared_Test { function setUp() public virtual override(Fuzz_Test, Pro_Shared_Test) { - // Both of these contracts inherit from `Base_Test`, which is fine because multiple inheritance is - // allowed in Solidity, and `Base_Test.setUp` will only be called once. + // Both of these contracts inherit from {Base_Test}, but this is fine because multiple inheritance is + // allowed in Solidity, and {Base_Test-setUp} will only be called once. Fuzz_Test.setUp(); Pro_Shared_Test.setUp(); - // Cast the pro contract as `ISablierV2Config` and `ISablierV2Lockup`. + // Cast the pro contract as {ISablierV2Config} and {ISablierV2Lockup}. config = ISablierV2Config(pro); lockup = ISablierV2Lockup(pro); diff --git a/test/fuzz/lockup/pro/create-with-deltas/createWithDeltas.t.sol b/test/fuzz/lockup/pro/create-with-deltas/createWithDeltas.t.sol index b3ec414ac..dcc7315c2 100644 --- a/test/fuzz/lockup/pro/create-with-deltas/createWithDeltas.t.sol +++ b/test/fuzz/lockup/pro/create-with-deltas/createWithDeltas.t.sol @@ -68,9 +68,9 @@ contract CreateWithDeltas_Pro_Fuzz_Test is Pro_Fuzz_Test { milestonesCalculationsDoNotOverflow { delta0 = boundUint40(delta0, 1, 100); - delta1 = boundUint40(delta1, 1, UINT40_MAX - getBlockTimestamp() - delta0); + delta1 = boundUint40(delta1, 1, MAX_UNIX_TIMESTAMP - getBlockTimestamp() - delta0); - // Create the deltas. + // Create the deltas array. uint40[] memory deltas = Solarray.uint40s(delta0, delta1); // Adjust the segment milestones to match the fuzzed deltas. diff --git a/test/fuzz/lockup/pro/create-with-milestones/createWithMilestones.t.sol b/test/fuzz/lockup/pro/create-with-milestones/createWithMilestones.t.sol index 6f1314c5a..25015fdd9 100644 --- a/test/fuzz/lockup/pro/create-with-milestones/createWithMilestones.t.sol +++ b/test/fuzz/lockup/pro/create-with-milestones/createWithMilestones.t.sol @@ -251,6 +251,7 @@ contract CreateWithMilestones_Pro_Fuzz_Test is Pro_Fuzz_Test { uint128 initialProtocolRevenues; uint128 depositAmount; uint128 protocolFeeAmount; + LockupPro.Segment[] segments; } /// @dev it should perform the ERC-20 transfers, create the stream, bump the next stream id, record the protocol @@ -299,7 +300,7 @@ contract CreateWithMilestones_Pro_Fuzz_Test is Pro_Fuzz_Test { // Make the fuzzed funder the caller in this test. changePrank(params.funder); - // Mint enough assets to the fuzzed funder. + // Mint enough ERC-20 assets to the fuzzed funder. deal({ token: address(DEFAULT_ASSET), to: params.funder, give: params.totalAmount }); // Approve the {SablierV2LockupPro} contract to transfer the assets from the funder. @@ -315,8 +316,8 @@ contract CreateWithMilestones_Pro_Fuzz_Test is Pro_Fuzz_Test { vars.depositAmount = params.totalAmount - vars.protocolFeeAmount - vars.brokerFeeAmount; // Adjust the segment amounts based on the fuzzed deposit amount. - LockupPro.Segment[] memory segments = defaultParams.createWithMilestones.segments; - adjustSegmentAmounts(segments, vars.depositAmount); + vars.segments = defaultParams.createWithMilestones.segments; + adjustSegmentAmounts(vars.segments, vars.depositAmount); // Expect the ERC-20 assets to be transferred from the funder to the {SablierV2LockupPro} contract. vm.expectCall( @@ -347,7 +348,7 @@ contract CreateWithMilestones_Pro_Fuzz_Test is Pro_Fuzz_Test { protocolFee: vars.protocolFeeAmount, brokerFee: vars.brokerFeeAmount }), - segments: segments, + segments: vars.segments, asset: DEFAULT_ASSET, cancelable: params.cancelable, range: LockupPro.Range({ start: params.startTime, end: DEFAULT_END_TIME }), @@ -359,7 +360,7 @@ contract CreateWithMilestones_Pro_Fuzz_Test is Pro_Fuzz_Test { params.sender, params.recipient, params.totalAmount, - segments, + vars.segments, DEFAULT_ASSET, params.cancelable, params.startTime, @@ -373,7 +374,7 @@ contract CreateWithMilestones_Pro_Fuzz_Test is Pro_Fuzz_Test { assertEq(actualStream.isCancelable, params.cancelable, "isCancelable"); assertEq(actualStream.range, LockupPro.Range({ start: params.startTime, end: defaultStream.range.end })); assertEq(actualStream.sender, params.sender, "sender"); - assertEq(actualStream.segments, segments); + assertEq(actualStream.segments, vars.segments); assertEq(actualStream.status, defaultStream.status); // Assert that the next stream id was bumped. diff --git a/test/fuzz/lockup/shared/withdraw-multiple/withdrawMultiple.t.sol b/test/fuzz/lockup/shared/withdraw-multiple/withdrawMultiple.t.sol index 00f8dd3f3..1de2a366c 100644 --- a/test/fuzz/lockup/shared/withdraw-multiple/withdrawMultiple.t.sol +++ b/test/fuzz/lockup/shared/withdraw-multiple/withdrawMultiple.t.sol @@ -185,9 +185,9 @@ abstract contract WithdrawMultiple_Fuzz_Test is Fuzz_Test, Lockup_Shared_Test { } struct Params { + uint128 ongoingWithdrawAmount; uint256 timeWarp; address to; - uint128 ongoingWithdrawAmount; } struct Vars { diff --git a/test/invariant/Invariant.t.sol b/test/invariant/Invariant.t.sol new file mode 100644 index 000000000..00afd71d8 --- /dev/null +++ b/test/invariant/Invariant.t.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { InvariantTest as ForgeInvariantTest } from "forge-std/InvariantTest.sol"; + +import { SablierV2Comptroller } from "src/SablierV2Comptroller.sol"; + +import { GoodFlashLoanReceiver } from "../shared/mockups/flash-loan/GoodFlashLoanReceiver.t.sol"; +import { Base_Test } from "../Base.t.sol"; +import { ComptrollerHandler } from "./handlers/ComptrollerHandler.t.sol"; + +/// @title Invariant_Test +/// @notice Base test contract with common logic needed by all invariant test contracts. +abstract contract Invariant_Test is Base_Test, ForgeInvariantTest { + /*////////////////////////////////////////////////////////////////////////// + TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + ComptrollerHandler internal comptrollerHandler; + + /*////////////////////////////////////////////////////////////////////////// + SET-UP FUNCTION + //////////////////////////////////////////////////////////////////////////*/ + + function setUp() public virtual override { + Base_Test.setUp(); + + // Deploy the comptroller and its handler. + comptroller = new SablierV2Comptroller({ initialAdmin: users.admin }); + comptrollerHandler = new ComptrollerHandler(comptroller); + vm.prank({ msgSender: users.admin }); + comptroller.transferAdmin(address(comptrollerHandler)); + + // Target only the comptroller handler for invariant testing (to avoid getting reverts). + targetContract(address(comptrollerHandler)); + + // Label the base contracts. + vm.label({ account: address(comptroller), newLabel: "Comptroller" }); + vm.label({ account: address(comptrollerHandler), newLabel: "ComptrollerHandler" }); + } +} diff --git a/test/invariant/handlers/BaseHandler.t.sol b/test/invariant/handlers/BaseHandler.t.sol new file mode 100644 index 000000000..2fc6deace --- /dev/null +++ b/test/invariant/handlers/BaseHandler.t.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { StdCheats } from "forge-std/StdCheats.sol"; +import { Vm } from "@prb/test/Vm.sol"; + +import { Calculations } from "../../shared/helpers/Calculations.t.sol"; +import { Constants } from "../../shared/helpers/Constants.t.sol"; +import { Utils } from "../../shared/helpers/Utils.t.sol"; + +/// @title BaseHandler +/// @notice Base contract with common logic needed by all handler contracts. +abstract contract BaseHandler is Constants, Calculations, Utils, StdCheats { + /*////////////////////////////////////////////////////////////////////////// + CONSTANTS + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev The address of the HEVM contract. + address internal constant HEVM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code")))); + + /*////////////////////////////////////////////////////////////////////////// + TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev An instance of the HEVM. + Vm internal constant vm = Vm(HEVM_ADDRESS); + + /*////////////////////////////////////////////////////////////////////////// + TEST VARIABLES + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Maps function names to the number of times they have been called. + mapping(string => uint256) public calls; + + /// @dev The total number of calls made to this contract. + uint256 public totalCalls; + + /*////////////////////////////////////////////////////////////////////////// + MODIFIERS + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Records a function call for instrumentation purposes. + modifier instrument(string memory func) { + calls[func]++; + totalCalls++; + _; + } +} diff --git a/test/invariant/handlers/ComptrollerHandler.t.sol b/test/invariant/handlers/ComptrollerHandler.t.sol new file mode 100644 index 000000000..a98b1d3ad --- /dev/null +++ b/test/invariant/handlers/ComptrollerHandler.t.sol @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; +import { UD60x18, UNIT } from "@prb/math/UD60x18.sol"; + +import { ISablierV2Comptroller } from "src/interfaces/ISablierV2Comptroller.sol"; + +import { BaseHandler } from "./BaseHandler.t.sol"; + +/// @title ComptrollerHandler +/// @dev This contract and not {SablierV2Comptroller} is exposed to Foundry for invariant testing. The point is +/// to bound and restrict the inputs that get passed to the real-world contract to avoid getting reverts. +contract ComptrollerHandler is BaseHandler { + /*////////////////////////////////////////////////////////////////////////// + TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + ISablierV2Comptroller public comptroller; + + /*////////////////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////////////////*/ + + constructor(ISablierV2Comptroller comptroller_) { + comptroller = comptroller_; + } + + /*////////////////////////////////////////////////////////////////////////// + SABLIER-V2-COMPTROLLER + //////////////////////////////////////////////////////////////////////////*/ + + function setFlashFee(UD60x18 newFlashFee) external instrument("setFlashFee") { + newFlashFee = bound(newFlashFee, 0, UNIT); + comptroller.setFlashFee(newFlashFee); + } + + function setProtocolFee(IERC20 asset, UD60x18 newProtocolFee) external instrument("setProtocolFee") { + newProtocolFee = bound(newProtocolFee, 0, DEFAULT_MAX_FEE); + comptroller.setProtocolFee(asset, newProtocolFee); + } + + function toggleFlashAsset(IERC20 asset) external instrument("toggleFlashAsset") { + comptroller.toggleFlashAsset(asset); + } +} diff --git a/test/invariant/handlers/FlashLoanHandler.t.sol b/test/invariant/handlers/FlashLoanHandler.t.sol new file mode 100644 index 000000000..e50b80b07 --- /dev/null +++ b/test/invariant/handlers/FlashLoanHandler.t.sol @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; +import { IERC3156FlashBorrower } from "erc3156/interfaces/IERC3156FlashBorrower.sol"; +import { UD60x18 } from "@prb/math/UD60x18.sol"; + +import { SablierV2FlashLoan } from "src/abstracts/SablierV2FlashLoan.sol"; +import { ISablierV2Comptroller } from "src/interfaces/ISablierV2Comptroller.sol"; + +import { GoodFlashLoanReceiver } from "../../shared/mockups/flash-loan/GoodFlashLoanReceiver.t.sol"; +import { BaseHandler } from "./BaseHandler.t.sol"; + +/// @title FlashLoanHandler +/// @dev This contract and not {SablierV2FlashLoan} is exposed to Foundry for invariant testing. The point is +/// to bound and restrict the inputs that get passed to the real-world contract to avoid getting reverts. +contract FlashLoanHandler is BaseHandler { + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + IERC20 public asset; + ISablierV2Comptroller public comptroller; + SablierV2FlashLoan public flashLoan; + IERC3156FlashBorrower internal receiver; + + /*////////////////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////////////////*/ + + constructor( + IERC20 asset_, + ISablierV2Comptroller comptroller_, + SablierV2FlashLoan flashLoan_, + IERC3156FlashBorrower receiver_ + ) { + asset = asset_; + comptroller = comptroller_; + flashLoan = flashLoan_; + receiver = receiver_; + } + + /*////////////////////////////////////////////////////////////////////////// + SABLIER-V2-FLASH-LOAN + //////////////////////////////////////////////////////////////////////////*/ + + function flashLoanFunction(uint128 amount) external instrument("flashLoan") { + // Only supported ERC-20 assets can be flash loaned. + bool isFlashLoanable = comptroller.isFlashLoanable(asset); + if (!isFlashLoanable) { + return; + } + + // The flash fee must be less than or equal to type(uint128).max + uint256 fee = flashLoan.flashFee(address(asset), amount); + if (fee > type(uint128).max) { + return; + } + + // Mint the flash loan amount to the contract. + deal({ token: address(asset), to: address(flashLoan), give: amount }); + + // Mint the flash fee to the receiver so that they can repay the flash loan. + deal({ token: address(asset), to: address(receiver), give: fee }); + + // Execute the flash loan. + bool response = flashLoan.flashLoan({ + receiver: receiver, + asset: address(asset), + amount: amount, + data: bytes("Some Data") + }); + + // Silence the compiler warning. + response; + } +} diff --git a/test/invariant/handlers/LockupHandler.t.sol b/test/invariant/handlers/LockupHandler.t.sol new file mode 100644 index 000000000..7f514ad91 --- /dev/null +++ b/test/invariant/handlers/LockupHandler.t.sol @@ -0,0 +1,233 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; + +import { ISablierV2Lockup } from "src/interfaces/ISablierV2Lockup.sol"; +import { Broker, Lockup } from "src/types/DataTypes.sol"; + +import { BaseHandler } from "./BaseHandler.t.sol"; + +/// @title LockupHandler +/// @dev Common handler logic between {SablierV2LockupLinear} and {SablierV2LockupPro}. +abstract contract LockupHandler is BaseHandler { + /*////////////////////////////////////////////////////////////////////////// + CONSTANTS + //////////////////////////////////////////////////////////////////////////*/ + + uint256 internal constant MAX_STREAM_COUNT = 100; + + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + ISablierV2Lockup public lockup; + + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST VARIABLES + //////////////////////////////////////////////////////////////////////////*/ + + address public admin; + IERC20 public asset; + uint256 public lastStreamId; + uint128 public returnedAmountsSum; + mapping(uint256 => address) public streamIdsToRecipients; + mapping(uint256 => address) public streamIdsToSenders; + uint256[] public streamIds; + + /*////////////////////////////////////////////////////////////////////////// + PRIVATE TEST VARIABLES + //////////////////////////////////////////////////////////////////////////*/ + + address internal currentRecipient; + address internal currentSender; + uint256 internal currentStreamId; + + /*////////////////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////////////////*/ + + constructor(address admin_, IERC20 asset_, ISablierV2Lockup linear_) { + admin = admin_; + asset = asset_; + lockup = linear_; + } + + /*////////////////////////////////////////////////////////////////////////// + MODIFIERS + //////////////////////////////////////////////////////////////////////////*/ + + modifier useAdmin() { + vm.startPrank(admin); + _; + vm.stopPrank(); + } + + modifier useFuzzedStreamRecipient(uint256 streamIndexSeed) { + if (lastStreamId == 0) { + return; + } + currentStreamId = streamIds[bound(streamIndexSeed, 0, lastStreamId - 1)]; + currentRecipient = streamIdsToRecipients[currentStreamId]; + vm.startPrank(currentRecipient); + _; + vm.stopPrank(); + } + + modifier useFuzzedStreamSender(uint256 streamIndexSeed) { + if (lastStreamId == 0) { + return; + } + currentStreamId = streamIds[bound(streamIndexSeed, 0, lastStreamId - 1)]; + currentSender = streamIdsToSenders[currentStreamId]; + vm.startPrank(currentSender); + _; + vm.stopPrank(); + } + + modifier useNewSender(address sender) { + vm.startPrank(sender); + _; + vm.stopPrank(); + } + + /*////////////////////////////////////////////////////////////////////////// + SABLIER-V2-LOCKUP + //////////////////////////////////////////////////////////////////////////*/ + + function burn(uint256 streamIndexSeed) external instrument("burn") useFuzzedStreamRecipient(streamIndexSeed) { + // Only canceled and depleted streams can be burned. + Lockup.Status status = lockup.getStatus(currentStreamId); + if (status != Lockup.Status.CANCELED && status != Lockup.Status.DEPLETED) { + return; + } + + // Only NFTs that still exist can be burned. + if (currentRecipient == address(0)) { + return; + } + + // Burn the NFT. + lockup.burn(currentStreamId); + + // Update the recipient associated with this stream id. + streamIdsToRecipients[currentStreamId] = address(0); + } + + function cancel(uint256 streamIndexSeed) external instrument("cancel") useFuzzedStreamSender(streamIndexSeed) { + // Non-cancelable streams cannot be canceled. + bool isCancelable = lockup.isCancelable(currentStreamId); + if (!isCancelable) { + return; + } + + // Record the returned amount by adding it to the ghost variable `returnedAmountsSum`. This is needed to + // check invariants against the contract's balance. + uint128 returnedAmount = lockup.returnableAmountOf(currentStreamId); + returnedAmountsSum += returnedAmount; + + // Cancel the stream. + lockup.cancel(currentStreamId); + } + + function claimProtocolRevenues() external instrument("claimProtocolRevenues") useAdmin { + // Can claim revenues only if the protocol has revenues. + uint128 protocolRevenues = lockup.getProtocolRevenues(asset); + if (protocolRevenues == 0) { + return; + } + + // Claim the revenues. + lockup.claimProtocolRevenues(asset); + } + + function renounce(uint256 streamIndexSeed) external instrument("renounce") useFuzzedStreamSender(streamIndexSeed) { + // Non-cancelable streams cannot be renounced. + bool isCancelable = lockup.isCancelable(currentStreamId); + if (!isCancelable) { + return; + } + + // Renounce the stream (make it non-cancelable). + lockup.renounce(currentStreamId); + } + + function withdraw( + uint256 streamIndexSeed, + address to, + uint128 withdrawAmount + ) external instrument("withdraw") useFuzzedStreamRecipient(streamIndexSeed) { + // The protocol doesn't allow the `to` address to be the zero address. + if (to == address(0)) { + return; + } + + // The protocol doesn't allow a zero amount to be withdrawn. + uint128 withdrawableAmount = lockup.withdrawableAmountOf(currentStreamId); + if (withdrawableAmount == 0) { + return; + } + + // Bound the withdraw amount so that it is not zero. + withdrawAmount = boundUint128(withdrawAmount, 1, withdrawableAmount); + + // Non-active streams cannot be withdrawn from. + Lockup.Status status = lockup.getStatus(currentStreamId); + if (status != Lockup.Status.ACTIVE) { + return; + } + + // Renounce the stream (make it non-cancelable). + lockup.withdraw({ streamId: currentStreamId, to: to, amount: withdrawAmount }); + } + + function withdrawMax( + uint256 streamIndexSeed, + address to + ) external instrument("withdrawMax") useFuzzedStreamRecipient(streamIndexSeed) { + // The protocol doesn't allow the `to` address to be the zero address. + if (to == address(0)) { + return; + } + + // The protocol doesn't allow a zero amount to be withdrawn. + uint128 withdrawableAmount = lockup.withdrawableAmountOf(currentStreamId); + if (withdrawableAmount == 0) { + return; + } + + // Non-active streams cannot be withdrawn from. + Lockup.Status status = lockup.getStatus(currentStreamId); + if (status != Lockup.Status.ACTIVE) { + return; + } + + // Renounce the stream (make it non-cancelable). + lockup.withdrawMax({ streamId: currentStreamId, to: to }); + } + + /*////////////////////////////////////////////////////////////////////////// + ERC-721 + //////////////////////////////////////////////////////////////////////////*/ + + function transferNFT( + uint256 streamIndexSeed, + address newRecipient + ) external instrument("transferNFT") useFuzzedStreamRecipient(streamIndexSeed) { + // The ERC-721 contract doesn't allow the new recipient to be the zero address. + if (newRecipient == address(0)) { + return; + } + + // Only NFTs that still exist can be transferred. + if (currentRecipient == address(0)) { + return; + } + + // Transfer the NFT to the new recipient. + lockup.transferFrom({ from: currentRecipient, to: newRecipient, tokenId: currentStreamId }); + + // Update the recipient associated with this stream id. + streamIdsToRecipients[currentStreamId] = newRecipient; + } +} diff --git a/test/invariant/handlers/LockupLinearHandler.t.sol b/test/invariant/handlers/LockupLinearHandler.t.sol new file mode 100644 index 000000000..03ab99459 --- /dev/null +++ b/test/invariant/handlers/LockupLinearHandler.t.sol @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; + +import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol"; +import { Broker, LockupLinear } from "src/types/DataTypes.sol"; + +import { LockupHandler } from "./LockupHandler.t.sol"; + +/// @title LockupLinearHandler +/// @dev This contract and not {SablierV2LockupLinear} is exposed to Foundry for invariant testing. The point is +/// to bound and restrict the inputs that get passed to the real-world contract to avoid getting reverts. +contract LockupLinearHandler is LockupHandler { + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + ISablierV2LockupLinear public linear; + + /*////////////////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////////////////*/ + + constructor(address admin_, IERC20 asset_, ISablierV2LockupLinear linear_) LockupHandler(admin_, asset_, linear_) { + linear = linear_; + } + + /*////////////////////////////////////////////////////////////////////////// + FUNCTIONS + //////////////////////////////////////////////////////////////////////////*/ + + struct CreateWithDurationsParams { + Broker broker; + bool cancelable; + LockupLinear.Durations durations; + address recipient; + address sender; + uint128 totalAmount; + } + + struct CreateWithDurationsVars { + uint256 streamId; + } + + function createWithDurations( + CreateWithDurationsParams memory params + ) public instrument("createWithDurations") useNewSender(params.sender) { + params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); + params.durations.cliff = boundUint40(params.durations.cliff, 1, 1_000); + params.durations.total = boundUint40(params.durations.total, params.durations.cliff + 1, MAX_UNIX_TIMESTAMP); + params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); + + // We don't want to fuzz more than a certain number of streams. + if (streamIds.length > MAX_STREAM_COUNT) { + return; + } + + // The protocol doesn't allow the sender, recipient or broker to be the zero address. + if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { + return; + } + + // Mint enough ERC-20 assets to the sender. + deal({ token: address(asset), to: params.sender, give: params.totalAmount }); + + // Approve the {SablierV2LockupLinear} contract to spend the ERC-20 assets. + asset.approve({ spender: address(linear), amount: params.totalAmount }); + + // Create the stream. + CreateWithDurationsVars memory vars; + vars.streamId = linear.createWithDurations({ + sender: params.sender, + recipient: params.recipient, + totalAmount: params.totalAmount, + asset: asset, + cancelable: params.cancelable, + durations: params.durations, + broker: params.broker + }); + + // Store the stream id in the ids array and the reverse mapping. + streamIds.push(vars.streamId); + streamIdsToRecipients[vars.streamId] = params.recipient; + streamIdsToSenders[vars.streamId] = params.sender; + + // Update the last stream id. + lastStreamId = vars.streamId; + } + + /// @dev This function exists only to bias the invariant calls toward {createWithDurations}, so that more streams + /// get created. + function createWithDurations_Bias(CreateWithDurationsParams memory params) external { + createWithDurations(params); + } + + struct CreateWithRangeParams { + Broker broker; + bool cancelable; + LockupLinear.Range range; + address recipient; + address sender; + uint128 totalAmount; + } + + struct CreateWithRangeVars { + uint256 streamId; + } + + function createWithRange( + CreateWithRangeParams memory params + ) public instrument("createWithRange") useNewSender(params.sender) { + params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); + params.range.start = boundUint40(params.range.start, 0, 1_000); + params.range.cliff = boundUint40(params.range.cliff, params.range.start, 5_000); + params.range.end = boundUint40(params.range.end, params.range.cliff + 1, MAX_UNIX_TIMESTAMP); + params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); + + // We don't want to fuzz more than a certain number of streams. + if (streamIds.length > MAX_STREAM_COUNT) { + return; + } + + // The protocol doesn't allow the sender, recipient or broker to be the zero address. + if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { + return; + } + + // Mint enough ERC-20 assets to the sender. + deal({ token: address(asset), to: params.sender, give: params.totalAmount }); + + // Approve the {SablierV2LockupLinear} contract to spend the ERC-20 assets. + asset.approve({ spender: address(linear), amount: params.totalAmount }); + + // Create the stream. + CreateWithRangeVars memory vars; + vars.streamId = linear.createWithRange({ + sender: params.sender, + recipient: params.recipient, + totalAmount: params.totalAmount, + asset: asset, + cancelable: params.cancelable, + range: params.range, + broker: params.broker + }); + + // Store the stream id in the ids array and the reverse mapping. + streamIds.push(vars.streamId); + streamIdsToRecipients[vars.streamId] = params.recipient; + streamIdsToSenders[vars.streamId] = params.sender; + + // Update the last stream id. + lastStreamId = vars.streamId; + } + + /// @dev This function exists only to bias the invariant calls toward {createWithRange}, so that more streams + /// get created. + function createWithRange_Bias(CreateWithRangeParams memory params) external { + createWithRange(params); + } +} diff --git a/test/invariant/handlers/LockupProHandler.t.sol b/test/invariant/handlers/LockupProHandler.t.sol new file mode 100644 index 000000000..5cefc7cc6 --- /dev/null +++ b/test/invariant/handlers/LockupProHandler.t.sol @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; +import { UD60x18 } from "@prb/math/UD60x18.sol"; +import { Solarray } from "solarray/Solarray.sol"; + +import { ISablierV2Comptroller } from "src/interfaces/ISablierV2Comptroller.sol"; +import { ISablierV2LockupPro } from "src/interfaces/ISablierV2LockupPro.sol"; +import { Broker, LockupPro } from "src/types/DataTypes.sol"; + +import { LockupHandler } from "./LockupHandler.t.sol"; + +/// @title LockupProHandler +/// @dev This contract and not {SablierV2LockupPro} is exposed to Foundry for invariant testing. The point is +/// to bound and restrict the inputs that get passed to the real-world contract to avoid getting reverts. +contract LockupProHandler is LockupHandler { + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + ISablierV2Comptroller public comptroller; + ISablierV2LockupPro public pro; + + /*////////////////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////////////////*/ + + constructor( + address admin_, + IERC20 asset_, + ISablierV2Comptroller comptroller_, + ISablierV2LockupPro pro_ + ) LockupHandler(admin_, asset_, pro_) { + comptroller = comptroller_; + pro = pro_; + } + + /*////////////////////////////////////////////////////////////////////////// + FUNCTIONS + //////////////////////////////////////////////////////////////////////////*/ + + struct CreateWithDeltasParams { + Broker broker; + bool cancelable; + uint40 delta0; + uint40 delta1; + address recipient; + address sender; + uint128 totalAmount; + } + + struct CreateWithDeltasVars { + uint256 streamId; + uint40[] deltas; + UD60x18 protocolFee; + uint128 depositAmount; + LockupPro.Segment[] segments; + } + + function createWithDeltas( + CreateWithDeltasParams memory params + ) public instrument("createWithDeltas") useNewSender(params.sender) { + params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); + params.delta0 = boundUint40(params.delta0, 1, 100); + params.delta1 = boundUint40(params.delta1, 1, MAX_UNIX_TIMESTAMP - uint40(block.timestamp) - params.delta0); + params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); + + // We don't want to fuzz more than a certain number of streams. + if (streamIds.length > MAX_STREAM_COUNT) { + return; + } + + // The protocol doesn't allow the sender, recipient or broker to be the zero address. + if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { + return; + } + + // Create the deltas array. + CreateWithDeltasVars memory vars; + vars.deltas = Solarray.uint40s(params.delta0, params.delta1); + + // Adjust the segment milestones to match the fuzzed deltas. + vars.segments = DEFAULT_SEGMENTS; + vars.segments[0].milestone = uint40(block.timestamp) + params.delta0; + vars.segments[1].milestone = vars.segments[0].milestone + params.delta1; + + // Calculate the deposit amount. + vars.depositAmount = calculateDepositAmount(params.totalAmount, vars.protocolFee, params.broker.fee); + + // Adjust the segment amounts based on the fuzzed deposit amount. + adjustSegmentAmounts(vars.segments, vars.depositAmount); + + // Mint enough ERC-20 assets to the sender. + deal({ token: address(asset), to: params.sender, give: params.totalAmount }); + + // Approve the {SablierV2LockupPro} contract to spend the ERC-20 assets. + asset.approve({ spender: address(pro), amount: params.totalAmount }); + + // Create the stream. + vars.streamId = pro.createWithDeltas({ + sender: params.sender, + recipient: params.recipient, + totalAmount: params.totalAmount, + segments: vars.segments, + asset: asset, + cancelable: params.cancelable, + deltas: vars.deltas, + broker: params.broker + }); + + // Store the stream id in the ids array and the reverse mapping. + streamIds.push(vars.streamId); + streamIdsToRecipients[vars.streamId] = params.recipient; + streamIdsToSenders[vars.streamId] = params.sender; + + // Update the last stream id. + lastStreamId = vars.streamId; + } + + /// @dev This function exists only to bias the invariant calls toward {createWithDeltas}, so that more streams + /// get created. + function createWithDeltas_Bias(CreateWithDeltasParams memory params) external { + createWithDeltas(params); + } + + struct CreateWithMilestonesParams { + Broker broker; + bool cancelable; + uint128 totalAmount; + address recipient; + address sender; + uint40 startTime; + } + + struct CreateWithMilestonesVars { + uint128 depositAmount; + UD60x18 protocolFee; + LockupPro.Segment[] segments; + uint256 streamId; + } + + function createWithMilestones( + CreateWithMilestonesParams memory params + ) public instrument("createWithMilestones") useNewSender(params.sender) { + params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); + params.startTime = boundUint40(params.startTime, 0, DEFAULT_SEGMENTS[0].milestone - 1); + params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); + + // We don't want to fuzz more than a certain number of streams. + if (streamIds.length > MAX_STREAM_COUNT) { + return; + } + + // The protocol doesn't allow the sender, recipient or broker to be the zero address. + if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { + return; + } + + // Get the current protocol fee. + CreateWithMilestonesVars memory vars; + vars.protocolFee = comptroller.getProtocolFee(asset); + + // Calculate the deposit amount. + vars.depositAmount = calculateDepositAmount(params.totalAmount, vars.protocolFee, params.broker.fee); + + // Adjust the segment amounts based on the fuzzed deposit amount. + vars.segments = DEFAULT_SEGMENTS; + adjustSegmentAmounts(vars.segments, vars.depositAmount); + + // Mint enough ERC-20 assets to the sender. + deal({ token: address(asset), to: params.sender, give: params.totalAmount }); + + // Approve the {SablierV2LockupPro} contract to spend the ERC-20 assets. + asset.approve({ spender: address(pro), amount: params.totalAmount }); + + // Create the stream. + vars.streamId = pro.createWithMilestones({ + sender: params.sender, + recipient: params.recipient, + totalAmount: params.totalAmount, + segments: vars.segments, + asset: asset, + cancelable: params.cancelable, + startTime: params.startTime, + broker: params.broker + }); + + // Store the stream id in the ids array and the reverse mapping. + streamIds.push(vars.streamId); + streamIdsToRecipients[vars.streamId] = params.recipient; + streamIdsToSenders[vars.streamId] = params.sender; + + // Update the last stream id. + lastStreamId = vars.streamId; + } + + /// @dev This function exists only to bias the invariant calls toward {createWithMilestones}, so that more streams + /// get created. + function createWithMilestones_Bias(CreateWithMilestonesParams memory params) external { + createWithMilestones(params); + } +} diff --git a/test/invariant/lockup/Lockup.t.sol b/test/invariant/lockup/Lockup.t.sol new file mode 100644 index 000000000..15b342107 --- /dev/null +++ b/test/invariant/lockup/Lockup.t.sol @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { SablierV2FlashLoan } from "src/abstracts/SablierV2FlashLoan.sol"; +import { ISablierV2Lockup } from "src/interfaces/ISablierV2Lockup.sol"; +import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol"; +import { SablierV2LockupLinear } from "src/SablierV2LockupLinear.sol"; + +import { Invariant_Test } from "../Invariant.t.sol"; +import { FlashLoanHandler } from "../handlers/FlashLoanHandler.t.sol"; +import { LockupHandler } from "../handlers/LockupHandler.t.sol"; + +/// @title Lockup_Invariant_Test +/// @notice Common invariant test logic needed across contracts that inherit from {SablierV2Lockup}. +abstract contract Lockup_Invariant_Test is Invariant_Test { + /*////////////////////////////////////////////////////////////////////////// + TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + FlashLoanHandler internal flashLoanHandler; + ISablierV2Lockup internal lockup; + LockupHandler internal lockupHandler; + + /*////////////////////////////////////////////////////////////////////////// + SET-UP FUNCTION + //////////////////////////////////////////////////////////////////////////*/ + + function setUp() public virtual override { + Invariant_Test.setUp(); + } + + /*////////////////////////////////////////////////////////////////////////// + INVARIANTS + //////////////////////////////////////////////////////////////////////////*/ + + // solhint-disable max-line-length + function invariant_ContractBalance() external { + uint256 contractBalance = DEFAULT_ASSET.balanceOf(address(lockup)); + uint256 protocolRevenues = lockup.getProtocolRevenues(DEFAULT_ASSET); + uint256 returnedAmountsSum = lockupHandler.returnedAmountsSum(); + + uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 depositAmountsSum; + uint256 withdrawnAmountsSum; + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = lockupHandler.streamIds(i); + depositAmountsSum += uint256(lockup.getDepositAmount(streamId)); + withdrawnAmountsSum += uint256(lockup.getWithdrawnAmount(streamId)); + unchecked { + i += 1; + } + } + + assertGte( + contractBalance, + depositAmountsSum + protocolRevenues - returnedAmountsSum - withdrawnAmountsSum, + unicode"Invariant violated: contract balances < Σ deposit amounts + protocol revenues - Σ returned amounts - Σ withdrawn amounts" + ); + } + + function invariant_DepositAmountGteStreamedAmount() external { + uint256 lastStreamId = lockupHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = lockupHandler.streamIds(i); + assertGte( + lockup.getDepositAmount(streamId), + lockup.streamedAmountOf(streamId), + "Invariant violated: deposit amount < streamed amount" + ); + unchecked { + i += 1; + } + } + } + + function invariant_DepositAmountGteWithdrawableAmount() external { + uint256 lastStreamId = lockupHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = lockupHandler.streamIds(i); + assertGte( + lockup.getDepositAmount(streamId), + lockup.withdrawableAmountOf(streamId), + "Invariant violated: deposit amount < withdrawable amount" + ); + unchecked { + i += 1; + } + } + } + + function invariant_DepositAmountGteWithdrawnAmount() external { + uint256 lastStreamId = lockupHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = lockupHandler.streamIds(i); + assertGte( + lockup.getDepositAmount(streamId), + lockup.getWithdrawnAmount(streamId), + "Invariant violated: deposit amount < withdrawn amount" + ); + unchecked { + i += 1; + } + } + } + + function invariant_EndTimeGteStartTime() external { + uint256 lastStreamId = lockupHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = lockupHandler.streamIds(i); + assertGte( + lockup.getEndTime(streamId), + lockup.getStartTime(streamId), + "Invariant violated: end time < start time" + ); + unchecked { + i += 1; + } + } + } + + function invariant_NextStreamIdIncrement() external { + uint256 lastStreamId = lockupHandler.lastStreamId(); + for (uint256 i = 1; i < lastStreamId; ) { + uint256 nextStreamId = lockup.nextStreamId(); + assertEq(nextStreamId, lastStreamId + 1, "Invariant violated: nonce did not increment"); + unchecked { + i += 1; + } + } + } + + function invariant_StreamedAmountGteWithdrawableAmount() external { + uint256 lastStreamId = lockupHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = lockupHandler.streamIds(i); + assertGte( + lockup.streamedAmountOf(streamId), + lockup.withdrawableAmountOf(streamId), + "Invariant violated: streamed amount < withdrawable amount" + ); + unchecked { + i += 1; + } + } + } + + function invariant_StreamedAmountGteWithdrawnAmount() external { + uint256 lastStreamId = lockupHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = lockupHandler.streamIds(i); + assertGte( + lockup.streamedAmountOf(streamId), + lockup.getWithdrawnAmount(streamId), + "Invariant violated: streamed amount < withdrawn amount" + ); + unchecked { + i += 1; + } + } + } +} diff --git a/test/invariant/lockup/linear/Linear.t.sol b/test/invariant/lockup/linear/Linear.t.sol new file mode 100644 index 000000000..41007f88f --- /dev/null +++ b/test/invariant/lockup/linear/Linear.t.sol @@ -0,0 +1,160 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { console2 } from "forge-std/console2.sol"; + +import { SablierV2FlashLoan } from "src/abstracts/SablierV2FlashLoan.sol"; +import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol"; +import { SablierV2LockupLinear } from "src/SablierV2LockupLinear.sol"; +import { Lockup, LockupLinear } from "src/types/DataTypes.sol"; + +import { Lockup_Invariant_Test } from "../Lockup.t.sol"; +import { FlashLoanHandler } from "../../handlers/FlashLoanHandler.t.sol"; +import { LockupLinearHandler } from "../../handlers/LockupLinearHandler.t.sol"; + +/// @title Linear_Invariant_Test +/// @dev Invariants for the {SablierV2LockupLinear} contract. +contract Linear_Invariant_Test is Lockup_Invariant_Test { + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + LockupLinearHandler internal linearHandler; + + /*////////////////////////////////////////////////////////////////////////// + SET-UP FUNCTION + //////////////////////////////////////////////////////////////////////////*/ + + function setUp() public virtual override { + Lockup_Invariant_Test.setUp(); + + // Deploy the linear contract and its handler. + linear = new SablierV2LockupLinear({ + initialAdmin: users.admin, + initialComptroller: comptroller, + maxFee: DEFAULT_MAX_FEE + }); + linearHandler = new LockupLinearHandler({ admin_: users.admin, asset_: DEFAULT_ASSET, linear_: linear }); + + // Cast the linear contract as {SablierV2Lockup} and the linear handler as {LockupHandler}. + lockup = linear; + lockupHandler = linearHandler; + + // Deploy the flash loan handler by casting the linear contract as {SablierV2FlashLoan}. + flashLoanHandler = new FlashLoanHandler({ + asset_: DEFAULT_ASSET, + comptroller_: comptroller, + flashLoan_: SablierV2FlashLoan(address(linear)), + receiver_: goodFlashLoanReceiver + }); + + // Target the flash loan handler and the linear handler for invariant testing. + targetContract(address(flashLoanHandler)); + targetContract(address(linearHandler)); + + // Label the linear contract and its handler. + vm.label({ account: address(linear), newLabel: "LockupLinear" }); + vm.label({ account: address(linearHandler), newLabel: "LockupLinearHandler" }); + } + + /*////////////////////////////////////////////////////////////////////////// + INVARIANTS + //////////////////////////////////////////////////////////////////////////*/ + + // prettier-ignore + // solhint-disable max-line-length + function invariant_NullStatus() external { + uint256 lastStreamId = linearHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId;) { + uint256 streamId = linearHandler.streamIds(i); + LockupLinear.Stream memory actualStream = linear.getStream(streamId); + address actualRecipient = lockup.getRecipient(streamId); + + // If the stream is null, it should contain only zero values. + if (lockup.getStatus(streamId) == Lockup.Status.NULL) { + assertEq(actualStream.amounts.deposit, 0, "Invariant violated: stream null, deposit amount not zero"); + assertEq( actualStream.amounts.withdrawn, 0, "Invariant violated: stream null, withdrawn amount not zero"); + assertEq(address(actualStream.asset), address(0), "Invariant violated: stream null, asset not zero address"); + assertEq(actualStream.isCancelable, false, "Invariant violated: stream null, isCancelable not false"); + assertEq(actualStream.range.cliff, 0, "Invariant violated: stream null, cliff time not zero"); + assertEq(actualStream.range.end, 0, "Invariant violated: stream null, end time not zero"); + assertEq(actualStream.range.start, 0, "Invariant violated: stream null, start time not zero"); + assertEq(actualStream.sender, address(0), "Invariant violated: stream null, sender not zero address"); + assertEq(actualRecipient, address(0), "Invariant violated: stream null, recipient not zero address"); + } + // If the stream is not null, it should contain a non-zero deposit amount. + else { + assertNotEq(actualStream.amounts.deposit, 0, "Invariant violated: stream non-null, deposit amount zero"); + assertNotEq(actualStream.range.end, 0, "Invariant violated: stream non-null, end time zero"); + } + unchecked { + i += 1; + } + } + } + + function invariant_CliffTimeGteStartTime() external { + uint256 lastStreamId = linearHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = linearHandler.streamIds(i); + assertGte( + linear.getCliffTime(streamId), + linear.getStartTime(streamId), + "Invariant violated: cliff time < start time" + ); + unchecked { + i += 1; + } + } + } + + function invariant_EndTimeGteCliffTime() external { + uint256 lastStreamId = linearHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = linearHandler.streamIds(i); + assertGte( + linear.getEndTime(streamId), + linear.getCliffTime(streamId), + "Invariant violated: end time < cliff time" + ); + unchecked { + i += 1; + } + } + } + + /*////////////////////////////////////////////////////////////////////////// + CALL SUMMARY + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Mark this function as `external` to enable call summaries. + function invariant_CallSummary() external onlyInCI { + console2.log("\nCall Summary\n"); + console2.log("Comptroller"); + console2.log("setFlashFee ", comptrollerHandler.calls("setFlashFee")); + console2.log("setProtocolFee ", comptrollerHandler.calls("setProtocolFee")); + console2.log("toggleFlashAsset ", comptrollerHandler.calls("toggleFlashAsset")); + console2.log("\n ------------------------\n"); + + console2.log("FlashLoan"); + console2.log("flashLoan ", flashLoanHandler.calls("flashLoan")); + console2.log("\n ------------------------\n"); + + console2.log("LockupLinear"); + console2.log("burn ", linearHandler.calls("burn")); + console2.log("cancel ", linearHandler.calls("cancel")); + console2.log("claimProtocolRevenues", linearHandler.calls("claimProtocolRevenues")); + console2.log("createWithRange ", linearHandler.calls("createWithRange")); + console2.log("createWithDurations ", linearHandler.calls("createWithDurations")); + console2.log("renounce ", linearHandler.calls("renounce")); + console2.log("transferNFT ", linearHandler.calls("transferNFT")); + console2.log("withdraw ", linearHandler.calls("withdraw")); + console2.log("withdrawMax ", linearHandler.calls("withdrawMax")); + console2.log("\n -----------------------\n"); + + console2.log( + "Total calls: ", + comptrollerHandler.totalCalls() + flashLoanHandler.totalCalls() + linearHandler.totalCalls() + ); + } +} diff --git a/test/invariant/lockup/pro/Pro.t.sol b/test/invariant/lockup/pro/Pro.t.sol new file mode 100644 index 000000000..3d5b14e6f --- /dev/null +++ b/test/invariant/lockup/pro/Pro.t.sol @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { console2 } from "forge-std/console2.sol"; + +import { SablierV2FlashLoan } from "src/abstracts/SablierV2FlashLoan.sol"; +import { ISablierV2LockupPro } from "src/interfaces/ISablierV2LockupPro.sol"; +import { SablierV2LockupPro } from "src/SablierV2LockupPro.sol"; +import { Lockup, LockupPro } from "src/types/DataTypes.sol"; + +import { Lockup_Invariant_Test } from "../Lockup.t.sol"; +import { FlashLoanHandler } from "../../handlers/FlashLoanHandler.t.sol"; +import { LockupProHandler } from "../../handlers/LockupProHandler.t.sol"; + +/// @title Pro_Invariant_Test +/// @dev Invariants for the {SablierV2LockupPro} contract. +contract Pro_Invariant_Test is Lockup_Invariant_Test { + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + LockupProHandler internal proHandler; + + /*////////////////////////////////////////////////////////////////////////// + SET-UP FUNCTION + //////////////////////////////////////////////////////////////////////////*/ + + function setUp() public virtual override { + Lockup_Invariant_Test.setUp(); + + // Deploy the pro contract and its handler. + pro = new SablierV2LockupPro({ + initialAdmin: users.admin, + initialComptroller: comptroller, + maxFee: DEFAULT_MAX_FEE, + maxSegmentCount: DEFAULT_MAX_SEGMENT_COUNT + }); + proHandler = new LockupProHandler({ + admin_: users.admin, + asset_: DEFAULT_ASSET, + comptroller_: comptroller, + pro_: pro + }); + + // Cast the pro contract as {SablierV2Lockup} and the pro handler as {LockupHandler}. + lockup = pro; + lockupHandler = proHandler; + + // Deploy the flash loan handler by casting the pro contract as {SablierV2FlashLoan}. + flashLoanHandler = new FlashLoanHandler({ + asset_: DEFAULT_ASSET, + comptroller_: comptroller, + flashLoan_: SablierV2FlashLoan(address(pro)), + receiver_: goodFlashLoanReceiver + }); + + // Target the flash loan handler and the pro handler for invariant testing. + targetContract(address(flashLoanHandler)); + targetContract(address(proHandler)); + + // Label the pro contract and its handler. + vm.label({ account: address(pro), newLabel: "LockupPro" }); + vm.label({ account: address(lockupHandler), newLabel: "LockupProHandler" }); + } + + /*////////////////////////////////////////////////////////////////////////// + INVARIANTS + //////////////////////////////////////////////////////////////////////////*/ + + // prettier-ignore + // solhint-disable max-line-length + function invariant_NullStatus() external { + uint256 lastStreamId = proHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ) { + uint256 streamId = proHandler.streamIds(i); + LockupPro.Stream memory actualStream = pro.getStream(streamId); + address actualRecipient = lockup.getRecipient(streamId); + + // If the stream is null, it should contain only zero values. + if (lockup.getStatus(streamId) == Lockup.Status.NULL) { + assertEq(actualStream.amounts.deposit, 0, "Invariant violated: stream null, deposit amount not zero"); + assertEq( actualStream.amounts.withdrawn, 0, "Invariant violated: stream null, withdrawn amount not zero"); + assertEq(address(actualStream.asset), address(0), "Invariant violated: stream null, asset not zero address"); + assertEq(actualStream.range.end, 0, "Invariant violated: stream null, end time not zero"); + assertEq(actualStream.range.start, 0, "Invariant violated: stream null, start time not zero"); + assertEq(actualStream.isCancelable, false, "Invariant violated: stream null, isCancelable not false"); + assertEq(actualStream.segments.length, 0, "Invariant violated: stream null, segment count not zero"); + assertEq(actualStream.sender, address(0), "Invariant violated: stream null, sender not zero address"); + assertEq(actualRecipient, address(0), "Invariant violated: stream null, recipient not zero address"); + } + // If the stream is not null, it should contain a non-zero deposit amount. + else { + assertNotEq(actualStream.amounts.deposit, 0, "Invariant violated: stream non-null, deposit amount zero"); + assertNotEq(actualStream.range.end, 0, "Invariant violated: stream non-null, end time zero"); + } + unchecked { + i += 1; + } + } + } + + function invariant_SegmentMilestonesOrdered() external { + unchecked { + uint256 lastStreamId = proHandler.lastStreamId(); + for (uint256 i = 0; i < lastStreamId; ++i) { + uint256 streamId = proHandler.streamIds(i); + + // If the stream is null, it doesn't have segments. + if (pro.getStatus(streamId) != Lockup.Status.NULL) { + continue; + } + + LockupPro.Segment[] memory segments = pro.getSegments(streamId); + uint40 previousMilestone = segments[0].milestone; + for (uint256 j = 1; j < segments.length; ++j) { + assertGt( + segments[j].milestone, + previousMilestone, + "Invariant violated: segment milestones not ordered" + ); + previousMilestone = segments[j].milestone; + } + } + } + } + + /*////////////////////////////////////////////////////////////////////////// + CALL SUMMARY + //////////////////////////////////////////////////////////////////////////*/ + + /// @dev Mark this function as `external` to enable call summaries. + function invariant_CallSummary() external onlyInCI { + console2.log("\nCall Summary\n"); + console2.log("Comptroller"); + console2.log("setFlashFee ", comptrollerHandler.calls("setFlashFee")); + console2.log("setProtocolFee ", comptrollerHandler.calls("setProtocolFee")); + console2.log("toggleFlashAsset ", comptrollerHandler.calls("toggleFlashAsset")); + console2.log("\n ------------------------\n"); + + console2.log("FlashLoan"); + console2.log("flashLoan ", flashLoanHandler.calls("flashLoan")); + console2.log("\n ------------------------\n"); + + console2.log("LockupPro"); + console2.log("burn ", proHandler.calls("burn")); + console2.log("cancel ", proHandler.calls("cancel")); + console2.log("claimProtocolRevenues", proHandler.calls("claimProtocolRevenues")); + console2.log("createWithDeltas ", proHandler.calls("createWithDeltas")); + console2.log("createWithMilestones ", proHandler.calls("createWithMilestones")); + console2.log("renounce ", proHandler.calls("renounce")); + console2.log("transferNFT ", proHandler.calls("transferNFT")); + console2.log("withdraw ", proHandler.calls("withdraw")); + console2.log("withdrawMax ", proHandler.calls("withdrawMax")); + console2.log("\n -----------------------\n"); + + console2.log( + "Total calls: ", + comptrollerHandler.totalCalls() + flashLoanHandler.totalCalls() + proHandler.totalCalls() + ); + } +} diff --git a/test/shared/helpers/Calculations.t.sol b/test/shared/helpers/Calculations.t.sol index 10e350061..d9b1077f8 100644 --- a/test/shared/helpers/Calculations.t.sol +++ b/test/shared/helpers/Calculations.t.sol @@ -25,7 +25,19 @@ abstract contract Calculations is Constants { segments[1].amount = depositAmount - segments[0].amount; } - /// @dev Helper function that replicates the logic of the {SablierV2LockupLinear-getStreamedAmount} function. + /// @dev Calculates the deposit amount by calculating and subtracting the protocol fee amount and the + /// broker fee amount from the total amount. + function calculateDepositAmount( + uint128 totalAmount, + UD60x18 protocolFee, + UD60x18 brokerFee + ) internal pure returns (uint128 depositAmount) { + uint128 protocolFeeAmount = ud(totalAmount).mul(protocolFee).intoUint128(); + uint128 brokerFeeAmount = ud(totalAmount).mul(brokerFee).intoUint128(); + depositAmount = totalAmount - protocolFeeAmount - brokerFeeAmount; + } + + /// @dev Helper function that replicates the logic of the {SablierV2LockupLinear-streamedAmountOf} function. function calculateStreamedAmount( uint40 currentTime, uint128 depositAmount diff --git a/test/shared/helpers/Constants.t.sol b/test/shared/helpers/Constants.t.sol index 975cc6e09..ac9b0584b 100644 --- a/test/shared/helpers/Constants.t.sol +++ b/test/shared/helpers/Constants.t.sol @@ -28,6 +28,7 @@ abstract contract Constants { uint40 internal constant DEFAULT_TOTAL_DURATION = 10_000 seconds; uint128 internal constant DEFAULT_WITHDRAW_AMOUNT = 2_600e18; bytes32 internal constant FLASH_LOAN_CALLBACK_SUCCESS = keccak256("ERC3156FlashBorrower.onFlashLoan"); + uint40 internal constant MAX_UNIX_TIMESTAMP = 2_147_483_647; uint128 internal constant UINT128_MAX = type(uint128).max; uint256 internal constant UINT256_MAX = type(uint256).max; uint40 internal constant UINT40_MAX = type(uint40).max; diff --git a/test/shared/lockup/Lockup.t.sol b/test/shared/lockup/Lockup.t.sol index b63daeece..f5e849c69 100644 --- a/test/shared/lockup/Lockup.t.sol +++ b/test/shared/lockup/Lockup.t.sol @@ -43,7 +43,7 @@ abstract contract Lockup_Shared_Test is Base_Test { function createDefaultStreamWithSender(address sender) internal virtual returns (uint256 streamId); /*////////////////////////////////////////////////////////////////////////// - SETUP FUNCTION + SET-UP FUNCTION //////////////////////////////////////////////////////////////////////////*/ function setUp() public virtual override { diff --git a/test/shared/lockup/linear/Linear.t.sol b/test/shared/lockup/linear/Linear.t.sol index 4c4cb559f..c9fcc2951 100644 --- a/test/shared/lockup/linear/Linear.t.sol +++ b/test/shared/lockup/linear/Linear.t.sol @@ -47,7 +47,7 @@ abstract contract Linear_Shared_Test is Lockup_Shared_Test { DefaultParams internal defaultParams; /*////////////////////////////////////////////////////////////////////////// - SETUP FUNCTION + SET-UP FUNCTION //////////////////////////////////////////////////////////////////////////*/ function setUp() public virtual override { diff --git a/test/shared/lockup/pro/Pro.t.sol b/test/shared/lockup/pro/Pro.t.sol index 0c8753203..d6694806f 100644 --- a/test/shared/lockup/pro/Pro.t.sol +++ b/test/shared/lockup/pro/Pro.t.sol @@ -49,7 +49,7 @@ abstract contract Pro_Shared_Test is Lockup_Shared_Test { DefaultParams internal defaultParams; /*////////////////////////////////////////////////////////////////////////// - SETUP FUNCTION + SET-UP FUNCTION //////////////////////////////////////////////////////////////////////////*/ function setUp() public virtual override { diff --git a/test/shared/mockups/flash-loan/FaultyFlashLoanReceiver.t.sol b/test/shared/mockups/flash-loan/FaultyFlashLoanReceiver.t.sol index 4342ef02e..fea612239 100644 --- a/test/shared/mockups/flash-loan/FaultyFlashLoanReceiver.t.sol +++ b/test/shared/mockups/flash-loan/FaultyFlashLoanReceiver.t.sol @@ -2,7 +2,7 @@ pragma solidity >=0.8.13 <0.9.0; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; -import { IERC3156FlashBorrower } from "erc3156/contracts/interfaces/IERC3156FlashBorrower.sol"; +import { IERC3156FlashBorrower } from "erc3156/interfaces/IERC3156FlashBorrower.sol"; contract FaultyFlashLoanReceiver is IERC3156FlashBorrower { bytes32 internal constant FAULTY_RESPONSE = keccak256("This is a faulty response"); diff --git a/test/shared/mockups/flash-loan/GoodFlashLoanReceiver.t.sol b/test/shared/mockups/flash-loan/GoodFlashLoanReceiver.t.sol index 681813b28..1c6a724b6 100644 --- a/test/shared/mockups/flash-loan/GoodFlashLoanReceiver.t.sol +++ b/test/shared/mockups/flash-loan/GoodFlashLoanReceiver.t.sol @@ -2,7 +2,7 @@ pragma solidity >=0.8.13 <0.9.0; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; -import { IERC3156FlashBorrower } from "erc3156/contracts/interfaces/IERC3156FlashBorrower.sol"; +import { IERC3156FlashBorrower } from "erc3156/interfaces/IERC3156FlashBorrower.sol"; import { Constants } from "../../helpers/Constants.t.sol"; diff --git a/test/shared/mockups/flash-loan/ReentrantFlashLoanReceiver.t.sol b/test/shared/mockups/flash-loan/ReentrantFlashLoanReceiver.t.sol index d28f526b3..290ee15e2 100644 --- a/test/shared/mockups/flash-loan/ReentrantFlashLoanReceiver.t.sol +++ b/test/shared/mockups/flash-loan/ReentrantFlashLoanReceiver.t.sol @@ -2,8 +2,8 @@ pragma solidity >=0.8.13 <0.9.0; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.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 { Constants } from "../../helpers/Constants.t.sol"; diff --git a/test/unit/Unit.t.sol b/test/unit/Unit.t.sol index 95c3a85f1..5f0422ccc 100644 --- a/test/unit/Unit.t.sol +++ b/test/unit/Unit.t.sol @@ -11,7 +11,7 @@ import { RevertingRecipient } from "../shared/mockups/hooks/RevertingRecipient.t import { RevertingSender } from "../shared/mockups/hooks/RevertingSender.t.sol"; /// @title Unit_Test -/// @notice Base unit test contract that contains common logic needed by all unit test contracts. +/// @notice Base unit test contract with common logic needed by all unit test contracts. abstract contract Unit_Test is Base_Test { /*////////////////////////////////////////////////////////////////////////// TEST CONTRACTS @@ -53,10 +53,11 @@ abstract contract Unit_Test is Base_Test { function labelTestContracts() internal { vm.label({ account: address(empty), newLabel: "Empty" }); vm.label({ account: address(faultyFlashLoanReceiver), newLabel: "Faulty Flash Loan Receiver" }); + vm.label({ account: address(nonCompliantAsset), newLabel: "Non-Compliant ERC-20 Asset" }); vm.label({ account: address(reentrantFlashLoanReceiver), newLabel: "Reentrant Flash Loan Receiver" }); - vm.label({ account: address(reentrantRecipient), newLabel: "Reentrant Recipient" }); - vm.label({ account: address(reentrantSender), newLabel: "Reentrant Sender" }); - vm.label({ account: address(revertingRecipient), newLabel: "Reverting Recipient" }); - vm.label({ account: address(revertingSender), newLabel: "Reverting Sender" }); + vm.label({ account: address(reentrantRecipient), newLabel: "Reentrant Lockup Recipient" }); + vm.label({ account: address(reentrantSender), newLabel: "Reentrant Lockup Sender" }); + vm.label({ account: address(revertingRecipient), newLabel: "Reverting Lockup Recipient" }); + vm.label({ account: address(revertingSender), newLabel: "Reverting Lockup Sender" }); } } diff --git a/test/unit/adminable/Adminable.t.sol b/test/unit/adminable/Adminable.t.sol index 0a4a53850..d3ca481fc 100644 --- a/test/unit/adminable/Adminable.t.sol +++ b/test/unit/adminable/Adminable.t.sol @@ -21,7 +21,7 @@ abstract contract Adminable_Unit_Test is Unit_Test { function setUp() public virtual override { Unit_Test.setUp(); - // Cast the linear contract as the `ISablierV2Adminable` contract. + // Cast the linear contract as the {ISablierV2Adminable} contract. adminable = ISablierV2Adminable(address(linear)); } } diff --git a/test/unit/flash-loan/FlashLoan.t.sol b/test/unit/flash-loan/FlashLoan.t.sol index c07605116..5195cc175 100644 --- a/test/unit/flash-loan/FlashLoan.t.sol +++ b/test/unit/flash-loan/FlashLoan.t.sol @@ -21,7 +21,7 @@ abstract contract FlashLoan_Unit_Test is Unit_Test { function setUp() public virtual override { Unit_Test.setUp(); - // Cast the linear contract as the `SablierV2FlashLoan` contract. + // Cast the linear contract as {SablierV2FlashLoan}. flashLoan = SablierV2FlashLoan(address(linear)); // Set the default flash fee in the comptroller. diff --git a/test/unit/flash-loan/flash-loan/flashLoan.t.sol b/test/unit/flash-loan/flash-loan/flashLoan.t.sol index 7abb8db60..b162ef0a1 100644 --- a/test/unit/flash-loan/flash-loan/flashLoan.t.sol +++ b/test/unit/flash-loan/flash-loan/flashLoan.t.sol @@ -3,7 +3,7 @@ pragma solidity >=0.8.13; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; import { UD60x18, ud } from "@prb/math/UD60x18.sol"; -import { IERC3156FlashBorrower } from "erc3156/contracts/interfaces/IERC3156FlashBorrower.sol"; +import { IERC3156FlashBorrower } from "erc3156/interfaces/IERC3156FlashBorrower.sol"; import { Errors } from "src/libraries/Errors.sol"; import { Events } from "src/libraries/Events.sol"; @@ -163,10 +163,10 @@ contract FlashLoanFunction_Unit_Test is FlashLoan_Unit_Test { // Load the flash fee. uint256 fee = flashLoan.flashFee({ asset: address(DEFAULT_ASSET), amount: amount }); - // Deal the flash loan amount to the contract. + // Mint the flash loan amount to the contract. deal({ token: address(DEFAULT_ASSET), to: address(flashLoan), give: amount }); - // Deal the flash fee to the receiver so that they can repay the flash loan. + // Mint the flash fee to the receiver so that they can repay the flash loan. deal({ token: address(DEFAULT_ASSET), to: address(goodFlashLoanReceiver), give: fee }); // Expect `amount` of ERC-20 assets to be transferred from the {SablierV2FlashLoan} contract to the receiver. diff --git a/test/unit/lockup/linear/Linear.t.sol b/test/unit/lockup/linear/Linear.t.sol index c9f8acf28..f9bae6900 100644 --- a/test/unit/lockup/linear/Linear.t.sol +++ b/test/unit/lockup/linear/Linear.t.sol @@ -39,12 +39,12 @@ import { WithdrawMultiple_Unit_Test } from "../shared/withdraw-multiple/withdraw /// @notice Common testing logic needed across {SablierV2LockupLinear} unit tests. abstract contract Linear_Unit_Test is Unit_Test, Linear_Shared_Test { function setUp() public virtual override(Unit_Test, Linear_Shared_Test) { - // Both of these contracts inherit from `Base_Test`, which is fine because multiple inheritance is - // allowed in Solidity, and `Base_Test.setUp` will only be called once. + // Both of these contracts inherit from {Base_Test}, but this is fine because multiple inheritance is + // allowed in Solidity, and {Base_Test-setUp} will only be called once. Unit_Test.setUp(); Linear_Shared_Test.setUp(); - // Cast the linear contract as `ISablierV2Config` and `ISablierV2Lockup`. + // Cast the linear contract as {ISablierV2Config} and {ISablierV2Lockup}. config = ISablierV2Config(linear); lockup = ISablierV2Lockup(linear); diff --git a/test/unit/lockup/pro/Pro.t.sol b/test/unit/lockup/pro/Pro.t.sol index 56ca29124..ab4cfb84d 100644 --- a/test/unit/lockup/pro/Pro.t.sol +++ b/test/unit/lockup/pro/Pro.t.sol @@ -39,16 +39,16 @@ import { WithdrawMultiple_Unit_Test } from "../shared/withdraw-multiple/withdraw /// @notice Common testing logic needed across {SablierV2LockupPro} unit tests. abstract contract Pro_Unit_Test is Unit_Test, Pro_Shared_Test { /*////////////////////////////////////////////////////////////////////////// - SETUP FUNCTION + SET-UP FUNCTION //////////////////////////////////////////////////////////////////////////*/ function setUp() public virtual override(Unit_Test, Pro_Shared_Test) { - // Both of these contracts inherit from `Base_Test`, which is fine because multiple inheritance is - // allowed in Solidity, and `Base_Test.setUp` will only be called once. + // Both of these contracts inherit from {Base_Test}, but this is fine because multiple inheritance is + // allowed in Solidity, and {Base_Test-setUp} will only be called once. Unit_Test.setUp(); Pro_Shared_Test.setUp(); - // Cast the linear contract as `ISablierV2Config` and `ISablierV2Lockup`. + // Cast the linear contract as {ISablierV2Config} and {ISablierV2Lockup}. config = ISablierV2Lockup(pro); lockup = ISablierV2Lockup(pro); From b04a090c700ae746f8e637df881075033815e248 Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Wed, 1 Feb 2023 13:42:19 +0200 Subject: [PATCH 05/11] ci: run invariant tests in CI test: add "onlyInCI" modifier test: apply the "onlyInCI" modifier to the "invariant_CallSummary" functions --- .github/workflows/ci.yml | 24 ++++++++++++++++++++++++ test/Base.t.sol | 24 ++++++++++++++++++------ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1cf5fc03f..75c178cad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -135,6 +135,30 @@ jobs: echo "## Fuzz tests result" >> $GITHUB_STEP_SUMMARY echo "✅ Passed" >> $GITHUB_STEP_SUMMARY + test-invariant: + env: + FOUNDRY_INVARIANT_RUNS: "10000" + 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: "Produce an optimized build with --via-ir" + run: "FOUNDRY_PROFILE=optimized forge build" + + - 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: diff --git a/test/Base.t.sol b/test/Base.t.sol index a9e2bbc9b..0b20cf6cd 100644 --- a/test/Base.t.sol +++ b/test/Base.t.sol @@ -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 //////////////////////////////////////////////////////////////////////////*/ @@ -194,4 +200,10 @@ abstract contract Base_Test is Assertions, Constants, Calculations, Utils, StdCh 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"); + } } From 92933ffcbbd74a0730ca0085c8cc3ec5cf170372 Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Wed, 1 Feb 2023 15:07:27 +0200 Subject: [PATCH 06/11] ci: cache the optimized build between jobs --- .github/workflows/ci.yml | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 75c178cad..8dc70deaa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,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: @@ -99,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\"" @@ -124,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\"" @@ -149,8 +163,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 invariant tests against the optimized build" run: "FOUNDRY_PROFILE=test-optimized forge test --match-path \"test/invariant/**/*.sol\"" @@ -174,8 +192,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: > From 431704690f1dedca9bebc31df796d417356dd4cb Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Sat, 4 Feb 2023 14:13:03 +0200 Subject: [PATCH 07/11] test: separate create functions in a separate handler test: load admin from contract in invariant handlers --- test/invariant/handlers/LockupHandler.t.sol | 54 ++--- .../handlers/LockupHandlerStore.t.sol | 42 ++++ .../handlers/LockupLinearCreateHandler.t.sol | 160 ++++++++++++++ .../handlers/LockupLinearHandler.t.sol | 153 +------------ .../handlers/LockupProCreateHandler.t.sol | 202 ++++++++++++++++++ .../invariant/handlers/LockupProHandler.t.sol | 191 +---------------- test/invariant/lockup/Lockup.t.sol | 37 ++-- test/invariant/lockup/linear/Linear.t.sol | 30 ++- test/invariant/lockup/pro/Pro.t.sol | 26 ++- 9 files changed, 487 insertions(+), 408 deletions(-) create mode 100644 test/invariant/handlers/LockupHandlerStore.t.sol create mode 100644 test/invariant/handlers/LockupLinearCreateHandler.t.sol create mode 100644 test/invariant/handlers/LockupProCreateHandler.t.sol diff --git a/test/invariant/handlers/LockupHandler.t.sol b/test/invariant/handlers/LockupHandler.t.sol index 7f514ad91..eb612adbe 100644 --- a/test/invariant/handlers/LockupHandler.t.sol +++ b/test/invariant/handlers/LockupHandler.t.sol @@ -7,33 +7,18 @@ import { ISablierV2Lockup } from "src/interfaces/ISablierV2Lockup.sol"; import { Broker, Lockup } from "src/types/DataTypes.sol"; import { BaseHandler } from "./BaseHandler.t.sol"; +import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; /// @title LockupHandler /// @dev Common handler logic between {SablierV2LockupLinear} and {SablierV2LockupPro}. abstract contract LockupHandler is BaseHandler { - /*////////////////////////////////////////////////////////////////////////// - CONSTANTS - //////////////////////////////////////////////////////////////////////////*/ - - uint256 internal constant MAX_STREAM_COUNT = 100; - /*////////////////////////////////////////////////////////////////////////// PUBLIC TEST CONTRACTS //////////////////////////////////////////////////////////////////////////*/ - ISablierV2Lockup public lockup; - - /*////////////////////////////////////////////////////////////////////////// - PUBLIC TEST VARIABLES - //////////////////////////////////////////////////////////////////////////*/ - - address public admin; IERC20 public asset; - uint256 public lastStreamId; - uint128 public returnedAmountsSum; - mapping(uint256 => address) public streamIdsToRecipients; - mapping(uint256 => address) public streamIdsToSenders; - uint256[] public streamIds; + ISablierV2Lockup public lockup; + LockupHandlerStore public store; /*////////////////////////////////////////////////////////////////////////// PRIVATE TEST VARIABLES @@ -47,10 +32,10 @@ abstract contract LockupHandler is BaseHandler { CONSTRUCTOR //////////////////////////////////////////////////////////////////////////*/ - constructor(address admin_, IERC20 asset_, ISablierV2Lockup linear_) { - admin = admin_; + constructor(IERC20 asset_, ISablierV2Lockup lockup_, LockupHandlerStore store_) { asset = asset_; - lockup = linear_; + lockup = lockup_; + store = store_; } /*////////////////////////////////////////////////////////////////////////// @@ -58,39 +43,36 @@ abstract contract LockupHandler is BaseHandler { //////////////////////////////////////////////////////////////////////////*/ modifier useAdmin() { + address admin = lockup.admin(); vm.startPrank(admin); _; vm.stopPrank(); } modifier useFuzzedStreamRecipient(uint256 streamIndexSeed) { + uint256 lastStreamId = store.lastStreamId(); if (lastStreamId == 0) { return; } - currentStreamId = streamIds[bound(streamIndexSeed, 0, lastStreamId - 1)]; - currentRecipient = streamIdsToRecipients[currentStreamId]; + currentStreamId = store.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); + currentRecipient = store.streamIdsToRecipients(currentStreamId); vm.startPrank(currentRecipient); _; vm.stopPrank(); } modifier useFuzzedStreamSender(uint256 streamIndexSeed) { + uint256 lastStreamId = store.lastStreamId(); if (lastStreamId == 0) { return; } - currentStreamId = streamIds[bound(streamIndexSeed, 0, lastStreamId - 1)]; - currentSender = streamIdsToSenders[currentStreamId]; + currentStreamId = store.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); + currentSender = store.streamIdsToSenders(currentStreamId); vm.startPrank(currentSender); _; vm.stopPrank(); } - modifier useNewSender(address sender) { - vm.startPrank(sender); - _; - vm.stopPrank(); - } - /*////////////////////////////////////////////////////////////////////////// SABLIER-V2-LOCKUP //////////////////////////////////////////////////////////////////////////*/ @@ -110,8 +92,8 @@ abstract contract LockupHandler is BaseHandler { // Burn the NFT. lockup.burn(currentStreamId); - // Update the recipient associated with this stream id. - streamIdsToRecipients[currentStreamId] = address(0); + // Set the recipient associated with this stream to the zero address. + store.updateRecipient(currentStreamId, address(0)); } function cancel(uint256 streamIndexSeed) external instrument("cancel") useFuzzedStreamSender(streamIndexSeed) { @@ -124,7 +106,7 @@ abstract contract LockupHandler is BaseHandler { // Record the returned amount by adding it to the ghost variable `returnedAmountsSum`. This is needed to // check invariants against the contract's balance. uint128 returnedAmount = lockup.returnableAmountOf(currentStreamId); - returnedAmountsSum += returnedAmount; + store.addReturnedAmount(returnedAmount); // Cancel the stream. lockup.cancel(currentStreamId); @@ -137,7 +119,7 @@ abstract contract LockupHandler is BaseHandler { return; } - // Claim the revenues. + // Claim the protocol revenues. lockup.claimProtocolRevenues(asset); } @@ -228,6 +210,6 @@ abstract contract LockupHandler is BaseHandler { lockup.transferFrom({ from: currentRecipient, to: newRecipient, tokenId: currentStreamId }); // Update the recipient associated with this stream id. - streamIdsToRecipients[currentStreamId] = newRecipient; + store.updateRecipient(currentStreamId, newRecipient); } } diff --git a/test/invariant/handlers/LockupHandlerStore.t.sol b/test/invariant/handlers/LockupHandlerStore.t.sol new file mode 100644 index 000000000..02bfbbd87 --- /dev/null +++ b/test/invariant/handlers/LockupHandlerStore.t.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +/// @title LockupHandlerStore +/// @dev Storage contract for the lockup handler streams. +contract LockupHandlerStore { + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST VARIABLES + //////////////////////////////////////////////////////////////////////////*/ + + uint256 public lastStreamId; + uint128 public returnedAmountsSum; + mapping(uint256 => address) public streamIdsToRecipients; + mapping(uint256 => address) public streamIdsToSenders; + uint256[] public streamIds; + + /*////////////////////////////////////////////////////////////////////////// + NON-CONSTANT FUNCTIONS + //////////////////////////////////////////////////////////////////////////*/ + + function addReturnedAmount(uint128 returnedAmount) external { + returnedAmountsSum += returnedAmount; + } + + function pushStreamId(uint256 streamId, address sender, address recipient) external { + // Store the stream id in the ids array and the reverse mappings. + streamIds.push(streamId); + streamIdsToSenders[streamId] = sender; + streamIdsToRecipients[streamId] = recipient; + + // Update the last stream id. + lastStreamId = streamId; + } + + function updateRecipient(uint256 streamId, address newRecipient) external { + streamIdsToRecipients[streamId] = newRecipient; + } + + function updateSender(uint256 streamId, address newSender) external { + streamIdsToSenders[streamId] = newSender; + } +} diff --git a/test/invariant/handlers/LockupLinearCreateHandler.t.sol b/test/invariant/handlers/LockupLinearCreateHandler.t.sol new file mode 100644 index 000000000..cb0f6b3cc --- /dev/null +++ b/test/invariant/handlers/LockupLinearCreateHandler.t.sol @@ -0,0 +1,160 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; + +import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol"; +import { Broker, LockupLinear } from "src/types/DataTypes.sol"; + +import { BaseHandler } from "./BaseHandler.t.sol"; +import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; + +/// @title LockupLinearCreateHandler +/// @dev This contract is a complement of {LockupLinearHandler}. The goal is to bias the invariant calls +/// toward the lockup functions by creating multiple handlers for the lockup contracts. +contract LockupLinearCreateHandler is BaseHandler { + /*////////////////////////////////////////////////////////////////////////// + CONSTANTS + //////////////////////////////////////////////////////////////////////////*/ + + uint256 internal constant MAX_STREAM_COUNT = 100; + + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + IERC20 public asset; + ISablierV2LockupLinear public linear; + LockupHandlerStore public store; + + /*////////////////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////////////////*/ + + constructor(IERC20 asset_, ISablierV2LockupLinear linear_, LockupHandlerStore store_) { + asset = asset_; + linear = linear_; + store = store_; + } + + /*////////////////////////////////////////////////////////////////////////// + MODIFIERS + //////////////////////////////////////////////////////////////////////////*/ + + modifier useNewSender(address sender) { + vm.startPrank(sender); + _; + vm.stopPrank(); + } + + /*////////////////////////////////////////////////////////////////////////// + FUNCTIONS + //////////////////////////////////////////////////////////////////////////*/ + + struct CreateWithDurationsParams { + Broker broker; + bool cancelable; + LockupLinear.Durations durations; + address recipient; + address sender; + uint128 totalAmount; + } + + struct CreateWithDurationsVars { + uint256 streamId; + } + + function createWithDurations( + CreateWithDurationsParams memory params + ) public instrument("createWithDurations") useNewSender(params.sender) { + params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); + params.durations.cliff = boundUint40(params.durations.cliff, 1, 1_000); + params.durations.total = boundUint40(params.durations.total, params.durations.cliff + 1, MAX_UNIX_TIMESTAMP); + params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); + + // We don't want to fuzz more than a certain number of streams. + if (store.lastStreamId() >= MAX_STREAM_COUNT) { + return; + } + + // The protocol doesn't allow the sender, recipient or broker to be the zero address. + if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { + return; + } + + // Mint enough ERC-20 assets to the sender. + deal({ token: address(asset), to: params.sender, give: params.totalAmount }); + + // Approve the {SablierV2LockupLinear} contract to spend the ERC-20 assets. + asset.approve({ spender: address(linear), amount: params.totalAmount }); + + // Create the stream. + CreateWithDurationsVars memory vars; + vars.streamId = linear.createWithDurations({ + sender: params.sender, + recipient: params.recipient, + totalAmount: params.totalAmount, + asset: asset, + cancelable: params.cancelable, + durations: params.durations, + broker: params.broker + }); + + // Store the stream id. + store.pushStreamId(vars.streamId, params.sender, params.recipient); + } + + struct CreateWithRangeParams { + Broker broker; + bool cancelable; + LockupLinear.Range range; + address recipient; + address sender; + uint128 totalAmount; + } + + struct CreateWithRangeVars { + uint256 streamId; + } + + function createWithRange( + CreateWithRangeParams memory params + ) public instrument("createWithRange") useNewSender(params.sender) { + params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); + params.range.start = boundUint40(params.range.start, 0, 1_000); + params.range.cliff = boundUint40(params.range.cliff, params.range.start, 5_000); + params.range.end = boundUint40(params.range.end, params.range.cliff + 1, MAX_UNIX_TIMESTAMP); + params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); + + // We don't want to fuzz more than a certain number of streams. + if (store.lastStreamId() >= MAX_STREAM_COUNT) { + return; + } + + // The protocol doesn't allow the sender, recipient or broker to be the zero address. + if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { + return; + } + + // Mint enough ERC-20 assets to the sender. + deal({ token: address(asset), to: params.sender, give: params.totalAmount }); + + // Approve the {SablierV2LockupLinear} contract to spend the ERC-20 assets. + asset.approve({ spender: address(linear), amount: params.totalAmount }); + + // Create the stream. + CreateWithRangeVars memory vars; + vars.streamId = linear.createWithRange({ + sender: params.sender, + recipient: params.recipient, + totalAmount: params.totalAmount, + asset: asset, + cancelable: params.cancelable, + range: params.range, + broker: params.broker + }); + + // Store the stream id. + store.pushStreamId(vars.streamId, params.sender, params.recipient); + } +} diff --git a/test/invariant/handlers/LockupLinearHandler.t.sol b/test/invariant/handlers/LockupLinearHandler.t.sol index 03ab99459..244175b3a 100644 --- a/test/invariant/handlers/LockupLinearHandler.t.sol +++ b/test/invariant/handlers/LockupLinearHandler.t.sol @@ -4,158 +4,17 @@ pragma solidity >=0.8.13 <0.9.0; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol"; -import { Broker, LockupLinear } from "src/types/DataTypes.sol"; import { LockupHandler } from "./LockupHandler.t.sol"; +import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; /// @title LockupLinearHandler /// @dev This contract and not {SablierV2LockupLinear} is exposed to Foundry for invariant testing. The point is /// to bound and restrict the inputs that get passed to the real-world contract to avoid getting reverts. contract LockupLinearHandler is LockupHandler { - /*////////////////////////////////////////////////////////////////////////// - PUBLIC TEST CONTRACTS - //////////////////////////////////////////////////////////////////////////*/ - - ISablierV2LockupLinear public linear; - - /*////////////////////////////////////////////////////////////////////////// - CONSTRUCTOR - //////////////////////////////////////////////////////////////////////////*/ - - constructor(address admin_, IERC20 asset_, ISablierV2LockupLinear linear_) LockupHandler(admin_, asset_, linear_) { - linear = linear_; - } - - /*////////////////////////////////////////////////////////////////////////// - FUNCTIONS - //////////////////////////////////////////////////////////////////////////*/ - - struct CreateWithDurationsParams { - Broker broker; - bool cancelable; - LockupLinear.Durations durations; - address recipient; - address sender; - uint128 totalAmount; - } - - struct CreateWithDurationsVars { - uint256 streamId; - } - - function createWithDurations( - CreateWithDurationsParams memory params - ) public instrument("createWithDurations") useNewSender(params.sender) { - params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); - params.durations.cliff = boundUint40(params.durations.cliff, 1, 1_000); - params.durations.total = boundUint40(params.durations.total, params.durations.cliff + 1, MAX_UNIX_TIMESTAMP); - params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); - - // We don't want to fuzz more than a certain number of streams. - if (streamIds.length > MAX_STREAM_COUNT) { - return; - } - - // The protocol doesn't allow the sender, recipient or broker to be the zero address. - if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { - return; - } - - // Mint enough ERC-20 assets to the sender. - deal({ token: address(asset), to: params.sender, give: params.totalAmount }); - - // Approve the {SablierV2LockupLinear} contract to spend the ERC-20 assets. - asset.approve({ spender: address(linear), amount: params.totalAmount }); - - // Create the stream. - CreateWithDurationsVars memory vars; - vars.streamId = linear.createWithDurations({ - sender: params.sender, - recipient: params.recipient, - totalAmount: params.totalAmount, - asset: asset, - cancelable: params.cancelable, - durations: params.durations, - broker: params.broker - }); - - // Store the stream id in the ids array and the reverse mapping. - streamIds.push(vars.streamId); - streamIdsToRecipients[vars.streamId] = params.recipient; - streamIdsToSenders[vars.streamId] = params.sender; - - // Update the last stream id. - lastStreamId = vars.streamId; - } - - /// @dev This function exists only to bias the invariant calls toward {createWithDurations}, so that more streams - /// get created. - function createWithDurations_Bias(CreateWithDurationsParams memory params) external { - createWithDurations(params); - } - - struct CreateWithRangeParams { - Broker broker; - bool cancelable; - LockupLinear.Range range; - address recipient; - address sender; - uint128 totalAmount; - } - - struct CreateWithRangeVars { - uint256 streamId; - } - - function createWithRange( - CreateWithRangeParams memory params - ) public instrument("createWithRange") useNewSender(params.sender) { - params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); - params.range.start = boundUint40(params.range.start, 0, 1_000); - params.range.cliff = boundUint40(params.range.cliff, params.range.start, 5_000); - params.range.end = boundUint40(params.range.end, params.range.cliff + 1, MAX_UNIX_TIMESTAMP); - params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); - - // We don't want to fuzz more than a certain number of streams. - if (streamIds.length > MAX_STREAM_COUNT) { - return; - } - - // The protocol doesn't allow the sender, recipient or broker to be the zero address. - if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { - return; - } - - // Mint enough ERC-20 assets to the sender. - deal({ token: address(asset), to: params.sender, give: params.totalAmount }); - - // Approve the {SablierV2LockupLinear} contract to spend the ERC-20 assets. - asset.approve({ spender: address(linear), amount: params.totalAmount }); - - // Create the stream. - CreateWithRangeVars memory vars; - vars.streamId = linear.createWithRange({ - sender: params.sender, - recipient: params.recipient, - totalAmount: params.totalAmount, - asset: asset, - cancelable: params.cancelable, - range: params.range, - broker: params.broker - }); - - // Store the stream id in the ids array and the reverse mapping. - streamIds.push(vars.streamId); - streamIdsToRecipients[vars.streamId] = params.recipient; - streamIdsToSenders[vars.streamId] = params.sender; - - // Update the last stream id. - lastStreamId = vars.streamId; - } - - /// @dev This function exists only to bias the invariant calls toward {createWithRange}, so that more streams - /// get created. - function createWithRange_Bias(CreateWithRangeParams memory params) external { - createWithRange(params); - } + constructor( + IERC20 asset_, + ISablierV2LockupLinear linear_, + LockupHandlerStore store_ + ) LockupHandler(asset_, linear_, store_) {} } diff --git a/test/invariant/handlers/LockupProCreateHandler.t.sol b/test/invariant/handlers/LockupProCreateHandler.t.sol new file mode 100644 index 000000000..268b5ba05 --- /dev/null +++ b/test/invariant/handlers/LockupProCreateHandler.t.sol @@ -0,0 +1,202 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13 <0.9.0; + +import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; +import { UD60x18 } from "@prb/math/UD60x18.sol"; +import { Solarray } from "solarray/Solarray.sol"; + +import { ISablierV2Comptroller } from "src/interfaces/ISablierV2Comptroller.sol"; +import { ISablierV2LockupPro } from "src/interfaces/ISablierV2LockupPro.sol"; +import { Broker, LockupPro } from "src/types/DataTypes.sol"; + +import { BaseHandler } from "./BaseHandler.t.sol"; +import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; + +/// @title LockupProCreateHandler +/// @dev This contract is a complement of {LockupProHandler}. The goal is to bias the invariant calls +/// toward the lockup functions by creating multiple handlers for the lockup contracts. +contract LockupProCreateHandler is BaseHandler { + /*////////////////////////////////////////////////////////////////////////// + CONSTANTS + //////////////////////////////////////////////////////////////////////////*/ + + uint256 internal constant MAX_STREAM_COUNT = 100; + + /*////////////////////////////////////////////////////////////////////////// + PUBLIC TEST CONTRACTS + //////////////////////////////////////////////////////////////////////////*/ + + IERC20 public asset; + ISablierV2Comptroller public comptroller; + ISablierV2LockupPro public pro; + LockupHandlerStore public store; + + /*////////////////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////////////////*/ + + constructor( + IERC20 asset_, + ISablierV2Comptroller comptroller_, + ISablierV2LockupPro pro_, + LockupHandlerStore store_ + ) { + asset = asset_; + comptroller = comptroller_; + pro = pro_; + store = store_; + } + + /*////////////////////////////////////////////////////////////////////////// + MODIFIERS + //////////////////////////////////////////////////////////////////////////*/ + + modifier useNewSender(address sender) { + vm.startPrank(sender); + _; + vm.stopPrank(); + } + + /*////////////////////////////////////////////////////////////////////////// + FUNCTIONS + //////////////////////////////////////////////////////////////////////////*/ + + struct CreateWithDeltasParams { + Broker broker; + bool cancelable; + uint40 delta0; + uint40 delta1; + address recipient; + address sender; + uint128 totalAmount; + } + + struct CreateWithDeltasVars { + uint256 streamId; + uint40[] deltas; + UD60x18 protocolFee; + uint128 depositAmount; + LockupPro.Segment[] segments; + } + + function createWithDeltas( + CreateWithDeltasParams memory params + ) public instrument("createWithDeltas") useNewSender(params.sender) { + params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); + params.delta0 = boundUint40(params.delta0, 1, 100); + params.delta1 = boundUint40(params.delta1, 1, MAX_UNIX_TIMESTAMP - uint40(block.timestamp) - params.delta0); + params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); + + // We don't want to fuzz more than a certain number of streams. + if (store.lastStreamId() > MAX_STREAM_COUNT) { + return; + } + + // The protocol doesn't allow the sender, recipient or broker to be the zero address. + if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { + return; + } + + // Create the deltas array. + CreateWithDeltasVars memory vars; + vars.deltas = Solarray.uint40s(params.delta0, params.delta1); + + // Adjust the segment milestones to match the fuzzed deltas. + vars.segments = DEFAULT_SEGMENTS; + vars.segments[0].milestone = uint40(block.timestamp) + params.delta0; + vars.segments[1].milestone = vars.segments[0].milestone + params.delta1; + + // Calculate the deposit amount. + vars.depositAmount = calculateDepositAmount(params.totalAmount, vars.protocolFee, params.broker.fee); + + // Adjust the segment amounts based on the fuzzed deposit amount. + adjustSegmentAmounts(vars.segments, vars.depositAmount); + + // Mint enough ERC-20 assets to the sender. + deal({ token: address(asset), to: params.sender, give: params.totalAmount }); + + // Approve the {SablierV2LockupPro} contract to spend the ERC-20 assets. + asset.approve({ spender: address(pro), amount: params.totalAmount }); + + // Create the stream. + vars.streamId = pro.createWithDeltas({ + sender: params.sender, + recipient: params.recipient, + totalAmount: params.totalAmount, + segments: vars.segments, + asset: asset, + cancelable: params.cancelable, + deltas: vars.deltas, + broker: params.broker + }); + + // Store the stream id. + store.pushStreamId(vars.streamId, params.sender, params.recipient); + } + + struct CreateWithMilestonesParams { + Broker broker; + bool cancelable; + uint128 totalAmount; + address recipient; + address sender; + uint40 startTime; + } + + struct CreateWithMilestonesVars { + uint128 depositAmount; + UD60x18 protocolFee; + LockupPro.Segment[] segments; + uint256 streamId; + } + + function createWithMilestones( + CreateWithMilestonesParams memory params + ) public instrument("createWithMilestones") useNewSender(params.sender) { + params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); + params.startTime = boundUint40(params.startTime, 0, DEFAULT_SEGMENTS[0].milestone - 1); + params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); + + // We don't want to fuzz more than a certain number of streams. + if (store.lastStreamId() >= MAX_STREAM_COUNT) { + return; + } + + // The protocol doesn't allow the sender, recipient or broker to be the zero address. + if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { + return; + } + + // Get the current protocol fee. + CreateWithMilestonesVars memory vars; + vars.protocolFee = comptroller.getProtocolFee(asset); + + // Calculate the deposit amount. + vars.depositAmount = calculateDepositAmount(params.totalAmount, vars.protocolFee, params.broker.fee); + + // Adjust the segment amounts based on the fuzzed deposit amount. + vars.segments = DEFAULT_SEGMENTS; + adjustSegmentAmounts(vars.segments, vars.depositAmount); + + // Mint enough ERC-20 assets to the sender. + deal({ token: address(asset), to: params.sender, give: params.totalAmount }); + + // Approve the {SablierV2LockupPro} contract to spend the ERC-20 assets. + asset.approve({ spender: address(pro), amount: params.totalAmount }); + + // Create the stream. + vars.streamId = pro.createWithMilestones({ + sender: params.sender, + recipient: params.recipient, + totalAmount: params.totalAmount, + segments: vars.segments, + asset: asset, + cancelable: params.cancelable, + startTime: params.startTime, + broker: params.broker + }); + + // Store the stream id. + store.pushStreamId(vars.streamId, params.sender, params.recipient); + } +} diff --git a/test/invariant/handlers/LockupProHandler.t.sol b/test/invariant/handlers/LockupProHandler.t.sol index 5cefc7cc6..17ec608a9 100644 --- a/test/invariant/handlers/LockupProHandler.t.sol +++ b/test/invariant/handlers/LockupProHandler.t.sol @@ -2,202 +2,19 @@ pragma solidity >=0.8.13 <0.9.0; import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; -import { UD60x18 } from "@prb/math/UD60x18.sol"; -import { Solarray } from "solarray/Solarray.sol"; -import { ISablierV2Comptroller } from "src/interfaces/ISablierV2Comptroller.sol"; import { ISablierV2LockupPro } from "src/interfaces/ISablierV2LockupPro.sol"; -import { Broker, LockupPro } from "src/types/DataTypes.sol"; import { LockupHandler } from "./LockupHandler.t.sol"; +import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; /// @title LockupProHandler /// @dev This contract and not {SablierV2LockupPro} is exposed to Foundry for invariant testing. The point is /// to bound and restrict the inputs that get passed to the real-world contract to avoid getting reverts. contract LockupProHandler is LockupHandler { - /*////////////////////////////////////////////////////////////////////////// - PUBLIC TEST CONTRACTS - //////////////////////////////////////////////////////////////////////////*/ - - ISablierV2Comptroller public comptroller; - ISablierV2LockupPro public pro; - - /*////////////////////////////////////////////////////////////////////////// - CONSTRUCTOR - //////////////////////////////////////////////////////////////////////////*/ - constructor( - address admin_, IERC20 asset_, - ISablierV2Comptroller comptroller_, - ISablierV2LockupPro pro_ - ) LockupHandler(admin_, asset_, pro_) { - comptroller = comptroller_; - pro = pro_; - } - - /*////////////////////////////////////////////////////////////////////////// - FUNCTIONS - //////////////////////////////////////////////////////////////////////////*/ - - struct CreateWithDeltasParams { - Broker broker; - bool cancelable; - uint40 delta0; - uint40 delta1; - address recipient; - address sender; - uint128 totalAmount; - } - - struct CreateWithDeltasVars { - uint256 streamId; - uint40[] deltas; - UD60x18 protocolFee; - uint128 depositAmount; - LockupPro.Segment[] segments; - } - - function createWithDeltas( - CreateWithDeltasParams memory params - ) public instrument("createWithDeltas") useNewSender(params.sender) { - params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); - params.delta0 = boundUint40(params.delta0, 1, 100); - params.delta1 = boundUint40(params.delta1, 1, MAX_UNIX_TIMESTAMP - uint40(block.timestamp) - params.delta0); - params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); - - // We don't want to fuzz more than a certain number of streams. - if (streamIds.length > MAX_STREAM_COUNT) { - return; - } - - // The protocol doesn't allow the sender, recipient or broker to be the zero address. - if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { - return; - } - - // Create the deltas array. - CreateWithDeltasVars memory vars; - vars.deltas = Solarray.uint40s(params.delta0, params.delta1); - - // Adjust the segment milestones to match the fuzzed deltas. - vars.segments = DEFAULT_SEGMENTS; - vars.segments[0].milestone = uint40(block.timestamp) + params.delta0; - vars.segments[1].milestone = vars.segments[0].milestone + params.delta1; - - // Calculate the deposit amount. - vars.depositAmount = calculateDepositAmount(params.totalAmount, vars.protocolFee, params.broker.fee); - - // Adjust the segment amounts based on the fuzzed deposit amount. - adjustSegmentAmounts(vars.segments, vars.depositAmount); - - // Mint enough ERC-20 assets to the sender. - deal({ token: address(asset), to: params.sender, give: params.totalAmount }); - - // Approve the {SablierV2LockupPro} contract to spend the ERC-20 assets. - asset.approve({ spender: address(pro), amount: params.totalAmount }); - - // Create the stream. - vars.streamId = pro.createWithDeltas({ - sender: params.sender, - recipient: params.recipient, - totalAmount: params.totalAmount, - segments: vars.segments, - asset: asset, - cancelable: params.cancelable, - deltas: vars.deltas, - broker: params.broker - }); - - // Store the stream id in the ids array and the reverse mapping. - streamIds.push(vars.streamId); - streamIdsToRecipients[vars.streamId] = params.recipient; - streamIdsToSenders[vars.streamId] = params.sender; - - // Update the last stream id. - lastStreamId = vars.streamId; - } - - /// @dev This function exists only to bias the invariant calls toward {createWithDeltas}, so that more streams - /// get created. - function createWithDeltas_Bias(CreateWithDeltasParams memory params) external { - createWithDeltas(params); - } - - struct CreateWithMilestonesParams { - Broker broker; - bool cancelable; - uint128 totalAmount; - address recipient; - address sender; - uint40 startTime; - } - - struct CreateWithMilestonesVars { - uint128 depositAmount; - UD60x18 protocolFee; - LockupPro.Segment[] segments; - uint256 streamId; - } - - function createWithMilestones( - CreateWithMilestonesParams memory params - ) public instrument("createWithMilestones") useNewSender(params.sender) { - params.broker.fee = bound(params.broker.fee, 0, DEFAULT_MAX_FEE); - params.startTime = boundUint40(params.startTime, 0, DEFAULT_SEGMENTS[0].milestone - 1); - params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); - - // We don't want to fuzz more than a certain number of streams. - if (streamIds.length > MAX_STREAM_COUNT) { - return; - } - - // The protocol doesn't allow the sender, recipient or broker to be the zero address. - if (params.sender == address(0) || params.recipient == address(0) || params.broker.addr == address(0)) { - return; - } - - // Get the current protocol fee. - CreateWithMilestonesVars memory vars; - vars.protocolFee = comptroller.getProtocolFee(asset); - - // Calculate the deposit amount. - vars.depositAmount = calculateDepositAmount(params.totalAmount, vars.protocolFee, params.broker.fee); - - // Adjust the segment amounts based on the fuzzed deposit amount. - vars.segments = DEFAULT_SEGMENTS; - adjustSegmentAmounts(vars.segments, vars.depositAmount); - - // Mint enough ERC-20 assets to the sender. - deal({ token: address(asset), to: params.sender, give: params.totalAmount }); - - // Approve the {SablierV2LockupPro} contract to spend the ERC-20 assets. - asset.approve({ spender: address(pro), amount: params.totalAmount }); - - // Create the stream. - vars.streamId = pro.createWithMilestones({ - sender: params.sender, - recipient: params.recipient, - totalAmount: params.totalAmount, - segments: vars.segments, - asset: asset, - cancelable: params.cancelable, - startTime: params.startTime, - broker: params.broker - }); - - // Store the stream id in the ids array and the reverse mapping. - streamIds.push(vars.streamId); - streamIdsToRecipients[vars.streamId] = params.recipient; - streamIdsToSenders[vars.streamId] = params.sender; - - // Update the last stream id. - lastStreamId = vars.streamId; - } - - /// @dev This function exists only to bias the invariant calls toward {createWithMilestones}, so that more streams - /// get created. - function createWithMilestones_Bias(CreateWithMilestonesParams memory params) external { - createWithMilestones(params); - } + ISablierV2LockupPro pro_, + LockupHandlerStore store_ + ) LockupHandler(asset_, pro_, store_) {} } diff --git a/test/invariant/lockup/Lockup.t.sol b/test/invariant/lockup/Lockup.t.sol index 15b342107..e453ba59a 100644 --- a/test/invariant/lockup/Lockup.t.sol +++ b/test/invariant/lockup/Lockup.t.sol @@ -9,6 +9,7 @@ import { SablierV2LockupLinear } from "src/SablierV2LockupLinear.sol"; import { Invariant_Test } from "../Invariant.t.sol"; import { FlashLoanHandler } from "../handlers/FlashLoanHandler.t.sol"; import { LockupHandler } from "../handlers/LockupHandler.t.sol"; +import { LockupHandlerStore } from "../handlers/LockupHandlerStore.t.sol"; /// @title Lockup_Invariant_Test /// @notice Common invariant test logic needed across contracts that inherit from {SablierV2Lockup}. @@ -20,6 +21,7 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { FlashLoanHandler internal flashLoanHandler; ISablierV2Lockup internal lockup; LockupHandler internal lockupHandler; + LockupHandlerStore internal lockupHandlerStore; /*////////////////////////////////////////////////////////////////////////// SET-UP FUNCTION @@ -27,6 +29,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { function setUp() public virtual override { Invariant_Test.setUp(); + + // Deploy the lockup lockupHandler lockupHandlerStore. + lockupHandlerStore = new LockupHandlerStore(); } /*////////////////////////////////////////////////////////////////////////// @@ -37,13 +42,13 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { function invariant_ContractBalance() external { uint256 contractBalance = DEFAULT_ASSET.balanceOf(address(lockup)); uint256 protocolRevenues = lockup.getProtocolRevenues(DEFAULT_ASSET); - uint256 returnedAmountsSum = lockupHandler.returnedAmountsSum(); + uint256 returnedAmountsSum = lockupHandlerStore.returnedAmountsSum(); - uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); uint256 depositAmountsSum; uint256 withdrawnAmountsSum; for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); depositAmountsSum += uint256(lockup.getDepositAmount(streamId)); withdrawnAmountsSum += uint256(lockup.getWithdrawnAmount(streamId)); unchecked { @@ -59,9 +64,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_DepositAmountGteStreamedAmount() external { - uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); assertGte( lockup.getDepositAmount(streamId), lockup.streamedAmountOf(streamId), @@ -74,9 +79,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_DepositAmountGteWithdrawableAmount() external { - uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); assertGte( lockup.getDepositAmount(streamId), lockup.withdrawableAmountOf(streamId), @@ -89,9 +94,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_DepositAmountGteWithdrawnAmount() external { - uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); assertGte( lockup.getDepositAmount(streamId), lockup.getWithdrawnAmount(streamId), @@ -104,9 +109,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_EndTimeGteStartTime() external { - uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); assertGte( lockup.getEndTime(streamId), lockup.getStartTime(streamId), @@ -119,7 +124,7 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_NextStreamIdIncrement() external { - uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 1; i < lastStreamId; ) { uint256 nextStreamId = lockup.nextStreamId(); assertEq(nextStreamId, lastStreamId + 1, "Invariant violated: nonce did not increment"); @@ -130,9 +135,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_StreamedAmountGteWithdrawableAmount() external { - uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); assertGte( lockup.streamedAmountOf(streamId), lockup.withdrawableAmountOf(streamId), @@ -145,9 +150,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_StreamedAmountGteWithdrawnAmount() external { - uint256 lastStreamId = lockupHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); assertGte( lockup.streamedAmountOf(streamId), lockup.getWithdrawnAmount(streamId), diff --git a/test/invariant/lockup/linear/Linear.t.sol b/test/invariant/lockup/linear/Linear.t.sol index 41007f88f..68916f11f 100644 --- a/test/invariant/lockup/linear/Linear.t.sol +++ b/test/invariant/lockup/linear/Linear.t.sol @@ -11,6 +11,7 @@ import { Lockup, LockupLinear } from "src/types/DataTypes.sol"; import { Lockup_Invariant_Test } from "../Lockup.t.sol"; import { FlashLoanHandler } from "../../handlers/FlashLoanHandler.t.sol"; import { LockupLinearHandler } from "../../handlers/LockupLinearHandler.t.sol"; +import { LockupLinearCreateHandler } from "../../handlers/LockupLinearCreateHandler.t.sol"; /// @title Linear_Invariant_Test /// @dev Invariants for the {SablierV2LockupLinear} contract. @@ -20,6 +21,7 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { //////////////////////////////////////////////////////////////////////////*/ LockupLinearHandler internal linearHandler; + LockupLinearCreateHandler internal linearCreateHandler; /*////////////////////////////////////////////////////////////////////////// SET-UP FUNCTION @@ -28,13 +30,18 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { function setUp() public virtual override { Lockup_Invariant_Test.setUp(); - // Deploy the linear contract and its handler. + // Deploy the linear contract and its handlers. linear = new SablierV2LockupLinear({ initialAdmin: users.admin, initialComptroller: comptroller, maxFee: DEFAULT_MAX_FEE }); - linearHandler = new LockupLinearHandler({ admin_: users.admin, asset_: DEFAULT_ASSET, linear_: linear }); + linearHandler = new LockupLinearHandler({ asset_: DEFAULT_ASSET, linear_: linear, store_: lockupHandlerStore }); + linearCreateHandler = new LockupLinearCreateHandler({ + asset_: DEFAULT_ASSET, + linear_: linear, + store_: lockupHandlerStore + }); // Cast the linear contract as {SablierV2Lockup} and the linear handler as {LockupHandler}. lockup = linear; @@ -48,9 +55,10 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { receiver_: goodFlashLoanReceiver }); - // Target the flash loan handler and the linear handler for invariant testing. + // Target the flash loan handler and the linear handlers for invariant testing. targetContract(address(flashLoanHandler)); targetContract(address(linearHandler)); + targetContract(address(linearCreateHandler)); // Label the linear contract and its handler. vm.label({ account: address(linear), newLabel: "LockupLinear" }); @@ -64,9 +72,9 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { // prettier-ignore // solhint-disable max-line-length function invariant_NullStatus() external { - uint256 lastStreamId = linearHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId;) { - uint256 streamId = linearHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); LockupLinear.Stream memory actualStream = linear.getStream(streamId); address actualRecipient = lockup.getRecipient(streamId); @@ -94,9 +102,9 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { } function invariant_CliffTimeGteStartTime() external { - uint256 lastStreamId = linearHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = linearHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); assertGte( linear.getCliffTime(streamId), linear.getStartTime(streamId), @@ -109,9 +117,9 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { } function invariant_EndTimeGteCliffTime() external { - uint256 lastStreamId = linearHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = linearHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); assertGte( linear.getEndTime(streamId), linear.getCliffTime(streamId), @@ -144,8 +152,8 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { console2.log("burn ", linearHandler.calls("burn")); console2.log("cancel ", linearHandler.calls("cancel")); console2.log("claimProtocolRevenues", linearHandler.calls("claimProtocolRevenues")); - console2.log("createWithRange ", linearHandler.calls("createWithRange")); - console2.log("createWithDurations ", linearHandler.calls("createWithDurations")); + console2.log("createWithRange ", linearCreateHandler.calls("createWithRange")); + console2.log("createWithDurations ", linearCreateHandler.calls("createWithDurations")); console2.log("renounce ", linearHandler.calls("renounce")); console2.log("transferNFT ", linearHandler.calls("transferNFT")); console2.log("withdraw ", linearHandler.calls("withdraw")); diff --git a/test/invariant/lockup/pro/Pro.t.sol b/test/invariant/lockup/pro/Pro.t.sol index 3d5b14e6f..56c736539 100644 --- a/test/invariant/lockup/pro/Pro.t.sol +++ b/test/invariant/lockup/pro/Pro.t.sol @@ -11,6 +11,7 @@ import { Lockup, LockupPro } from "src/types/DataTypes.sol"; import { Lockup_Invariant_Test } from "../Lockup.t.sol"; import { FlashLoanHandler } from "../../handlers/FlashLoanHandler.t.sol"; import { LockupProHandler } from "../../handlers/LockupProHandler.t.sol"; +import { LockupProCreateHandler } from "../../handlers/LockupProCreateHandler.t.sol"; /// @title Pro_Invariant_Test /// @dev Invariants for the {SablierV2LockupPro} contract. @@ -20,6 +21,7 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { //////////////////////////////////////////////////////////////////////////*/ LockupProHandler internal proHandler; + LockupProCreateHandler internal proCreateHandler; /*////////////////////////////////////////////////////////////////////////// SET-UP FUNCTION @@ -28,18 +30,19 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { function setUp() public virtual override { Lockup_Invariant_Test.setUp(); - // Deploy the pro contract and its handler. + // Deploy the pro contract and its handlers. pro = new SablierV2LockupPro({ initialAdmin: users.admin, initialComptroller: comptroller, maxFee: DEFAULT_MAX_FEE, maxSegmentCount: DEFAULT_MAX_SEGMENT_COUNT }); - proHandler = new LockupProHandler({ - admin_: users.admin, + proHandler = new LockupProHandler({ asset_: DEFAULT_ASSET, pro_: pro, store_: lockupHandlerStore }); + proCreateHandler = new LockupProCreateHandler({ asset_: DEFAULT_ASSET, comptroller_: comptroller, - pro_: pro + pro_: pro, + store_: lockupHandlerStore }); // Cast the pro contract as {SablierV2Lockup} and the pro handler as {LockupHandler}. @@ -54,9 +57,10 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { receiver_: goodFlashLoanReceiver }); - // Target the flash loan handler and the pro handler for invariant testing. + // Target the flash loan handler and the pro handlers for invariant testing. targetContract(address(flashLoanHandler)); targetContract(address(proHandler)); + targetContract(address(proCreateHandler)); // Label the pro contract and its handler. vm.label({ account: address(pro), newLabel: "LockupPro" }); @@ -70,9 +74,9 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { // prettier-ignore // solhint-disable max-line-length function invariant_NullStatus() external { - uint256 lastStreamId = proHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = proHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); LockupPro.Stream memory actualStream = pro.getStream(streamId); address actualRecipient = lockup.getRecipient(streamId); @@ -101,9 +105,9 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { function invariant_SegmentMilestonesOrdered() external { unchecked { - uint256 lastStreamId = proHandler.lastStreamId(); + uint256 lastStreamId = lockupHandlerStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ++i) { - uint256 streamId = proHandler.streamIds(i); + uint256 streamId = lockupHandlerStore.streamIds(i); // If the stream is null, it doesn't have segments. if (pro.getStatus(streamId) != Lockup.Status.NULL) { @@ -145,8 +149,8 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { console2.log("burn ", proHandler.calls("burn")); console2.log("cancel ", proHandler.calls("cancel")); console2.log("claimProtocolRevenues", proHandler.calls("claimProtocolRevenues")); - console2.log("createWithDeltas ", proHandler.calls("createWithDeltas")); - console2.log("createWithMilestones ", proHandler.calls("createWithMilestones")); + console2.log("createWithDeltas ", proCreateHandler.calls("createWithDeltas")); + console2.log("createWithMilestones ", proCreateHandler.calls("createWithMilestones")); console2.log("renounce ", proHandler.calls("renounce")); console2.log("transferNFT ", proHandler.calls("transferNFT")); console2.log("withdraw ", proHandler.calls("withdraw")); From 9db6ddbc32d687066c47816e142b23f353985fb9 Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Sat, 4 Feb 2023 14:20:38 +0200 Subject: [PATCH 08/11] ci: lower test invariant depth and runs ci: change fuzz runs to 50,000 in CI --- .github/workflows/ci.yml | 4 +--- foundry.toml | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8dc70deaa..7d3db18e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -122,7 +122,7 @@ jobs: test-fuzz: env: - FOUNDRY_FUZZ_RUNS: "20000" + FOUNDRY_FUZZ_RUNS: "50000" needs: ["lint", "build"] runs-on: "ubuntu-latest" steps: @@ -150,8 +150,6 @@ jobs: echo "✅ Passed" >> $GITHUB_STEP_SUMMARY test-invariant: - env: - FOUNDRY_INVARIANT_RUNS: "10000" needs: ["lint", "build"] runs-on: "ubuntu-latest" steps: diff --git a/foundry.toml b/foundry.toml index 236291f54..9f55d44f7 100644 --- a/foundry.toml +++ b/foundry.toml @@ -39,7 +39,7 @@ # Test the optimized contracts without re-compiling them [profile.test-optimized] fuzz = { runs = 5_000 } - invariant = { depth = 1_000, runs = 5_000 } + invariant = { depth = 250, runs = 200 } src = "test" verbosity = 4 From fa5c007af048ec7b9a840f4b70fcd6d78650cb60 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 6 Feb 2023 23:14:57 +0200 Subject: [PATCH 09/11] test: inherit "Base_Test" in "BaseHandler" test: rename "LockupHandlerStore" to "LockupHandlerStorage" test: use "excludeSender" cheatcode for core contracts test: use "excludeSender" cheatcode for handlers contracts in the invariants tests test: remove unnecessary function from "LockupHandlerStorage" test: rename "flashLoan" contract to "flashLoanContract" test: rename "flashLoanFunction" function to "flashLoan" test: rename "store" to "_storage" chore: remove unused imports --- test/Base.t.sol | 2 +- test/invariant/Invariant.t.sol | 7 ++- test/invariant/handlers/BaseHandler.t.sol | 18 ++----- .../handlers/ComptrollerHandler.t.sol | 6 --- .../invariant/handlers/FlashLoanHandler.t.sol | 23 ++++----- test/invariant/handlers/LockupHandler.t.sol | 28 +++++------ ...Store.t.sol => LockupHandlerStorage.t.sol} | 8 +-- .../handlers/LockupLinearCreateHandler.t.sol | 33 ++++-------- .../handlers/LockupLinearHandler.t.sol | 6 +-- .../handlers/LockupProCreateHandler.t.sol | 20 ++++---- .../invariant/handlers/LockupProHandler.t.sol | 6 +-- test/invariant/lockup/Lockup.t.sol | 50 +++++++++---------- test/invariant/lockup/linear/Linear.t.sol | 36 ++++++++----- test/invariant/lockup/pro/Pro.t.sol | 22 ++++---- 14 files changed, 122 insertions(+), 143 deletions(-) rename test/invariant/handlers/{LockupHandlerStore.t.sol => LockupHandlerStorage.t.sol} (88%) diff --git a/test/Base.t.sol b/test/Base.t.sol index 0b20cf6cd..347b60740 100644 --- a/test/Base.t.sol +++ b/test/Base.t.sol @@ -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 diff --git a/test/invariant/Invariant.t.sol b/test/invariant/Invariant.t.sol index 00afd71d8..9edd36cd5 100644 --- a/test/invariant/Invariant.t.sol +++ b/test/invariant/Invariant.t.sol @@ -5,7 +5,6 @@ import { InvariantTest as ForgeInvariantTest } from "forge-std/InvariantTest.sol import { SablierV2Comptroller } from "src/SablierV2Comptroller.sol"; -import { GoodFlashLoanReceiver } from "../shared/mockups/flash-loan/GoodFlashLoanReceiver.t.sol"; import { Base_Test } from "../Base.t.sol"; import { ComptrollerHandler } from "./handlers/ComptrollerHandler.t.sol"; @@ -34,6 +33,12 @@ abstract contract Invariant_Test is Base_Test, ForgeInvariantTest { // Target only the comptroller handler for invariant testing (to avoid getting reverts). targetContract(address(comptrollerHandler)); + // Exclude the comptroller, linear and pro for being the `msg.sender`. + excludeSender(address(comptroller)); + + // Exclude the comptroller handler for being the `msg.sender`. + excludeSender(address(comptrollerHandler)); + // Label the base contracts. vm.label({ account: address(comptroller), newLabel: "Comptroller" }); vm.label({ account: address(comptrollerHandler), newLabel: "ComptrollerHandler" }); diff --git a/test/invariant/handlers/BaseHandler.t.sol b/test/invariant/handlers/BaseHandler.t.sol index 2fc6deace..b7ff03c8f 100644 --- a/test/invariant/handlers/BaseHandler.t.sol +++ b/test/invariant/handlers/BaseHandler.t.sol @@ -8,23 +8,11 @@ import { Calculations } from "../../shared/helpers/Calculations.t.sol"; import { Constants } from "../../shared/helpers/Constants.t.sol"; import { Utils } from "../../shared/helpers/Utils.t.sol"; +import { Base_Test } from "../../Base.t.sol"; + /// @title BaseHandler /// @notice Base contract with common logic needed by all handler contracts. -abstract contract BaseHandler is Constants, Calculations, Utils, StdCheats { - /*////////////////////////////////////////////////////////////////////////// - CONSTANTS - //////////////////////////////////////////////////////////////////////////*/ - - /// @dev The address of the HEVM contract. - address internal constant HEVM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code")))); - - /*////////////////////////////////////////////////////////////////////////// - TEST CONTRACTS - //////////////////////////////////////////////////////////////////////////*/ - - /// @dev An instance of the HEVM. - Vm internal constant vm = Vm(HEVM_ADDRESS); - +abstract contract BaseHandler is Base_Test { /*////////////////////////////////////////////////////////////////////////// TEST VARIABLES //////////////////////////////////////////////////////////////////////////*/ diff --git a/test/invariant/handlers/ComptrollerHandler.t.sol b/test/invariant/handlers/ComptrollerHandler.t.sol index a98b1d3ad..9a0ff5de0 100644 --- a/test/invariant/handlers/ComptrollerHandler.t.sol +++ b/test/invariant/handlers/ComptrollerHandler.t.sol @@ -12,12 +12,6 @@ import { BaseHandler } from "./BaseHandler.t.sol"; /// @dev This contract and not {SablierV2Comptroller} is exposed to Foundry for invariant testing. The point is /// to bound and restrict the inputs that get passed to the real-world contract to avoid getting reverts. contract ComptrollerHandler is BaseHandler { - /*////////////////////////////////////////////////////////////////////////// - TEST CONTRACTS - //////////////////////////////////////////////////////////////////////////*/ - - ISablierV2Comptroller public comptroller; - /*////////////////////////////////////////////////////////////////////////// CONSTRUCTOR //////////////////////////////////////////////////////////////////////////*/ diff --git a/test/invariant/handlers/FlashLoanHandler.t.sol b/test/invariant/handlers/FlashLoanHandler.t.sol index e50b80b07..4763c9a51 100644 --- a/test/invariant/handlers/FlashLoanHandler.t.sol +++ b/test/invariant/handlers/FlashLoanHandler.t.sol @@ -8,7 +8,6 @@ import { UD60x18 } from "@prb/math/UD60x18.sol"; import { SablierV2FlashLoan } from "src/abstracts/SablierV2FlashLoan.sol"; import { ISablierV2Comptroller } from "src/interfaces/ISablierV2Comptroller.sol"; -import { GoodFlashLoanReceiver } from "../../shared/mockups/flash-loan/GoodFlashLoanReceiver.t.sol"; import { BaseHandler } from "./BaseHandler.t.sol"; /// @title FlashLoanHandler @@ -16,12 +15,11 @@ import { BaseHandler } from "./BaseHandler.t.sol"; /// to bound and restrict the inputs that get passed to the real-world contract to avoid getting reverts. contract FlashLoanHandler is BaseHandler { /*////////////////////////////////////////////////////////////////////////// - PUBLIC TEST CONTRACTS + TEST CONTRACTS //////////////////////////////////////////////////////////////////////////*/ IERC20 public asset; - ISablierV2Comptroller public comptroller; - SablierV2FlashLoan public flashLoan; + SablierV2FlashLoan public flashLoanContract; IERC3156FlashBorrower internal receiver; /*////////////////////////////////////////////////////////////////////////// @@ -31,20 +29,20 @@ contract FlashLoanHandler is BaseHandler { constructor( IERC20 asset_, ISablierV2Comptroller comptroller_, - SablierV2FlashLoan flashLoan_, + SablierV2FlashLoan flashLoanContract_, IERC3156FlashBorrower receiver_ ) { asset = asset_; comptroller = comptroller_; - flashLoan = flashLoan_; - receiver = receiver_; + flashLoanContract = flashLoanContract_; + receiver_ = receiver; } /*////////////////////////////////////////////////////////////////////////// SABLIER-V2-FLASH-LOAN //////////////////////////////////////////////////////////////////////////*/ - function flashLoanFunction(uint128 amount) external instrument("flashLoan") { + function flashLoan(uint128 amount) external instrument("flashLoan") { // Only supported ERC-20 assets can be flash loaned. bool isFlashLoanable = comptroller.isFlashLoanable(asset); if (!isFlashLoanable) { @@ -52,26 +50,23 @@ contract FlashLoanHandler is BaseHandler { } // The flash fee must be less than or equal to type(uint128).max - uint256 fee = flashLoan.flashFee(address(asset), amount); + uint256 fee = flashLoanContract.flashFee(address(asset), amount); if (fee > type(uint128).max) { return; } // Mint the flash loan amount to the contract. - deal({ token: address(asset), to: address(flashLoan), give: amount }); + deal({ token: address(asset), to: address(flashLoanContract), give: amount }); // Mint the flash fee to the receiver so that they can repay the flash loan. deal({ token: address(asset), to: address(receiver), give: fee }); // Execute the flash loan. - bool response = flashLoan.flashLoan({ + flashLoanContract.flashLoan({ receiver: receiver, asset: address(asset), amount: amount, data: bytes("Some Data") }); - - // Silence the compiler warning. - response; } } diff --git a/test/invariant/handlers/LockupHandler.t.sol b/test/invariant/handlers/LockupHandler.t.sol index eb612adbe..98e69b5b5 100644 --- a/test/invariant/handlers/LockupHandler.t.sol +++ b/test/invariant/handlers/LockupHandler.t.sol @@ -7,18 +7,18 @@ import { ISablierV2Lockup } from "src/interfaces/ISablierV2Lockup.sol"; import { Broker, Lockup } from "src/types/DataTypes.sol"; import { BaseHandler } from "./BaseHandler.t.sol"; -import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; +import { LockupHandlerStorage } from "./LockupHandlerStorage.t.sol"; /// @title LockupHandler /// @dev Common handler logic between {SablierV2LockupLinear} and {SablierV2LockupPro}. abstract contract LockupHandler is BaseHandler { /*////////////////////////////////////////////////////////////////////////// - PUBLIC TEST CONTRACTS + TEST CONTRACTS //////////////////////////////////////////////////////////////////////////*/ IERC20 public asset; ISablierV2Lockup public lockup; - LockupHandlerStore public store; + LockupHandlerStorage public _storage; /*////////////////////////////////////////////////////////////////////////// PRIVATE TEST VARIABLES @@ -32,10 +32,10 @@ abstract contract LockupHandler is BaseHandler { CONSTRUCTOR //////////////////////////////////////////////////////////////////////////*/ - constructor(IERC20 asset_, ISablierV2Lockup lockup_, LockupHandlerStore store_) { + constructor(IERC20 asset_, ISablierV2Lockup lockup_, LockupHandlerStorage _storage_) { asset = asset_; lockup = lockup_; - store = store_; + _storage = _storage_; } /*////////////////////////////////////////////////////////////////////////// @@ -50,24 +50,24 @@ abstract contract LockupHandler is BaseHandler { } modifier useFuzzedStreamRecipient(uint256 streamIndexSeed) { - uint256 lastStreamId = store.lastStreamId(); + uint256 lastStreamId = _storage.lastStreamId(); if (lastStreamId == 0) { return; } - currentStreamId = store.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); - currentRecipient = store.streamIdsToRecipients(currentStreamId); + currentStreamId = _storage.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); + currentRecipient = _storage.streamIdsToRecipients(currentStreamId); vm.startPrank(currentRecipient); _; vm.stopPrank(); } modifier useFuzzedStreamSender(uint256 streamIndexSeed) { - uint256 lastStreamId = store.lastStreamId(); + uint256 lastStreamId = _storage.lastStreamId(); if (lastStreamId == 0) { return; } - currentStreamId = store.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); - currentSender = store.streamIdsToSenders(currentStreamId); + currentStreamId = _storage.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); + currentSender = _storage.streamIdsToSenders(currentStreamId); vm.startPrank(currentSender); _; vm.stopPrank(); @@ -93,7 +93,7 @@ abstract contract LockupHandler is BaseHandler { lockup.burn(currentStreamId); // Set the recipient associated with this stream to the zero address. - store.updateRecipient(currentStreamId, address(0)); + _storage.updateRecipient(currentStreamId, address(0)); } function cancel(uint256 streamIndexSeed) external instrument("cancel") useFuzzedStreamSender(streamIndexSeed) { @@ -106,7 +106,7 @@ abstract contract LockupHandler is BaseHandler { // Record the returned amount by adding it to the ghost variable `returnedAmountsSum`. This is needed to // check invariants against the contract's balance. uint128 returnedAmount = lockup.returnableAmountOf(currentStreamId); - store.addReturnedAmount(returnedAmount); + _storage.addReturnedAmount(returnedAmount); // Cancel the stream. lockup.cancel(currentStreamId); @@ -210,6 +210,6 @@ abstract contract LockupHandler is BaseHandler { lockup.transferFrom({ from: currentRecipient, to: newRecipient, tokenId: currentStreamId }); // Update the recipient associated with this stream id. - store.updateRecipient(currentStreamId, newRecipient); + _storage.updateRecipient(currentStreamId, newRecipient); } } diff --git a/test/invariant/handlers/LockupHandlerStore.t.sol b/test/invariant/handlers/LockupHandlerStorage.t.sol similarity index 88% rename from test/invariant/handlers/LockupHandlerStore.t.sol rename to test/invariant/handlers/LockupHandlerStorage.t.sol index 02bfbbd87..ec462f3c7 100644 --- a/test/invariant/handlers/LockupHandlerStore.t.sol +++ b/test/invariant/handlers/LockupHandlerStorage.t.sol @@ -1,9 +1,9 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.13 <0.9.0; -/// @title LockupHandlerStore +/// @title LockupHandlerStorage /// @dev Storage contract for the lockup handler streams. -contract LockupHandlerStore { +contract LockupHandlerStorage { /*////////////////////////////////////////////////////////////////////////// PUBLIC TEST VARIABLES //////////////////////////////////////////////////////////////////////////*/ @@ -35,8 +35,4 @@ contract LockupHandlerStore { function updateRecipient(uint256 streamId, address newRecipient) external { streamIdsToRecipients[streamId] = newRecipient; } - - function updateSender(uint256 streamId, address newSender) external { - streamIdsToSenders[streamId] = newSender; - } } diff --git a/test/invariant/handlers/LockupLinearCreateHandler.t.sol b/test/invariant/handlers/LockupLinearCreateHandler.t.sol index cb0f6b3cc..cf4027748 100644 --- a/test/invariant/handlers/LockupLinearCreateHandler.t.sol +++ b/test/invariant/handlers/LockupLinearCreateHandler.t.sol @@ -7,7 +7,7 @@ import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.so import { Broker, LockupLinear } from "src/types/DataTypes.sol"; import { BaseHandler } from "./BaseHandler.t.sol"; -import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; +import { LockupHandlerStorage } from "./LockupHandlerStorage.t.sol"; /// @title LockupLinearCreateHandler /// @dev This contract is a complement of {LockupLinearHandler}. The goal is to bias the invariant calls @@ -20,21 +20,20 @@ contract LockupLinearCreateHandler is BaseHandler { uint256 internal constant MAX_STREAM_COUNT = 100; /*////////////////////////////////////////////////////////////////////////// - PUBLIC TEST CONTRACTS + TEST CONTRACTS //////////////////////////////////////////////////////////////////////////*/ IERC20 public asset; - ISablierV2LockupLinear public linear; - LockupHandlerStore public store; + LockupHandlerStorage public _storage; /*////////////////////////////////////////////////////////////////////////// CONSTRUCTOR //////////////////////////////////////////////////////////////////////////*/ - constructor(IERC20 asset_, ISablierV2LockupLinear linear_, LockupHandlerStore store_) { + constructor(IERC20 asset_, ISablierV2LockupLinear linear_, LockupHandlerStorage _storage_) { asset = asset_; linear = linear_; - store = store_; + _storage = _storage_; } /*////////////////////////////////////////////////////////////////////////// @@ -60,10 +59,6 @@ contract LockupLinearCreateHandler is BaseHandler { uint128 totalAmount; } - struct CreateWithDurationsVars { - uint256 streamId; - } - function createWithDurations( CreateWithDurationsParams memory params ) public instrument("createWithDurations") useNewSender(params.sender) { @@ -73,7 +68,7 @@ contract LockupLinearCreateHandler is BaseHandler { params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); // We don't want to fuzz more than a certain number of streams. - if (store.lastStreamId() >= MAX_STREAM_COUNT) { + if (_storage.lastStreamId() >= MAX_STREAM_COUNT) { return; } @@ -89,8 +84,7 @@ contract LockupLinearCreateHandler is BaseHandler { asset.approve({ spender: address(linear), amount: params.totalAmount }); // Create the stream. - CreateWithDurationsVars memory vars; - vars.streamId = linear.createWithDurations({ + uint256 streamId = linear.createWithDurations({ sender: params.sender, recipient: params.recipient, totalAmount: params.totalAmount, @@ -101,7 +95,7 @@ contract LockupLinearCreateHandler is BaseHandler { }); // Store the stream id. - store.pushStreamId(vars.streamId, params.sender, params.recipient); + _storage.pushStreamId(streamId, params.sender, params.recipient); } struct CreateWithRangeParams { @@ -113,10 +107,6 @@ contract LockupLinearCreateHandler is BaseHandler { uint128 totalAmount; } - struct CreateWithRangeVars { - uint256 streamId; - } - function createWithRange( CreateWithRangeParams memory params ) public instrument("createWithRange") useNewSender(params.sender) { @@ -127,7 +117,7 @@ contract LockupLinearCreateHandler is BaseHandler { params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); // We don't want to fuzz more than a certain number of streams. - if (store.lastStreamId() >= MAX_STREAM_COUNT) { + if (_storage.lastStreamId() >= MAX_STREAM_COUNT) { return; } @@ -143,8 +133,7 @@ contract LockupLinearCreateHandler is BaseHandler { asset.approve({ spender: address(linear), amount: params.totalAmount }); // Create the stream. - CreateWithRangeVars memory vars; - vars.streamId = linear.createWithRange({ + uint256 streamId = linear.createWithRange({ sender: params.sender, recipient: params.recipient, totalAmount: params.totalAmount, @@ -155,6 +144,6 @@ contract LockupLinearCreateHandler is BaseHandler { }); // Store the stream id. - store.pushStreamId(vars.streamId, params.sender, params.recipient); + _storage.pushStreamId(streamId, params.sender, params.recipient); } } diff --git a/test/invariant/handlers/LockupLinearHandler.t.sol b/test/invariant/handlers/LockupLinearHandler.t.sol index 244175b3a..5d1319131 100644 --- a/test/invariant/handlers/LockupLinearHandler.t.sol +++ b/test/invariant/handlers/LockupLinearHandler.t.sol @@ -6,7 +6,7 @@ import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol"; import { LockupHandler } from "./LockupHandler.t.sol"; -import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; +import { LockupHandlerStorage } from "./LockupHandlerStorage.t.sol"; /// @title LockupLinearHandler /// @dev This contract and not {SablierV2LockupLinear} is exposed to Foundry for invariant testing. The point is @@ -15,6 +15,6 @@ contract LockupLinearHandler is LockupHandler { constructor( IERC20 asset_, ISablierV2LockupLinear linear_, - LockupHandlerStore store_ - ) LockupHandler(asset_, linear_, store_) {} + LockupHandlerStorage _storage_ + ) LockupHandler(asset_, linear_, _storage_) {} } diff --git a/test/invariant/handlers/LockupProCreateHandler.t.sol b/test/invariant/handlers/LockupProCreateHandler.t.sol index 268b5ba05..c43a6c9f8 100644 --- a/test/invariant/handlers/LockupProCreateHandler.t.sol +++ b/test/invariant/handlers/LockupProCreateHandler.t.sol @@ -10,7 +10,7 @@ import { ISablierV2LockupPro } from "src/interfaces/ISablierV2LockupPro.sol"; import { Broker, LockupPro } from "src/types/DataTypes.sol"; import { BaseHandler } from "./BaseHandler.t.sol"; -import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; +import { LockupHandlerStorage } from "./LockupHandlerStorage.t.sol"; /// @title LockupProCreateHandler /// @dev This contract is a complement of {LockupProHandler}. The goal is to bias the invariant calls @@ -23,13 +23,11 @@ contract LockupProCreateHandler is BaseHandler { uint256 internal constant MAX_STREAM_COUNT = 100; /*////////////////////////////////////////////////////////////////////////// - PUBLIC TEST CONTRACTS + TEST CONTRACTS //////////////////////////////////////////////////////////////////////////*/ IERC20 public asset; - ISablierV2Comptroller public comptroller; - ISablierV2LockupPro public pro; - LockupHandlerStore public store; + LockupHandlerStorage public _storage; /*////////////////////////////////////////////////////////////////////////// CONSTRUCTOR @@ -39,12 +37,12 @@ contract LockupProCreateHandler is BaseHandler { IERC20 asset_, ISablierV2Comptroller comptroller_, ISablierV2LockupPro pro_, - LockupHandlerStore store_ + LockupHandlerStorage _storage_ ) { asset = asset_; comptroller = comptroller_; pro = pro_; - store = store_; + _storage = _storage_; } /*////////////////////////////////////////////////////////////////////////// @@ -88,7 +86,7 @@ contract LockupProCreateHandler is BaseHandler { params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); // We don't want to fuzz more than a certain number of streams. - if (store.lastStreamId() > MAX_STREAM_COUNT) { + if (_storage.lastStreamId() > MAX_STREAM_COUNT) { return; } @@ -131,7 +129,7 @@ contract LockupProCreateHandler is BaseHandler { }); // Store the stream id. - store.pushStreamId(vars.streamId, params.sender, params.recipient); + _storage.pushStreamId(vars.streamId, params.sender, params.recipient); } struct CreateWithMilestonesParams { @@ -158,7 +156,7 @@ contract LockupProCreateHandler is BaseHandler { params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); // We don't want to fuzz more than a certain number of streams. - if (store.lastStreamId() >= MAX_STREAM_COUNT) { + if (_storage.lastStreamId() >= MAX_STREAM_COUNT) { return; } @@ -197,6 +195,6 @@ contract LockupProCreateHandler is BaseHandler { }); // Store the stream id. - store.pushStreamId(vars.streamId, params.sender, params.recipient); + _storage.pushStreamId(vars.streamId, params.sender, params.recipient); } } diff --git a/test/invariant/handlers/LockupProHandler.t.sol b/test/invariant/handlers/LockupProHandler.t.sol index 17ec608a9..af87ae353 100644 --- a/test/invariant/handlers/LockupProHandler.t.sol +++ b/test/invariant/handlers/LockupProHandler.t.sol @@ -6,7 +6,7 @@ import { IERC20 } from "@openzeppelin/token/ERC20/IERC20.sol"; import { ISablierV2LockupPro } from "src/interfaces/ISablierV2LockupPro.sol"; import { LockupHandler } from "./LockupHandler.t.sol"; -import { LockupHandlerStore } from "./LockupHandlerStore.t.sol"; +import { LockupHandlerStorage } from "./LockupHandlerStorage.t.sol"; /// @title LockupProHandler /// @dev This contract and not {SablierV2LockupPro} is exposed to Foundry for invariant testing. The point is @@ -15,6 +15,6 @@ contract LockupProHandler is LockupHandler { constructor( IERC20 asset_, ISablierV2LockupPro pro_, - LockupHandlerStore store_ - ) LockupHandler(asset_, pro_, store_) {} + LockupHandlerStorage _storage_ + ) LockupHandler(asset_, pro_, _storage_) {} } diff --git a/test/invariant/lockup/Lockup.t.sol b/test/invariant/lockup/Lockup.t.sol index e453ba59a..952155264 100644 --- a/test/invariant/lockup/Lockup.t.sol +++ b/test/invariant/lockup/Lockup.t.sol @@ -1,15 +1,12 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.13 <0.9.0; -import { SablierV2FlashLoan } from "src/abstracts/SablierV2FlashLoan.sol"; import { ISablierV2Lockup } from "src/interfaces/ISablierV2Lockup.sol"; -import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol"; -import { SablierV2LockupLinear } from "src/SablierV2LockupLinear.sol"; import { Invariant_Test } from "../Invariant.t.sol"; import { FlashLoanHandler } from "../handlers/FlashLoanHandler.t.sol"; import { LockupHandler } from "../handlers/LockupHandler.t.sol"; -import { LockupHandlerStore } from "../handlers/LockupHandlerStore.t.sol"; +import { LockupHandlerStorage } from "../handlers/LockupHandlerStorage.t.sol"; /// @title Lockup_Invariant_Test /// @notice Common invariant test logic needed across contracts that inherit from {SablierV2Lockup}. @@ -21,7 +18,7 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { FlashLoanHandler internal flashLoanHandler; ISablierV2Lockup internal lockup; LockupHandler internal lockupHandler; - LockupHandlerStore internal lockupHandlerStore; + LockupHandlerStorage internal lockupHandlerStorage; /*////////////////////////////////////////////////////////////////////////// SET-UP FUNCTION @@ -30,8 +27,11 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { function setUp() public virtual override { Invariant_Test.setUp(); - // Deploy the lockup lockupHandler lockupHandlerStore. - lockupHandlerStore = new LockupHandlerStore(); + // Deploy the lockupHandlerStorage. + lockupHandlerStorage = new LockupHandlerStorage(); + + // Exclude the lockup handler store for being the `msg.sender`. + excludeSender(address(lockupHandlerStorage)); } /*////////////////////////////////////////////////////////////////////////// @@ -42,13 +42,13 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { function invariant_ContractBalance() external { uint256 contractBalance = DEFAULT_ASSET.balanceOf(address(lockup)); uint256 protocolRevenues = lockup.getProtocolRevenues(DEFAULT_ASSET); - uint256 returnedAmountsSum = lockupHandlerStore.returnedAmountsSum(); + uint256 returnedAmountsSum = lockupHandlerStorage.returnedAmountsSum(); - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); uint256 depositAmountsSum; uint256 withdrawnAmountsSum; for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); depositAmountsSum += uint256(lockup.getDepositAmount(streamId)); withdrawnAmountsSum += uint256(lockup.getWithdrawnAmount(streamId)); unchecked { @@ -64,9 +64,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_DepositAmountGteStreamedAmount() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); assertGte( lockup.getDepositAmount(streamId), lockup.streamedAmountOf(streamId), @@ -79,9 +79,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_DepositAmountGteWithdrawableAmount() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); assertGte( lockup.getDepositAmount(streamId), lockup.withdrawableAmountOf(streamId), @@ -94,9 +94,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_DepositAmountGteWithdrawnAmount() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); assertGte( lockup.getDepositAmount(streamId), lockup.getWithdrawnAmount(streamId), @@ -108,11 +108,11 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } } - function invariant_EndTimeGteStartTime() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + function invariant_EndTimeGtStartTime() external { + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); - assertGte( + uint256 streamId = lockupHandlerStorage.streamIds(i); + assertGt( lockup.getEndTime(streamId), lockup.getStartTime(streamId), "Invariant violated: end time < start time" @@ -124,7 +124,7 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_NextStreamIdIncrement() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 1; i < lastStreamId; ) { uint256 nextStreamId = lockup.nextStreamId(); assertEq(nextStreamId, lastStreamId + 1, "Invariant violated: nonce did not increment"); @@ -135,9 +135,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_StreamedAmountGteWithdrawableAmount() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); assertGte( lockup.streamedAmountOf(streamId), lockup.withdrawableAmountOf(streamId), @@ -150,9 +150,9 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { } function invariant_StreamedAmountGteWithdrawnAmount() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); assertGte( lockup.streamedAmountOf(streamId), lockup.getWithdrawnAmount(streamId), diff --git a/test/invariant/lockup/linear/Linear.t.sol b/test/invariant/lockup/linear/Linear.t.sol index 68916f11f..4f580e88b 100644 --- a/test/invariant/lockup/linear/Linear.t.sol +++ b/test/invariant/lockup/linear/Linear.t.sol @@ -4,7 +4,6 @@ pragma solidity >=0.8.13 <0.9.0; import { console2 } from "forge-std/console2.sol"; import { SablierV2FlashLoan } from "src/abstracts/SablierV2FlashLoan.sol"; -import { ISablierV2LockupLinear } from "src/interfaces/ISablierV2LockupLinear.sol"; import { SablierV2LockupLinear } from "src/SablierV2LockupLinear.sol"; import { Lockup, LockupLinear } from "src/types/DataTypes.sol"; @@ -36,11 +35,15 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { initialComptroller: comptroller, maxFee: DEFAULT_MAX_FEE }); - linearHandler = new LockupLinearHandler({ asset_: DEFAULT_ASSET, linear_: linear, store_: lockupHandlerStore }); + linearHandler = new LockupLinearHandler({ + asset_: DEFAULT_ASSET, + linear_: linear, + _storage_: lockupHandlerStorage + }); linearCreateHandler = new LockupLinearCreateHandler({ asset_: DEFAULT_ASSET, linear_: linear, - store_: lockupHandlerStore + _storage_: lockupHandlerStorage }); // Cast the linear contract as {SablierV2Lockup} and the linear handler as {LockupHandler}. @@ -51,7 +54,7 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { flashLoanHandler = new FlashLoanHandler({ asset_: DEFAULT_ASSET, comptroller_: comptroller, - flashLoan_: SablierV2FlashLoan(address(linear)), + flashLoanContract_: SablierV2FlashLoan(address(linear)), receiver_: goodFlashLoanReceiver }); @@ -60,6 +63,13 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { targetContract(address(linearHandler)); targetContract(address(linearCreateHandler)); + excludeSender(address(linear)); + + // Exclude the linear and the linear handlers for being the `msg.sender`. + excludeSender(address(flashLoanHandler)); + excludeSender(address(linearHandler)); + excludeSender(address(linearCreateHandler)); + // Label the linear contract and its handler. vm.label({ account: address(linear), newLabel: "LockupLinear" }); vm.label({ account: address(linearHandler), newLabel: "LockupLinearHandler" }); @@ -72,16 +82,16 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { // prettier-ignore // solhint-disable max-line-length function invariant_NullStatus() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId;) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); LockupLinear.Stream memory actualStream = linear.getStream(streamId); address actualRecipient = lockup.getRecipient(streamId); // If the stream is null, it should contain only zero values. if (lockup.getStatus(streamId) == Lockup.Status.NULL) { assertEq(actualStream.amounts.deposit, 0, "Invariant violated: stream null, deposit amount not zero"); - assertEq( actualStream.amounts.withdrawn, 0, "Invariant violated: stream null, withdrawn amount not zero"); + assertEq(actualStream.amounts.withdrawn, 0, "Invariant violated: stream null, withdrawn amount not zero"); assertEq(address(actualStream.asset), address(0), "Invariant violated: stream null, asset not zero address"); assertEq(actualStream.isCancelable, false, "Invariant violated: stream null, isCancelable not false"); assertEq(actualStream.range.cliff, 0, "Invariant violated: stream null, cliff time not zero"); @@ -102,9 +112,9 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { } function invariant_CliffTimeGteStartTime() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); assertGte( linear.getCliffTime(streamId), linear.getStartTime(streamId), @@ -116,11 +126,11 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { } } - function invariant_EndTimeGteCliffTime() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + function invariant_EndTimeGtCliffTime() external { + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); - assertGte( + uint256 streamId = lockupHandlerStorage.streamIds(i); + assertGt( linear.getEndTime(streamId), linear.getCliffTime(streamId), "Invariant violated: end time < cliff time" diff --git a/test/invariant/lockup/pro/Pro.t.sol b/test/invariant/lockup/pro/Pro.t.sol index 56c736539..5d1bb21a3 100644 --- a/test/invariant/lockup/pro/Pro.t.sol +++ b/test/invariant/lockup/pro/Pro.t.sol @@ -4,7 +4,6 @@ pragma solidity >=0.8.13 <0.9.0; import { console2 } from "forge-std/console2.sol"; import { SablierV2FlashLoan } from "src/abstracts/SablierV2FlashLoan.sol"; -import { ISablierV2LockupPro } from "src/interfaces/ISablierV2LockupPro.sol"; import { SablierV2LockupPro } from "src/SablierV2LockupPro.sol"; import { Lockup, LockupPro } from "src/types/DataTypes.sol"; @@ -37,12 +36,12 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { maxFee: DEFAULT_MAX_FEE, maxSegmentCount: DEFAULT_MAX_SEGMENT_COUNT }); - proHandler = new LockupProHandler({ asset_: DEFAULT_ASSET, pro_: pro, store_: lockupHandlerStore }); + proHandler = new LockupProHandler({ asset_: DEFAULT_ASSET, pro_: pro, _storage_: lockupHandlerStorage }); proCreateHandler = new LockupProCreateHandler({ asset_: DEFAULT_ASSET, comptroller_: comptroller, pro_: pro, - store_: lockupHandlerStore + _storage_: lockupHandlerStorage }); // Cast the pro contract as {SablierV2Lockup} and the pro handler as {LockupHandler}. @@ -53,7 +52,7 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { flashLoanHandler = new FlashLoanHandler({ asset_: DEFAULT_ASSET, comptroller_: comptroller, - flashLoan_: SablierV2FlashLoan(address(pro)), + flashLoanContract_: SablierV2FlashLoan(address(pro)), receiver_: goodFlashLoanReceiver }); @@ -62,6 +61,11 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { targetContract(address(proHandler)); targetContract(address(proCreateHandler)); + // Exclude the pro handlers for being the `msg.sender`. + excludeSender(address(flashLoanHandler)); + excludeSender(address(proHandler)); + excludeSender(address(proCreateHandler)); + // Label the pro contract and its handler. vm.label({ account: address(pro), newLabel: "LockupPro" }); vm.label({ account: address(lockupHandler), newLabel: "LockupProHandler" }); @@ -74,16 +78,16 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { // prettier-ignore // solhint-disable max-line-length function invariant_NullStatus() external { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); LockupPro.Stream memory actualStream = pro.getStream(streamId); address actualRecipient = lockup.getRecipient(streamId); // If the stream is null, it should contain only zero values. if (lockup.getStatus(streamId) == Lockup.Status.NULL) { assertEq(actualStream.amounts.deposit, 0, "Invariant violated: stream null, deposit amount not zero"); - assertEq( actualStream.amounts.withdrawn, 0, "Invariant violated: stream null, withdrawn amount not zero"); + assertEq(actualStream.amounts.withdrawn, 0, "Invariant violated: stream null, withdrawn amount not zero"); assertEq(address(actualStream.asset), address(0), "Invariant violated: stream null, asset not zero address"); assertEq(actualStream.range.end, 0, "Invariant violated: stream null, end time not zero"); assertEq(actualStream.range.start, 0, "Invariant violated: stream null, start time not zero"); @@ -105,9 +109,9 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { function invariant_SegmentMilestonesOrdered() external { unchecked { - uint256 lastStreamId = lockupHandlerStore.lastStreamId(); + uint256 lastStreamId = lockupHandlerStorage.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ++i) { - uint256 streamId = lockupHandlerStore.streamIds(i); + uint256 streamId = lockupHandlerStorage.streamIds(i); // If the stream is null, it doesn't have segments. if (pro.getStatus(streamId) != Lockup.Status.NULL) { From 06fa329c0b21359d8b329f0efb09a4737f6652cf Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 6 Feb 2023 23:36:14 +0200 Subject: [PATCH 10/11] test: deploy the protocol with "deployProtocol()" --- test/invariant/Invariant.t.sol | 11 +++++++---- test/invariant/lockup/linear/Linear.t.sol | 14 +++----------- test/invariant/lockup/pro/Pro.t.sol | 11 ++--------- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/test/invariant/Invariant.t.sol b/test/invariant/Invariant.t.sol index 9edd36cd5..c3b7a7646 100644 --- a/test/invariant/Invariant.t.sol +++ b/test/invariant/Invariant.t.sol @@ -24,8 +24,10 @@ abstract contract Invariant_Test is Base_Test, ForgeInvariantTest { function setUp() public virtual override { Base_Test.setUp(); - // Deploy the comptroller and its handler. - comptroller = new SablierV2Comptroller({ initialAdmin: users.admin }); + // Deploy the entire protocol. + deployProtocol(); + + // Deploy the comptroller handler. comptrollerHandler = new ComptrollerHandler(comptroller); vm.prank({ msgSender: users.admin }); comptroller.transferAdmin(address(comptrollerHandler)); @@ -35,12 +37,13 @@ abstract contract Invariant_Test is Base_Test, ForgeInvariantTest { // Exclude the comptroller, linear and pro for being the `msg.sender`. excludeSender(address(comptroller)); + excludeSender(address(linear)); + excludeSender(address(pro)); // Exclude the comptroller handler for being the `msg.sender`. excludeSender(address(comptrollerHandler)); - // Label the base contracts. - vm.label({ account: address(comptroller), newLabel: "Comptroller" }); + // Label the comptroller handler. vm.label({ account: address(comptrollerHandler), newLabel: "ComptrollerHandler" }); } } diff --git a/test/invariant/lockup/linear/Linear.t.sol b/test/invariant/lockup/linear/Linear.t.sol index 4f580e88b..58d1bbc36 100644 --- a/test/invariant/lockup/linear/Linear.t.sol +++ b/test/invariant/lockup/linear/Linear.t.sol @@ -29,12 +29,7 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { function setUp() public virtual override { Lockup_Invariant_Test.setUp(); - // Deploy the linear contract and its handlers. - linear = new SablierV2LockupLinear({ - initialAdmin: users.admin, - initialComptroller: comptroller, - maxFee: DEFAULT_MAX_FEE - }); + // Deploy the linear contract handlers. linearHandler = new LockupLinearHandler({ asset_: DEFAULT_ASSET, linear_: linear, @@ -63,15 +58,12 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { targetContract(address(linearHandler)); targetContract(address(linearCreateHandler)); - excludeSender(address(linear)); - - // Exclude the linear and the linear handlers for being the `msg.sender`. + // Exclude the linear handlers for being the `msg.sender`. excludeSender(address(flashLoanHandler)); excludeSender(address(linearHandler)); excludeSender(address(linearCreateHandler)); - // Label the linear contract and its handler. - vm.label({ account: address(linear), newLabel: "LockupLinear" }); + // Label the linear handler. vm.label({ account: address(linearHandler), newLabel: "LockupLinearHandler" }); } diff --git a/test/invariant/lockup/pro/Pro.t.sol b/test/invariant/lockup/pro/Pro.t.sol index 5d1bb21a3..c347e08b6 100644 --- a/test/invariant/lockup/pro/Pro.t.sol +++ b/test/invariant/lockup/pro/Pro.t.sol @@ -29,13 +29,7 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { function setUp() public virtual override { Lockup_Invariant_Test.setUp(); - // Deploy the pro contract and its handlers. - pro = new SablierV2LockupPro({ - initialAdmin: users.admin, - initialComptroller: comptroller, - maxFee: DEFAULT_MAX_FEE, - maxSegmentCount: DEFAULT_MAX_SEGMENT_COUNT - }); + // Deploy the pro contract handlers. proHandler = new LockupProHandler({ asset_: DEFAULT_ASSET, pro_: pro, _storage_: lockupHandlerStorage }); proCreateHandler = new LockupProCreateHandler({ asset_: DEFAULT_ASSET, @@ -66,8 +60,7 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { excludeSender(address(proHandler)); excludeSender(address(proCreateHandler)); - // Label the pro contract and its handler. - vm.label({ account: address(pro), newLabel: "LockupPro" }); + // Label the pro handler. vm.label({ account: address(lockupHandler), newLabel: "LockupProHandler" }); } From eb5485697af938a2cc82b27be3361fb160314418 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 6 Feb 2023 23:59:01 +0200 Subject: [PATCH 11/11] test: rename "_storage" contract to "store" test: improve wording in comments --- test/invariant/Invariant.t.sol | 4 ++-- test/invariant/handlers/LockupHandler.t.sol | 24 +++++++++---------- .../handlers/LockupLinearCreateHandler.t.sol | 14 +++++------ .../handlers/LockupLinearHandler.t.sol | 4 ++-- .../handlers/LockupProCreateHandler.t.sol | 14 +++++------ .../invariant/handlers/LockupProHandler.t.sol | 4 ++-- test/invariant/lockup/Lockup.t.sol | 2 +- test/invariant/lockup/linear/Linear.t.sol | 6 ++--- test/invariant/lockup/pro/Pro.t.sol | 6 ++--- 9 files changed, 39 insertions(+), 39 deletions(-) diff --git a/test/invariant/Invariant.t.sol b/test/invariant/Invariant.t.sol index c3b7a7646..45b0fb464 100644 --- a/test/invariant/Invariant.t.sol +++ b/test/invariant/Invariant.t.sol @@ -35,12 +35,12 @@ abstract contract Invariant_Test is Base_Test, ForgeInvariantTest { // Target only the comptroller handler for invariant testing (to avoid getting reverts). targetContract(address(comptrollerHandler)); - // Exclude the comptroller, linear and pro for being the `msg.sender`. + // Exclude the comptroller, linear and pro from being the `msg.sender`. excludeSender(address(comptroller)); excludeSender(address(linear)); excludeSender(address(pro)); - // Exclude the comptroller handler for being the `msg.sender`. + // Exclude the comptroller handler from being the `msg.sender`. excludeSender(address(comptrollerHandler)); // Label the comptroller handler. diff --git a/test/invariant/handlers/LockupHandler.t.sol b/test/invariant/handlers/LockupHandler.t.sol index 98e69b5b5..5b2b2ca74 100644 --- a/test/invariant/handlers/LockupHandler.t.sol +++ b/test/invariant/handlers/LockupHandler.t.sol @@ -18,7 +18,7 @@ abstract contract LockupHandler is BaseHandler { IERC20 public asset; ISablierV2Lockup public lockup; - LockupHandlerStorage public _storage; + LockupHandlerStorage public store; /*////////////////////////////////////////////////////////////////////////// PRIVATE TEST VARIABLES @@ -32,10 +32,10 @@ abstract contract LockupHandler is BaseHandler { CONSTRUCTOR //////////////////////////////////////////////////////////////////////////*/ - constructor(IERC20 asset_, ISablierV2Lockup lockup_, LockupHandlerStorage _storage_) { + constructor(IERC20 asset_, ISablierV2Lockup lockup_, LockupHandlerStorage store_) { asset = asset_; lockup = lockup_; - _storage = _storage_; + store = store_; } /*////////////////////////////////////////////////////////////////////////// @@ -50,24 +50,24 @@ abstract contract LockupHandler is BaseHandler { } modifier useFuzzedStreamRecipient(uint256 streamIndexSeed) { - uint256 lastStreamId = _storage.lastStreamId(); + uint256 lastStreamId = store.lastStreamId(); if (lastStreamId == 0) { return; } - currentStreamId = _storage.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); - currentRecipient = _storage.streamIdsToRecipients(currentStreamId); + currentStreamId = store.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); + currentRecipient = store.streamIdsToRecipients(currentStreamId); vm.startPrank(currentRecipient); _; vm.stopPrank(); } modifier useFuzzedStreamSender(uint256 streamIndexSeed) { - uint256 lastStreamId = _storage.lastStreamId(); + uint256 lastStreamId = store.lastStreamId(); if (lastStreamId == 0) { return; } - currentStreamId = _storage.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); - currentSender = _storage.streamIdsToSenders(currentStreamId); + currentStreamId = store.streamIds(bound(streamIndexSeed, 0, lastStreamId - 1)); + currentSender = store.streamIdsToSenders(currentStreamId); vm.startPrank(currentSender); _; vm.stopPrank(); @@ -93,7 +93,7 @@ abstract contract LockupHandler is BaseHandler { lockup.burn(currentStreamId); // Set the recipient associated with this stream to the zero address. - _storage.updateRecipient(currentStreamId, address(0)); + store.updateRecipient(currentStreamId, address(0)); } function cancel(uint256 streamIndexSeed) external instrument("cancel") useFuzzedStreamSender(streamIndexSeed) { @@ -106,7 +106,7 @@ abstract contract LockupHandler is BaseHandler { // Record the returned amount by adding it to the ghost variable `returnedAmountsSum`. This is needed to // check invariants against the contract's balance. uint128 returnedAmount = lockup.returnableAmountOf(currentStreamId); - _storage.addReturnedAmount(returnedAmount); + store.addReturnedAmount(returnedAmount); // Cancel the stream. lockup.cancel(currentStreamId); @@ -210,6 +210,6 @@ abstract contract LockupHandler is BaseHandler { lockup.transferFrom({ from: currentRecipient, to: newRecipient, tokenId: currentStreamId }); // Update the recipient associated with this stream id. - _storage.updateRecipient(currentStreamId, newRecipient); + store.updateRecipient(currentStreamId, newRecipient); } } diff --git a/test/invariant/handlers/LockupLinearCreateHandler.t.sol b/test/invariant/handlers/LockupLinearCreateHandler.t.sol index cf4027748..865d23ddc 100644 --- a/test/invariant/handlers/LockupLinearCreateHandler.t.sol +++ b/test/invariant/handlers/LockupLinearCreateHandler.t.sol @@ -24,16 +24,16 @@ contract LockupLinearCreateHandler is BaseHandler { //////////////////////////////////////////////////////////////////////////*/ IERC20 public asset; - LockupHandlerStorage public _storage; + LockupHandlerStorage public store; /*////////////////////////////////////////////////////////////////////////// CONSTRUCTOR //////////////////////////////////////////////////////////////////////////*/ - constructor(IERC20 asset_, ISablierV2LockupLinear linear_, LockupHandlerStorage _storage_) { + constructor(IERC20 asset_, ISablierV2LockupLinear linear_, LockupHandlerStorage store_) { asset = asset_; linear = linear_; - _storage = _storage_; + store = store_; } /*////////////////////////////////////////////////////////////////////////// @@ -68,7 +68,7 @@ contract LockupLinearCreateHandler is BaseHandler { params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); // We don't want to fuzz more than a certain number of streams. - if (_storage.lastStreamId() >= MAX_STREAM_COUNT) { + if (store.lastStreamId() >= MAX_STREAM_COUNT) { return; } @@ -95,7 +95,7 @@ contract LockupLinearCreateHandler is BaseHandler { }); // Store the stream id. - _storage.pushStreamId(streamId, params.sender, params.recipient); + store.pushStreamId(streamId, params.sender, params.recipient); } struct CreateWithRangeParams { @@ -117,7 +117,7 @@ contract LockupLinearCreateHandler is BaseHandler { params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); // We don't want to fuzz more than a certain number of streams. - if (_storage.lastStreamId() >= MAX_STREAM_COUNT) { + if (store.lastStreamId() >= MAX_STREAM_COUNT) { return; } @@ -144,6 +144,6 @@ contract LockupLinearCreateHandler is BaseHandler { }); // Store the stream id. - _storage.pushStreamId(streamId, params.sender, params.recipient); + store.pushStreamId(streamId, params.sender, params.recipient); } } diff --git a/test/invariant/handlers/LockupLinearHandler.t.sol b/test/invariant/handlers/LockupLinearHandler.t.sol index 5d1319131..a6cc29017 100644 --- a/test/invariant/handlers/LockupLinearHandler.t.sol +++ b/test/invariant/handlers/LockupLinearHandler.t.sol @@ -15,6 +15,6 @@ contract LockupLinearHandler is LockupHandler { constructor( IERC20 asset_, ISablierV2LockupLinear linear_, - LockupHandlerStorage _storage_ - ) LockupHandler(asset_, linear_, _storage_) {} + LockupHandlerStorage store_ + ) LockupHandler(asset_, linear_, store_) {} } diff --git a/test/invariant/handlers/LockupProCreateHandler.t.sol b/test/invariant/handlers/LockupProCreateHandler.t.sol index c43a6c9f8..bedbf02fb 100644 --- a/test/invariant/handlers/LockupProCreateHandler.t.sol +++ b/test/invariant/handlers/LockupProCreateHandler.t.sol @@ -27,7 +27,7 @@ contract LockupProCreateHandler is BaseHandler { //////////////////////////////////////////////////////////////////////////*/ IERC20 public asset; - LockupHandlerStorage public _storage; + LockupHandlerStorage public store; /*////////////////////////////////////////////////////////////////////////// CONSTRUCTOR @@ -37,12 +37,12 @@ contract LockupProCreateHandler is BaseHandler { IERC20 asset_, ISablierV2Comptroller comptroller_, ISablierV2LockupPro pro_, - LockupHandlerStorage _storage_ + LockupHandlerStorage store_ ) { asset = asset_; comptroller = comptroller_; pro = pro_; - _storage = _storage_; + store = store_; } /*////////////////////////////////////////////////////////////////////////// @@ -86,7 +86,7 @@ contract LockupProCreateHandler is BaseHandler { params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); // We don't want to fuzz more than a certain number of streams. - if (_storage.lastStreamId() > MAX_STREAM_COUNT) { + if (store.lastStreamId() > MAX_STREAM_COUNT) { return; } @@ -129,7 +129,7 @@ contract LockupProCreateHandler is BaseHandler { }); // Store the stream id. - _storage.pushStreamId(vars.streamId, params.sender, params.recipient); + store.pushStreamId(vars.streamId, params.sender, params.recipient); } struct CreateWithMilestonesParams { @@ -156,7 +156,7 @@ contract LockupProCreateHandler is BaseHandler { params.totalAmount = boundUint128(params.totalAmount, 1, 1_000_000_000e18); // We don't want to fuzz more than a certain number of streams. - if (_storage.lastStreamId() >= MAX_STREAM_COUNT) { + if (store.lastStreamId() >= MAX_STREAM_COUNT) { return; } @@ -195,6 +195,6 @@ contract LockupProCreateHandler is BaseHandler { }); // Store the stream id. - _storage.pushStreamId(vars.streamId, params.sender, params.recipient); + store.pushStreamId(vars.streamId, params.sender, params.recipient); } } diff --git a/test/invariant/handlers/LockupProHandler.t.sol b/test/invariant/handlers/LockupProHandler.t.sol index af87ae353..797ce006c 100644 --- a/test/invariant/handlers/LockupProHandler.t.sol +++ b/test/invariant/handlers/LockupProHandler.t.sol @@ -15,6 +15,6 @@ contract LockupProHandler is LockupHandler { constructor( IERC20 asset_, ISablierV2LockupPro pro_, - LockupHandlerStorage _storage_ - ) LockupHandler(asset_, pro_, _storage_) {} + LockupHandlerStorage store_ + ) LockupHandler(asset_, pro_, store_) {} } diff --git a/test/invariant/lockup/Lockup.t.sol b/test/invariant/lockup/Lockup.t.sol index 952155264..509bae273 100644 --- a/test/invariant/lockup/Lockup.t.sol +++ b/test/invariant/lockup/Lockup.t.sol @@ -30,7 +30,7 @@ abstract contract Lockup_Invariant_Test is Invariant_Test { // Deploy the lockupHandlerStorage. lockupHandlerStorage = new LockupHandlerStorage(); - // Exclude the lockup handler store for being the `msg.sender`. + // Exclude the lockup handler store from being the `msg.sender`. excludeSender(address(lockupHandlerStorage)); } diff --git a/test/invariant/lockup/linear/Linear.t.sol b/test/invariant/lockup/linear/Linear.t.sol index 58d1bbc36..ae2b09dca 100644 --- a/test/invariant/lockup/linear/Linear.t.sol +++ b/test/invariant/lockup/linear/Linear.t.sol @@ -33,12 +33,12 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { linearHandler = new LockupLinearHandler({ asset_: DEFAULT_ASSET, linear_: linear, - _storage_: lockupHandlerStorage + store_: lockupHandlerStorage }); linearCreateHandler = new LockupLinearCreateHandler({ asset_: DEFAULT_ASSET, linear_: linear, - _storage_: lockupHandlerStorage + store_: lockupHandlerStorage }); // Cast the linear contract as {SablierV2Lockup} and the linear handler as {LockupHandler}. @@ -58,7 +58,7 @@ contract Linear_Invariant_Test is Lockup_Invariant_Test { targetContract(address(linearHandler)); targetContract(address(linearCreateHandler)); - // Exclude the linear handlers for being the `msg.sender`. + // Exclude the linear handlers from being the `msg.sender`. excludeSender(address(flashLoanHandler)); excludeSender(address(linearHandler)); excludeSender(address(linearCreateHandler)); diff --git a/test/invariant/lockup/pro/Pro.t.sol b/test/invariant/lockup/pro/Pro.t.sol index c347e08b6..243e232ad 100644 --- a/test/invariant/lockup/pro/Pro.t.sol +++ b/test/invariant/lockup/pro/Pro.t.sol @@ -30,12 +30,12 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { Lockup_Invariant_Test.setUp(); // Deploy the pro contract handlers. - proHandler = new LockupProHandler({ asset_: DEFAULT_ASSET, pro_: pro, _storage_: lockupHandlerStorage }); + proHandler = new LockupProHandler({ asset_: DEFAULT_ASSET, pro_: pro, store_: lockupHandlerStorage }); proCreateHandler = new LockupProCreateHandler({ asset_: DEFAULT_ASSET, comptroller_: comptroller, pro_: pro, - _storage_: lockupHandlerStorage + store_: lockupHandlerStorage }); // Cast the pro contract as {SablierV2Lockup} and the pro handler as {LockupHandler}. @@ -55,7 +55,7 @@ contract Pro_Invariant_Test is Lockup_Invariant_Test { targetContract(address(proHandler)); targetContract(address(proCreateHandler)); - // Exclude the pro handlers for being the `msg.sender`. + // Exclude the pro handlers from being the `msg.sender`. excludeSender(address(flashLoanHandler)); excludeSender(address(proHandler)); excludeSender(address(proCreateHandler));