fix: apply review comments

Author: jpraynaud

mithril-aggregator/src/configuration.rs

@@ -155,7 +155,7 @@ pub struct Configuration {
     pub allow_unparsable_block: bool,
 
     /// Cardano transactions prover cache pool size
-    pub cardano_transactions_prover_cache_pool_size: u32,
+    pub cardano_transactions_prover_cache_pool_size: usize,
 }
 
 /// Uploader needed to copy the snapshot once computed.

mithril-aggregator/src/dependency_injection/builder.rs

@@ -1367,7 +1367,7 @@ impl DependenciesBuilder {
     pub async fn build_prover_service(&mut self) -> Result<Arc<dyn ProverService>> {
         let mk_map_pool_size = self
             .configuration
-            .cardano_transactions_prover_cache_pool_size as usize;
+            .cardano_transactions_prover_cache_pool_size;
         let transaction_retriever = self.get_transaction_repository().await?;
         let block_range_root_retriever = self.get_transaction_repository().await?;
         let logger = self.get_logger().await?;

mithril-aggregator/src/services/prover.rs

@@ -150,7 +150,7 @@ impl ProverService for MithrilProverService {
         // 5 - Compute the proof for all transactions
         if let Ok(mk_proof) = mk_map.compute_proof(transaction_hashes) {
             self.mk_map_pool
-                .return_resource(mk_map.into_inner(), mk_map.discriminant())?;
+                .give_back_resource(mk_map.into_inner(), mk_map.discriminant())?;
 
             let transaction_hashes_certified: Vec<TransactionHash> = transaction_hashes
                 .iter()
@@ -171,7 +171,7 @@ impl ProverService for MithrilProverService {
         let pool_size = self.mk_map_pool.size();
         info!(
             self.logger,
-            "Prover starts computing the Merkle map pool pool resource of size {pool_size}"
+            "Prover starts computing the Merkle map pool resource of size {pool_size}"
         );
         let mk_map_cache = self
             .block_range_root_retriever
@@ -185,15 +185,15 @@ impl ProverService for MithrilProverService {
             .map(|i| {
                 debug!(
                     self.logger,
-                    "Prover is computing the Merkle map pool pool resource {i}/{pool_size}"
+                    "Prover is computing the Merkle map pool resource {i}/{pool_size}"
                 );
                 self.mk_map_pool
-                    .return_resource(mk_map_cache.clone(), discriminant_new)
+                    .give_back_resource(mk_map_cache.clone(), discriminant_new)
             })
             .collect::<StdResult<()>>()?;
         info!(
             self.logger,
-            "Prover completed computing the Merkle map pool pool resource of size {pool_size}"
+            "Prover completed computing the Merkle map pool resource of size {pool_size}"
         );
 
         Ok(())

mithril-common/src/resource_pool.rs

@@ -69,10 +69,10 @@ impl<T: Send + Sync> ResourcePool<T> {
         Ok(ResourcePoolItem::new(self, resources.pop_front().unwrap()))
     }
 
-    /// Return a resource to the pool
-    /// A resource is returned to the pool only if the discriminant matches
+    /// Give back a resource to the pool
+    /// A resource is given back to the pool only if the discriminant matches
     /// and if the pool is not already full
-    pub fn return_resource(&self, resource: T, discriminant: u64) -> StdResult<()> {
+    pub fn give_back_resource(&self, resource: T, discriminant: u64) -> StdResult<()> {
         if self.count()? == self.size {
             // Pool is full
             return Ok(());
@@ -81,7 +81,7 @@ impl<T: Send + Sync> ResourcePool<T> {
             .resources
             .lock()
             .map_err(|_| ResourcePoolError::PoisonedLock())
-            .with_context(|| "Resource pool 'return_resource' failed locking Mutex")?;
+            .with_context(|| "Resource pool 'give_back_resource' failed locking Mutex")?;
         if self.discriminant()? != discriminant {
             // Stale resource
             return Ok(());
@@ -195,7 +195,7 @@ impl<T: Send + Sync> Drop for ResourcePoolItem<'_, T> {
             let resource = self.into_inner();
             let _ = self
                 .resource_pool
-                .return_resource(resource, self.discriminant);
+                .give_back_resource(resource, self.discriminant);
         }
     }
 }
@@ -254,28 +254,29 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_resource_pool_returns_fresh_resource() {
+    async fn test_resource_pool_gives_back_fresh_resource() {
         let pool_size = 10;
         let resources_expected: Vec<String> = (0..pool_size).map(|i| i.to_string()).collect();
         let pool = ResourcePool::<String>::new(pool_size, resources_expected.clone());
         assert_eq!(pool.count().unwrap(), pool_size);
 
         let mut resource_item = pool.acquire_resource(Duration::from_millis(1000)).unwrap();
         assert_eq!(pool.count().unwrap(), pool_size - 1);
-        pool.return_resource(resource_item.into_inner(), pool.discriminant().unwrap())
+        pool.give_back_resource(resource_item.into_inner(), pool.discriminant().unwrap())
             .unwrap();
 
         assert_eq!(pool.count().unwrap(), pool_size);
     }
 
     #[tokio::test]
-    async fn test_resource_pool_returns_resource_automatically() {
+    async fn test_resource_pool_gives_back_resource_automatically() {
         let pool_size = 10;
         let resources_expected: Vec<String> = (0..pool_size).map(|i| i.to_string()).collect();
         let pool = ResourcePool::<String>::new(pool_size, resources_expected.clone());
         assert_eq!(pool.count().unwrap(), pool_size);
 
         {
+            // Resource will be returned when resource item is dropped (will occur when exiting this block scope)
             let _resource_item = pool.acquire_resource(Duration::from_millis(1000)).unwrap();
             assert_eq!(pool.count().unwrap(), pool_size - 1);
         }
@@ -284,20 +285,20 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn test_resource_pool_does_not_return_resource_when_pool_is_full() {
+    async fn test_resource_pool_does_not_give_back_resource_when_pool_is_full() {
         let pool_size = 10;
         let resources_expected: Vec<String> = (0..pool_size).map(|i| i.to_string()).collect();
         let pool = ResourcePool::<String>::new(pool_size, resources_expected.clone());
         assert_eq!(pool.count().unwrap(), pool_size);
 
-        pool.return_resource("resource".to_string(), pool.discriminant().unwrap())
+        pool.give_back_resource("resource".to_string(), pool.discriminant().unwrap())
             .unwrap();
 
         assert_eq!(pool.count().unwrap(), pool_size);
     }
 
     #[tokio::test]
-    async fn test_resource_pool_does_not_return_stale_resource() {
+    async fn test_resource_pool_does_not_giev_back_stale_resource() {
         let pool_size = 10;
         let resources_expected: Vec<String> = (0..pool_size).map(|i| i.to_string()).collect();
         let pool = ResourcePool::<String>::new(pool_size, resources_expected.clone());
@@ -308,7 +309,7 @@ mod tests {
         let discriminant_stale = pool.discriminant().unwrap();
         pool.set_discriminant(pool.discriminant().unwrap() + 1)
             .unwrap();
-        pool.return_resource(resource_item.into_inner(), discriminant_stale)
+        pool.give_back_resource(resource_item.into_inner(), discriminant_stale)
             .unwrap();
 
         assert_eq!(pool.count().unwrap(), pool_size - 1);