From ca3ae1bc1ba3761d05f5809b3cd6ebe2106c7a62 Mon Sep 17 00:00:00 2001 From: William Morriss Date: Wed, 28 Jan 2026 13:35:08 -0600 Subject: [PATCH 1/5] fix #266 --- src/RateChangeQueue.sol | 30 +++++++++++++++++++----------- test/RateChangeQueue.t.sol | 9 ++++++--- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/RateChangeQueue.sol b/src/RateChangeQueue.sol index d8a3c8e3..1edc53c6 100644 --- a/src/RateChangeQueue.sol +++ b/src/RateChangeQueue.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.27; library RateChangeQueue { + error EmptyQueue(); + struct RateChange { // The payment rate to apply uint256 rate; @@ -18,13 +20,15 @@ library RateChangeQueue { queue.changes.push(RateChange(rate, untilEpoch)); } - function dequeue(Queue storage queue) internal returns (RateChange memory) { + function dequeue(Queue storage queue) internal returns (RateChange memory change) { RateChange[] storage c = queue.changes; - require(queue.head < c.length, "Queue is empty"); - RateChange memory change = c[queue.head]; - delete c[queue.head]; + require(queue.head < c.length, EmptyQueue()); + unchecked { + change = c[queue.head]; + delete c[queue.head]; + } - if (isEmpty(queue)) { + if (queue.head + 1 == c.length) { queue.head = 0; // The array is already empty, waste no time zeroing it. assembly { @@ -37,14 +41,18 @@ library RateChangeQueue { return change; } - function peek(Queue storage queue) internal view returns (RateChange memory) { - require(queue.head < queue.changes.length, "Queue is empty"); - return queue.changes[queue.head]; + function peek(Queue storage queue) internal view returns (RateChange memory change) { + require(queue.head < queue.changes.length, EmptyQueue()); + unchecked { + change = queue.changes[queue.head]; + } } - function peekTail(Queue storage queue) internal view returns (RateChange memory) { - require(queue.head < queue.changes.length, "Queue is empty"); - return queue.changes[queue.changes.length - 1]; + function peekTail(Queue storage queue) internal view returns (RateChange memory change) { + require(queue.head < queue.changes.length, EmptyQueue()); + unchecked { + change = queue.changes[queue.changes.length - 1]; + } } function isEmpty(Queue storage queue) internal view returns (bool) { diff --git a/test/RateChangeQueue.t.sol b/test/RateChangeQueue.t.sol index 34c2adbb..674fbc51 100644 --- a/test/RateChangeQueue.t.sol +++ b/test/RateChangeQueue.t.sol @@ -117,6 +117,9 @@ contract RateChangeQueueTest is Test { // Queue should now be empty assertTrue(RateChangeQueue.isEmpty(queue())); assertEq(RateChangeQueue.size(queue()), 0); + + // Empty queue should have reset to head = 0 + assertEq(queue().head, 0); } /// forge-config: default.allow_internal_expect_revert = true @@ -124,7 +127,7 @@ contract RateChangeQueueTest is Test { createEmptyQueue(); // Test dequeue on empty queue - vm.expectRevert("Queue is empty"); + vm.expectRevert(abi.encodeWithSelector(RateChangeQueue.EmptyQueue.selector)); RateChangeQueue.dequeue(queue()); } @@ -133,7 +136,7 @@ contract RateChangeQueueTest is Test { createEmptyQueue(); // Test peek on empty queue - vm.expectRevert("Queue is empty"); + vm.expectRevert(abi.encodeWithSelector(RateChangeQueue.EmptyQueue.selector)); RateChangeQueue.peek(queue()); } @@ -142,7 +145,7 @@ contract RateChangeQueueTest is Test { createEmptyQueue(); // Test peekTail on empty queue - vm.expectRevert("Queue is empty"); + vm.expectRevert(abi.encodeWithSelector(RateChangeQueue.EmptyQueue.selector)); RateChangeQueue.peekTail(queue()); } From 068d5f4e25fee6a0b5f1a3defbb590a6ab0d7e6a Mon Sep 17 00:00:00 2001 From: William Morriss Date: Wed, 28 Jan 2026 13:43:25 -0600 Subject: [PATCH 2/5] can rm this return --- src/RateChangeQueue.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/RateChangeQueue.sol b/src/RateChangeQueue.sol index 1edc53c6..950b1fcc 100644 --- a/src/RateChangeQueue.sol +++ b/src/RateChangeQueue.sol @@ -37,8 +37,6 @@ library RateChangeQueue { } else { queue.head++; } - - return change; } function peek(Queue storage queue) internal view returns (RateChange memory change) { From a8051f0e6a332a39b2fe0554896e0b373851c8b5 Mon Sep 17 00:00:00 2001 From: William Morriss Date: Wed, 28 Jan 2026 15:17:18 -0600 Subject: [PATCH 3/5] zero out queue when zeroing out rail --- src/FilecoinPayV1.sol | 1 + src/RateChangeQueue.sol | 19 ++++++++++--------- test/RateChangeQueue.t.sol | 2 ++ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/FilecoinPayV1.sol b/src/FilecoinPayV1.sol index 0d1dc78b..5f94c433 100644 --- a/src/FilecoinPayV1.sol +++ b/src/FilecoinPayV1.sol @@ -1596,6 +1596,7 @@ contract FilecoinPayV1 is ReentrancyGuard { rail.settledUpTo = 0; rail.endEpoch = 0; rail.commissionRateBps = 0; + rail.rateChangeQueue.clearEmpty(); } function updateOperatorRateUsage(OperatorApproval storage approval, uint256 oldRate, uint256 newRate) internal { diff --git a/src/RateChangeQueue.sol b/src/RateChangeQueue.sol index 950b1fcc..709f4bdd 100644 --- a/src/RateChangeQueue.sol +++ b/src/RateChangeQueue.sol @@ -25,20 +25,21 @@ library RateChangeQueue { require(queue.head < c.length, EmptyQueue()); unchecked { change = c[queue.head]; - delete c[queue.head]; + delete c[queue.head++]; } + } - if (queue.head + 1 == c.length) { - queue.head = 0; - // The array is already empty, waste no time zeroing it. - assembly { - sstore(c.slot, 0) - } - } else { - queue.head++; + // Clears the storage of the Queue + // If the queue isEmpty, all queue storage will be cleared + function clearEmpty(Queue storage queue) internal { + queue.head = 0; + RateChange[] storage c = queue.changes; + assembly ("memory-safe") { + sstore(c.slot, 0) } } + function peek(Queue storage queue) internal view returns (RateChange memory change) { require(queue.head < queue.changes.length, EmptyQueue()); unchecked { diff --git a/test/RateChangeQueue.t.sol b/test/RateChangeQueue.t.sol index 674fbc51..31d209e9 100644 --- a/test/RateChangeQueue.t.sol +++ b/test/RateChangeQueue.t.sol @@ -119,7 +119,9 @@ contract RateChangeQueueTest is Test { assertEq(RateChangeQueue.size(queue()), 0); // Empty queue should have reset to head = 0 + queue().clearEmpty(); assertEq(queue().head, 0); + assertEq(queue().changes.length, 0); } /// forge-config: default.allow_internal_expect_revert = true From cb671ea143791a21c9c28ffb36da9a73b0436786 Mon Sep 17 00:00:00 2001 From: William Morriss Date: Wed, 28 Jan 2026 15:25:04 -0600 Subject: [PATCH 4/5] fmt --- src/RateChangeQueue.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/RateChangeQueue.sol b/src/RateChangeQueue.sol index 709f4bdd..b626b8f6 100644 --- a/src/RateChangeQueue.sol +++ b/src/RateChangeQueue.sol @@ -39,7 +39,6 @@ library RateChangeQueue { } } - function peek(Queue storage queue) internal view returns (RateChange memory change) { require(queue.head < queue.changes.length, EmptyQueue()); unchecked { From 0b90251f2e8fed72017616d2dc3a51db0b9ca01b Mon Sep 17 00:00:00 2001 From: William Morriss Date: Wed, 28 Jan 2026 15:41:47 -0600 Subject: [PATCH 5/5] document the behavior if invoked when queue is not empty --- src/RateChangeQueue.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/RateChangeQueue.sol b/src/RateChangeQueue.sol index b626b8f6..b0714046 100644 --- a/src/RateChangeQueue.sol +++ b/src/RateChangeQueue.sol @@ -31,6 +31,7 @@ library RateChangeQueue { // Clears the storage of the Queue // If the queue isEmpty, all queue storage will be cleared + // Otherwise, the queue is functionally emptied but pending RateChange are not cleared from storage function clearEmpty(Queue storage queue) internal { queue.head = 0; RateChange[] storage c = queue.changes;