Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd: Fix bug preventing --lightserv NN w/ --syncmode=full #17803

Merged
merged 1 commit into from
Oct 8, 2018

Conversation

ryanschneider
Copy link
Contributor

Currently in master this combination of flags fails:

$ ./build/bin/geth dumpconfig --lightserv 90 --syncmode=full --gcmode=archive
INFO [10-01|14:15:20.621] Maximum peer count                       ETH=25 LES=100 total=125
Fatal: Flags --lightserv, --syncmode can't be used at the same time

However, the intent was to prevent --lightserv NN and --syncmode=light.
There was a bug in the validation logic, this PR fixes said logic so that the above works, but the intended conflict still fails:

$ ./build/bin/geth dumpconfig --lightserv 90 --syncmode=light
INFO [10-01|14:18:05.744] Maximum peer count                       ETH=0 LES=100 total=125
Fatal: Flags --lightserv, --syncmode=light can't be used at the same time
$ ./build/bin/geth dumpconfig --lightserv 90 --syncmode=full
INFO [10-01|14:18:22.200] Maximum peer count                       ETH=25 LES=100 total=125
[Eth]
NetworkId = 1
...rest of config elided..

Note this is effectively a refresh of #16862 which was sitting in PR limbo for awhile.

@ryanschneider
Copy link
Contributor Author

FWIW the Appveyor build failures seem to be unrelated failures w/ swarm:

# github.com/ethereum/go-ethereum/cmd/swarm [github.com/ethereum/go-ethereum/cmd/swarm.test]
378cmd\swarm\run_test.go:245:30: undefined: loglevel
379cmd\swarm\run_test.go:321:30: undefined: loglevel
380util.go:45: exit status 2
381exit status 1
382Command exited with code 1

Also FWIW I don't run into these failures when running the tests locally:

$ GOCACHE=off go test ./cmd/utils/...
ok  	github.com/ethereum/go-ethereum/cmd/utils	0.048s

@ryanschneider
Copy link
Contributor Author

Based off past interactions I think @fjl or @holiman would both be good candidates as reviewers for this PR, sorry for the pings but this module isn't listed in CODEOWNERS.

@ryanschneider
Copy link
Contributor Author

@karalabe would be nice to get this in 1.8.17, as it lets Archive nodes act as LES servers, whereas currently they can't due to this parsing bug.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

Huh, weird corner case and good catch. Took me a long while to understand what's wrong and why your patch fixes it. LGTM!

@karalabe karalabe added this to the 1.8.17 milestone Oct 8, 2018
@karalabe karalabe merged commit cfcc475 into ethereum:master Oct 8, 2018
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 19, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 21, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 26, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 28, 2024
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