Skip to content

cmd/utils: allow HTTPHost and WSHost flags precede#2060

Merged
zzzckck merged 1 commit intobnb-chain:developfrom
weiihann:fix-cmd
Dec 14, 2023
Merged

cmd/utils: allow HTTPHost and WSHost flags precede#2060
zzzckck merged 1 commit intobnb-chain:developfrom
weiihann:fix-cmd

Conversation

@weiihann
Copy link
Copy Markdown
Contributor

@weiihann weiihann commented Dec 12, 2023

Description

Fixes #2052.

When users specify the http.addr and ws.addr command line flags, they will overwrite the HTTPHost and WSHost in the config file.

Rationale

As a user, I would assume that specifying flags would overwrite the config files. While this is true for most flags, the behavior is the opposite for http.addr and ws.addr.

Example

config.toml

[Node]
HTTPHost = "127.0.0.1"
WSHost = "127.0.0.1"

Command:

./geth --datadir node --mainnet --http --http.addr 0.0.0.0 --ws --ws.addr 0.0.0.0

Expected output:

...
INFO [12-12|14:17:21.325] HTTP server started                      endpoint=[::]:8545 auth=false prefix= cors= vhosts=localhost
INFO [12-12|14:17:21.326] WebSocket enabled                        url=ws://[::]:8546
...

Actual output:

...
INFO [12-12|14:16:47.073] HTTP server started                      endpoint=127.0.0.1:8545 auth=false prefix= cors= vhosts=localhost
INFO [12-12|14:16:47.073] WebSocket enabled                        url=ws://127.0.0.1:8546
...

Changes

Flags will now overwrite the config file instead.

kaladinlight
kaladinlight approved these changes Dec 12, 2023
View reviewed changes
Copy link
Copy Markdown

@kaladinlight kaladinlight left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the quick update. This change allows the user to override --http.addr and --ws.addr which accurately reflects upstream geth as seen here:

@zzzckck zzzckck merged commit e4910b9 into bnb-chain:develop Dec 14, 2023
This was referenced Dec 14, 2023
Merged
Merged
@buddh0 buddh0 mentioned this pull request Jan 3, 2024
Closed
@weiihann weiihann deleted the fix-cmd branch March 13, 2024 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zzzckck zzzckck zzzckck approved these changes

+2 more reviewers

@buddh0 buddh0 buddh0 approved these changes

@kaladinlight kaladinlight kaladinlight approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@weiihann @buddh0 @kaladinlight @zzzckck