Skip to content

Conversation

@cuiweixie
Copy link
Contributor

@cuiweixie cuiweixie commented Nov 26, 2025

No matter what value of these two flag is set in config file, it will be overwriten in cli flag. See below:

go-ethereum/cmd/utils/flags.go

Lines 1371 to 1372 in f8e5b53

cfg.DiscoveryV4 = ctx.Bool(DiscoveryV4Flag.Name)
cfg.DiscoveryV5 = ctx.Bool(DiscoveryV5Flag.Name)

@fjl
Copy link
Contributor

fjl commented Nov 26, 2025

Maybe should fix the overriding?

@cuiweixie
Copy link
Contributor Author

cuiweixie commented Nov 26, 2025

Maybe should fix the overriding?

What behavior is expected? I ignore the config in config file for compatible with old --flags. if we change the flags behavior may caused some issue when upgrade to new version.
Maybe we can disccuss the behavior in new version of geth.

@cuiweixie
Copy link
Contributor Author

How about change to this.

	if ctx.IsSet(DiscoveryV4Flag.Name) {
		cfg.DiscoveryV4 = ctx.Bool(DiscoveryV4Flag.Name)
	}
	if ctx.IsSet(DiscoveryV5Flag.Name) {
		cfg.DiscoveryV5 = ctx.Bool(DiscoveryV5Flag.Name)
	}

I consider it as my first version. But when user upgrade it may caused some issue. if both config file and cli flag is not set.
the flag is true in old version, but false in new version. What do you think about this.

@cuiweixie cuiweixie force-pushed the flags_in_config_file branch from 83fe18c to b9b9779 Compare November 26, 2025 14:54
@cuiweixie
Copy link
Contributor Author

How about change to this.

	if ctx.IsSet(DiscoveryV4Flag.Name) {
		cfg.DiscoveryV4 = ctx.Bool(DiscoveryV4Flag.Name)
	}
	if ctx.IsSet(DiscoveryV5Flag.Name) {
		cfg.DiscoveryV5 = ctx.Bool(DiscoveryV5Flag.Name)
	}

I consider it as my first version. But when user upgrade it may caused some issue. if both config file and cli flag is not set. the flag is true in old version, but false in new version. What do you think about this.

Got it. just change the default value in nodes/defaults.go to true, so even user configure neither config file or cli flags still get true.

@cuiweixie cuiweixie force-pushed the flags_in_config_file branch from b9b9779 to a8cfd72 Compare November 26, 2025 15:08
@cuiweixie cuiweixie changed the title p2p: flags never work in config file p2p: fix flags never work in config file Nov 26, 2025
@fjl fjl changed the title p2p: fix flags never work in config file cmd/utils: fix disabling discovery through config file Nov 26, 2025
@cuiweixie cuiweixie force-pushed the flags_in_config_file branch from a8cfd72 to d15d1ee Compare November 26, 2025 15:34
@fjl fjl added this to the 1.16.8 milestone Nov 28, 2025
@fjl fjl merged commit f691d66 into ethereum:master Nov 28, 2025
7 of 8 checks passed
fjl pushed a commit to lightclient/go-ethereum that referenced this pull request Nov 28, 2025
No matter what value of P2P.DiscoveryV4 or DiscoveryV5 is set in config file,
it will be overwritten by the CLI flag, even if the flag is not set. This fixes it
to apply the flag only if set.
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.

2 participants