From 5a30b7d1730bf9941e0b3c17714342c8f9ba52f6 Mon Sep 17 00:00:00 2001 From: Hubert Bugaj Date: Thu, 17 Jul 2025 17:47:45 +0200 Subject: [PATCH 1/4] fix: remove bad block reason [skip ci] --- src/chain_sync/bad_block_cache.rs | 8 ++++---- src/chain_sync/chain_follower.rs | 23 +++++++++-------------- src/chain_sync/validation.rs | 8 ++++---- src/rpc/methods/sync.rs | 3 ++- 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/chain_sync/bad_block_cache.rs b/src/chain_sync/bad_block_cache.rs index 6b6f2b278a67..cbf1ee6807e5 100644 --- a/src/chain_sync/bad_block_cache.rs +++ b/src/chain_sync/bad_block_cache.rs @@ -13,7 +13,7 @@ use parking_lot::Mutex; /// work. #[derive(Debug)] pub struct BadBlockCache { - cache: Mutex>, + cache: Mutex>, } impl Default for BadBlockCache { @@ -30,13 +30,13 @@ impl BadBlockCache { } /// Puts a bad block `Cid` in the cache with a given reason. - pub fn put(&self, c: Cid, reason: String) -> Option { - self.cache.lock().put(c, reason) + pub fn put(&self, c: Cid) -> Option<()> { + self.cache.lock().put(c, ()) } /// Returns `Some` with the reason 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 { + pub fn peek(&self, c: &Cid) -> Option<()> { self.cache.lock().peek(c).cloned() } } diff --git a/src/chain_sync/chain_follower.rs b/src/chain_sync/chain_follower.rs index 524540eee5b5..7b798c756f78 100644 --- a/src/chain_sync/chain_follower.rs +++ b/src/chain_sync/chain_follower.rs @@ -507,7 +507,7 @@ fn load_full_tipset( enum SyncEvent { NewFullTipsets(Vec>), - BadTipset(Arc, String), + BadTipset(Arc), ValidatedTipset { tipset: Arc, 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 SyncStateMachine { ) { 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 SyncStateMachine { 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,7 +686,7 @@ impl SyncStateMachine { // 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, reason: String) { + fn mark_bad_tipset(&mut self, tipset: Arc) { let mut stack = vec![tipset]; while let Some(tipset) = stack.pop() { @@ -699,7 +694,7 @@ impl SyncStateMachine { // 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 SyncStateMachine { 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)) } } } diff --git a/src/chain_sync/validation.rs b/src/chain_sync/validation.rs index bd7062c3006a..545876f6ce57 100644 --- a/src/chain_sync/validation.rs +++ b/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())); } } } diff --git a/src/rpc/methods/sync.rs b/src/rpc/methods/sync.rs index 25357b61fe21..7263a16d7cd4 100644 --- a/src/rpc/methods/sync.rs +++ b/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(()) } } From 21caa5961d5617dc0f1c3c249479240a4ef9ad6a Mon Sep 17 00:00:00 2001 From: Hubert Bugaj Date: Fri, 18 Jul 2025 12:20:18 +0200 Subject: [PATCH 2/4] chore: changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a25d2eec3449..fbae9492aa47 100644 --- a/CHANGELOG.md +++ b/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 From 165700f49520d5177b54e96902e0f8e2e5ac3b64 Mon Sep 17 00:00:00 2001 From: Hubert Bugaj Date: Fri, 18 Jul 2025 12:23:11 +0200 Subject: [PATCH 3/4] chore: update comments --- src/chain_sync/bad_block_cache.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/chain_sync/bad_block_cache.rs b/src/chain_sync/bad_block_cache.rs index cbf1ee6807e5..8bd78db1f121 100644 --- a/src/chain_sync/bad_block_cache.rs +++ b/src/chain_sync/bad_block_cache.rs @@ -29,12 +29,11 @@ impl BadBlockCache { } } - /// Puts a bad block `Cid` in the cache with a given reason. - pub fn put(&self, c: Cid) -> Option<()> { - self.cache.lock().put(c, ()) + 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<()> { self.cache.lock().peek(c).cloned() From f6dee424e50190d285641101f79719f253245111 Mon Sep 17 00:00:00 2001 From: Hubert Bugaj Date: Fri, 18 Jul 2025 12:33:55 +0200 Subject: [PATCH 4/4] fix test --- src/rpc/methods/sync.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/methods/sync.rs b/src/rpc/methods/sync.rs index 7263a16d7cd4..84ceb99a14bf 100644 --- a/src/rpc/methods/sync.rs +++ b/src/rpc/methods/sync.rs @@ -266,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]