Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[R4R] #262 Fix tx search pagination #19

Merged
merged 1 commit into from
Nov 27, 2018
Merged

Conversation

ackratos
Copy link
Contributor

@ackratos ackratos commented Nov 26, 2018

Related issue: bnb-chain/node#262

tendermint change: bnb-chain/bnc-tendermint#12

tested with http://127.0.0.1:8080/txs?tag=tx.height=303&page=0&perPage=10000

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
  • Updated all relevant documentation (docs/)
  • Updated all relevant code comments
  • Wrote tests
  • Added entries in PENDING.md that include links to the relevant issue or PR that most accurately describes the change.
  • Updated cmd/gaia and examples/

For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@ackratos ackratos added the bug Something isn't working label Nov 26, 2018
@ackratos ackratos self-assigned this Nov 26, 2018
@ackratos ackratos changed the title #262 Fix tx search pagination [WIP] #262 Fix tx search pagination Nov 26, 2018
@ackratos ackratos force-pushed the upgrade25_pagination branch from 91cbd5b to a47db78 Compare November 26, 2018 11:52
@ackratos ackratos changed the title [WIP] #262 Fix tx search pagination [R4R] #262 Fix tx search pagination Nov 26, 2018
@ackratos ackratos requested a review from darren-liu November 26, 2018 12:51
w.Write([]byte("page parameter is not a valid integer"))
return
}

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. we should reject negative value
  2. how should we handle if 2 31-digit number cannot hold all the txs, especially max per page is just 10000?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2. how should we handle if 2 31-digit number cannot hold all the txs, especially max per page is just 10000?

Then the API user should pass page from 0 to 2^32 / 10000 and perPage always 10000 to get all txs.

Our usage is for explorer query txs in every block (by tag tx.height), which won't exceed 4000 (currently 1000) as our performance limitation.

@ackratos ackratos force-pushed the upgrade25_pagination branch from a47db78 to f746c39 Compare November 27, 2018 05:47
@ackratos ackratos merged commit 769777e into upgrade25 Nov 27, 2018
@forcodedancing forcodedancing deleted the upgrade25_pagination branch May 18, 2022 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants