fix: withdrawAndPull rebalances to targetHealth when pullFromTopUpSource is true

Previously, withdrawAndPull only triggered the top-up source when health
dropped below minHealth, while depositAndPush always rebalanced to
targetHealth. This asymmetry left positions between minHealth and
targetHealth without rebalancing. Now both operations consistently
rebalance to targetHealth when their respective flags are set.

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

Author: jordanschalm

cadence/contracts/FlowALPv0.cdc

@@ -1237,8 +1237,8 @@ access(all) contract FlowALPv0 {
         /// Withdraws the requested funds from the specified position
         /// with the configurable `pullFromTopUpSource` option.
         ///
-        /// If `pullFromTopUpSource` is true, deficient value putting the position below its min health
-        /// is pulled from the position's configured `topUpSource`.
+        /// If `pullFromTopUpSource` is true, any deficiency below the position's target health
+        /// is pulled from the position's configured `topUpSource` (consistent with depositAndPush).
         /// TODO(jord): ~150-line function - consider refactoring.
         access(FlowALPModels.EPosition) fun withdrawAndPull(
             pid: UInt64,
@@ -1275,40 +1275,46 @@ access(all) contract FlowALPv0 {
             let topUpSource = position.borrowTopUpSource()
             let topUpType = topUpSource?.getSourceType() ?? self.state.getDefaultToken()
 
-            let requiredDeposit = self.fundsRequiredForTargetHealthAfterWithdrawing(
+            // Compute the deposit required to maintain minHealth — the hard requirement.
+            let minHealthDeposit = self.fundsRequiredForTargetHealthAfterWithdrawing(
                 pid: pid,
                 depositType: topUpType,
                 targetHealth: position.getMinHealth(),
                 withdrawType: type,
                 withdrawAmount: amount
             )
 
+            // When pullFromTopUpSource is true, also check whether a deposit is needed
+            // to maintain targetHealth (consistent with depositAndPush behaviour).
+            let targetHealthDeposit = pullFromTopUpSource
+                ? self.fundsRequiredForTargetHealthAfterWithdrawing(
+                    pid: pid,
+                    depositType: topUpType,
+                    targetHealth: position.getTargetHealth(),
+                    withdrawType: type,
+                    withdrawAmount: amount
+                )
+                : 0.0
+
             var canWithdraw = false
 
-            if requiredDeposit == 0.0 {
-                // We can service this withdrawal without any top up
+            if minHealthDeposit == 0.0 && targetHealthDeposit == 0.0 {
+                // No top-up needed: position stays above targetHealth (or minHealth when not pulling)
                 canWithdraw = true
             } else if pullFromTopUpSource {
                 // We need more funds to service this withdrawal, see if they are available from the top up source
                 if let topUpSource = topUpSource {
-                    // If we have to rebalance, let's try to rebalance to the target health, not just the minimum
-                    let idealDeposit = self.fundsRequiredForTargetHealthAfterWithdrawing(
-                        pid: pid,
-                        depositType: topUpType,
-                        targetHealth: position.getTargetHealth(),
-                        withdrawType: type,
-                        withdrawAmount: amount
-                    )
+                    // Try to rebalance to target health
+                    let idealDeposit = targetHealthDeposit > 0.0 ? targetHealthDeposit : minHealthDeposit
 
                     let pulledVault <- topUpSource.withdrawAvailable(maxAmount: idealDeposit)
                     assert(pulledVault.getType() == topUpType, message: "topUpSource returned unexpected token type")
                     let pulledAmount = pulledVault.balance
 
-
-                    // NOTE: We requested the "ideal" deposit, but we compare against the required deposit here.
-                    // The top up source may not have enough funds get us to the target health, but could have
-                    // enough to keep us over the minimum.
-                    if pulledAmount >= requiredDeposit {
+                    // NOTE: We requested the "ideal" deposit (targetHealth), but the hard requirement
+                    // is minHealth. The top up source may not have enough to reach targetHealth,
+                    // but the withdrawal can proceed as long as we stay above minHealth.
+                    if pulledAmount >= minHealthDeposit {
                         // We can service this withdrawal if we deposit funds from our top up source
                         self._depositEffectsOnly(
                             pid: pid,
@@ -1334,7 +1340,7 @@ access(all) contract FlowALPv0 {
                     log("    [CONTRACT] Token type: \(type.identifier)")
                     log("    [CONTRACT] Requested amount: \(amount)")
                     log("    [CONTRACT] Available balance (without topUp): \(availableBalance)")
-                    log("    [CONTRACT] Required deposit for minHealth: \(requiredDeposit)")
+                    log("    [CONTRACT] Required deposit for minHealth: \(minHealthDeposit)")
                     log("    [CONTRACT] Pull from topUpSource: \(pullFromTopUpSource)")
                 }
                 // We can't service this withdrawal, so we just abort

cadence/tests/withdraw_and_pull_rebalance_test.cdc

@@ -0,0 +1,89 @@
+import Test
+import BlockchainHelpers
+
+import "MOET"
+import "test_helpers.cdc"
+
+access(all) var snapshot: UInt64 = 0
+
+access(all)
+fun setup() {
+    deployContracts()
+
+    snapshot = getCurrentBlockHeight()
+}
+
+/// Verifies that withdrawAndPull with pullFromTopUpSource=true rebalances
+/// the position back to targetHealth, not just minHealth.
+///
+/// Setup:
+///   - User deposits 1000 FLOW (price=1.0, CF=0.8) with auto-borrow.
+///   - Position starts at targetHealth=1.3 with ~615.38 MOET debt.
+///   - User's MOET vault (topUpSource) holds ~615.38 MOET.
+///
+/// Action:
+///   - Withdraw a small FLOW amount that drops health below targetHealth
+///     but keeps it above minHealth (1.1).
+///   - Use pullFromTopUpSource=true.
+///
+/// Expected (consistent with depositAndPush):
+///   - The protocol should pull from the topUpSource to rebalance
+///     back to targetHealth, not just leave the position between
+///     minHealth and targetHealth.
+access(all)
+fun test_withdrawAndPull_rebalancesToTargetHealth() {
+    let initialPrice = 1.0
+    setMockOraclePrice(signer: PROTOCOL_ACCOUNT, forTokenIdentifier: FLOW_TOKEN_IDENTIFIER, price: initialPrice)
+
+    createAndStorePool(signer: PROTOCOL_ACCOUNT, defaultTokenIdentifier: MOET_TOKEN_IDENTIFIER, beFailed: false)
+    addSupportedTokenZeroRateCurve(
+        signer: PROTOCOL_ACCOUNT,
+        tokenTypeIdentifier: FLOW_TOKEN_IDENTIFIER,
+        collateralFactor: 0.8,
+        borrowFactor: 1.0,
+        depositRate: 1_000_000.0,
+        depositCapacityCap: 1_000_000.0
+    )
+
+    let user = Test.createAccount()
+    setupMoetVault(user, beFailed: false)
+    mintFlow(to: user, amount: 1_000.0)
+    grantBetaPoolParticipantAccess(PROTOCOL_ACCOUNT, user)
+
+    // Open position with auto-borrow: deposits 1000 FLOW, borrows ~615.38 MOET.
+    // Health starts at targetHealth (1.3).
+    let openRes = executeTransaction(
+        "../transactions/flow-alp/position/create_position.cdc",
+        [1_000.0, FLOW_VAULT_STORAGE_PATH, true],
+        user
+    )
+    Test.expect(openRes, Test.beSucceeded())
+
+    let healthBefore = getPositionHealth(pid: 0, beFailed: false)
+    let tolerance: UFix128 = 0.01
+    Test.assert(
+        healthBefore >= INT_TARGET_HEALTH - tolerance && healthBefore <= INT_TARGET_HEALTH + tolerance,
+        message: "Position should start at target health (~1.3) but was ".concat(healthBefore.toString())
+    )
+
+    // Withdraw 50 FLOW with pullFromTopUpSource=true.
+    // Without the fix: health drops below targetHealth but stays above minHealth,
+    // so pullFromTopUpSource is ignored and the position is NOT rebalanced.
+    // With the fix: the protocol should pull from topUpSource to restore targetHealth.
+    withdrawFromPosition(
+        signer: user,
+        positionId: 0,
+        tokenTypeIdentifier: FLOW_TOKEN_IDENTIFIER,
+        amount: 50.0,
+        pullFromTopUpSource: true
+    )
+
+    let healthAfter = getPositionHealth(pid: 0, beFailed: false)
+
+    // The position health should be restored to targetHealth (1.3),
+    // NOT left between minHealth and targetHealth.
+    Test.assert(
+        healthAfter >= INT_TARGET_HEALTH - tolerance,
+        message: "With pullFromTopUpSource=true, position should be rebalanced to target health (~1.3) but health was ".concat(healthAfter.toString())
+    )
+}