Redis SMEMBERS-backed pagination is unstable across airdrops, alerts, and API-key listing endpoints

[prodbycorne]

Overview

Several list endpoints implement pagination by first fetching an entire unordered Redis Set via SMEMBERS, then slicing the resulting array in application code:

// src/services/airdrops.js
async function list(page = 1, limit = 20) {
  const redis = cache.getClient();
  const ids = await redis.smembers(IDS_KEY);
  const start = (page - 1) * limit;
  const end = start + limit;
  const paginatedIds = ids.slice(start, end);
  ...
}

// src/services/alerts.js
async function listPaginated({ offset = 0, limit = 20 } = {}) {
  const redis = cache.getClient();
  const ids = await redis.smembers(IDS_KEY);
  ...
  const paginatedIds = ids.slice(offset, offset + limit);
  ...
}

// src/services/apiKeys.js
async function listKeys() {
  const redis = cache.getClient();
  const ids = await redis.smembers(IDS_KEY);
  ...
}

Redis Sets have no guaranteed stable iteration order across separate SMEMBERS calls, especially as members are added/removed between requests (which is exactly what happens in production — airdrops and alerts are created and deleted continuously). A client paginating through GET /airdrops?page=1 then GET /airdrops?page=2 can observe the same item twice, skip an item entirely, or see the total item count (ids.length, used for total_pages) change between pages in a way that makes has_next/total_pages unreliable — because each request independently re-fetches and re-slices a set whose order is not guaranteed to be consistent between calls, particularly after any create/delete has occurred concurrently. This is functionally different from (and worse than) normal "data changed between page loads" pagination drift, because even with zero writes occurring, SMEMBERS order is an implementation detail of Redis's internal hash table and is not contractually stable.

Contrast this with deliveryRepository.js's listByWebhook, which correctly uses a Redis sorted set (ZADD/ZREVRANGE) keyed by creation timestamp — a stable, order-preserving structure — for exactly this reason.

Requirements

• Convert the id-tracking structures in airdropsService, alertsService, and apiKeys from Redis Sets (SADD/SMEMBERS/SREM) to sorted sets scored by creation timestamp (ZADD score=Date.now()/ZREVRANGE/ZREM), matching the pattern already established in deliveryRepository.js.
• Pagination must then use ZREVRANGE/ZRANGEBYSCORE with stable, deterministic ordering (e.g. newest-first) rather than slicing an unordered array fetched via SMEMBERS.
• Preserve existing public API response shapes (pagination.total, pagination.total_pages, etc.) — only the internal storage/retrieval mechanism changes.
• Apply consistently across all three affected services in a single coordinated change so behavior is uniform.

Acceptance Criteria

• Paginating through all airdrops across multiple page values, with no writes occurring in between, returns each airdrop exactly once, in a stable, deterministic order.
• The same guarantee holds for GET /api/v1/alerts and GET /api/v1/keys.
• A test creates N items, paginates through all pages with limit smaller than N, and asserts the concatenation of all pages equals the full set with no duplicates or omissions.
• A test with concurrent creates/deletes between page fetches demonstrates measurably better behavior than the old SMEMBERS-based approach (e.g. new items appended at the correct end rather than potentially shifting earlier pages).
• test/airdrops-service.test.js, test/alerts.test.js, and any API-key tests continue to pass against the new storage model.

Additional Notes

Additional edge cases / failure modes

• Two items created within the same millisecond (ZADD score=Date.now()) get identical scores — ZREVRANGE's tie-breaking behavior for equal scores falls back to lexicographic ordering of the member (the id string), which is stable but not necessarily "creation order" for same-millisecond ties. Since ids are generated via crypto.randomUUID()-derived strings (generateId() in each service), this tie-break order is effectively random-but-stable across repeated calls — acceptable for pagination stability (the core bug being fixed) but worth a one-line doc note that "stable" here means "consistent across page requests," not "guaranteed strict creation-order for same-millisecond ties."
• Migration path: this is a schema change to the storage format of existing production keys (airdrops:ids, alerts:ids, and whatever apiKeys's equivalent IDS_KEY is called) — if any of these have existing production data as plain Sets, a migration step is needed (read existing SMEMBERS, look up each record's created_at if available to backfill scores, ZADD into a new sorted-set key, then cut over) rather than just changing the code and expecting SMEMBERS-populated keys to work with ZREVRANGE (they won't — wrong Redis type, WRONGTYPE error). Confirm whether this system has any existing production/staging data that needs migrating, or whether this is greenfield enough that a clean cutover (delete and recreate the key type) is acceptable.
• apiKeys.js was referenced in the Overview/Requirements but not read in depth for this notes pass — confirm its listKeys() function (and any pagination wrapper around it, if routes/keys.js exists) is updated with the same sorted-set conversion; the Requirements explicitly list it as in-scope alongside airdropsService/alertsService.
• alertsService.listPaginated({ offset, limit }) uses offset-based pagination while airdropsService.list(page, limit) uses page-based pagination with an internally-computed start/end — these two services have different pagination call conventions even though both suffer the same underlying SMEMBERS bug; the sorted-set fix should preserve each service's existing external call signature (don't accidentally unify them as part of this fix, since that would be a separate, larger API-shape change out of scope here) while fixing the internal ordering mechanism for both.
• total/total_pages computation (ids.length) becomes ZCARD on the sorted set — cheap and correct, but confirm this is computed from the same Redis call/transaction as the page slice where practical, to avoid a race where the reported total reflects a state slightly different from the actual page contents if a create/delete happens between the ZCARD and the ZREVRANGE calls (a MULTI/EXEC pipeline, or accepting this small residual race as "good enough" now that the core ordering bug is fixed, are both reasonable — but this should be a stated decision, not an oversight).

Implementation sketch

// airdrops.js
async function create(data) {
  ...
  const redis = cache.getClient();
  await cache.set(airdropKey(id), airdrop);
  await redis.zadd(IDS_KEY, Date.now(), id); // was: redis.sadd(IDS_KEY, id)
  ...
}

async function list(page = 1, limit = 20) {
  const redis = cache.getClient();
  const total = await redis.zcard(IDS_KEY);
  const start = (page - 1) * limit;
  const end = start + limit - 1;
  const paginatedIds = await redis.zrevrange(IDS_KEY, start, end); // was: smembers + slice
  const airdrops = await Promise.all(paginatedIds.map((id) => cache.get(airdropKey(id))));
  return {
    airdrops: airdrops.filter(Boolean),
    pagination: { page, limit, total, total_pages: Math.ceil(total / limit) },
  };
}

async function remove(id) {
  ...
  await redis.zrem(IDS_KEY, id); // was: redis.srem(IDS_KEY, id)
  ...
}

Apply the equivalent sadd→zadd, smembers+slice→zrevrange, srem→zrem substitution in alertsService and apiKeys.

Test / reproduction plan

1. Create 25 airdrops sequentially (so creation order is well-defined); paginate with limit=10 across pages 1-3; assert the concatenation of all three pages' ids exactly matches the creation order reversed (newest-first), with no duplicates or gaps.
2. Interleave: fetch page 1, then delete an airdrop that was on page 1, then fetch page 2; assert page 2's contents are unaffected by the deletion on page 1 (the classic "shifting window" bug this fix targets) — contrast explicitly with the old SMEMBERS+slice behavior in a comment or a "before" test snapshot if feasible.
3. Repeat both tests for alertsService.listPaginated and apiKeys.listKeys.
4. If a migration script is written, test it against a pre-populated plain-Set key and confirm correct sorted-set output with sensible fallback scoring for records lacking a usable created_at.

Related issues in this batch

• webhook_delivery:* records are never expired or pruned from Redis — unbounded memory growth #79 (unbounded delivery record growth) — deliveryRepository.js's existing sorted-set pattern is the reference implementation this issue is generalizing to the other services; the two issues touch adjacent but distinct concerns (retention/TTL vs. ordering) on the same underlying data structure choice.
• No duplicate-recipient protection across multiple POST /airdrops/:id/recipients calls — double-payment risk #81 (duplicate recipient protection) — proposes introducing a separate new Redis Set (for atomic address-membership checks) that is unrelated to and should not be conflated with the sorted-set ordering fix here, since they solve different problems on different keys.
• Repository layer has zero Redis-outage error handling — a Redis blip returns 500s instead of degrading gracefully #83 (repository-layer Redis error handling) — since this issue touches every read/write call site in airdropsService, alertsService, and apiKeys, landing it alongside or before Repository layer has zero Redis-outage error handling — a Redis blip returns 500s instead of degrading gracefully #83's error-handling wrapper avoids two separate PRs both touching the same lines in close succession.

[paris27-A]

Hi Stellar team 👋, I’ve reviewed this issue and the current pagination approach using SMEMBERS + slicing is fundamentally unreliable due to the lack of deterministic ordering in Redis Sets, which can lead to duplicates, skipped items, and inconsistent pagination even without concurrent writes; I can refactor the affected services (airdropsService, alertsService, and apiKeys) to use Redis Sorted Sets (ZADD with timestamp scoring + ZREVRANGE) to ensure stable, deterministic pagination aligned with the existing pattern in deliveryRepository.js, while preserving the current API response structure. I’ll also add comprehensive tests to verify correctness across pagination boundaries and improved behavior under concurrent mutations. I can deliver a clean, tested PR within 2 days.

[grantfox-oss]

🦊 GrantFox — @paris27-A has been assigned to this issue as part of the Official Campaign | FWC26 campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #82)
2. Your PR will be reviewed by the SmartDropLabs maintainers

Good luck! Track your progress on GrantFox.