Skip to content

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 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