feat(lightpush): add waku lightpush protocol client

[LNSD]

This refactoring was hastened due to the upcoming Dandelion++ support for the waku lightpush protocol.

• Split waku lightpush protocol on server and client modules
• Change message push handling to support any kind of handling (not only waku relay publishing)
• Update waku node accordingly
• Update tests accordingly

[status-im-auto]

Jenkins Builds

Click to see older builds (2)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	df62ce7	#1	2022-10-25 00:28:40	~5 min	linux	📄log
❌	df62ce7	#1	2022-10-25 00:30:20	~7 min	macos	📄log

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	24a4b5d	#2	2022-10-25 10:24:27	~14 min	macos	📦bin
✔️	24a4b5d	#2	2022-10-25 10:26:52	~17 min	linux	📦bin
✔️	5711b25	#3	2022-10-25 11:57:50	~15 min	linux	📦bin
✔️	5711b25	#3	2022-10-25 11:59:09	~16 min	macos	📦bin

[rymnc]

LGTM 😁

[alrevuelta]

some minor comments

[alrevuelta]

Just a question.

Is there a common consensus on weather to use isNil, isOk or isNil() and isOk()? Can see both usages across the repo, and afaiu its exactly the same, just like echo x or echo(x) I guess.

Both are fine for me just wondering which one to use. I can actually see that nimbus also mixes both.

[rymnc]

I don't think there is consensus yet, maybe a good addition to the nwaku style guide?

[alrevuelta]

invoking @jm-clius then, since he is writing a style guide :)

[jm-clius]

Right, I don't have a strong preference here. Personally I find echo x to be the most readable, but for more obvious templates and procedures I like isNil() and isOk() with () (also because my editor highlighting works better this way).

I'm happy to add this (or whatever we have consensus on) to the coding/style guide.

However, Nim syntax inherently allows for some flexibility. In some cases where there is a clear benefit to preferring one alternative above the others, we should be prescriptive. Example here would be to avoid using result returns, avoid overly long lines, use std/ prefix for standard imports, etc.
I'm worried though that being prescriptive everywhere where there's flexibility, would tie contributors up in trying to follow a pedantic style guide rather than just understanding the language and its usage. An example here would be if we become prescriptive in using explicit return vs expression based returns. Not sure where the empty proc/template invocations above would fit in, but imo the impact of one choice above the other is low. The coding guide should be a simple reference, rather than keep us occupied with endless refactoring. Wdyt?

[alrevuelta]

yeap, totally agree. no need to be overly pedantic nor too strict in the coding guide.

imho agreeing that there is no consensus on isOk/isOk() is an actual consensus.

Perhaps add just a suggestion. "in these occasions we tend to use isOk and in these isOk() because of this". But no need to enforce it.

[LNSD]

I have an strong opinion on this. I am following these rules for the sake of consistency and readability:

• Verbs must be followed by the parenthesis, since it denotes a "question" (e.g., isSomething(), isOk(), isNil())
• Nouns must go without the parenthesis. A proc with a noun name should go without parenthesis. They describe "properties" (or "derived properties" in the proc with a noun name) of certain object. (e.g., Result.error, Result.value)

I prefer languages more opinionated in this sense. Nim is a language prone to style inconsistency.

[jm-clius]

@LNSD, fair point. I think this is a good general guideline for us to add.

[jm-clius]

LGTM!

[LNSD]

Merging this PR.

@kaiserd, feel free to comment here after the changes are merged. I will address any concerns or suggestions in any of the upcoming PRs