cmd/geth: add --config file flag #13875

fjl · 2017-04-06T16:48:26Z

In this PR, geth gets a new --config <file> option. The new option causes it to read configuration options from a file in TOML syntax. The implementation approach and motivation for it is described in https://twurst.com/articles/geth-config-file.html.

karalabe · 2017-04-06T17:17:41Z

cmd/geth/config.go

@@ -1,3 +1,19 @@
+// Copyright 2015 The go-ethereum Authors


karalabe

Looked though the code, seems ok, just a few questions. Still have to try it out though.

karalabe · 2017-04-11T12:59:49Z

p2p/server.go

-	// Discovery specifies whether the peer discovery mechanism should be started
-	// or not. Disabling is usually useful for protocol debugging (manual topology).
-	Discovery bool
+	// NoDiscowery can be used to disable the peer discovery mechanism.


NoDiscoWery ;)

karalabe · 2017-04-11T13:04:56Z

node/node.go

+		n.serverConfig.TrustedNodes = n.config.TrusterNodes()
+	}
+	if n.serverConfig.NodeDatabase == "" {
+		n.serverConfig.NodeDatabase = n.config.NodeDB()


When does it happen that n.serverConfig.Xyz is not nil? I.e. when the above code should not override?

n.serverConfig is set to (a copy of) n.config.P2P, which is created from defaults + config file + flags. If there is something set through any of these, that value is used.

karalabe · 2017-04-11T13:07:59Z

mobile/geth.go

-		Version:          params.Version,
-		DataDir:          datadir,
-		KeyStoreDir:      filepath.Join(datadir, "keystore"), // Mobile should never use internal keystores!
-		NoDiscovery:      true,


Didn"t the NoDiscover get lost accidentally?

Yes, looks like it.

karalabe · 2017-04-11T13:32:21Z

Did"t you add --genesis to support dumping the genesis file too? That"s really needed for custom networks as we"ve talked.

Also I"d really like to have --syncMode as a flag too, defaulting to fast. Without the flag, I can"t set it to "full" manually. Maybe I missed osmething?

karalabe · 2017-04-11T13:36:02Z

eth/config.go

+
+// DefaultConfig contains default settings for use on the Ethereum main net.
+var DefaultConfig = Config{
+	SyncMode:             downloader.FullSync,


We want to default to fast sync, with full sync being opt in. Users new to geth always just freak out and don"t understand why it takes so long. Archive nodes are the rarity nowadays, not the common config.

bas-vk

I went through the code but have not tried it. Will do that tomorrow.

bas-vk · 2017-04-11T19:11:31Z

eth/config.go

+	EnablePreimageRecording bool
+
+	// Miscellaneous options
+	SolcPath  string


Do we still need the solc path option?

Not really.

bas-vk · 2017-04-11T19:17:06Z

cmd/geth/config.go

+
+// dumpConfig is the dumpconfig command.
+func dumpConfig(ctx *cli.Context) error {
+	_, cfg := makeConfigNode(ctx)


In some situation this can lead to undesired output.

> geth dumpconfig WARN [04-11|21:13:43] No etherbase set and no accounts found as default [Eth] NetworkId = 1 SyncMode = "full"

The warning goes to stderr though.

bas-vk · 2017-04-11T19:27:40Z

cmd/geth/config.go

+	P2P:         p2p.Config{MaxPeers: 25},
+	HTTPModules: []string{"eth", "net", "web3"},
+	WSModules:   []string{"eth", "net", "web3"},
+}


What is the reason that some fields are given a default value and others not?

The zero value is a good default for the other fields.

Some values are missing though. They"re added now.

bas-vk · 2017-04-11T19:51:32Z

cmd/utils/flags.go

+	case ctx.GlobalIsSet(IPCPathFlag.Name):
+		cfg.IPCPath = ctx.GlobalString(IPCPathFlag.Name)
+	}
+}


If the user starts up geth without any ipc path/disable related options geth currently starts the ipc interface on a default endpoint. This method will leave it empty causing the node not to start it.

#13875 (comment)

bas-vk · 2017-04-11T20:09:13Z

cmd/utils/flags.go

+	if ctx.GlobalIsSet(RPCListenAddrFlag.Name) {
+		cfg.HTTPHost = ctx.GlobalString(RPCListenAddrFlag.Name)
+	}
+	if ctx.GlobalIsSet(RPCPortFlag.Name) {


If the user doesn"t specify a custom http port cfg.HTTPPort will be 0.

The default port and APIs are set in node.DefaultConfig.

Commit 098ba171e4b485774879bdb96d74d10e8ebf863e that fixes this was added after I reviewed it.

bas-vk · 2017-04-11T20:09:41Z

cmd/utils/flags.go

+	if ctx.GlobalIsSet(RPCCORSDomainFlag.Name) {
+		cfg.HTTPCors = ctx.GlobalString(RPCCORSDomainFlag.Name)
+	}
+	if ctx.GlobalIsSet(RPCApiFlag.Name) {


If the user doesn"t specify custom modules cfg.HTTPModules will be empty.

#13875 (comment)

bas-vk · 2017-04-11T20:10:05Z

cmd/utils/flags.go

+	if ctx.GlobalIsSet(WSAllowedOriginsFlag.Name) {
+		cfg.WSOrigins = ctx.GlobalString(WSAllowedOriginsFlag.Name)
+	}
+	if ctx.GlobalIsSet(WSApiFlag.Name) {


If the user doesn"t specify custom modules cfg.WSModules will be empty.

#13875 (comment)

bas-vk · 2017-04-11T20:10:33Z

cmd/utils/flags.go

+	if ctx.GlobalIsSet(WSListenAddrFlag.Name) {
+		cfg.WSHost = ctx.GlobalString(WSListenAddrFlag.Name)
+	}
+	if ctx.GlobalIsSet(WSPortFlag.Name) {


If the user doesn"t specify custom port cfg.WSPort will be empty.

#13875 (comment)

fjl · 2017-04-12T00:00:34Z

Did"t you add --genesis to support dumping the genesis file too? That"s really needed for custom networks as we"ve talked.

It would require adding toml struct tags to params.Config. I"ll do it in a follow-up PR.

fjl · 2017-04-12T00:00:41Z

@karalabe @bas-vk PTAL

bas-vk · 2017-04-12T08:31:33Z

cmd/utils/flags.go

-		comps = append(comps, "JIT")
-	}
-	return strings.Join(comps, "/")
+	cfg.UserIdent = strings.Join(comps, "/")
 }


If cfg.UserIdent is empty strings.split will return an array with a single string of length 0. If the user specified a custom identify this is appended to this array. When later joined with "/" it will lead to /myIdentify.

geth --identity "myIdentity" > admin.nodeInfo.name "Geth//myIdentity/v1.6.0-unstable-3e2ed7e3/linux-amd64/go1.8.1"

bas-vk · 2017-04-12T08:52:47Z

node/config.go


 	// HTTPCors is the Cross-Origin Resource Sharing header to send to requesting
 	// clients. Please be aware that CORS is a browser enforced security, it's fully
 	// useless for custom HTTP clients.
-	HTTPCors string
+	HTTPCors string `toml:",omitempty"`


It would be a bit nicer if this was an array just like HTTPModules. Currently the string is a comma separated string that is is splitted when the cors handler is created. I can do this in an follow up PR. Same thing for the WSOrigins.

This makes the zero Config slightly more useful. It also fixes package node tests because Node detects reuse of the datadir through the NodeDatabase.

This removes the FastSync, LightSync flags in favour of a more general SyncMode flag.

It now reads: Fatal: flags --dev, --testnet can"t be used at the same time

This exposed a couple of places that needed to be updated to use node.DefaultConfig.

Set it in the frontends instead.

karalabe

LGTM

karalabe reviewed Apr 6, 2017

View reviewed changes

cmd/geth/config.go

@@ -1,3 +1,19 @@

// Copyright 2015 The go-ethereum Authors

Copy link

Member

karalabe Apr 6, 2017

Choose a reason for hiding this comment

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

"17 ;)

karalabe added this to the 1.6.0 milestone Apr 7, 2017

fjl mentioned this pull request Apr 10, 2017

cmd/geth, cmd/utils: support config files for cli params #3424

Closed

fjl force-pushed the config-wip branch 2 times, most recently from 1aea7b9 to 38fd8bd Compare April 11, 2017 09:39

karalabe requested changes Apr 11, 2017

View reviewed changes

karalabe reviewed Apr 11, 2017

View reviewed changes

bas-vk suggested changes Apr 11, 2017

View reviewed changes

bas-vk reviewed Apr 12, 2017

View reviewed changes

fjl added 17 commits April 12, 2017 15:53

p2p/discover, p2p/discv5: add marshaling methods to Node

3b48a35

p2p/netutil: make Netlist decodable from TOML

2a69d5a

common/math: encode nil HexOrDecimal256 as 0x0

5077f02

cmd/geth: add --config file flag

5185203

cmd/geth: add missing license header

5053cab

eth: prettify Config again, fix tests

f65fa21

eth: use gasprice.Config instead of duplicating its fields

12d627e

eth/gasprice: hide nil default from dumpconfig output

f3fb2fc

cmd/geth: hide genesis block in dumpconfig output

e6e5664

node: make tests compile

6bb697d

console: fix tests

ee69b41

cmd/geth: make TOML keys look exactly like Go struct fields

075642f

p2p: use discovery by default

0df01af

This makes the zero Config slightly more useful. It also fixes package node tests because Node detects reuse of the datadir through the NodeDatabase.

cmd/geth: make ethstats URL settable through config file

03cf680

cmd/faucet: fix configuration

c507c9d

cmd/geth: dedup attach tests

2bc31a3

eth: add comment for DefaultConfig

7608af1

fjl added 15 commits April 12, 2017 15:53

eth: pass downloader.SyncMode in Config

041a001

This removes the FastSync, LightSync flags in favour of a more general SyncMode flag.

cmd/utils: remove jitvm flags

766ce7a

cmd/utils: make mutually exclusive flag error prettier

08c62b1

It now reads: Fatal: flags --dev, --testnet can"t be used at the same time

p2p: fix typo

85b3861

node: add DefaultConfig, use it for geth

087d06f

mobile: add missing NoDiscovery option

8af4bfa

cmd/utils: drop MakeNode

214f0f8

This exposed a couple of places that needed to be updated to use node.DefaultConfig.

node: fix typo

9d097d8

eth: make fast sync the default mode

342898f

cmd/utils: remove IPCApiFlag (unused)

443c00e

node: remove default IPC path

026f312

Set it in the frontends instead.

cmd/geth: add --syncmode

48339d4

cmd/utils: make --ipcdisable and --ipcpath mutually exclusive

0f0d063

cmd/utils: don"t enable WS, HTTP when setting addr

0e3ef84

cmd/utils: fix --identity

75e08ae

karalabe approved these changes Apr 12, 2017

View reviewed changes

fjl force-pushed the config-wip branch from 20346e1 to 75e08ae Compare April 12, 2017 13:54

karalabe merged commit 30d706c into ethereum:master Apr 12, 2017

This was referenced Sep 25, 2017

Allow configuration via a config file #3332

Closed

geth start-up configuration file #2067

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/geth: add --config file flag #13875

cmd/geth: add --config file flag #13875

fjl commented Apr 6, 2017 •

edited

Loading

karalabe Apr 6, 2017

karalabe left a comment

karalabe Apr 11, 2017

karalabe Apr 11, 2017

fjl Apr 11, 2017

karalabe Apr 11, 2017

fjl Apr 11, 2017

karalabe commented Apr 11, 2017 •

edited

Loading

karalabe Apr 11, 2017

fjl Apr 11, 2017

bas-vk left a comment

bas-vk Apr 11, 2017

fjl Apr 11, 2017

bas-vk Apr 11, 2017

fjl Apr 11, 2017 •

edited

Loading

bas-vk Apr 11, 2017

fjl Apr 11, 2017

fjl Apr 11, 2017

bas-vk Apr 11, 2017

fjl Apr 11, 2017

bas-vk Apr 11, 2017

fjl Apr 11, 2017 •

edited

Loading

bas-vk Apr 12, 2017

bas-vk Apr 11, 2017

fjl Apr 11, 2017

bas-vk Apr 11, 2017

fjl Apr 11, 2017

bas-vk Apr 11, 2017

fjl Apr 11, 2017

fjl commented Apr 12, 2017

fjl commented Apr 12, 2017

bas-vk Apr 12, 2017

bas-vk Apr 12, 2017 •

edited

Loading

karalabe left a comment

cmd/geth: add --config file flag #13875

cmd/geth: add --config file flag #13875

Conversation

fjl commented Apr 6, 2017 • edited Loading

Choose a reason for hiding this comment

karalabe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karalabe commented Apr 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bas-vk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjl Apr 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjl Apr 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjl commented Apr 12, 2017

fjl commented Apr 12, 2017

Choose a reason for hiding this comment

bas-vk Apr 12, 2017 • edited Loading

Choose a reason for hiding this comment

karalabe left a comment

Choose a reason for hiding this comment

fjl commented Apr 6, 2017 •

edited

Loading

karalabe commented Apr 11, 2017 •

edited

Loading

fjl Apr 11, 2017 •

edited

Loading

fjl Apr 11, 2017 •

edited

Loading

bas-vk Apr 12, 2017 •

edited

Loading