Introduce support for binding sockets to ports picked by the OS

[theohax]

This PR allows the builder of a nano::node not to specify a peering port (specify it as 0), or a websocket one, and let the OS pick an available port when the socket is bound. He would be able to later retrieve that port by calling node.network.endpoint ().port(), amongst other options.

The PR also adds a bunch of comments about how the mechanism works and where to pay attention if stuff gets changed.

Additionally, a couple unrelated changes made their way here and are strictly related to fixing clang-tidy warnings (like shadowed variable declarations, useless copies, etc).

[dsiganos]

It would be nice to separate (in different commits) the stylistic changes from the necessary changes, to make it easier to know what are the important changes. For example, in this PR, I cannot tell which changes are stylistic and which are crucial.

[theohax]

It would be nice to separate (in different commits) the stylistic changes from the necessary changes, to make it easier to know what are the important changes. For example, in this PR, I cannot tell which changes are stylistic and which are crucial.

Yeah I tried to make as good as a separation as possible (even have a couple PRs that I haven't opened yet due to exactly this reason of not mixing things up and avoiding conflicts), but some I couldn't or was too tedious to separate. I'll try to keep it in mind to do better.

[dsiganos]

Is setting node.network.port a good idea? It seems to be breaking encapsulation.
It does not seem like a good idea for bootstrap_listener to be changing a global setting.
What if we had 2 bootstrap_listeners? Then this would break.

[dsiganos]

I think that node.network.port should remain as is. So that in the case of zero, it continues to signify that we want automatic binding. If we now create a bootstrap_listener and then destroy it and create another one, then we lose the setting that binding should be automatic.

[theohax]

node.network.port is important to be up to date with the actual value resulted after automatic binding

[dsiganos]

I feel that this is not an improvement and that we are introducing traps for the future.
Let's discuss it tomorrow at the dev meeting.

Problems that I see:

• you cannot have multiple bootstrap_listeners
• you cannot create, destroy and re-create a bootstrap_listener whilst preserving settings

[theohax]

I agree that the propagating of the port picked inside bootstrap_listener towards network.port could be done in a better place, perhaps the node itself can do this, but the core concept of propagating this setting needs to stay there.

I have a series of other PRs based on this one so unless you feel this impediment must be addressed right away I'd rather address it in a future PR and yes of course we can discuss tomorrow how exactly it could work, e.g. like I suggested keep the node in charge of the propagation or maybe a different approach.

About creating multiple bootstrap_listeners, I am not sure that is even a use case, because network.endpoint() won't be a thing anymore, which endpoint exactly?

But yeah please keep reviewing and let me know about any other things I should address so that I make up a list with them.

@clemahieu keeping you in the loop here, any thoughts about this?

[theohax]

@dsiganos just addressed this one -- certainly isn't a lot much better but at least now only the node deals with setting stuff that belongs to network, instead of the bootstrap_listener doing that via node, which was an even worse encapsulation breakage.

[theohax]

Just rebased this on top of latest develop and addressed some code review.

Also, @reviewers, since #3555 got merged into this already, I'd like to finally merge everything into develop as well, so please let me know if there are other code review things to address or approve the PR otherwise. Thanks!