From c202759171e775c4c1de2874dff0e035fed8d2ac Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Thu, 1 Feb 2024 02:28:49 +0000 Subject: [PATCH 01/15] chore(Echidna): switch to assertion mode --- tests/invariants/MarketEchidna.t.sol | 19 ++++++++++--------- tests/invariants/MarketEchidna.yaml | 4 ++-- tests/invariants/MarketEchidnaAdvanced.yaml | 4 ++-- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/tests/invariants/MarketEchidna.t.sol b/tests/invariants/MarketEchidna.t.sol index ad97026a..158fa075 100644 --- a/tests/invariants/MarketEchidna.t.sol +++ b/tests/invariants/MarketEchidna.t.sol @@ -22,7 +22,7 @@ interface IHevm { // // Reference: https://github.com/overlay-market/v1-core/blob/main/tests/markets/conftest.py contract MarketEchidna { - IHevm hevm = IHevm(address(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D)); + IHevm hevm = IHevm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D); // contracts required for test OverlayV1Factory factory; @@ -30,8 +30,8 @@ contract MarketEchidna { OverlayV1Token ovl; // make these constant to match Echidna config - address public constant ALICE = address(0x1000000000000000000000000000000000000000); - address public constant BOB = address(0x2000000000000000000000000000000000000000); + address constant ALICE = address(0x1000000000000000000000000000000000000000); + address constant BOB = address(0x2000000000000000000000000000000000000000); uint256 constant MIN_COLLATERAL = 1e14; uint256 constant CAP_NOTIONAL = 8e23; @@ -88,8 +88,7 @@ contract MarketEchidna { // event to raise if the invariant is broken event OiAfterFunding(uint256 oiProductBefore, uint256 oiProductAfter); - function invariant_oi_product_after_funding() public returns (bool) { - uint256 lastUpdate = market.timestampUpdateLast(); + function check_oi_product_after_funding(uint256 timeElapsed) public { uint256 oiLong = market.oiLong(); uint256 oiShort = market.oiShort(); uint256 oiOverweightBefore = oiLong > oiShort ? oiLong : oiShort; @@ -100,9 +99,7 @@ contract MarketEchidna { (uint256 oiOverweightAfter, uint256 oiUnderweightAfter) = market.oiAfterFunding({ oiOverweight: oiOverweightBefore, oiUnderweight: oiUnderweightBefore, - // FIXME: block.timestamp is always 0 in the test - // timeElapsed: block.timestamp - lastUpdate - timeElapsed: 1 + timeElapsed: timeElapsed }); uint256 oiProductAfter = oiOverweightAfter * oiUnderweightAfter; @@ -111,6 +108,10 @@ contract MarketEchidna { emit OiAfterFunding(oiProductBefore, oiProductAfter); // 0.5% tolerance - return (TestUtils.isApproxEqRel(oiProductBefore, oiProductAfter, 0.5e16)); + assert(TestUtils.isApproxEqRel(oiProductBefore, oiProductAfter, 0.5e16)); } + + // Invariant 2) Market's oi and oi shares should increase after a build + + // TODO: implement invariant_oi_increase_after_build } diff --git a/tests/invariants/MarketEchidna.yaml b/tests/invariants/MarketEchidna.yaml index 96702d87..b69c640c 100644 --- a/tests/invariants/MarketEchidna.yaml +++ b/tests/invariants/MarketEchidna.yaml @@ -11,5 +11,5 @@ sender: ["0x1000000000000000000000000000000000000000", "0x2000000000000000000000 # fuzzer executes corpusDir: "./tests/invariants/coverage-echidna" -# use same prefix as Foundry invariant tests -prefix: "invariant_" +# use assertion mode to include dynamic parameters in the invariant checks +testMode: assertion diff --git a/tests/invariants/MarketEchidnaAdvanced.yaml b/tests/invariants/MarketEchidnaAdvanced.yaml index 43a2323e..22fa62a2 100644 --- a/tests/invariants/MarketEchidnaAdvanced.yaml +++ b/tests/invariants/MarketEchidnaAdvanced.yaml @@ -13,8 +13,8 @@ sender: ["0x1000000000000000000000000000000000000000", "0x2000000000000000000000 # fuzzer executes corpusDir: "./tests/invariants/coverage-echidna" -# use same prefix as Foundry invariant tests -prefix: "invariant_" +# use assertion mode to include dynamic parameters in the invariant checks +testMode: assertion # increase number of works to speed up test workers: 10 From 125ce3ff538867fac166e5d4ae53e9abaceb4855 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Thu, 1 Feb 2024 16:00:10 +0000 Subject: [PATCH 02/15] feat(Echidna): add build, unwind and liquidate invariants --- tests/invariants/MarketEchidna.t.sol | 10 +-- tests/invariants/MarketEchidnaAdvanced.t.sol | 78 ++++++++++++++++++-- 2 files changed, 77 insertions(+), 11 deletions(-) diff --git a/tests/invariants/MarketEchidna.t.sol b/tests/invariants/MarketEchidna.t.sol index 158fa075..f6479675 100644 --- a/tests/invariants/MarketEchidna.t.sol +++ b/tests/invariants/MarketEchidna.t.sol @@ -5,6 +5,7 @@ import {OverlayV1Factory} from "../../contracts/OverlayV1Factory.sol"; import {OverlayV1Market} from "../../contracts/OverlayV1Market.sol"; import {OverlayV1Token} from "../../contracts/OverlayV1Token.sol"; import {OverlayV1FeedFactoryMock} from "../../contracts/mocks/OverlayV1FeedFactoryMock.sol"; +import {OverlayV1FeedMock} from "../../contracts/mocks/OverlayV1FeedMock.sol"; import {GOVERNOR_ROLE, MINTER_ROLE} from "../../contracts/interfaces/IOverlayV1Token.sol"; import {TestUtils} from "./TestUtils.sol"; @@ -28,6 +29,7 @@ contract MarketEchidna { OverlayV1Factory factory; OverlayV1Market market; OverlayV1Token ovl; + OverlayV1FeedMock feed; // make these constant to match Echidna config address constant ALICE = address(0x1000000000000000000000000000000000000000); @@ -58,7 +60,7 @@ contract MarketEchidna { factory.addFeedFactory(address(feedFactory)); // market config and deployment - address feed = feedFactory.deployFeed({price: 1e29, reserve: 2_000_000e18}); + feed = OverlayV1FeedMock(feedFactory.deployFeed({price: 1e29, reserve: 2_000_000e18})); uint256[15] memory params = [ uint256(122000000000), // k 500000000000000000, // lmbda @@ -76,7 +78,7 @@ contract MarketEchidna { 25000000000000, // priceDriftUpperLimit 250 // averageBlockTime // FIXME: this will be different in Arbitrum ]; - market = OverlayV1Market(factory.deployMarket(address(feedFactory), feed, params)); + market = OverlayV1Market(factory.deployMarket(address(feedFactory), address(feed), params)); hevm.prank(ALICE); ovl.approve(address(market), ovlSupply / 2); hevm.prank(BOB); @@ -110,8 +112,4 @@ contract MarketEchidna { // 0.5% tolerance assert(TestUtils.isApproxEqRel(oiProductBefore, oiProductAfter, 0.5e16)); } - - // Invariant 2) Market's oi and oi shares should increase after a build - - // TODO: implement invariant_oi_increase_after_build } diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index 123f481b..c3ce0976 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -12,16 +12,15 @@ import {TestUtils} from "./TestUtils.sol"; // echidna tests/invariants/MarketEchidnaAdvanced.t.sol --contract MarketEchidnaAdvanced --config tests/invariants/MarketEchidnaAdvanced.yaml contract MarketEchidnaAdvanced is MarketEchidna { // wrapper around market.build() to "guide" the fuzz test - event BuildWrapper(bool isLong, uint256 collateral); - - function buildWrapper(bool isLong, uint256 collateral) public { + // note: wrappers have to be public in order to be called by Echidna + function buildWrapper(bool isLong, uint256 collateral) public returns (uint256) { // bound collateral to avoid reverts collateral = TestUtils.clampBetween(collateral, MIN_COLLATERAL, CAP_NOTIONAL); // use the senders specified in the yaml config hevm.prank(msg.sender); - market.build({ + return market.build({ collateral: collateral, leverage: 1e18, isLong: isLong, @@ -29,5 +28,74 @@ contract MarketEchidnaAdvanced is MarketEchidna { }); } - // invariants inherited from base contract + // Helper functions + + function _getOiAndShares(bool isLong) + internal + view + returns (uint256 oi, uint256 oiShares) + { + oi = isLong ? market.oiLong() : market.oiShort(); + oiShares = isLong ? market.oiLongShares() : market.oiShortShares(); + } + + // Invariants inherited from base contract + + // Invariant 2) Market's oi and oi shares should increase after a build + + function check_oi_after_build(bool isLong, uint256 collateral) public { + // trigger funding payment so that it doesn't affect the test + market.update(); + + (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); + + // build a position + buildWrapper(isLong, collateral); + + (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); + + assert(oiAfter > oiBefore); + assert(oiSharesAfter > oiSharesBefore); + } + + // Invariant 3) Market's oi and oi shares should decrease after an unwind + + function check_oi_after_unwind(bool isLong, uint256 collateral) public { + // build a position + uint256 posId = buildWrapper(isLong, collateral); + + (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); + + // unwind the whole position + market.unwind(posId, 1e18, isLong ? 0 : type(uint256).max); + + (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); + + assert(oiAfter < oiBefore); + assert(oiSharesAfter < oiSharesBefore); + } + + // Invariant 4) Market's oi and oi shares should decrease after a liquidation + + function check_oi_after_liquidate(bool isLong, uint256 collateral) public { + // build a position + uint256 posId = buildWrapper(isLong, collateral); + + (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); + + // adjust the price so that position becomes liquidatable + uint256 originalPrice = feed.price(); + feed.setPrice(isLong ? originalPrice / 2 : originalPrice * 2); + + // liquidate the position + market.liquidate(msg.sender, posId); + + // reset the price to keep the test environment consistent + feed.setPrice(originalPrice); + + (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); + + assert(oiAfter < oiBefore); + assert(oiSharesAfter < oiSharesBefore); + } } From 3a52cb02eb1be326c7f2ad01486cfce94cd522a9 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Thu, 1 Feb 2024 16:12:33 +0000 Subject: [PATCH 03/15] chore(Echidna): fund account before building a position --- tests/invariants/MarketEchidna.t.sol | 15 ++++++--------- tests/invariants/MarketEchidnaAdvanced.t.sol | 1 + 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/invariants/MarketEchidna.t.sol b/tests/invariants/MarketEchidna.t.sol index f6479675..c627aa97 100644 --- a/tests/invariants/MarketEchidna.t.sol +++ b/tests/invariants/MarketEchidna.t.sol @@ -44,13 +44,10 @@ contract MarketEchidna { factory = new OverlayV1Factory(address(ovl), address(0x111)); // market will be later deployed by factory - // ovl config - uint256 ovlSupply = 8_000_000e18; + // ovl config; fund test accounts ovl.grantRole(MINTER_ROLE, address(this)); - ovl.mint(address(this), ovlSupply); - ovl.renounceRole(MINTER_ROLE, address(this)); - ovl.transfer(ALICE, ovlSupply / 2); - ovl.transfer(BOB, ovlSupply / 2); + ovl.mint(ALICE, 4_000_000e18); + ovl.mint(BOB, 4_000_000e18); // factory config ovl.grantRole(GOVERNOR_ROLE, address(this)); @@ -76,13 +73,13 @@ contract MarketEchidna { 750000000000000, // tradingFeeRate MIN_COLLATERAL, // minCollateral 25000000000000, // priceDriftUpperLimit - 250 // averageBlockTime // FIXME: this will be different in Arbitrum + 250 // averageBlockTime ]; market = OverlayV1Market(factory.deployMarket(address(feedFactory), address(feed), params)); hevm.prank(ALICE); - ovl.approve(address(market), ovlSupply / 2); + ovl.approve(address(market), type(uint256).max); hevm.prank(BOB); - ovl.approve(address(market), ovlSupply / 2); + ovl.approve(address(market), type(uint256).max); } // Invariant 1) `oiOverweight * oiUnderweight` remains constant after funding payments diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index c3ce0976..e3a9a105 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -18,6 +18,7 @@ contract MarketEchidnaAdvanced is MarketEchidna { collateral = TestUtils.clampBetween(collateral, MIN_COLLATERAL, CAP_NOTIONAL); // use the senders specified in the yaml config + ovl.mint(msg.sender, collateral); hevm.prank(msg.sender); return market.build({ From 54eba31facdede867898cce6ff4a983c8d922d67 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Thu, 1 Feb 2024 16:23:25 +0000 Subject: [PATCH 04/15] chore(Echidna): revert state changes during invariant checks --- tests/invariants/MarketEchidna.t.sol | 2 ++ tests/invariants/MarketEchidnaAdvanced.t.sol | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/invariants/MarketEchidna.t.sol b/tests/invariants/MarketEchidna.t.sol index c627aa97..ebb62c66 100644 --- a/tests/invariants/MarketEchidna.t.sol +++ b/tests/invariants/MarketEchidna.t.sol @@ -108,5 +108,7 @@ contract MarketEchidna { // 0.5% tolerance assert(TestUtils.isApproxEqRel(oiProductBefore, oiProductAfter, 0.5e16)); + + revert(); // revert state changes during the invariant check } } diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index e3a9a105..36a6b1c8 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -57,6 +57,8 @@ contract MarketEchidnaAdvanced is MarketEchidna { assert(oiAfter > oiBefore); assert(oiSharesAfter > oiSharesBefore); + + revert(); // revert state changes during the invariant check } // Invariant 3) Market's oi and oi shares should decrease after an unwind @@ -74,6 +76,8 @@ contract MarketEchidnaAdvanced is MarketEchidna { assert(oiAfter < oiBefore); assert(oiSharesAfter < oiSharesBefore); + + revert(); // revert state changes during the invariant check } // Invariant 4) Market's oi and oi shares should decrease after a liquidation @@ -91,12 +95,11 @@ contract MarketEchidnaAdvanced is MarketEchidna { // liquidate the position market.liquidate(msg.sender, posId); - // reset the price to keep the test environment consistent - feed.setPrice(originalPrice); - (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); assert(oiAfter < oiBefore); assert(oiSharesAfter < oiSharesBefore); + + revert(); // revert state changes during the invariant check } } From 1ad5e4a028f826999556b21a1def8ff14e68071c Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Thu, 1 Feb 2024 16:39:27 +0000 Subject: [PATCH 05/15] feat(Echidna): add feed price changes to the tests --- tests/invariants/MarketEchidna.t.sol | 4 ++-- tests/invariants/MarketEchidnaAdvanced.t.sol | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/invariants/MarketEchidna.t.sol b/tests/invariants/MarketEchidna.t.sol index ebb62c66..7a344142 100644 --- a/tests/invariants/MarketEchidna.t.sol +++ b/tests/invariants/MarketEchidna.t.sol @@ -85,7 +85,7 @@ contract MarketEchidna { // Invariant 1) `oiOverweight * oiUnderweight` remains constant after funding payments // event to raise if the invariant is broken - event OiAfterFunding(uint256 oiProductBefore, uint256 oiProductAfter); + event OiAfterFunding(uint256 oiProductBefore, uint256 oiProductAfter, uint256 price); function check_oi_product_after_funding(uint256 timeElapsed) public { uint256 oiLong = market.oiLong(); @@ -104,7 +104,7 @@ contract MarketEchidna { uint256 oiProductAfter = oiOverweightAfter * oiUnderweightAfter; // only visible when invariant fails - emit OiAfterFunding(oiProductBefore, oiProductAfter); + emit OiAfterFunding(oiProductBefore, oiProductAfter, feed.price()); // 0.5% tolerance assert(TestUtils.isApproxEqRel(oiProductBefore, oiProductAfter, 0.5e16)); diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index 36a6b1c8..41d8b3cb 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -11,8 +11,10 @@ import {TestUtils} from "./TestUtils.sol"; // run from base project directory with: // echidna tests/invariants/MarketEchidnaAdvanced.t.sol --contract MarketEchidnaAdvanced --config tests/invariants/MarketEchidnaAdvanced.yaml contract MarketEchidnaAdvanced is MarketEchidna { - // wrapper around market.build() to "guide" the fuzz test - // note: wrappers have to be public in order to be called by Echidna + // Handler functions to "guide" the stateful fuzz test + // Note: handlers have to be public in order to be called by Echidna + + // wrapper around market.build() function buildWrapper(bool isLong, uint256 collateral) public returns (uint256) { // bound collateral to avoid reverts collateral = TestUtils.clampBetween(collateral, MIN_COLLATERAL, CAP_NOTIONAL); @@ -29,6 +31,12 @@ contract MarketEchidnaAdvanced is MarketEchidna { }); } + function setPrice(uint256 price) public { + // bound market price to a reasonable range + price = TestUtils.clampBetween(price, 1, 1e29); + feed.setPrice(price); + } + // Helper functions function _getOiAndShares(bool isLong) From 7c7b1d59a7cbad6e1f21c3f784237157bde36695 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Thu, 1 Feb 2024 18:51:02 +0000 Subject: [PATCH 06/15] ci: run echidna tests --- .github/workflows/test-echidna.yaml | 36 +++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 .github/workflows/test-echidna.yaml diff --git a/.github/workflows/test-echidna.yaml b/.github/workflows/test-echidna.yaml new file mode 100644 index 00000000..2c1ca2f2 --- /dev/null +++ b/.github/workflows/test-echidna.yaml @@ -0,0 +1,36 @@ +name: Echidna Test + +on: + push: + branches: [ main, staging ] + pull_request: + branches: [ main, staging ] + +env: + FOUNDRY_PROFILE: ci + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + submodules: recursive + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + with: + version: nightly + + - name: Compile contracts + run: | + forge build --build-info + + - name: Run Echidna + uses: crytic/echidna-action@v2 + with: + files: . + contract: MarketEchidnaAdvanced + config: tests/invariants/MarketEchidnaAdvanced.yaml + crytic-args: --ignore-compile \ No newline at end of file From 7863f07e51a3387beab02a339d27b009a9d2a51f Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Thu, 1 Feb 2024 18:55:46 +0000 Subject: [PATCH 07/15] ci: fix echidna tests workflow --- .github/workflows/test-echidna.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-echidna.yaml b/.github/workflows/test-echidna.yaml index 2c1ca2f2..f88564c4 100644 --- a/.github/workflows/test-echidna.yaml +++ b/.github/workflows/test-echidna.yaml @@ -33,4 +33,4 @@ jobs: files: . contract: MarketEchidnaAdvanced config: tests/invariants/MarketEchidnaAdvanced.yaml - crytic-args: --ignore-compile \ No newline at end of file + crytic-args: --ignore-compile --foundry-out-directory forge-out \ No newline at end of file From 53a556ef13d7d073b91b9e988fa67dbf6d2eb6de Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Sat, 3 Feb 2024 06:18:21 +0000 Subject: [PATCH 08/15] feat(Echidna): add unwinds and liquidations to test environment --- tests/invariants/MarketEchidnaAdvanced.t.sol | 51 ++++++++++++++++++-- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index 41d8b3cb..dc75440a 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -11,11 +11,14 @@ import {TestUtils} from "./TestUtils.sol"; // run from base project directory with: // echidna tests/invariants/MarketEchidnaAdvanced.t.sol --contract MarketEchidnaAdvanced --config tests/invariants/MarketEchidnaAdvanced.yaml contract MarketEchidnaAdvanced is MarketEchidna { + // ghost variables used to verify invariants + mapping(address => uint256[]) positions; // owner => positionIds + // Handler functions to "guide" the stateful fuzz test // Note: handlers have to be public in order to be called by Echidna // wrapper around market.build() - function buildWrapper(bool isLong, uint256 collateral) public returns (uint256) { + function buildWrapper(bool isLong, uint256 collateral) public returns (uint256 posId) { // bound collateral to avoid reverts collateral = TestUtils.clampBetween(collateral, MIN_COLLATERAL, CAP_NOTIONAL); @@ -23,12 +26,15 @@ contract MarketEchidnaAdvanced is MarketEchidna { ovl.mint(msg.sender, collateral); hevm.prank(msg.sender); - return market.build({ + posId = market.build({ collateral: collateral, leverage: 1e18, isLong: isLong, priceLimit: isLong ? type(uint256).max : 0 }); + + // save positions built by the sender to use in other handlers + positions[msg.sender].push(posId); } function setPrice(uint256 price) public { @@ -37,6 +43,41 @@ contract MarketEchidnaAdvanced is MarketEchidna { feed.setPrice(price); } + function unwind(uint256 fraction) public { + fraction = TestUtils.clampBetween(fraction, 1e14, 1e18); + + uint256[] storage posIds = positions[msg.sender]; + require(posIds.length > 0, "No positions to unwind"); + uint256 posId = posIds[posIds.length - 1]; + + (,,,,bool isLong,,,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + + hevm.prank(msg.sender); + market.unwind(posId, fraction, isLong ? 0 : type(uint256).max); + + (,,,,,,,uint16 fractionRemaining) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + + // remove the position if fully unwound + if (fractionRemaining == 0) posIds.pop(); + } + + function liquidate() public { + uint256[] storage posIds = positions[msg.sender]; + require(posIds.length > 0, "No positions to unwind"); + uint256 posId = posIds[posIds.length - 1]; + + (,,,,bool isLong,,,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + + // adjust the price so that position becomes liquidatable + setPrice(isLong ? feed.price() / 2 : feed.price() * 2); + + // liquidate the position + market.liquidate(msg.sender, posId); + + // remove the liquidated position + posIds.pop(); + } + // Helper functions function _getOiAndShares(bool isLong) @@ -78,6 +119,7 @@ contract MarketEchidnaAdvanced is MarketEchidna { (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); // unwind the whole position + hevm.prank(msg.sender); market.unwind(posId, 1e18, isLong ? 0 : type(uint256).max); (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); @@ -97,10 +139,9 @@ contract MarketEchidnaAdvanced is MarketEchidna { (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); // adjust the price so that position becomes liquidatable - uint256 originalPrice = feed.price(); - feed.setPrice(isLong ? originalPrice / 2 : originalPrice * 2); + feed.setPrice(isLong ? feed.price() / 2 : feed.price() * 2); - // liquidate the position + // liquidate the position (don't need to prank the sender here) market.liquidate(msg.sender, posId); (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); From a96fa4c05c9839732cd0952d13eda31f16eb5445 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Mon, 5 Feb 2024 19:50:26 +0000 Subject: [PATCH 09/15] chore(Echidna): make build, unwind and liquidate tests more precise --- tests/invariants/MarketEchidna.t.sol | 6 ++-- tests/invariants/MarketEchidnaAdvanced.t.sol | 29 +++++++++++++++----- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/tests/invariants/MarketEchidna.t.sol b/tests/invariants/MarketEchidna.t.sol index 7a344142..d2fb64e1 100644 --- a/tests/invariants/MarketEchidna.t.sol +++ b/tests/invariants/MarketEchidna.t.sol @@ -57,7 +57,7 @@ contract MarketEchidna { factory.addFeedFactory(address(feedFactory)); // market config and deployment - feed = OverlayV1FeedMock(feedFactory.deployFeed({price: 1e29, reserve: 2_000_000e18})); + feed = OverlayV1FeedMock(feedFactory.deployFeed({price: 1e25, reserve: 2_000_000e18})); uint256[15] memory params = [ uint256(122000000000), // k 500000000000000000, // lmbda @@ -106,8 +106,8 @@ contract MarketEchidna { // only visible when invariant fails emit OiAfterFunding(oiProductBefore, oiProductAfter, feed.price()); - // 0.5% tolerance - assert(TestUtils.isApproxEqRel(oiProductBefore, oiProductAfter, 0.5e16)); + // 10% tolerance + assert(TestUtils.isApproxEqRel(oiProductBefore, oiProductAfter, 10e16)); revert(); // revert state changes during the invariant check } diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index dc75440a..c9fc947f 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -39,7 +39,7 @@ contract MarketEchidnaAdvanced is MarketEchidna { function setPrice(uint256 price) public { // bound market price to a reasonable range - price = TestUtils.clampBetween(price, 1, 1e29); + price = TestUtils.clampBetween(price, 1e1, 1e25); feed.setPrice(price); } @@ -94,6 +94,9 @@ contract MarketEchidnaAdvanced is MarketEchidna { // Invariant 2) Market's oi and oi shares should increase after a build function check_oi_after_build(bool isLong, uint256 collateral) public { + // bound collateral here to use in oi calculations + collateral = TestUtils.clampBetween(collateral, MIN_COLLATERAL, CAP_NOTIONAL); + // trigger funding payment so that it doesn't affect the test market.update(); @@ -102,10 +105,16 @@ contract MarketEchidnaAdvanced is MarketEchidna { // build a position buildWrapper(isLong, collateral); + // calculate the expected oi and oi shares + uint256 posOi = collateral * 1e18 / feed.price(); + uint256 posOiShares = (oiBefore == 0 || oiSharesBefore == 0) + ? posOi + : oiSharesBefore * posOi / oiBefore; + (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); - assert(oiAfter > oiBefore); - assert(oiSharesAfter > oiSharesBefore); + assert(oiAfter == oiBefore + posOi); + assert(oiSharesAfter == oiSharesBefore + posOiShares); revert(); // revert state changes during the invariant check } @@ -118,14 +127,17 @@ contract MarketEchidnaAdvanced is MarketEchidna { (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); + (,,,,,,uint240 posOiShares,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + uint256 posOi = oiSharesBefore == 0 ? 0 : oiBefore * posOiShares / oiSharesBefore; + // unwind the whole position hevm.prank(msg.sender); market.unwind(posId, 1e18, isLong ? 0 : type(uint256).max); (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); - assert(oiAfter < oiBefore); - assert(oiSharesAfter < oiSharesBefore); + assert(oiAfter == oiBefore - posOi); + assert(oiSharesAfter == oiSharesBefore - posOiShares); revert(); // revert state changes during the invariant check } @@ -138,6 +150,9 @@ contract MarketEchidnaAdvanced is MarketEchidna { (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); + (,,,,,,uint240 posOiShares,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + uint256 posOi = oiSharesBefore == 0 ? 0 : oiBefore * posOiShares / oiSharesBefore; + // adjust the price so that position becomes liquidatable feed.setPrice(isLong ? feed.price() / 2 : feed.price() * 2); @@ -146,8 +161,8 @@ contract MarketEchidnaAdvanced is MarketEchidna { (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); - assert(oiAfter < oiBefore); - assert(oiSharesAfter < oiSharesBefore); + assert(oiAfter == oiBefore - posOi); + assert(oiSharesAfter == oiSharesBefore - posOiShares); revert(); // revert state changes during the invariant check } From 04b7ca5a6aeceb2409f24b096dd58d420f40fa8b Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Wed, 21 Feb 2024 02:14:36 +0000 Subject: [PATCH 10/15] feat: run echidna tests with medusa --- medusa.json | 78 ++++++++++++++++++++ tests/invariants/MarketEchidna.t.sol | 5 +- tests/invariants/MarketEchidnaAdvanced.t.sol | 5 +- 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 medusa.json diff --git a/medusa.json b/medusa.json new file mode 100644 index 00000000..120c29b0 --- /dev/null +++ b/medusa.json @@ -0,0 +1,78 @@ +{ + "fuzzing": { + "workers": 10, + "workerResetLimit": 50, + "timeout": 0, + "testLimit": 0, + "callSequenceLength": 100, + "corpusDirectory": "", + "coverageEnabled": true, + "deploymentOrder": ["MarketEchidnaAdvanced"], + "constructorArgs": {}, + "deployerAddress": "0x30000", + "senderAddresses": [ + "0x10000", + "0x20000", + "0x30000" + ], + "blockNumberDelayMax": 60480, + "blockTimestampDelayMax": 604800, + "blockGasLimit": 125000000, + "transactionGasLimit": 12500000, + "testing": { + "stopOnFailedTest": true, + "stopOnFailedContractMatching": true, + "stopOnNoTests": true, + "testAllContracts": false, + "traceAll": false, + "assertionTesting": { + "enabled": true, + "testViewMethods": false, + "assertionModes": { + "failOnCompilerInsertedPanic": false, + "failOnAssertion": true, + "failOnArithmeticUnderflow": false, + "failOnDivideByZero": false, + "failOnEnumTypeConversionOutOfBounds": false, + "failOnIncorrectStorageAccess": false, + "failOnPopEmptyArray": false, + "failOnOutOfBoundsArrayAccess": false, + "failOnAllocateTooMuchMemory": false, + "failOnCallUninitializedVariable": false + } + }, + "propertyTesting": { + "enabled": false, + "testPrefixes": [ + "fuzz_" + ] + }, + "optimizationTesting": { + "enabled": false, + "testPrefixes": [ + "optimize_" + ] + } + }, + "chainConfig": { + "codeSizeCheckDisabled": true, + "cheatCodes": { + "cheatCodesEnabled": true, + "enableFFI": false + } + } + }, + "compilation": { + "platform": "crytic-compile", + "platformConfig": { + "target": "tests/invariants/MarketEchidnaAdvanced.t.sol", + "solcVersion": "", + "exportDirectory": "", + "args": ["--foundry-out-directory", "forge-out"] + } + }, + "logging": { + "level": "info", + "logDirectory": "" + } +} \ No newline at end of file diff --git a/tests/invariants/MarketEchidna.t.sol b/tests/invariants/MarketEchidna.t.sol index d2fb64e1..24afa90b 100644 --- a/tests/invariants/MarketEchidna.t.sol +++ b/tests/invariants/MarketEchidna.t.sol @@ -18,9 +18,12 @@ interface IHevm { // solc-select install 0.8.10 // solc-select use 0.8.10 // -// run from base project directory with: +// run with echidna from base project directory with: // echidna tests/invariants/MarketEchidna.t.sol --contract MarketEchidna --config tests/invariants/MarketEchidna.yaml // +// run with medusa from base project directory with: +// medusa fuzz +// // Reference: https://github.com/overlay-market/v1-core/blob/main/tests/markets/conftest.py contract MarketEchidna { IHevm hevm = IHevm(0x7109709ECfa91a80626fF3989D68f67F5b1DD12D); diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index c9fc947f..0a2673ff 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -8,8 +8,11 @@ import {TestUtils} from "./TestUtils.sol"; // solc-select install 0.8.10 // solc-select use 0.8.10 // -// run from base project directory with: +// run with echidna from base project directory with: // echidna tests/invariants/MarketEchidnaAdvanced.t.sol --contract MarketEchidnaAdvanced --config tests/invariants/MarketEchidnaAdvanced.yaml +// +// run with medusa from base project directory with: +// medusa fuzz contract MarketEchidnaAdvanced is MarketEchidna { // ghost variables used to verify invariants mapping(address => uint256[]) positions; // owner => positionIds From 626d998b669b3728a55cc9e3d289c2dd622fd9d1 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Wed, 21 Feb 2024 02:22:11 +0000 Subject: [PATCH 11/15] fix(medusa): remove invalid sender address --- medusa.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/medusa.json b/medusa.json index 120c29b0..ba49142c 100644 --- a/medusa.json +++ b/medusa.json @@ -11,9 +11,8 @@ "constructorArgs": {}, "deployerAddress": "0x30000", "senderAddresses": [ - "0x10000", - "0x20000", - "0x30000" + "0x1000000000000000000000000000000000000000", + "0x2000000000000000000000000000000000000000" ], "blockNumberDelayMax": 60480, "blockTimestampDelayMax": 604800, From 24a5a5a6ad3be9933097095ccd299e66dde4ab17 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Wed, 20 Mar 2024 19:52:53 +0000 Subject: [PATCH 12/15] feat: add oi shares invariant test --- tests/invariants/MarketEchidnaAdvanced.t.sol | 49 +++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index 0a2673ff..87eca30a 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -17,6 +17,9 @@ contract MarketEchidnaAdvanced is MarketEchidna { // ghost variables used to verify invariants mapping(address => uint256[]) positions; // owner => positionIds + // same as the ones specified in the fuzzer config + address[] senders = [ALICE, BOB]; + // Handler functions to "guide" the stateful fuzz test // Note: handlers have to be public in order to be called by Echidna @@ -66,7 +69,7 @@ contract MarketEchidnaAdvanced is MarketEchidna { function liquidate() public { uint256[] storage posIds = positions[msg.sender]; - require(posIds.length > 0, "No positions to unwind"); + require(posIds.length > 0, "No positions to liquidate"); uint256 posId = posIds[posIds.length - 1]; (,,,,bool isLong,,,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); @@ -169,4 +172,48 @@ contract MarketEchidnaAdvanced is MarketEchidna { revert(); // revert state changes during the invariant check } + + // Invariant 5) Sum of open position's oi shares should be equal to market's total oi shares + + event InvariantOiSharesSum(uint256 numPositions, uint256 sumLongExpected, uint256 sumLong, uint256 sumShortExpected, uint256 sumShort); + + function check_oi_shares_sum() public { + uint256 numPositions; + uint256 sumOiLongShares; + uint256 sumOiShortShares; + + // iterate over all the senders + for (uint256 i = 0; i < senders.length; i++) { + // iterate over all the positions of the sender + for (uint256 j = 0; j < positions[senders[i]].length; j++) { + (,,,,bool isLong,,uint240 oiShares,) = market.positions(keccak256( + abi.encodePacked(senders[i], positions[senders[i]][j]) + )); + + if (isLong) + sumOiLongShares += oiShares; + else + sumOiShortShares += oiShares; + + numPositions++; + } + } + + // FIXME: echidna breaks the invariant (parameters are not bounded on this output). + // check_oi_shares_sum(): failed!💥 + // Call sequence, shrinking 1675/5000: + // buildWrapper(false,0) + // unwind(29678335934853632302057079988033485840348142661768149768981112452983) + // unwind(7311677140176116913925425463138979182172112599739844530104983646231913891) + // check_oi_shares_sum() + // Event sequence: + // InvariantOiSharesSum(0, 0, 0, 275, 0) + // + // The issue is that, after unwinding a position, its fractionRemaining can be set to 0 (ie. the position is effectively closed) while its oiShares are still greater than 0. + // Example position: fractionRemaining = 0 ; oiShares = 1.73e12 + emit InvariantOiSharesSum(numPositions, market.oiLongShares(), sumOiLongShares, market.oiShortShares(), sumOiShortShares); + + assert(sumOiLongShares == market.oiLongShares()); + assert(sumOiShortShares == market.oiShortShares()); + } } From c1277438925e06dd10a7bf3dc36f37db3bdf6b96 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Mon, 25 Mar 2024 18:20:35 +0000 Subject: [PATCH 13/15] chore: format echidna tests --- tests/invariants/MarketEchidnaAdvanced.t.sol | 55 ++++++++++++-------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/tests/invariants/MarketEchidnaAdvanced.t.sol b/tests/invariants/MarketEchidnaAdvanced.t.sol index 87eca30a..5969d307 100644 --- a/tests/invariants/MarketEchidnaAdvanced.t.sol +++ b/tests/invariants/MarketEchidnaAdvanced.t.sol @@ -56,12 +56,13 @@ contract MarketEchidnaAdvanced is MarketEchidna { require(posIds.length > 0, "No positions to unwind"); uint256 posId = posIds[posIds.length - 1]; - (,,,,bool isLong,,,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + (,,,, bool isLong,,,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); hevm.prank(msg.sender); market.unwind(posId, fraction, isLong ? 0 : type(uint256).max); - (,,,,,,,uint16 fractionRemaining) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + (,,,,,,, uint16 fractionRemaining) = + market.positions(keccak256(abi.encodePacked(msg.sender, posId))); // remove the position if fully unwound if (fractionRemaining == 0) posIds.pop(); @@ -72,7 +73,7 @@ contract MarketEchidnaAdvanced is MarketEchidna { require(posIds.length > 0, "No positions to liquidate"); uint256 posId = posIds[posIds.length - 1]; - (,,,,bool isLong,,,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + (,,,, bool isLong,,,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); // adjust the price so that position becomes liquidatable setPrice(isLong ? feed.price() / 2 : feed.price() * 2); @@ -86,11 +87,7 @@ contract MarketEchidnaAdvanced is MarketEchidna { // Helper functions - function _getOiAndShares(bool isLong) - internal - view - returns (uint256 oi, uint256 oiShares) - { + function _getOiAndShares(bool isLong) internal view returns (uint256 oi, uint256 oiShares) { oi = isLong ? market.oiLong() : market.oiShort(); oiShares = isLong ? market.oiLongShares() : market.oiShortShares(); } @@ -113,9 +110,8 @@ contract MarketEchidnaAdvanced is MarketEchidna { // calculate the expected oi and oi shares uint256 posOi = collateral * 1e18 / feed.price(); - uint256 posOiShares = (oiBefore == 0 || oiSharesBefore == 0) - ? posOi - : oiSharesBefore * posOi / oiBefore; + uint256 posOiShares = + (oiBefore == 0 || oiSharesBefore == 0) ? posOi : oiSharesBefore * posOi / oiBefore; (uint256 oiAfter, uint256 oiSharesAfter) = _getOiAndShares(isLong); @@ -133,7 +129,8 @@ contract MarketEchidnaAdvanced is MarketEchidna { (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); - (,,,,,,uint240 posOiShares,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + (,,,,,, uint240 posOiShares,) = + market.positions(keccak256(abi.encodePacked(msg.sender, posId))); uint256 posOi = oiSharesBefore == 0 ? 0 : oiBefore * posOiShares / oiSharesBefore; // unwind the whole position @@ -156,7 +153,8 @@ contract MarketEchidnaAdvanced is MarketEchidna { (uint256 oiBefore, uint256 oiSharesBefore) = _getOiAndShares(isLong); - (,,,,,,uint240 posOiShares,) = market.positions(keccak256(abi.encodePacked(msg.sender, posId))); + (,,,,,, uint240 posOiShares,) = + market.positions(keccak256(abi.encodePacked(msg.sender, posId))); uint256 posOi = oiSharesBefore == 0 ? 0 : oiBefore * posOiShares / oiSharesBefore; // adjust the price so that position becomes liquidatable @@ -174,8 +172,14 @@ contract MarketEchidnaAdvanced is MarketEchidna { } // Invariant 5) Sum of open position's oi shares should be equal to market's total oi shares - - event InvariantOiSharesSum(uint256 numPositions, uint256 sumLongExpected, uint256 sumLong, uint256 sumShortExpected, uint256 sumShort); + + event InvariantOiSharesSum( + uint256 numPositions, + uint256 sumLongExpected, + uint256 sumLong, + uint256 sumShortExpected, + uint256 sumShort + ); function check_oi_shares_sum() public { uint256 numPositions; @@ -186,21 +190,22 @@ contract MarketEchidnaAdvanced is MarketEchidna { for (uint256 i = 0; i < senders.length; i++) { // iterate over all the positions of the sender for (uint256 j = 0; j < positions[senders[i]].length; j++) { - (,,,,bool isLong,,uint240 oiShares,) = market.positions(keccak256( - abi.encodePacked(senders[i], positions[senders[i]][j]) - )); + (,,,, bool isLong,, uint240 oiShares,) = market.positions( + keccak256(abi.encodePacked(senders[i], positions[senders[i]][j])) + ); - if (isLong) + if (isLong) { sumOiLongShares += oiShares; - else + } else { sumOiShortShares += oiShares; + } numPositions++; } } // FIXME: echidna breaks the invariant (parameters are not bounded on this output). - // check_oi_shares_sum(): failed!💥 + // check_oi_shares_sum(): failed!💥 // Call sequence, shrinking 1675/5000: // buildWrapper(false,0) // unwind(29678335934853632302057079988033485840348142661768149768981112452983) @@ -211,7 +216,13 @@ contract MarketEchidnaAdvanced is MarketEchidna { // // The issue is that, after unwinding a position, its fractionRemaining can be set to 0 (ie. the position is effectively closed) while its oiShares are still greater than 0. // Example position: fractionRemaining = 0 ; oiShares = 1.73e12 - emit InvariantOiSharesSum(numPositions, market.oiLongShares(), sumOiLongShares, market.oiShortShares(), sumOiShortShares); + emit InvariantOiSharesSum( + numPositions, + market.oiLongShares(), + sumOiLongShares, + market.oiShortShares(), + sumOiShortShares + ); assert(sumOiLongShares == market.oiLongShares()); assert(sumOiShortShares == market.oiShortShares()); From db5b117d8fef9e6f9f6799a898c8d6018131ba30 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Mon, 25 Mar 2024 15:22:35 -0300 Subject: [PATCH 14/15] fix(market): dead shares after position is fully unwound --- contracts/OverlayV1Market.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/contracts/OverlayV1Market.sol b/contracts/OverlayV1Market.sol index 36de250a..05ef9e99 100644 --- a/contracts/OverlayV1Market.sol +++ b/contracts/OverlayV1Market.sol @@ -358,6 +358,11 @@ contract OverlayV1Market is IOverlayV1Market, Pausable { // oiShares and fraction remaining of initial position pos.oiShares -= uint240(pos.oiSharesCurrent(fraction)); pos.fractionRemaining = pos.updatedFractionRemaining(fraction); + // ensure there are no dead shares left + if (pos.fractionRemaining == 0) { + _reduceOIAndOIShares(pos, ONE); + pos.oiShares = 0; + } positions.set(msg.sender, positionId, pos); } From 2164a5da3cc1dbe443c89a0edc9d60e72a252638 Mon Sep 17 00:00:00 2001 From: Ezequiel Date: Mon, 25 Mar 2024 16:05:35 -0300 Subject: [PATCH 15/15] fix: only apply fix when needed --- contracts/OverlayV1Market.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/OverlayV1Market.sol b/contracts/OverlayV1Market.sol index 05ef9e99..f128f8df 100644 --- a/contracts/OverlayV1Market.sol +++ b/contracts/OverlayV1Market.sol @@ -359,7 +359,7 @@ contract OverlayV1Market is IOverlayV1Market, Pausable { pos.oiShares -= uint240(pos.oiSharesCurrent(fraction)); pos.fractionRemaining = pos.updatedFractionRemaining(fraction); // ensure there are no dead shares left - if (pos.fractionRemaining == 0) { + if (pos.fractionRemaining == 0 && pos.oiShares > 0) { _reduceOIAndOIShares(pos, ONE); pos.oiShares = 0; }