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

Develop Functionality to Supply Collateral and Borrow w/ CompoundV2 #167

Closed
wants to merge 41 commits into from

Conversation

0xEinCodes
Copy link
Contributor

@0xEinCodes 0xEinCodes commented Dec 14, 2023

Core changes:

This PR encompasses the integration of CompoundV2 into Sommelier. It revolves around the development and testing of an adaptor (or adaptors) that includes features such as:

  1. Supply BaseAssets to different Compound V2 lending markets (already done w/ CTokenAdaptor.sol
  2. Deciding to mark supplied base assets as collateral or not. This is done via calling enterMarkets() on the Comptroller
  3. Borrowing and repaying back BaseAssets to different Compound V2 lending markets.
  4. Claiming Rewards from Compound V2 lending positions.

The design may be split into two or more different adaptors. It is TBD as this PR develops.

Currently, this PR is in the research, design, and discussion phase.


Current Design / TODOs

  1. General Research
  • Review Docs, codebase and any other miscellaneous aspects (blog posts, etc.)
  • Begin a conversation with their integrations teams so we have people to discuss with
  • Discuss w/ Crispy design
  • Review etherscan txs to verify code tx flows for supplying assets, adding Collateral, open borrows, repayments.
  1. CTokenAdaptorV2.sol handles supplying 'Collateral` to a Cellar's lending market account / position.
  • Review CompoundV2 Docs and Codebase
  • Write out function architecture for smart contracts w/ pseudo code
  • PR Review concepts before writing implementation code
  • Write Implementation Code
  • Write tests
  • Peer Review
  • Resolve PR CRs
  • Final PR Review
  • Audit
  1. CompoundV2DebtAdaptor.sol handles borrowing BaseAsset (has to be coordinated with a CompoundV3CollateralAdaptor position with collateral provided.
  • Review CompoundV2 Docs and Codebase
  • Write out function architecture for smart contracts w/ pseudo code
  • PR Review concepts before writing implementation code
  • Write Implementation Code
  • Write tests
  • Peer Review
  • Resolve PR CRs
  • Final PR Review
  • Audit
  1. CompoundLendingAdaptor.sol handles lending BaseAsset to a respective CompoundMarket.
  • This has already been taken care of via the CTokenAdaptor.sol previously made
  1. There may be a HealthFactorLogic.sol type file providing helpers in calculating updated health factors.

References

  1. CompoundV2 Docs
  2. CompoundV2 Supply Assets Blog Post
  3. CompoundV2 Borrow Assets Blog Post
  4. CompoundV2 Repo

crispymangoes and others added 16 commits November 22, 2023 13:35
* Work on strategist functions

* Finish setup for test

* Get crvUSD Pool working

* Test adaptor functions for working with ERC20s and native ETH

* Get a good test contract going, and add comments about handling rate tokens

* Get as many pools working as possible before handling rate pools

* Add support for rate assets with CurveEMAExtension

* Get all pools working with liquidity add and removes

* Add in test for staking and unstaking curve LP in gauge

* update test

* Finish vast majority of curve adaptor tests

* Update comments in CurveAdaptor

* Try to fix CI failing

* Add missing 2Pool extension tests

* Add in natspec to CurveAdaptor

* Add in extra TODOs

* Add in a ton of tests, and natsepc to curve helper

* remove intentional revert test

* Add in last missing test

* Try to fix weird fuzz test failure

* Add in TODOs from initial talk with auditor
* Write pseudo-code strat fns w/ ConvexAdaptor

* Develop deposit & withdraw fns w/ convex adaptor

* Develop draft-version remaining adaptor fns w/ convex

* Outline testing concepts in comments in ConvexAdaptor.t.sol

* Update nat spec w/ ConvexAdaptor.sol

* Begin writing setup() for convex adaptor tests

* Debug ConvexAdaptor.sol so it compiles

* Begin working on ConvexFraxAdaptor.sol

* Replace immutable booster w/ immutable voterProxy

* Write withdrawLockedAndUnwrap() implementation

* Write comments outlining next steps for getReward()

* Reconfig setup() in ConvexCurveAdaptor.t.sol

* Finish rough setup() for ConvexCurveAdaptor.t.sol

* Write majority rough tests w/ ConvexCurveAdaptor

* Begin working through compilation errors for convex-curve

* Reduce storage reads via adding lpt  param to adaptorData

* Reformat & remove TODOs that are resolved

* Write comparison logic btw adaptorData & queried PoolInfo

* Resolve PR#155 CRs & async CRs w/ re-entrancy checks

* Write base fns tests w/ CurveConvexAdaptor & minor fixes

* Debug more compilation errors in test & adaptor

* Resolve main bugs w/ testManagingVanillaCurveLPTs1

* Continue troubleshooting ConvexCurveAdaptor.t.sol tests

* Resolve non-managingCurveLPT unit tests

* Resolve last bugs w/ ConvexCurveAdaptor.t.sol for PR

* PR#155 CRs - Write CVX reward tests & extra validate code

* Resolve extra CRs w/ PR#155

* Reformat & remove unused test code

* Add CurveHelper to ConvexCurveAdaptor test suite

* Remove lingering TODOs w/ ConvexCurveAdaptor

* Fix  &  & remove convex-frax files

* Fix lingering period in code

* Resolve warnings about unused params w/ getRewards()
* Rough out implementation

* Add simple gas test

* Rework withdraw queue

* plan out user set fee

* Update queue with simpler logic

* Add comment about front run attack vector

* Add share to events

* Hash out more tests

* Add in potential TODO

* Add in solver helper function

* Add natspec to WithdrawQueue

* Plan out simple Solver

* Update comments, and add important bug fix

* Add missing natspec, and TODO that is an auditor Q

* Add in extra safety checks to simple solver

* Add extra TODO note
* Finish 1 TODO and add missing test TODO

* Add reentrancy guard with unstructured storage

* Fully implement unstructured storage reentrancy lock

* Add in missing test where we repeat native eth in input array

* Add checks to validate curve addresses Cellar is trying to work with

* Add in TODOs from audit check in

* Merge changes from dev but remove changes to Curve code

* Add proof of concept attack vector

* Add helper function to get the underlying token array.

* Fix attack vector where strateagist passes in the wrong token array

* Implement delta balance transfers for proxy functions

* Add informational to Curve2PoolExtension about additional protections not seen in the extension.

* Remove unsued code, and update comments.

* Update 2 pool extension to use safer pricing method, and add new tests (#163)

* Update 2 pool extension to use safer pricing method, and add new tests

* Add missing natspec

* Remove old TODO
* Write rough deposit() logic

* Write rough withdraw() logic

* Write rough mint() logic

* Write rough redeem() logic

* Reformat natspec

* Optimize deposit() w/ previewDeposit()

* Write pseudo-code for happy-path tests

* Write test basic setup() & testDeposit()

* Finish rough happy path tests

* Start reversion tests

* Finish rough test code w/ TODOs

* Resolve CRs from PR #161

* Debug tests up to underflow/overlow

* Resolve remaining failing tests

* Add _revokeExternalApproval() helper fn

* Resolve PR #161 CRs from Crispy

* Remove TODOs after chat w/ Crispy
* Add in TODOs

* Add in bounds checks, just need to refactor tests, and add tests checking for bounds reverts

* Draft up MockCurvePricingSource.sol for tests

* Resolve compilation errors due to _enforceBounds

* Write remaining _enforeBounds() tests

* Remove TODO & outdated comments

---------

Co-authored-by: 0xEinCodes <[email protected]>
* Add isLiquid bool to ERC20 Adaptor

* Add ERC20 Adaptor tests
#166)

* Add initiator value to finishSolve and greatly simplify SimpleSolver logic

* Finish remaining TODOs
@0xEinCodes 0xEinCodes self-assigned this Dec 14, 2023
@0xEinCodes 0xEinCodes changed the base branch from main to dev/macro-audit-14 December 14, 2023 04:03
Base automatically changed from dev/macro-audit-14 to main December 15, 2023 15:16
@0xEinCodes 0xEinCodes changed the base branch from main to dev/macro-audit-15 December 15, 2023 15:34
@0xEinCodes
Copy link
Contributor Author

0xEinCodes commented Dec 15, 2023

Questions for Compound:

CTokenAdaptor.sol

  • redemption: do we need to call exitMarket() really? It seems like we don't need to when redeeming when looking at redeemAllowedInternal logic within CToken.sol
    • Upon chat w/ Crispy we want to actually have this function so we can 'toggle' a CollateralPosition to be a lending position (aka not supporting any open borrows). --> TODO we need to have HF checked within a exitMarket() check

CompoundV2DebtAdaptor.sol

  • Clarification on the math, specifically for health factor calculations --> within getHypotheticalAccountLiquidityInternal() in the comptroller we are seeing usage of struct Exp that should be converting respective params into 18 decimal representations for fixed decimals (5.1 = 5.1e18).
    • What are the units of exchangeRate, oraclePrice, tokensToDenom? Is it underlying/cToken, usd/underlying, usd/cToken, respectively?

@0xEinCodes
Copy link
Contributor Author

0xEinCodes commented Dec 20, 2023

This PR is being shelved until sometime in January 2024 when it will be picked up to be audited again. Here are the important notes when picking it up again:

What was done so far:

Contracts developed: CompoundV2DebtAdaptor.sol, and additional tweaks to CTokenAdaptor.sol was done where integration w/ mutative functions from CompoundV2 were carried out (entering / exiting markets, borrowing, repaying debt)

Contracts being troubleshot: CompoundV2HelperLogicVersionB.sol, and CompoundV2HelperLogicVersionA.sol, and aspects of the CompoundV2DebtAdaptor.sol where health factor was involved. Once health factor was figured out then the CTokenAdaptor.sol could also be adjusted as needed since its positions could affect a cellar's health factor ultimately.

Main Aspects that Need Further Work (where a dev will pick up from):

*See TODOs outlined in code files.

  • Troubleshoot HealthFactor with both versions to see if lossy-ness is of concern when calculating HF vars compared to how CompoundV2 does it. AKA is there much discrepancy, if not, go with optionA as it is simpler and more gas efficient.
  • Figure out the best way to accommodate native assets per network (native ETH for example). This may require adjustments to the cellar code so we need to look at this more. We were initially hoping to just do the same workaround as we did for Curve integration. That does not seem possible with CompoundV2's codebase.
  • Finalize with full test suite

Detailed Notes to Assist when Picking this Up Again:

Decimals for HF:

  • Development of these adaptors was fine until I was stuck on figuring out the best way to mirror CompoundV2's calculations for a position's final liquidity (and thus their health factor implicitly). This was to be done within reason wrt precision. I went into their code, researched their docs, and then troubleshot with tests. Further detailed discussions with Crispy help outline two possible paths to take, and THUS there are two versions of HelperLogic to consider. Both options basically carry out the high level equations:

Equation 1: totalActualCollateral(before Liquidation) = looped sum of ( cToken balance * exchangeRate * priceOracleValue * collateralFactor) and
Equation 2: totalBorrowForPosition = looped sum of (borrowBalance * oraclePriceFromCompoundOracles)

The difference between the two versions is the scaling factors used throughout each version. Version A gives up precision throughout calculating HF (when compared to what CompoundV2 does behind the scenes), and Version B mimics the precision of CompoundV2 but has to deal with more vars and more gas consumption.

  • Option A: scales with 10 ** 18
  • Option B: scales with different factors as per the decimals of exchangeRate and oracle.getUnderlyingPrice(asset).

Extra Extra Notes

  • See both contract comments for more details, AND see extra test file: CompoundTempHFTest.
  • CompoundTempHFTest.sol was made because Compound.t.sol was having errors when trying to add a cUSDC debt position to the test setup, and instead of reconfiguring already auditing test-code for the previous version of CTokenAdaptor.sol within Compound.t.sol, I felt it better to make a temporary test contract to work with when troubleshooting HF and debt positions for now. This is a temporary fix until we get Compound.t.sol working with debt positions like cUSDC in addition to its current setup. The error that came up basically was limiting me on how many positions I could have in the test cellar.

Handling Native Assets (like ETH):

  • Crispy took a look at this more in depth and it seems that cellar code has to be adjusted.

Below are screenshots from the CompoundV2 docs regarding the scalingFactors used for variables within the calculation of health factor.
image
image

The links below were useful for dissecting the CompoundV2 codebase alongside their github and docs.

https://etherscan.deth.net/address/0x39aa39c021dfbae8fac545936693ac917d5e7563
https://etherscan.deth.net/address/0x3d9819210A31b4961b30EF54bE2aeD79B9c9Cd3B

Base automatically changed from dev/macro-audit-15 to main January 15, 2024 16:36
crispymangoes and others added 12 commits January 15, 2024 09:16
* Feat/curve position support (#156)

* Work on strategist functions

* Finish setup for test

* Get crvUSD Pool working

* Test adaptor functions for working with ERC20s and native ETH

* Get a good test contract going, and add comments about handling rate tokens

* Get as many pools working as possible before handling rate pools

* Add support for rate assets with CurveEMAExtension

* Get all pools working with liquidity add and removes

* Add in test for staking and unstaking curve LP in gauge

* update test

* Finish vast majority of curve adaptor tests

* Update comments in CurveAdaptor

* Try to fix CI failing

* Add missing 2Pool extension tests

* Add in natspec to CurveAdaptor

* Add in extra TODOs

* Add in a ton of tests, and natsepc to curve helper

* remove intentional revert test

* Add in last missing test

* Try to fix weird fuzz test failure

* Add in TODOs from initial talk with auditor

* Design, Develop, and Test Convex Adaptors (#155)

* Write pseudo-code strat fns w/ ConvexAdaptor

* Develop deposit & withdraw fns w/ convex adaptor

* Develop draft-version remaining adaptor fns w/ convex

* Outline testing concepts in comments in ConvexAdaptor.t.sol

* Update nat spec w/ ConvexAdaptor.sol

* Begin writing setup() for convex adaptor tests

* Debug ConvexAdaptor.sol so it compiles

* Begin working on ConvexFraxAdaptor.sol

* Replace immutable booster w/ immutable voterProxy

* Write withdrawLockedAndUnwrap() implementation

* Write comments outlining next steps for getReward()

* Reconfig setup() in ConvexCurveAdaptor.t.sol

* Finish rough setup() for ConvexCurveAdaptor.t.sol

* Write majority rough tests w/ ConvexCurveAdaptor

* Begin working through compilation errors for convex-curve

* Reduce storage reads via adding lpt  param to adaptorData

* Reformat & remove TODOs that are resolved

* Write comparison logic btw adaptorData & queried PoolInfo

* Resolve PR#155 CRs & async CRs w/ re-entrancy checks

* Write base fns tests w/ CurveConvexAdaptor & minor fixes

* Debug more compilation errors in test & adaptor

* Resolve main bugs w/ testManagingVanillaCurveLPTs1

* Continue troubleshooting ConvexCurveAdaptor.t.sol tests

* Resolve non-managingCurveLPT unit tests

* Resolve last bugs w/ ConvexCurveAdaptor.t.sol for PR

* PR#155 CRs - Write CVX reward tests & extra validate code

* Resolve extra CRs w/ PR#155

* Reformat & remove unused test code

* Add CurveHelper to ConvexCurveAdaptor test suite

* Remove lingering TODOs w/ ConvexCurveAdaptor

* Fix  &  & remove convex-frax files

* Fix lingering period in code

* Resolve warnings about unused params w/ getRewards()

* Add lpt transferrance w/ withdraw()

* Remove unnecessary LoCs in withdraw()

* Feat/withdraw queue (#158)

* Rough out implementation

* Add simple gas test

* Rework withdraw queue

* plan out user set fee

* Update queue with simpler logic

* Add comment about front run attack vector

* Add share to events

* Hash out more tests

* Add in potential TODO

* Add in solver helper function

* Add natspec to WithdrawQueue

* Plan out simple Solver

* Update comments, and add important bug fix

* Add missing natspec, and TODO that is an auditor Q

* Add in extra safety checks to simple solver

* Add extra TODO note

* Resolve minor fixes from Macro Audit w/ ConvexCurveAdaptor

* Fix/macro audit 14 (#157)

* Finish 1 TODO and add missing test TODO

* Add reentrancy guard with unstructured storage

* Fully implement unstructured storage reentrancy lock

* Add in missing test where we repeat native eth in input array

* Add checks to validate curve addresses Cellar is trying to work with

* Add in TODOs from audit check in

* Merge changes from dev but remove changes to Curve code

* Add proof of concept attack vector

* Add helper function to get the underlying token array.

* Fix attack vector where strateagist passes in the wrong token array

* Implement delta balance transfers for proxy functions

* Add informational to Curve2PoolExtension about additional protections not seen in the extension.

* Remove unsued code, and update comments.

* Update 2 pool extension to use safer pricing method, and add new tests (#163)

* Update 2 pool extension to use safer pricing method, and add new tests

* Add missing natspec

* Remove old TODO

* Make a small simplification refactor

* Move variable declaration up so all values sit in the same slot

* Add evm version to toml, and remove console import from ConvexCurveAdaptor

* Feat/simple slippage router (#161)

* Write rough deposit() logic

* Write rough withdraw() logic

* Write rough mint() logic

* Write rough redeem() logic

* Reformat natspec

* Optimize deposit() w/ previewDeposit()

* Write pseudo-code for happy-path tests

* Write test basic setup() & testDeposit()

* Finish rough happy path tests

* Start reversion tests

* Finish rough test code w/ TODOs

* Resolve CRs from PR #161

* Debug tests up to underflow/overlow

* Resolve remaining failing tests

* Add _revokeExternalApproval() helper fn

* Resolve PR #161 CRs from Crispy

* Remove TODOs after chat w/ Crispy

* Fix/bounding curve pricing (#165)

* Add in TODOs

* Add in bounds checks, just need to refactor tests, and add tests checking for bounds reverts

* Draft up MockCurvePricingSource.sol for tests

* Resolve compilation errors due to _enforceBounds

* Write remaining _enforeBounds() tests

* Remove TODO & outdated comments

---------

Co-authored-by: 0xEinCodes <[email protected]>

* Feat/illiquid erc20 (#164)

* Add isLiquid bool to ERC20 Adaptor

* Add ERC20 Adaptor tests

* Add initiator value to finishSolve and greatly simplify SimpleSolver … (#166)

* Add initiator value to finishSolve and greatly simplify SimpleSolver logic

* Finish remaining TODOs

* Plan out multi asset support

* Rework math to account for share appreciation

* Update mullti asset deposit math

* Add natspec and add revert tests

* Add advanced cellar permutations, and reduce cellar contract size

* Update Cellar so deposit event emits asset deposited, and beforeDeposit now knows the asset deposited

* Fix PR comments

---------

Co-authored-by: 0xEinCodes <[email protected]>
Co-authored-by: 0xEinCodes <[email protected]>
* Write rough MorphoBlueSupplyAdaptor.sol

* Write rough MorphoBlueCollateralAdaptor.sol

* Write very rough debt adaptor w/ morpho blue

* Finish rough implementation w/ MBSupplyAdaptor

* Write rough implementation of HF calcs

* Write MBCollateralAdaptor rough implementation

* Write MBDebtAdaptor rough implementation

* Work on implementation of repayment fn

* Finish basic repayMBDebt implementation

* Update natspec for MB adaptors

* Start debugging compilation errors

* Continue debugging up to Id UserDefined Syntax errors

* Finish debugging compilation errors

* Begin writing tests & write strat fns for supplyAdaptor

* Debug supplyAdaptor strat fns & continue tests

* Write basic fuzz tests for collateralAdaptor

* Finish rough separate tests for collateral & debt adaptors

* Write rough implementation of combo collat/debt tests

* Write remaining rough tests for collat/debt adaptors

* Write rough tests for supplyAdaptor

* Clean up setup() & begin debugging tests

* Continue to debug MorphoBlue adaptors & hf calcs

* Resolve hf calcs by fixing mockMBPriceFeed

* Debug repayment & multiple position tests

* Resolve remaining collat & debt adaptor tests except 1 fuzz

* Debug most MorphoBlueSupplyAdaptor tests

* Delete resolved TODOs in morphoBlue tests

* Remove resolved TODOs within morphoBlue adaptors

* Resolve more TODOs tests

* Resolve supplyAdaptor withdraw bug w/ receiver

* Add IrmMock in supply tests to try resolving testAccrueInterest()

* Resolve accrueInterest() test w/ Crispy

* Resolve testMultipleMorphoBluePositions fuzzing

* Resolve some CRs from PR #154 Review

* Resolve remaining CRs for PR #154

* Finish balanceOf test for supplyAdaptor

* Clean up small formatting fixes in tests

* Rename MBHealthFactor logic file to MBHelperLogic

* Make most CRs as per Morpho team PR #154 review

* Use MorphoLib for _userBorrowBalance() in helper logic

* Reformat MBSupplyAdaptor code & tests

* Reformat MBCollateral adaptorData input

* Reformat MBDebt adaptorData w/ MarketParams

* Add isLiquid config to supplyAdaptor

* Make final PR #154 CRs w/ crispy

* Fix failing tests

---------

Co-authored-by: crispymangoes <[email protected]>
* Separate normal deposits and multi asset deposits, and add preview function.

* Add missing test and remove old commented code

* Copy over code to more advanced cellar permutations

* Add natspec clarifying who can call multi asset only owner functions.

* Add missing natspec

* Address warning

* Implement missing advanced permutations

* Separate deposit events into 2 separate events

* Add better natspec to the multi asset deposit event
* Copy over multichain share files

* Fix submodules

* Try to debug dependency bug

* forge install: ccip

v2.6.0

* Still debugging

* forge install: ccip

ccip-develop

* update gitignore

* Write additional natspec for CCIP share-bridging contracts

* Correct small natspec spelling error

* Fix PR comments

* Address hidden PR comments

---------

Co-authored-by: 0xEinCodes <[email protected]>
Copy link
Contributor

coderabbitai bot commented Feb 7, 2024

Important

Auto Review Skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

// Check for errors.
if (errorCode != 0) revert CTokenAdaptor__NonZeroCompoundErrorCode(errorCode);

uint256 hf = _getHealthFactor(address(this), comptroller);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is left here currently as I am troubleshooting tests testBorrowInSameCollateralMarket and testStrategistWithdrawTooLowHF in CompoundV2AdditionalTests.t.sol.

I was comparing the HF calculated here vs the methods I created within the test file itself. It all shows that the HF is calculated the same way. The problem is the amountToWithdraw calculated using the helper. That seems to be incorrect but I keep looking at it and can't seem to get atop of what is incorrect.

);

uint256 lowerThanMinHF = 1.05e18;
uint256 amountToWithdraw = _generateAmountBasedOnHFOptionA(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crispymangoes, this is what my comment is referring to in regards to calculating the proper amountToWithdraw that will make this test pass.

// uint256 borrowAmountNeeded = (sumCollateral.mulDivDown(1e18, _hfRequested) - sumBorrow);
// return borrowAmountNeeded;
} else {
uint256 withdrawAmountNeeded = (sumCollateral - (sumBorrow.mulDivDown(_hfRequested, 1e18)) / (10 ** (18 - _borrowDecimals)));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crispymangoes this is the helper function that is giving me issues for the two failing tests, specifically the lines of code within this else statement.

@0xEinCodes
Copy link
Contributor Author

0xEinCodes commented Feb 13, 2024

TODOs that I'd like some help with:

Checking over Health Factor calculations and helpers / tests

  • I reconfigured tests: testBorrowInSameCollateralMarket and testStrategistWithdrawTooLowHF in CompoundV2AdditionalTests.t.sol, using the helper _generateAmountBasedOnHFOptionA, and need someone else to take a look at the algebra. I've checked by hand the needed withdrawal amount to get the requested health factor to cause these tests to fail, but when checking with the HF logic within the implementation and test logic, it shows that the resultant health factor is not at all the one we desire. See LoC 1096 onward, and LoC 325 and LoC 1330.
  • Use of native assets (ETH for example)

@0xEinCodes
Copy link
Contributor Author

This is paused due to other priorities right now. Notes on where things are left off for future reference:

  • Handling of native assets was being debugged and closer to being implemented.
  • Health Factor just needs to be relooked at.

@0xEinCodes 0xEinCodes closed this Jul 3, 2024
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.

2 participants