Merge pull request #1566 from input-output-hk/ensemble/handle-pallas-hardano-parsing-error

Handle `pallas-hardano` parsing errors

Author: dlachaume

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-aggregator"
-version = "0.4.46"
+version = "0.4.47"
 description = "A Mithril Aggregator server"
 authors = { workspace = true }
 edition = { workspace = true }

mithril-aggregator/Cargo.toml

@@ -746,7 +746,10 @@ impl DependenciesBuilder {
     }
 
     async fn build_transaction_parser(&mut self) -> Result<Arc<dyn TransactionParser>> {
-        let transaction_parser = CardanoTransactionParser::default();
+        // TODO: 'allow_unparsable_block' parameter should be configurable and its default value set to false
+        let allow_unparsable_block = true;
+        let transaction_parser =
+            CardanoTransactionParser::new(self.get_logger().await?, allow_unparsable_block);
 
         Ok(Arc::new(transaction_parser))
     }

mithril-aggregator/src/dependency_injection/builder.rs

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-common"
-version = "0.3.17"
+version = "0.3.18"
 description = "Common types, interfaces, and utilities for Mithril nodes."
 authors = { workspace = true }
 edition = { workspace = true }

mithril-common/Cargo.toml

@@ -8,6 +8,7 @@ use anyhow::{anyhow, Context};
 use async_trait::async_trait;
 use pallas_hardano::storage::immutable::chunk::{read_blocks, Reader};
 use pallas_traverse::MultiEraBlock;
+use slog::{error, Logger};
 use std::path::Path;
 use tokio::sync::RwLock;
 
@@ -87,35 +88,43 @@ struct Block {
 }
 
 impl Block {
-    fn try_convert(
-        multi_era_block: MultiEraBlock,
-        immutable_file_number: ImmutableFileNumber,
-    ) -> StdResult<Self> {
+    fn convert(multi_era_block: MultiEraBlock, immutable_file_number: ImmutableFileNumber) -> Self {
         let mut transactions = Vec::new();
         for tx in &multi_era_block.txs() {
             transactions.push(tx.hash().to_string());
         }
-        let block = Block {
+
+        Block {
             block_number: multi_era_block.number(),
             immutable_file_number,
             transactions,
-        };
-
-        Ok(block)
+        }
     }
 }
 
 /// Cardano transaction parser
-pub struct CardanoTransactionParser {}
+pub struct CardanoTransactionParser {
+    logger: Logger,
+    /// When set to true, no error is returned in case of unparsable block, and an error log is written instead.
+    /// This can occur when the crate 'pallas-hardano' doesn't support some non final encoding for a Cardano era.
+    /// This situation should only happen on the test networks and not on the mainnet.
+    allow_unparsable_block: bool,
+}
 
 impl CardanoTransactionParser {
     /// Factory
-    pub fn new() -> Self {
-        Self {}
+    pub fn new(logger: Logger, allow_unparsable_block: bool) -> Self {
+        Self {
+            logger,
+            allow_unparsable_block,
+        }
     }
 
     /// Read blocks from immutable file
-    fn read_blocks_from_immutable_file(immutable_file: &ImmutableFile) -> StdResult<Vec<Block>> {
+    fn read_blocks_from_immutable_file(
+        &self,
+        immutable_file: &ImmutableFile,
+    ) -> StdResult<Vec<Block>> {
         let cardano_blocks_reader =
             CardanoTransactionParser::cardano_blocks_reader(immutable_file)?;
 
@@ -127,10 +136,19 @@ impl CardanoTransactionParser {
                     immutable_file.path
                 )
             })?;
-            blocks.push(CardanoTransactionParser::convert_to_block(
-                &block,
-                immutable_file,
-            )?);
+            match CardanoTransactionParser::convert_to_block(&block, immutable_file) {
+                Ok(convert_to_block) => {
+                    blocks.push(convert_to_block);
+                }
+                Err(err) if self.allow_unparsable_block => {
+                    error!(
+                        self.logger,
+                        "The cbor encoded block could not be parsed";
+                        "error" => ?err, "immutable_file_number" => immutable_file.number
+                    );
+                }
+                Err(e) => return Err(e),
+            }
         }
         Ok(blocks)
     }
@@ -143,12 +161,7 @@ impl CardanoTransactionParser {
             )
         })?;
 
-        Block::try_convert(multi_era_block, immutable_file.number).with_context(|| {
-            format!(
-                "CardanoTransactionParser could not read data from block in immutable file: {:?}",
-                immutable_file.path
-            )
-        })
+        Ok(Block::convert(multi_era_block, immutable_file.number))
     }
 
     fn cardano_blocks_reader(immutable_file: &ImmutableFile) -> StdResult<Reader> {
@@ -169,12 +182,6 @@ impl CardanoTransactionParser {
     }
 }
 
-impl Default for CardanoTransactionParser {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
 #[async_trait]
 impl TransactionParser for CardanoTransactionParser {
     async fn parse(&self, dirpath: &Path, beacon: &Beacon) -> StdResult<Vec<CardanoTransaction>> {
@@ -186,8 +193,9 @@ impl TransactionParser for CardanoTransactionParser {
         let mut transactions: Vec<CardanoTransaction> = vec![];
 
         for immutable_file in &immutable_chunks {
-            let blocks =
-                Self::read_blocks_from_immutable_file(immutable_file).with_context(|| {
+            let blocks = self
+                .read_blocks_from_immutable_file(immutable_file)
+                .with_context(|| {
                     format!(
                         "CardanoTransactionParser could read blocks from immutable file: '{}'.",
                         immutable_file.path.display()
@@ -219,6 +227,11 @@ mod tests {
 
     use super::*;
 
+    use crate::test_utils::TempDir;
+
+    use slog::Drain;
+    use std::{fs::File, sync::Arc};
+
     fn get_number_of_immutable_chunk_in_dir(dir: &Path) -> usize {
         ImmutableFile::list_completed_in_dir(dir)
             .unwrap()
@@ -227,6 +240,15 @@ mod tests {
             .len()
     }
 
+    fn create_file_logger(filepath: &Path) -> slog::Logger {
+        let writer = File::create(filepath).unwrap();
+        let decorator = slog_term::PlainDecorator::new(writer);
+        let drain = slog_term::CompactFormat::new(decorator).build().fuse();
+        let drain = slog_async::Async::new(drain).build().fuse();
+
+        Logger::root(Arc::new(drain), slog::o!())
+    }
+
     #[tokio::test]
     async fn test_parse_expected_number_of_transactions() {
         // We known the number of transactions in those prebuilt immutables
@@ -239,7 +261,8 @@ mod tests {
             ..Beacon::default()
         };
         let tx_count: usize = immutable_files.iter().map(|(_, count)| *count).sum();
-        let cardano_transaction_parser = CardanoTransactionParser::new();
+        let cardano_transaction_parser =
+            CardanoTransactionParser::new(Logger::root(slog::Discard, slog::o!()), false);
 
         let transactions = cardano_transaction_parser
             .parse(db_path, &beacon)
@@ -249,6 +272,48 @@ mod tests {
         assert_eq!(transactions.len(), tx_count);
     }
 
+    #[tokio::test]
+    async fn test_parse_should_error_with_unparsable_block_format() {
+        let db_path = Path::new("../mithril-test-lab/test_data/parsing_error/immutable/");
+        let beacon = Beacon {
+            immutable_file_number: 4831,
+            ..Beacon::default()
+        };
+        let cardano_transaction_parser =
+            CardanoTransactionParser::new(Logger::root(slog::Discard, slog::o!()), false);
+
+        let result = cardano_transaction_parser.parse(db_path, &beacon).await;
+
+        assert!(result.is_err());
+    }
+
+    #[tokio::test]
+    async fn test_parse_should_log_error_with_unparsable_block_format() {
+        let temp_dir = TempDir::create(
+            "cardano_transaction_parser",
+            "test_parse_should_log_error_with_unparsable_block_format",
+        );
+        let filepath = temp_dir.join("test.log");
+        let db_path = Path::new("../mithril-test-lab/test_data/parsing_error/immutable/");
+        let beacon = Beacon {
+            immutable_file_number: 4831,
+            ..Beacon::default()
+        };
+        // We create a block to drop the logger and force a flush before we read the log file.
+        {
+            let cardano_transaction_parser =
+                CardanoTransactionParser::new(create_file_logger(&filepath), true);
+
+            cardano_transaction_parser
+                .parse(db_path, &beacon)
+                .await
+                .unwrap();
+        }
+
+        let log_file = std::fs::read_to_string(&filepath).unwrap();
+        assert!(log_file.contains("The cbor encoded block could not be parsed"));
+    }
+
     #[tokio::test]
     async fn test_parse_up_to_given_beacon() {
         // We known the number of transactions in those prebuilt immutables
@@ -261,7 +326,8 @@ mod tests {
             ..Beacon::default()
         };
         let tx_count: usize = immutable_files.iter().map(|(_, count)| *count).sum();
-        let cardano_transaction_parser = CardanoTransactionParser::new();
+        let cardano_transaction_parser =
+            CardanoTransactionParser::new(Logger::root(slog::Discard, slog::o!()), false);
 
         let transactions = cardano_transaction_parser
             .parse(db_path, &beacon)

mithril-common/src/cardano_transaction_parser.rs

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-signer"
-version = "0.2.114"
+version = "0.2.115"
 description = "A Mithril Signer"
 authors = { workspace = true }
 edition = { workspace = true }

mithril-signer/Cargo.toml

@@ -267,7 +267,12 @@ impl<'a> ServiceBuilder for ProductionServiceBuilder<'a> {
             ));
         let mithril_stake_distribution_signable_builder =
             Arc::new(MithrilStakeDistributionSignableBuilder::default());
-        let transaction_parser = Arc::new(CardanoTransactionParser::default());
+        // TODO: 'allow_unparsable_block' parameter should be configurable and its default value set to false
+        let allow_unparsable_block = true;
+        let transaction_parser = Arc::new(CardanoTransactionParser::new(
+            slog_scope::logger(),
+            allow_unparsable_block,
+        ));
         let transaction_store = Arc::new(CardanoTransactionRepository::new(
             transaction_sqlite_connection,
         ));

mithril-signer/src/runtime/signer_services.rs

@@ -7,3 +7,5 @@
 > [!NOTE]
 > The immutable files (`.chunk`, `.primary` and `.secondary`) files are data that are the result of
 > the `mithril-end-to-end` test command execution.
+> The `parsing_error/` directory contains the `04831` and `04832` immutable files with some unparsable blocks in the first one (`04831` have been produced by the Sanchonet network, and `04832` created manually).
+> They are needed for testing of the Cardano transactions parser.