feat: implementing port 0 support #2125
Conversation
gabrielmer
changed the title
|
You can find the image built from this PR at Built from b0fd5f0 |
gabrielmer
force-pushed
the
feat-implementing-port-0-support
branch
from
fcd8c15 to
7a25d49
Compare
gabrielmer
assigned vpavlin and NagyZoltanPeter and unassigned vpavlin and NagyZoltanPeter
apps/wakunode2/app.nim
Outdated
| let wsAddress = initTAddress(a).valueOr: | ||
| return err(error) | ||
| websocketPort = some(wsAddress.port) | ||
| elif not a.isWsAddress() and tcpPort.isNone(): |
There was a problem hiding this comment.
It's a nitpick, but how about
if a.WsAddress():
...handle wsPort...
else:
...handle nonWsPort...
Just to skip doing the ws address check twice
There was a problem hiding this comment.
Another thing - this will obviously take a port from the first listen address it encounters - is that possible an issue? Could it mean we use non-public port instead of a public one, or is that not a problem anymore with all the previous changes regarding listen addresses?
There was a problem hiding this comment.
Thank you! Just refactored it to avoid doing the ws address check twice :)
Regarding the ports, the proc takes the tcp and ws(s) ports the switch got bound to - there should be only one of each (this listenAddrs seq is part of the switch's struct).
Afterwards we recreate the App's and WakuNode's netConfig as if we had manually configured to bind to those ports.
These lines should not concern public ports: if we configured previously an extPort, then NetConfig will be recreated with the same extPort as before. And on the other hand, if we use the ext-multi-addr-only CLI flag, we would still only (and blindly) announce the multiaddresses passed by the user.
Done! Thanks for the feedback! :) |
Description
Updating data structures to correctly reflect port number when the port is dynamically allocated by the kernel.
Changes
Issue
closes #2042