Skip to content

fix(api-server): page token transactions by offset instead of cursor - #2136

Open
nullPointerEnjoyer wants to merge 7 commits into
masterfrom
fix/token-transactions-paging-port
Open

nullPointerEnjoyer wants to merge 7 commits into
masterfrom
fix/token-transactions-paging-port

Conversation

@nullPointerEnjoyer

Copy link
Copy Markdown
Collaborator

Summary

  • Port of fix(api-server): page token transactions by offset instead of cursor #2135 to master (same bug exists here): GET /api/v2/token/{id}/transactions returned an
    empty page for every token, because the route passes the request's page offset (skip-N, default 0)
    while the storage layer interpreted the parameter as a cursor (WHERE tx_global_index < $2).
  • Align the storage layer with the skip-N semantics the HTTP API uses: newest-first list, offset
    entries skipped, empty page at or past the end. Trait parameter renamed to offset across the
    trait, both backends, and wrappers.
  • Guard oversized offsets uniformly: an offset too big for a bigint is past the end of any result
    set, so postgres returns an empty page (five queries previously wrapped as i64, yielding 500s);
    the in-memory backend skips by usize::MAX instead of truncating as usize (seven queries),
    which also fixes first-page-instead-of-empty-page wraps on narrow-pointer targets.
  • Cherry-picked from release-v1.4.1 with conflict resolutions where master's pool/order listings
    diverged (sort and balance-decode rework); behavior of other endpoints unchanged.

Test plan

  • New storage-level regression test token_transactions_offset_paging (both backends): default
    page starts at the newest transaction, offset skips entries, offsets at/past the end (u64::MAX and
    1<<35) yield an empty page instead of an error.
  • Existing storage tests updated to the offset semantics; in-memory suite green.
  • ./do_checks.sh green; OCR review clean.

The route handler for GET /api/v2/token/{id}/transactions passes the
request's page offset (skip-N, default 0) but the storage layer treated
the parameter as a cursor (return transactions older than the given
tx_global_index). With the default offset of 0, the cursor predicate
matched nothing and the endpoint returned an empty page for every token
that had no transaction at global index zero.

Align the storage layer with the offset semantics the HTTP API (and every
other paginated endpoint) documents: return the newest transactions
starting at the given offset. An offset past the end of the result set
yields an empty page. Rename the parameter to offset in the trait,
implementations, and wrappers.
Add a storage-level regression test pinning the skip-N semantics of the
get_token_transactions offset parameter: the default page starts at the
newest transaction, the offset skips entries, and an offset at or past
the end of the list yields an empty page instead of an error, including
for offsets too large to represent in the database's integer type.
Five postgres queries cast the page offset to a bigint with , which
wraps offsets above i64::MAX to a negative number. Postgres then rejects
the negative OFFSET and the endpoint fails with an internal error.

Guard the conversion the same way the token id queries already do: an
offset too big for a bigint is past the end of any result set, so return
an empty page.
Truncating the offset to usize with 'as' can wrap an offset that exceeds
the pointer width into a small number, silently returning the first page
instead of an empty one. Skip by usize::MAX instead, which is always past
the end of any in-memory result set.

Also deduplicate the bigint offset guard in the postgres queries into a
single helper.
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

🔍 OpenCodeReview found 1 issue(s) in this PR.

  • ✅ Successfully posted inline: 0 comment(s)
  • 📋 Routed to summary by policy: 1 comment(s)

test · low

📄 api-server/storage-test-suite/src/basic.rs (L2771-L2774)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category test)

All reads here go through the same read-write transaction (tx) that performed the writes, so this only exercises within-transaction visibility. Consider asserting at least one page after tx.commit() via a fresh read-only transaction, to verify the offset paging also works against committed state in both backends.

💡 Suggested Change

Before:

    tx.commit().await.unwrap();

    Ok(())
}

After:

    tx.commit().await.unwrap();

    // Verify offset paging against committed state via a read-only transaction.
    let ro = storage.transaction_ro().await.unwrap();
    assert!(ro.get_token_transactions(token_id, 10, 10).await.unwrap().is_empty());
    assert_eq!(ro.get_token_transactions(token_id, 4, 3).await.unwrap(), &newest_first[3..7]);

    Ok(())
}

The end-to-end test requested pages with an offset larger than any global
index, which only returned data while the storage layer misread the offset
as a cursor. Request the pages by their skip counts instead.
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