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

Improve builder profit estimation by taking bid adjustments into account #55

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

ababino
Copy link
Contributor

@ababino ababino commented Oct 8, 2024

📝 Summary

  1. Added a new script cmd/core/bid-adjustments-backfill.go to fetch bid adjustments from ultrasound relay.
  2. Amended builder profit query in GetBuilderProfits in database/database.go. Now, if the slot is a bid-adjusted-slot, the query will subtract the validator profit from coinbase_diff.

⛱ Motivation and Context

Builder profit is not accurate when bids are adjusted by ultrasound relay.


✅ I have run these commands

  • make lint
  • make test
  • go mod tidy

@ababino ababino requested a review from metachris October 8, 2024 20:13
round(PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY CASE WHEN coinbase_diff_eth IS NOT NULL THEN coinbase_diff_eth ELSE 0 END), 4) as median_profit_per_block,
round(sum(CASE WHEN coinbase_diff_eth IS NOT NULL THEN coinbase_diff_eth ELSE 0 END), 4) as total_profit,
round(abs(sum(CASE WHEN coinbase_diff_eth < 0 THEN coinbase_diff_eth ELSE 0 END)), 4) as total_subsidies
FROM adjusted_payloads
Copy link
Contributor

Choose a reason for hiding this comment

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

This query worries me a little bit -- how much more expensive is it to run? Not sure if this could be a bottleneck. Do you have any data or estimates on query performance?

@ababino
Copy link
Contributor Author

ababino commented Oct 25, 2024

@metachris I made some changes:

  1. I optimized the new query by adding a better index
  2. I added a backfill_adjustments script
  3. I added the new commands to the README file

Regarding the performance of the new query, I ran it with 1 day of data, and these are the stats (I used EXPLAIN ANALYZE):
Old query:
Execution Time: 27.580 ms
Peak Memory Usage: 913kB

New query:
Execution Time: 59.042 ms
Peak Memory Usage: 1681kB

So, we are talking about 2x slower and 2x more memory. I don't know what our hardware requirements are. If we need more performance, we'd have to change the way we store the data, so we move the computation from query time to data fetching time.

Let me know how we should proceed.

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.

2 participants