Skip to content

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

Open
RonTuretzky wants to merge 2 commits into
45-optimize-recipient-queue-lookup-in-baserecipeintregistry-for-large-queues-addition-and-removalfrom
fix/sorted-queue-no-mapping
Open

refactor: replace queue mappings with sorted-array invariant#116
RonTuretzky wants to merge 2 commits into
45-optimize-recipient-queue-lookup-in-baserecipeintregistry-for-large-queues-addition-and-removalfrom
fix/sorted-queue-no-mapping

Conversation

@RonTuretzky
Copy link
Copy Markdown
Contributor

Summary

Addresses the review feedback on #90:

  • Removes isQueuedForAdditionMapping and isQueuedForRemovalMapping from storage entirely. Instead, queues enforce ascending address order — duplicate detection is a single <= comparison against the last element, O(1).
  • clearAdditionQueue / clearRemovalQueue are O(1) again — just delete array, no loop needed to clear mapping entries. This resolves the gas-unbounded concern.
  • isQueuedForAddition / isQueuedForRemoval view functions use binary search on the sorted array.
  • Adds QueueNotSorted error to the interface for when callers submit addresses out of order.
  • Adds test coverage for: re-queue after removal, re-queue after clear, duplicate queue revert, out-of-order revert, cross-queue independence, and clear-removal-then-requeue.

Tradeoff: callers must submit addresses in ascending order. For admin/governance-gated calls this is trivially enforced off-chain.

Test plan

  • All 18 AdminRecipientRegistryTest tests pass (6 new)
  • All 24 VotingRecipientRegistryTest tests pass
  • All 20 RecipientRegistryTest tests pass
  • 62 total tests, 0 failures

… duplicate detection

Remove isQueuedForAdditionMapping and isQueuedForRemovalMapping from storage.
Instead, require queue entries to be submitted in ascending address order —
duplicate and ordering checks become a single comparison against the last element.

This eliminates the O(n) loop in clearAdditionQueue/clearRemovalQueue (now just
`delete array`) and removes two storage mappings entirely. View functions
(isQueuedForAddition/isQueuedForRemoval) use binary search on the sorted array.

Adds test coverage for re-queue after removal, re-queue after clear,
duplicate/out-of-order reverts, and cross-queue independence.
@RonTuretzky RonTuretzky requested a review from exo404 April 7, 2026 18:12
Merge remote branch and resolve conflicts:
- Keep MAX_QUEUE_SIZE (100) from remote as additional safety bound
- Keep sorted-array invariant for duplicate detection (no mappings)
- Update RecipientRegistry tests to expect QueueNotSorted instead of
  RecipientAlreadyQueued for duplicate queue entries
RonTuretzky added a commit that referenced this pull request Apr 15, 2026
Replace O(1) mapping-based duplicate detection with sorted-array
invariant + binary search for O(log n) lookup. Require ascending
address order for queue submissions. Simplify clear operations back
to simple delete. Add QueueNotSorted error. Add 6 new tests for
sorted order enforcement and re-queue scenarios.
RonTuretzky added a commit that referenced this pull request Apr 16, 2026
Replace O(1) mapping-based duplicate detection with sorted-array
invariant + binary search for O(log n) lookup. Require ascending
address order for queue submissions. Simplify clear operations back
to simple delete. Add QueueNotSorted error. Add 6 new tests for
sorted order enforcement and re-queue scenarios.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant