-
Notifications
You must be signed in to change notification settings - Fork 184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hotfix(katana): include missing udc class if custom genesis is specified #2575
Conversation
WalkthroughOhayo, sensei! This pull request introduces significant modifications to the Changes
Possibly related PRs
🪧 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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2575 +/- ##
=======================================
Coverage 69.58% 69.59%
=======================================
Files 401 401
Lines 50745 50760 +15
=======================================
+ Hits 35312 35327 +15
Misses 15433 15433 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
crates/katana/primitives/src/chain_spec.rs (1)
219-237
: Ohayo, sensei! Consider adding error handling for entry operations.The new helper function looks good, but we should consider handling potential errors from the
entry
operations, even though they're unlikely to fail in this context.Consider adding error handling:
fn add_default_udc(states: &mut StateUpdatesWithDeclaredClasses) { // declare UDC class - states + if let Err(e) = states .declared_compiled_classes .entry(DEFAULT_LEGACY_UDC_CLASS_HASH) - .or_insert_with(|| DEFAULT_LEGACY_UDC_CASM.clone()); + .or_insert_with(|| DEFAULT_LEGACY_UDC_CASM.clone()) { + log::warn!("Failed to insert UDC compiled class: {}", e); + }Also, let's add a docstring to explain the function's purpose:
+/// Adds the default Universal Deployer Contract (UDC) to the state updates. +/// This includes declaring the UDC class and deploying the UDC contract if not already present. fn add_default_udc(states: &mut StateUpdatesWithDeclaredClasses) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- crates/katana/primitives/src/chain_spec.rs (3 hunks)
🔇 Additional comments (3)
crates/katana/primitives/src/chain_spec.rs (3)
16-17
: LGTM! Constants are well-organized.The new UDC-related constants are properly integrated with existing constants, maintaining code organization.
134-134
: LGTM! Clean refactoring of UDC initialization.The UDC initialization has been cleanly extracted into a dedicated helper function, improving code organization.
219-237
: Verify UDC initialization in tests.The function's behavior should be verified when the UDC is already present in the state.
Summary by CodeRabbit
New Features
GenesisJson
structure by removing unused fields.Bug Fixes
Tests