submitter: add blob config set for blob fee calc #798
Conversation
WalkthroughA new blob fee configuration subsystem is introduced with support for multiple Ethereum forks and per-chain settings. The Rollup service integrates ChainConfigMap to dynamically compute blob fees based on fork timings and chain identifiers using a priority-based denominator resolution mechanism. Changes
Sequence DiagramsequenceDiagram
participant Rollup
participant ChainConfigMap
participant GetBlobFeeDenominator
participant BlobFeeConfig
Rollup->>Rollup: GetGasTipAndCap() called
Rollup->>ChainConfigMap: Lookup ChainId in map
ChainConfigMap-->>Rollup: Return BlobFeeConfig (or DefaultBlobConfig)
Rollup->>GetBlobFeeDenominator: Call with BlobFeeConfig and block time
GetBlobFeeDenominator->>BlobFeeConfig: Check fork timings in priority order<br/>(BPO5 → BPO1 → Cancun)
BlobFeeConfig-->>GetBlobFeeDenominator: Return matching BlobConfig UpdateFraction
GetBlobFeeDenominator-->>Rollup: Return denominator (or default + warning log)
Rollup->>Rollup: Calculate blobFee = ExcessBlobGas / denominator<br/>Apply 3× multiplier for congestion
Rollup-->>Rollup: Return gasTip and gasCap with adjusted blob fee
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tx-submitter/services/rollup.go (1)
1911-1926: Add iteration limit to prevent potential infinite loop.While the Taylor series should converge as
accumis divided byieach iteration, consider adding a maximum iteration limit as a safety measure against edge cases with extreme inputs or integer overflow scenarios.Apply this diff to add a safety limit:
func fakeExponential(factor, numerator, denominator *big.Int) *big.Int { var ( output = new(big.Int) accum = new(big.Int).Mul(factor, denominator) ) - for i := 1; accum.Sign() > 0; i++ { + const maxIterations = 100 + for i := 1; accum.Sign() > 0 && i < maxIterations; i++ { output.Add(output, accum) accum.Mul(accum, numerator) accum.Div(accum, denominator) accum.Div(accum, big.NewInt(int64(i))) } return output.Div(output, denominator) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
go-ethereum(1 hunks)tx-submitter/services/rollup.go(5 hunks)tx-submitter/types/blob_config.go(1 hunks)tx-submitter/types/blob_params.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tx-submitter/types/blob_params.go (1)
tx-submitter/types/blob_config.go (2)
BlobFeeConfig(10-37)BlobConfig(40-43)
tx-submitter/services/rollup.go (2)
tx-submitter/types/blob_params.go (3)
ChainConfigMap(12-15)ChainBlobConfigs(20-20)MinBlobGasPrice(10-10)tx-submitter/types/blob_config.go (1)
GetBlobFeeDenominator(92-136)
tx-submitter/types/blob_config.go (1)
tx-submitter/types/blob_params.go (1)
DefaultOsakaBlobConfig(62-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
tx-submitter/services/rollup.go (2)
85-85: LGTM: Clean integration of chain-specific blob configuration.The new
ChainConfigMapfield is properly declared and initialized, enabling per-chain blob fee calculation.Also applies to: 130-130
1284-1290: The suggested logging already exists in GetBlobFeeDenominator.The review comment recommends adding a warning log when a chain is not found in ChainConfigMap. However, this is already implemented: when
blobConfigis nil, theGetBlobFeeDenominatorfunction (blob_config.go, lines 93-97) logs"BlobFeeConfig not set, using default denominator"and safely falls back toDefaultOsakaBlobConfig. No changes needed.Likely an incorrect or invalid review comment.
tx-submitter/types/blob_params.go (4)
9-16: LGTM: Clean initialization of blob fee parameters.The global variables are properly initialized, establishing per-chain blob configurations for Ethereum mainnet and Hoodi testnet.
35-49: LGTM: Hoodi testnet configuration is well-structured.The configuration properly defines fork timings and blob configs for the Hoodi testnet with appropriate defaults.
24-32: Configuration verified as correct.The
PragueTimetimestamp of1746612311is accurate and matches the Prague (Pectra) mainnet activation at May 7, 2025 10:05:11 UTC. Since this fork has already occurred, the configuration is up-to-date and requires no changes.
52-81: BPO4 UpdateFraction value and unused fork configurations require verification.BPO1, BPO2, and BPO3 values match official Ethereum test fork documentation, but BPO4 (13739630) does not appear in the official specification. Additionally, BPO4 and BPO3 are not currently assigned to any chain configuration in the codebase—they are defined but unused. The BPO4 value remains lower than BPO3 (20609697), breaking the expected UpdateFraction progression. Clarify whether BPO4 is an upcoming fork requiring a corrected value, experimental code to be removed, or incomplete implementation; the same applies to BPO3.
tx-submitter/types/blob_config.go (4)
9-43: LGTM: Well-structured configuration types.The
BlobFeeConfigandBlobConfigstructs provide a clean, extensible foundation for chain-aware blob fee calculations with clear field naming and documentation.
45-89: LGTM: Consistent fork detection logic.The fork detection methods follow a uniform pattern, correctly checking that the London fork has been reached before evaluating timestamp-based forks. This aligns with Ethereum's fork activation mechanism.
91-136: LGTM: Robust fork-aware denominator selection.The function correctly prioritizes forks from newest (BPO5) to oldest (Cancun), handles nil configurations gracefully, and provides helpful logging. The triple-check pattern (time != nil, fork active, config != nil) ensures safety.
138-156: LGTM: Clean helper functions with proper nil handling.The fork checking helpers are simple, correct, and safely handle nil inputs. The explicit separation of block and timestamp variants improves code clarity.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tx-submitter/types/blob_config_test.go (1)
167-233: LGTM! Critical priority logic validated.The ChainConfigMap test validates lookup mechanics while the ForkPriority test ensures correct fork selection order—both add valuable coverage.
Consider adding test cases for scenarios where fork times are configured but corresponding BlobConfig is nil, to verify the fallback behavior handles partial configurations gracefully.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tx-submitter/types/blob_config_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tx-submitter/types/blob_config_test.go (2)
tx-submitter/types/blob_params.go (8)
MainnetChainConfig(24-32)DefaultOsakaBlobConfig(62-64)DefaultCancunBlobConfig(54-56)DefaultPragueBlobConfig(58-60)HoodiChainConfig(35-49)DefaultBPO1BlobConfig(66-68)DefaultBPO2BlobConfig(70-72)ChainConfigMap(12-15)tx-submitter/types/blob_config.go (2)
GetBlobFeeDenominator(92-136)BlobFeeConfig(10-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
tx-submitter/types/blob_config_test.go (5)
1-8: LGTM!Package declaration and imports are appropriate for the test suite.
10-60: Excellent test coverage for Mainnet fork transitions!The test cases correctly validate fork boundary behavior across Cancun and Prague, with accurate timestamps and expected results.
62-142: Comprehensive Hoodi chain fork testing!Excellent coverage of the complete fork timeline from Cancun through BPO2, with thorough boundary testing at each transition.
144-150: LGTM!Correctly validates the nil config fallback to DefaultOsakaBlobConfig.
152-165: LGTM!Good sanity check that validates all chain configs with a far-future timestamp, ensuring basic invariants hold.
Kukoomomo
force-pushed
the
submitter/blobfee
branch
from
6292a01 to
8a31f2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tx-submitter/types/blob_config.go (1)
91-136: Ensure LondonBlock is validated before use.The function correctly implements fork-priority selection, but
londonBlock(line 101) is used directly in fork checks without verifying it's non-nil. WhileIsLondonhandles nil internally, consider adding an explicit check or log warning ifLondonBlockis nil for better diagnostics.Apply this diff to add defensive nil check:
cfg := blobFeeConfig londonBlock := cfg.LondonBlock // London block number for fork determination. + if londonBlock == nil { + log.Warn("LondonBlock not set in BlobFeeConfig, fork detection may not work correctly", + "chainID", cfg.ChainID) + }tx-submitter/types/blob_params.go (1)
7-14: DefaultBlobConfig points to HoodiChainConfig.The
DefaultBlobConfigis set toHoodiChainConfig(testnet), which will be used as the fallback when a chain ID is not found inChainConfigMap. This might be unexpected for production deployments. Consider whether this should default toMainnetChainConfigor a separate explicit default configuration.Consider creating an explicit default configuration instead of reusing HoodiChainConfig:
var ( - DefaultBlobConfig = HoodiChainConfig + DefaultBlobConfig = &BlobFeeConfig{ + ChainID: big.NewInt(0), + LondonBlock: big.NewInt(0), + Default: DefaultOsakaBlobConfig, + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.work.sumis excluded by!**/*.sumtx-submitter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
tx-submitter/go.mod(1 hunks)tx-submitter/services/rollup.go(4 hunks)tx-submitter/types/blob_config.go(1 hunks)tx-submitter/types/blob_config_test.go(1 hunks)tx-submitter/types/blob_params.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
tx-submitter/types/blob_config_test.go (2)
tx-submitter/types/blob_params.go (8)
MainnetChainConfig(22-30)DefaultOsakaBlobConfig(60-62)DefaultCancunBlobConfig(52-54)DefaultPragueBlobConfig(56-58)HoodiChainConfig(33-47)DefaultBPO1BlobConfig(64-66)DefaultBPO2BlobConfig(68-70)ChainConfigMap(10-13)tx-submitter/types/blob_config.go (2)
GetBlobFeeDenominator(92-136)BlobFeeConfig(10-37)
tx-submitter/services/rollup.go (2)
tx-submitter/types/blob_params.go (3)
ChainConfigMap(10-13)ChainBlobConfigs(18-18)DefaultBlobConfig(8-8)tx-submitter/types/blob_config.go (1)
GetBlobFeeDenominator(92-136)
tx-submitter/types/blob_params.go (1)
tx-submitter/types/blob_config.go (2)
BlobFeeConfig(10-37)BlobConfig(40-43)
tx-submitter/types/blob_config.go (2)
ops/l2-genesis/morph-chain-ops/deployer/deployer.go (1)
ChainID(22-22)tx-submitter/types/blob_params.go (1)
DefaultOsakaBlobConfig(60-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: check
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (11)
tx-submitter/types/blob_config_test.go (1)
1-233: Excellent test coverage!The test suite provides comprehensive coverage of the blob fee denominator logic:
- Fork-based denominator selection across multiple chains
- Boundary conditions for fork activation times
- Nil config handling with default fallbacks
- Fork priority ordering validation
The tests are well-structured and clearly document expected behavior.
tx-submitter/types/blob_config.go (3)
9-43: Well-designed configuration types.The
BlobFeeConfigandBlobConfigtypes provide a clean, extensible structure for per-chain and per-fork blob fee configuration. The design allows for future fork additions (BPO3-5) without breaking changes.
45-84: Fork detection methods follow consistent pattern.All fork detection methods correctly verify the London fork activation before checking timestamp-based forks. This ensures proper fork ordering and prevents activation of later forks if London hasn't activated.
138-156: Helper functions handle nil cases correctly.The
isBlockForkedandisTimestampForkedhelpers properly handle nil parameters and implement the standard fork activation logic.tx-submitter/services/rollup.go (3)
85-87: ChainConfigMap field added correctly.The new
ChainConfigMapfield is properly added to theRollupstruct to support per-chain blob configuration lookups.
131-131: ChainConfigMap properly initialized.The field is correctly initialized with
types.ChainConfigMapin the constructor, ensuring the rollup service has access to per-chain blob configurations.
304-304: Loop refactoring maintains equivalent behavior.The change from
for i := range 3tofor i := 0; i < 3; i++is functionally equivalent. The traditional form may be more readable and compatible with older Go versions if that's a consideration.tx-submitter/types/blob_params.go (4)
21-30: Mainnet configuration looks correct.The Mainnet configuration includes historical fork times (Cancun) and a scheduled future fork (Prague). The absence of Osaka and BPO fork times is appropriate as these haven't been scheduled for Mainnet.
32-47: Hoodi testnet configuration includes multiple test forks.The Hoodi configuration includes additional fork timings (Osaka, BPO1, BPO2) that aren't yet scheduled for Mainnet, which is appropriate for a testnet used to trial upcoming features.
16-16: Helper function is well-designed.The
newUint64helper provides a clean way to create*uint64pointers for timestamp fields in the configuration structs.
50-79: UpdateFraction values verified against official specifications.Confirmed: Cancun (3,338,477), Prague (5,007,716), and BPO1-BPO2 values match EIP-7840/EIP-7892 specifications. BPO3 (20609697) and BPO4 (13739630) values match go-ethereum's default configuration, the canonical Ethereum execution client. All values in your code are correct.
Note: BPO4's target and max are lower than BPO3's (14 vs 21 blobs, 21 vs 32 max), which reflects the actual client defaults, though this is an unusual pattern for a progression.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tx-submitter/services/rollup.go (1)
85-86: Consider making ChainConfigMap private.The
ChainConfigMapfield is exported (public), but it appears to be used only internally within theRollupservice. Making it private (chainConfigMap) would better encapsulate the implementation and prevent external modification.Apply this diff if you want to make it private:
- ChainConfigMap types.ChainBlobConfigs + chainConfigMap types.ChainBlobConfigsDon't forget to update line 131 and line 1285 accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tx-submitter/services/rollup.go(4 hunks)tx-submitter/types/blob_config_test.go(1 hunks)tx-submitter/types/blob_params.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tx-submitter/types/blob_config_test.go (2)
tx-submitter/types/blob_params.go (8)
MainnetChainConfig(23-31)DefaultOsakaBlobConfig(78-80)DefaultCancunBlobConfig(70-72)DefaultPragueBlobConfig(74-76)HoodiChainConfig(34-48)DefaultBPO1BlobConfig(82-84)DefaultBPO2BlobConfig(86-88)ChainConfigMap(10-14)tx-submitter/types/blob_config.go (2)
GetBlobFeeDenominator(92-136)BlobFeeConfig(10-37)
tx-submitter/services/rollup.go (2)
tx-submitter/types/blob_params.go (3)
ChainConfigMap(10-14)ChainBlobConfigs(19-19)DefaultBlobConfig(8-8)tx-submitter/types/blob_config.go (1)
GetBlobFeeDenominator(92-136)
tx-submitter/types/blob_params.go (1)
tx-submitter/types/blob_config.go (2)
BlobFeeConfig(10-37)BlobConfig(40-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (go)
🔇 Additional comments (14)
tx-submitter/services/rollup.go (3)
131-131: LGTM: ChainConfigMap initialization.The initialization correctly wires the global chain configuration map into the Rollup service.
304-304: LGTM: Reorg retry loop.The loop correctly implements 3 retry attempts with exponential backoff (1s, 2s, 3s delays).
1283-1293: LGTM: Blob fee calculation with per-chain configuration.The blob fee calculation correctly:
- Retrieves chain-specific config using uint64 key (fixes the past pointer equality issue)
- Falls back to
DefaultBlobConfigfor unknown chains- Computes fork-aware denominator based on block time
- Applies 3x multiplier for congestion
Based on learnings (past review comments).
tx-submitter/types/blob_params.go (5)
7-8: Clarify why HoodiChainConfig is the default.
DefaultBlobConfigis set toHoodiChainConfig, which is a testnet configuration. If this is a fallback for unknown chains, consider:
- Using a more neutral default (e.g., most conservative UpdateFraction)
- Adding a comment explaining why Hoodi is the default
- Ensuring production chains won't unintentionally use testnet parameters
Is this intentional? Should the default be more conservative or better documented?
10-14: LGTM: ChainConfigMap definition.The map uses
uint64keys (chain IDs), which correctly addresses the pointer equality issue identified in past reviews. The initialization includes Mainnet, Hoodi, and Devnet configurations.Based on learnings (past review comments).
17-19: LGTM: Helper function and type alias.The
newUint64helper simplifies pointer creation, andChainBlobConfigsprovides a clear type alias for the configuration map.
21-66: Verify identical fork times for Hoodi and Devnet.
HoodiChainConfigandDevnetChainConfighave identical fork activation times:
- PragueTime: 1742999832
- OsakaTime: 1761677592
- BPO1Time: 1762365720
- BPO2Time: 1762955544
Is this intentional, or should Devnet have different (possibly earlier) fork times for testing purposes?
68-97: Verify UpdateFraction values for fork configurations.Some observations about the UpdateFraction values:
- Prague and Osaka have identical values (5007716) - Line 75 and 79. Is this intentional?
- BPO4 (13739630) is less than BPO3 (20609697) - Lines 91 and 95. This breaks the ascending pattern. Is this correct, or should BPO4 be higher?
The progression appears to be: Cancun (3.3M) < Prague/Osaka (5.0M) < BPO1 (8.3M) < BPO2 (11.6M) < BPO4 (13.7M) < BPO3 (20.6M).
Please confirm these values match the intended EIP specifications.
tx-submitter/types/blob_config_test.go (6)
10-60: LGTM: Comprehensive mainnet fork testing.The test thoroughly validates fork-based denominator selection for Mainnet, covering boundary conditions at each fork activation time. The direct configuration access avoids map lookup issues.
62-142: LGTM: Comprehensive Hoodi fork testing.The test validates all fork transitions for the Hoodi testnet, including the special case where CancunTime is 0. Boundary conditions for Prague, Osaka, BPO1, and BPO2 are thoroughly covered.
144-150: LGTM: Nil config fallback test.The test correctly verifies that a nil configuration falls back to the default Osaka blob config, ensuring defensive behavior.
152-165: LGTM: All chain configs validation.The test ensures all chain configurations in
ChainConfigMapare well-formed (non-nil config and LondonBlock) and return valid denominators. The large timestamp test ensures each config can handle far-future blocks.
167-237: LGTM: ChainConfigMap lookup validation.The test validates that each chain configuration can be correctly retrieved from
ChainConfigMapby chain ID and produces expected denominators at specific fork times. This ensures the uint64-keyed map works correctly.
239-258: LGTM: Fork priority ordering test.The test validates that
GetBlobFeeDenominatorcorrectly implements fork priority, ensuring higher-priority forks (BPO2, BPO1, Osaka) are selected over lower-priority ones when their activation times are reached.
FletcherMan
changed the title
Summary by CodeRabbit
Release Notes
New Features
Tests