FLOW-2 Panicking Autobalancer May Cause a Dos for the Supervisor Recovery Loop

[vishalchangrani]

• Needs confirmation/further investigation
• We did think about this issue earlier and separated out rebalances

[holyfuchs]

The issue is valid, if any AutoBalancer panics (rather than returning an error), the entire transaction reverts, preventing
recovery of all other pending vaults.

However this is limited towards that block and a call to FlowYieldVaultsSchedulerRegistry.unregister removes the vault from the pending queue, fixing the issue for future runs.
ScheduleNextRebalance is already implemented to fail gracefully.

I wouldn't see how its possible to add a retry counter easily, and I don't think the additional overhead and complexity is worth it.

[vishalchangrani]

@Kay-Zee @nialexsan -do you agree with @holyfuchs conclusion? If yes, then we can close this issue.

[liobrasil]

A panicking AutoBalancer doesn't just block other pending vaults in that batch — it can kill the Supervisor's own self-rescheduling loop entirely.
Here's why: in Supervisor.executeTransaction(), the self-reschedule happens last (Step 3), after the pending vault recovery loop (Steps 1–2). If anything in Steps 1–2 panics the entire transaction reverts. That means:

• Every dequeue/enqueue that succeeded earlier in the loop is rolled back
• Step 3 never runs, so the Supervisor doesn't schedule its next execution

The result: one broken AutoBalancer silently stops the entire automated recovery mechanism. No future Supervisor runs happen.

@nvdtf curious to get your perspective here — you've looked into scheduling transaction resilience patterns and liveness guarantees, any thoughts on how to best address this?

[liobrasil]

In executeTransaction, the main failure risk is getRegisteredYieldVaultIDs(), calling getRegisteredYieldVaultIDs() allocates an array of all registered vault IDs upfront. For a large registry, this could exceed gas/computation limits and revert the transaction.

[holyfuchs]

Yeah this could revert the transaction but if it reverts because the array is to large rescheduling won't help.
This is a design flaw of the whole rebalancing logic.

[liobrasil]

@nvdtf and @holyfuchs you're the more knowledgeable about best practices and architecture of transaction scheduling. We should inspire from your work to propose something robust here (ie onflow/FlowYieldVaultsEVM#44 and onflow/FlowALP#131 )

[nvdtf]

In the EVM worker PR I tried to build around the fact that the worker might panic and the scheduler recovers it. One of the design assumptions is that we're passing 1 work to each worker, so if it reverts we know what request was reverted and we can mark it as failed in the scheduler (no observed side effects + no Scheduled status for the tx = panic).
In this situation, it seems more complex as we're trying to handle multiple work per handler run and if one of them panics, all of them revert. I was also trying to make the worker in my design accept a batch of requests. The idea was if it panics, the scheduler can then divide the work in two and run two workers to find out which request group panicked (or go stratight to 1-by-1 execution to find out the panic). Divide and continue until a single request is reverted. Of course this is theoretical and haven't been implemented (I assumed 1 work per worker for now). I'm not sure if this is an overkill in this situation or not. I might have to spend more time to familiarize myself with the problem here.

[holyfuchs]

This function and everything it calls are written so nothing can panic.
Failures are handled as errors and the loop continues, so one bad vault cannot DoS recovery of the rest.

• getStuckScanCandidates: View; returns a slice; slice(from:upTo:) is bounds-safe.
• isStuckYieldVault: Returns Bool only; uses optional borrow and returns false on nil; when nextExpected is nil returns true (stuck) without force unwrap.
• enqueuePending / dequeuePending: Dict add/remove only; no assertions or unwraps.
• getPendingYieldVaultIDsPaginated: View; slice on keys and bounds-safe slice; no panic.
• getScheduleCap: View; returns optional; caller checks nil and check() before use.
• scheduleNextRebalance: Returns optional String error; This method is written to fail as gracefully as possible, reporting any failures to schedule the next execution to the as an event.
• scheduleNextRecurringExecution: uses nil checks and emits SupervisorRescheduleFailed; no panic on this path.

[vishalchangrani]

no change needed for this issue as the Autobalancer will not panic.