Skip to content

Commit 2749c7f

Browse files
m-Peterm-Peter
committed
Validate requests state in startProcessingBatch()
1 parent 0004434 commit 2749c7f

2 files changed

Lines changed: 153 additions & 28 deletions

File tree

‎solidity/src/FlowYieldVaultsRequests.sol‎

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,9 @@ contract FlowYieldVaultsRequests is ReentrancyGuard, Ownable2Step {
304304
/// @notice Processed request does not match the head of requestsQueue
305305
error RequestProcessOutOfOrder(uint256 expectedId, uint256 processedId);
306306

307+
/// @notice Request is not included in requestsQueue
308+
error RequestNotInQueue(uint256 requestId);
309+
307310
// ============================================
308311
// Events
309312
// ============================================
@@ -969,6 +972,7 @@ contract FlowYieldVaultsRequests is ReentrancyGuard, Ownable2Step {
969972
* Requests are classified as successful/rejected based on validation
970973
* logic that is performed on Cadence side, and not on the authorized
971974
* COA's discretion.
975+
* Both arrays containing the request IDs must be in ascending FIFO queue order.
972976
* Single-request processing is supported by passing one request id in
973977
* successfulRequestIds and an empty rejectedRequestIds array.
974978
* @param successfulRequestIds The request ids to start processing (PENDING -> PROCESSING)
@@ -978,39 +982,52 @@ contract FlowYieldVaultsRequests is ReentrancyGuard, Ownable2Step {
978982
uint256[] calldata successfulRequestIds,
979983
uint256[] calldata rejectedRequestIds
980984
) external onlyAuthorizedCOA nonReentrant {
981-
uint256 totalRequests = successfulRequestIds.length + rejectedRequestIds.length;
982-
983985
uint256 j = 0;
984986
uint256 k = 0;
987+
// Validate that the given IDs for successful/rejected requests,
988+
// are according to the FIFO queue order.
989+
uint256 totalRequests = successfulRequestIds.length + rejectedRequestIds.length;
985990
for (uint256 i = 0; i < totalRequests; i++) {
986-
uint256 requestId = _requestsQueue[_requestsQueueHead+i];
987-
uint256 reqId;
991+
uint256 requestId;
992+
uint256 reqId = _requestsQueue[_requestsQueueHead+i];
988993
if (j < successfulRequestIds.length) {
989-
reqId = successfulRequestIds[j];
990-
Request storage request = requests[reqId];
991-
992-
// === VALIDATION ===
993-
if (request.id != reqId) revert RequestNotFound();
994-
if (request.status != RequestStatus.PENDING)
995-
revert InvalidRequestState();
996-
994+
requestId = successfulRequestIds[j];
997995
if (reqId == requestId) {
998996
j++;
999997
continue;
1000998
}
1001999
}
10021000

10031001
if (k < rejectedRequestIds.length) {
1004-
reqId = rejectedRequestIds[k];
1002+
requestId = rejectedRequestIds[k];
10051003
if (reqId == requestId) {
10061004
k++;
10071005
continue;
10081006
}
10091007
}
10101008

1011-
revert RequestProcessOutOfOrder(requestId, reqId);
1009+
// If reqId is 0, it means we went over the boundaries of
1010+
// _requestsQueue. The ID we couldn't match is always `requestId`
1011+
// at this point because the loop only reaches here when neither
1012+
// branch advanced.
1013+
if (reqId == 0) revert RequestNotInQueue(requestId);
1014+
1015+
// === VALIDATION ===
1016+
Request storage request = requests[requestId];
1017+
if (request.status != RequestStatus.PENDING)
1018+
revert InvalidRequestState();
1019+
1020+
// requestId currently holds the last-assigned candidate
1021+
// (from rejectedIds if both branches ran).
1022+
// Prefer the successful candidate for a clearer error.
1023+
uint256 candidateReqId = (j < successfulRequestIds.length)
1024+
? successfulRequestIds[j]
1025+
: requestId;
1026+
revert RequestProcessOutOfOrder(reqId, candidateReqId);
10121027
}
10131028

1029+
// First the rejected request IDs are dropped, so successful
1030+
// request IDs are contiguous at the head before dequeue
10141031
// === REJECTED REQUESTS ===
10151032
_dropRequestsInternal(rejectedRequestIds);
10161033

@@ -1558,6 +1575,14 @@ contract FlowYieldVaultsRequests is ReentrancyGuard, Ownable2Step {
15581575
function _startProcessingInternal(uint256 requestId) internal {
15591576
Request storage request = requests[requestId];
15601577

1578+
// === VALIDATION ===
1579+
if (request.id != requestId) revert RequestNotFound();
1580+
if (request.status != RequestStatus.PENDING)
1581+
revert InvalidRequestState();
1582+
1583+
uint256 reqId = _dequeueRequest();
1584+
if (reqId != requestId) revert RequestProcessOutOfOrder(reqId, requestId);
1585+
15611586
// === TRANSITION TO PROCESSING ===
15621587
// This prevents cancellation and ensures atomicity with completeProcessing
15631588
request.status = RequestStatus.PROCESSING;
@@ -1615,8 +1640,6 @@ contract FlowYieldVaultsRequests is ReentrancyGuard, Ownable2Step {
16151640
userPendingRequestCount[request.user]--;
16161641
}
16171642
_removeUserPendingRequest(requestId);
1618-
uint256 reqId = _dequeueRequest();
1619-
if (reqId != requestId) revert RequestProcessOutOfOrder(reqId, requestId);
16201643

16211644
emit RequestProcessed(
16221645
requestId,

‎solidity/test/FlowYieldVaultsRequests.t.sol‎

Lines changed: 114 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -334,15 +334,30 @@ contract FlowYieldVaultsRequestsTest is Test {
334334
assertEq(uint8(req.status), uint8(FlowYieldVaultsRequests.RequestStatus.PROCESSING));
335335
}
336336

337-
function test_StartProcessingBatch_RevertNotPending() public {
337+
function test_StartProcessingBatch_RevertRequestNotInQueueWhenQueueEmpty() public {
338+
vm.startPrank(coa);
339+
vm.expectRevert(abi.encodeWithSelector(
340+
FlowYieldVaultsRequests.RequestNotInQueue.selector,
341+
1
342+
));
343+
_startProcessingBatch(1);
344+
vm.stopPrank();
345+
}
346+
347+
function test_StartProcessingBatch_RevertRequestNotInQueueWhenQueueNonEmpty() public {
338348
vm.prank(user);
339349
uint256 reqId = c.createYieldVault{value: 1 ether}(NATIVE_FLOW, 1 ether, VAULT_ID, STRATEGY_ID);
340350

341351
vm.startPrank(coa);
342-
_startProcessingBatch(reqId);
352+
uint256[] memory successfulRequestIds = new uint256[](5);
353+
successfulRequestIds[0] = reqId;
354+
successfulRequestIds[1] = reqId+2;
343355

344-
vm.expectRevert(FlowYieldVaultsRequests.InvalidRequestState.selector);
345-
_startProcessingBatch(reqId);
356+
vm.expectRevert(abi.encodeWithSelector(
357+
FlowYieldVaultsRequests.RequestNotInQueue.selector,
358+
reqId+2
359+
));
360+
c.startProcessingBatch(successfulRequestIds, new uint256[](0));
346361
vm.stopPrank();
347362
}
348363

@@ -355,6 +370,36 @@ contract FlowYieldVaultsRequestsTest is Test {
355370
_startProcessingBatch(reqId);
356371
}
357372

373+
function test_StartProcessingBatch_WithMoreRequestIdsThanQueued() public {
374+
vm.startPrank(user);
375+
uint256 req1 = c.createYieldVault{value: 1 ether}(NATIVE_FLOW, 1 ether, VAULT_ID, STRATEGY_ID);
376+
uint256 req2 = c.createYieldVault{value: 2 ether}(NATIVE_FLOW, 2 ether, VAULT_ID, STRATEGY_ID);
377+
uint256 req3 = c.createYieldVault{value: 3 ether}(NATIVE_FLOW, 3 ether, VAULT_ID, STRATEGY_ID);
378+
vm.stopPrank();
379+
380+
vm.prank(coa);
381+
uint256[] memory successfulRequestIds = new uint256[](5);
382+
successfulRequestIds[0] = req1;
383+
successfulRequestIds[1] = req2;
384+
successfulRequestIds[2] = req3;
385+
successfulRequestIds[3] = req3+5;
386+
successfulRequestIds[4] = req3+10;
387+
388+
vm.expectRevert(abi.encodeWithSelector(
389+
FlowYieldVaultsRequests.RequestNotInQueue.selector,
390+
req3+5
391+
));
392+
c.startProcessingBatch(successfulRequestIds, new uint256[](0));
393+
394+
(uint256[] memory requestIds, , , , , , , , , , ) = c.getPendingRequestsUnpacked(0, 0);
395+
assertEq(requestIds.length, 3);
396+
397+
for (uint256 i = 0; i < requestIds.length; i++) {
398+
FlowYieldVaultsRequests.Request memory req = c.getRequest(successfulRequestIds[i]);
399+
assertEq(uint8(req.status), uint8(FlowYieldVaultsRequests.RequestStatus.PENDING));
400+
}
401+
}
402+
358403
function test_StartProcessingBatch_RevertRequestProcessOutOfOrder() public {
359404
vm.startPrank(user);
360405
uint256 req1 = c.createYieldVault{value: 1 ether}(NATIVE_FLOW, 1 ether, VAULT_ID, STRATEGY_ID);
@@ -378,19 +423,76 @@ contract FlowYieldVaultsRequestsTest is Test {
378423
uint256 req3 = c.createYieldVault{value: 3 ether}(NATIVE_FLOW, 3 ether, VAULT_ID, STRATEGY_ID);
379424
uint256 req4 = c.createYieldVault{value: 4 ether}(NATIVE_FLOW, 4 ether, VAULT_ID, STRATEGY_ID);
380425
uint256 req5 = c.createYieldVault{value: 5 ether}(NATIVE_FLOW, 5 ether, VAULT_ID, STRATEGY_ID);
426+
uint256 req6 = c.createYieldVault{value: 6 ether}(NATIVE_FLOW, 6 ether, VAULT_ID, STRATEGY_ID);
381427
vm.stopPrank();
382428

383-
vm.prank(coa);
384-
// All 5 successful, transition to PROCESSING
385-
uint256[] memory successfulRequestIds = new uint256[](5);
429+
vm.startPrank(coa);
430+
// First 3 successful, transition to PROCESSING
431+
uint256[] memory successfulRequestIds = new uint256[](3);
386432
successfulRequestIds[0] = req1;
387433
successfulRequestIds[1] = req2;
388434
successfulRequestIds[2] = req3;
389-
successfulRequestIds[3] = req4;
390-
successfulRequestIds[4] = req5;
391435
c.startProcessingBatch(successfulRequestIds, new uint256[](0));
392436

393437
(uint256[] memory requestIds, , , , , , , , , , ) = c.getPendingRequestsUnpacked(0, 0);
438+
assertEq(requestIds.length, 3);
439+
440+
for (uint256 i = 0; i < successfulRequestIds.length; i++) {
441+
FlowYieldVaultsRequests.Request memory req = c.getRequest(successfulRequestIds[i]);
442+
assertEq(uint8(req.status), uint8(FlowYieldVaultsRequests.RequestStatus.PROCESSING));
443+
}
444+
445+
// Remaining 3 successful, transition to PROCESSING
446+
successfulRequestIds[0] = req4;
447+
successfulRequestIds[1] = req5;
448+
successfulRequestIds[2] = req6;
449+
c.startProcessingBatch(successfulRequestIds, new uint256[](0));
450+
451+
(requestIds, , , , , , , , , , ) = c.getPendingRequestsUnpacked(0, 0);
452+
assertEq(requestIds.length, 0);
453+
454+
for (uint256 i = 0; i < successfulRequestIds.length; i++) {
455+
FlowYieldVaultsRequests.Request memory req = c.getRequest(successfulRequestIds[i]);
456+
assertEq(uint8(req.status), uint8(FlowYieldVaultsRequests.RequestStatus.PROCESSING));
457+
}
458+
vm.stopPrank();
459+
}
460+
461+
function test_StartProcessingBatch_WithUserCancellationRace() public {
462+
vm.startPrank(user);
463+
uint256 req1 = c.createYieldVault{value: 1 ether}(NATIVE_FLOW, 1 ether, VAULT_ID, STRATEGY_ID);
464+
uint256 req2 = c.createYieldVault{value: 2 ether}(NATIVE_FLOW, 2 ether, VAULT_ID, STRATEGY_ID);
465+
uint256 req3 = c.createYieldVault{value: 3 ether}(NATIVE_FLOW, 3 ether, VAULT_ID, STRATEGY_ID);
466+
c.cancelRequest(req2);
467+
vm.stopPrank();
468+
469+
vm.startPrank(coa);
470+
// First and third requests, transition to PROCESSING
471+
uint256[] memory successfulRequestIds = new uint256[](2);
472+
successfulRequestIds[0] = req1;
473+
successfulRequestIds[1] = req3;
474+
// Second request, transition to FAILED
475+
uint256[] memory rejectedRequestIds = new uint256[](1);
476+
rejectedRequestIds[0] = req2;
477+
478+
vm.expectRevert(abi.encodeWithSelector(
479+
FlowYieldVaultsRequests.RequestNotInQueue.selector,
480+
req2
481+
));
482+
c.startProcessingBatch(successfulRequestIds, rejectedRequestIds);
483+
484+
(uint256[] memory requestIds, , , , , , , , , , ) = c.getPendingRequestsUnpacked(0, 0);
485+
assertEq(requestIds.length, 2);
486+
487+
for (uint256 i = 0; i < successfulRequestIds.length; i++) {
488+
FlowYieldVaultsRequests.Request memory req = c.getRequest(successfulRequestIds[i]);
489+
assertEq(uint8(req.status), uint8(FlowYieldVaultsRequests.RequestStatus.PENDING));
490+
}
491+
492+
// Try again, without the user-cancelled request
493+
c.startProcessingBatch(successfulRequestIds, new uint256[](0));
494+
495+
(requestIds, , , , , , , , , , ) = c.getPendingRequestsUnpacked(0, 0);
394496
assertEq(requestIds.length, 0);
395497

396498
for (uint256 i = 0; i < successfulRequestIds.length; i++) {
@@ -483,8 +585,8 @@ contract FlowYieldVaultsRequestsTest is Test {
483585

484586
vm.expectRevert(abi.encodeWithSelector(
485587
FlowYieldVaultsRequests.RequestProcessOutOfOrder.selector,
486-
req2, // the expected requestId, in the queue head, after rejections have already been removed
487-
req3 // the provided requestId
588+
req2, // queue[head+1] — the current queue position that doesn't match either input array
589+
req4 // the first provided requestId, from successfulRequestIds
488590
));
489591
c.startProcessingBatch(successfulRequestIds, rejectedRequestIds);
490592

@@ -525,7 +627,7 @@ contract FlowYieldVaultsRequestsTest is Test {
525627
vm.expectRevert(abi.encodeWithSelector(
526628
FlowYieldVaultsRequests.RequestProcessOutOfOrder.selector,
527629
req1, // the expected requestId, in the queue head
528-
req3 // the provided requestId
630+
req4 // the first provided requestId, from successfulRequestIds
529631
));
530632
c.startProcessingBatch(successfulRequestIds, rejectedRequestIds);
531633

0 commit comments

Comments
 (0)