feat: remove starknet-messaging feature
#3005
Conversation
|
Ohayo, sensei! Here’s the breakdown of the changes: WalkthroughThis pull request removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MessagingService
participant EthHandler as "Ethereum Handler"
Client->>MessagingService: Send message
MessagingService->>EthHandler: Process Ethereum message
EthHandler-->>MessagingService: Return acknowledgement
MessagingService-->>Client: Relay acknowledgement
Possibly related PRs
Suggested reviewers
🪧 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/core/src/service/messaging/mod.rs (1)
167-171: Consider adding debug logs for initialization, sensei!While the error handling is good, adding debug logs during initialization could help with troubleshooting.
match StarknetMessaging::new(config).await { Ok(m_sn) => { info!(target: LOG_TARGET, "Messaging enabled [Starknet]."); + debug!(target: LOG_TARGET, ?config, "Starknet messaging initialized with config"); Ok(MessengerMode::Starknet(m_sn)) } Err(e) => { error!(target: LOG_TARGET, error = %e, "Starknet messenger init."); + debug!(target: LOG_TARGET, ?config, "Failed to initialize Starknet messaging with config"); Err(Error::InitError) } }Also applies to: 187-196
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
bin/katana/Cargo.toml(0 hunks)crates/katana/cli/Cargo.toml(0 hunks)crates/katana/contracts/messaging/README.md(1 hunks)crates/katana/core/Cargo.toml(1 hunks)crates/katana/core/src/service/messaging/mod.rs(1 hunks)crates/katana/core/src/service/messaging/service.rs(0 hunks)crates/katana/node/Cargo.toml(0 hunks)
💤 Files with no reviewable changes (4)
- crates/katana/cli/Cargo.toml
- crates/katana/core/src/service/messaging/service.rs
- bin/katana/Cargo.toml
- crates/katana/node/Cargo.toml
✅ Files skipped from review due to trivial changes (1)
- crates/katana/contracts/messaging/README.md
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: fmt
🔇 Additional comments (4)
crates/katana/core/src/service/messaging/mod.rs (2)
20-21: Ohayo! Documentation update looks good, sensei!The updated documentation accurately reflects that Starknet messaging is now a built-in feature rather than optional.
36-37: Verify module dependencies after feature flag removalThe removal of feature flags means these modules are now always included. Let's verify there are no missing dependencies.
Also applies to: 57-57, 60-61
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Check for any remaining feature flags related to starknet messaging rg -l '#\[cfg\(feature\s*=\s*"starknet-messaging"\)\]' . # Check for any potential circular dependencies cargo tree -e features katana-coreLength of output: 191
Ohayo, sensei! We verified that there are no remaining feature-flag usages for "starknet-messaging" (the regex search returned no hits). However, our circular dependency check with
cargo tree -e features katana-coredidn't run because the cargo command wasn’t available in our sandbox. Could you please run that check manually in your local environment to confirm no circular dependencies exist?crates/katana/core/Cargo.toml (2)
41-41: Ohayo! Dependency update looks good, sensei!The
starknet-cryptodependency is now correctly set as a required workspace dependency, which aligns with Starknet messaging being a built-in feature.
1-62: Verify dependency versions across workspaceLet's ensure all related crates have consistent dependency versions after the feature removal.
❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Check for any inconsistent starknet-related dependency versions across workspace cargo tree -e features | grep -i starknet # Verify no orphaned dependencies cargo tree --no-dev-dependencies -dLength of output: 273
Ohayo, sensei! It looks like the automated dependency checks couldn’t run since the cargo command wasn’t available in the current environment. That means we can’t automatically verify the dependency version consistency across the workspace. I’d recommend manually running the cargo commands locally to check for any inconsistent dependency versions or orphaned dependencies. Thanks for your hard work on this, sensei!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3005 +/- ##
==========================================
+ Coverage 56.99% 57.00% +0.01%
==========================================
Files 431 431
Lines 57151 57151
==========================================
+ Hits 32572 32580 +8
+ Misses 24579 24571 -8 ☔ View full report in Codecov by Sentry. |
starknet-messagingfeature was added as an initial work to explore appchains on katana.Now that appchains are unlocked, this feature is by default included in Katana.
Summary by CodeRabbit