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

[MEVBlocker] cleanup after fee increase to 30% #133

Merged
merged 2 commits into from
Feb 10, 2025

Conversation

fleupold
Copy link
Contributor

@fleupold fleupold commented Feb 8, 2025

This PR is a follow up to #132, it

  1. Removes the temporary fee reduction (as last week we were still charging a 20% fee) for billing purposes so that as of next week we not only announce but also charge the new fee
  2. Includes Titan's table for which transaction hashes were not exclusive to MEV Blocker. Those should be excluded for billing purposes.

Note, that this PR needs to be merged before next Wednesday to take effect in the payouts.

Summary by CodeRabbit

  • Bug Fixes
    • Adjusted fee calculations so fees now reflect the complete average value, ensuring weekly fees and amounts due are accurate.
    • Refined the processing of transaction data to more precisely distinguish exclusive transactions, improving the accuracy of reported results.
    • Updated filtering logic for user transactions to include a broader scope of transactions in the analysis.
    • Standardized data type usage across the script for consistency.
    • Renamed variables for clarity without altering functionality in calculations.

Copy link

coderabbitai bot commented Feb 8, 2025

Walkthrough

This pull request updates three SQL queries across different modules. In the payment_due query, the average block fee calculation is modified to use the full avg_block_fee_wei value instead of a reduced fraction. The value_per_tx query replaces the mempool CTE with nonexclusive_flow, expanding the scope of transactions considered and updating the filtering condition accordingly. Lastly, in the balancer query, the lp_token_price CTE is renamed to lpt_price, while maintaining the same functionality. The overall structure of the queries remains unchanged.

Changes

File(s) Change Summary
mevblocker/fees/...4005800.sql Modified the fee calculation by removing the division by 3 from avg_block_fee_wei, so weekly_fee and amount_due_wei now use the complete average block fee value.
mevblocker/fees/...4188777.sql Replaced the mempool CTE with nonexclusive_flow to include additional transaction data, updated the user_txs filtering condition, and standardized data types to uint256.
cowamm/profitability/...balancer_4106553.sql Renamed the CTE from lp_token_price to lpt_price, maintaining the same logic for calculating LP token price based on total LP supply and TVL.

Poem

I'm a rabbit in a SQL field so bright,
Hopping through queries with pure delight.
Fees now flow full, no fractions to hide,
And transactions sorted with a rabbit's pride.
Code bounds hop, crisp and clear—hop on, my dear! 🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9c03ed and 191d162.

📒 Files selected for processing (1)
  • cowamm/profitability/10k_growth/balancer_4106553.sql (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • cowamm/profitability/10k_growth/balancer_4106553.sql

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (8)
mevblocker/fees/value_per_tx_4188777.sql (8)

16-17: Correct Spelling in Comment
The comment currently reads “perfomance optimisation” – please fix the typo to “performance optimisation” for clarity.


17-25: Union Clause and CAST Keyword Consistency
Within the nonexclusive_flow CTE, the UNION operator is used without a qualifier. To meet style guidelines, explicitly use “UNION DISTINCT” (since duplicates are being removed). Additionally, update the expression from
  cast(hash as varchar)
to
  CAST(hash AS varchar)
to ensure consistent casing of SQL keywords.

🧰 Tools
🪛 GitHub Actions: Lint

[warning] 23-23: 'UNION [DISTINCT|ALL]' is preferred over just 'UNION'.


[warning] 24-24: Function names must be consistently upper case.


[warning] 24-24: Keywords must be consistently upper case.


147-147: Updated Filter in user_txs CTE
The added condition now excludes transactions found in nonexclusive_flow. Please update “CAST(tx.hash AS VARCHAR)” to “CAST(tx.hash AS varchar)” so that the datatype is lowercased per the style guidelines.

🧰 Tools
🪛 GitHub Actions: Lint

[warning] 147-147: Datatypes must be consistently lower case.


61-62: Consistent Data Type Casing in CAST Function
In the mev_blocker_tx CTE, the expressions
  CAST(JSON_EXTRACT(mb.transactions, '$[0].hash') AS VARCHAR)
and
  CAST(JSON_EXTRACT(mb.transactions, '$[0].from') AS VARCHAR)
should be updated to use “varchar” (i.e. AS varchar) to satisfy the requirement that datatypes be lowercase.

🧰 Tools
🪛 GitHub Actions: Lint

[warning] 61-61: Datatypes must be consistently lower case.


[warning] 62-62: Datatypes must be consistently lower case.


70-71: Standardize Datatype Casing in JSON_TABLE
In the JSON_TABLE definition, change “VARCHAR(255)” to “varchar(255)” so that datatypes are consistently in lower case throughout the query.

🧰 Tools
🪛 GitHub Actions: Lint

[warning] 71-71: Datatypes must be consistently lower case.


120-120: Lowercase Data Type in CAST Expression (kickback_txs)
Please modify “CAST(et.gas_used AS UINT256)” to “CAST(et.gas_used AS uint256)” to comply with the requirement for datatypes to be lower case.

🧰 Tools
🪛 GitHub Actions: Lint

[warning] 120-120: Datatypes must be consistently lower case.


142-142: Standardize CAST Data Type Casing (user_txs)
Update “CAST(tx.gas_used AS UINT256)” to “CAST(tx.gas_used AS uint256)” so that datatype names follow the lowercasing guideline.

🧰 Tools
🪛 GitHub Actions: Lint

[warning] 142-142: Datatypes must be consistently lower case.


162-162: Consistent Data Type Casing in Fee Calculation
In the final fee calculation, convert “CAST(0.3 * (...) AS UINT256)” to “CAST(0.3 * (...) AS uint256)” so that the data type is in lower case, as required by the style rules.

🧰 Tools
🪛 GitHub Actions: Lint

[warning] 162-162: Datatypes must be consistently lower case.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb8992b and 62371a0.

📒 Files selected for processing (2)
  • mevblocker/fees/payment_due_4005800.sql (1 hunks)
  • mevblocker/fees/value_per_tx_4188777.sql (3 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint
mevblocker/fees/value_per_tx_4188777.sql

[warning] 23-23: 'UNION [DISTINCT|ALL]' is preferred over just 'UNION'.


[warning] 24-24: Function names must be consistently upper case.


[warning] 24-24: Keywords must be consistently upper case.


[warning] 61-61: Datatypes must be consistently lower case.


[warning] 62-62: Datatypes must be consistently lower case.


[warning] 71-71: Datatypes must be consistently lower case.


[warning] 120-120: Datatypes must be consistently lower case.


[warning] 142-142: Datatypes must be consistently lower case.


[warning] 147-147: Datatypes must be consistently lower case.


[warning] 162-162: Datatypes must be consistently lower case.

🔇 Additional comments (2)
mevblocker/fees/payment_due_4005800.sql (2)

17-20: Update in final_fee CTE – Full Fee Utilization
The temporary fee reduction has been removed by selecting the full avg_block_fee_wei value. Please verify that downstream calculations (weekly_fee, amount_due_wei, and amount_due_eth) are now correctly aligned with the new fee structure changes.


39-45: Final SELECT Calculation Verification
The final SELECT clause multiplies the full avg_block_fee_wei to compute weekly_fee and amount_due values. Double-check that applying the complete fee (rather than a reduced fraction) meets the intended business logic for the upcoming 30% fee increase.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
mevblocker/fees/value_per_tx_4188777.sql (1)

162-162: Adoption of the New 30% Fee Structure
The fee calculation has been updated to incorporate a hard-coded 0.3 multiplier, replacing the previous fee reduction. This adjustment meets the PR objective for the fee increase. As a recommendation for future maintainability, consider externalizing this fee rate into a configuration parameter so that further adjustments can be made with minimal code changes. Also, verify that the divisor (9) within the formula remains logically consistent under the new fee regime.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62371a0 and f9c03ed.

📒 Files selected for processing (1)
  • mevblocker/fees/value_per_tx_4188777.sql (6 hunks)
🔇 Additional comments (5)
mevblocker/fees/value_per_tx_4188777.sql (5)

16-28: Broadened Transaction Scope via nonexclusive_flow CTE
The updated CTE now aggregates non-exclusive transactions from both the flashbots dataset and the mev_blocker_non_exclusive_txs table using a UNION DISTINCT. This change not only expands the inclusion criteria but also standardizes the casting by using varchar. Please verify that the expanded transaction set aligns correctly with downstream filters and performance expectations.


61-62: Validation of JSON Extraction for Transaction Hashes
The modifications using FROM_HEX wrapped around JSON_EXTRACT (for extracting tx_1 and tx_from_1) appear correctly implemented. Ensure that the JSON structure is consistently formatted so that these extractions do not inadvertently fail during runtime.


120-120: Consistent Data Type Casting in Kickback Transactions
The casting of et.gas_used as uint256 in the kickback_txs CTE standardizes the data type and ensures consistency with subsequent arithmetic operations. This change is appropriate provided that all related calculations expect the lower-case uint256 format.


141-142: Standardized Casting in User Transactions Calculation
The expression for user_tip_wei now uses CAST(tx.gas_used AS uint256), which harmonizes the data type usage across the script. Confirm that this update does not lead to any unintended side effects in the fee calculations elsewhere.


146-147: Updated Filter Condition Utilizing nonexclusive_flow
Changing the filtering condition to use "CAST(tx.hash AS varchar) NOT IN (SELECT hash FROM nonexclusive_flow)" properly excludes transactions flagged as non-exclusive. Please verify that this logic aligns with overall transaction inclusion criteria and no valid user transactions are mistakenly filtered out.

@PoloX2021
Copy link
Contributor

The query looks good.
Should we stick to capital letters or not? The linting commits seem to be doing both.

@alex-zadorozhnyi
Copy link
Contributor

Looks good to me

@fleupold fleupold force-pushed the mevblocker/fee_increase_30_cleanup branch from 191d162 to f9c03ed Compare February 10, 2025 15:34
@fleupold fleupold merged commit 8ddb9ef into main Feb 10, 2025
4 of 5 checks passed
@fleupold fleupold deleted the mevblocker/fee_increase_30_cleanup branch February 10, 2025 15:35
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.

3 participants