Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update epsilon for F1 and F2 invariants #765

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Conversation

prateek105
Copy link
Contributor

Description of bug or vulnerability and solution

  • F1 and F2 invariants fail in settleAuction handler of ERC20Pool, with the difference between pool deposit and fenwick deposits of <1e17 but >1e16 for deposits of order 1e25
  • Fixed by updating epsilon in F1, F2 from 1e16 to 1e17.

Copy link
Contributor

@mattcushman mattcushman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine, diff is on the order of 1e-8

@prateek105 prateek105 merged commit 2be0f7b into develop Apr 26, 2023
@prateek105 prateek105 deleted the fix-invariant-F1-F2 branch April 26, 2023 04:59
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants