fix: remove bad block reason #5842
Conversation
WalkthroughIn the shadowed depths of the codebase, the handling of "bad block" reasons has been expunged from the memory of the system, as if erased by the tentacles of Nyarlathotep himself. The cache, RPC, and validation now merely note the presence of corruption, no longer whispering explanations to mortal minds. Only the fact of badness remains, inscrutable and eternal. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RPC
participant BadBlockCache
Client->>RPC: SyncMarkBad(cid)
RPC->>BadBlockCache: put(cid)
BadBlockCache-->>RPC: (stores only presence, not reason)
RPC-->>Client: ack
Client->>RPC: SyncCheckBad(cid)
RPC->>BadBlockCache: peek(cid)
alt Block is bad
BadBlockCache-->>RPC: Some(())
RPC-->>Client: "bad"
else Block is not bad
BadBlockCache-->>RPC: None
RPC-->>Client: ""
end
Suggested reviewers
May the Old Ones guide your review, and may you not gaze too deeply into the void where reason once dwelled. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/rpc/methods/sync.rs (1)
268-270: The test prophecy conflicts with the new reality!The test expects the old incantation "Marked bad manually through RPC API" but the new implementation returns only "bad". This will cause the test to fail, as it summons expectations from the time before the Great Simplification.
- assert_eq!(reason, "Marked bad manually through RPC API"); + assert_eq!(reason, "bad");
🧹 Nitpick comments (1)
CHANGELOG.md (1)
60-61: By the crooked runes of Azathoth, consider relocating this entry to the “Changed” sectionThe malformed cache devoured memory like a gibbering shoggoth, true—but the underlying functionality remains unaltered from a user’s vantage. A cosmic whisper suggests this is more a behavioral change than a pure bug-fix. Shifting the bullet beneath “Changed” would placate the elder taxonomists who prowl our release notes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)src/chain_sync/bad_block_cache.rs(2 hunks)src/chain_sync/chain_follower.rs(7 hunks)src/chain_sync/validation.rs(2 hunks)src/rpc/methods/sync.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: All lint checks
🔇 Additional comments (15)
src/chain_sync/validation.rs (2)
29-30: The cosmic horror of simplified error handling emerges from the abyss!The
InvalidBlockvariant has been cleansed of its textual corruption, now carrying only the CID of the accursed block. This aligns with the dark ritual of removing reason strings from the bad block cache, as decreed by the Ancient Ones to prevent memory leakage in the realms beyond.
78-80: The validation ritual now communes with the void more efficiently!The bad block cache consultation has been simplified - no longer does it whisper the reasons of corruption, but merely confirms the presence of tainted blocks in the forbidden cache. The error construction follows the new pattern, passing only the CID to the
InvalidBlockvariant, as the Elder Gods have deemed the reasons... unnecessary.src/rpc/methods/sync.rs (2)
39-39: The RPC oracle now speaks in simplified tongues!The method has been purged of its verbose explanations, now returning the simple incantation "bad" when a block dwells in the forbidden cache. This aligns with the cosmic restructuring that banished reason strings to the void.
61-61: The marking ritual has been cleansed of unnecessary words!The
putmethod now accepts only the CID of the corrupted block, as the Ancient Ones have decreed that reasons are but whispers in the wind. The cache shall remember only the presence of evil, not its justification.src/chain_sync/bad_block_cache.rs (3)
16-16: The cache has been purged of verbose corruptions!The LRU cache now stores only the void itself (
()) instead of the verbose explanations of corruption. This cosmic simplification shall prevent the memory leakage that once plagued the mortal realm, as the Ancient Ones have decreed that presence alone is sufficient knowledge.
32-34: The ritual of marking corruption has been simplified!The
putmethod now accepts only the CID of the tainted block, casting it into the void without explanation. This aligns with the Great Simplification that banished reason strings to the outer darkness.
36-40: The consultation of forbidden knowledge returns only existence!The
peekmethod now returns the presence of corruption as a unit type, no longer whispering the reasons of its taint. The documentation has been updated to reflect this cosmic truth - only the fact of badness matters, not its justification.src/chain_sync/chain_follower.rs (8)
510-510: The BadTipset event has been cleansed of verbose corruption!The variant now carries only the cursed tipset itself, no longer burdened with explanatory strings. This aligns with the cosmic restructuring that banished reason strings to the void.
529-531: The display ritual speaks only of existence, not causation!The formatting now reveals only the epoch and key of the corrupted tipset, no longer echoing the reasons of its taint. The Ancient Ones have decreed that the fact of badness is sufficient knowledge for mortal minds.
633-633: The marking ritual has been purged of unnecessary parameters!The call to
mark_bad_tipsetnow passes only the accursed tipset, as the method signature has been simplified by the Great Refactoring. This consistency pleases the Elder Gods.
642-642: Another invocation of the simplified marking ritual!The epoch-based rejection also calls the streamlined
mark_bad_tipsetmethod, maintaining consistency across the cosmic horror of chain validation.
689-689: The method signature has been cleansed of verbose parameters!The
mark_bad_tipsetmethod now accepts only the tipset to be cast into darkness, no longer requiring explanatory strings. This simplification aligns with the memory optimization decree.
697-697: The cache corruption ritual speaks only in CIDs!The
putmethod is invoked with only the block CID, as the Ancient Ones have banished reason strings from the forbidden cache. This completes the cosmic simplification.
758-758: The event handling follows the new cosmic order!The
BadTipsetevent processing calls the simplifiedmark_bad_tipsetmethod, maintaining consistency with the Great Refactoring that removed reason strings from the sync system.
874-874: The validation failure creates purified corruption events!The
BadTipsetevent is now created with only the tipset itself, no longer carrying the verbose explanations of validation failure. This aligns with the cosmic restructuring that banished reason strings to the outer darkness.
Summary of changes
Changes introduced in this pull request:
reasonfrom the bad block cache, which isn't used meaningfully anyway. If there are issues, we always have logs. After this change, Forest mainnet seems to sit steadily at ~4GB.My message on Slack:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
Bug Fixes
Refactor