feat!: Introduced --preset and clean up config
#2859
Conversation
|
You can find the image built from this PR at Built from c04988b |
fryorcraken
changed the title
There was a problem hiding this comment.
Thanks so much! I reaaaally like the changes!
The only thing I'm not sure about is removing Relay Peer Exchange.
I know Waku Peer Exchange and Discv5 are preferred, but it still works and is a valid discovery alternative
@jm-clius WDYT? Should it be removed?
waku/factory/waku.nim
Outdated
| # cluster-id=1 (aka The Waku Network) | ||
| of 1: | ||
| case confCopy.preset | ||
| of "default": |
There was a problem hiding this comment.
To be more clear I would personally name the default "TWN" or something along those lines, but I'm ok too with "default"
There was a problem hiding this comment.
The reasoning behind it is to start my moving away from "the waku network" terminology.
|
Updates description with a todo list. |
fryorcraken
force-pushed
the
clean-config
branch
4 times, most recently
from
5489bd3
to
5a36061
Compare
fryorcraken
changed the title
--preset and clean up config
|
LGTM so far except, max connection is too low because the actual number of connection is divided by a factor for peer management purposes. egg. incoming relay peers use 17% of total, service peers 30%, etc... Having the actual numbers as config params could be done but is even more obscure params a good idea? |
fryorcraken
force-pushed
the
clean-config
branch
from
c115c48
to
c447ffe
Compare
I think it's fine to keep a global parameter. I used the default value from nwaku-compose. What value do you think should be set? I think we setup 300 in the fleets. |
fryorcraken
force-pushed
the
clean-config
branch
3 times, most recently
from
12551d0
to
8998e46
Compare
200 is a bit low I feel. 300 gives us ~15 in and 20 out replay peers last I remember. |
fryorcraken
force-pushed
the
clean-config
branch
2 times, most recently
from
ae7f07b
to
72c2fca
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Does not need to be configurable on the CLI.
Not a protocol we want to use now.
Does not really make sense to have a default value set to localhost and assume user has a local eth node running. On the contrary, this can lead to issue where user doesn't understand why the node tries to connect to localhost.
Similar value to nwaku-compose. `wakunode2` is assumed to be used as service node by default.
To easily build and run wakunode2 from makefile.
Fails faster when ethereum client address is not passed.
fryorcraken
force-pushed
the
clean-config
branch
from
72c2fca
to
6251ddd
Compare
Description
Breaking change: TWN config is applied when
--preset=defaultis passed instead of--cluster-id=1.Clean-up config by reducing the number of arguments and also aligning default config with waku-org/nwaku-compose.
Note for reviewers: commits are kept clean, hence review per commit may be preferred.
Changes
presetoption instead of deducing from cluster-id.cluster-id=1still applies TWN config for now.