Skip to content

fix: GetBlocks pagination off-by-one returning PerPage+1 results#330

Open
floflo777 wants to merge 1 commit intocanopy-network:mainfrom
floflo777:fix/getblocks-pagination-off-by-one
Open

fix: GetBlocks pagination off-by-one returning PerPage+1 results#330
floflo777 wants to merge 1 commit intocanopy-network:mainfrom
floflo777:fix/getblocks-pagination-off-by-one

Conversation

@floflo777
Copy link

Summary

Fixes #197

GetBlocks was returning PerPage + 1 results instead of PerPage. The root cause is that page.PerPage += 1 inside the Load() callback mutates the same PerPage field that Load() uses to control iteration count, causing it to iterate one extra time beyond the requested page size. This also corrupts Count and TotalPages in the response.

Changes

  • Create a separate PageParams with PerPage + 1 before calling Load(), so the iteration limit is set upfront without mutation inside the callback
  • Use the original p.PerPage as the guard for results = append(results, block) so exactly PerPage blocks are returned
  • Remove the page.PerPage += 1 mutation from inside the callback
  • Recompute page.Count and page.TotalPages after Load() using the original PerPage value

Before

Request: PerPage=10
Response: Count=11, TotalPages=wrong

After

Request: PerPage=10
Response: Count=10, TotalPages=correct

The extra block is still fetched (needed to compute Took duration for the last visible block), but it is no longer included in the results or pagination metadata.

Test plan

  • go build ./store/... passes
  • go test ./store/... -v all 33 tests pass

The GetBlocks callback was mutating page.PerPage inside the closure
(page.PerPage += 1) to fetch one extra block for computing the Took
duration metadata. This caused Load() to iterate PerPage+1 times
instead of PerPage, corrupting Count and TotalPages in the response.

Instead, create a separate PageParams with PerPage+1 before calling
Load(), and use the original p.PerPage for the results guard and
to restore correct pagination metadata afterwards.

Fixes canopy-network#197
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.

[BUG] GetBlocks pageParams adds 1 additional item

1 participant