fix(exchange): load every order history page

[ethicnology]

listOrderSummaries only fetched page 1 (50 orders), silently dropping older transactions. Now pages through until the backend returns a short page.

Closes #2528

[claude]

Claude finished @ethicnology's task in 2m 49s —— View job

---

Review: fix(exchange): load every order history page

• Gather context (diff vs origin/develop, read changed files)
• Review pagination logic for correctness bugs and regressions
• Review new test
• Post review findings

The fix is sound: the loop pages until a short/empty page, dedupes by orderId (which also handles page-boundary drift when new orders arrive mid-pagination, since the sort is createdAt desc), and the putIfAbsent + repeated-page guard prevents an infinite loop against a backend that ignores the page param. Insertion order is preserved by the map, so the returned list keeps the API's ordering. The tests correctly exercise both the multi-page path and the repeated-page guard. A few findings below, roughly in order of importance.

Note: I can't post inline review comments (I can only update this single comment), so findings are referenced by file/line instead.

Findings

1. Termination silently trusts that the server honors the requested pageSize — bullbitcoin_api_datasource.dart:274

if (elements.length < pageSize || ...) break; assumes a full page always contains exactly pageSize elements. If the backend ever clamps pageSize below the requested value (e.g. client asks for 50, server caps at 25), every page comes back "short" and the loop stops after page 1 — silently reproducing the exact bug this PR fixes, with the tests still green because the mock always honors the request. The API already returns result.totalElements for this paginator style (see how bullbitcoin_api_recipients_gateway.dart:106 uses it). Looping until ordersById.length >= totalElements (keeping the empty-page and duplicate-page guards as backstops) removes both this assumption and finding 2. Fix this →

2. The loop has no absolute upper bound — bullbitcoin_api_datasource.dart:247

for (var page = 1; ; page++) only stops on an empty page, a short page, or a page that adds zero new ids. The duplicate guard covers a backend that repeats pages, but a buggy backend that keeps serving full pages of distinct ids loops indefinitely, hammering the API from the client. A hard cap (or the totalElements-based bound from finding 1) makes termination unconditional.

3. Edge case: a full page of all-duplicates aborts the remaining history — bullbitcoin_api_datasource.dart:274

With createdAt desc, if ≥ pageSize new orders are created between fetching page N and page N+1, page N+1 contains exactly the orders already seen on page N, ordersById.length == previousLength fires, and everything older is silently dropped. Needing 50 new orders mid-flight makes this very unlikely at the default page size, but it's another case that totalElements termination would handle correctly (the guard would then only be a backstop against a truly broken backend).

4. Performance / failure-amplification regression for the callers — exchange_order_repository_impl.dart:66, exchange_order_repository_impl.dart:103

getOrders() runs on every transactions-screen load (get_transactions_usecase.dart:54), and getOrderByTxId now pulls the entire order history just to find one transaction. For exactly the users this PR targets (long histories), each of those calls becomes N sequential round trips, and a non-200 on page k throws away pages 1..k-1 (bullbitcoin_api_datasource.dart:261-263) — so the whole call is both slower and N× more likely to fail outright than before. Probably acceptable as the correctness fix, but worth a follow-up: getOrders(limit, offset) already receives pagination params it ignores, and the existing comment at exchange_order_repository_impl.dart:111 notes this should be served from disk.

5. Pre-existing, now reachable mid-loop: resp.data['result']['elements'] at bullbitcoin_api_datasource.dart:265 throws a raw NoSuchMethodError if the API returns HTTP 200 with a JSON-RPC error body and no result (the pattern createBuyOrder explicitly handles). Not introduced by this PR, but a null-check would turn an opaque crash into a clear exception.

Test nit: the exact-multiple case (last page completely full, next page empty — the elements.isEmpty break) isn't covered; a third test with e.g. pages {1: [o1, o2], 2: [o3, o4], 3: []} expecting requestedPages == [1, 2, 3] would pin it down.

Summary

No blocking bugs — the happy path is correct and the infinite-loop guards work. The one change I'd recommend before merge is basing termination on result.totalElements (finding 1), which the backend already provides and which eliminates findings 1–3 in one move. Finding 4 is a real but pre-structured performance cost that likely deserves a follow-up issue rather than scope creep here.

---