diff --git a/cadence/contracts/FlowALPRebalancerPaidv1.cdc b/cadence/contracts/FlowALPRebalancerPaidv1.cdc index 144962bf..d2a26faf 100644 --- a/cadence/contracts/FlowALPRebalancerPaidv1.cdc +++ b/cadence/contracts/FlowALPRebalancerPaidv1.cdc @@ -108,17 +108,21 @@ access(all) contract FlowALPRebalancerPaidv1 { /// Idempotent: if no next run is scheduled, try to schedule it (e.g. after a transient failure). access(all) fun fixReschedule() { - FlowALPRebalancerPaidv1.fixReschedule(positionID: self.positionID) + let _ = FlowALPRebalancerPaidv1.fixReschedule(positionID: self.positionID) } } /// Idempotent: for the given paid rebalancer, if there is no scheduled transaction, schedule the next run. /// Callable by anyone (e.g. the Supervisor or the RebalancerPaid owner). + /// Returns true if the rebalancer was found and processed, false if the UUID is stale (rebalancer no longer exists). access(all) fun fixReschedule( positionID: UInt64, - ) { - let rebalancer = FlowALPRebalancerPaidv1.borrowRebalancer(positionID: positionID)! - rebalancer.fixReschedule() + ): Bool { + if let rebalancer = FlowALPRebalancerPaidv1.borrowRebalancer(positionID: positionID) { + rebalancer.fixReschedule() + return true + } + return false } /// Storage path where a user would store their RebalancerPaid for the given position (convention for discovery). diff --git a/cadence/contracts/FlowALPSupervisorv1.cdc b/cadence/contracts/FlowALPSupervisorv1.cdc index 18c7a740..92468569 100644 --- a/cadence/contracts/FlowALPSupervisorv1.cdc +++ b/cadence/contracts/FlowALPSupervisorv1.cdc @@ -45,11 +45,15 @@ access(all) contract FlowALPSupervisorv1 { } /// Scheduler callback: on each tick, call fixReschedule on every registered paid rebalancer, - /// recovering any that failed to schedule their next transaction. + /// recovering any that failed to schedule their next transaction. Stale UUIDs (rebalancer + /// deleted without being removed from this set) are pruned automatically. access(FlowTransactionScheduler.Execute) fun executeTransaction(id: UInt64, data: AnyStruct?) { emit Executed(id: id) for positionID in self.paidRebalancers.keys { - FlowALPRebalancerPaidv1.fixReschedule(positionID: positionID) + let found = FlowALPRebalancerPaidv1.fixReschedule(positionID: positionID) + if !found { + let _ = self.removePaidRebalancer(positionID: positionID) + } } } } diff --git a/cadence/tests/paid_auto_balance_test.cdc b/cadence/tests/paid_auto_balance_test.cdc index bcd2d1d1..855b70b1 100644 --- a/cadence/tests/paid_auto_balance_test.cdc +++ b/cadence/tests/paid_auto_balance_test.cdc @@ -273,6 +273,45 @@ access(all) fun test_supervisor_executed() { Test.assertEqual(2, evts.length) } +/// Regression test for FLO-27: if a paid rebalancer is deleted without removing its UUID from +/// the Supervisor's set, the next Supervisor tick must NOT panic. Before the fix, +/// 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() { + // Get the UUID of the paid rebalancer created during setup. + let createdEvts = Test.eventsOfType(Type()) + Test.assertEqual(1, createdEvts.length) + let created = createdEvts[0] as! FlowALPRebalancerv1.CreatedRebalancer + + // Register the UUID with the Supervisor so it will call fixReschedule on it each tick. + addPaidRebalancerToSupervisor(signer: userAccount, positionID: created.positionID, supervisorStoragePath: supervisorStoragePath) + + // Delete the paid rebalancer WITHOUT removing its UUID from the Supervisor — this leaves a + // 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. + 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()) + Test.assert(executedEvts.length >= 1, message: "Supervisor should have executed at least 1 time") + + // The stale UUID must have been pruned from the Supervisor's set. + let removedEvts = Test.eventsOfType(Type()) + Test.assertEqual(1, removedEvts.length) + let removed = removedEvts[0] as! FlowALPSupervisorv1.RemovedPaidRebalancer + Test.assertEqual(created.positionID, removed.positionID) + + // A second tick should not emit another RemovedPaidRebalancer — the UUID was already cleaned up. + Test.moveTime(by: 60.0 * 60.0) + Test.commitBlock() + let removedEvts2 = Test.eventsOfType(Type()) + Test.assertEqual(1, removedEvts2.length) +} + access(all) fun test_supervisor() { Test.moveTime(by: 100.0) Test.commitBlock()