Skip to content

config: default config values if not set in config.toml#3111

Merged
zzzckck merged 10 commits intodevelopfrom
config-def
Jun 6, 2025
Merged

config: default config values if not set in config.toml#3111
zzzckck merged 10 commits intodevelopfrom
config-def

Conversation

@emailtovamos
Copy link
Contributor

Description

Addresses #2986 with the default values mentioned here: #2986 (comment)

Rationale

tell us why we need these changes...

Example

add an example CLI or API response...

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

@emailtovamos emailtovamos requested a review from zzzckck May 20, 2025 09:16
@zzzckck
Copy link
Collaborator

zzzckck commented May 21, 2025

Here is some of the options that better to have expected default value:

[Eth.Miner]
Recommit = 10000000000  //  10 seconds

[Eth.Miner.Mev]
BuilderFeeCeil= "0"

[Eth.TxPool]
Journal = "transactions.rlp"
Rejournal = 3600000000000   //     3600 seconds
PriceLimit = 1                              
PriceBump = 10                           
AccountSlots = 200
GlobalSlots = 8000
AccountQueue = 200
GlobalQueue = 4000
Lifetime = 600000000000

[Eth.GPO]
Blocks = 20
Percentile = 60
OracleThreshold = 1000

[Node.LogConfig]
TimeFormat = "01-02|15:04:05.000"

@emailtovamos emailtovamos requested a review from MatusKysel May 28, 2025 09:02
@MatusKysel MatusKysel requested a review from Copilot May 28, 2025 09:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces fallback defaults for configuration fields not set in config.toml and updates several built-in defaults across the node, mining, and txpool subsystems.

  • Converts MEV settings to pointer fields and applies defaults at startup
  • Adds a default log time format in node/defaults.go
  • Adjusts default timeout and queue capacity values in ethconfig and txpool

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
node/defaults.go Add LogConfig.TimeFormat default and stringPtr helper
miner/minerconfig/config.go Switch MEV fields to pointers, define defaults, update ApplyDefaultMinerConfig
miner/miner_mev.go Dereference BuilderFeeCeil pointer when parsing into big.Int
miner/bid_simulator.go Guard config.Enabled pointer before use
eth/ethconfig/config.go Change default TrieTimeout from 60m to 10m
core/txpool/legacypool/legacypool.go Update default slots, queues, and lifetime values
cmd/utils/flags.go Use pointer deref for Mev.Enabled in feature parsing
Comments suppressed due to low confidence (1)

miner/minerconfig/config.go:124

  • Include a default assignment for cfg.Mev.ValidatorCommission (and any other pointer fields) inside ApplyDefaultMinerConfig, mirroring the pattern used for Enabled and BuilderFeeCeil, to avoid nil dereferences when these fields are unset.
func ApplyDefaultMinerConfig(cfg *Config) {

Comment on lines +175 to +178
AccountSlots: 200,
GlobalSlots: 8000,
AccountQueue: 200,
GlobalQueue: 4000,
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The new default queue capacities (200, 8000, 200, 4000) lack explanatory comments; consider adding a note explaining the chosen values or the expected ratio.

Suggested change
AccountSlots: 200,
GlobalSlots: 8000,
AccountQueue: 200,
GlobalQueue: 4000,
AccountSlots: 200, // Number of executable transaction slots guaranteed per account.
GlobalSlots: 8000, // Maximum number of executable transaction slots for all accounts.
AccountQueue: 200, // Maximum number of non-executable transaction slots permitted per account.
GlobalQueue: 4000, // Maximum number of non-executable transaction slots for all accounts.

Copilot uses AI. Check for mistakes.
MatusKysel
MatusKysel previously approved these changes May 28, 2025
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
emailtovamos and others added 2 commits May 29, 2025 10:26
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@emailtovamos emailtovamos requested a review from MatusKysel June 5, 2025 12:07
@zzzckck zzzckck merged commit b908fd3 into develop Jun 6, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments