feat: update evm gas price to 5gwei #904
Conversation
WalkthroughThis pull request introduces comprehensive changes to gas pricing configuration across multiple files in the project. The modifications primarily focus on updating the default gas price from Changes
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -24,6 +24,8 @@ | |||
| FXDenom = "FX" | |||
|
|
|||
| AddrLen = 20 | |||
|
|
|||
| DefaultGasPrice = 5 * 1e9 | |||
Check notice
Code scanning / CodeQL
Floating point arithmetic Note
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/genesis.go(1 hunks)app/genesis.json(1 hunks)app/upgrades/v8/upgrade.go(4 hunks)cmd/config.go(1 hunks)cmd/config_test.go(1 hunks)cmd/root.go(1 hunks)server/config/config_cmd_test.go(1 hunks)testutil/network.go(1 hunks)types/constant.go(2 hunks)x/evm/keeper/msg_server_test.go(1 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
types/constant.go
[notice] 28-28: Floating point arithmetic
Floating point arithmetic operations are not associative and a possible source of non-determinism
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (12)
server/config/config_cmd_test.go (1)
62-62: LGTM! Function call updated for consistency.The change from
GetDefGasPrice()toGetDefMinGasPrices()aligns with the new gas price configuration approach.types/constant.go (1)
56-57: Verify the significant gas price reduction.The gas price has been reduced from 500 gwei to 5 gwei (factor of 100). Please ensure this significant reduction has been thoroughly tested and won't cause issues with:
- Transaction processing
- Network economics
- Spam prevention
✅ Verification successful
Gas price reduction has been properly implemented across the codebase
The gas price reduction from 500 gwei to 5 gwei has been consistently implemented across all necessary components:
- Base fee and minimum gas price in fee market parameters
- Default configuration settings
- Upgrade handlers ensure proper migration
- Gas price validations remain intact
The change appears to be thorough and well-coordinated across the codebase, maintaining consistency in all relevant components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any hardcoded gas prices that might need updating rg -l "500.*1e9|500.*000000000" --type go # Check for any gas price related configurations in JSON files rg -l "\"base_fee\"|\"min_gas_price\"" --type jsonLength of output: 349
Script:
#!/bin/bash # Check genesis.json content echo "=== Genesis.json gas settings ===" cat app/genesis.json | jq '.app_state.feemarket' # Check context of 500*1e9 occurrences echo -e "\n=== Context of 500*1e9 occurrences ===" rg "500.*1e9|500.*000000000" --type go -C 2 # Check gas price related constants echo -e "\n=== Gas price related constants ===" rg "GasPrice|gas.*price" --type go -C 2 # Check the previous value in the changed file echo -e "\n=== Previous gas price value ===" git log -p types/constant.go | rg -A 5 -B 5 "DefaultGasPrice"Length of output: 113288
Script:
#!/bin/bash # Search for gas price constants and their values echo "=== Gas price constants ===" rg -A 2 -B 2 "DefaultGasPrice.*=|MinGasPrice.*=|gas.*price.*=" --type go # Search for gas price validations echo -e "\n=== Gas price validations ===" rg -A 2 -B 2 "ValidateBasic.*gas|validate.*gas|check.*gas" --type go # Search for gas price in test files echo -e "\n=== Gas price in tests ===" rg -A 2 -B 2 "gas.*price.*test" --type goLength of output: 22669
cmd/config.go (1)
56-56: LGTM! Configuration updated consistently.The change to use
GetDefMinGasPrices()maintains consistency with the new gas price implementation.x/evm/keeper/msg_server_test.go (1)
33-33: LGTM! Test updated to use new gas price constant.The change to use
fxtypes.DefaultGasPricemaintains consistency with the new gas price implementation.cmd/config_test.go (1)
102-102: LGTM! Consistent gas price configuration update.The change from
GetDefGasPricetoGetDefMinGasPricesaligns with the PR's objective to update gas prices and maintains consistency with other configuration changes.app/genesis.go (1)
133-134: Verify the economic implications of reduced gas prices.The significant reduction in gas prices (from 500gwei to 5gwei) warrants careful consideration:
- Could this make the network more vulnerable to spam attacks?
- How does this compare with gas prices on other EVM chains?
- What's the rationale for choosing 5gwei specifically?
Let's analyze the gas price settings across the codebase:
✅ Verification successful
Gas price reduction is well-protected by EIP-1559 mechanisms
The reduction to 5gwei is reasonable and protected against spam attacks:
- The EIP-1559 dynamic fee mechanism automatically adjusts the base fee based on network demand
- Validators can set their own minimum gas prices higher if needed
- 5gwei is still higher than many other EVM chains (e.g., Polygon 30gwei, BSC 3gwei)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all gas price references echo "Searching for gas price constants and configurations..." rg -A 2 "DefaultGasPrice|MinGasPrice|BaseFee" --type go # Search for any comments or documentation about gas price rationale echo "Searching for gas price documentation..." rg -i "gas.?price" --type mdLength of output: 37538
Script:
#!/bin/bash # Search for EIP1559 related files and configurations echo "Searching for EIP1559 related files..." rg -i "eip.*1559|dynamic.*fee" --type go -A 3 # Search for documentation echo "Searching for docs..." fd -t f . docs/ cat docs/README.md 2>/dev/null || true # Search for gas price test cases echo "Searching for gas price test cases..." rg "TestGasPrice|test.*gas.*price" --type go -A 3Length of output: 1481
Script:
#!/bin/bash # Search for fee market module files echo "Searching for fee market module..." fd -t f . "feemarket" # Search for fee market parameters echo "Searching for fee market parameters..." rg "feemarket.*param|min.*gas|base.*fee" --type go -A 3 # Search for recent gas price related commits echo "Searching for gas price related commits..." git log --oneline -n 10 | grep -i "gas.*price\|fee.*market"Length of output: 67121
testutil/network.go (1)
74-74: LGTM! Test configuration aligned with new gas price.The change to use
GetDefMinGasPricesmaintains consistency with the application's new gas price settings.Let's verify that all test cases using this configuration remain valid:
✅ Verification successful
Test configuration change verified successfully
The change to use
GetDefMinGasPrices()is consistent with how gas prices are configured across the test suite and maintains compatibility with all test cases.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for test cases that might be affected by gas price changes rg -A 5 "MinGasPrices|gasPrice|fee" --type go --glob "*_test.go"Length of output: 98116
Script:
#!/bin/bash # Search for GetDefMinGasPrices usage in test files rg -A 2 "GetDefMinGasPrices" --type go --glob "*_test.go" # Search for other min gas price configurations in test files rg -A 2 "MinGasPrices.*=|SetMinGasPrices" --type go --glob "*_test.go"Length of output: 1190
cmd/root.go (1)
128-128: Verify upgrade path for existing nodes.While the change to use
GetDefMinGasPricesis consistent with the PR's objective, we should ensure:
- The upgrade path handles the gas price change smoothly
- Existing nodes can adapt to the new gas price without disruption
- Documentation is updated to reflect this significant change
Let's check the upgrade handling:
✅ Verification successful
Default gas price change is configuration-based and safe
The change to use
GetDefMinGasPrices()is a safe configuration update that:
- Only affects default values, not existing state
- Requires no special upgrade handling
- Will be naturally adopted by nodes on restart
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for upgrade handlers that might need to handle gas price changes echo "Checking upgrade handlers..." rg -A 10 "func.*Upgrade" --type go # Search for documentation that needs updating echo "Checking documentation..." fd -e md -e rst | xargs rg -l "gas.?price"Length of output: 90956
Script:
#!/bin/bash # Check for gas price documentation and configuration echo "Checking gas price documentation and configuration..." rg -l "GetDefMinGasPrices|MinGasPrices" --type go # Check for any gas price related configuration or state changes echo "Checking gas price implementation..." rg -A 5 "func.*GetDefMinGasPrices" --type goLength of output: 1065
app/upgrades/v8/upgrade.go (3)
81-83: LGTM!The error handling for the fee market gas price migration is consistent with the rest of the function.
161-164: LGTM!The error handling for the fee market gas price migration is consistent with the rest of the function. Correctly returns
toVMwith the error since other migrations have already been applied.
524-529: LGTM!The function correctly updates both
BaseFeeandMinGasPriceto use the new default gas price. The implementation is clean, focused, and has proper error handling.app/genesis.json (1)
320-321: LGTM!The fee market parameters are correctly updated to use the new gas price of 5gwei. The changes are consistent with the migration function in
upgrade.go.
| @@ -24,6 +24,8 @@ | |||
| FXDenom = "FX" | |||
|
|
|||
| AddrLen = 20 | |||
|
|
|||
| DefaultGasPrice = 5 * 1e9 | |||
There was a problem hiding this comment.
Use integer arithmetic instead of floating point.
The current implementation uses floating point arithmetic which could be a source of non-determinism. Consider using integer arithmetic instead.
-DefaultGasPrice = 5 * 1e9
+DefaultGasPrice = 5000000000 // 5 gwei📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DefaultGasPrice = 5 * 1e9 | |
| DefaultGasPrice = 5000000000 // 5 gwei |
🧰 Tools
🪛 GitHub Check: CodeQL
[notice] 28-28: Floating point arithmetic
Floating point arithmetic operations are not associative and a possible source of non-determinism
Summary by CodeRabbit
Release Notes
New Features
Improvements
Configuration Changes