Use Range instead of RangeInclusive in functions signatures

Alenar · Alenar · commit b6e0cb81c92e · 2024-04-26T11:37:48.000+02:00
To uniformise use of ranges since `BlockRange` use a Range internaly.
diff --git a/mithril-common/src/entities/block_range.rs b/mithril-common/src/entities/block_range.rs
@@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize};
 use std::{
     cmp::Ordering,
     fmt::{Display, Formatter, Result},
-    ops::{Deref, Range, RangeInclusive},
+    ops::{Deref, Range},
 };
 
 use crate::{
@@ -59,7 +59,7 @@ impl BlockRange {
     }
 
     /// Get all [BlockRange] contained in the given interval
-    pub fn all_ranges_in(interval: RangeInclusive<BlockNumber>) -> Vec<BlockRange> {
+    pub fn all_ranges_in(interval: Range<BlockNumber>) -> Vec<BlockRange> {
         let all_numbers: Vec<BlockNumber> =
             interval.skip_while(|i| i % Self::LENGTH != 0).collect();
         all_numbers
@@ -214,26 +214,28 @@ mod tests {
 
     #[test]
     fn test_block_range_all_ranges_in() {
-        assert_eq!(BlockRange::all_ranges_in(0..=0), vec![]);
-        assert_eq!(BlockRange::all_ranges_in(1..=16), vec![]);
+        assert_eq!(BlockRange::all_ranges_in(0..0), vec![]);
+        assert_eq!(BlockRange::all_ranges_in(0..1), vec![]);
+        assert_eq!(BlockRange::all_ranges_in(0..14), vec![]);
+        assert_eq!(BlockRange::all_ranges_in(1..15), vec![]);
         assert_eq!(
-            BlockRange::all_ranges_in(0..=15),
+            BlockRange::all_ranges_in(0..15),
             vec![BlockRange::new(0, 15)]
         );
         assert_eq!(
-            BlockRange::all_ranges_in(0..=16),
+            BlockRange::all_ranges_in(0..16),
             vec![BlockRange::new(0, 15)]
         );
         assert_eq!(
-            BlockRange::all_ranges_in(14..=29),
+            BlockRange::all_ranges_in(14..30),
             vec![BlockRange::new(15, 30)]
         );
         assert_eq!(
-            BlockRange::all_ranges_in(14..=30),
+            BlockRange::all_ranges_in(14..31),
             vec![BlockRange::new(15, 30)]
         );
         assert_eq!(
-            BlockRange::all_ranges_in(14..=61),
+            BlockRange::all_ranges_in(14..61),
             vec![
                 BlockRange::new(15, 30),
                 BlockRange::new(30, 45),
diff --git a/mithril-signer/src/cardano_transactions_importer.rs b/mithril-signer/src/cardano_transactions_importer.rs
@@ -1,4 +1,4 @@
-use std::ops::RangeInclusive;
+use std::ops::Range;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
@@ -18,17 +18,6 @@ pub trait TransactionStore: Send + Sync {
     /// Get the highest known transaction beacon
     async fn get_highest_beacon(&self) -> StdResult<Option<ImmutableFileNumber>>;
 
-    /// Get the interval of blocks whose merkle root has yet to be computed
-    async fn get_block_interval_without_block_range_root(
-        &self,
-    ) -> StdResult<Option<RangeInclusive<BlockNumber>>>;
-
-    /// Get transactions between two block numbers
-    async fn get_transactions_between(
-        &self,
-        range: RangeInclusive<BlockNumber>,
-    ) -> StdResult<Vec<CardanoTransaction>>;
-
     /// Get stored transactions up to the given beacon
     async fn get_up_to(
         &self,
@@ -38,6 +27,17 @@ pub trait TransactionStore: Send + Sync {
     /// Store list of transactions
     async fn store_transactions(&self, transactions: Vec<CardanoTransaction>) -> StdResult<()>;
 
+    /// Get the interval of blocks whose merkle root has yet to be computed
+    async fn get_block_interval_without_block_range_root(
+        &self,
+    ) -> StdResult<Option<Range<BlockNumber>>>;
+
+    /// Get transactions between two block numbers
+    async fn get_transactions_between(
+        &self,
+        range: Range<BlockNumber>,
+    ) -> StdResult<Vec<CardanoTransaction>>;
+
     /// Store list of block ranges with their corresponding merkle root
     async fn store_block_ranges(
         &self,
@@ -134,7 +134,7 @@ impl CardanoTransactionsImporter {
             }
             Some(range) => {
                 let block_ranges =
-                    BlockRange::all_ranges_in(BlockRange::start(*range.start())..=*range.end());
+                    BlockRange::all_ranges_in(BlockRange::start(range.start)..range.end);
 
                 if block_ranges.is_empty() {
                     return Ok(());
@@ -144,7 +144,7 @@ impl CardanoTransactionsImporter {
                 for block_range in block_ranges {
                     let transactions = self
                         .transaction_store
-                        .get_transactions_between(block_range.start..=(block_range.end - 1))
+                        .get_transactions_between(block_range.start..block_range.end)
                         .await?;
                     let merkle_root = MKTree::new(&transactions)?.compute_root()?;
                     block_ranges_with_merkle_root.push((block_range, merkle_root));
@@ -468,10 +468,8 @@ mod tests {
     #[tokio::test]
     // async fn block_range_root_compute_work_on_block_range_starting_before_last_block_range_root_and_finishing_before_last_tx(
     async fn block_range_root_only_retrieve_only_strictly_required_transactions() {
-        fn transactions_for_block(
-            range: RangeInclusive<BlockNumber>,
-        ) -> StdResult<Vec<CardanoTransaction>> {
-            Ok(build_blocks(*range.start(), range.count() as BlockNumber)
+        fn transactions_for_block(range: Range<BlockNumber>) -> StdResult<Vec<CardanoTransaction>> {
+            Ok(build_blocks(range.start, range.count() as BlockNumber)
                 .iter()
                 .flat_map(|b| b.clone().into_transactions())
                 .collect())
@@ -484,11 +482,7 @@ mod tests {
                 // Specification of the interval without block range root
                 // Note: in reality the lower bound will always be a multiple of BlockRange::LENGTH
                 // since it's computed from the `block_range_root` table
-                .returning(|| {
-                    Ok(Some(
-                        (BlockRange::LENGTH + 2)..=(BlockRange::LENGTH * 5 + 1),
-                    ))
-                })
+                .returning(|| Ok(Some((BlockRange::LENGTH + 2)..(BlockRange::LENGTH * 5))))
                 .once();
             store_mock
                 .expect_get_transactions_between()
@@ -501,7 +495,7 @@ mod tests {
                 // included in a block range
                 .withf(|range| {
                     let expected_range = BlockRange::LENGTH..=(BlockRange::LENGTH * 5);
-                    expected_range.contains(range.start()) && expected_range.contains(range.end())
+                    expected_range.contains(&range.start) && expected_range.contains(&range.end)
                 })
                 .returning(transactions_for_block);
             store_mock.expect_store_block_ranges().returning(|_| Ok(()));
diff --git a/mithril-signer/src/database/provider/cardano_transaction/get_cardano_transaction.rs b/mithril-signer/src/database/provider/cardano_transaction/get_cardano_transaction.rs
@@ -1,5 +1,5 @@
 use sqlite::Value;
-use std::ops::RangeInclusive;
+use std::ops::Range;
 
 use mithril_common::entities::{BlockNumber, ImmutableFileNumber, TransactionHash};
 use mithril_persistence::sqlite::{
@@ -42,15 +42,15 @@ impl<'client> GetCardanoTransactionProvider<'client> {
 
     pub fn get_transaction_between_blocks_condition(
         &self,
-        range: RangeInclusive<BlockNumber>,
+        range: Range<BlockNumber>,
     ) -> WhereCondition {
         WhereCondition::new(
             "block_number >= ?*",
-            vec![Value::Integer(*range.start() as i64)],
+            vec![Value::Integer(range.start as i64)],
         )
         .and_where(WhereCondition::new(
-            "block_number <= ?*",
-            vec![Value::Integer(*range.end() as i64)],
+            "block_number < ?*",
+            vec![Value::Integer(range.end as i64)],
         ))
     }
 }
diff --git a/mithril-signer/src/database/repository/cardano_transaction_repository.rs b/mithril-signer/src/database/repository/cardano_transaction_repository.rs
@@ -1,4 +1,4 @@
-use std::ops::RangeInclusive;
+use std::ops::Range;
 use std::sync::Arc;
 
 use anyhow::Context;
@@ -46,7 +46,7 @@ impl CardanoTransactionRepository {
     /// Return all the [CardanoTransactionRecord]s in the database using chronological order.
     pub async fn get_transactions_between_blocks(
         &self,
-        range: RangeInclusive<BlockNumber>,
+        range: Range<BlockNumber>,
     ) -> StdResult<Vec<CardanoTransactionRecord>> {
         let provider = GetCardanoTransactionProvider::new(&self.connection);
         let filters = provider.get_transaction_between_blocks_condition(range);
@@ -180,9 +180,27 @@ impl TransactionStore for CardanoTransactionRepository {
         }
     }
 
+    async fn get_up_to(&self, beacon: ImmutableFileNumber) -> StdResult<Vec<CardanoTransaction>> {
+        self.get_transactions_up_to(beacon).await.map(|v| {
+            v.into_iter()
+                .map(|record| record.into())
+                .collect::<Vec<CardanoTransaction>>()
+        })
+    }
+
+    async fn store_transactions(&self, transactions: Vec<CardanoTransaction>) -> StdResult<()> {
+        // Chunk transactions to avoid an error when we exceed sqlite binding limitations
+        for transactions_in_chunk in transactions.chunks(100) {
+            self.create_transactions(transactions_in_chunk.to_vec())
+                .await
+                .with_context(|| "CardanoTransactionRepository can not store transactions")?;
+        }
+        Ok(())
+    }
+
     async fn get_block_interval_without_block_range_root(
         &self,
-    ) -> StdResult<Option<RangeInclusive<BlockNumber>>> {
+    ) -> StdResult<Option<Range<BlockNumber>>> {
         let provider = GetIntervalWithoutBlockRangeRootProvider::new(&self.connection);
         let row = provider
             .find(provider.get_interval_without_block_range_condition())?
@@ -193,7 +211,7 @@ impl TransactionStore for CardanoTransactionRepository {
             None => panic!("IntervalWithoutBlockRangeProvider should always return a single row"),
             Some(interval) => match (interval.start, interval.end) {
                 (_, None) => Ok(None),
-                (None, Some(end)) => Ok(Some(0..=end)),
+                (None, Some(end)) => Ok(Some(0..(end + 1))),
                 (Some(start), Some(end)) if end < start => {
                     // To discuss : should we prune all block ranges from the DB to force recompute ?
                     warn!(
@@ -204,14 +222,14 @@ impl TransactionStore for CardanoTransactionRepository {
                     );
                     Ok(None)
                 }
-                (Some(start), Some(end)) => Ok(Some(start..=end)),
+                (Some(start), Some(end)) => Ok(Some(start..(end + 1))),
             },
         }
     }
 
     async fn get_transactions_between(
         &self,
-        range: RangeInclusive<BlockNumber>,
+        range: Range<BlockNumber>,
     ) -> StdResult<Vec<CardanoTransaction>> {
         self.get_transactions_between_blocks(range).await.map(|v| {
             v.into_iter()
@@ -220,24 +238,6 @@ impl TransactionStore for CardanoTransactionRepository {
         })
     }
 
-    async fn get_up_to(&self, beacon: ImmutableFileNumber) -> StdResult<Vec<CardanoTransaction>> {
-        self.get_transactions_up_to(beacon).await.map(|v| {
-            v.into_iter()
-                .map(|record| record.into())
-                .collect::<Vec<CardanoTransaction>>()
-        })
-    }
-
-    async fn store_transactions(&self, transactions: Vec<CardanoTransaction>) -> StdResult<()> {
-        // Chunk transactions to avoid an error when we exceed sqlite binding limitations
-        for transactions_in_chunk in transactions.chunks(100) {
-            self.create_transactions(transactions_in_chunk.to_vec())
-                .await
-                .with_context(|| "CardanoTransactionRepository can not store transactions")?;
-        }
-        Ok(())
-    }
-
     async fn store_block_ranges(
         &self,
         block_ranges: Vec<(BlockRange, MKTreeNode)>,
@@ -481,23 +481,23 @@ mod tests {
             .unwrap();
 
         {
-            let transaction_result = repository.get_transactions_between(0..=9).await.unwrap();
+            let transaction_result = repository.get_transactions_between(0..10).await.unwrap();
             assert_eq!(Vec::<CardanoTransaction>::new(), transaction_result);
         }
         {
-            let transaction_result = repository.get_transactions_between(13..=20).await.unwrap();
+            let transaction_result = repository.get_transactions_between(13..21).await.unwrap();
             assert_eq!(Vec::<CardanoTransaction>::new(), transaction_result);
         }
         {
-            let transaction_result = repository.get_transactions_between(9..=11).await.unwrap();
+            let transaction_result = repository.get_transactions_between(9..12).await.unwrap();
             assert_eq!(transactions[0..=1].to_vec(), transaction_result);
         }
         {
-            let transaction_result = repository.get_transactions_between(10..=12).await.unwrap();
+            let transaction_result = repository.get_transactions_between(10..13).await.unwrap();
             assert_eq!(transactions.clone(), transaction_result);
         }
         {
-            let transaction_result = repository.get_transactions_between(11..=13).await.unwrap();
+            let transaction_result = repository.get_transactions_between(11..14).await.unwrap();
             assert_eq!(transactions[1..=2].to_vec(), transaction_result);
         }
     }
@@ -529,7 +529,7 @@ mod tests {
                 .await
                 .unwrap();
 
-            assert_eq!(Some(0..=last_transaction_block_number), interval);
+            assert_eq!(Some(0..(last_transaction_block_number + 1)), interval);
         }
         {
             // The last block range give the lower bound
@@ -548,7 +548,7 @@ mod tests {
                 .unwrap();
 
             assert_eq!(
-                Some(last_block_range.end..=last_transaction_block_number),
+                Some(last_block_range.end..(last_transaction_block_number + 1)),
                 interval
             );
         }
