[VPD-893]: Integrate DeviationBoundedOracle for conservative CF path#668
[VPD-893]: Integrate DeviationBoundedOracle for conservative CF path#668
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…or bounded CF-path pricing
…ountLiquidity, improve docs
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
| VToken[] memory assets = ComptrollerInterface(comptroller).getAssetsIn(account); | ||
| uint assetsCount = assets.length; | ||
| if (weightingStrategy == WeightFunction.USE_COLLATERAL_FACTOR) { | ||
| vars.boundedOracle = ComptrollerInterface(comptroller).deviationBoundedOracle(); |
There was a problem hiding this comment.
meaning in this case spotOracle = addr(0) ? why not we always return both ?
There was a problem hiding this comment.
Yes, because only one of the oracle is used based on the chosen weighting strategy.
|
I guess we will need to update the VenusLens contract as well. It's not directly used by any other contract, but I'm thinking that fe/be might have certain dependencies. Probably we could return three prices instead of just the spot price. |
also same thing to |
|
in VAI controller#getMintableVAI, should we update this function as well ? What I saw is that we are still using the spot price instead of the updated DeviationBoundedOracle price. "Function that returns the amount of VAI a user can mint based on their account liquidy and the VAI mint rate" |
| * are gas-efficient. Should be called before any liquidity calculation in the CF path. | ||
| * @param account The account whose collateral assets need protection state updates | ||
| */ | ||
| function _getAccountLiquidity( |
There was a problem hiding this comment.
so this _getAccountLiquidity is removed ?
There was a problem hiding this comment.
yes just its just a additional refactor, It was originally just a wrapper around getHypotheticalAccountLiquidity. If we decide to keep it, we would need to maintain two versions, one view and one non-view.
Functionally, it only abstracts setting the hypothetical borrow/mint values to zero in the getHypotheticalAccountLiquidityInternal. However, this abstraction is already handled in the main external getAccountLiquidity function, which makes the wrapper unnecessory.
And this also improves readability that there only exist one getHypotheticalAccountLiquidity (and no internal wrapper which does the same thing)
There was a problem hiding this comment.
yeah, agree its fine to remove it
|
While I don't think this is a major issue or that it will have any substantial impact, would it be better to use a Deviation Oracle here? As thats what would actually be used in a borrowing scenario. 
|
2 participants
Description
Integrate DeviationBoundedOracle into Comptroller for conservative CF-path pricing
Introduce
ComptrollerV19Storagewith a newdeviationBoundedOracleslotand add
setDeviationBoundedOracleadmin setter inSetterFacetCall
dbo.updateProtectionState()for all account assets before liquiditychecks in
redeemAllowedandborrowAllowed(populates transient price cache)In
ComptrollerLens, use bounded collateral/debt prices from the DBO in thecollateral-factor path; liquidation-threshold path continues to use spot prices
I have updated the documentation to account for the changes in the code.
If I added new functionality, I added tests covering it.
If I fixed a bug, I added a test preventing this bug from silently reappearing again.
My contribution follows Venus contribution guidelines.