Skip to content

FLO-15: Fix incorrect linearization in fundsAvailableAboveTargetHealthAfterDepositing#284

Open
jordanschalm wants to merge 4 commits intomainfrom
jord/flo-15
Open

FLO-15: Fix incorrect linearization in fundsAvailableAboveTargetHealthAfterDepositing#284
jordanschalm wants to merge 4 commits intomainfrom
jord/flo-15

Conversation

@jordanschalm
Copy link
Copy Markdown
Member

Closes #224.

Extends #275 (review that first).

This PR:

  • Adds a test case for FLO-15
  • Adds buildTokenSnapshot to avoid inline construction of token snapshots
  • Modifies the implementation of fundsAvailableAboveTargetHealthAfterDepositing to correctly handle same deposit/withdrawal types, by simulating the deposit on the Balance passed to the underlying health calculation function.

jordanschalm and others added 2 commits March 19, 2026 10:56
The shortcut in fundsAvailableAboveTargetHealthAfterDepositing incorrectly
treats a same-type deposit as a simple additive offset. When the deposit
flips a debit balance to credit, the first portion repays debt (scaled by
1/borrowFactor) while the excess becomes collateral (scaled by
collateralFactor). With cf=0.8 and bf=1.0, this overestimates by 10
(returns 150 instead of 140).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove the incorrect shortcut in fundsAvailableAboveTargetHealthAfterDepositing
that assumed depositing and withdrawing the same token is a simple additive
offset. Instead, always compute the post-deposit true balance via
trueBalanceAfterDelta, which correctly handles debt repayment vs collateral
addition when a deposit flips a debit balance to credit.

Also extract buildTokenSnapshot helper to deduplicate 6 identical
TokenSnapshot constructor calls in FlowALPv0, and change
FlowALPHealth.computeAvailableWithdrawal to accept a true balance directly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jordanschalm jordanschalm requested a review from a team as a code owner March 19, 2026 20:47
Base automatically changed from balance-sheet-health-statement-refactor to main April 6, 2026 15:30
jordanschalm and others added 2 commits April 6, 2026 11:26
…tation

- Fix mismatched argument label/type: convert InternalBalance to true Balance
  via TokenSnapshot before calling FlowALPHealth.computeAvailableWithdrawal
- Avoid redundant _getUpdatedBalanceSheet calls in _rebalancePositionNoLock by
  reusing the already-computed balance sheet for withdrawal/deposit calculations
- Add early returns for zero-amount deposit/withdrawal to skip buildTokenSnapshot
- Fix missing variance argument in equalWithinVariance test assertions

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@tim-barry tim-barry left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +463 to +464
// TODO
// - Test depositing withdraw type without pushing to sink, creating a Credit balance before testing
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Was this planned as part of this PR?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This TODO was just moved around in the file (from here), wasn't planning to address it here.

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