Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ributions - Add HealthStatement struct for aggregate-only health summaries - Refactor BalanceSheet to store per-token effective collateral/debt maps - Add withUpdatedContributions method for immutable per-token updates - Fix sumUFix128 bug (missing .values on effectiveDebt map) - Update _getUpdatedBalanceSheet to build per-token maps - Update FlowALPHealth adjustment functions to use withUpdatedContributions - Bundle effectiveCollateral/effectiveDebt params into HealthStatement for computeRequiredDepositForHealth and computeAvailableWithdrawal Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… pattern Replace delta-based switch/case logic with a cleaner approach: 1. Compute post-withdrawal true balance via trueBalanceAfterWithdrawal helper 2. Compute new effective contribution from the resulting balance 3. Use withUpdatedContributions to build the new BalanceSheet The wrapper now constructs a TokenSnapshot instead of passing individual price/factor/index parameters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ttern Symmetric to the withdrawal refactor. Adds trueBalanceAfterDeposit helper. The wrapper now constructs a TokenSnapshot instead of passing individual price/factor/index parameters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…elpers
These ~40-line functions duplicated the adjustment logic. Now they
delegate to computeAdjustedBalancesAfter{Deposit,Withdrawal} and
return adjusted.health.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eAvailableWithdrawal Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ose it Renames the SignedQuantity struct to Balance (more natural name) and eliminates the redundant direction field from InternalBalance by making its scaledBalance field a Balance (direction + quantity) instead of a bare UFix128. The InternalBalance init signature is preserved to minimize churn at construction sites. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Balance Consolidate the repeated switch-on-direction pattern in FlowALPHealth into two new methods that work with Balance directly, replacing the old withUpdatedContributions which took separate optional collateral/debt values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…terDelta Consolidate the two mirror-image functions into a single trueBalanceAfterDelta that accepts a Balance (direction + quantity) as the delta argument. Deposits pass a Credit delta, withdrawals pass a Debit delta. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The balance field was globally mutable -- this makes it only mutable through type-defined functions.
…trueBalance helper Add TokenSnapshot.trueBalance helper to deduplicate scaled→true balance conversion. Refactor computeRequiredDepositForHealth and computeAvailableWithdrawal from 8 params to 4 by accepting TokenSnapshot instead of individual scalars. Remove debug logging, rename variables to generic names, and rename balanceSheet→initialBalanceSheet. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| let snap = view.snapshots[tokenType]! | ||
|
|
||
| switch balance.direction { | ||
| switch balance.getScaledBalance().direction { |
There was a problem hiding this comment.
It seems that a few lines above, we already calculated the effectiveCollateralTotal & effectiveDebtTotal, as part of:
let preHealth = FlowALPModels.healthFactor(view: view)And FlowALPModels.healthFactor, seems to be doing the exact same calculation here.
Not directly related to the changes, but I thought of bringing this up, as a potential code reduction to include as well.
There was a problem hiding this comment.
I agree. There is a secondary implementation of the health logic, on PositionView, which can execute in the view read-only context. I'll take a look to see if healthFactor can easily return a BalanceSheet -- then we'd still have the duplicated balance sheet construction logic in the codebase, but not the duplicated work in this function, at least.
Conceptually, it would be ideal if there was one balance sheet construction object that was oblivious to whether you gave it a PositionView or an InteralPosition, as long as they can both provide the necessary data (likewise with TokenSnapshot vs TokenState).
| let price = UFix128(self.config.getPriceOracle().price(ofToken: type)!) | ||
|
|
||
| switch balance.direction { | ||
| switch balance.getScaledBalance().direction { |
There was a problem hiding this comment.
It seems that a bit of indirection was added, to be able to access the direction & quantity properties of the Balance struct. Maybe we could have dedicated getters for those 2, in the InternalBalance type?
| let convertedCollateralFactor = UFix128(self.config.getCollateralFactor(tokenType: type)) | ||
| effectiveCollateral = effectiveCollateral + (value * convertedCollateralFactor) | ||
| let collateralFactor = UFix128(self.config.getCollateralFactor(tokenType: type)) | ||
| effectiveCollateralByToken[type] = (price * trueBalance) * collateralFactor |
There was a problem hiding this comment.
The parentheses here shouldn't be necessary, unless they convey more meaning when grouping the individual terms.
| /// Returns the true balance of the given token in this position, accounting for interest. | ||
| /// Returns balance 0.0 if the position has no balance stored for the given token. | ||
| access(all) view fun trueBalance(ofToken: Type): UFix128 { | ||
| access(all) fun trueBalance(ofToken: Type): UFix128 { |
There was a problem hiding this comment.
Was there any particular reason to remove the view annotation?
There was a problem hiding this comment.
Because it calls trueBalance, which has a compiler error when I make it view. The compiler error was on the line where Balance instance is constructed. I don't really understand why that line violates view annotation rules, but I didn't look into it very deeply.
| initial: FlowALPModels.Balance, | ||
| target: FlowALPModels.Balance | ||
| ): UFix128 { | ||
| let Credit = FlowALPModels.BalanceDirection.Credit |
There was a problem hiding this comment.
I could be wrong here, but I think we can compute this by re-using minDepositForTargetBalance, e.g.:
return self.minDepositForTargetBalance(initial: target, target: initial)We are saving only a few lines of code, not sure if it's worth the extra mental effort to understand it, but I feel like this invariant should hold. Thoughts?
Co-authored-by: Ardit Marku <markoupetr@gmail.com> Co-authored-by: Jordan Schalm <jordan.schalm@gmail.com>
2 participants
This PR refactors several functions and types related to health calculations, with the goal of reducing complexity and duplication.
Balancetype, representing a signed numeric valueBalancetype and handle both deposits and withdrawals.InternalBalancenow has aBalance-typed internal field. This field was changed to beaccess(self)(previously was globally mutable).BalanceSheet:HealthStatementis a summary of effective debt/collateral totals (equivalent toBalanceSheetstructure before)BalanceSheetnow includes a per-token accounting of effective debts and collaterals. This makes some health-related calculations much simpler, because we can isolate the effect of one token.