fix: harden supervisor recovery and stuck scan#207
fix: harden supervisor recovery and stuck scan#207liobrasil wants to merge 4 commits intoholyfuchs/supervisor-fixfrom
Conversation
| return true | ||
| } | ||
|
|
||
| if status == FlowTransactionScheduler.Status.Executed |
There was a problem hiding this comment.
This is the same issue we faced in onflow/FlowYieldVaultsEVM#70
The problem with the fix is that if the transaction panics, lastRebalanceTimestamp is not updated. This makes the rebalancer permanantely stuck because it's Executed and the lastRebalanceTimestamp was never updated.
You might want to consider a grace period based fix.
| /// A transaction is considered active when it is: | ||
| /// - still `Scheduled`, or | ||
| /// - already marked `Executed` by FlowTransactionScheduler, but the AutoBalancer has not | ||
| /// yet advanced its last rebalance timestamp past that transaction's scheduled time. | ||
| /// | ||
| /// The second case matters because FlowTransactionScheduler flips status to `Executed` | ||
| /// before the handler actually runs. Without treating that in-flight window as active, | ||
| /// the Supervisor can falsely classify healthy vaults as stuck and recover them twice. | ||
| /// |
There was a problem hiding this comment.
Could you explain this?
I am not sure I understand.
The status can be Executed even though it is not executed?
| yieldVaultID: uniqueID.id, | ||
| handlerCap: handlerCap, | ||
| scheduleCap: scheduleCap, | ||
| participatesInStuckScan: recurringConfig != nil |
There was a problem hiding this comment.
The contract mentions its:
A registry of all yield vault IDs that participate in scheduled rebalancing
Would we ever have an instance where this is not the case?
To me it seems the better approach to ensure that SchedulerRegistry only has Vaults that are getting scheduled and we shouldn't add the ones which aren't.
3 participants
Summary
Verification