Invariant framework improvements #769
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grandizzy
approved these changes
Apr 26, 2023
grandizzy
dismissed
ith-harvey’s stale review
per Prateek comment, please review considering the last 2 commits added
ith-harvey
approved these changes
Apr 26, 2023
There was a problem hiding this comment.
What we have here looks good. I'm worried about deposit take i.e. bucketTake when deposit flag is true...
Looks like we are not prepping a bucket to have deposit in it but simply performing the take regardless of the bucket's deposit amount?
I'm fine if this issue is tackled in a separate PR though.
Will brainstorm on this and add this in a separate PR. |
ith-harvey
pushed a commit
that referenced
this pull request
Apr 27, 2023
* changes needed for contract verification (#764) * changes needed for contract verification * gotcha * Make NFT tokens claimable after `take`, `bucketTake`, `settle` actions (#763) * Add test failure * When ERC721.bucketTake always rebalance token ids from borrower to pool claimable array * Consistent take with bucketTake, always rebalance tokens * Check invariants in regression test * Always rebalance tokens on settle (settle could enter with 0 collateral but still with bad debt) * Changed epsilon from 1e16 to 1e17 in F1 and F2 invariants (#765) * Add variable token precision for ERC721Pool invariants (#755) * Add multiple precision of quote token in erc721Pool invariants * Add shebang and set -e option * Invariant framework improvements (#769) * Update forge library, set default env vars using envOr instead reading from env file * Remove InvariantsTestHelpers contract and move invariants helper functions to BaseInvariants * env cleanup for invariants * Update invariant testing scripts * Add skipTime in merge collateral handler * Add fuzzed time skip between kick and take/bucketTake --------- Co-authored-by: grandizzy <[email protected]> * Removing brownie TODOs as these tests won't be further expanded (#767) * ERC20 R1_R2_R3_R4_R5_R6_R7_R8 invariant failure (#771) * Add failing test * Change cutoff for deminimus check to 1 LP in a bucket for exchange rate diffs --------- Co-authored-by: mwc <[email protected]> * Erc721 CT2 invariant failure (#770) * Reproduce CT2 invariant failure * Add collateral that could be compensated in bucket 7388 when auction settled * Make sure MAX bucket not accounted twice. Fix test-regression target * Fix bucketTake invariant logic in place instead in CT2 invariant. Added regression test comments. * Fix CI, lock foundry version until resolution for foundry-rs/foundry#4835 --------- Co-authored-by: Ed Noepel <[email protected]> Co-authored-by: Prateek Gupta <[email protected]> Co-authored-by: mwc <[email protected]>
ith-harvey
added a commit
that referenced
this pull request
Jun 7, 2023
* initial draft * got initial scaffolding running * included Matts invaraints * added PM1 and PM5 * Reward invariants merged (#773) * changes needed for contract verification (#764) * changes needed for contract verification * gotcha * Make NFT tokens claimable after `take`, `bucketTake`, `settle` actions (#763) * Add test failure * When ERC721.bucketTake always rebalance token ids from borrower to pool claimable array * Consistent take with bucketTake, always rebalance tokens * Check invariants in regression test * Always rebalance tokens on settle (settle could enter with 0 collateral but still with bad debt) * Changed epsilon from 1e16 to 1e17 in F1 and F2 invariants (#765) * Add variable token precision for ERC721Pool invariants (#755) * Add multiple precision of quote token in erc721Pool invariants * Add shebang and set -e option * Invariant framework improvements (#769) * Update forge library, set default env vars using envOr instead reading from env file * Remove InvariantsTestHelpers contract and move invariants helper functions to BaseInvariants * env cleanup for invariants * Update invariant testing scripts * Add skipTime in merge collateral handler * Add fuzzed time skip between kick and take/bucketTake --------- Co-authored-by: grandizzy <[email protected]> * Removing brownie TODOs as these tests won't be further expanded (#767) * ERC20 R1_R2_R3_R4_R5_R6_R7_R8 invariant failure (#771) * Add failing test * Change cutoff for deminimus check to 1 LP in a bucket for exchange rate diffs --------- Co-authored-by: mwc <[email protected]> * Erc721 CT2 invariant failure (#770) * Reproduce CT2 invariant failure * Add collateral that could be compensated in bucket 7388 when auction settled * Make sure MAX bucket not accounted twice. Fix test-regression target * Fix bucketTake invariant logic in place instead in CT2 invariant. Added regression test comments. * Fix CI, lock foundry version until resolution for foundry-rs/foundry#4835 --------- Co-authored-by: Ed Noepel <[email protected]> Co-authored-by: Prateek Gupta <[email protected]> Co-authored-by: mwc <[email protected]> * working through PM3 * wrapped position calls * made getPoolAccumulators a free function & cleaned up logging * included free function * increased logging robustness * working through adding positions without passing positions to baseclass * cleanup * Fix position manager invariant testing setup * Fix EVM reverts * Fix some assertions in moveliquidity handler * Move post action checks in try to run only when transaction is successful * position man initially working * removed redundant invariant check * got rewards structure running * Update Rewards Manager handler structure * Remove redundancy in Rewards Handler * regression PM1 failing * Fix invariant PM1 * Fix invariant RW1 * Add remaining rewards manager handlers * Add drawDebt before kick reserve auction to generate some reserves * Fix drawDebt evm revert in preUnstake * handled broken rewardsCap * Fixed evm revert and PM1 invariant * Add failing regression test * Update make commands for Position and Rewards Manager * added check in moveLiquidity that ensure QT amount for actor doesn't change * cleaned up * revised invariants per Prototech labs suggestion * cleanup invar * added update interest to fix QT diff on moveLiquidity * working through test failure * resolved fee on move liquidity * working through rewards * initial commit * cleanup * low haning fruit feedback * fixed takeReserves in rewards invariant * added NFT position tracking for rewards manager * cleanup * Add comments for failing regression test * responded to Mikes feedback * EOF line addition * cleaned up requires, formally added more PositionManager invar * Quote to lps inc on move (#877) * cleaned up requires, formally added more PositionManager invar * cleaned up requires, formally added more PositionManager invar * added logs * works with diff * cleanup --------- Co-authored-by: Ian Harvey <[email protected]> * cleaned up rewards and position naming * responded to class naming * cleaned up spaced --------- Co-authored-by: Ian Harvey <[email protected]> Co-authored-by: grandizzy <[email protected]> Co-authored-by: Ed Noepel <[email protected]> Co-authored-by: Prateek Gupta <[email protected]> Co-authored-by: mwc <[email protected]> Co-authored-by: prateek105 <[email protected]>
ith-harvey
added a commit
that referenced
this pull request
Jun 16, 2023
* initial draft * got initial scaffolding running * included Matts invaraints * added PM1 and PM5 * Reward invariants merged (#773) * changes needed for contract verification (#764) * changes needed for contract verification * gotcha * Make NFT tokens claimable after `take`, `bucketTake`, `settle` actions (#763) * Add test failure * When ERC721.bucketTake always rebalance token ids from borrower to pool claimable array * Consistent take with bucketTake, always rebalance tokens * Check invariants in regression test * Always rebalance tokens on settle (settle could enter with 0 collateral but still with bad debt) * Changed epsilon from 1e16 to 1e17 in F1 and F2 invariants (#765) * Add variable token precision for ERC721Pool invariants (#755) * Add multiple precision of quote token in erc721Pool invariants * Add shebang and set -e option * Invariant framework improvements (#769) * Update forge library, set default env vars using envOr instead reading from env file * Remove InvariantsTestHelpers contract and move invariants helper functions to BaseInvariants * env cleanup for invariants * Update invariant testing scripts * Add skipTime in merge collateral handler * Add fuzzed time skip between kick and take/bucketTake --------- Co-authored-by: grandizzy <[email protected]> * Removing brownie TODOs as these tests won't be further expanded (#767) * ERC20 R1_R2_R3_R4_R5_R6_R7_R8 invariant failure (#771) * Add failing test * Change cutoff for deminimus check to 1 LP in a bucket for exchange rate diffs --------- Co-authored-by: mwc <[email protected]> * Erc721 CT2 invariant failure (#770) * Reproduce CT2 invariant failure * Add collateral that could be compensated in bucket 7388 when auction settled * Make sure MAX bucket not accounted twice. Fix test-regression target * Fix bucketTake invariant logic in place instead in CT2 invariant. Added regression test comments. * Fix CI, lock foundry version until resolution for foundry-rs/foundry#4835 --------- Co-authored-by: Ed Noepel <[email protected]> Co-authored-by: Prateek Gupta <[email protected]> Co-authored-by: mwc <[email protected]> * working through PM3 * wrapped position calls * made getPoolAccumulators a free function & cleaned up logging * included free function * increased logging robustness * working through adding positions without passing positions to baseclass * cleanup * Fix position manager invariant testing setup * Fix EVM reverts * Fix some assertions in moveliquidity handler * Move post action checks in try to run only when transaction is successful * position man initially working * removed redundant invariant check * got rewards structure running * Update Rewards Manager handler structure * Remove redundancy in Rewards Handler * regression PM1 failing * Fix invariant PM1 * Fix invariant RW1 * Add remaining rewards manager handlers * Add drawDebt before kick reserve auction to generate some reserves * Fix drawDebt evm revert in preUnstake * handled broken rewardsCap * Fixed evm revert and PM1 invariant * Add failing regression test * Update make commands for Position and Rewards Manager * added check in moveLiquidity that ensure QT amount for actor doesn't change * cleaned up * revised invariants per Prototech labs suggestion * cleanup invar * added update interest to fix QT diff on moveLiquidity * working through test failure * resolved fee on move liquidity * working through rewards * initial commit * cleanup * low haning fruit feedback * fixed takeReserves in rewards invariant * added NFT position tracking for rewards manager * cleanup * Add comments for failing regression test * responded to Mikes feedback * EOF line addition * added * cleaned up requires, formally added more PositionManager invar * Quote to lps inc on move (#877) * cleaned up requires, formally added more PositionManager invar * cleaned up requires, formally added more PositionManager invar * added logs * works with diff * cleanup --------- Co-authored-by: Ian Harvey <[email protected]> * cleaned up rewards and position naming * responded to class naming * cleaned up spaced * working through rewards, tests passing * working through invariants * rewards working cleanly * clean up before looking into RW5 * finnished adding RW6 * cleanup * comment cleanup * updated with emergency unstake * cleaned up error handling --------- Co-authored-by: Ian Harvey <[email protected]> Co-authored-by: grandizzy <[email protected]> Co-authored-by: Ed Noepel <[email protected]> Co-authored-by: Prateek Gupta <[email protected]> Co-authored-by: mwc <[email protected]> Co-authored-by: prateek105 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
High level
vm.envOr.forge-stdsubmodule to 'v1.5.3'.skipTimein merge Collateral handler.kickandtake/bucketTakein take and bucketTake handler.