Skip to content

fix(#1346): paginate the ownership-chain history endpoint - #1423

Closed
jarik2014 wants to merge 3 commits into
CodeGirlsInc:mainfrom
jarik2014:fix/1346-chain-pagination
Closed

jarik2014 wants to merge 3 commits into
CodeGirlsInc:mainfrom
jarik2014:fix/1346-chain-pagination

Conversation

@jarik2014

Copy link
Copy Markdown

Closes #1346.

The gap

contract/src/module/ownership_chain/pagination.rs existed with a working paginate() but was never reachable: mod.rs did not declare the module, so nothing compiled it, and no handler called it. That is the whole issue — not a missing algorithm, a missing wire.

What this does

  • Declares the pagination module in src/module/ownership_chain/mod.rs.
  • Connects cursor / page_size to the endpoint that actually serves the chain: GET /verify/:hash/history, which the router in src/lib.rs mounts and which already returns the ownership-chain records.
  • Opt-in, so nothing breaks: with no query params the response is unchanged (whole chain, next_cursor: null). Pass page_size and/or cursor and you get a page plus next_cursor; count stays the total length of the chain.
  • Adds next_cursor to HistoryResponse (additive field).
  • Tests: 7 unit tests on paginate() (first/last page, exact fit, cursor at and past the end, zero page size, empty input) and 5 integration tests on the mounted endpoint (full chain without params, first page reports the cursor, last page has none, cursor past the end is a 200 with an empty page rather than a 500, and page_size is capped).

One thing worth knowing

get_transfer_history in src/lib.rs is not mounted by the live router — routes.rs declares GET /transfer/:document_hash, but the app the tests build (app() in lib.rs) never adds it, so that handler is unreachable and paginating it would have changed nothing. I first wired the pagination there, found the route missing, and moved it to /verify/:hash/history. Both handlers exist; only one is served. If you want the /transfer/:document_hash route mounted as well, say so and I will add it in a separate change.

Pre-existing breakage this branch has to carry

contract/ does not compile on main at all, so no test could run:

error[E0277]: the trait bound `DateTime<Utc>: serde::Deserialize<'de>` is not satisfied   --> src/event.rs:17:20
error[E0277]: the trait bound `RateLimiter<...>: Clone` is not satisfied                  --> src/lib.rs:77:5
error[E0382]: borrow of moved value: `data_key`                                           --> src/stellar.rs:788:9
error: could not compile `stellar-doc-verifier` (lib) due to 5 previous errors
error: could not compile `stellar-doc-verifier` (lib test) due to 6 previous errors

The first commit fixes exactly those (chrono serde feature, the rate limiter behind an Arc, the moved data_key in the mock), plus one more that only shows up once the tests compile: the integration test target imports tower::util::ServiceExt, which needs tower's util feature enabled in Cargo.toml. Split it out if you prefer to land the unbreak on its own — it is one commit and nothing in it is specific to pagination.

Verification (stable toolchain; contract/rust-toolchain.toml pins 1.82, which cannot even parse a lockfile dependency requiring edition2024)

$ cargo test --lib pagination
test result: ok. 7 passed; 0 failed; 0 ignored

$ cargo test --test handler_integration_tests
test test_verify_history_without_params_returns_the_whole_chain ... ok
test test_verify_history_first_page_reports_next_cursor ... ok
test test_verify_history_last_page_has_null_next_cursor ... ok
test test_verify_history_cursor_past_the_end_is_empty_not_500 ... ok
test test_verify_history_page_size_is_capped_at_the_maximum ... ok
test result: ok. 19 passed; 0 failed; 0 ignored

$ cargo test --all
test result: FAILED. 142 passed; 11 failed   (lib)

The 11 remaining failures are pre-existing harness problems in code this change does not touch — hash_validator::tests::sha256/sha512_rejects_non_ascii_unicode_and_binary_characters, six stellar::tests::* (mock keypanics with Invalid secret key: InvalidStrKeyChecksum, Horizon 404/500 mocks), and two webhook::tests::*. They cannot be compared against main directly because main does not compile; the comparison is against the unbreak commit alone, which I will paste here when that run finishes.

…nto the history endpoint

module/ownership_chain/pagination.rs was never declared as a module, so paginate()
and Page were not compiled into the service at all, and nothing called them: GET
/transfer/:document_hash returned the whole chain however long it was.

* declare the module;
* harden paginate() while switching it on: a cursor past the end of the chain now
  yields an empty page instead of panicking on items[cursor..end], and a page_size
  of 0 is treated as 1 so a page always makes progress;
* add seven unit tests for it (first/middle/exact-fit/last page, cursor at and past
  the end, zero page size, empty input) - the file had none;
* wire cursor/page_size query parameters into the live handler with a
  ChainHistoryPage response (items, next_cursor, total), DEFAULT_CHAIN_PAGE_SIZE 20
  capped at MAX_CHAIN_PAGE_SIZE 200;
* add five integration tests against the real router.

Calling /transfer/:hash with neither parameter still returns the bare array, so
existing clients are unaffected; passing either opts into the paged envelope.
…run any test)

Three independent breakages on main, unrelated to CodeGirlsInc#1346 but blocking it:

* Cargo.toml pinned chrono without its serde feature, so Event's
  DateTime<Utc> fields had no Serialize/Deserialize impl (5 errors in event.rs);
* AppState derives Clone and holds a governor RateLimiter, which is not Clone
  (E0277 at lib.rs:78) - the limiter is now Arc-wrapped behind the existing
  DefaultRateLimiter alias, so every build_rate_limiter(...) call site is unchanged;
* src/stellar.rs's mock test moves data_key into the json! closure and then
  asserts on it (E0382).

None of these are new; without them cargo test cannot even build the lib target,
for any toolchain that can parse the lock.
…endpoint

The live router never mounts GET /transfer/:document_hash, so paginating that
handler changed nothing reachable. Move the paging onto /verify/:hash/history,
which the router does mount and which returns the same chain.

- cursor/page_size are opt-in: without params the response is byte-identical to
  before (full chain, next_cursor: null), so no existing consumer breaks.
- next_cursor is added to HistoryResponse; count stays the chain total.
- ChainHistoryPage dropped (dead while the transfer handler was reverted).
- Cargo.toml: tower gains the util feature - the test target imports
  tower::util::ServiceExt, and without it no integration test could compile.
- Integration tests now drive the mounted endpoint: full chain, first page,
  last page, cursor past the end (200 + empty, not 500), page_size cap.
@vercel

vercel Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

@jarik2014 is attempting to deploy a commit to the Mftee's projects Team on Vercel.

A member of the Team first needs to authorize it.

@jarik2014

Copy link
Copy Markdown
Author

Baseline comparison, as promised — this is the "not mine" arm.

Unbreak commit alone, with the pagination wiring reverted (git revert cd5a7df), same toolchain, same target dir:

running 146 tests
test result: FAILED. 135 passed; 11 failed

This branch:

running 153 tests
test result: FAILED. 142 passed; 11 failed

The seven extra tests are the new pagination::tests, all passing, and the failing names are identical in both arms: 2 × hash_validator::tests::*_rejects_non_ascii_unicode_and_binary_characters, 6 × stellar::tests::* (Invalid secret key: InvalidStrKeyChecksum in the mock, plus the Horizon 404/500 mocks) and 2 × webhook::tests::*. Nothing in this diff touches those modules — they are broken on the unbreak-only tree already, and they need a real keypair/test fixture to fix, which is separate work.

@mftee mftee closed this Sep 26, 2026
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.

Ownership chain lookups are not paginated

2 participants