Merge pull request #297 from onflow/illia-malachyn/get-positions-should-not-panic

Get positions script should not panic

Author: illia-malachyn

FlowActions

@@ -1927,13 +1927,29 @@ access(all) contract FlowALPv0 {
             )
 
             return PositionDetails(
+                id: pid,
                 balances: balances,
                 poolDefaultToken: self.defaultToken,
                 defaultTokenAvailableBalance: defaultTokenAvailable,
                 health: health
             )
         }
 
+        /// Returns the details of a given position, or nil if the position does not exist.
+        /// This is the non-panicking variant of getPositionDetails.
+        access(all) fun tryGetPositionDetails(pid: UInt64): PositionDetails? {
+            if self.debugLogging {
+                log("    [CONTRACT] tryGetPositionDetails(pid: \(pid))")
+            }
+
+            if self._tryBorrowPosition(pid: pid) == nil {
+                return nil
+            }
+
+            return self.getPositionDetails(pid: pid)
+        }
+
+
         /// Any external party can perform a manual liquidation on a position under the following circumstances:
         /// - the position has health < 1
         /// - the liquidation price offered is better than what is available on a DEX
@@ -4079,9 +4095,14 @@ access(all) contract FlowALPv0 {
             }
         }
 
-        /// Returns an authorized reference to the requested InternalPosition or `nil` if the position does not exist
-        access(self) view fun _borrowPosition(pid: UInt64): auth(EImplementation) &InternalPosition {
+        /// Returns an authorized reference to the requested InternalPosition, or nil if it does not exist.
+        access(self) view fun _tryBorrowPosition(pid: UInt64): (auth(EImplementation) &InternalPosition)? {
             return &self.positions[pid] as auth(EImplementation) &InternalPosition?
+        }
+
+        /// Returns an authorized reference to the requested InternalPosition or panics if the position does not exist.
+        access(self) view fun _borrowPosition(pid: UInt64): auth(EImplementation) &InternalPosition {
+            return self._tryBorrowPosition(pid: pid)
                 ?? panic("Invalid position ID \(pid) - could not find an InternalPosition with the requested ID in the Pool")
         }
 
@@ -4762,6 +4783,9 @@ access(all) contract FlowALPv0 {
     /// This structure is NOT used internally.
     access(all) struct PositionDetails {
 
+        /// The unique identifier of the position
+        access(all) let id: UInt64
+
         /// Balance details about each Vault Type deposited to the related Position
         access(all) let balances: [PositionBalance]
 
@@ -4775,11 +4799,13 @@ access(all) contract FlowALPv0 {
         access(all) let health: UFix128
 
         init(
+            id: UInt64,
             balances: [PositionBalance],
             poolDefaultToken: Type,
             defaultTokenAvailableBalance: UFix64,
             health: UFix128
         ) {
+            self.id = id
             self.balances = balances
             self.poolDefaultToken = poolDefaultToken
             self.defaultTokenAvailableBalance = defaultTokenAvailableBalance

cadence/contracts/FlowALPv0.cdc

@@ -1,4 +1,6 @@
 // Returns the details of multiple positions by their IDs.
+// Positions that no longer exist (e.g., closed between fetching IDs and executing this script)
+// are silently skipped.
 import "FlowALPv0"
 
 access(all) fun main(positionIDs: [UInt64]): [FlowALPv0.PositionDetails] {
@@ -9,7 +11,9 @@ access(all) fun main(positionIDs: [UInt64]): [FlowALPv0.PositionDetails] {
 
     let details: [FlowALPv0.PositionDetails] = []
     for id in positionIDs {
-        details.append(pool.getPositionDetails(pid: id))
+        if let detail = pool.tryGetPositionDetails(pid: id) {
+            details.append(detail)
+        }
     }
     return details
 }

cadence/scripts/flow-alp/get_positions_by_ids.cdc

@@ -0,0 +1,11 @@
+// Returns the details of a position by its ID, or nil if the position does not exist.
+import "FlowALPv0"
+
+access(all) fun main(positionID: UInt64): FlowALPv0.PositionDetails? {
+    let protocolAddress = Type<@FlowALPv0.Pool>().address!
+    let account = getAccount(protocolAddress)
+    let pool = account.capabilities.borrow<&FlowALPv0.Pool>(FlowALPv0.PoolPublicPath)
+        ?? panic("Could not find Pool at path \(FlowALPv0.PoolPublicPath)")
+
+    return pool.tryGetPositionDetails(pid: positionID)
+}

cadence/scripts/flow-alp/try_get_position_details.cdc

@@ -65,4 +65,20 @@ fun test_getPositionsByIDs() {
     let singleDetails = getPositionsByIDs(positionIDs: [UInt64(0)])
     Test.assertEqual(1, singleDetails.length)
     Test.assertEqual(details0.health, singleDetails[0].health)
+
+    // --- Closed positions are silently skipped ---
+    // Close position 1, then request both IDs — should only return position 0
+    closePosition(user: user, positionID: 1)
+    let afterClose = getPositionsByIDs(positionIDs: [UInt64(0), UInt64(1)])
+    Test.assertEqual(1, afterClose.length)
+    Test.assertEqual(details0.health, afterClose[0].health)
+
+    // --- All IDs closed/invalid returns empty array ---
+    closePosition(user: user, positionID: 0)
+    let allClosed = getPositionsByIDs(positionIDs: [UInt64(0), UInt64(1)])
+    Test.assertEqual(0, allClosed.length)
+
+    // --- Non-existent IDs are skipped ---
+    let nonExistent = getPositionsByIDs(positionIDs: [UInt64(999), UInt64(1000)])
+    Test.assertEqual(0, nonExistent.length)
 }

cadence/tests/get_positions_by_ids_test.cdc

@@ -810,6 +810,16 @@ fun getPositionsByIDs(positionIDs: [UInt64]): [FlowALPv0.PositionDetails] {
     return res.returnValue as! [FlowALPv0.PositionDetails]
 }
 
+access(all)
+fun tryGetPositionDetails(pid: UInt64): FlowALPv0.PositionDetails? {
+    let res = _executeScript(
+        "../scripts/flow-alp/try_get_position_details.cdc",
+        [pid]
+    )
+    Test.expect(res, Test.beSucceeded())
+    return res.returnValue as! FlowALPv0.PositionDetails?
+}
+
 access(all)
 fun closePosition(user: Test.TestAccount, positionID: UInt64) {
     let res = _executeTransaction(

cadence/tests/test_helpers.cdc

@@ -0,0 +1,66 @@
+import Test
+import BlockchainHelpers
+
+import "MOET"
+import "FlowALPv0"
+import "test_helpers.cdc"
+
+// -----------------------------------------------------------------------------
+// tryGetPositionDetails Test
+//
+// Verifies that Pool.tryGetPositionDetails() returns position details for
+// existing positions and nil for non-existent or closed positions.
+// -----------------------------------------------------------------------------
+
+access(all)
+fun setup() {
+    deployContracts()
+}
+
+// =============================================================================
+// Test: tryGetPositionDetails returns details for open positions and nil otherwise
+// =============================================================================
+access(all)
+fun test_tryGetPositionDetails() {
+    // --- Setup ---
+    setMockOraclePrice(signer: PROTOCOL_ACCOUNT, forTokenIdentifier: FLOW_TOKEN_IDENTIFIER, price: 1.0)
+
+    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: 10_000.0)
+
+    // --- Non-existent position returns nil ---
+    let nonExistent = tryGetPositionDetails(pid: 0)
+    Test.assertEqual(nil, nonExistent)
+
+    // --- Open a position ---
+    createPosition(signer: user, amount: 100.0, vaultStoragePath: FLOW_VAULT_STORAGE_PATH, pushToDrawDownSink: false)
+
+    // --- Existing position returns details ---
+    let details = tryGetPositionDetails(pid: 0)
+    Test.assert(details != nil, message: "Expected non-nil details for open position")
+
+    // --- Result matches getPositionDetails ---
+    let expected = getPositionDetails(pid: 0, beFailed: false)
+    Test.assertEqual(expected.health, details!.health)
+    Test.assertEqual(expected.balances.length, details!.balances.length)
+
+    // --- Still nil for non-existent ID ---
+    let stillNil = tryGetPositionDetails(pid: 999)
+    Test.assertEqual(nil, stillNil)
+
+    // --- Close the position, should return nil ---
+    closePosition(user: user, positionID: 0)
+    let afterClose = tryGetPositionDetails(pid: 0)
+    Test.assertEqual(nil, afterClose)
+}