From 427a41b611fa615e89568423a935e3e1dbc26f7c Mon Sep 17 00:00:00 2001 From: Lucas Manuel Date: Wed, 26 Jun 2024 11:29:54 -0400 Subject: [PATCH] test: Add basic invariant testing (SC-459) (#7) * feat: first test working * feat: use larger numbers: * feat: test with initial burn amount passing * feat: update tests to work with updated burn logic, move conversion functions around and use previews * feat: remove todos * fix: update to remove console and update comment * feat: get swap tests working * feat: get all swap tests working * fix: update for three assets in logic * feat: all tests passing * fix: rm commented out test * feat: add preview swap tests * feat: move logic out of single use internal and use conversion rate everywhere * feat: move divRoundUp out of single use internal * feat: add full coverage for conversion tests * feat: add more preview cases * feat: refactor PSM to use three assets * fix: rm comment * feat: add interface, natspec, events, referral code, tests passing * fix: update to rm consolegp * feat: add events testing * feat: make precisions internal and add state var natspec * feat: finish natspec * feat: add readme * feat: add referral code note * fix: update constructor test * fix: update links * fix: reformatting * fix: update testing section * fix: improve overview * feat: add emojis * feat: remove all share burn logic, get all non inflation attack tests to pass * fix: cleanup diff * fix: update to use initial deposit instead of burn * feat: add readme section explaining attack * fix: minimize diff * fix: address bartek comments * feat: update all tests to work with new interfaces * feat: add deposit failure mode tests * feat: update to add assertions for return in deposit * feat: add withdraw failure tests * feat: update to address comments outside sharesToBurn * feat: update inflation attack test and readme * fix: update readme * feat: update test to constrain deposit/withdraw * feat: update to add both cases * feat: update per review * feat: update to use underscore bound, fix test * fix: typo * feat: add overrides, remove referrals, update referral type * fix: update expect emit * feat: update name and remove todos * feat: move files and set up structure * feat: update to rename files, contracts, and errors * fix: rm dup file, update toml * feat: get deposits working * chore: refactor into proper inheritance structure * feat: get all functions working with reverts * feat: update conversion * feat: get swaps working without reverts * feat: add fully working deposit/withdraw/swaps, invariant_B failing * ci: update for ci * fix: update name * chore: rm basly cased file * chore: re add * fix: re add invariant * ci: experiment with 2 million total calls * ci: add show progress flag * fix: move file back * ci: update verbosity * ci: add PR profile * fix: rm redundant files * feat: update from review changes * fix: update invariant * fix: add fuzz failure * chore: rm indexing comment * fix: rm redundant files from merge * feat: update to add seeding as part of invariants --- .github/workflows/{ci.yml => master.yml} | 7 +- .github/workflows/pr.yml | 87 ++++++++++++++++++++++ foundry.toml | 18 +++++ src/interfaces/IPSM3.sol | 2 - test/invariant/Invariants.t.sol | 80 ++++++++++++++++++++ test/invariant/handlers/HandlerBase.sol | 39 ++++++++++ test/invariant/handlers/LpHandler.sol | 63 ++++++++++++++++ test/invariant/handlers/SwapperHandler.sol | 86 +++++++++++++++++++++ test/{ => unit}/Constructor.t.sol | 2 +- test/{ => unit}/Conversions.t.sol | 2 +- test/{ => unit}/Deposit.t.sol | 2 +- test/{ => unit}/DoSAttack.t.sol | 0 test/{ => unit}/Events.t.sol | 0 test/{ => unit}/Getters.t.sol | 2 +- test/{ => unit}/InflationAttack.t.sol | 0 test/{ => unit}/Previews.t.sol | 0 test/{ => unit}/Rounding.t.sol | 0 test/{ => unit}/Swaps.t.sol | 2 +- test/{ => unit}/Withdraw.t.sol | 2 +- test/{ => unit}/harnesses/PSM3Harness.sol | 0 20 files changed, 381 insertions(+), 13 deletions(-) rename .github/workflows/{ci.yml => master.yml} (96%) create mode 100644 .github/workflows/pr.yml create mode 100644 test/invariant/Invariants.t.sol create mode 100644 test/invariant/handlers/HandlerBase.sol create mode 100644 test/invariant/handlers/LpHandler.sol create mode 100644 test/invariant/handlers/SwapperHandler.sol rename test/{ => unit}/Constructor.t.sol (98%) rename test/{ => unit}/Conversions.t.sol (99%) rename test/{ => unit}/Deposit.t.sol (99%) rename test/{ => unit}/DoSAttack.t.sol (100%) rename test/{ => unit}/Events.t.sol (100%) rename test/{ => unit}/Getters.t.sol (99%) rename test/{ => unit}/InflationAttack.t.sol (100%) rename test/{ => unit}/Previews.t.sol (100%) rename test/{ => unit}/Rounding.t.sol (100%) rename test/{ => unit}/Swaps.t.sol (99%) rename test/{ => unit}/Withdraw.t.sol (99%) rename test/{ => unit}/harnesses/PSM3Harness.sol (100%) diff --git a/.github/workflows/ci.yml b/.github/workflows/master.yml similarity index 96% rename from .github/workflows/ci.yml rename to .github/workflows/master.yml index 579beed..726353e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/master.yml @@ -1,11 +1,8 @@ name: CI on: - workflow_dispatch: - pull_request: push: - branches: - - master + branches: [master] env: FOUNDRY_PROFILE: ci @@ -40,7 +37,7 @@ jobs: ARBITRUM_NOVA_RPC_URL: ${{secrets.ARBITRUM_NOVA_RPC_URL}} GNOSIS_CHAIN_RPC_URL: ${{secrets.GNOSIS_CHAIN_RPC_URL}} BASE_RPC_URL: ${{secrets.BASE_RPC_URL}} - run: FOUNDRY_PROFILE=ci forge test + run: FOUNDRY_PROFILE=master forge test -vv --show-progress # coverage: # runs-on: ubuntu-latest diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml new file mode 100644 index 0000000..5614607 --- /dev/null +++ b/.github/workflows/pr.yml @@ -0,0 +1,87 @@ +name: CI + +on: [pull_request] + +env: + FOUNDRY_PROFILE: ci + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + + - name: Build contracts + run: | + forge --version + forge build --sizes + + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + + - name: Run tests + env: + MAINNET_RPC_URL: ${{secrets.MAINNET_RPC_URL}} + OPTIMISM_RPC_URL: ${{secrets.OPTIMISM_RPC_URL}} + ARBITRUM_ONE_RPC_URL: ${{secrets.ARBITRUM_ONE_RPC_URL}} + ARBITRUM_NOVA_RPC_URL: ${{secrets.ARBITRUM_NOVA_RPC_URL}} + GNOSIS_CHAIN_RPC_URL: ${{secrets.GNOSIS_CHAIN_RPC_URL}} + BASE_RPC_URL: ${{secrets.BASE_RPC_URL}} + run: FOUNDRY_PROFILE=pr forge test -vv --show-progress + + # coverage: + # runs-on: ubuntu-latest + # steps: + # - uses: actions/checkout@v3 + + # - name: Install Foundry + # uses: foundry-rs/foundry-toolchain@v1 + + # - name: Run coverage + # env: + # MAINNET_RPC_URL: ${{secrets.MAINNET_RPC_URL}} + # OPTIMISM_RPC_URL: ${{secrets.OPTIMISM_RPC_URL}} + # ARBITRUM_ONE_RPC_URL: ${{secrets.ARBITRUM_ONE_RPC_URL}} + # ARBITRUM_NOVA_RPC_URL: ${{secrets.ARBITRUM_NOVA_RPC_URL}} + # GNOSIS_CHAIN_RPC_URL: ${{secrets.GNOSIS_CHAIN_RPC_URL}} + # BASE_RPC_URL: ${{secrets.BASE_RPC_URL}} + # run: forge coverage --report summary --report lcov + + # # To ignore coverage for certain directories modify the paths in this step as needed. The + # # below default ignores coverage results for the test and script directories. Alternatively, + # # to include coverage in all directories, comment out this step. Note that because this + # # filtering applies to the lcov file, the summary table generated in the previous step will + # # still include all files and directories. + # # The `--rc lcov_branch_coverage=1` part keeps branch info in the filtered report, since lcov + # # defaults to removing branch info. + # - name: Filter directories + # run: | + # sudo apt update && sudo apt install -y lcov + # lcov --remove lcov.info 'test/*' 'script/*' --output-file lcov.info --rc lcov_branch_coverage=1 + + # # This step posts a detailed coverage report as a comment and deletes previous comments on + # # each push. The below step is used to fail coverage if the specified coverage threshold is + # # not met. The below step can post a comment (when it's `github-token` is specified) but it's + # # not as useful, and this action cannot fail CI based on a minimum coverage threshold, which + # # is why we use both in this way. + # - name: Post coverage report + # if: github.event_name == 'pull_request' # This action fails when ran outside of a pull request. + # uses: romeovs/lcov-reporter-action@v0.3.1 + # with: + # delete-old-comments: true + # lcov-file: ./lcov.info + # github-token: ${{ secrets.GITHUB_TOKEN }} # Adds a coverage summary comment to the PR. + + # - name: Verify minimum coverage + # uses: zgosalvez/github-actions-report-lcov@v2 + # with: + # coverage-files: ./lcov.info + # minimum-coverage: 90 # Set coverage threshold. diff --git a/foundry.toml b/foundry.toml index 4963041..d92768d 100644 --- a/foundry.toml +++ b/foundry.toml @@ -9,6 +9,24 @@ optimizer_runs = 200 [fuzz] runs = 1000 +[invariant] +runs = 20 +depth = 1000 + +[profile.pr.invariant] +runs = 200 +depth = 1000 + +[profile.pr.fuzz] +runs = 100_000 + +[profile.master.invariant] +runs = 200 +depth = 10_000 + +[profile.master.fuzz] +runs = 1_000_000 + # See more config options https://github.com/foundry-rs/foundry/tree/master/config remappings = [ diff --git a/src/interfaces/IPSM3.sol b/src/interfaces/IPSM3.sol index 6230014..06adef4 100644 --- a/src/interfaces/IPSM3.sol +++ b/src/interfaces/IPSM3.sol @@ -5,8 +5,6 @@ import { IERC20 } from "erc20-helpers/interfaces/IERC20.sol"; interface IPSM3 { - // TODO: Determine priority for indexing - /**********************************************************************************************/ /*** Events ***/ /**********************************************************************************************/ diff --git a/test/invariant/Invariants.t.sol b/test/invariant/Invariants.t.sol new file mode 100644 index 0000000..c2d7013 --- /dev/null +++ b/test/invariant/Invariants.t.sol @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; + +import { PSMTestBase } from "test/PSMTestBase.sol"; + +import { LpHandler } from "test/invariant/handlers/LpHandler.sol"; +import { SwapperHandler } from "test/invariant/handlers/SwapperHandler.sol"; + +contract PSMInvariantTests is PSMTestBase { + + LpHandler public lpHandler; + SwapperHandler public swapperHandler; + + address BURN_ADDRESS = makeAddr("burn-address"); + + // NOTE [CRITICAL]: All invariant tests are operating under the assumption that the initial seed + // deposit of 1e18 shares has been made. This is a key requirement and + // assumption for all invariant tests. + function setUp() public override { + super.setUp(); + + // Seed the pool with 1e18 shares (1e18 of value) + _deposit(address(dai), BURN_ADDRESS, 1e18); + + lpHandler = new LpHandler(psm, dai, usdc, sDai, 3); + swapperHandler = new SwapperHandler(psm, dai, usdc, sDai, 3); + + // TODO: Add rate updates + rateProvider.__setConversionRate(1.25e27); + + targetContract(address(lpHandler)); + targetContract(address(swapperHandler)); + } + + function invariant_A() public view { + assertEq( + psm.shares(address(lpHandler.lps(0))) + + psm.shares(address(lpHandler.lps(1))) + + psm.shares(address(lpHandler.lps(2))) + + 1e18, // Seed amount + psm.totalShares() + ); + } + + function invariant_B() public view { + assertApproxEqAbs( + psm.getPsmTotalValue(), + psm.convertToAssetValue(psm.totalShares()), + 2 + ); + } + + function invariant_C() public view { + assertApproxEqAbs( + psm.convertToAssetValue(psm.shares(address(lpHandler.lps(0)))) + + psm.convertToAssetValue(psm.shares(address(lpHandler.lps(1)))) + + psm.convertToAssetValue(psm.shares(address(lpHandler.lps(2)))) + + psm.convertToAssetValue(1e18), // Seed amount + psm.getPsmTotalValue(), + 4 + ); + } + + function invariant_logs() public view { + console.log("depositCount ", lpHandler.depositCount()); + console.log("withdrawCount ", lpHandler.withdrawCount()); + console.log("swapCount ", swapperHandler.swapCount()); + console.log("zeroBalanceCount", swapperHandler.zeroBalanceCount()); + console.log( + "sum ", + lpHandler.depositCount() + + lpHandler.withdrawCount() + + swapperHandler.swapCount() + + swapperHandler.zeroBalanceCount() + ); + } + +} diff --git a/test/invariant/handlers/HandlerBase.sol b/test/invariant/handlers/HandlerBase.sol new file mode 100644 index 0000000..fbc8dff --- /dev/null +++ b/test/invariant/handlers/HandlerBase.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +pragma solidity ^0.8.13; + +import { MockERC20 } from "erc20-helpers/MockERC20.sol"; + +import { CommonBase } from "forge-std/Base.sol"; +import { StdCheatsSafe } from "forge-std/StdCheats.sol"; +import { StdUtils } from "forge-std/StdUtils.sol"; + +import { PSM3 } from "src/PSM3.sol"; + +contract HandlerBase is CommonBase, StdCheatsSafe, StdUtils { + + PSM3 public psm; + + MockERC20[3] public assets; + + constructor( + PSM3 psm_, + MockERC20 asset0, + MockERC20 asset1, + MockERC20 asset2 + ) { + psm = psm_; + + assets[0] = asset0; + assets[1] = asset1; + assets[2] = asset2; + } + + function _getAsset(uint256 indexSeed) internal view returns (MockERC20) { + return assets[indexSeed % assets.length]; + } + + function _hash(uint256 number_, string memory salt) internal pure returns (uint256 hash_) { + hash_ = uint256(keccak256(abi.encode(number_, salt))); + } + +} diff --git a/test/invariant/handlers/LpHandler.sol b/test/invariant/handlers/LpHandler.sol new file mode 100644 index 0000000..f321e2a --- /dev/null +++ b/test/invariant/handlers/LpHandler.sol @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +pragma solidity ^0.8.13; + +import { MockERC20 } from "erc20-helpers/MockERC20.sol"; + +import { HandlerBase } from "test/invariant/handlers/HandlerBase.sol"; + +import { PSM3 } from "src/PSM3.sol"; + +contract LpHandler is HandlerBase { + + address[] public lps; + + uint256 public depositCount; + uint256 public withdrawCount; + + uint256 public constant TRILLION = 1e12; + + constructor( + PSM3 psm_, + MockERC20 asset0, + MockERC20 asset1, + MockERC20 asset2, + uint256 lpCount + ) HandlerBase(psm_, asset0, asset1, asset2) { + for (uint256 i = 0; i < lpCount; i++) { + lps.push(makeAddr(string(abi.encodePacked("lp-", i)))); + } + } + + function _getLP(uint256 indexSeed) internal view returns (address) { + return lps[indexSeed % lps.length]; + } + + function deposit(uint256 assetSeed, uint256 lpSeed, uint256 amount) public { + MockERC20 asset = _getAsset(assetSeed); + address lp = _getLP(lpSeed); + + amount = _bound(amount, 1, TRILLION * 10 ** asset.decimals()); + + vm.startPrank(lp); + asset.mint(lp, amount); + asset.approve(address(psm), amount); + psm.deposit(address(asset), lp, amount); + vm.stopPrank(); + + depositCount++; + } + + function withdraw(uint256 assetSeed, uint256 lpSeed, uint256 amount) public { + MockERC20 asset = _getAsset(assetSeed); + address lp = _getLP(lpSeed); + + amount = _bound(amount, 1, TRILLION * 10 ** asset.decimals()); + + vm.prank(lp); + psm.withdraw(address(asset), lp, amount); + vm.stopPrank(); + + withdrawCount++; + } + +} diff --git a/test/invariant/handlers/SwapperHandler.sol b/test/invariant/handlers/SwapperHandler.sol new file mode 100644 index 0000000..c0e2bb1 --- /dev/null +++ b/test/invariant/handlers/SwapperHandler.sol @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +pragma solidity ^0.8.13; + +import { MockERC20 } from "erc20-helpers/MockERC20.sol"; + +import { HandlerBase } from "test/invariant/handlers/HandlerBase.sol"; + +import { PSM3 } from "src/PSM3.sol"; + +contract SwapperHandler is HandlerBase { + + address[] public swappers; + + uint256 public swapCount; + uint256 public zeroBalanceCount; + + constructor( + PSM3 psm_, + MockERC20 asset0, + MockERC20 asset1, + MockERC20 asset2, + uint256 lpCount + ) HandlerBase(psm_, asset0, asset1, asset2) { + for (uint256 i = 0; i < lpCount; i++) { + swappers.push(makeAddr(string(abi.encodePacked("swapper-", i)))); + } + } + + function _getSwapper(uint256 indexSeed) internal view returns (address) { + return swappers[indexSeed % swappers.length]; + } + + function swap( + uint256 assetInSeed, + uint256 assetOutSeed, + uint256 swapperSeed, + uint256 amountIn, + uint256 minAmountOut + ) + public + { + // Prevent overflow in if statement below + assetOutSeed = _bound(assetOutSeed, 0, type(uint256).max - 2); + + MockERC20 assetIn = _getAsset(assetInSeed); + MockERC20 assetOut = _getAsset(assetOutSeed); + address swapper = _getSwapper(swapperSeed); + + // Handle case where randomly selected assets match + if (assetIn == assetOut) { + assetOut = _getAsset(assetOutSeed + 2); + } + + // By calculating the amount of assetIn we can get from the max asset out, we can + // determine the max amount of assetIn we can swap since its the same both ways. + uint256 maxAmountIn = psm.previewSwap( + address(assetOut), + address(assetIn), + assetOut.balanceOf(address(psm)) + ); + + // If there's zero balance a swap can't be performed + if (maxAmountIn == 0) { + zeroBalanceCount++; + return; + } + + amountIn = _bound(amountIn, 1, maxAmountIn); + + // Fuzz between zero and the expected amount out from the swap + minAmountOut = _bound( + minAmountOut, + 0, + psm.previewSwap(address(assetIn), address(assetOut), amountIn) + ); + + vm.startPrank(swapper); + assetIn.mint(swapper, amountIn); + assetIn.approve(address(psm), amountIn); + psm.swap(address(assetIn), address(assetOut), amountIn, minAmountOut, swapper, 0); + vm.stopPrank(); + + swapCount++; + } + +} diff --git a/test/Constructor.t.sol b/test/unit/Constructor.t.sol similarity index 98% rename from test/Constructor.t.sol rename to test/unit/Constructor.t.sol index 1b3e22a..67ece94 100644 --- a/test/Constructor.t.sol +++ b/test/unit/Constructor.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.13; import "forge-std/Test.sol"; -import { PSM3 } from "../src/PSM3.sol"; +import { PSM3 } from "src/PSM3.sol"; import { PSMTestBase } from "test/PSMTestBase.sol"; diff --git a/test/Conversions.t.sol b/test/unit/Conversions.t.sol similarity index 99% rename from test/Conversions.t.sol rename to test/unit/Conversions.t.sol index 90f2e89..1b1c484 100644 --- a/test/Conversions.t.sol +++ b/test/unit/Conversions.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.13; import "forge-std/Test.sol"; -import { PSM3 } from "../src/PSM3.sol"; +import { PSM3 } from "src/PSM3.sol"; import { PSMTestBase } from "test/PSMTestBase.sol"; diff --git a/test/Deposit.t.sol b/test/unit/Deposit.t.sol similarity index 99% rename from test/Deposit.t.sol rename to test/unit/Deposit.t.sol index 0876ced..c85e54a 100644 --- a/test/Deposit.t.sol +++ b/test/unit/Deposit.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.13; import "forge-std/Test.sol"; -import { PSM3 } from "../src/PSM3.sol"; +import { PSM3 } from "src/PSM3.sol"; import { PSMTestBase } from "test/PSMTestBase.sol"; diff --git a/test/DoSAttack.t.sol b/test/unit/DoSAttack.t.sol similarity index 100% rename from test/DoSAttack.t.sol rename to test/unit/DoSAttack.t.sol diff --git a/test/Events.t.sol b/test/unit/Events.t.sol similarity index 100% rename from test/Events.t.sol rename to test/unit/Events.t.sol diff --git a/test/Getters.t.sol b/test/unit/Getters.t.sol similarity index 99% rename from test/Getters.t.sol rename to test/unit/Getters.t.sol index 2146bea..eb2c2e3 100644 --- a/test/Getters.t.sol +++ b/test/unit/Getters.t.sol @@ -5,7 +5,7 @@ import "forge-std/Test.sol"; import { PSMTestBase } from "test/PSMTestBase.sol"; -import { PSM3Harness } from "test/harnesses/PSM3Harness.sol"; +import { PSM3Harness } from "test/unit/harnesses/PSM3Harness.sol"; contract PSMHarnessTests is PSMTestBase { diff --git a/test/InflationAttack.t.sol b/test/unit/InflationAttack.t.sol similarity index 100% rename from test/InflationAttack.t.sol rename to test/unit/InflationAttack.t.sol diff --git a/test/Previews.t.sol b/test/unit/Previews.t.sol similarity index 100% rename from test/Previews.t.sol rename to test/unit/Previews.t.sol diff --git a/test/Rounding.t.sol b/test/unit/Rounding.t.sol similarity index 100% rename from test/Rounding.t.sol rename to test/unit/Rounding.t.sol diff --git a/test/Swaps.t.sol b/test/unit/Swaps.t.sol similarity index 99% rename from test/Swaps.t.sol rename to test/unit/Swaps.t.sol index 5db9680..43e4bbd 100644 --- a/test/Swaps.t.sol +++ b/test/unit/Swaps.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.13; import "forge-std/Test.sol"; -import { PSM3 } from "../src/PSM3.sol"; +import { PSM3 } from "src/PSM3.sol"; import { MockERC20, PSMTestBase } from "test/PSMTestBase.sol"; diff --git a/test/Withdraw.t.sol b/test/unit/Withdraw.t.sol similarity index 99% rename from test/Withdraw.t.sol rename to test/unit/Withdraw.t.sol index 041da77..e864ce6 100644 --- a/test/Withdraw.t.sol +++ b/test/unit/Withdraw.t.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.13; import "forge-std/Test.sol"; -import { PSM3 } from "../src/PSM3.sol"; +import { PSM3 } from "src/PSM3.sol"; import { MockERC20 } from "erc20-helpers/MockERC20.sol"; diff --git a/test/harnesses/PSM3Harness.sol b/test/unit/harnesses/PSM3Harness.sol similarity index 100% rename from test/harnesses/PSM3Harness.sol rename to test/unit/harnesses/PSM3Harness.sol