Merge pull request #175 from onflow/claude/fix-flowalp-beta-capability-XU9nk

Remove EPosition auth requirement from FlowALP beta cap transactions

Author: jordanschalm

cadence/tests/cap_test.cdc

@@ -22,15 +22,6 @@ import "test_helpers.cdc"
 //                              Published via the FIXED publish_beta_cap.cdc.
 //                              Cap stored at FlowALPv0.PoolCapStoragePath.
 //
-//   ePositionUser            — Capability<auth(EPosition) &Pool>
-//                              EPosition-only capability; can perform pool-level position
-//                              ops on any position by ID. No EParticipant.
-//                              Cap stored at FlowALPv0.PoolCapStoragePath.
-//
-//   eParticipantPositionUser — Capability<auth(EParticipant, EPosition) &Pool> over-grant
-//                              Current (unfixed) beta cap — grants EPosition unnecessarily.
-//                              Cap stored at FlowALPv0.PoolCapStoragePath.
-//
 //   eRebalanceUser           — Capability<auth(ERebalance) &Pool>
 //                              Narrowly-scoped cap for rebalancer contracts.
 //                              Cap stored at FlowALPv0.PoolCapStoragePath.
@@ -52,7 +43,7 @@ import "test_helpers.cdc"
 // =============================================================================
 
 
-// Position created for PROTOCOL_ACCOUNT in setup — used as target for EPosition tests.
+// Position created for PROTOCOL_ACCOUNT in setup — used as target for ERebalance tests.
 access(all) var setupPid: UInt64 = 0
 access(all) var ePositionAdminPid: UInt64 = 0
 
@@ -61,8 +52,6 @@ access(all) var snapshot: UInt64 = 0
 // Role accounts
 access(all) var userWithoutCap = Test.createAccount()
 access(all) var eParticipantUser = Test.createAccount()
-access(all) var ePositionUser = Test.createAccount()
-access(all) var eParticipantPositionUser = Test.createAccount()
 access(all) var eRebalanceUser = Test.createAccount()
 access(all) var ePositionAdminUser = Test.createAccount()
 access(all) var eGovernanceUser = Test.createAccount()
@@ -71,7 +60,7 @@ access(all) var eGovernanceUser = Test.createAccount()
 /// Used in negative tests to verify governance methods are inaccessible to them.
 access(all)
 fun getNonGovernanceUsers(): [Test.TestAccount] {
-    return [eParticipantUser, ePositionUser, eParticipantPositionUser, eRebalanceUser, ePositionAdminUser]
+    return [eParticipantUser, eRebalanceUser, ePositionAdminUser]
 }
 
 access(all)
@@ -150,28 +139,6 @@ fun setup() {
         Test.beSucceeded()
     )
 
-    // ─────────────────────────────────────────────────────────────────────────
-    // EPosition user — EPosition-ONLY capability (no EParticipant)
-    // ─────────────────────────────────────────────────────────────────────────
-    setupMoetVault(ePositionUser, beFailed: false)
-    mintMoet(signer: PROTOCOL_ACCOUNT, to: ePositionUser.address, amount: 100.0, beFailed: false)
-    Test.expect(
-        _execute2Signers(
-            "../tests/transactions/flow-alp/setup/grant_eposition_cap.cdc",
-            [],
-            PROTOCOL_ACCOUNT,
-            ePositionUser
-        ),
-        Test.beSucceeded()
-    )
-
-    // ─────────────────────────────────────────────────────────────────────────
-    // EParticipantPosition user — EParticipant+EPosition capability (current over-grant)
-    // ─────────────────────────────────────────────────────────────────────────
-    setupMoetVault(eParticipantPositionUser, beFailed: false)
-    mintMoet(signer: PROTOCOL_ACCOUNT, to: eParticipantPositionUser.address, amount: 100.0, beFailed: false)
-    grantBetaPoolParticipantAccess(PROTOCOL_ACCOUNT, eParticipantPositionUser)
-
     // ─────────────────────────────────────────────────────────────────────────
     // ERebalance user — ERebalance-only capability (rebalancer simulation)
     // ─────────────────────────────────────────────────────────────────────────
@@ -277,222 +244,6 @@ fun testEParticipant_CreateAndDeposit() {
     Test.assertEqual(6.0, creditBalance)
 }
 
-// =============================================================================
-// EParticipant+EPosition — over-grant (current beta cap via publish_beta_cap.cdc)
-// =============================================================================
-//
-// Actor: eParticipantPositionUser — Capability<auth(EParticipant, EPosition) &Pool>
-//        Issued by publish_beta_cap.cdc and stored at FlowALPv0.PoolCapStoragePath.
-//        This is the CURRENT (unfixed) beta cap. EPosition is NOT needed for normal
-//        user actions; its presence lets this actor perform pool-level position ops
-//        on ANY position, including positions owned by other accounts.
-//
-// Matrix rows: createPosition (EParticipant), depositToPosition (EParticipant),
-//              withdraw [OVERGRANT], withdrawAndPull [OVERGRANT], depositAndPush [OVERGRANT],
-//              lockPosition [OVERGRANT], unlockPosition [OVERGRANT], rebalancePosition [OVERGRANT],
-//              rebalance (Position) [OVERGRANT — same entry point as rebalancePosition]
-//
-// The [OVERGRANT] rows confirm the security issue: a normal beta user can operate on
-// positions they do not own (setupPid is owned by PROTOCOL_ACCOUNT).
-
-/// Over-granted beta cap still allows EParticipant operations (createPosition, depositToPosition).
-access(all)
-fun testEParticipantPosition_CreateAndDeposit() {
-    safeReset()
-
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eparticipant/create_and_deposit_via_cap.cdc",
-        [],
-        eParticipantPositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-
-    // Verify position was created and funded: create_and_deposit_via_cap.cdc deposits
-    // 5.0 MOET (createPosition) + 1.0 MOET (depositToPosition) = 6.0 MOET credit.
-    let newPid = getLastPositionId()
-    let creditBalance = getCreditBalanceForType(
-        details: getPositionDetails(pid: newPid, beFailed: false),
-        vaultType: Type<@MOET.Vault>()
-    )
-    Test.assertEqual(6.0, creditBalance)
-}
-
-/// Over-granted beta cap allows Pool.withdraw on ANY position — including
-/// setupPid owned by PROTOCOL_ACCOUNT.
-access(all)
-fun testEParticipantPosition_WithdrawAnyPosition() {
-    safeReset()
-
-    let balanceBefore = getBalance(address: eParticipantPositionUser.address, vaultPublicPath: MOET.VaultPublicPath)!
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/withdraw_any.cdc",
-        [setupPid, 1.0],
-        eParticipantPositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-    let balanceAfter = getBalance(address: eParticipantPositionUser.address, vaultPublicPath: MOET.VaultPublicPath)!
-    Test.assertEqual(balanceAfter, balanceBefore + 1.0)
-}
-
-/// Over-granted beta cap allows Pool.withdrawAndPull on ANY position — including
-/// positions owned by other accounts.
-access(all)
-fun testEParticipantPosition_WithdrawAndPullAnyPosition() {
-    safeReset()
-
-    let balanceBefore = getBalance(address: eParticipantPositionUser.address, vaultPublicPath: MOET.VaultPublicPath)!
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/withdraw_and_pull_any.cdc",
-        [setupPid, 1.0],
-        eParticipantPositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-    let balanceAfter = getBalance(address: eParticipantPositionUser.address, vaultPublicPath: MOET.VaultPublicPath)!
-    Test.assertEqual(balanceAfter, balanceBefore + 1.0)
-}
-
-/// Over-granted beta cap allows Pool.depositAndPush on ANY position — including
-/// positions owned by other accounts.
-access(all)
-fun testEParticipantPosition_DepositAndPushAnyPosition() {
-    safeReset()
-
-    let creditBefore = getCreditBalanceForType(
-        details: getPositionDetails(pid: setupPid, beFailed: false),
-        vaultType: Type<@MOET.Vault>()
-    )
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/deposit_and_push_any.cdc",
-        [setupPid, 1.0],
-        eParticipantPositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-    let creditAfter = getCreditBalanceForType(
-        details: getPositionDetails(pid: setupPid, beFailed: false),
-        vaultType: Type<@MOET.Vault>()
-    )
-    Test.assertEqual(creditBefore + 1.0, creditAfter)
-}
-
-/// Over-granted beta cap allows Pool.lockPosition and Pool.unlockPosition on ANY position —
-/// including positions owned by other accounts.
-access(all)
-fun testEParticipantPosition_LockUnlockAnyPosition() {
-    safeReset()
-
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/lock_any.cdc",
-        [setupPid],
-        eParticipantPositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-}
-
-/// Over-granted beta cap allows Pool.rebalancePosition on any position.
-access(all)
-fun testEParticipantPosition_RebalancePosition() {
-    safeReset()
-
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/rebalance_position_via_cap.cdc",
-        [setupPid, true],
-        eParticipantPositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-}
-
-// =============================================================================
-// EPosition — narrowly-scoped EPosition-only Pool capability
-// =============================================================================
-//
-// Actor: ePositionUser — Capability<auth(EPosition) &Pool>
-// Matrix rows: withdraw, withdrawAndPull, depositAndPush, lockPosition, unlockPosition,
-//              rebalancePosition
-
-/// EPosition cap allows Pool.withdraw on ANY position by ID — including
-/// setupPid owned by PROTOCOL_ACCOUNT.
-access(all)
-fun testEPosition_WithdrawAnyPosition() {
-    safeReset()
-
-    let balanceBefore = getBalance(address: ePositionUser.address, vaultPublicPath: MOET.VaultPublicPath)!
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/withdraw_any.cdc",
-        [setupPid, 1.0],
-        ePositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-    let balanceAfter = getBalance(address: ePositionUser.address, vaultPublicPath: MOET.VaultPublicPath)!
-    Test.assertEqual(balanceAfter, balanceBefore + 1.0)
-}
-
-/// EPosition cap allows Pool.withdrawAndPull on ANY position — including positions
-/// owned by other accounts.
-access(all)
-fun testEPosition_WithdrawAndPullAnyPosition() {
-    safeReset()
-
-    let balanceBefore = getBalance(address: ePositionUser.address, vaultPublicPath: MOET.VaultPublicPath)!
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/withdraw_and_pull_any.cdc",
-        [setupPid, 1.0],
-        ePositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-    let balanceAfter = getBalance(address: ePositionUser.address, vaultPublicPath: MOET.VaultPublicPath)!
-    Test.assertEqual(balanceAfter, balanceBefore + 1.0)
-}
-
-/// EPosition cap allows Pool.depositAndPush on ANY position — including positions
-/// owned by other accounts.
-access(all)
-fun testEPosition_DepositAndPushAnyPosition() {
-    safeReset()
-
-    let creditBefore = getCreditBalanceForType(
-        details: getPositionDetails(pid: setupPid, beFailed: false),
-        vaultType: Type<@MOET.Vault>()
-    )
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/deposit_and_push_any.cdc",
-        [setupPid, 1.0],
-        ePositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-    let creditAfter = getCreditBalanceForType(
-        details: getPositionDetails(pid: setupPid, beFailed: false),
-        vaultType: Type<@MOET.Vault>()
-    )
-    Test.assertEqual(creditBefore + 1.0, creditAfter)
-}
-
-/// EPosition cap allows Pool.lockPosition and Pool.unlockPosition on ANY position —
-/// including positions owned by other accounts.
-access(all)
-fun testEPosition_LockUnlockAnyPosition() {
-    safeReset()
-
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/lock_any.cdc",
-        [setupPid],
-        ePositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-}
-
-/// EPosition cap allows Pool.rebalancePosition.
-access(all)
-fun testEPosition_RebalancePosition() {
-    safeReset()
-
-    let result = _executeTransaction(
-        "../tests/transactions/flow-alp/eposition/rebalance_position_via_cap.cdc",
-        [setupPid, true],
-        ePositionUser
-    )
-    Test.expect(result, Test.beSucceeded())
-}
-
 // =============================================================================
 // ERebalance — narrowly-scoped rebalancer capability
 // =============================================================================

cadence/tests/contracts/AdversarialReentrancyConnectors.cdc

@@ -4,6 +4,7 @@ import "FungibleTokenMetadataViews"
 import "DeFiActionsUtils"
 import "DeFiActions"
 import "FlowALPv0"
+import "FlowALPPositionResources"
 import "FlowALPModels"
 
 import "MOET"
@@ -107,17 +108,18 @@ access(all) contract AdversarialReentrancyConnectors {
     }
 
     access(all) resource LiveData {
-        /// Optional: Pool capability for recursive withdrawAndPull call
-        access(all) var recursivePool: Capability<auth(FlowALPModels.EPosition) &FlowALPv0.Pool>?
-        /// Optional: Position ID for recursive withdrawAndPull call
+        /// Capability to the attacker's PositionManager for recursive withdrawal
+        access(all) var positionManagerCap: Capability<auth(FungibleToken.Withdraw, FlowALPModels.EPositionAdmin) &FlowALPPositionResources.PositionManager>?
+        /// Position ID for recursive withdrawal
         access(all) var recursivePositionID: UInt64?
 
-        init() { self.recursivePositionID = nil; self.recursivePool = nil }
-        access(all) fun setRecursivePool(_ pool: Capability<auth(FlowALPModels.EPosition) &FlowALPv0.Pool>) {
-            self.recursivePool = pool
-        }
-        access(all) fun setRecursivePositionID(_ positionID: UInt64) {
-            self.recursivePositionID = positionID
+        init() { self.recursivePositionID = nil; self.positionManagerCap = nil }
+        access(all) fun setRecursivePosition(
+            managerCap: Capability<auth(FungibleToken.Withdraw, FlowALPModels.EPositionAdmin) &FlowALPPositionResources.PositionManager>,
+            pid: UInt64
+        ) {
+            self.positionManagerCap = managerCap
+            self.recursivePositionID = pid
         }
     }
     access(all) fun createLiveData(): @LiveData {
@@ -203,27 +205,18 @@ access(all) contract AdversarialReentrancyConnectors {
         access(FungibleToken.Withdraw) fun withdrawAvailable(maxAmount: UFix64): @{FungibleToken.Vault} {
             // If recursive withdrawAndPull is configured, call it first
             log("VaultSource.withdrawAvailable called with maxAmount: \(maxAmount)")
-            log("=====Recursive pool: \(self.liveDataCap.check())")
+            log("=====Recursive position manager: \(self.liveDataCap.check())")
             let liveData = self.liveDataCap.borrow() ?? panic("cant borrow LiveData")
-            let poolRef = liveData.recursivePool!.borrow() ?? panic("cant borrow Recursive pool is nil")
-            // Call withdrawAndPull on the position
-            let recursiveVault <- poolRef.withdrawAndPull(
-                pid: liveData.recursivePositionID!,
-                // type: Type<@MOET.Vault>(),
+            let manager = liveData.positionManagerCap!.borrow() ?? panic("cant borrow PositionManager")
+            let position = manager.borrowAuthorizedPosition(pid: liveData.recursivePositionID!)
+            // Attempt reentrant withdrawal via Position (should fail due to position lock)
+            let recursiveVault <- position.withdraw(
                 type: Type<@FlowToken.Vault>(),
-                // type: tokenType,
-                amount: 900.0,
-                pullFromTopUpSource: false
+                amount: 900.0
             )
-            log("Recursive withdrawAndPull returned vault with balance: \(recursiveVault.balance)")
-            // If we got funds from the recursive call, return them
-            if recursiveVault.balance > 0.0 {
-                return <-recursiveVault
-            }
-            // Otherwise, destroy the empty vault and continue with normal withdrawal
+            log("Recursive withdraw succeeded with balance: \(recursiveVault.balance) (should not reach here)")
             destroy recursiveVault
 
-            
             // Normal vault withdrawal
             let available = self.minimumAvailable()
             if !self.withdrawVault.check() || available == 0.0 || maxAmount == 0.0 {

cadence/tests/paid_auto_balance_test.cdc

@@ -316,6 +316,15 @@ access(all) fun test_supervisor_executed() {
 /// fixReschedule(uuid:) force-unwrapped borrowRebalancer(uuid)! which panicked on a stale UUID,
 /// reverting the whole executeTransaction and blocking recovery for all other rebalancers.
 access(all) fun test_supervisor_stale_uuid_does_not_panic() {
+    // Let the initial cron tick fire first (supervisor set is empty, so it does nothing
+    // except emit Executed). This avoids a race where the cron fires during the add/delete
+    // transactions below before the stale state is set up.
+    Test.moveTime(by: 100.0)
+    Test.commitBlock()
+
+    let initialExecutedEvts = Test.eventsOfType(Type<FlowALPSupervisorv1.Executed>())
+    Test.assert(initialExecutedEvts.length >= 1, message: "Initial cron tick should have fired")
+
     // Get the UUID of the paid rebalancer created during setup.
     let createdEvts = Test.eventsOfType(Type<FlowALPRebalancerv1.CreatedRebalancer>())
     Test.assertEqual(1, createdEvts.length)
@@ -328,14 +337,14 @@ access(all) fun test_supervisor_stale_uuid_does_not_panic() {
     // stale UUID in the Supervisor's paidRebalancers set, simulating the FLO-27 bug scenario.
     deletePaidRebalancer(signer: userAccount, paidRebalancerStoragePath: paidRebalancerStoragePath)
 
-    // Advance time to trigger the Supervisor's scheduled tick.
+    // Advance time to trigger the next Supervisor tick.
     Test.moveTime(by: 60.0 * 60.0)
     Test.commitBlock()
 
     // The Supervisor must have executed without panicking. If fixReschedule force-unwrapped
     // the missing rebalancer the entire transaction would revert and Executed would not be emitted.
     let executedEvts = Test.eventsOfType(Type<FlowALPSupervisorv1.Executed>())
-    Test.assert(executedEvts.length >= 1, message: "Supervisor should have executed at least 1 time")
+    Test.assert(executedEvts.length >= 2, message: "Supervisor should have executed at least 2 times (initial + stale prune)")
 
     // The stale UUID must have been pruned from the Supervisor's set.
     let removedEvts = Test.eventsOfType(Type<FlowALPSupervisorv1.RemovedPaidRebalancer>())