fix(credit): honour USE_SAFE_PRICES_FLAG in partial-liquidation collateral check - #313
Closed
SashaMIT wants to merge 1 commit into
Closed
Conversation
…teral check partiallyLiquidateCreditAccount captures the flags returned by _tryWithdrawPhantomToken - which routes an adapter call into an external contract whose state a main price feed can read - but reads them only for EXTERNAL_CONTRACT_WAS_CALLED_FLAG and then hardcodes useSafePrices: false in the final _fullCollateralCheck. The multicall path reacts to the same condition by switching the final collateral check to safe prices (flags & USE_SAFE_PRICES_FLAG). Safe pricing is materially stricter: PriceOracleV3._getSafePrice returns min(main, reserve) and ZERO for a token with no reserve feed, so a check that should have failed on safe prices can pass on a manipulated main-feed price. The useSafePrices:false literal predates the phantom-token call itself (introduced in e3ff71e; phantom withdrawals grafted in later in 4c7d434), which supports this being an omission rather than a decision. Honour the flag.
Author
|
Superseded by a stronger fix: always use safe prices after partial-liquidation withdrawals (matches multicall withdraw semantics; #313 only forwarded the phantom adapter flag). Closing in favor of the new PR. |
This file contains hidden or 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
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.
Problem
partiallyLiquidateCreditAccountdoes the two things that, everywhere else in this facade, switch the final collateral check onto safe prices:_tryWithdrawPhantomTokenroutes an adapter call into an arbitrary registered target contract via_externalCall(L421).It captures the returned flags — but reads them only for
EXTERNAL_CONTRACT_WAS_CALLED_FLAG(L424) and then passes a hardcodeduseSafePrices: falseinto_fullCollateralCheck(L434). L434 is the only hardcodedfalsein the facade; the multicall path reacts to the identical condition withuseSafePrices: flags & USE_SAFE_PRICES_FLAG != 0(L648).Safe pricing is materially stricter, not cosmetically:
PriceOracleV3._getSafePricereturnsmin(main, reserve), and returns zero for any token with no reserve feed configured. So a check that should have failed on safe prices can pass on a manipulated main-feed price.The git history supports this being an omission, not a decision: the
useSafePrices: falseliteral entered this function ine3ff71e(feat: partial liquidations), while the phantom-token adapter call was grafted into the same function later, in4c7d434(feat: phantom token withdrawals). The safe-price decision predates the introduction of the external call it was meant to guard against.Fix
Pass
flags & USE_SAFE_PRICES_FLAG != 0instead offalse, matching the multicall path's handling of the identical condition.Verification
forge buildis clean (all lint warnings pre-exist incontracts/test/). One-line change inside the existing flags plumbing; no interface or signature changes.Made with Cursor
Made with Cursor