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

bitcoin finality request+query efficiency #346

Closed
wants to merge 6 commits into from

Conversation

ClaytonNorthey92
Copy link
Contributor

Summary

  • make "recent finalities" call "finalities by keystone" with recent keystones
  • remove expensive joins, prefer additional (fast) queries

Changes
see summary

@github-actions github-actions bot added the area: bfg This is a change to BFG (Bitcoin Finality Governor) label Dec 18, 2024
@ClaytonNorthey92 ClaytonNorthey92 marked this pull request as ready for review December 18, 2024 03:19
@ClaytonNorthey92 ClaytonNorthey92 requested a review from a team as a code owner December 18, 2024 03:19
Copy link
Contributor

@marcopeereboom marcopeereboom left a comment

Choose a reason for hiding this comment

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

Since this is a band aid i don't think this makes it worse :-)

@ClaytonNorthey92 ClaytonNorthey92 force-pushed the clayton/finality-efficiency branch 2 times, most recently from 64912ff to eef7536 Compare December 20, 2024 01:08
@ClaytonNorthey92 ClaytonNorthey92 marked this pull request as draft December 20, 2024 01:45
service/bfg/bfg.go Outdated Show resolved Hide resolved
SELECT` +
// DISTINCT ON returns the first result for each unique l2 block number
`
DISTINCT ON (l2_block_number)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is still a bit inefficient

@ClaytonNorthey92
Copy link
Contributor Author

I have to give this a bit more thought; don't merge quite yet

@ClaytonNorthey92 ClaytonNorthey92 force-pushed the clayton/finality-efficiency branch from fa48c63 to fba541c Compare December 23, 2024 21:46
@ClaytonNorthey92 ClaytonNorthey92 marked this pull request as ready for review December 23, 2024 21:58
* make "recent finalities" call "finalities by keystone" with recent keystones
* remove expensive joins, prefer additional (fast) queries
@ClaytonNorthey92 ClaytonNorthey92 force-pushed the clayton/finality-efficiency branch from 32f5e53 to 32ebf8e Compare December 23, 2024 22:04
@ClaytonNorthey92 ClaytonNorthey92 marked this pull request as draft December 26, 2024 16:01
@ClaytonNorthey92
Copy link
Contributor Author

still doing some testing before merge 🧑‍💻

@ClaytonNorthey92
Copy link
Contributor Author

after some discussion with @max-sanchez , going with a different approach here. the functionality will be quite different and require new reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: bfg This is a change to BFG (Bitcoin Finality Governor)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants