fix(l2): Fix EIP-4844 blob fee bump to use percentage scale #5415
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in the EIP-4844 blob fee bump calculation that was causing blob transaction fees to decrease instead of increase during retries. The fix aligns the blob fee bump formula with the existing EIP-1559 fee bump strategy.
- Corrects the
max_fee_per_blob_gasbump formula to multiply by(100 + bump_percentage)/100instead of the incorrect division-based calculation - Ensures consistency across all fee bump operations (EIP-1559 and EIP-4844)
- Prevents underpriced blob transaction retries that would violate replacement rules
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Oppen
reviewed
Nov 26, 2025
Comment on lines
815
to
+819
| if let Some(max_fee_per_blob_gas) = &mut tx.max_fee_per_blob_gas { | ||
| let factor = 1 + (bump_percentage / 100) * 10; | ||
| let factor = 100 + bump_percentage; | ||
| *max_fee_per_blob_gas = max_fee_per_blob_gas | ||
| .saturating_mul(U256::from(factor)) | ||
| .div(10); | ||
| .div(100); |
Contributor
There was a problem hiding this comment.
To keep the same semantics without the integer truncation issue what you actually need is:
Suggested change
| if let Some(max_fee_per_blob_gas) = &mut tx.max_fee_per_blob_gas { | |
| let factor = 1 + (bump_percentage / 100) * 10; | |
| let factor = 100 + bump_percentage; | |
| *max_fee_per_blob_gas = max_fee_per_blob_gas | |
| .saturating_mul(U256::from(factor)) | |
| .div(10); | |
| .div(100); | |
| if let Some(max_fee_per_blob_gas) = &mut tx.max_fee_per_blob_gas { | |
| let factor = 100 + bump_percentage * 10; | |
| *max_fee_per_blob_gas = max_fee_per_blob_gas | |
| .saturating_mul(U256::from(factor)) | |
| .div(1000); |
I don't know why the magic multiply and divide by 10 was there, but if there was a reason you'll want to keep it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The bump formula max_fee_per_blob_gas in crates/l2/sdk/src/sdk.rs::bump_gas_generic_tx() is incorrect and decreases the value instead of increasing it.
Description
Adjust the bump logic for EIP-4844 max_fee_per_blob_gas in bump_gas_generic_tx() to increase by (100 + bump_percentage)/100, mirroring the existing EIP-1559 fee bump strategy. The previous implementation used a factor with integer division and an extra division by 10, which effectively reduced the blob fee (e.g., with bump_percentage=30 it divided by 10) and conflicted with replacement rules and inclusion probabilities for blob transactions. This change aligns behavior with how max_fee_per_gas and max_priority_fee_per_gas are bumped, respects client-side clamping via maximum_allowed_max_fee_per_blob_gas, and prevents underpriced retries for blob transactions.