Finalized block event triggers tx maintanance (paritytech#12305)

* finalized block event triggers tx maintanance * tx-pool: enactment helper introduced * tx-pool: ChainApi: added tree_route method * enactment logic implemented + tests Signed-off-by: Michal Kucharczyk <[email protected]> * Some additional tests * minor improvements * trigger CI job * fix compilation errors ChainApi::tree_route return type changed to Result<Option<..>>, as some implementations (tests) are not required to provide this tree route. * formatting * trait removed * implementation slightly simplified (thanks to @koute) * get rid of Arc<> in EnactmentState return value * minor improvement * Apply suggestions from code review Co-authored-by: André Silva <[email protected]> * Apply suggestions from code review * comment updated + formatting * Apply suggestions from code review Co-authored-by: André Silva <[email protected]> Co-authored-by: Davide Galassi <[email protected]> * formatting * finalization notification bug fix + new test case + log::warn message when finalized block is being retracted by new event * added error message on tree_route failure * Apply suggestions from code review Co-authored-by: Bastian Köcher <[email protected]> * use provided tree_route in Finalized event * Option removed from ChainApi::tree_route * doc added, test and logs improved * handle_enactment aligned with original implementation * use async-await * Apply suggestions from code review Co-authored-by: André Silva <[email protected]> * Apply suggestions from code review Co-authored-by: Bastian Köcher <[email protected]> * formatting + warn->debug * compilation error fix * enactment_state initializers added * enactment_state: Option removed * manual-seal: compilation & tests fix * manual-seal: tests fixed * tests cleanup * another compilation error fixed * TreeRoute::new added * get rid of pub hack * one more test added * formatting * TreeRoute::new doc added + formatting * Apply suggestions from code review Co-authored-by: Davide Galassi <[email protected]> * (bool,Option) simplified to Option * log message improved * yet another review suggestions applied * get rid of hash in handle_enactment * Apply suggestions from code review Co-authored-by: Bastian Köcher <[email protected]> * Update client/transaction-pool/src/lib.rs Co-authored-by: Bastian Köcher <[email protected]> * minor corrections * EnactmentState moved to new file * File header corrected * error formatting aligned with codebase * Apply suggestions from code review Co-authored-by: Bastian Köcher <[email protected]> * remove commented code * small nits Signed-off-by: Michal Kucharczyk <[email protected]> Co-authored-by: André Silva <[email protected]> Co-authored-by: Davide Galassi <[email protected]> Co-authored-by: Bastian Köcher <[email protected]> Co-authored-by: André Silva <[email protected]>
AstarNetwork · Oct 11, 2022 · 94b731c · 94b731c
1 parent ccc8f6c
commit 94b731c
Show file tree

Hide file tree

Showing 14 changed files with 1,466 additions and 191 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/client/consensus/manual-seal/src/lib.rs b/client/consensus/manual-seal/src/lib.rs
@@ -305,9 +305,8 @@ pub async fn run_instant_seal_and_finalize<B, BI, CB, E, C, TP, SC, CIDP, P>(
 mod tests {
 	use super::*;
 	use sc_basic_authorship::ProposerFactory;
-	use sc_client_api::BlockBackend;
 	use sc_consensus::ImportedAux;
-	use sc_transaction_pool::{BasicPool, Options, RevalidationType};
+	use sc_transaction_pool::{BasicPool, FullChainApi, Options, RevalidationType};
 	use sc_transaction_pool_api::{MaintainedTransactionPool, TransactionPool, TransactionSource};
 	use sp_inherents::InherentData;
 	use sp_runtime::generic::{BlockId, Digest, DigestItem};
@@ -359,6 +358,7 @@ mod tests {
 		let (client, select_chain) = builder.build_with_longest_chain();
 		let client = Arc::new(client);
 		let spawner = sp_core::testing::TaskExecutor::new();
+		let genesis_hash = client.header(&BlockId::Number(0)).unwrap().unwrap().hash();
 		let pool = Arc::new(BasicPool::with_revalidation_type(
 			Options::default(),
 			true.into(),
@@ -367,6 +367,8 @@ mod tests {
 			RevalidationType::Full,
 			spawner.clone(),
 			0,
+			genesis_hash,
+			genesis_hash,
 		));
 		let env = ProposerFactory::new(spawner.clone(), client.clone(), pool.clone(), None, None);
 		// this test checks that blocks are created as soon as transactions are imported into the
@@ -429,6 +431,7 @@ mod tests {
 		let (client, select_chain) = builder.build_with_longest_chain();
 		let client = Arc::new(client);
 		let spawner = sp_core::testing::TaskExecutor::new();
+		let genesis_hash = client.header(&BlockId::Number(0)).unwrap().unwrap().hash();
 		let pool = Arc::new(BasicPool::with_revalidation_type(
 			Options::default(),
 			true.into(),
@@ -437,6 +440,8 @@ mod tests {
 			RevalidationType::Full,
 			spawner.clone(),
 			0,
+			genesis_hash,
+			genesis_hash,
 		));
 		let env = ProposerFactory::new(spawner.clone(), client.clone(), pool.clone(), None, None);
 		// this test checks that blocks are created as soon as an engine command is sent over the
@@ -505,8 +510,13 @@ mod tests {
 		let builder = TestClientBuilder::new();
 		let (client, select_chain) = builder.build_with_longest_chain();
 		let client = Arc::new(client);
-		let pool_api = api();
+		let pool_api = Arc::new(FullChainApi::new(
+			client.clone(),
+			None,
+			&sp_core::testing::TaskExecutor::new(),
+		));
 		let spawner = sp_core::testing::TaskExecutor::new();
+		let genesis_hash = client.header(&BlockId::Number(0)).unwrap().unwrap().hash();
 		let pool = Arc::new(BasicPool::with_revalidation_type(
 			Options::default(),
 			true.into(),
@@ -515,6 +525,8 @@ mod tests {
 			RevalidationType::Full,
 			spawner.clone(),
 			0,
+			genesis_hash,
+			genesis_hash,
 		));
 		let env = ProposerFactory::new(spawner.clone(), client.clone(), pool.clone(), None, None);
 		// this test checks that blocks are created as soon as an engine command is sent over the
@@ -550,7 +562,6 @@ mod tests {
 		.await
 		.unwrap();
 		let created_block = rx.await.unwrap().unwrap();
-		pool_api.increment_nonce(Alice.into());
 
 		// assert that the background task returns ok
 		assert_eq!(
@@ -566,8 +577,7 @@ mod tests {
 				}
 			}
 		);
-		let block = client.block(&BlockId::Number(1)).unwrap().unwrap().block;
-		pool_api.add_block(block, true);
+
 		assert!(pool.submit_one(&BlockId::Number(1), SOURCE, uxt(Alice, 1)).await.is_ok());
 
 		let header = client.header(&BlockId::Number(1)).expect("db error").expect("imported above");
@@ -588,9 +598,6 @@ mod tests {
 			.await
 			.is_ok());
 		assert_matches::assert_matches!(rx1.await.expect("should be no error receiving"), Ok(_));
-		let block = client.block(&BlockId::Number(2)).unwrap().unwrap().block;
-		pool_api.add_block(block, true);
-		pool_api.increment_nonce(Alice.into());
 
 		assert!(pool.submit_one(&BlockId::Number(1), SOURCE, uxt(Bob, 0)).await.is_ok());
 		let (tx2, rx2) = futures::channel::oneshot::channel();
@@ -614,6 +621,7 @@ mod tests {
 		let (client, select_chain) = builder.build_with_longest_chain();
 		let client = Arc::new(client);
 		let spawner = sp_core::testing::TaskExecutor::new();
+		let genesis_hash = client.header(&BlockId::Number(0)).unwrap().unwrap().hash();
 		let pool = Arc::new(BasicPool::with_revalidation_type(
 			Options::default(),
 			true.into(),
@@ -622,6 +630,8 @@ mod tests {
 			RevalidationType::Full,
 			spawner.clone(),
 			0,
+			genesis_hash,
+			genesis_hash,
 		));
 		let env = ProposerFactory::new(spawner.clone(), client.clone(), pool.clone(), None, None);
 

diff --git a/client/transaction-pool/Cargo.toml b/client/transaction-pool/Cargo.toml
@@ -13,6 +13,7 @@ readme = "README.md"
 targets = ["x86_64-unknown-linux-gnu"]
 
 [dependencies]
+async-trait = "0.1.57"
 codec = { package = "parity-scale-codec", version = "3.0.0" }
 futures = "0.3.21"
 futures-timer = "3.0.2"

diff --git a/client/transaction-pool/api/Cargo.toml b/client/transaction-pool/api/Cargo.toml
@@ -9,6 +9,7 @@ repository = "https://github.com/paritytech/substrate/"
 description = "Transaction pool client facing API."
 
 [dependencies]
+async-trait = "0.1.57"
 futures = "0.3.21"
 log = "0.4.17"
 serde = { version = "1.0.136", features = ["derive"] }

diff --git a/client/transaction-pool/api/src/lib.rs b/client/transaction-pool/api/src/lib.rs
@@ -21,6 +21,7 @@
 
 pub mod error;
 
+use async_trait::async_trait;
 use futures::{Future, Stream};
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 use sp_runtime::{
@@ -303,9 +304,10 @@ pub enum ChainEvent<B: BlockT> {
 }
 
 /// Trait for transaction pool maintenance.
+#[async_trait]
 pub trait MaintainedTransactionPool: TransactionPool {
 	/// Perform maintenance
-	fn maintain(&self, event: ChainEvent<Self::Block>) -> Pin<Box<dyn Future<Output = ()> + Send>>;
+	async fn maintain(&self, event: ChainEvent<Self::Block>);
 }
 
 /// Transaction pool interface for submitting local transactions that exposes a

diff --git a/client/transaction-pool/benches/basics.rs b/client/transaction-pool/benches/basics.rs
@@ -121,6 +121,14 @@ impl ChainApi for TestApi {
 	) -> Result<Option<<Self::Block as BlockT>::Header>, Self::Error> {
 		Ok(None)
 	}
+
+	fn tree_route(
+		&self,
+		_from: <Self::Block as BlockT>::Hash,
+		_to: <Self::Block as BlockT>::Hash,
+	) -> Result<sp_blockchain::TreeRoute<Self::Block>, Self::Error> {
+		unimplemented!()
+	}
 }
 
 fn uxt(transfer: Transfer) -> Extrinsic {

diff --git a/client/transaction-pool/src/api.rs b/client/transaction-pool/src/api.rs
@@ -30,6 +30,7 @@ use std::{marker::PhantomData, pin::Pin, sync::Arc};
 use prometheus_endpoint::Registry as PrometheusRegistry;
 use sc_client_api::{blockchain::HeaderBackend, BlockBackend};
 use sp_api::{ApiExt, ProvideRuntimeApi};
+use sp_blockchain::{HeaderMetadata, TreeRoute};
 use sp_core::traits::SpawnEssentialNamed;
 use sp_runtime::{
 	generic::BlockId,
@@ -111,8 +112,11 @@ impl<Client, Block> FullChainApi<Client, Block> {
 impl<Client, Block> graph::ChainApi for FullChainApi<Client, Block>
 where
 	Block: BlockT,
-	Client:
-		ProvideRuntimeApi<Block> + BlockBackend<Block> + BlockIdTo<Block> + HeaderBackend<Block>,
+	Client: ProvideRuntimeApi<Block>
+		+ BlockBackend<Block>
+		+ BlockIdTo<Block>
+		+ HeaderBackend<Block>
+		+ HeaderMetadata<Block, Error = sp_blockchain::Error>,
 	Client: Send + Sync + 'static,
 	Client::Api: TaggedTransactionQueue<Block>,
 {
@@ -190,6 +194,14 @@ where
 	) -> Result<Option<<Self::Block as BlockT>::Header>, Self::Error> {
 		self.client.header(*at).map_err(Into::into)
 	}
+
+	fn tree_route(
+		&self,
+		from: <Self::Block as BlockT>::Hash,
+		to: <Self::Block as BlockT>::Hash,
+	) -> Result<TreeRoute<Self::Block>, Self::Error> {
+		sp_blockchain::tree_route::<Block, Client>(&*self.client, from, to).map_err(Into::into)
+	}
 }
 
 /// Helper function to validate a transaction using a full chain API.
@@ -202,8 +214,11 @@ fn validate_transaction_blocking<Client, Block>(
 ) -> error::Result<TransactionValidity>
 where
 	Block: BlockT,
-	Client:
-		ProvideRuntimeApi<Block> + BlockBackend<Block> + BlockIdTo<Block> + HeaderBackend<Block>,
+	Client: ProvideRuntimeApi<Block>
+		+ BlockBackend<Block>
+		+ BlockIdTo<Block>
+		+ HeaderBackend<Block>
+		+ HeaderMetadata<Block, Error = sp_blockchain::Error>,
 	Client: Send + Sync + 'static,
 	Client::Api: TaggedTransactionQueue<Block>,
 {
@@ -264,8 +279,11 @@ where
 impl<Client, Block> FullChainApi<Client, Block>
 where
 	Block: BlockT,
-	Client:
-		ProvideRuntimeApi<Block> + BlockBackend<Block> + BlockIdTo<Block> + HeaderBackend<Block>,
+	Client: ProvideRuntimeApi<Block>
+		+ BlockBackend<Block>
+		+ BlockIdTo<Block>
+		+ HeaderBackend<Block>
+		+ HeaderMetadata<Block, Error = sp_blockchain::Error>,
 	Client: Send + Sync + 'static,
 	Client::Api: TaggedTransactionQueue<Block>,
 {