From 24a60a501543565310b65f7fad5eccd1c0490e6e Mon Sep 17 00:00:00 2001 From: Jordan Schalm Date: Thu, 19 Mar 2026 10:56:42 -0700 Subject: [PATCH 1/2] Add test exposing same-type deposit/withdraw linearization bug (FLO-15) 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) --- ...nds_available_above_target_health_test.cdc | 111 +++++++++++++++++- 1 file changed, 108 insertions(+), 3 deletions(-) diff --git a/cadence/tests/funds_available_above_target_health_test.cdc b/cadence/tests/funds_available_above_target_health_test.cdc index e7bec820..7579c8de 100644 --- a/cadence/tests/funds_available_above_target_health_test.cdc +++ b/cadence/tests/funds_available_above_target_health_test.cdc @@ -4,6 +4,7 @@ import BlockchainHelpers import "test_helpers.cdc" import "MOET" +import "FlowToken" import "FlowALPv0" import "FlowALPEvents" import "FlowALPModels" @@ -321,9 +322,110 @@ fun testFundsAvailableAboveTargetHealthAfterDepositingWithoutPushFromOvercollate log("==============================") } -// TODO -// - Test deposit & withdraw same type -// - Test depositing withdraw type without pushing to sink, creating a Credit balance before testing +/// Demonstrates that the same-type shortcut in fundsAvailableAboveTargetHealthAfterDepositing +/// produces an incorrect (overestimated) result when: +/// 1. depositType == withdrawType +/// 2. The position holds a Debit balance in that type +/// 3. depositAmount > the Debit balance (causing a direction flip from Debit to Credit) +/// 4. collateralFactor != borrowFactor for that token +/// +/// The shortcut returns fundsAvailableAboveTargetHealth + depositAmount, +/// assuming a simple additive offset. But when the deposit first repays debt +/// (scaled by 1/borrowFactor) and the excess becomes collateral (scaled by +/// collateralFactor), the two legs have different health impacts. +access(all) +fun testfundsAvailableAboveTargetHealthAfterDepositing_flo15_sameTypeDepositWithdrawLinearizationBug() { + if snapshot < getCurrentBlockHeight() { + Test.reset(to: snapshot) + } + + // --- Setup: LP provides FLOW liquidity, then borrower takes a FLOW loan --- + + // Mint extra FLOW for LP liquidity (setup already minted positionFundingAmount=100) + mintFlow(to: userAccount, amount: 50.0) + + // LP position (pid 0): deposit 150 FLOW as liquidity + let lpFlowDeposit = 150.0 + let lpOpenRes = executeTransaction( + "../transactions/flow-alp/position/create_position.cdc", + [lpFlowDeposit, FLOW_VAULT_STORAGE_PATH, false], + userAccount + ) + Test.expect(lpOpenRes, Test.beSucceeded()) + + // Mint MOET for the borrower position's collateral + let moetCollateral = 130.0 + mintMoet(signer: PROTOCOL_ACCOUNT, to: userAccount.address, amount: moetCollateral, beFailed: false) + + // Borrower position (pid 1): deposit MOET as collateral + let borrowerOpenRes = executeTransaction( + "../transactions/flow-alp/position/create_position.cdc", + [moetCollateral, MOET.VaultStoragePath, false], + userAccount + ) + Test.expect(borrowerOpenRes, Test.beSucceeded()) + + // Borrow 110 FLOW against the MOET collateral -> creates FLOW Debit + let flowDebt = 110.0 + borrowFromPosition( + signer: userAccount, + positionId: 1, + tokenTypeIdentifier: FLOW_TOKEN_IDENTIFIER, + vaultStoragePath: FLOW_VAULT_STORAGE_PATH, + amount: flowDebt, + beFailed: false + ) + + let borrowerPid: UInt64 = 1 + + // Verify initial state + let details = getPositionDetails(pid: borrowerPid, beFailed: false) + let moetCredit = getCreditBalanceForType(details: details, vaultType: Type<@MOET.Vault>()) + let flowDebitBal = getDebitBalanceForType(details: details, vaultType: Type<@FlowToken.Vault>()) + Test.assert(equalWithinVariance(moetCollateral, moetCredit), message: "Expected \(moetCollateral) MOET Credit, got \(moetCredit)") + Test.assert(equalWithinVariance(flowDebt, flowDebitBal), message: "Expected \(flowDebt) FLOW Debit, got \(flowDebitBal)") + + // Call fundsAvailableAboveTargetHealthAfterDepositing with a FLOW deposit exceeding our debit balance and a FLOW withdrawal type + let depositAmount = 150.0 + let result = fundsAvailableAboveTargetHealthAfterDepositing( + pid: borrowerPid, + withdrawType: FLOW_TOKEN_IDENTIFIER, + targetHealth: INT_TARGET_HEALTH, // 1.3 + depositType: FLOW_TOKEN_IDENTIFIER, + depositAmount: depositAmount, + beFailed: false + ) + log("[TEST] fundsAvailableAboveTargetHealthAfterDepositing returned: \(result)") + + // --- Expected correct result --- + // + // After depositing 150 FLOW into a position with 110 FLOW Debit: + // - First 110 repays debt entirely (FLOW Debit -> 0) + // - Remaining 40 becomes collateral (FLOW Credit = 40) + // + // New effective values (FLOW price=1.0, cf=0.8, bf=1.0): + // EC = 130 * 1.0 * 1.0 + 40 * 1.0 * 0.8 = 130 + 32 = 162 + // ED = 0 + // + // Max FLOW withdrawal from this state: + // Withdraw X <= 40: reduces FLOW Credit, no debt -> health stays infinite + // Withdraw X > 40: FLOW Debit = X - 40 + // EC = 130 (MOET only) + // ED = (X - 40) * 1.0 / 1.0 + // health = 130 / (X - 40) >= 1.3 + // X - 40 <= 130 / 1.3 = 100 + // X <= 140 + // + // The buggy shortcut does not account for different collateral/borrow factors: + // fundsAvailableAboveTargetHealth(FLOW) = 0 (health 1.182 < target 1.3) + // shortcut = 0 + 150 = 150 <- WRONG (overestimates by 10) + // + let expectedCorrect = (depositAmount - flowDebt) + (moetCollateral / 1.3) + log("[TEST] Expected correct result: \(expectedCorrect)") + log("[TEST] Buggy shortcut would return: \(depositAmount)") + + Test.assert(equalWithinVariance(expectedCorrect, result), message: "Same-type linearization bug: expected ~\(expectedCorrect), got \(result).") +} /* --- Parameterized runner --- */ @@ -357,3 +459,6 @@ fun runFundsAvailableAboveTargetHealthAfterDepositing( Test.assert(equalWithinVariance(expectedAvailable, actualAvailable), message: "Values are not equal within variance - expected: \(expectedAvailable), actual: \(actualAvailable)") } + +// TODO +// - Test depositing withdraw type without pushing to sink, creating a Credit balance before testing From 97cc2b52d921dbfb9fa7713cd3170bbf58983bf6 Mon Sep 17 00:00:00 2001 From: Jordan Schalm Date: Thu, 19 Mar 2026 13:43:02 -0700 Subject: [PATCH 2/2] Fix same-type deposit/withdraw linearization bug (FLO-15) 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) --- cadence/contracts/FlowALPHealth.cdc | 9 +- cadence/contracts/FlowALPv0.cdc | 133 ++++++++-------------------- 2 files changed, 41 insertions(+), 101 deletions(-) diff --git a/cadence/contracts/FlowALPHealth.cdc b/cadence/contracts/FlowALPHealth.cdc index f074d05e..a9a779dd 100644 --- a/cadence/contracts/FlowALPHealth.cdc +++ b/cadence/contracts/FlowALPHealth.cdc @@ -57,7 +57,7 @@ access(all) contract FlowALPHealth { /// @param delta: The deposit or withdrawal to apply to the balance. /// @param tokenSnapshot: The TokenSnapshot for the token type denominating the balance and delta parameters. /// @return The true balance after applying the delta. - access(self) fun trueBalanceAfterDelta( + access(account) fun trueBalanceAfterDelta( balance maybeInitialBalance: FlowALPModels.InternalBalance?, delta: FlowALPModels.Balance, tokenSnapshot: FlowALPModels.TokenSnapshot @@ -307,7 +307,7 @@ access(all) contract FlowALPHealth { /// @param targetHealth: The minimum health ratio to maintain. /// @return The maximum amount of tokens (in UFix64) that can be withdrawn. access(account) fun computeAvailableWithdrawal( - withdrawBalance: FlowALPModels.InternalBalance?, + withdrawTrueBalance: FlowALPModels.Balance, withdrawType: Type, withdrawSnapshot: FlowALPModels.TokenSnapshot, initialBalanceSheet: FlowALPModels.BalanceSheet, @@ -324,10 +324,7 @@ access(all) contract FlowALPHealth { targetHealth: targetHealth ) - let initialBalance = withdrawBalance ?? FlowALPModels.makeZeroInternalBalance() - let currentTrueBalance = withdrawSnapshot.trueBalance(balance: initialBalance) - - let delta = self.maxWithdrawalForTargetBalance(initial: currentTrueBalance, target: requiredBalance) + let delta = self.maxWithdrawalForTargetBalance(initial: withdrawTrueBalance, target: requiredBalance) return FlowALPMath.toUFix64RoundDown(delta) } } diff --git a/cadence/contracts/FlowALPv0.cdc b/cadence/contracts/FlowALPv0.cdc index 86e6e403..6ca6def4 100644 --- a/cadence/contracts/FlowALPv0.cdc +++ b/cadence/contracts/FlowALPv0.cdc @@ -394,16 +394,7 @@ access(all) contract FlowALPv0 { let view = self.buildPositionView(pid: pid) // Build a TokenSnapshot for the requested withdraw type (may not exist in view.snapshots) - let tokenState = self._borrowUpdatedTokenState(type: type) - let snap = FlowALPModels.TokenSnapshot( - price: UFix128(self.config.getPriceOracle().price(ofToken: type)!), - credit: tokenState.getCreditInterestIndex(), - debit: tokenState.getDebitInterestIndex(), - risk: FlowALPModels.RiskParamsImplv1( - collateralFactor: UFix128(self.config.getCollateralFactor(tokenType: type)), - borrowFactor: UFix128(self.config.getBorrowFactor(tokenType: type)), - ) - ) + let snap = self.buildTokenSnapshot(type: type) let withdrawBal = view.balances[type] let uintMax = FlowALPv0.maxWithdraw( @@ -699,16 +690,7 @@ access(all) contract FlowALPv0 { withdrawType: Type, withdrawAmount: UFix64 ): FlowALPModels.BalanceSheet { - let tokenState = self._borrowUpdatedTokenState(type: withdrawType) - let snapshot = FlowALPModels.TokenSnapshot( - price: UFix128(self.config.getPriceOracle().price(ofToken: withdrawType)!), - credit: tokenState.getCreditInterestIndex(), - debit: tokenState.getDebitInterestIndex(), - risk: FlowALPModels.RiskParamsImplv1( - collateralFactor: UFix128(self.config.getCollateralFactor(tokenType: withdrawType)), - borrowFactor: UFix128(self.config.getBorrowFactor(tokenType: withdrawType)) - ) - ) + let snapshot = self.buildTokenSnapshot(type: withdrawType) return FlowALPHealth.computeAdjustedBalancesAfterWithdrawal( initialBalanceSheet: initialBalanceSheet, @@ -726,21 +708,10 @@ access(all) contract FlowALPv0 { initialBalanceSheet: FlowALPModels.BalanceSheet, targetHealth: UFix128 ): UFix64 { - let tokenState = self._borrowUpdatedTokenState(type: depositType) - let depositSnapshot = FlowALPModels.TokenSnapshot( - price: UFix128(self.config.getPriceOracle().price(ofToken: depositType)!), - credit: tokenState.getCreditInterestIndex(), - debit: tokenState.getDebitInterestIndex(), - risk: FlowALPModels.RiskParamsImplv1( - collateralFactor: UFix128(self.config.getCollateralFactor(tokenType: depositType)), - borrowFactor: UFix128(self.config.getBorrowFactor(tokenType: depositType)) - ) - ) - return FlowALPHealth.computeRequiredDepositForHealth( initialBalance: position.getBalance(depositType), depositType: depositType, - depositSnapshot: depositSnapshot, + depositSnapshot: self.buildTokenSnapshot(type: depositType), initialBalanceSheet: initialBalanceSheet, targetHealth: targetHealth ) @@ -771,35 +742,54 @@ access(all) contract FlowALPv0 { if self.config.isDebugLogging() { log(" [CONTRACT] fundsAvailableAboveTargetHealthAfterDepositing(pid: \(pid), withdrawType: \(withdrawType.contractName!), targetHealth: \(targetHealth), depositType: \(depositType.contractName!), depositAmount: \(depositAmount))") } - if depositType == withdrawType && depositAmount > 0.0 { - // If the deposit and withdrawal types are the same, we compute the available funds assuming - // no deposit (which is less work) and increase that by the deposit amount at the end - let fundsAvailable = self.fundsAvailableAboveTargetHealth( - pid: pid, - type: withdrawType, - targetHealth: targetHealth - ) - return fundsAvailable + depositAmount - } let balanceSheet = self._getUpdatedBalanceSheet(pid: pid) let position = self._borrowPosition(pid: pid) - let adjusted = self.computeAdjustedBalancesAfterDeposit( + let balanceSheetAfterDeposit = self.computeAdjustedBalancesAfterDeposit( initialBalanceSheet: balanceSheet, position: position, depositType: depositType, depositAmount: depositAmount ) - return self.computeAvailableWithdrawal( - position: position, + // Compute the post-deposit true balance for the withdraw type. + // When depositType == withdrawType, trueBalanceAfterDelta accounts for the deposit's effect. + // When they differ, the zero delta leaves the balance unchanged. + let withdrawSnapshot = self.buildTokenSnapshot(type: withdrawType) + let withdrawDepositAmount = depositType == withdrawType ? depositAmount : 0.0 + let withdrawTrueBalance = FlowALPHealth.trueBalanceAfterDelta( + balance: position.getBalance(withdrawType), + delta: FlowALPModels.Balance( + direction: FlowALPModels.BalanceDirection.Credit, + quantity: UFix128(withdrawDepositAmount) + ), + tokenSnapshot: withdrawSnapshot + ) + + return FlowALPHealth.computeAvailableWithdrawal( + withdrawTrueBalance: withdrawTrueBalance, withdrawType: withdrawType, - initialBalanceSheet: adjusted, + withdrawSnapshot: withdrawSnapshot, + initialBalanceSheet: balanceSheetAfterDeposit, targetHealth: targetHealth ) } + // Builds a up-to-date TokenSnapshot instance for the given type. + access(self) fun buildTokenSnapshot(type: Type): FlowALPModels.TokenSnapshot { + let tokenState = self._borrowUpdatedTokenState(type: type) + return FlowALPModels.TokenSnapshot( + price: UFix128(self.config.getPriceOracle().price(ofToken: type)!), + credit: tokenState.getCreditInterestIndex(), + debit: tokenState.getDebitInterestIndex(), + risk: FlowALPModels.RiskParamsImplv1( + collateralFactor: UFix128(self.config.getCollateralFactor(tokenType: type)), + borrowFactor: UFix128(self.config.getBorrowFactor(tokenType: type)) + ) + ) + } + // Helper function to compute balances after deposit access(self) fun computeAdjustedBalancesAfterDeposit( initialBalanceSheet: FlowALPModels.BalanceSheet, @@ -807,50 +797,12 @@ access(all) contract FlowALPv0 { depositType: Type, depositAmount: UFix64 ): FlowALPModels.BalanceSheet { - let tokenState = self._borrowUpdatedTokenState(type: depositType) - let snapshot = FlowALPModels.TokenSnapshot( - price: UFix128(self.config.getPriceOracle().price(ofToken: depositType)!), - credit: tokenState.getCreditInterestIndex(), - debit: tokenState.getDebitInterestIndex(), - risk: FlowALPModels.RiskParamsImplv1( - collateralFactor: UFix128(self.config.getCollateralFactor(tokenType: depositType)), - borrowFactor: UFix128(self.config.getBorrowFactor(tokenType: depositType)) - ) - ) - return FlowALPHealth.computeAdjustedBalancesAfterDeposit( initialBalanceSheet: initialBalanceSheet, depositBalance: position.getBalance(depositType), depositType: depositType, depositAmount: depositAmount, - tokenSnapshot: snapshot - ) - } - - // Helper function to compute available withdrawal - access(self) fun computeAvailableWithdrawal( - position: &{FlowALPModels.InternalPosition}, - withdrawType: Type, - initialBalanceSheet: FlowALPModels.BalanceSheet, - targetHealth: UFix128 - ): UFix64 { - let tokenState = self._borrowUpdatedTokenState(type: withdrawType) - let withdrawSnapshot = FlowALPModels.TokenSnapshot( - price: UFix128(self.config.getPriceOracle().price(ofToken: withdrawType)!), - credit: tokenState.getCreditInterestIndex(), - debit: tokenState.getDebitInterestIndex(), - risk: FlowALPModels.RiskParamsImplv1( - collateralFactor: UFix128(self.config.getCollateralFactor(tokenType: withdrawType)), - borrowFactor: UFix128(self.config.getBorrowFactor(tokenType: withdrawType)) - ) - ) - - return FlowALPHealth.computeAvailableWithdrawal( - withdrawBalance: position.getBalance(withdrawType), - withdrawType: withdrawType, - withdrawSnapshot: withdrawSnapshot, - initialBalanceSheet: initialBalanceSheet, - targetHealth: targetHealth + tokenSnapshot: self.buildTokenSnapshot(type: depositType) ) } @@ -2024,16 +1976,7 @@ access(all) contract FlowALPv0 { let snaps: {Type: FlowALPModels.TokenSnapshot} = {} let balancesCopy = position.copyBalances() for t in position.getBalanceKeys() { - let tokenState = self._borrowUpdatedTokenState(type: t) - snaps[t] = FlowALPModels.TokenSnapshot( - price: UFix128(self.config.getPriceOracle().price(ofToken: t)!), - credit: tokenState.getCreditInterestIndex(), - debit: tokenState.getDebitInterestIndex(), - risk: FlowALPModels.RiskParamsImplv1( - collateralFactor: UFix128(self.config.getCollateralFactor(tokenType: t)), - borrowFactor: UFix128(self.config.getBorrowFactor(tokenType: t)), - ) - ) + snaps[t] = self.buildTokenSnapshot(type: t) } return FlowALPModels.PositionView( balances: balancesCopy,