feat(wakunode): advertise custom multiaddresses

[rymnc]

Should allow the operator to set custom multiaddresses if the node's TLS termination happens upstream of nwaku (k8s ingress, dappnodes).
Fixes #1475

It is possible to set it while running wakunode2 -

./build/wakunode2 --ext-multiaddr:/ip4/200.19.20.21/tcp/60012/wss

This could allow operators to introduce bad inputs (wrong ip addresses/ports), so not sure if I should add a mechanism to test for it

[LNSD]

This could allow operators to introduce bad inputs (wrong ip addresses/ports), so not sure if I should add a mechanism to test for it

IMO, it depends on the consequences of adding a "bad multiaddress" (e.g., malformed). E.g., if it crashes the node abruptly, it should be tested and fail graciously.

[LNSD]

LGTM ✅

[rymnc]

This could allow operators to introduce bad inputs (wrong ip addresses/ports), so not sure if I should add a mechanism to test for it

IMO, it depends on the consequences of adding a "bad multiaddress" (e.g., malformed). E.g., if it crashes the node abruptly, it should be tested and fail graciously.

It wouldn't crash the node, but advertise incorrectly to other peers.

[jm-clius]

Thanks for this! Will review more detailed tomorrow, but note that #1475 is not about announced addresses in identify, which I think you're changing here, but about adding manually specified multiaddrs to the discoverable ENR (both for DNS discovery and discv5). Doesn't mean this can't be used, just that there may be a follow-up PR before that issue can be closed :)

[rymnc]

Thanks for this! Will review more detailed tomorrow, but note that #1475 is not about announced addresses in identify, which I think you're changing here, but about adding manually specified multiaddrs to the discoverable ENR (both for DNS discovery and discv5). Doesn't mean this can't be used, just that there may be a follow-up PR before that issue can be closed :)

Was a simple fix, Addressed in 10cc174 :)

[status-im-auto]

Jenkins Builds

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	10cc174	#1	2023-01-25 23:11:18	~24 min	macos	📄log
✔️	10cc174	#2	2023-01-26 05:14:44	~20 min	macos	📦bin

[alrevuelta]

thanks for this! lgtm!

This could allow operators to introduce bad inputs (wrong ip addresses/ports), so not sure if I should add a mechanism to test for it

Guess this won't be new, I mean with --nat=extip you can also provide a wrong ip.

./build/wakunode2 --ext-multiaddr:/ip4/200.19.20.21/tcp/60012/ws --nat=extip:9.9.9.9

With this you will be advertising a wrong ip (assuming 9.9.9.9 is wrong)

[alrevuelta]

im starting to use Port(0) which should get one among the available ones. just an fyi :)
https://github.com/waku-org/nwaku/blob/master/tests/v2/test_peer_manager.nim#L157

[rymnc]

Will address in a subsequent pr! Don't want to retrigger ci :)

[jm-clius]

Approving, because I think subsequent steps can be left for next PRs. This still does not close #1475 . :D This is because the ENR we use on discv5 is different from the one you modified here. See: #915

Probably the way to solve that once and for all is to move wakuDiscv5 creation from the app to the node and use that enr for the node (unless discv5 is disabled, in which case we call initEnr like before). Happy to chat about a strategy to do this, as it will require more config to be available to the node during creation.

[rymnc]

Approving, because I think subsequent steps can be left for next PRs. This still does not close #1475 . :D This is because the ENR we use on discv5 is different from the one you modified here. See: #915

Probably the way to solve that once and for all is to move wakuDiscv5 creation from the app to the node and use that enr for the node (unless discv5 is disabled, in which case we call initEnr like before). Happy to chat about a strategy to do this, as it will require more config to be available to the node during creation.

Just saw

nwaku/apps/wakunode2/wakunode2.nim

Line 320 in 10cc174

[], # Empty enr fields, for now

!
Agree that we should move the wakuDiscv5 creation to the node