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 poolInfoUtils.borrowerInfo to return thresholdPrice #1017

Merged
merged 80 commits into from
Dec 19, 2023

Conversation

MikeHathaway
Copy link
Collaborator

@MikeHathaway MikeHathaway commented Dec 8, 2023

Description

update poolInfoUtils.borrowerInfo interface

Intended for consumption by Subgraph.

Purpose

Impact

Tasks

  • Changes to protocol contracts are covered by unit tests executed by CI.
  • Protocol contract size limits have not been exceeded.
  • Gas consumption for impacted transactions have been compared with the target branch, and nontrivial changes cited in the Impact section above.
  • Scope labels have been assigned as appropriate.
  • Invariant tests have been manually executed as appropriate for the nature of the change.

EdNoepel and others added 30 commits November 20, 2023 09:12
* charge fee on all deposit

* unit tests compile

* test harness updates

* working on ERC20PoolQuoteTokenTest

* removed deposit fee cap

* more work on ERC20PoolQuoteTokenTest

* do not charge deposit fee if moving liquidity to higher price
* this underflows instead of giving expected revert

* move isCollateralized check after updating borrower collateral

* remove local calculation of encumbered collateral

* trying to properly fix testBorrowRepayPrecision

* resolve rounding issue in fuzz test

* testCollateralization improvements

* updated unit tests for new _collateralization implementation

* more collateralization tests

* Add 1.04 factor in borrower collateralization

* Update nptp ratio to '1 + sqrt(r)/2'

* Remove Settle debt with pool reserves

* Remove 0.995 factor from claimable reserves calculation

* Update bond factor calculation to minimum 0.005

* added testcase where debt exceeds deposit

* updated test so debt exceeds deposit

* allow  up to half of current orig fee to be used to settle bad debt

* updated testTakeAndSettle

* more test fixes

* Enabled settling with all reserves if
 Deposits.treeSum==0 or 72 hrs pass

* cleanup

* Half orig fee res | Matt example (#966)

* added Matts test as proof that attack no longer works on his branch

* Revert "Remove multicall from position manager (#948)" (#961)

This reverts commit f540c8a.

* added test testSpendOrigFeePushBadDebtToBorrowers test

* cleaned up testStealReservesWithMarginm to match minted balances

* responded to Matts comments

---------

Co-authored-by: Ian Harvey <iharvey@comcast.net>
Co-authored-by: Mike Hathaway <mahathaway93@gmail.com>

* Revert "Remove Settle debt with pool reserves"

This reverts commit 290d6cf.

* Update half origination fees reserves settlement time to 144 hours from kickTime

* Fix alignment and extra spaces

* Fix some unit tests

* PR feedback

* Update encumberance and collateralization method in poolInfoUtils

* Fix some unit tests

---------

Co-authored-by: Ed Noepel <ed@noepel.net>
Co-authored-by: Ian Harvey <iharvey@comcast.net>
Co-authored-by: mwc <matt@ajna.finance>
Co-authored-by: Ian Harvey <ith.harvey@gmail.com>
Co-authored-by: Mike Hathaway <mahathaway93@gmail.com>
…and ERC20PoolLiquidationsLenderKickAuctionTest
* fix most position manager tests

* fix additional pm tests

* fix rewards requiredCollateralRewards setup

* fix ClaimRewards tests

* update additional rewards manager tests

* fix additional tests

* more test fixes

* commit wip bankruptcy tests

* fixed testMoveLiquidityToOverwriteBankruptBucket

* fix additional tests

* fix testMoveLiquidityWithDebtInPool

* fix remaining rewards manager tests

---------

Co-authored-by: Mike <mikehathaway@makerdao.com>
Co-authored-by: mwc <matt@ajna.finance>
* Add 1.04 factor in HTP calculations

* Add COLLATERALIZATION_FACTOR constant in PoolHelpers

* Add collateralization factor in dwatp

* Fix poolPricesInfo

* Update ERC20PoolBorrowFuzzyTest

* Fix some unit tests

* Fix some unit tests

* Fix some unit tests

* Update ERC20PoolTransferLPs

* fix most rewards manager tests

* update remaining rewards manager tests

* update ERC721SubsetPoolBorrowTest and commit wip changes to testMergeOrRemoveERC721Collateral

* updated testSettlePartialDebtSubsetPool (#988)

* updated testSettlePartialDebtSubsetPool

* re-added teardown

---------

Co-authored-by: Ian Harvey <iharvey@comcast.net>

* fix ERC721PoolCollateral tests

* fix borrowRepayDebtFuzzy and additional PM tests

* cleaned up testBorrowAndRepayWith4DecimalQuote

---------

Co-authored-by: Mike <mikehathaway@makerdao.com>
Co-authored-by: Ian Harvey <ith.harvey@gmail.com>
Co-authored-by: Ian Harvey <iharvey@comcast.net>
Copy link
Contributor

@EdNoepel EdNoepel left a comment

Choose a reason for hiding this comment

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

Comment inline.

src/PoolInfoUtils.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@EdNoepel EdNoepel left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@prateek105 prateek105 left a comment

Choose a reason for hiding this comment

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

lgtm, Added a comment

* @return debt_ Current debt owed by borrower (`WAD`).
* @return collateral_ Pledged collateral, including encumbered (`WAD`).
* @return t0Np_ `Neutral price` (`WAD`).
* @return thresholdPrice_ Borrower's `Threshold Price` in t0 terms (`WAD`).
Copy link
Contributor

Choose a reason for hiding this comment

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

  • I think thresholdPrice_ is not in t0 terms.
  • Nit: Natspec spacing

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed -> 685f2e1

Copy link
Contributor

@ith-harvey ith-harvey left a comment

Choose a reason for hiding this comment

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

LGTM

@ith-harvey ith-harvey merged commit f23c988 into develop Dec 19, 2023
3 checks passed
@MikeHathaway MikeHathaway deleted the update-borrower-info branch December 20, 2023 16:31
Copy link

@dmitriia dmitriia left a comment

Choose a reason for hiding this comment

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

Looks ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants