feat: update base fee via cli #1183
Conversation
|
""" WalkthroughThis update introduces new configuration options for L2 base fee calculation, adds relevant CLI flags, and refactors the fee computation logic to use configurable parameters. It also updates documentation, versioning, and test coverage to reflect these changes, and organizes rollup-related CLI flags for better usability. Changes
Sequence Diagram(s)sequenceDiagram
participant User as CLI User
participant Geth as Geth Node (main.go)
participant Utils as CLI Utils (flags.go)
participant Config as ethconfig.Config
participant Params as params.ScrollConfig
participant Consensus as consensus/misc/eip1559.go
User->>Geth: Start node with L2 base fee flags
Geth->>Utils: Parse CLI flags (basefee.scalar, basefee.overhead)
Utils->>Config: Set BaseFeeScalar, BaseFeeOverhead in config
Geth->>Params: Pass config parameters to ScrollConfig
Consensus->>Params: Retrieve scalar, overhead for base fee calculation
Consensus->>Consensus: Calculate L2 base fee using parameters
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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)
eth/ethconfig/config.go (1)
234-237: Configuration fields added for L2 base fee calculationThese new fields appropriately add support for configurable base fee parameters. The accompanying comment clearly explains the formula in which these parameters will be used.
Could you add information about what the default values are for these parameters, or add a reference to where they're defined?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
cmd/geth/main.go(1 hunks)cmd/geth/usage.go(2 hunks)cmd/utils/flags.go(4 hunks)consensus/misc/eip1559.go(2 hunks)consensus/misc/eip1559_test.go(2 hunks)eth/backend.go(1 hunks)eth/ethconfig/config.go(1 hunks)params/config.go(2 hunks)params/version.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
cmd/geth/main.go (1)
cmd/utils/flags.go (2)
L2BaseFeeScalarFlag(940-944)L2BaseFeeOverheadFlag(945-949)
cmd/geth/usage.go (1)
cmd/utils/flags.go (20)
MaxBlockRangeFlag(886-889)L1EndpointFlag(850-853)L1ConfirmationsFlag(854-857)L1DeploymentBlockFlag(858-861)L1DisableMessageQueueV2Flag(862-865)RollupVerifyEnabledFlag(880-883)L2BaseFeeScalarFlag(940-944)L2BaseFeeOverheadFlag(945-949)DASyncEnabledFlag(898-901)DABlobScanAPIEndpointFlag(902-905)DABlockNativeAPIEndpointFlag(906-909)DABeaconNodeAPIEndpointFlag(910-913)DARecoveryModeFlag(914-917)DARecoveryInitialL1BlockFlag(918-921)DARecoveryInitialBatchFlag(922-925)DARecoverySignBlocksFlag(926-929)DARecoveryL2EndBlockFlag(930-933)DARecoveryProduceBlocksFlag(934-937)CircuitCapacityCheckEnabledFlag(868-871)CircuitCapacityCheckWorkersFlag(873-877)
consensus/misc/eip1559_test.go (1)
consensus/misc/eip1559.go (2)
CalcBaseFee(59-65)MinBaseFee(91-93)
consensus/misc/eip1559.go (1)
params/config.go (1)
ChainConfig(632-675)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: check
- GitHub Check: Analyze (go)
🔇 Additional comments (18)
params/version.go (1)
27-27: Version increment is appropriate for the new featureThe patch version has been incremented from 44 to 45, which aligns with semantic versioning practices for adding a non-breaking feature.
cmd/geth/main.go (1)
188-189: New CLI flags for base fee parametersThe addition of these flags aligns well with the PR objective to make base fee parameters configurable via CLI, improving operational flexibility.
Based on the snippet from flags.go, these flags are properly defined with descriptive usage messages that explain their purpose in L2 base fee calculation.
eth/backend.go (1)
153-158:✅ Verification successful
Workaround to propagate base fee parameters to chain configuration
While the comment describes this as a "hacky workaround," it's a reasonable approach to make these parameters accessible to fee calculation logic that uses the chain configuration.
Consider creating a more structured approach in a future refactoring to pass these configuration parameters without relying on this workaround.
🏁 Script executed:
#!/bin/bash # Check where and how these parameters are used in fee calculation rg -A 5 "CalcBaseFee" --type goLength of output: 7106
Approve: Temporary propagation of base-fee parameters is acceptable
I’ve confirmed that
BaseFeeScalarandBaseFeeOverheadare defined inparams/config.goand consumed bymisc.CalcBaseFeeacross the codebase (e.g., inscroll_worker.go,ethapi/api.go,core/chain_makers.go, etc.). Injecting them viachainConfig.Scrollineth/backend.go(lines 153–158) is a reasonable short-term solution.• File to review:
eth/backend.go(lines 153–158)
• Context:// Hacky workaround: // It's hard to pass these fields to `CalcBaseFee`, etc. // So pass them as part of the genesis config instead. chainConfig.Scroll.BaseFeeScalar = config.BaseFeeScalar chainConfig.Scroll.BaseFeeOverhead = config.BaseFeeOverheadConsider a future refactoring to pass these parameters more explicitly—e.g., by extending the
CalcBaseFeeAPI or introducing a dedicated base-fee config struct.consensus/misc/eip1559_test.go (3)
114-130: Well-structured test suite with custom configuration.The test code effectively validates the base fee calculation with a custom configuration (scalar = 10,000,000, overhead = 1), demonstrating how the parameterized formula behaves with different coefficients. This tests the core functionality of the new configurable base fee mechanism.
132-148: Good test organization.Renaming the original test cases to
testsWithDefaultsimproves clarity by separating tests for default configuration from tests with custom configuration. The original test cases are preserved, maintaining backward compatibility testing.
151-163: Complete test coverage for new functionality.The
TestMinBaseFeefunction correctly verifies theMinBaseFeefunction with both default and custom configurations. This ensures that the minimum base fee calculation works properly in all scenarios.params/config.go (3)
708-712: Good documentation of the new configuration fields.The added fields are clearly documented with a description of their purpose and how they're used in the base fee formula. Marking them with
json:"-"correctly indicates these are runtime configuration parameters that shouldn't be serialized.
761-769: Proper string representation for new fields.The string representation logic correctly handles nil values by displaying "" for unset parameters, maintaining consistency with the existing pattern in the codebase.
771-772: Updated string format to include new parameters.The string format is updated to include the new base fee parameters, ensuring they're visible in logs and debug output. This is essential for operational monitoring and troubleshooting.
cmd/geth/usage.go (2)
164-164: Added MaxBlockRangeFlag to the API group.The
MaxBlockRangeFlagis appropriately added to the "API AND CONSOLE" flag group, as it relates to API functionality.
231-254: Well-organized ROLLUP flag group.Creating a dedicated "ROLLUP" flag group improves CLI organization by grouping related rollup parameters together, including the new base fee configuration flags. This makes the CLI more intuitive for users configuring L2 nodes.
consensus/misc/eip1559.go (3)
31-35: Constants and defaults are well-defined.The introduction of
BaseFeePrecision,DefaultBaseFeeScalar, andDefaultBaseFeeOverheadas package variables provides clear defaults and ensures consistency across the codebase.
64-65: Good refactoring to use helper function.The refactoring to use a helper function
calcBaseFeeimproves code organization and reuse, allowing bothCalcBaseFeeandMinBaseFeeto share the same core logic.
90-93: Clean implementation of MinBaseFee.The
MinBaseFeefunction is a simple and effective way to calculate the minimum base fee by reusing thecalcBaseFeefunction with a zero L1 base fee. This ensures consistency in the calculation method.cmd/utils/flags.go (4)
50-50: New import for base fee calculation.The addition of the
miscpackage import is necessary to access the default L2 base fee parameters and minimum fee calculation functionality.
939-949: Well-structured CLI flags for configurable base fee parameters.These new global CLI flags allow runtime configuration of L2 base fee calculation parameters without requiring a software release. The flags follow the established pattern in the codebase and are properly documented with clear usage descriptions.
1725-1746: Well-implemented base fee configuration logic.The new
setBaseFeefunction correctly:
- Initializes base fee parameters from flags or falls back to defaults
- Logs the configured values for observability
- Enforces a minimum transaction pool price limit based on the minimum base fee
This implementation ensures transactions with fees below the calculated minimum base fee will be rejected by the transaction pool, which is important for proper network operation.
1824-1824: Proper integration with existing configuration flow.The
setBaseFeefunction is correctly integrated into theSetEthConfigfunction, ensuring the base fee parameters are initialized during Ethereum client configuration.
1. Purpose or design rationale of this PR
Add two new flags
--basefee.scalarand--basefee.overheadthat allows us to update a node's basefee calculation without publishing a new release.Example logs:
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
Summary by CodeRabbit