-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Severity: Low
Files Affected
solidity/src/FlowYieldVaultsRequests.sol
Description
Requests are tracked in the global pendingRequestIds array, and lifecycle transitions (cancelRequest(), dropRequests(), and completeProcessing()) remove a request from this queue via _removePendingRequest(requestId), which shifts elements to preserve FIFO ordering.
However, startProcessing() and completeProcessing() do not enforce that the processed requestId matches the head of pendingRequestIds (or otherwise respects submission order), meaning FIFO is not a contract-level guarantee and can be bypassed (intentionally or accidentally) by the authorized COA.
At the same time, _removePendingRequest() still pays the cost of preserving FIFO order on-chain via O(n) shifting, which can become increasingly expensive as pendingRequestIds grows and, in worst cases, risk hitting block gas limits and reverting during finalization.
This creates a "cost without enforcement" situation where the system bears O(n) cleanup overhead even if ordering is effectively treated as an off-chain policy.
Example (out-of-order processing bypasses FIFO but still pays O(n))
- Global queue is
pendingRequestIds = [req1 (older), req2 (newer), ...]. - Authorized COA processes out of order: calls
startProcessing(req2)and thencompleteProcessing(req2, ...)even thoughpendingRequestIds[0] != req2. - FIFO fairness is bypassed on-chain, yet
_removePendingRequest(req2)still shifts the global array to preserve FIFO order (O(n)). - Result: the system pays O(n) queue-maintenance cost without FIFO being a contract-level guarantee; with large queues this can make finalization expensive or even revert.
Recommendation
Consider either enforcing FIFO on-chain (e.g., require the head request to be processed) using a scalable queue structure, or treating ordering as an off-chain COA policy while switching on-chain removal to an O(1) pending-set pattern.
Parent Issue: #15