FLO-5: Consistent behaviour in withdrawAndPull/depositAndPush

cadence/contracts/FlowALPModels.cdc

@@ -394,6 +394,7 @@ access(all) contract FlowALPModels {
         }
 
         /// Returns the true balance for the given internal (scaled) balance, accounting for accrued interest.
+        /// TODO: This could be view, but Cadence seems to interpret constructing a Balance as modifying state
         access(all) fun trueBalance(balance: InternalBalance): Balance {
             let scaled = balance.getScaledBalance()
             let interestIndex = scaled.direction == BalanceDirection.Credit
@@ -2247,14 +2248,14 @@ access(all) contract FlowALPModels {
         /// Borrows an authorized internal position reference.
         access(EPosition) view fun borrowPosition(pid: UInt64): auth(EImplementation) &{InternalPosition}
 
-        /// Deposits funds to a position and optionally pushes excess to draw-down sink.
+        /// Deposits funds to a position. If `pushToDrawDownSink` is true, always rebalances to targetHealth.
         access(EPosition) fun depositAndPush(
             pid: UInt64,
             from: @{FungibleToken.Vault},
             pushToDrawDownSink: Bool
         )
 
-        /// Withdraws funds from a position and optionally pulls deficit from top-up source.
+        /// Withdraws funds from a position. If `pullFromTopUpSource` is true, always rebalances to targetHealth.
         access(EPosition) fun withdrawAndPull(
             pid: UInt64,
             type: Type,

cadence/contracts/FlowALPPositionResources.cdc

@@ -120,8 +120,8 @@ access(all) contract FlowALPPositionResources {
             )
         }
 
-        /// Deposits funds to the Position enabling the caller to configure whether excess value
-        /// should be pushed to the drawDownSink if the deposit puts the Position above its maximum health
+        /// Deposits funds to the Position. If `pushToDrawDownSink` is true, a rebalance is always
+        /// triggered at deposit time, pushing excess value to the drawDownSink to restore targetHealth.
         /// NOTE: Anyone is allowed to deposit to any position.
         access(all) fun depositAndPush(
             from: @{FungibleToken.Vault},
@@ -145,8 +145,8 @@ access(all) contract FlowALPPositionResources {
             )
         }
 
-        /// Withdraws funds from the Position enabling the caller to configure whether insufficient value
-        /// should be pulled from the topUpSource if the withdrawal puts the Position below its minimum health
+        /// Withdraws funds from the Position. If `pullFromTopUpSource` is true, a rebalance is always
+        /// triggered at withdrawal time, pulling value from the topUpSource to restore targetHealth.
         access(FungibleToken.Withdraw) fun withdrawAndPull(
             type: Type,
             amount: UFix64,

cadence/contracts/FlowALPv0.cdc

@@ -723,6 +723,20 @@ access(all) contract FlowALPv0 {
             )
         }
 
+        // 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))
+                )
+            )
+        }
+
         /// Computes the deposit amount needed to bring effective values to the target health.
         /// Accounts for whether the deposit reduces debt or adds collateral based on the
         /// position's current balance direction for the deposit token.
@@ -960,6 +974,7 @@ access(all) contract FlowALPv0 {
                 !self.isPaused(): "Withdrawal, deposits, and liquidations are paused by governance"
                 self.state.getTokenState(funds.getType()) != nil:
                     "Invalid token type \(funds.getType().identifier) - not supported by this Pool"
+                funds.balance > 0.0: "Cannot create a position without deposit"
                 self.positionSatisfiesMinimumBalance(type: funds.getType(), balance: UFix128(funds.balance)):
                     "Insufficient funds to create position. Minimum deposit of \(funds.getType().identifier) is \(self.state.getTokenState(funds.getType())!.getMinimumTokenBalancePerPosition())"
                 // TODO(jord): Sink/source should be valid
@@ -1014,12 +1029,13 @@ access(all) contract FlowALPv0 {
         /// This function is used to validate that positions maintain a minimum balance to prevent
         /// dust positions and ensure operational efficiency. The minimum requirement applies to
         /// credit (deposit) balances and is enforced at position creation and during withdrawals.
+        /// Zero balances are always allowed.
         ///
         /// @param type: The token type to check (e.g., Type<@FlowToken.Vault>())
-        /// @param balance: The balance amount to validate
+        /// @param balance: The (true) balance amount to validate
         /// @return true if the balance meets or exceeds the minimum requirement, false otherwise
         access(self) view fun positionSatisfiesMinimumBalance(type: Type, balance: UFix128): Bool {
-            return balance >= UFix128(self.state.getTokenState(type)!.getMinimumTokenBalancePerPosition())
+            return balance == 0.0 || balance >= UFix128(self.state.getTokenState(type)!.getMinimumTokenBalancePerPosition())
         }
 
         /// Allows anyone to deposit funds into any position.
@@ -1134,8 +1150,8 @@ access(all) contract FlowALPv0 {
         }
 
         /// Deposits the provided funds to the specified position with the configurable `pushToDrawDownSink` option.
-        /// If `pushToDrawDownSink` is true, excess value putting the position above its max health
-        /// is pushed to the position's configured `drawDownSink`.
+        /// If `pushToDrawDownSink` is true, a rebalance is always triggered at deposit time,
+        /// pushing excess value to the position's configured `drawDownSink` to restore targetHealth.
         access(FlowALPModels.EPosition) fun depositAndPush(
             pid: UInt64,
             from: @{FungibleToken.Vault},
@@ -1188,8 +1204,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, a rebalance is always triggered at withdrawal time,
+        /// pulling value from the position's configured `topUpSource` to restore targetHealth.
         /// TODO(jord): ~150-line function - consider refactoring.
         access(FlowALPModels.EPosition) fun withdrawAndPull(
             pid: UInt64,
@@ -1219,119 +1235,73 @@ access(all) contract FlowALPv0 {
             // Get a reference to the user's position and global token state for the affected token.
             let position = self._borrowPosition(pid: pid)
             let tokenState = self._borrowUpdatedTokenState(type: type)
+            let tokenSnapshot = self.buildTokenSnapshot(type: type)
 
-            // Global interest indices are updated via tokenState() helper
-
-            // Preflight to see if the funds are available
-            let topUpSource = position.borrowTopUpSource()
-            let topUpType = topUpSource?.getSourceType() ?? self.state.getDefaultToken()
-
-            let requiredDeposit = self.fundsRequiredForTargetHealthAfterWithdrawing(
-                pid: pid,
-                depositType: topUpType,
-                targetHealth: position.getMinHealth(),
-                withdrawType: type,
-                withdrawAmount: amount
-            )
-
-            var canWithdraw = false
-
-            if requiredDeposit == 0.0 {
-                // We can service this withdrawal without any top up
-                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(
+            if pullFromTopUpSource {
+                if let topUpSource = position.borrowTopUpSource() {
+                    // NOTE: getSourceType can lie, but we are resilient to this because:
+                    // (1) we check the vault type returned by the source matches its purported type, below
+                    // (2) computing target health deposit below will panic if the purported type is not supported
+                    let purportedTopUpType = topUpSource.getSourceType()
+                    let targetHealthDeposit = self.fundsRequiredForTargetHealthAfterWithdrawing(
                         pid: pid,
-                        depositType: topUpType,
+                        depositType: purportedTopUpType,
                         targetHealth: position.getTargetHealth(),
                         withdrawType: type,
                         withdrawAmount: amount
                     )
-
-                    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 {
-                        // We can service this withdrawal if we deposit funds from our top up source
-                        self._depositEffectsOnly(
-                            pid: pid,
-                            from: <-pulledVault
-                        )
-                        canWithdraw = true
-                    } else {
-                        // We can't get the funds required to service this withdrawal, so we need to redeposit what we got
-                        self._depositEffectsOnly(
-                            pid: pid,
-                            from: <-pulledVault
-                        )
-                    }
-                }
-            }
-
-            if !canWithdraw {
-                // Log detailed information about the failed withdrawal (only if debugging enabled)
-                if self.config.isDebugLogging() {
-                    let availableBalance = self.availableBalance(pid: pid, type: type, pullFromTopUpSource: false)
-                    log("    [CONTRACT] WITHDRAWAL FAILED:")
-                    log("    [CONTRACT] Position ID: \(pid)")
-                    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] Pull from topUpSource: \(pullFromTopUpSource)")
+
+                    let pulledVault <- topUpSource.withdrawAvailable(maxAmount: targetHealthDeposit)
+                    assert(pulledVault.getType() == purportedTopUpType, message: "topUpSource returned unexpected token type")
+                    self._depositEffectsOnly(
+                        pid: pid,
+                        from: <-pulledVault
+                    )
                 }
-                // We can't service this withdrawal, so we just abort
-                panic("Cannot withdraw \(amount) of \(type.identifier) from position ID \(pid) - Insufficient funds for withdrawal")
             }
 
-            // If this position doesn't currently have an entry for this token, create one.
             if position.getBalance(type) == nil {
                 position.setBalance(type, FlowALPModels.InternalBalance(
                     direction: FlowALPModels.BalanceDirection.Credit,
                     scaledBalance: 0.0
                 ))
             }
 
-            let reserveVault = self.state.borrowReserve(type)!
-
             // Reflect the withdrawal in the position's balance
-            let uintAmount = UFix128(amount)
             position.borrowBalance(type)!.recordWithdrawal(
-                amount: uintAmount,
+                amount: UFix128(amount),
                 tokenState: tokenState
             )
-            // Attempt to pull additional collateral from the top-up source (if configured)
-            // to keep the position above minHealth after the withdrawal.
-            // Regardless of whether a top-up occurs, the position must be healthy post-withdrawal.
-            let postHealth = self.positionHealth(pid: pid)
-            assert(
-                postHealth >= 1.0,
-                message: "Post-withdrawal position health (\(postHealth)) is unhealthy"
-            )
 
-            // Ensure that the remaining balance meets the minimum requirement (or is zero)
-            // Building the position view does require copying the balances, so it's less efficient than accessing the balance directly.
-            // Since most positions will have a single token type, we're okay with this for now.
-            let positionView = self.buildPositionView(pid: pid)
-            let remainingBalance = positionView.trueBalance(ofToken: type)
+            // Safety checks!
+            self._assertPositionSatisfiesMinimumBalance(type: type, position: position, tokenSnapshot: tokenSnapshot)
 
-            // This is applied to both credit and debit balances, with the main goal being to avoid dust positions.
-            assert(
-                remainingBalance == 0.0 || self.positionSatisfiesMinimumBalance(type: type, balance: remainingBalance),
-                message: "Withdrawal would leave position below minimum balance requirement of \(self.state.getTokenState(type)!.getMinimumTokenBalancePerPosition()). Remaining balance would be \(remainingBalance)."
-            )
+            let postHealth = self.positionHealth(pid: pid)
+            if postHealth < position.getMinHealth() {
+                if self.config.isDebugLogging() {
+                    let topUpType = position.borrowTopUpSource()?.getSourceType() ?? self.state.getDefaultToken()
+                    let minHealthDeposit = self.fundsRequiredForTargetHealthAfterWithdrawing(
+                        pid: pid,
+                        depositType: topUpType,
+                        targetHealth: position.getMinHealth(),
+                        withdrawType: type,
+                        withdrawAmount: amount
+                    )
+                    let availableBalance = self.availableBalance(pid: pid, type: type, pullFromTopUpSource: false)
+                    log("    [CONTRACT] WITHDRAWAL FAILED:")
+                    log("    [CONTRACT] Position ID: \(pid)")
+                    log("    [CONTRACT] Token type: \(type.identifier)")
+                    log("    [CONTRACT] Requested amount: \(amount)")
+                    log("    [CONTRACT] Available balance (without topUp): \(availableBalance)")
+                    log("    [CONTRACT] Required deposit for minHealth: \(minHealthDeposit)")
+                    log("    [CONTRACT] Pull from topUpSource: \(pullFromTopUpSource)")
+                }
+                // We can't service this withdrawal, so we just abort
+                panic("Cannot withdraw \(amount) of \(type.identifier) from position ID \(pid) - Insufficient funds for withdrawal")
+            }
 
-            // Queue for update if necessary
             self._queuePositionForUpdateIfNecessary(pid: pid)
-
+            let reserveVault = self.state.borrowReserve(type)!
             let withdrawn <- reserveVault.withdraw(amount: amount)
 
             FlowALPEvents.emitWithdrawn(
@@ -1346,6 +1316,21 @@ access(all) contract FlowALPv0 {
             return <- withdrawn
         }
 
+        /// Asserts that the remaining balance of `type` meets the minimum per-position requirement
+        /// (or is exactly zero). Panics with a descriptive message if not satisfied.
+        access(self) fun _assertPositionSatisfiesMinimumBalance(
+            type: Type,
+            position: &{FlowALPModels.InternalPosition},
+            tokenSnapshot: FlowALPModels.TokenSnapshot
+        ) {
+            if let bal = position.getBalance(type) {
+                let trueBal = tokenSnapshot.trueBalance(balance: bal)
+                assert(
+                    self.positionSatisfiesMinimumBalance(type: type, balance: trueBal.quantity),
+                    message: "Withdrawal would leave position below minimum balance requirement of \(self.state.getTokenState(type)!.getMinimumTokenBalancePerPosition()). Remaining balance would be \(trueBal.quantity).")
+            }
+        }
+
         ///////////////////////
         // POOL MANAGEMENT
         ///////////////////////

cadence/tests/rebalance_undercollateralised_test.cdc

@@ -175,4 +175,4 @@ fun testRebalanceUndercollateralised_InsufficientTopUpSource() {
     )
     Test.expect(rebalanceRes, Test.beFailed())
     Test.assertError(rebalanceRes, errorMessage: "topUpSource insufficient to save position from liquidation")
-}
+}

cadence/tests/transactions/position-manager/create_position_no_connectors.cdc

@@ -0,0 +1,52 @@
+import "FungibleToken"
+
+import "FlowALPv0"
+import "FlowALPPositionResources"
+import "FlowALPModels"
+
+/// TEST TRANSACTION - DO NOT USE IN PRODUCTION
+///
+/// Creates a position without a topUpSource or drawDownSink.
+///
+transaction(amount: UFix64, vaultStoragePath: StoragePath) {
+
+    let collateral: @{FungibleToken.Vault}
+    let positionManager: auth(FlowALPModels.EPositionAdmin) &FlowALPPositionResources.PositionManager
+    let poolCap: Capability<auth(FlowALPModels.EParticipant, FlowALPModels.EPosition) &FlowALPv0.Pool>
+    let signerAccount: auth(Storage) &Account
+
+    prepare(signer: auth(BorrowValue, Storage, Capabilities) &Account) {
+        self.signerAccount = signer
+
+        let collateralSource = signer.storage.borrow<auth(FungibleToken.Withdraw) &{FungibleToken.Vault}>(from: vaultStoragePath)
+            ?? panic("Could not borrow reference to Vault from \(vaultStoragePath)")
+        self.collateral <- collateralSource.withdraw(amount: amount)
+
+        if signer.storage.borrow<&FlowALPPositionResources.PositionManager>(from: FlowALPv0.PositionStoragePath) == nil {
+            let manager <- FlowALPv0.createPositionManager()
+            signer.storage.save(<-manager, to: FlowALPv0.PositionStoragePath)
+            let readCap = signer.capabilities.storage.issue<&FlowALPPositionResources.PositionManager>(FlowALPv0.PositionStoragePath)
+            signer.capabilities.publish(readCap, at: FlowALPv0.PositionPublicPath)
+        }
+        self.positionManager = signer.storage.borrow<auth(FlowALPModels.EPositionAdmin) &FlowALPPositionResources.PositionManager>(from: FlowALPv0.PositionStoragePath)
+            ?? panic("PositionManager not found")
+
+        self.poolCap = signer.storage.load<Capability<auth(FlowALPModels.EParticipant, FlowALPModels.EPosition) &FlowALPv0.Pool>>(
+            from: FlowALPv0.PoolCapStoragePath
+        ) ?? panic("Could not load Pool capability from storage")
+    }
+
+    execute {
+        let poolRef = self.poolCap.borrow() ?? panic("Could not borrow Pool capability")
+
+        let position <- poolRef.createPosition(
+            funds: <-self.collateral,
+            issuanceSink: nil,
+            repaymentSource: nil,
+            pushToDrawDownSink: false
+        )
+
+        self.positionManager.addPosition(position: <-position)
+        self.signerAccount.storage.save(self.poolCap, to: FlowALPv0.PoolCapStoragePath)
+    }
+}