-
Notifications
You must be signed in to change notification settings - Fork 363
Conversation
WalkthroughThe update involves importing a new chain configuration from the Changes
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- eth/core/genesis.go (3 hunks)
Additional comments: 3
eth/core/genesis.go (3)
26-32: The new import statement for "github.com/berachain/polaris/eth/params" is added. Ensure that this package is properly versioned and maintained to avoid future compatibility issues.
39-47: The addition of the "Config" field to the
DefaultGenesis
struct with the value "params.DefaultChainConfig" changes the default configuration for the genesis block. This should be thoroughly tested to ensure that it does not introduce any unintended side effects or incompatibilities with the existing Ethereum network.54-82: The balance allocations for several addresses have been updated. It is important to ensure that these changes are intentional and have been agreed upon by the relevant stakeholders. Additionally, the use of
big.NewInt(1e18)
multiplied bybig.NewInt(1e7)
for each address implies a very large initial balance. The implications of these balances on the network's economy should be carefully considered.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1315 +/- ##
=======================================
Coverage 48.45% 48.45%
=======================================
Files 84 84
Lines 4870 4870
=======================================
Hits 2360 2360
Misses 2336 2336
Partials 174 174
|
Balance: big.NewInt(0).Mul(big.NewInt(5e18), big.NewInt(100)), //nolint:gomnd // its okay. | ||
// 0xbac | ||
common.HexToAddress("0xFE94Cc9f0dfbb657a6C6850701aBF6356227F8c3"): { | ||
Balance: big.NewInt(0).Mul(big.NewInt(1e18), big.NewInt(1e7)), //nolint:gomnd // its okay. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would suggest doing:
var (
coins = big.NewInt(0).Mul(big.NewInt(1e18), big.NewInt(1e7))
)
...
common.HexToAddress("0xFE94Cc9f0dfbb657a6C6850701aBF6356227F8c3"): {
Balance: coins,
and pass it into each balance so:
- you don't have to do the nolint line so many times
- easily configurable if you ever need to refactor the amt of these balances in one swoop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BrickBera the Config: isn't used in genesis atm, also is this change for devnet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BrickBera the Config: isn't used in genesis atm, also is this change for devnet?
Need the config to change evm chain-id. The balances change is for future devnet redeploys
should be done via configuration of the node, not via changing the default config |
Summary by CodeRabbit