swarm/: Enable dialing a specific fixed set of addresses for a single peer

[mxinden]

Status Quo

One can either

• (a) request a dial of a specific peer by their PeerId via NetworkBehaviourAction::DialPeer, where the concrete addresses to be dialed are collected via NetworkBehaviour::addresses_of_peer

• or (b) request a dial of a single specific address via NetworkBehaviourAction::DialAddress

Problem

A NetworkBehaviour implementation might want to choose the specific fixed set of addresses to be dialed for a given peer. This is the case in #2076 where the remote provides a specific set of addresses to be dialed by the local node.

Using NetworkBehaviourAction::DialPeer and providing the set of addresses via NetworkBehaviour::addresses_of_peer does not prevent other NetworkBehaviour implementations to add bogus addresses to the set via NetworkBehaviour::addresses_of_peer.

Proposal

• Merge NetworkBehaviourAction::DialPeer and NetworkBehaviourAction::DialAddress into a single NetworkBehaviourAction::Dial.
• In NetworkBehaviourAction::Dial allow specifying either:
  • A PeerId only, where the Swarm retrieves corresponding addresses via NetworkBehaviour::addresses_of_peer.
  • A PeerId and a set of addresses
    • with the option of the Swarm extending the set of addresses via NetworkBehaviour::addresses_of_peer.
  • A set of addresses.

Miscellaneous

• Next to enabling a NetworkBehaviour implementation to specify a specific fixed set of addresses to be dialed, the change above would simplify other implementations. E.g. in the case where a NetworkBehaviour implementation wants to dial a peer by its PeerId, also providing some addresses, it can provide these addresses right away within NetworkBehaviourAction::Dial, instead of first emitting a NetworkBehaviourAction::DialPeer and then providing the set of addresses via NetworkBehaviour::addresses_of_peer.

• To reduce conflicts I would suggest tackling this after core/: Concurrent connection attempts #2248 is merged.

[thomaseizinger]

In NetworkBehaviourAction::Dial allow specifying either:

• A PeerId only, where the Swarm retrieves corresponding addresses via NetworkBehaviour::addresses_of_peer.
• A PeerId and a set of addresses
• with the option of the Swarm extending the set of addresses via NetworkBehaviour::addresses_of_peer.
• A set of addresses.

We could express this with:

enum NetworkBehaviourAction {
	Dial {
		peer_id: Option<PeerId>,
		addresses: Option<Vec<Multiaddress>>
	}
}

With the following conventions:

• peer_id: Some + addresses: None: Current Dial behaviour
• peer_id: None + addresses: Some: Current DialAddress behaviour (extended to use all addresses)
• peer_id: Some + addresses: Some: Dial only provided addresses and enforce we always reach PeerId (to support protocols: Implement Direct Connection Upgrade through Relay (dcutr) #2076)
• peer_id: None + addresses: None: Invalid combination

This would flatten out the structure a bit and avoid having another enum that we need to import / use. To make things easier to construct, we could have constructor functions on NetworkBehaviourAction.

Having written that though, maybe the best combination would be constructor functions + a dedicated enum internally to remove the "Invalid combination" case.

[mxinden]

Having written that though, maybe the best combination would be constructor functions + a dedicated enum internally to remove the "Invalid combination" case.

I am leaning towards making invalid combinations invalid at compile time, e.g. through a dedicated enum. I still need to give this more thoughts and experiment.