fix: remove bad block reason

CHANGELOG.md

@@ -57,6 +57,8 @@
 
 - [#5730](https://github.com/ChainSafe/forest/issues/5730) Fixed various bugs in the mempool selection logic, including gas overpricing and incorrect message chain pruning. Additional logic was added to limit the number of messages in the block.
 
+- [#5842](https://github.com/ChainSafe/forest/pull/5842) Fixed a memory leak in the bad block cache that could lead to excessive memory usage over time.
+
 ## Forest v0.27.0 "Whisperer in Darkness"
 
 This is a non-mandatory but highly recommended release for all node operators. It introduces a fix for the forest node

src/chain_sync/bad_block_cache.rs

@@ -13,7 +13,7 @@ use parking_lot::Mutex;
 /// work.
 #[derive(Debug)]
 pub struct BadBlockCache {
-    cache: Mutex<LruCache<Cid, String>>,
+    cache: Mutex<LruCache<Cid, ()>>,
 }
 
 impl Default for BadBlockCache {
@@ -29,14 +29,13 @@ impl BadBlockCache {
         }
     }
 
-    /// Puts a bad block `Cid` in the cache with a given reason.
-    pub fn put(&self, c: Cid, reason: String) -> Option<String> {
-        self.cache.lock().put(c, reason)
+    pub fn put(&self, c: Cid) {
+        self.cache.lock().put(c, ());
     }
 
-    /// Returns `Some` with the reason if the block CID is in bad block cache.
+    /// Returns `Some` if the block CID is in bad block cache.
     /// This function does not update the head position of the `Cid` key.
-    pub fn peek(&self, c: &Cid) -> Option<String> {
+    pub fn peek(&self, c: &Cid) -> Option<()> {
         self.cache.lock().peek(c).cloned()
     }
 }

src/chain_sync/chain_follower.rs

@@ -507,7 +507,7 @@ fn load_full_tipset<DB: Blockstore>(
 
 enum SyncEvent {
     NewFullTipsets(Vec<Arc<FullTipset>>),
-    BadTipset(Arc<FullTipset>, String),
+    BadTipset(Arc<FullTipset>),
     ValidatedTipset {
         tipset: Arc<FullTipset>,
         is_proposed_head: bool,
@@ -526,13 +526,8 @@ impl std::fmt::Display for SyncEvent {
 
         match self {
             Self::NewFullTipsets(tss) => write!(f, "NewFullTipsets({})", tss_to_string(tss)),
-            Self::BadTipset(ts, reason) => {
-                write!(
-                    f,
-                    "BadTipset(reason: {reason}, epoch: {}, key: {})",
-                    ts.epoch(),
-                    ts.key()
-                )
+            Self::BadTipset(ts) => {
+                write!(f, "BadTipset(epoch: {}, key: {})", ts.epoch(), ts.key())
             }
             Self::ValidatedTipset {
                 tipset,
@@ -635,7 +630,7 @@ impl<DB: Blockstore> SyncStateMachine<DB> {
         ) {
             metrics::INVALID_TIPSET_TOTAL.inc();
             trace!("Skipping invalid tipset: {}", why);
-            self.mark_bad_tipset(tipset, why.to_string());
+            self.mark_bad_tipset(tipset);
             return;
         }
 
@@ -644,7 +639,7 @@ impl<DB: Blockstore> SyncStateMachine<DB> {
         let epoch_diff = heaviest.epoch() - tipset.epoch();
 
         if epoch_diff > self.cs.chain_config.policy.chain_finality {
-            self.mark_bad_tipset(tipset, "old tipset".to_string());
+            self.mark_bad_tipset(tipset);
             return;
         }
 
@@ -691,15 +686,15 @@ impl<DB: Blockstore> SyncStateMachine<DB> {
     // Mark blocks in tipset as bad.
     // Mark all descendants of tipsets as bad.
     // Remove all bad tipsets from the tipset map.
-    fn mark_bad_tipset(&mut self, tipset: Arc<FullTipset>, reason: String) {
+    fn mark_bad_tipset(&mut self, tipset: Arc<FullTipset>) {
         let mut stack = vec![tipset];
 
         while let Some(tipset) = stack.pop() {
             self.tipsets.remove(tipset.key());
             // Mark all blocks in the tipset as bad
             if let Some(bad_block_cache) = &self.bad_block_cache {
                 for block in tipset.blocks() {
-                    bad_block_cache.put(*block.cid(), reason.clone());
+                    bad_block_cache.put(*block.cid());
                 }
             }
 
@@ -760,7 +755,7 @@ impl<DB: Blockstore> SyncStateMachine<DB> {
                     self.add_full_tipset(tipset);
                 }
             }
-            SyncEvent::BadTipset(tipset, reason) => self.mark_bad_tipset(tipset, reason),
+            SyncEvent::BadTipset(tipset) => self.mark_bad_tipset(tipset),
             SyncEvent::ValidatedTipset {
                 tipset,
                 is_proposed_head,
@@ -876,7 +871,7 @@ impl SyncTask {
                     }),
                     Err(e) => {
                         warn!("Error validating tipset: {}", e);
-                        Some(SyncEvent::BadTipset(tipset, e.to_string()))
+                        Some(SyncEvent::BadTipset(tipset))
                     }
                 }
             }

src/chain_sync/validation.rs

@@ -26,8 +26,8 @@ pub enum TipsetValidationError {
     EpochTooLarge,
     #[error("Tipset has an insufficient weight")]
     InsufficientWeight,
-    #[error("Tipset block = [CID = {0}] is invalid: {1}")]
-    InvalidBlock(Cid, String),
+    #[error("Tipset block = [CID = {0}] is invalid")]
+    InvalidBlock(Cid),
     #[error("Tipset headers are invalid")]
     InvalidRoots,
     #[error("Tipset IPLD error: {0}")]
@@ -75,8 +75,8 @@ impl TipsetValidator<'_> {
         for block in self.0.blocks() {
             self.validate_msg_root(&chainstore.db, block)?;
             if let Some(bad_block_cache) = bad_block_cache {
-                if let Some(bad) = bad_block_cache.peek(block.cid()) {
-                    return Err(TipsetValidationError::InvalidBlock(*block.cid(), bad));
+                if bad_block_cache.peek(block.cid()).is_some() {
+                    return Err(TipsetValidationError::InvalidBlock(*block.cid()));
                 }
             }
         }

src/rpc/methods/sync.rs

@@ -36,6 +36,7 @@ impl RpcMethod<1> for SyncCheckBad {
             .as_ref()
             .context("bad block cache is disabled")?
             .peek(&cid)
+            .map(|_| "bad".to_string())
             .unwrap_or_default())
     }
 }
@@ -57,7 +58,7 @@ impl RpcMethod<1> for SyncMarkBad {
         ctx.bad_blocks
             .as_ref()
             .context("bad block cache is disabled")?
-            .put(cid, "Marked bad manually through RPC API".to_string());
+            .put(cid);
         Ok(())
     }
 }
@@ -265,7 +266,7 @@ mod tests {
         SyncMarkBad::handle(ctx.clone(), (cid,)).await.unwrap();
 
         let reason = SyncCheckBad::handle(ctx.clone(), (cid,)).await.unwrap();
-        assert_eq!(reason, "Marked bad manually through RPC API");
+        assert_eq!(reason, "bad");
     }
 
     #[tokio::test]