Skip to content

No test pins the behavior of get_history_page and get_history_page_with_meta against each other: the documented equivalence (identical items) is unasserted #503

Description

@usmanimamu17-create

Problem

get_history_page_with_meta (src/lib.rs/src/history.rs) promises: "The items slice is identical to what get_history_page returns for the same (offset, limit)". The two accessors are implemented separately (companion issue on the duplication), and the test suite does not contain a direct comparison test — the pagination tests exercise each accessor's own contract but not the equivalence the docstring promises.

Consequences:

  • The equivalence guarantee is unverified: if the two implementations diverge (e.g. one clamps limit differently — the saturating logic is duplicated), consumers that switch between the two accessors (the meta variant exists so consumers can avoid a second call) get different items for the same arguments with no test catching it.
  • The docstring is the only spec: the "identical items" claim lives in prose; a future refactor of one accessor can silently break it.
  • The total/has_more fields compound the risk: has_more is computed from the same end used for slicing, so a slicing bug corrupts the pagination metadata too — and nothing cross-checks has_more against a manual count.

Root cause

The two accessors were written together but tested separately; the equivalence invariant was documented, not asserted.

Why this is architecturally hard

  1. The direct fix is a parametrized test (for each offset/limit pair, assert items equality and has_more correctness against a known history) — mechanically simple, but it must be written against the policy (issue Create a policy note for history pagination size and offset safety when consumers rely on page-based lookups #263) not the current implementation, so the test survives the storage redesign (companion issue).
  2. The equivalence is only meaningful with a defined history state; the test must build a history of known length and iterate a range of offsets/limits including the edge cases (offset >= len, limit 0, limit > remaining).
  3. This test is a prerequisite for the de-duplication refactor (companion issue): consolidating the two implementations is only safe if the equivalence test exists first.

Acceptance criteria

  • A test asserts get_history_page_with_meta(offset, limit).items == get_history_page(offset, limit) across a matrix of offsets/limits on a fixed history.
  • The test covers edge cases (empty history, offset beyond end, limit 0, limit exceeding remaining).
  • has_more is asserted against a manual count for the same matrix.

Out of scope

The pagination de-duplication (companion issue) and the storage redesign (companion issue).

Getting started

just test

Good first files to read: apexchainx_calculator/src/lib.rs (get_history_page, get_history_page_with_meta), docs/HISTORY_PAGINATION_POLICY.md.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardStellar WaveIssues in the Stellar wave programThird CampaignCampaign: Third Campaignarea/historyImported campaign issue labelpriority/mediumImported campaign issue label

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions