Add geth iliad config and data folder defaults #7
Conversation
WalkthroughThe changes introduce support for the Iliad proof-of-stake test network in the Ethereum codebase. This includes the addition of a new command-line flag, enhancements for managing the genesis block, updates to bootnode configurations, and modifications to data directory paths. Robust test cases for the Iliad configurations have also been implemented, expanding the functionality and testing framework for the network. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Command Line Interface
participant Flags as Flags Management
participant Config as Configuration Management
participant Genesis as Genesis Block Handling
CLI->>Flags: Set IliadFlag
Flags->>Config: Check IliadFlag
Config->>Genesis: Configure Iliad Genesis Block
Genesis-->>Config: Return Iliad Chain Configuration
Config-->>CLI: Confirm Iliad Network Setup
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
leeren
force-pushed
the
feat/add-geth-config-defaults
branch
from
d8438f3 to
a6e403a
Compare
leeren
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- cmd/utils/flags.go (8 hunks)
- core/genesis.go (5 hunks)
- core/genesis_test.go (2 hunks)
- node/defaults.go (1 hunks)
- params/bootnodes.go (1 hunks)
- params/config.go (3 hunks)
Additional context used
golangci-lint
cmd/utils/flags.go
621-621: File is not
goimports-ed(goimports)
Additional comments not posted (17)
node/defaults.go (1)
86-98: Inform users about the data directory changes.The updated paths for data directories may affect users who have existing data in the old directories. Ensure that this change is communicated clearly to users to prevent data loss or confusion.
Verification successful
Old data directory paths are not found in the codebase.
The absence of old data directory paths suggests that the changes to the new paths are localized and consistent with the update in
node/defaults.go. Ensure that users are informed about this change to prevent any confusion regarding data storage locations.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the new data directory paths are used consistently across the codebase. # Test: Search for usage of old and new data directory paths. Expect: Only occurrences of the new paths. rg --type go -e 'Library/Ethereum' -e 'Library/Story/geth' -e 'AppData/Roaming/Ethereum' -e 'AppData/Roaming/Story/geth' -e 'home/.ethereum' -e 'home/.story/geth'Length of output: 152
Script:
#!/bin/bash # Description: Verify if any old data directory paths are still present in the codebase. # Test: Search for usage of old data directory paths. Expect: No occurrences of old paths. rg --type go -e 'Library/Ethereum' -e 'AppData/Roaming/Ethereum' -e 'home/.ethereum'Length of output: 80
params/bootnodes.go (1)
50-57: LGTM! New bootnodes added for Iliad test network.The addition of
IliadBootnodesis clear and well-documented. Ensure these bootnodes are correctly configured and reachable.core/genesis_test.go (2)
118-128: LGTM! New test cases for Iliad genesis configuration.The new test cases effectively validate the genesis block setup for the Iliad configuration, enhancing the test coverage.
200-200: LGTM! Inclusion of Iliad genesis hash in TestGenesisHashes.The addition of the Iliad genesis hash to the
TestGenesisHashesfunction ensures comprehensive coverage of genesis configurations.core/genesis.go (4)
212-213: LGTM! Addition of IliadGenesisHash case is consistent.The new case for
params.IliadGenesisHashis consistent with the existing pattern for other networks.
299-300: LGTM! Logging of Genesis Block Hash is a useful addition.The addition of the logging statement for the genesis block hash improves traceability and debugging.
416-417: LGTM! Addition of IliadGenesisHash case is consistent.The new case for
ghash == params.IliadGenesisHashis consistent with the existing pattern for other networks.
581-591: LGTM! DefaultIliadGenesisBlock is correctly implemented.The
DefaultIliadGenesisBlockfunction follows the established pattern for defining network-specific genesis blocks.params/config.go (3)
33-33: LGTM! IliadGenesisHash declaration is consistent.The declaration of
IliadGenesisHashis consistent with the existing pattern for other network genesis hashes.
142-160: LGTM! IliadChainConfig is correctly implemented.The
IliadChainConfigfollows the established pattern for defining network-specific chain configurations.
341-341: LGTM! NetworkNames map update is consistent.The update to include the Iliad network in the
NetworkNamesmap is consistent with existing entries.cmd/utils/flags.go (6)
620-624: LGTM! IliadFlag declaration is consistent.The declaration of
IliadFlagfollows the established pattern for defining network-specific CLI flags.Tools
golangci-lint
621-621: File is not
goimports-ed(goimports)
1005-1007: LGTM! IliadFlag integration in MakeDataDir is consistent.The integration of
IliadFlagin theMakeDataDirfunction is consistent with existing network flag handling.
1070-1071: LGTM! IliadFlag integration in setBootstrapNodes is consistent.The integration of
IliadFlagin thesetBootstrapNodesfunction is consistent with existing network flag handling.
1503-1504: LGTM! IliadFlag integration in SetDataDir is consistent.The integration of
IliadFlagin theSetDataDirfunction is consistent with existing network flag handling.
1660-1660: LGTM! IliadFlag integration in SetEthConfig is consistent.The integration of
IliadFlagin theSetEthConfigfunction is consistent with existing network flag handling.Also applies to: 1835-1840
2164-2165: LGTM! IliadFlag integration in MakeGenesis is consistent.The integration of
IliadFlagin theMakeGenesisfunction is consistent with existing network flag handling.
leeren
force-pushed
the
feat/add-geth-config-defaults
branch
from
5f402ff to
aa41239
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- cmd/utils/flags.go (8 hunks)
- core/genesis.go (4 hunks)
- core/genesis_test.go (2 hunks)
- node/defaults.go (1 hunks)
- params/bootnodes.go (1 hunks)
- params/config.go (3 hunks)
Files skipped from review due to trivial changes (2)
- params/bootnodes.go
- params/config.go
Files skipped from review as they are similar to previous changes (3)
- cmd/utils/flags.go
- core/genesis_test.go
- node/defaults.go
Additional context used
golangci-lint
core/genesis.go
612-612: expected operand, found '<<'
(typecheck)
Additional comments not posted (3)
core/genesis.go (3)
212-213: LGTM! The Iliad network integration is consistent.The addition of the Iliad genesis block handling in
getGenesisStateis consistent with other network configurations.
414-415: LGTM! The Iliad chain configuration is well integrated.The addition of the Iliad chain configuration handling in
configOrDefaultis consistent with other network configurations.
579-589: LGTM! The Iliad genesis block is well defined.The
DefaultIliadGenesisBlockfunction correctly encapsulates the Iliad network parameters, consistent with other network definitions.
core/genesis.go
Outdated
| @@ -593,7 +609,11 @@ func DeveloperGenesisBlock(gasLimit uint64, faucet *common.Address) *Genesis { | |||
| common.BytesToAddress([]byte{7}): {Balance: big.NewInt(1)}, // ECScalarMul | |||
| common.BytesToAddress([]byte{8}): {Balance: big.NewInt(1)}, // ECPairing | |||
| common.BytesToAddress([]byte{9}): {Balance: big.NewInt(1)}, // BLAKE2b | |||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
Fix syntax error: Unexpected operand.
There is a syntax error due to an unexpected operand at this line. Please ensure the code is correctly structured.
Tools
golangci-lint
612-612: expected operand, found '<<'
(typecheck)
leeren
force-pushed
the
feat/add-geth-config-defaults
branch
from
aa41239 to
a1ff718
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- cmd/utils/flags.go (8 hunks)
- core/genesis.go (4 hunks)
- core/genesis_test.go (2 hunks)
- node/defaults.go (1 hunks)
- params/bootnodes.go (1 hunks)
- params/config.go (3 hunks)
Files skipped from review due to trivial changes (1)
- params/config.go
Files skipped from review as they are similar to previous changes (5)
- cmd/utils/flags.go
- core/genesis.go
- core/genesis_test.go
- node/defaults.go
- params/bootnodes.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- cmd/utils/flags.go (8 hunks)
- params/bootnodes.go (1 hunks)
- params/config.go (3 hunks)
Files skipped from review as they are similar to previous changes (3)
- cmd/utils/flags.go
- params/bootnodes.go
- params/config.go
leeren
force-pushed
the
feat/add-geth-config-defaults
branch
from
fe7682a to
5043f1e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- cmd/utils/flags.go (8 hunks)
- core/genesis.go (4 hunks)
- core/genesis_test.go (2 hunks)
- node/defaults.go (1 hunks)
- params/bootnodes.go (1 hunks)
- params/config.go (3 hunks)
Files skipped from review as they are similar to previous changes (6)
- cmd/utils/flags.go
- core/genesis.go
- core/genesis_test.go
- node/defaults.go
- params/bootnodes.go
- params/config.go
| @@ -1819,6 +1832,12 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { | |||
| } | |||
| cfg.Genesis = core.DefaultGoerliGenesisBlock() | |||
| SetDNSDiscoveryDefaults(cfg, params.GoerliGenesisHash) | |||
| case ctx.Bool(IliadFlag.Name): | |||
| if !ctx.IsSet(NetworkIdFlag.Name) { | |||
| cfg.NetworkId = 1723078116 | |||
| Difficulty: big.NewInt(0x20000), | ||
| GasLimit: 0x7A1200, | ||
| Nonce: 0x42, | ||
| Timestamp: 0, |
| return &Genesis{ | ||
| Config: params.IliadChainConfig, | ||
| Difficulty: big.NewInt(0x20000), | ||
| GasLimit: 0x7A1200, |
There was a problem hiding this comment.
any impact of the gas limit at genesis? 8m
| common.BytesToAddress([]byte{7}): {Balance: big.NewInt(1)}, // ECScalarMul | ||
| common.BytesToAddress([]byte{8}): {Balance: big.NewInt(1)}, // ECPairing | ||
| common.BytesToAddress([]byte{9}): {Balance: big.NewInt(1)}, // BLAKE2b | ||
| common.BytesToAddress([]byte{1}): {Balance: big.NewInt(1)}, // ECRecover |
There was a problem hiding this comment.
nit, please remove the extra space to reduce the unnecessary changes.
| // Iliad test network. | ||
| var IliadBootnodes = []string{ | ||
| // Upstream bootnodes | ||
| "enode://5e85033276299eff126d0c86a42b76cbad98920b4f77ae894b8a52daffa558f36de1281beca96b71e67795955bf769ce6ab3e35af66790816b37ada3d9c2b09a@52.9.220.233:30303", |
There was a problem hiding this comment.
setup b1.testnet.storyrpc.io and b2.testnet.storyrpc.io
This change bakes in our
iliadtestnet network configurations togethso that users may bootstrap running agethnode simply by running./build/bin/geth --iliad --syncmode full(we are still fixing snap sync mode which is the default).Additionally, it proposes a more sensible default data directory so that:
Summary by CodeRabbit
New Features
Bug Fixes
Chores