feat: add bad block reason (#6235)

Description
---
add a reason to the bad block db to track why a block was marked as bad

Motivation and Context
---
Helps to track issues on bad blocks.

base_layer/core/src/base_node/comms_interface/inbound_handlers.rs

@@ -556,15 +556,19 @@ where B: BlockchainBackend + 'static
             );
             return Ok(true);
         }
-        if self.blockchain_db.bad_block_exists(block).await? {
+        let block_exist = self.blockchain_db.bad_block_exists(block).await?;
+        if block_exist.0 {
             debug!(
                 target: LOG_TARGET,
-                "Block with hash `{}` already validated as a bad block",
-                block.to_hex()
+                "Block with hash `{}` already validated as a bad block due to {}",
+                block.to_hex(), block_exist.1
             );
             return Err(CommsInterfaceError::ChainStorageError(
                 ChainStorageError::ValidationError {
-                    source: ValidationError::BadBlockFound { hash: block.to_hex() },
+                    source: ValidationError::BadBlockFound {
+                        hash: block.to_hex(),
+                        reason: block_exist.1,
+                    },
                 },
             ));
         }

base_layer/core/src/base_node/sync/block_sync/synchronizer.rs

@@ -336,7 +336,7 @@ impl<'a, B: BlockchainBackend + 'static> BlockSynchronizer<'a, B> {
                         .db
                         .write_transaction()
                         .delete_orphan(header_hash)
-                        .insert_bad_block(header_hash, current_height)
+                        .insert_bad_block(header_hash, current_height, err.to_string())
                         .commit()
                         .await
                     {

base_layer/core/src/base_node/sync/header_sync/validator.rs

@@ -146,7 +146,7 @@ impl<B: BlockchainBackend + 'static> BlockHeaderSyncValidator<B> {
 
             Err(e) => {
                 let mut txn = self.db.write_transaction();
-                txn.insert_bad_block(header.hash(), header.height);
+                txn.insert_bad_block(header.hash(), header.height, e.to_string());
                 txn.commit().await?;
                 return Err(e.into());
             },

base_layer/core/src/chain_storage/async_db.rs

@@ -239,9 +239,9 @@ impl<B: BlockchainBackend + 'static> AsyncBlockchainDb<B> {
 
     make_async_fn!(chain_header_or_orphan_exists(block_hash: BlockHash) -> bool, "header_exists");
 
-    make_async_fn!(bad_block_exists(block_hash: BlockHash) -> bool, "bad_block_exists");
+    make_async_fn!(bad_block_exists(block_hash: BlockHash) -> (bool, String), "bad_block_exists");
 
-    make_async_fn!(add_bad_block(hash: BlockHash, height: u64) -> (), "add_bad_block");
+    make_async_fn!(add_bad_block(hash: BlockHash, height: u64, reason: String) -> (), "add_bad_block");
 
     make_async_fn!(fetch_block(height: u64, compact: bool) -> HistoricalBlock, "fetch_block");
 
@@ -403,8 +403,8 @@ impl<'a, B: BlockchainBackend + 'static> AsyncDbTransaction<'a, B> {
         self
     }
 
-    pub fn insert_bad_block(&mut self, hash: HashOutput, height: u64) -> &mut Self {
-        self.transaction.insert_bad_block(hash, height);
+    pub fn insert_bad_block(&mut self, hash: HashOutput, height: u64, reason: String) -> &mut Self {
+        self.transaction.insert_bad_block(hash, height, reason);
         self
     }
 

base_layer/core/src/chain_storage/blockchain_backend.rs

@@ -165,7 +165,7 @@ pub trait BlockchainBackend: Send + Sync {
     fn fetch_total_size_stats(&self) -> Result<DbTotalSizeStats, ChainStorageError>;
 
     /// Check if a block hash is in the bad block list
-    fn bad_block_exists(&self, block_hash: HashOutput) -> Result<bool, ChainStorageError>;
+    fn bad_block_exists(&self, block_hash: HashOutput) -> Result<(bool, String), ChainStorageError>;
 
     /// Fetches all tracked reorgs
     fn fetch_all_reorgs(&self) -> Result<Vec<Reorg>, ChainStorageError>;

base_layer/core/src/chain_storage/blockchain_database.rs

@@ -975,10 +975,12 @@ where B: BlockchainBackend
         if db.contains(&DbKey::HeaderHash(block_hash))? {
             return Ok(BlockAddResult::BlockExists);
         }
-        if db.bad_block_exists(block_hash)? {
+        let block_exist = db.bad_block_exists(block_hash)?;
+        if block_exist.0 {
             return Err(ChainStorageError::ValidationError {
                 source: ValidationError::BadBlockFound {
                     hash: block_hash.to_hex(),
+                    reason: block_exist.1,
                 },
             });
         }
@@ -1152,16 +1154,16 @@ where B: BlockchainBackend
     }
 
     /// Returns true if this block exists in the chain, or is orphaned.
-    pub fn bad_block_exists(&self, hash: BlockHash) -> Result<bool, ChainStorageError> {
+    pub fn bad_block_exists(&self, hash: BlockHash) -> Result<(bool, String), ChainStorageError> {
         let db = self.db_read_access()?;
         db.bad_block_exists(hash)
     }
 
     /// Adds a block hash to the list of bad blocks so it wont get process again.
-    pub fn add_bad_block(&self, hash: BlockHash, height: u64) -> Result<(), ChainStorageError> {
+    pub fn add_bad_block(&self, hash: BlockHash, height: u64, reason: String) -> Result<(), ChainStorageError> {
         let mut db = self.db_write_access()?;
         let mut txn = DbTransaction::new();
-        txn.insert_bad_block(hash, height);
+        txn.insert_bad_block(hash, height, reason);
         db.write(txn)
     }
 
@@ -1914,7 +1916,7 @@ fn reorganize_chain<T: BlockchainBackend>(
                 e
             );
             if e.get_ban_reason().is_some() && e.get_ban_reason().unwrap().ban_duration != BanPeriod::Short {
-                txn.insert_bad_block(block.header().hash(), block.header().height);
+                txn.insert_bad_block(block.header().hash(), block.header().height, e.to_string());
             }
             // We removed a block from the orphan chain, so the chain is now "broken", so we remove the rest of the
             // remaining blocks as well.
@@ -2163,7 +2165,7 @@ fn insert_orphan_and_find_new_tips<T: BlockchainBackend>(
         },
 
         Err(e) => {
-            txn.insert_bad_block(candidate_block.header.hash(), candidate_block.header.height);
+            txn.insert_bad_block(candidate_block.header.hash(), candidate_block.header.height, e.to_string());
             db.write(txn)?;
             return Err(e.into());
         },

base_layer/core/src/chain_storage/db_transaction.rs

@@ -177,10 +177,11 @@ impl DbTransaction {
     }
 
     /// Inserts a block hash into the bad block list
-    pub fn insert_bad_block(&mut self, block_hash: HashOutput, height: u64) -> &mut Self {
+    pub fn insert_bad_block(&mut self, block_hash: HashOutput, height: u64, reason: String) -> &mut Self {
         self.operations.push(WriteOperation::InsertBadBlock {
             hash: block_hash,
             height,
+            reason,
         });
         self
     }
@@ -310,6 +311,7 @@ pub enum WriteOperation {
     InsertBadBlock {
         hash: HashOutput,
         height: u64,
+        reason: String,
     },
     DeleteHeader(u64),
     DeleteOrphan(HashOutput),
@@ -446,7 +448,9 @@ impl fmt::Display for WriteOperation {
             SetPrunedHeight { height, .. } => write!(f, "Set pruned height to {}", height),
             DeleteHeader(height) => write!(f, "Delete header at height: {}", height),
             DeleteOrphan(hash) => write!(f, "Delete orphan with hash: {}", hash),
-            InsertBadBlock { hash, height } => write!(f, "Insert bad block #{} {}", height, hash),
+            InsertBadBlock { hash, height, reason } => {
+                write!(f, "Insert bad block #{} {} for {}", height, hash, reason)
+            },
             SetHorizonData { .. } => write!(f, "Set horizon data"),
             InsertReorg { .. } => write!(f, "Insert reorg"),
             ClearAllReorgs => write!(f, "Clear all reorgs"),

base_layer/core/src/chain_storage/lmdb_db/lmdb_db.rs

@@ -471,8 +471,8 @@ impl LMDBDatabase {
                         &MetadataValue::HorizonData(horizon_data.clone()),
                     )?;
                 },
-                InsertBadBlock { hash, height } => {
-                    self.insert_bad_block_and_cleanup(&write_txn, hash, *height)?;
+                InsertBadBlock { hash, height, reason } => {
+                    self.insert_bad_block_and_cleanup(&write_txn, hash, *height, reason.to_string())?;
                 },
                 InsertReorg { reorg } => {
                     lmdb_replace(&write_txn, &self.reorgs, &reorg.local_time.timestamp(), &reorg)?;
@@ -1586,13 +1586,14 @@ impl LMDBDatabase {
         txn: &WriteTransaction<'_>,
         hash: &HashOutput,
         height: u64,
+        reason: String,
     ) -> Result<(), ChainStorageError> {
         #[cfg(test)]
         const CLEAN_BAD_BLOCKS_BEFORE_REL_HEIGHT: u64 = 10000;
         #[cfg(not(test))]
         const CLEAN_BAD_BLOCKS_BEFORE_REL_HEIGHT: u64 = 0;
 
-        lmdb_replace(txn, &self.bad_blocks, hash.deref(), &height)?;
+        lmdb_replace(txn, &self.bad_blocks, hash.deref(), &(height, reason))?;
         // Clean up bad blocks that are far from the tip
         let metadata = fetch_metadata(txn, &self.metadata_db)?;
         let deleted_before_height = metadata
@@ -1602,8 +1603,9 @@ impl LMDBDatabase {
             return Ok(());
         }
 
-        let num_deleted =
-            lmdb_delete_each_where::<[u8], u64, _>(txn, &self.bad_blocks, |_, v| Some(v < deleted_before_height))?;
+        let num_deleted = lmdb_delete_each_where::<[u8], (u64, String), _>(txn, &self.bad_blocks, |_, (v, _)| {
+            Some(v < deleted_before_height)
+        })?;
         debug!(target: LOG_TARGET, "Cleaned out {} stale bad blocks", num_deleted);
 
         Ok(())
@@ -2388,9 +2390,22 @@ impl BlockchainBackend for LMDBDatabase {
             .collect()
     }
 
-    fn bad_block_exists(&self, block_hash: HashOutput) -> Result<bool, ChainStorageError> {
+    fn bad_block_exists(&self, block_hash: HashOutput) -> Result<(bool, String), ChainStorageError> {
         let txn = self.read_transaction()?;
-        lmdb_exists(&txn, &self.bad_blocks, block_hash.deref())
+        // We do this to ensure backwards compatibility on older exising dbs that did not store a reason
+        let exist = lmdb_exists(&txn, &self.bad_blocks, block_hash.deref())?;
+        match lmdb_get::<_, (u64, String)>(&txn, &self.bad_blocks, block_hash.deref()) {
+            Ok(Some((_height, reason))) => Ok((true, reason)),
+            Ok(None) => Ok((false, "".to_string())),
+            Err(ChainStorageError::AccessError(e)) => {
+                if exist {
+                    Ok((true, "No reason recorded".to_string()))
+                } else {
+                    Err(ChainStorageError::AccessError(e))
+                }
+            },
+            Err(e) => Err(e),
+        }
     }
 
     fn clear_all_pending_headers(&self) -> Result<usize, ChainStorageError> {

base_layer/core/src/test_helpers/blockchain.rs

@@ -392,7 +392,7 @@ impl BlockchainBackend for TempDatabase {
         self.db.as_ref().unwrap().fetch_total_size_stats()
     }
 
-    fn bad_block_exists(&self, block_hash: HashOutput) -> Result<bool, ChainStorageError> {
+    fn bad_block_exists(&self, block_hash: HashOutput) -> Result<(bool, String), ChainStorageError> {
         self.db.as_ref().unwrap().bad_block_exists(block_hash)
     }
 

base_layer/core/src/validation/error.rs

@@ -89,7 +89,7 @@ pub enum ValidationError {
     #[error("Expected block previous hash to be {expected}, but was {block_hash}")]
     IncorrectPreviousHash { expected: String, block_hash: String },
     #[error("Bad block with hash {hash} found")]
-    BadBlockFound { hash: String },
+    BadBlockFound { hash: String, reason: String },
     #[error("Script exceeded maximum script size, expected less than {max_script_size} but was {actual_script_size}")]
     TariScriptExceedsMaxSize {
         max_script_size: usize,

base_layer/core/src/validation/header/header_full_validator.rs

@@ -174,8 +174,12 @@ pub fn check_timestamp_ftl(
 }
 
 fn check_not_bad_block<B: BlockchainBackend>(db: &B, hash: FixedHash) -> Result<(), ValidationError> {
-    if db.bad_block_exists(hash)? {
-        return Err(ValidationError::BadBlockFound { hash: hash.to_hex() });
+    let block_exist = db.bad_block_exists(hash)?;
+    if block_exist.0 {
+        return Err(ValidationError::BadBlockFound {
+            hash: hash.to_hex(),
+            reason: block_exist.1,
+        });
     }
     Ok(())
 }