Arc definition in TransactionPool (#7042)

closes #5978

---------

Co-authored-by: command-bot <>
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>

prdoc/pr_7042.prdoc

@@ -0,0 +1,9 @@
+title: `networking::TransactionPool` should accept `Arc`
+doc:
+- audience: Node Dev
+  description: The `sc_network_transactions::config::TransactionPool` trait now returns an `Arc` for transactions.
+crates:
+- name: sc-network-transactions
+  bump: minor
+- name: sc-service
+  bump: minor

substrate/client/network/transactions/src/config.rs

@@ -22,7 +22,7 @@ use futures::prelude::*;
 use sc_network::MAX_RESPONSE_SIZE;
 use sc_network_common::ExHashT;
 use sp_runtime::traits::Block as BlockT;
-use std::{collections::HashMap, future::Future, pin::Pin, time};
+use std::{collections::HashMap, future::Future, pin::Pin, sync::Arc, time};
 
 /// Interval at which we propagate transactions;
 pub(crate) const PROPAGATE_TIMEOUT: time::Duration = time::Duration::from_millis(2900);
@@ -57,7 +57,7 @@ pub type TransactionImportFuture = Pin<Box<dyn Future<Output = TransactionImport
 /// Transaction pool interface
 pub trait TransactionPool<H: ExHashT, B: BlockT>: Send + Sync {
 	/// Get transactions from the pool that are ready to be propagated.
-	fn transactions(&self) -> Vec<(H, B::Extrinsic)>;
+	fn transactions(&self) -> Vec<(H, Arc<B::Extrinsic>)>;
 	/// Get hash of transaction.
 	fn hash_of(&self, transaction: &B::Extrinsic) -> H;
 	/// Import a transaction into the pool.
@@ -67,7 +67,7 @@ pub trait TransactionPool<H: ExHashT, B: BlockT>: Send + Sync {
 	/// Notify the pool about transactions broadcast.
 	fn on_broadcasted(&self, propagations: HashMap<H, Vec<String>>);
 	/// Get transaction by hash.
-	fn transaction(&self, hash: &H) -> Option<B::Extrinsic>;
+	fn transaction(&self, hash: &H) -> Option<Arc<B::Extrinsic>>;
 }
 
 /// Dummy implementation of the [`TransactionPool`] trait for a transaction pool that is always
@@ -79,7 +79,7 @@ pub trait TransactionPool<H: ExHashT, B: BlockT>: Send + Sync {
 pub struct EmptyTransactionPool;
 
 impl<H: ExHashT + Default, B: BlockT> TransactionPool<H, B> for EmptyTransactionPool {
-	fn transactions(&self) -> Vec<(H, B::Extrinsic)> {
+	fn transactions(&self) -> Vec<(H, Arc<B::Extrinsic>)> {
 		Vec::new()
 	}
 
@@ -93,7 +93,7 @@ impl<H: ExHashT + Default, B: BlockT> TransactionPool<H, B> for EmptyTransaction
 
 	fn on_broadcasted(&self, _: HashMap<H, Vec<String>>) {}
 
-	fn transaction(&self, _h: &H) -> Option<B::Extrinsic> {
+	fn transaction(&self, _h: &H) -> Option<Arc<B::Extrinsic>> {
 		None
 	}
 }

substrate/client/network/transactions/src/lib.rs

@@ -469,7 +469,7 @@ where
 
 	fn do_propagate_transactions(
 		&mut self,
-		transactions: &[(H, B::Extrinsic)],
+		transactions: &[(H, Arc<B::Extrinsic>)],
 	) -> HashMap<H, Vec<String>> {
 		let mut propagated_to = HashMap::<_, Vec<_>>::new();
 		let mut propagated_transactions = 0;

substrate/client/service/src/lib.rs

@@ -474,7 +474,7 @@ impl<C, P> TransactionPoolAdapter<C, P> {
 /// Get transactions for propagation.
 ///
 /// Function extracted to simplify the test and prevent creating `ServiceFactory`.
-fn transactions_to_propagate<Pool, B, H, E>(pool: &Pool) -> Vec<(H, B::Extrinsic)>
+fn transactions_to_propagate<Pool, B, H, E>(pool: &Pool) -> Vec<(H, Arc<B::Extrinsic>)>
 where
 	Pool: TransactionPool<Block = B, Hash = H, Error = E>,
 	B: BlockT,
@@ -485,7 +485,7 @@ where
 		.filter(|t| t.is_propagable())
 		.map(|t| {
 			let hash = t.hash().clone();
-			let ex: B::Extrinsic = (**t.data()).clone();
+			let ex = t.data().clone();
 			(hash, ex)
 		})
 		.collect()
@@ -506,7 +506,7 @@ where
 	H: std::hash::Hash + Eq + sp_runtime::traits::Member + sp_runtime::traits::MaybeSerialize,
 	E: 'static + IntoPoolError + From<sc_transaction_pool_api::error::Error>,
 {
-	fn transactions(&self) -> Vec<(H, B::Extrinsic)> {
+	fn transactions(&self) -> Vec<(H, Arc<B::Extrinsic>)> {
 		transactions_to_propagate(&*self.pool)
 	}
 
@@ -559,10 +559,10 @@ where
 		self.pool.on_broadcasted(propagations)
 	}
 
-	fn transaction(&self, hash: &H) -> Option<B::Extrinsic> {
+	fn transaction(&self, hash: &H) -> Option<Arc<B::Extrinsic>> {
 		self.pool.ready_transaction(hash).and_then(
 			// Only propagable transactions should be resolved for network service.
-			|tx| if tx.is_propagable() { Some((**tx.data()).clone()) } else { None },
+			|tx| tx.is_propagable().then(|| tx.data().clone()),
 		)
 	}
 }
@@ -614,6 +614,6 @@ mod tests {
 
 		// then
 		assert_eq!(transactions.len(), 1);
-		assert!(TransferData::try_from(&transactions[0].1).is_ok());
+		assert!(TransferData::try_from(&*transactions[0].1).is_ok());
 	}
 }