Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
|
||
| /// Simulates the withdrawable amount of `type` if the position were fully rebalanced (liquidation scenario). | ||
| /// Returns the amount in units of `type`. | ||
| access(all) fun simulateLiquidationAmount(pid: UInt64, type: Type): UFix64 { |
There was a problem hiding this comment.
I'm realizing we may be overloading the term "liquidation" with this. When we say "liquidation" here, does this have anything to do with the liquidation path in the sense one would think of with respect to a lending protocol? In other words, are we using liquidation here to mean fully exiting the position or reclaiming collateral in the case of bad debt?
There was a problem hiding this comment.
in this case it means fully closing/exiting a position at that moment
There was a problem hiding this comment.
I left several comments, but I'll highlight my main concerns here:
- I don't think the simulation method deals with balances of multiple token types
- This method is a bit difficult to test. I think it would be best to implement this in the patterns consistent with the refactor (e.g. #36) where core calculations are made in pure functions. Maybe this method could take
BalanceSheet,topUpAvailable,topUpType, andwithdrawType(likely not exhaustive) which would allow the method to be more easily called from a unit test with minimal setup - It's not clear to me if
liquidationimplies the repayment of bad debt or closing out a position. Maybe we consider renaming if this isn't involved liquidation in the traditional sense. Between this and the previous point, the interface could look something like:
access(all)
fun calculateCloseoutBalance(
balanceSheet: BalanceSheet,
topUpAvailable: UFix64,
topUpType: Type,
topUpTokenState: &TokenState,
withdrawType: Type,
withdrawBalance: &InternalBalance?
): UFix64 {| let uintSourceAmount = DeFiActionsMathUtils.toUInt128(sourceAmountUFix) | ||
|
|
||
| // ----- Deposit token leg (this position's balance in `type`) ----- | ||
| let maybeDepositBalance = position.balances[type] |
There was a problem hiding this comment.
Couldn't a balance also exist for withdrawals on a type? I think the variable name is throwing me off since I'd think the balance returned is the amount available for withdrawal from the position, which in my mind conflicts with deposit here.
Also, I think we need to check the full position balance across all withdrawals, not just the source type and provided type. Once we're dealing with balances related to more than two type, I think the balances of the third type (and beyond) will be omitted from this calculation.
For instance, let's say the topUpSource provides MOET and the requested Type is FLOW, how would this method consider a credit or debit in a third token type?
| let sourceType = topUpSource.getSourceType() | ||
|
|
||
| // Prices (in default token units) | ||
| let maybeDepositTokenPrice = self.priceOracle.price(ofToken: type) |
There was a problem hiding this comment.
Isn't this the price of type being withdrawn?
There was a problem hiding this comment.
yes, it is, I think the confusion is between deposit as a noun and deposit as a verb, I'll rename it
| } | ||
| let uintLiquidationAmountInType = DeFiActionsMathUtils.div(netQuote, uintDepositTokenPrice) | ||
|
|
||
| return DeFiActionsMathUtils.toUFix64Round(uintLiquidationAmountInType) |
There was a problem hiding this comment.
I would think we want to round down here since this amount denotes a withdrawal from a position. Is that right?
Co-authored-by: Giovanni Sanchez <108043524+sisyphusSmiling@users.noreply.github.com>
|
closed in favour of another approach |
2 participants
connected PR
onflow/FlowActions#25