From 733fc67d99e949d2ddfb77cfa20e7dca8cfa59a1 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Thu, 5 Sep 2024 17:17:11 +0200 Subject: [PATCH] fix: remove rolled-back transactions edge cases Does not trigger an error when no block exists above the slot number, also find the closest block above the slot number if rolled back block does not have transaction. --- .../cardano_transaction_repository.rs | 184 +++++++++++++++++- .../cardano_transaction_repository.rs | 8 +- .../cardano_transaction_repository.rs | 8 +- 3 files changed, 179 insertions(+), 21 deletions(-) diff --git a/internal/mithril-persistence/src/database/repository/cardano_transaction_repository.rs b/internal/mithril-persistence/src/database/repository/cardano_transaction_repository.rs index 97e19023ed3..969fc6759d7 100644 --- a/internal/mithril-persistence/src/database/repository/cardano_transaction_repository.rs +++ b/internal/mithril-persistence/src/database/repository/cardano_transaction_repository.rs @@ -215,12 +215,12 @@ impl CardanoTransactionRepository { Ok(()) } - /// Get the block number for a given slot number - pub async fn get_block_number_by_slot_number( + /// Get the closest block number above a given slot number + pub async fn get_closest_block_number_above_slot_number( &self, slot_number: SlotNumber, ) -> StdResult> { - let query = GetCardanoTransactionQuery::by_slot_number(slot_number); + let query = GetCardanoTransactionQuery::above_slot_number(slot_number); let record = self.connection_pool.connection()?.fetch_first(query)?; Ok(record.map(|r| r.block_number)) @@ -277,7 +277,7 @@ impl CardanoTransactionRepository { /// /// * Remove transactions with block number strictly greater than the given block number /// * Remove block range roots that have lower bound range strictly above the given block number - pub async fn remove_rolled_back_transactions_and_block_range( + pub async fn remove_rolled_back_transactions_and_block_range_by_block_number( &self, block_number: BlockNumber, ) -> StdResult<()> { @@ -293,6 +293,25 @@ impl CardanoTransactionRepository { Ok(()) } + + /// Remove transactions and block range roots that are in a rolled-back fork + /// + /// * Remove transactions with closest block number strictly greater than the given slot number if exists + /// * Remove block range roots that have lower bound range strictly above the aforementioned block number + pub async fn remove_rolled_back_transactions_and_block_range_by_slot_number( + &self, + slot_number: SlotNumber, + ) -> StdResult<()> { + if let Some(block_number) = self + .get_closest_block_number_above_slot_number(slot_number) + .await? + { + self.remove_rolled_back_transactions_and_block_range_by_block_number(block_number) + .await?; + } + + Ok(()) + } } #[async_trait] @@ -910,7 +929,7 @@ mod tests { } #[tokio::test] - async fn repository_get_block_number_by_slot_number() { + async fn repository_get_closest_block_number_by_slot_number() { let connection = cardano_tx_db_connection().unwrap(); let repository = CardanoTransactionRepository::new(Arc::new( SqliteConnectionPool::build_from_connection(connection), @@ -927,7 +946,7 @@ mod tests { .unwrap(); let transaction_block_number_retrieved = repository - .get_block_number_by_slot_number(SlotNumber(500)) + .get_closest_block_number_above_slot_number(SlotNumber(500)) .await .unwrap(); @@ -1215,10 +1234,161 @@ mod tests { .unwrap(); repository - .remove_rolled_back_transactions_and_block_range(BlockRange::LENGTH * 3) + .remove_rolled_back_transactions_and_block_range_by_block_number(BlockRange::LENGTH * 3) .await .unwrap(); assert_eq!(2, repository.get_all_transactions().await.unwrap().len()); assert_eq!(2, repository.get_all_block_range_root().unwrap().len()); } + + #[tokio::test] + async fn remove_rolled_back_transactions_and_block_range_by_slot_number() { + let repository = CardanoTransactionRepository::new(Arc::new( + SqliteConnectionPool::build(1, cardano_tx_db_connection).unwrap(), + )); + + repository + .create_transactions(vec![ + CardanoTransactionRecord::new( + "tx_hash-123", + BlockNumber(10), + SlotNumber(50), + "block_hash-123", + ), + CardanoTransactionRecord::new( + "tx_hash-456", + BlockNumber(11), + SlotNumber(51), + "block_hash-456", + ), + CardanoTransactionRecord::new( + "tx_hash-789", + BlockNumber(13), + SlotNumber(52), + "block_hash-789", + ), + CardanoTransactionRecord::new( + "tx_hash-000", + BlockNumber(101), + SlotNumber(100), + "block_hash-000", + ), + ]) + .await + .unwrap(); + + { + repository + .remove_rolled_back_transactions_and_block_range_by_block_number(BlockNumber(110)) + .await + .expect("Failed to remove rolled back transactions"); + + let transactions = repository + .get_all() + .await + .unwrap() + .into_iter() + .map(|v| v.into()) + .collect::>(); + assert_eq!( + vec![ + CardanoTransactionRecord::new( + "tx_hash-123", + BlockNumber(10), + SlotNumber(50), + "block_hash-123", + ), + CardanoTransactionRecord::new( + "tx_hash-456", + BlockNumber(11), + SlotNumber(51), + "block_hash-456", + ), + CardanoTransactionRecord::new( + "tx_hash-789", + BlockNumber(13), + SlotNumber(52), + "block_hash-789", + ), + CardanoTransactionRecord::new( + "tx_hash-000", + BlockNumber(101), + SlotNumber(100), + "block_hash-000", + ), + ], + transactions + ); + } + + { + repository + .remove_rolled_back_transactions_and_block_range_by_block_number(BlockNumber(13)) + .await + .expect("Failed to remove rolled back transactions"); + + let transactions = repository + .get_all() + .await + .unwrap() + .into_iter() + .map(|v| v.into()) + .collect::>(); + assert_eq!( + vec![ + CardanoTransactionRecord::new( + "tx_hash-123", + BlockNumber(10), + SlotNumber(50), + "block_hash-123", + ), + CardanoTransactionRecord::new( + "tx_hash-456", + BlockNumber(11), + SlotNumber(51), + "block_hash-456", + ), + CardanoTransactionRecord::new( + "tx_hash-789", + BlockNumber(13), + SlotNumber(52), + "block_hash-789", + ), + ], + transactions + ); + } + + { + repository + .remove_rolled_back_transactions_and_block_range_by_block_number(BlockNumber(11)) + .await + .expect("Failed to remove rolled back transactions"); + + let transactions = repository + .get_all() + .await + .unwrap() + .into_iter() + .map(|v| v.into()) + .collect::>(); + assert_eq!( + vec![ + CardanoTransactionRecord::new( + "tx_hash-123", + BlockNumber(10), + SlotNumber(50), + "block_hash-123", + ), + CardanoTransactionRecord::new( + "tx_hash-456", + BlockNumber(11), + SlotNumber(51), + "block_hash-456", + ), + ], + transactions + ); + } + } } diff --git a/mithril-aggregator/src/database/repository/cardano_transaction_repository.rs b/mithril-aggregator/src/database/repository/cardano_transaction_repository.rs index 01eb25be4a5..08eb4937d6b 100644 --- a/mithril-aggregator/src/database/repository/cardano_transaction_repository.rs +++ b/mithril-aggregator/src/database/repository/cardano_transaction_repository.rs @@ -51,13 +51,7 @@ impl TransactionStore for CardanoTransactionRepository { &self, slot_number: SlotNumber, ) -> StdResult<()> { - let block_number = self - .get_block_number_by_slot_number(slot_number) - .await? - .ok_or_else(|| { - anyhow::anyhow!("No block number found for slot number {}", slot_number) - })?; - self.remove_rolled_back_transactions_and_block_range(block_number) + self.remove_rolled_back_transactions_and_block_range_by_slot_number(slot_number) .await } } diff --git a/mithril-signer/src/database/repository/cardano_transaction_repository.rs b/mithril-signer/src/database/repository/cardano_transaction_repository.rs index 9119e0711d1..7ce47d3409b 100644 --- a/mithril-signer/src/database/repository/cardano_transaction_repository.rs +++ b/mithril-signer/src/database/repository/cardano_transaction_repository.rs @@ -51,13 +51,7 @@ impl TransactionStore for CardanoTransactionRepository { &self, slot_number: SlotNumber, ) -> StdResult<()> { - let block_number = self - .get_block_number_by_slot_number(slot_number) - .await? - .ok_or_else(|| { - anyhow::anyhow!("No block number found for slot number {}", slot_number) - })?; - self.remove_rolled_back_transactions_and_block_range(block_number) + self.remove_rolled_back_transactions_and_block_range_by_slot_number(slot_number) .await } }