Merge pull request #198 from onflow/lionel/fix-close-position-stale-async-queue

Fix stale async queue pid after closePosition + add regression test

Author: nialexsan

cadence/contracts/FlowALPv0.cdc

@@ -3345,7 +3345,8 @@ access(all) contract FlowALPv0 {
             }
             destroy vaults
 
-            // Step 11: Destroy InternalPosition and unlock
+            // Step 11: Remove stale queue entry, then destroy InternalPosition and unlock
+            self._removePositionFromUpdateQueue(pid: pid)
             destroy self.positions.remove(key: pid)!
             self._unlockPosition(pid)
 
@@ -3828,6 +3829,12 @@ access(all) contract FlowALPv0 {
             var processed: UInt64 = 0
             while self.positionsNeedingUpdates.length > 0 && processed < self.positionsProcessedPerCallback {
                 let pid = self.positionsNeedingUpdates.removeFirst()
+                if self.positions[pid] == nil {
+                    // Stale queue entry: position may have been closed and removed from self.positions.
+                    // Skip to keep async updates progressing for the remaining queue entries.
+                    processed = processed + 1
+                    continue
+                }
                 self.asyncUpdatePosition(pid: pid)
                 self._queuePositionForUpdateIfNecessary(pid: pid)
                 processed = processed + 1
@@ -3945,6 +3952,21 @@ access(all) contract FlowALPv0 {
             }
         }
 
+        /// Removes a position from the async update queue.
+        /// This is needed when closing a position to prevent stale queue entries.
+        access(self) fun _removePositionFromUpdateQueue(pid: UInt64) {
+            // Keep this operation linear-time:
+            // find first matching pid, then remove once while preserving queue order.
+            var i = 0
+            while i < self.positionsNeedingUpdates.length {
+                if self.positionsNeedingUpdates[i] == pid {
+                    self.positionsNeedingUpdates.remove(at: i)
+                    return
+                }
+                i = i + 1
+            }
+        }
+
         /// Returns a position's BalanceSheet containing its effective collateral and debt as well as its current health
         /// TODO(jord): in all cases callers already are calling _borrowPosition, more efficient to pass in PositionView?
         access(self) fun _getUpdatedBalanceSheet(pid: UInt64): BalanceSheet {

cadence/tests/close_position_async_queue_stale_test.cdc

@@ -0,0 +1,85 @@
+import Test
+import BlockchainHelpers
+
+import "MOET"
+import "FlowALPv0"
+import "test_helpers.cdc"
+
+access(all)
+fun setup() {
+    deployContracts()
+    createAndStorePool(signer: PROTOCOL_ACCOUNT, defaultTokenIdentifier: MOET_TOKEN_IDENTIFIER, beFailed: false)
+}
+
+access(all)
+fun test_closePosition_clearsQueuedAsyncUpdateEntry() {
+    // Regression target:
+    // A position could remain in `positionsNeedingUpdates` after being closed.
+    // Then `asyncUpdate()` would pop that stale pid and panic when trying to
+    // update a position that no longer exists.
+    //
+    // This test recreates that exact sequence and asserts async callbacks
+    // succeed after close.
+    setMockOraclePrice(signer: PROTOCOL_ACCOUNT, forTokenIdentifier: FLOW_TOKEN_IDENTIFIER, price: 1.0)
+
+    // Keep deposit capacity low so new deposits can overflow active capacity and
+    // be queued for async processing (which queues the position id as well).
+    addSupportedTokenZeroRateCurve(
+        signer: PROTOCOL_ACCOUNT,
+        tokenTypeIdentifier: FLOW_TOKEN_IDENTIFIER,
+        collateralFactor: 0.8,
+        borrowFactor: 1.0,
+        depositRate: 100.0,
+        depositCapacityCap: 100.0
+    )
+
+    let user = Test.createAccount()
+    setupMoetVault(user, beFailed: false)
+    mintFlow(to: user, amount: 1_000.0)
+    grantBetaPoolParticipantAccess(PROTOCOL_ACCOUNT, user)
+
+    // Step 1: Open a position with a small initial deposit.
+    // This consumes part of the token's active capacity.
+    let openRes = _executeTransaction(
+        "../transactions/flow-alp/position/create_position.cdc",
+        [50.0, FLOW_VAULT_STORAGE_PATH, false],
+        user
+    )
+    Test.expect(openRes, Test.beSucceeded())
+
+    // Step 2: Deposit an amount that exceeds remaining active capacity.
+    // The overflow is queued, and the position is put in the async update queue.
+    let depositRes = _executeTransaction(
+        "./transactions/position/deposit_to_position_by_id.cdc",
+        [UInt64(0), 150.0, FLOW_VAULT_STORAGE_PATH, false],
+        user
+    )
+    Test.expect(depositRes, Test.beSucceeded())
+
+    // Step 3: Close the position before async callbacks drain the queue.
+    // This is the key condition that previously left a stale pid behind.
+    let closeRes = _executeTransaction(
+        "../transactions/flow-alp/position/repay_and_close_position.cdc",
+        [UInt64(0)],
+        user
+    )
+    Test.expect(closeRes, Test.beSucceeded())
+
+    // Step 4 (regression assertion): run async update callback.
+    // Before the fix, this could panic when touching a removed position.
+    // After the fix, stale entries are removed/skipped and callback succeeds.
+    let asyncRes = _executeTransaction(
+        "./transactions/flow-alp/pool-management/async_update_all.cdc",
+        [],
+        PROTOCOL_ACCOUNT
+    )
+    Test.expect(asyncRes, Test.beSucceeded())
+
+    // Step 5: run one more callback to prove queue state remains clean.
+    let asyncRes2 = _executeTransaction(
+        "./transactions/flow-alp/pool-management/async_update_all.cdc",
+        [],
+        PROTOCOL_ACCOUNT
+    )
+    Test.expect(asyncRes2, Test.beSucceeded())
+}

cadence/tests/transactions/flow-alp/pool-management/async_update_all.cdc

@@ -0,0 +1,14 @@
+import "FlowALPv0"
+
+transaction {
+    let pool: auth(FlowALPv0.EImplementation) &FlowALPv0.Pool
+
+    prepare(signer: auth(BorrowValue) &Account) {
+        self.pool = signer.storage.borrow<auth(FlowALPv0.EImplementation) &FlowALPv0.Pool>(from: FlowALPv0.PoolStoragePath)
+            ?? panic("Could not borrow Pool")
+    }
+
+    execute {
+        self.pool.asyncUpdate()
+    }
+}