Skip to content

fix: paginate bridge tracker activity endpoint 442 - #1888

Merged
joanestebanr merged 6 commits into
developfrom
feat/bridgetracker-activity-pagination
Oct 2, 2026
Merged

joanestebanr merged 6 commits into
developfrom
feat/bridgetracker-activity-pagination

Conversation

@joanestebanr

@joanestebanr joanestebanr commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

🔄 Changes Summary

  • GET /tracker/v1/activity/from/{from_address} now accepts page_number/page_size query params (same names, 1-based numbering, defaults and max as the bridge service's own paginated endpoints: default 1/20, max 200).
  • Results are sorted most recent bridge first (by CreatedAt, descending) before being paginated.
  • ActivityResponse gains a count field reporting the total number of bridges matching filterBridges across every page, mirroring BridgesResult/ClaimsResult's own count field.
  • CurrentAPIRevision bumped from 5 to 6 (moved to bridgetracker/api/revision.go), with the revision-history entry explaining the change.
  • Docs: new "Pagination" section in docs/bridgetracker/API.md, including the consistency caveats below.
  • No domain-interface or storage changes: both backing implementations (ActivityCache in-memory and the SQLite-backed store) already return the bounded, per-address bridge set, so sorting/pagination is applied once in the HTTP layer instead of duplicating it across both stores (and their generated mocks).

⚠️ Breaking Changes

  • 🔌 API/CLI: behavior change for GET /tracker/v1/activity/from/{from_address}: a client sending neither page_number nor page_size used to receive every bridge and now receives only the first 20 (most recent first), so it must page using count. The response schema itself is only extended (new count field). Hence api_revision 5 → 6.

📋 Config Updates

  • None.

🔌 API Updates

🔌 Bridge service API

  • None.

🔌 Proxy API

  • None.

🔌 Others API

  • 🔌 Bridge tracker API Update: GET /tracker/v1/activity/from/{from_address} — added optional page_number/page_size query params and a count field in the response body. The schema change is additive, but the default page size (20) changes what a client sending no pagination params receives, so it can break clients that assumed the full list (see Breaking Changes). api_revision bumped to 6.

✅ Testing

  • 🤖 Automatic: added unit tests in bridgetracker/api/activity_command_test.go covering most-recent-first sorting, default pagination, explicit page_number/page_size slicing, a page past the end, and invalid pagination params (400). go build ./..., go test ./bridgetracker/... and golangci-lint run ./bridgetracker/... all pass.
  • 🖱️ Manual: none beyond the above — no dev_ui/SDK consumer of this endpoint exists in this repository.

🐞 Issues

  • Fixes agglayer/pm#442

🔗 Related PRs

  • None.

📝 Notes

  • Pagination/sort is applied in the HTTP command layer rather than pushed into SQL or the in-memory cache: changing domain.ActivityQuerier.GetActivity's signature would have required updating both store implementations, their generated mocks, and ~50 existing test call sites, for no real benefit given the per-address result set is already bounded (one wallet's bridge history, not the global bridges table).
  • Pages are not a consistent snapshot (same offset-based mechanism as the bridge service): if new bridges arrive while a client pages, it can see duplicates across pages and miss new bridges until it refetches page 1. Clients should deduplicate by bridge.global_index and refetch page 1 if count changes. Documented in docs/bridgetracker/API.md. Cursor-based pagination was considered and left out to stay consistent with the bridge service.

🤖 Generated with Claude Code

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-02T08:54:27.438166Z 756531d PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 756531d34e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread bridgetracker/api/activity_command.go Outdated
Comment on lines +335 to +337
sort.SliceStable(entries, func(i, j int) bool {
return entries[i].CreatedAt.After(entries[j].CreatedAt)
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add a deterministic tie-breaker before slicing pages

When two or more bridges share a CreatedAt value—common for deposits in the same block—this comparator treats them as equal. The in-memory implementation reconstructs entries from map iteration (bridgetracker/activity.go:314-318), while the SQLite query also has no ORDER BY, so the relative order of tied entries can differ between requests; because SliceStable preserves that arbitrary input order, an entry can move across a page boundary and clients paging through results can receive duplicates or omit bridges. Sort equal timestamps by a unique stable field such as the global index before applying the offset.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fda589d. Entries are now sorted by CreatedAt descending and, for equal timestamps, by global index descending (compared numerically with big.Int, since it can exceed 2^64), so tied entries keep the same relative order on every request regardless of the input order (map iteration in the in-memory cache, unordered SQL in the SQLite store). Added TestActivityCommandExecute_TiedCreatedAtOrderedByGlobalIndex and updated the API docs, which previously said the order of ties was not guaranteed.

@joanestebanr
joanestebanr force-pushed the feat/bridgetracker-activity-pagination branch from fda589d to 73d9ef5 Compare October 2, 2026 09:41
@joanestebanr

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude

claude Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Claude finished @joanestebanr's task in 1m 0s —— View job


PR review: paginate bridge tracker activity endpoint (#1888)

I read the full diff against origin/develop. I could not run go test or golangci-lint here because the sandbox denied those commands, so none of this is test-verified. A grep for lines over 120 characters in the changed Go files found none.

Verdict: the change is correct and well scoped, and the Codex tie-breaker finding is properly fixed. I found no 🔴 blockers, only a few 🟡 and 🟢 points.

Findings

🟡 Docs describe creation_timestamp wrongly, and the new sort depends on it.

  • docs/bridgetracker/API.md:504 says it is "when this bridge was first cached by this endpoint". The ActivityItem.CreationTimestamp doc comment at bridgetracker/api/activity_command.go:87-89 says the same.
  • domain.ActivityEntry.CreatedAt (bridgetracker/domain/activity.go:98) is the bridge's origin deposit block timestamp, falling back to first-seen time only when BlockTimestamp is missing.
  • "Most recent bridge first" is now documented in terms of this field, so both places should be corrected. If they aren't, readers will expect the order to follow cache insertion time.

🟢 Comparator allocates big.Ints on every comparison (activity_command.go:356-363).

  • The sort calls SetString twice per comparison, so about 2·n·log n allocations per request. The set is bounded per address, so this is not a real problem.
  • Parsing each global index once before sorting would remove the allocations. A small slice of {entry, *big.Int} would do it.
  • The string-compare fallback for unparseable values is also not ordered numerically. Since BigIntString should always be decimal, I wouldn't spend effort on that path.

🟢 Entries with no block timestamp sort as the newest.

  • BlockTimeOrNow stamps a bridge with "now" when BlockTimestamp == 0. That value is carried forward, so the order stays stable. A bridge the bridge service hasn't fully indexed will still appear at the top of page 1 until it's cached.
  • That is probably acceptable, but a one-line note in the Pagination docs would avoid surprises.

🟢 Tie-break uniqueness depends on SQL keying. The SQLite table uses PRIMARY KEY (global_index) (bridgetracker0002.sql:58), so global_index is unique per bridge there, which matches the docs' claim. If the key ever becomes composite (for example with network ID), adding BridgeNetworkID as a third sort key would be cheap insurance.

🟢 Minor:

  • The doc comment on paginateActivityEntries (activity_command.go:330-335) is wrapped unevenly, with line 332 much longer than its neighbours. Re-wrapping it would help.
  • CurrentAPIRevision moved from types to api. All in-repo references are updated: health_command.go and bridgetracker_test.go. The remaining mentions are only comments and generated swagger text. External importers of types.CurrentAPIRevision would break at compile time, though I found none in this repo.

Behavior notes

  • Default page size: a client sending no pagination params now gets 20 bridges instead of all of them. The PR flags this and bumps api_revision to 6, so it is handled. I'd call it out in the release notes as well.
  • Count and page boundaries: Count = len(entries) is taken before slicing and offset uses uint64, so there is no overflow at page_number * page_size. A page past the end returns an empty, non-nil bridges array, which marshals as [].
  • Sorting in place: sort.Slice mutates the slice returned by GetActivity. That is fine as long as both stores return a fresh slice, which ActivityCache does (it rebuilds from map iteration).
  • Validation order: page_number and page_size are validated before flush_cache runs, so a bad param doesn't discard the cache. A test covers this: InvalidPaginationParamsRejectedBeforeFlush.
  • Test coverage:
    • Covered: sorting, tie-break, default page, explicit page, past the end, and invalid params.
    • Not covered: page_size exactly 200 and 201, and Count against a filtered set. Both are minor.

🚀 What's New

  • GET /tracker/v1/activity/from/{from_address} now takes optional page_number (1-based, default 1) and page_size (default 20, max 200). These follow the bridge service's conventions.
  • Bridges are sorted most recent first by CreatedAt. Ties are broken by global_index descending, compared numerically, so the order is the same on every request.
  • ActivityResponse gains a count field with the total number of bridges matching filterBridges across all pages.
  • CurrentAPIRevision is bumped from 5 to 6 and moved to bridgetracker/api/revision.go. Swagger files and docs/bridgetracker/API.md are updated, including a Pagination section with consistency caveats.
  • Sorting and pagination are applied in the HTTP layer, so no domain, storage or mock changes were needed.

🐛 Bug Fixes

None. The tie-break ordering was a review fix within this PR, not a fix to existing behavior.

📋 Config Updates

None.

⚠️ Breaking Changes

  • A client sending neither page_number nor page_size previously received every bridge and now receives only the 20 most recent. It must page through the result using count.
  • The response schema is only extended (new count field), and api_revision goes from 5 to 6.
  • CurrentAPIRevision moved from the bridgetracker/types package to bridgetracker/api, so Go code importing the old symbol must update.
    | Branch: feat/bridgetracker-activity-pagination

@joanestebanr

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Addressed in edc99c2:

  • creation_timestamp wording: corrected in the ActivityItem comment, docs/bridgetracker/API.md and the generated swagger files. It is the bridge's origin deposit block timestamp (falling back to first-cached time only when the bridge service has not populated it) and is the sort key.
  • big.Int allocations: global indexes are now parsed once before sorting (sortActivityEntries) instead of on every comparison.
  • Entries without a block timestamp: documented in the Pagination section that they sort at the top until the bridge service populates the timestamp.
  • Comment wrapping: paginateActivityEntries doc re-wrapped.
  • Test coverage: added a test for page_size=200 (accepted) and 201 (400).

Not changed:

  • A third sort key (BridgeNetworkID): global_index is the SQLite primary key today, so it is already unique. Easy to add if the key ever becomes composite.
  • A Count-with-filter test: filtering happens in the registry, not in this command, so a test here would only exercise the fake.
  • The release note about the default page size is on the PR description (Breaking Changes).

@joanestebanr joanestebanr self-assigned this Oct 2, 2026
joanestebanr and others added 6 commits October 2, 2026 13:31
GET /activity/from/{from_address} returned every cached bridge for an
address in one unordered response. Add page_number/page_size query
params with the same names, defaults (1/20) and max (200) as the
bridge service's own paginated endpoints, sort results most recent
bridge first, and report the total match count in a new "count" field
(mirroring BridgesResult/ClaimsResult).

Fixes agglayer/pm#442
Pages are not a consistent snapshot: new bridges can cause duplicates
across pages and be missed until the first page is refetched. Document
how clients should page (count, last page) and handle this.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…tion

The activity endpoint now paginates by default (first 20 bridges) and
returns a new count field, so clients must page to get every bridge.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Entries sharing a CreatedAt (e.g. deposits in the same block) had an
arbitrary relative order that could change between requests, letting a
bridge move across a page boundary. Sort ties by global index
descending, compared numerically.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Correct the creation_timestamp description (it is the bridge's deposit
block timestamp, now the sort key), parse global indexes once before
sorting, document how bridges without a block timestamp sort, rewrap a
comment and test the max page_size.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@joanestebanr
joanestebanr force-pushed the feat/bridgetracker-activity-pagination branch from edc99c2 to 0a7e65d Compare October 2, 2026 11:33
@joanestebanr
joanestebanr merged commit 5d64751 into develop Oct 2, 2026
33 checks passed
@joanestebanr
joanestebanr deleted the feat/bridgetracker-activity-pagination branch October 2, 2026 12:48
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.

2 participants