-
Notifications
You must be signed in to change notification settings - Fork 123
feat: tx type filter #2330
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
base: develop
Are you sure you want to change the base?
feat: tx type filter #2330
Conversation
Vercel deployment URL: https://stacks-blockchain-d4cix9gjs-hirosystems.vercel.app 🚀 |
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
379f278
to
d983942
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very good @He1DAr but we need to run some load tests on this before we can merge. Let's discuss how we can do this.
exports.up = pgm => { | ||
pgm.createIndex('txs', 'status', { | ||
where: 'canonical = TRUE AND microblock_canonical = TRUE', | ||
name: 'idx_txs_status_optimized' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to provide a name, node-pg-migrate
will define one for you with the current standards we use
exports.shorthands = undefined; | ||
|
||
exports.up = pgm => { | ||
pgm.createIndex('txs', 'status', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the index is great but I don't think it will help a lot especially for the success
status, given that the vast majority of transactions are successful.
I'm worried that running the endpoint on mainnet will cause performance issues when it tries to calculate the COUNT(*)
of records to show in the total
. We'll need to run load test on this.
Description
We're planning to support filtering by tx type in Stacks explorer search. This PR aims to add a new filter
type
to allow user filtering txs by type.Type of Change
Checklist
npm run test
passes