feat(swarm): implement Copy and Clone for FromSwarm

[thomaseizinger]

Description

We can make FromSwarm implement Copy and Close which makes it much easier to

a) generate code in libp2p-swarm-derive
b) manually wrap a NetworkBehaviour

Previously, we couldn't do this because ConnectionClosed would have a handler field that cannot be cloned / copied.

Related: #4076.
Related: #4581.

Notes & open questions

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[nazar-pc]

This is much nicer than before

[thomaseizinger]

Apply internal-change for autonat and request-response.

[mxinden]

👍 for Clone.

Copy: What if we want to add a non-copy enum variant in the future? That would be a breaking change? Is that worth the ergonomics through Copy?

[thomaseizinger]

👍 for Clone.

Copy: What if we want to add a non-copy enum variant in the future? That would be a breaking change? Is that worth the ergonomics through Copy?

At the moment, all non-Copy types in the enum variants are references which are Copy. I think long-term, we might want to pass a reference of the entire FromSwarm enum down instead such that we can return the actual FromSwarm instance as part of SwarmEvent.

[thomaseizinger]

👍 for Clone.
Copy: What if we want to add a non-copy enum variant in the future? That would be a breaking change? Is that worth the ergonomics through Copy?

At the moment, all non-Copy types in the enum variants are references which are Copy. I think long-term, we might want to pass a reference of the entire FromSwarm enum down instead such that we can return the actual FromSwarm instance as part of SwarmEvent.

We can remove Copy again at that point if we want to because users will just receive a reference anyway. Until then, I think Copy is the way to go.

[mxinden]

Thank you for expanding on the reasoning here.

Indeed my historical intuition of Copy is wrong. In other words agreed that Copy makes sense here.

Agreed with the long term goal to pass an &FromSwarm to the NetworkBehaviours and then the same FromSwarm to the user in the future.

I believe with this change libp2p-swarm-derive can be significantly simplified. I don't think we need the statements below any longer. Mind simplifying this as part of this pull request?

rust-libp2p/swarm-derive/src/lib.rs

Lines 224 to 536 in bb2b798

	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::ConnectionEstablished` variant.
	let on_connection_established_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::ConnectionEstablished(#connection_established {
	peer_id,
	connection_id,
	endpoint,
	failed_addresses,
	other_established,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::ConnectionEstablished(#connection_established {
	peer_id,
	connection_id,
	endpoint,
	failed_addresses,
	other_established,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::AddressChange variant`.
	let on_address_change_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::AddressChange(#address_change {
	peer_id,
	connection_id,
	old,
	new,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::AddressChange(#address_change {
	peer_id,
	connection_id,
	old,
	new,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::ConnectionClosed` variant.
	let on_connection_closed_stmts = {
	data_struct
	.fields
	.iter()
	.rev()
	.enumerate()
	.map(|(enum_n, field)| {
	let inject = match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::ConnectionClosed(#connection_closed {
	peer_id,
	connection_id,
	endpoint,
	remaining_established,
	}));
	},
	None => quote! {
	self.#enum_n.on_swarm_event(#from_swarm::ConnectionClosed(#connection_closed {
	peer_id,
	connection_id,
	endpoint,
	remaining_established,
	}));
	},
	};
	quote! {
	#inject;
	}
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::DialFailure` variant.
	let on_dial_failure_stmts = data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(enum_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::DialFailure(#dial_failure {
	peer_id,
	connection_id,
	error,
	}));
	},
	None => quote! {
	self.#enum_n.on_swarm_event(#from_swarm::DialFailure(#dial_failure {
	peer_id,
	connection_id,
	error,
	}));
	},
	});
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::ListenFailure` variant.
	let on_listen_failure_stmts = data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(enum_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::ListenFailure(#listen_failure {
	local_addr,
	send_back_addr,
	connection_id,
	error
	}));
	},
	None => quote! {
	self.#enum_n.on_swarm_event(#from_swarm::ListenFailure(#listen_failure {
	local_addr,
	send_back_addr,
	connection_id,
	error
	}));
	},
	});
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::NewListener` variant.
	let on_new_listener_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::NewListener(#new_listener {
	listener_id,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::NewListener(#new_listener {
	listener_id,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::NewListenAddr` variant.
	let on_new_listen_addr_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::NewListenAddr(#new_listen_addr {
	listener_id,
	addr,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::NewListenAddr(#new_listen_addr {
	listener_id,
	addr,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::ExpiredListenAddr` variant.
	let on_expired_listen_addr_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::ExpiredListenAddr(#expired_listen_addr {
	listener_id,
	addr,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::ExpiredListenAddr(#expired_listen_addr {
	listener_id,
	addr,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::NewExternalAddr` variant.
	let on_new_external_addr_candidate_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::NewExternalAddrCandidate(#new_external_addr_candidate {
	addr,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::NewExternalAddrCandidate(#new_external_addr_candidate {
	addr,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::ExternalAddrExpired` variant.
	let on_external_addr_expired_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::ExternalAddrExpired(#external_addr_expired {
	addr,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::ExternalAddrExpired(#external_addr_expired {
	addr,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::ExternalAddrConfirmed` variant.
	let on_external_addr_confirmed_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::ExternalAddrConfirmed(#external_addr_confirmed {
	addr,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::ExternalAddrConfirmed(#external_addr_confirmed {
	addr,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::ListenerError` variant.
	let on_listener_error_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::ListenerError(#listener_error {
	listener_id,
	err,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::ListenerError(#listener_error {
	listener_id,
	err,
	}));
	},
	})
	};
	// Build the list of statements to put in the body of `on_swarm_event()`
	// for the `FromSwarm::ListenerClosed` variant.
	let on_listener_closed_stmts = {
	data_struct
	.fields
	.iter()
	.enumerate()
	.map(|(field_n, field)| match field.ident {
	Some(ref i) => quote! {
	self.#i.on_swarm_event(#from_swarm::ListenerClosed(#listener_closed {
	listener_id,
	reason,
	}));
	},
	None => quote! {
	self.#field_n.on_swarm_event(#from_swarm::ListenerClosed(#listener_closed {
	listener_id,
	reason,
	}));
	},
	})
	};

[thomaseizinger]

I believe with this change libp2p-swarm-derive can be significantly simplified. I don't think we need the statements below any longer. Mind simplifying this as part of this pull request?

I already simplified parts of libp2p-swarm-derive. Is that what you mean or do you see further potential for improvement?

[thomaseizinger]

Friendly ping @mxinden.

[mergify]

This pull request has merge conflicts. Could you please resolve them @thomaseizinger? 🙏

[mxinden]

I believe with this change libp2p-swarm-derive can be significantly simplified. I don't think we need the statements below any longer. Mind simplifying this as part of this pull request?

I already simplified parts of libp2p-swarm-derive. Is that what you mean or do you see further potential for improvement?

Seems like I looked at the wrong revision. Sorry. The changes you made were the ones I had in mind.