diff --git a/cadence/contracts/FlowALPRebalancerPaidv1.cdc b/cadence/contracts/FlowALPRebalancerPaidv1.cdc index d95155eb..d0ffdc8f 100644 --- a/cadence/contracts/FlowALPRebalancerPaidv1.cdc +++ b/cadence/contracts/FlowALPRebalancerPaidv1.cdc @@ -104,17 +104,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(uuid: self.rebalancerUUID) + let _ = FlowALPRebalancerPaidv1.fixReschedule(uuid: self.rebalancerUUID) } } /// 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( uuid: UInt64, - ) { - let rebalancer = FlowALPRebalancerPaidv1.borrowRebalancer(uuid: uuid)! - rebalancer.fixReschedule() + ): Bool { + if let rebalancer = FlowALPRebalancerPaidv1.borrowRebalancer(uuid: uuid) { + rebalancer.fixReschedule() + return true + } + return false } /// Storage path where a user would store their RebalancerPaid for the given uuid (convention for discovery). diff --git a/cadence/contracts/FlowALPSupervisorv1.cdc b/cadence/contracts/FlowALPSupervisorv1.cdc index 9f450e37..63a3d82d 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 rebalancerUUID in self.paidRebalancers.keys { - FlowALPRebalancerPaidv1.fixReschedule(uuid: rebalancerUUID) + let found = FlowALPRebalancerPaidv1.fixReschedule(uuid: rebalancerUUID) + if !found { + let _ = self.removePaidRebalancer(uuid: rebalancerUUID) + } } } } diff --git a/cadence/tests/paid_auto_balance_test.cdc b/cadence/tests/paid_auto_balance_test.cdc index 989aa44a..0de64e1e 100644 --- a/cadence/tests/paid_auto_balance_test.cdc +++ b/cadence/tests/paid_auto_balance_test.cdc @@ -47,14 +47,14 @@ access(all) fun setup() { let evts = Test.eventsOfType(Type()) let paidRebalancerUUID = evts[0] as! FlowALPRebalancerv1.CreatedRebalancer createSupervisor( - signer: userAccount, + signer: userAccount, cronExpression: "0 * * * *", cronHandlerStoragePath: cronHandlerStoragePath, keeperExecutionEffort: 1000, executorExecutionEffort: 1000, supervisorStoragePath: supervisorStoragePath ) - + snapshot = getCurrentBlockHeight() } @@ -254,6 +254,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, uuid: created.uuid, 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.uuid, removed.uuid) + + // 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()