refactor: replace queue mappings with sorted-array invariant (#116)

src/abstract/AbstractRecipientRegistry.sol

@@ -24,12 +24,6 @@ abstract contract AbstractRecipientRegistry is IRecipientRegistry, OwnableUpgrad
         /// @notice Mapping to quickly check if an address is an active recipient
         /// @dev Maps recipient address to true if active, false otherwise
         mapping(address => bool) isRecipientMapping;
-        /// @notice Mapping to quickly check if an address is queued for addition
-        /// @dev Maps recipient address to true if queued for addition, false otherwise
-        mapping(address => bool) isQueuedForAdditionMapping;
-        /// @notice Mapping to quickly check if an address is queued for removal
-        /// @dev Maps recipient address to true if queued for removal, false otherwise
-        mapping(address => bool) isQueuedForRemovalMapping;
     }
 
     /// keccak256(abi.encode(uint256(keccak256("crowdstake.storage.AbstractRecipientRegistry")) - 1)) & ~bytes32(uint256(0xff))
@@ -89,7 +83,8 @@ abstract contract AbstractRecipientRegistry is IRecipientRegistry, OwnableUpgrad
     // ============ Internal Functions ============
 
     /// @notice Internal function to queue a recipient for addition
-    /// @param recipient Address to add to the queue
+    /// @param recipient Address to add to the queue; must be strictly greater than the last
+    ///        queued address (ascending order) to maintain the sorted-array invariant
     /// @dev This is an internal function that should be called by derived contracts
     /// @dev Validates the recipient address and checks for duplicates before queuing
     /// @dev Emits RecipientQueued event with isAddition=true
@@ -98,10 +93,13 @@ abstract contract AbstractRecipientRegistry is IRecipientRegistry, OwnableUpgrad
         AbstractRecipientRegistryStorage storage $ = _getAbstractRecipientRegistryStorage();
         if (recipient == address(0)) revert InvalidRecipient();
         if ($.isRecipientMapping[recipient]) revert RecipientAlreadyExists();
-        if ($.isQueuedForAdditionMapping[recipient]) revert RecipientAlreadyQueued();
         if ($.queuedRecipientsForAddition.length + 1 > MAX_QUEUE_SIZE) revert MaxQueueSizeReached();
 
-        $.isQueuedForAdditionMapping[recipient] = true;
+        uint256 len = $.queuedRecipientsForAddition.length;
+        if (len > 0 && uint160(recipient) <= uint160($.queuedRecipientsForAddition[len - 1])) {
+            revert QueueNotSorted();
+        }
+
         $.queuedRecipientsForAddition.push(recipient);
         emit RecipientQueued(recipient, true);
     }
@@ -116,10 +114,13 @@ abstract contract AbstractRecipientRegistry is IRecipientRegistry, OwnableUpgrad
         AbstractRecipientRegistryStorage storage $ = _getAbstractRecipientRegistryStorage();
         if (recipient == address(0)) revert InvalidRecipient();
         if (!$.isRecipientMapping[recipient]) revert RecipientNotFound();
-        if ($.isQueuedForRemovalMapping[recipient]) revert RecipientAlreadyQueued();
         if ($.queuedRecipientsForRemoval.length + 1 > MAX_QUEUE_SIZE) revert MaxQueueSizeReached();
 
-        $.isQueuedForRemovalMapping[recipient] = true;
+        uint256 len = $.queuedRecipientsForRemoval.length;
+        if (len > 0 && uint160(recipient) <= uint160($.queuedRecipientsForRemoval[len - 1])) {
+            revert QueueNotSorted();
+        }
+
         $.queuedRecipientsForRemoval.push(recipient);
         emit RecipientQueued(recipient, false);
     }
@@ -146,7 +147,6 @@ abstract contract AbstractRecipientRegistry is IRecipientRegistry, OwnableUpgrad
             address recipient = addedList[i];
             $.recipients.push(recipient);
             $.isRecipientMapping[recipient] = true;
-            $.isQueuedForAdditionMapping[recipient] = false;
             emit RecipientAdded(recipient);
         }
 
@@ -164,7 +164,6 @@ abstract contract AbstractRecipientRegistry is IRecipientRegistry, OwnableUpgrad
                     if (recipient == removedList[j]) {
                         shouldRemove = true;
                         $.isRecipientMapping[recipient] = false;
-                        $.isQueuedForRemovalMapping[recipient] = false;
                         emit RecipientRemoved(recipient);
                         break;
                     }
@@ -188,22 +187,14 @@ abstract contract AbstractRecipientRegistry is IRecipientRegistry, OwnableUpgrad
     /// @dev Only owner can clear the queue. Use this to cancel all pending additions
     /// @dev This will remove all addresses from the addition queue without adding them
     function clearAdditionQueue() external onlyOwner {
-        AbstractRecipientRegistryStorage storage $ = _getAbstractRecipientRegistryStorage();
-        for (uint256 i = 0; i < $.queuedRecipientsForAddition.length; i++) {
-            $.isQueuedForAdditionMapping[$.queuedRecipientsForAddition[i]] = false;
-        }
-        delete $.queuedRecipientsForAddition;
+        delete _getAbstractRecipientRegistryStorage().queuedRecipientsForAddition;
     }
 
     /// @notice Clear the removal queue without processing
     /// @dev Only owner can clear the queue. Use this to cancel all pending removals
     /// @dev This will remove all addresses from the removal queue without removing them
     function clearRemovalQueue() external onlyOwner {
-        AbstractRecipientRegistryStorage storage $ = _getAbstractRecipientRegistryStorage();
-        for (uint256 i = 0; i < $.queuedRecipientsForRemoval.length; i++) {
-            $.isQueuedForRemovalMapping[$.queuedRecipientsForRemoval[i]] = false;
-        }
-        delete $.queuedRecipientsForRemoval;
+        delete _getAbstractRecipientRegistryStorage().queuedRecipientsForRemoval;
     }
 
     /// @notice Get all active recipients
@@ -238,14 +229,36 @@ abstract contract AbstractRecipientRegistry is IRecipientRegistry, OwnableUpgrad
     /// @param recipient Address to check in the addition queue
     /// @return isQueued True if the address is queued for addition, false otherwise
     function isQueuedForAddition(address recipient) external view returns (bool isQueued) {
-        return _getAbstractRecipientRegistryStorage().isQueuedForAdditionMapping[recipient];
+        return _binarySearch(_getAbstractRecipientRegistryStorage().queuedRecipientsForAddition, recipient);
     }
 
     /// @notice Check if an address is queued for removal
     /// @param recipient Address to check in the removal queue
     /// @return isQueued True if the address is queued for removal, false otherwise
     function isQueuedForRemoval(address recipient) external view returns (bool isQueued) {
-        return _getAbstractRecipientRegistryStorage().isQueuedForRemovalMapping[recipient];
+        return _binarySearch(_getAbstractRecipientRegistryStorage().queuedRecipientsForRemoval, recipient);
+    }
+
+    /// @dev Binary search on a sorted address array
+    function _binarySearch(address[] storage arr, address target) internal view returns (bool) {
+        uint256 len = arr.length;
+        if (len == 0) return false;
+
+        uint256 low = 0;
+        uint256 high = len;
+        uint256 targetUint = uint160(target);
+
+        while (low < high) {
+            uint256 mid = low + (high - low) / 2;
+            uint256 midVal = uint160(arr[mid]);
+            if (midVal == targetUint) return true;
+            if (midVal < targetUint) {
+                low = mid + 1;
+            } else {
+                high = mid;
+            }
+        }
+        return false;
     }
 
     /// @notice Check if an address is currently an active recipient

src/interfaces/IRecipientRegistry.sol

@@ -40,6 +40,10 @@ interface IRecipientRegistry {
     /// @notice Thrown when attempting to queue a recipient that is already queued
     error RecipientAlreadyQueued();
 
+    /// @notice Thrown when a recipient address is not greater than the last queued address
+    /// @dev Queues must be submitted in ascending address order to enable O(1) enqueue + O(log n) lookup
+    error QueueNotSorted();
+
     /// @notice Thrown when attempting to queue a recipient when the queue has reached the maximum size
     error MaxQueueSizeReached();
 

test/AdminRecipientRegistry.t.sol

@@ -200,6 +200,113 @@ contract AdminRecipientRegistryTest is TestWrapper {
         registry.queueRecipientAddition(RECIPIENT_2);
     }
 
+    function test_WhenRe_queuingAfterRemoval() external {
+        // it should allow re-adding a previously removed recipient
+        vm.startPrank(ADMIN);
+
+        registry.queueRecipientAddition(RECIPIENT_1);
+        registry.processQueue();
+        assertTrue(registry.isRecipient(RECIPIENT_1));
+
+        registry.queueRecipientRemoval(RECIPIENT_1);
+        registry.processQueue();
+        assertFalse(registry.isRecipient(RECIPIENT_1));
+
+        // Re-add should succeed (queue is empty, no stale state)
+        registry.queueRecipientAddition(RECIPIENT_1);
+        registry.processQueue();
+        assertTrue(registry.isRecipient(RECIPIENT_1));
+
+        vm.stopPrank();
+    }
+
+    function test_WhenRe_queuingAfterClear() external {
+        // it should allow re-adding after clearing addition queue
+        vm.startPrank(ADMIN);
+
+        registry.queueRecipientAddition(RECIPIENT_1);
+        assertTrue(registry.isQueuedForAddition(RECIPIENT_1));
+
+        registry.clearAdditionQueue();
+        assertFalse(registry.isQueuedForAddition(RECIPIENT_1));
+
+        // Re-queue should succeed after clear
+        registry.queueRecipientAddition(RECIPIENT_1);
+        assertTrue(registry.isQueuedForAddition(RECIPIENT_1));
+        registry.processQueue();
+        assertTrue(registry.isRecipient(RECIPIENT_1));
+
+        vm.stopPrank();
+    }
+
+    function test_RevertWhen_QueuingDuplicateInSameBatch() external {
+        // it should revert with QueueNotSorted
+        vm.startPrank(ADMIN);
+        registry.queueRecipientAddition(RECIPIENT_1);
+
+        vm.expectRevert(IRecipientRegistry.QueueNotSorted.selector);
+        registry.queueRecipientAddition(RECIPIENT_1);
+        vm.stopPrank();
+    }
+
+    function test_RevertWhen_QueuingOutOfOrder() external {
+        // it should revert with QueueNotSorted
+        vm.startPrank(ADMIN);
+        registry.queueRecipientAddition(RECIPIENT_2);
+
+        // RECIPIENT_1 < RECIPIENT_2, so this should revert
+        vm.expectRevert(IRecipientRegistry.QueueNotSorted.selector);
+        registry.queueRecipientAddition(RECIPIENT_1);
+        vm.stopPrank();
+    }
+
+    function test_WhenCheckingCrossQueueIndependence() external {
+        // it should keep addition and removal queues independent
+        vm.startPrank(ADMIN);
+
+        // Add recipients first so they can be queued for removal
+        registry.queueRecipientAddition(RECIPIENT_1);
+        registry.queueRecipientAddition(RECIPIENT_2);
+        registry.processQueue();
+
+        // Queue RECIPIENT_1 for removal and a new address for addition independently
+        registry.queueRecipientRemoval(RECIPIENT_1);
+        registry.queueRecipientAddition(RECIPIENT_3);
+
+        assertTrue(registry.isQueuedForRemoval(RECIPIENT_1));
+        assertTrue(registry.isQueuedForAddition(RECIPIENT_3));
+
+        registry.processQueue();
+
+        assertFalse(registry.isRecipient(RECIPIENT_1));
+        assertTrue(registry.isRecipient(RECIPIENT_2));
+        assertTrue(registry.isRecipient(RECIPIENT_3));
+
+        vm.stopPrank();
+    }
+
+    function test_WhenClearingRemovalQueueAndRe_queuing() external {
+        // it should allow re-queuing for removal after clearing
+        vm.startPrank(ADMIN);
+
+        registry.queueRecipientAddition(RECIPIENT_1);
+        registry.processQueue();
+
+        registry.queueRecipientRemoval(RECIPIENT_1);
+        assertTrue(registry.isQueuedForRemoval(RECIPIENT_1));
+
+        registry.clearRemovalQueue();
+        assertFalse(registry.isQueuedForRemoval(RECIPIENT_1));
+
+        // Re-queue for removal should succeed after clear
+        registry.queueRecipientRemoval(RECIPIENT_1);
+        assertTrue(registry.isQueuedForRemoval(RECIPIENT_1));
+        registry.processQueue();
+        assertFalse(registry.isRecipient(RECIPIENT_1));
+
+        vm.stopPrank();
+    }
+
     function test_WhenPerformingLargeScaleOperations() external {
         // it should handle 100 additions and 50 removals
         vm.startPrank(ADMIN);

test/RecipientRegistry.t.sol

@@ -68,10 +68,10 @@ contract RecipientRegistryTest is TestWrapper {
     }
 
     function test_GivenTheRecipientIsAlreadyQueued() external {
-        // it should revert with RecipientAlreadyQueued
+        // it should revert with QueueNotSorted (duplicate = not sorted)
         registry.queueRecipientAddition(RECIPIENT_1);
 
-        vm.expectRevert(IRecipientRegistry.RecipientAlreadyQueued.selector);
+        vm.expectRevert(IRecipientRegistry.QueueNotSorted.selector);
         registry.queueRecipientAddition(RECIPIENT_1);
     }
 
@@ -102,13 +102,13 @@ contract RecipientRegistryTest is TestWrapper {
     }
 
     function test_GivenTheRecipientIsAlreadyQueuedForRemoval() external {
-        // it should revert with RecipientAlreadyQueued
+        // it should revert with QueueNotSorted (duplicate = not sorted)
         registry.queueRecipientAddition(RECIPIENT_1);
         registry.processQueue();
 
         registry.queueRecipientRemoval(RECIPIENT_1);
 
-        vm.expectRevert(IRecipientRegistry.RecipientAlreadyQueued.selector);
+        vm.expectRevert(IRecipientRegistry.QueueNotSorted.selector);
         registry.queueRecipientRemoval(RECIPIENT_1);
     }