fix(katana): ensure the messaging is not panicking when started in sovereign - #3107
Conversation
glihm
changed the title
WalkthroughOhayo, sensei! This PR adds support for a new messaging configuration for Sovereign chains. The changes update the configuration in the messaging module by introducing a new constant, a variant in the messenger mode enum, and an associated struct. The control flow is modified in both the configuration and the messaging service: instead of panicking, the system now creates a configuration for sovereign chains and logs appropriate warnings, particularly when a messaging interval is zero. Changes
Sequence Diagram(s)sequenceDiagram
participant CS as ChainSpec
participant MC as MessagingConfig
participant MM as MessengerMode
participant MS as MessagingService
CS->>MC: from_chain_spec(Sovereign chain)
MC->>MM: Initialize using CONFIG_CHAIN_SOVEREIGN with interval 60s
MM->>MS: Handle via from_config (logs unavailability)
MS->>MS: gather_messages detects Sovereign mode, returns (0, 0)
Possibly related PRs
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/katana/messaging/src/lib.rs (1)
185-186: Simple and sufficient struct definitionThe SovereignMessaging struct is appropriately minimal since no actual messaging occurs in sovereign mode. Consider adding a doc comment to explain its purpose.
#[derive(Debug)] -pub struct SovereignMessaging {} +/// Represents messaging for sovereign chains (no actual messaging occurs) +pub struct SovereignMessaging {}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/katana/messaging/src/lib.rs(4 hunks)crates/katana/messaging/src/service.rs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (7)
crates/katana/messaging/src/service.rs (3)
12-12: Ohayo! Good addition of warn level loggingYou've added the
warnimport from the tracing crate, which is used later for the interval warning. This is a positive change aligned with good logging practices.
101-101: Elegant handling of Sovereign mode, sensei!This new match arm for
MessengerMode::Sovereign(_)returns(0, 0)indicating no messages gathered, which aligns perfectly with the PR's objective to prevent panics in sovereign mode.
158-164: Nice defensive programming for interval handling!You've added protection against zero intervals by defaulting to 1 second with a warning message. This prevents potential issues with timer behavior and keeps the messaging service running smoothly.
crates/katana/messaging/src/lib.rs (4)
62-62: Ohayo! Good constant naming conventionAdding the
CONFIG_CHAIN_SOVEREIGNconstant follows the established pattern for other chain configurations. This maintains consistency in the codebase.
140-146: Well-structured Sovereign chain configuration, sensei!The new match arm for the Sovereign settlement layer creates an appropriate configuration with sensible defaults. Setting the interval to 60 seconds is reasonable since messaging is not actively used in sovereign mode.
182-182: Clean enum extensionAdding the Sovereign variant to MessengerMode enum is a clean way to represent the new mode. This maintains the pattern used for other messenger modes.
213-216: Clear logging and handling for Sovereign modeGood job providing clear logging that messaging is not available in Sovereign mode. This properly informs users about the expected behavior instead of causing a panic.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3107 +/- ##
==========================================
- Coverage 57.14% 57.08% -0.07%
==========================================
Files 442 442
Lines 61148 61199 +51
==========================================
- Hits 34946 34938 -8
- Misses 26202 26261 +59 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Messaging doesn't make sense in the sovereign mode at the moment. However we need this service to start in order to have Katana running as expected.
Summary by CodeRabbit