docs(iroh-net): Update endpoint docs

[flub]

Description

This rewrites quite a bit of the docs aiming to be more consistent and
clearly describe the various parts of the API and how they interact.

It also tries to get the style standardised following https://github.com/rust-lang/rfcs/blob/master/text/1574-more-api-documentation-conventions.md#appendix-a-full-conventions-text

The order of the functions has been deliberately changed, the order is used by rust doc as well so directly affects how users see the documentation.

Some PublicKey types have been changed into NodeId. They mostly
already had node_id as parameter names and were described as such.
But no other code changes have been made.

Breaking Changes

Notes & open questions

The sorting makes the diff rather difficult to read, sorry about that. But
maybe that's not so bad for the doc comments as the result is more important
than the diff.

I've taken to calling the logical thing the Endpoint controls an "iroh-net node". This is the thing that goes into the NodeMap etc, so is consistent with naming. But I'm usually using "iroh-net node" in prose to distinguish it from the iroh::node::Node.

Change checklist

• Self-review.
• Documentation updates if relevant.
• [ ] Tests if relevant.
• [ ] All breaking changes documented.

[divagant-martian]

This is not only an improvement on style but also on the docs itself, which is great.

However, I'm finding hard to justify the reordering of functions. Right now it's not alphabetically -accept used to be before connect-, so it's not clear what's the reasoning cargo docs is using for this, so that we can keep it. If we don't intend to keep it, little value is gained here. If we do intend to keep it, without understanding we are now bound to check cargo docs every time something new is added.

My general preference, and what I think had been going on here before, is to have first public constructors, then public getters before modifiers and private functions either at the end or close to the public ones that use them. And finally destructors, followed by #cfg(test) ones. As such, before this change we had:

• builder - constructor related
• bind - constructor related
• accept - out of place imo
• node_id - getter-like
• secret_key - getter-like
• discovery - getter-like
• local_addr - getter-like
• local_endpoints
• my_relay
• my_addr
• my_addr_with_endpoints - let's call it a getter...... although it's a weird one, to me it makes sense that it's after my_addr

ok the list is long but more or less the pattern is being followed except for some oddities. Modifiers I would consider accept, connect, add_node_addr, etc.

At the end we do then have close and the testing ones to get the underlying magic socket and quinn endpoint.

The point I'm trying to make is that, while imperfect, the previous order made some sense, in some way we can recognize. Now, at least me, I don't see it.

[divagant-martian]

Waiting on others to comment in internal discussions, but the Ip Endpoint term is not working for me here. I don't find it clear from the naming, it has never been introduced before and from our discussions, it's also slightly inaccurate.

One could say it's being introduced here in

the local IP endpoints, the IP addresses it can be reached on by other nodes

and yet it does not feel clear to me, in particular since direct addresses has been used as the remote counterpart. With this I mean: the ip-port pairs we use to connect to other nodes, we have consistently called direct addresses, yet the ip-port pairs that can be used to connect to us have a different name. From an user perspective, I don't think this provides really any value and instead generates confussion. It would be more useful to just use the same for both imo.

[flub]

Yeah, I agree this isn't very satisfactory yet. In this PR I don't want to change the function name yet, but I think that's how it will turn out. Just don't yet know what to. But we should get the doc comment working into a better shape already.

[flub]

ok, doubling down on "direct addresses" for this term. not changing APIs in this PR though, leaving that cleanup as a followup.

[flub]

However, I'm finding hard to justify the reordering of functions. Right now it's not alphabetically -accept used to be before connect-, so it's not clear what's the reasoning cargo docs is using for this, so that we can keep it. If we don't intend to keep it, little value is gained here. If we do intend to keep it, without understanding we are now bound to check cargo docs every time something new is added.

The point I'm trying to make is that, while imperfect, the previous order made some sense, in some way we can recognize. Now, at least me, I don't see it.

What I've been aiming for is to make reading the documentation the most helpful. So items you need the most commonly are shown first, then less common things. And finally also grouped per topic. So for Endpoint we have:

• builder - you need this first
• connect & accept - these are the two primary functions of the endpoint. connect_by_node_id here because of grouping.
• add_node_addr - manipulate the state of the endpoint
• secret_key, node_id, my_addr, my_relay, local_endpoints, local_addr - getters for information about yourself. some variations grouped in here:my_addr_with_endpoints, watch_home_relay
• connection_info & connection_infos - getters about other nodes
• conn_type_stream, dns_resolver, discovery - less common getters, partially information you already passed in via the builder. Maybe conn_type_stream doesn't belong here.
• network_change - uncommon usecase
• close - last thing you want to do with this, so at the end.

Note I'm ignoring items that are private or cfg_test, they can all come at the end or in the middle if they belong more in a group.

Would it help if we add comments to each group in the source code? That way it might be easier when adding items to help them get into the right location. Whichever order/groups we settle on.

For the Builder I've been doing similar: try to give the most common arguments first, the more exotic ones later. Maybe there you could argue that bind should be right at the front. My biggest issues is that I then can't decide if it should go before or after secret_key. Probably before, even though secret_key is basically always needed for any non-example code.

[flub]

@divagant-martian I've re-sorted the private methods a bit, and tried to document the reasoning for the ordering in the source. Feel free to still have different ideas about the actual ordering though.

[flub]

@divagant-martian please still leave your feedback if you're still not convinced of some ordering or wording even though it's merged. We can still improve the docs, just wanted this to at least partially make the release.