From c3e45a04fc50636c5b0ba2f7d4377f2f87eb8ff3 Mon Sep 17 00:00:00 2001
From: Bernhard Schuster <bernhard@ahoi.io>
Date: Mon, 26 Sep 2022 22:11:36 +0200
Subject: [PATCH] [orchestra] fix: require the initialization with `F: FnOnce`
 to be `Send` (#6051)

* add regression test for missing Send requirement

* fix: require the initialization with `F: FnOnce` to be `Send` as well

If creating intermediate variables of the builder type within
a future, rustc will complain about the future not being send,
while the thing itself isn't even using the closure based field
initialization. Adding an additional bound, resolves this and
pushes the error message "closer" to the user, and out of the
generated code.

* import fixins
---
 polkadot/node/orchestra/examples/solo.rs      | 37 +++++++++++--------
 .../orchestra/proc-macro/src/impl_builder.rs  |  6 +--
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/polkadot/node/orchestra/examples/solo.rs b/polkadot/node/orchestra/examples/solo.rs
index 67ebc292e4..cc82656759 100644
--- a/polkadot/node/orchestra/examples/solo.rs
+++ b/polkadot/node/orchestra/examples/solo.rs
@@ -46,24 +46,29 @@ impl<Context> Fortified {
 	}
 }
 
-fn main() {
-	use futures::{executor, pin_mut};
+async fn setup() {
+	let builder = Solo::builder();
 
-	executor::block_on(async move {
-		let (orchestra, _handle): (Solo<_>, _) = Solo::builder()
-			.goblin_tower(Fortified::default())
-			.spawner(DummySpawner)
-			.build()
-			.unwrap();
+	let builder = builder.goblin_tower(Fortified::default());
 
-		let orchestra_fut = orchestra
-			.running_subsystems
-			.into_future()
-			.timeout(std::time::Duration::from_millis(300))
-			.fuse();
+	let builder = builder.spawner(DummySpawner);
+	let (orchestra, _handle): (Solo<_>, _) = builder.build().unwrap();
 
-		pin_mut!(orchestra_fut);
+	let orchestra_fut = orchestra
+		.running_subsystems
+		.into_future()
+		.timeout(std::time::Duration::from_millis(300))
+		.fuse();
 
-		orchestra_fut.await
-	});
+	futures::pin_mut!(orchestra_fut);
+
+	orchestra_fut.await;
+}
+
+fn assert_t_impl_trait_send<T: Send>(_: &T) {}
+
+fn main() {
+	let x = setup();
+	assert_t_impl_trait_send(&x);
+	futures::executor::block_on(x);
 }
diff --git a/polkadot/node/orchestra/proc-macro/src/impl_builder.rs b/polkadot/node/orchestra/proc-macro/src/impl_builder.rs
index 1be25d45b5..1a88dc9261 100644
--- a/polkadot/node/orchestra/proc-macro/src/impl_builder.rs
+++ b/polkadot/node/orchestra/proc-macro/src/impl_builder.rs
@@ -172,7 +172,7 @@ pub(crate) fn impl_builder(info: &OrchestraInfo) -> proc_macro2::TokenStream {
 					pub fn #field_name_with<'a, F>(self, subsystem_init_fn: F ) ->
 						#builder <InitStateSpawner, #( #post_setter_state_generics, )* #( #baggage_passthrough_state_generics, )*>
 					where
-						F: 'static + FnOnce(#handle) ->
+						F: 'static + Send + FnOnce(#handle) ->
 							::std::result::Result<#field_type, #error_ty>,
 					{
 						let boxed_func = Init::<#field_type>::Fn(
@@ -206,7 +206,7 @@ pub(crate) fn impl_builder(info: &OrchestraInfo) -> proc_macro2::TokenStream {
 						-> #builder <InitStateSpawner, #( #post_replace_state_generics, )* #( #baggage_passthrough_state_generics, )*>
 					where
 						#field_type: 'static,
-						F: 'static + FnOnce(#field_type) -> NEW,
+						F: 'static + Send + FnOnce(#field_type) -> NEW,
 						NEW: #support_crate ::Subsystem<#subsystem_ctx_name< #subsystem_consumes >, #error_ty>,
 					{
 						let replacement: Init<NEW> = match self.#field_name {
@@ -333,7 +333,7 @@ pub(crate) fn impl_builder(info: &OrchestraInfo) -> proc_macro2::TokenStream {
 
 	let mut ts = quote! {
 		/// Convenience alias.
-		type SubsystemInitFn<T> = Box<dyn FnOnce(#handle) -> ::std::result::Result<T, #error_ty> >;
+		type SubsystemInitFn<T> = Box<dyn FnOnce(#handle) -> ::std::result::Result<T, #error_ty> + Send + 'static>;
 
 		/// Type for the initialized field of the orchestra builder
 		pub enum Init<T> {