-
Notifications
You must be signed in to change notification settings - Fork 194
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
feat(katana): configurable max gas for rpc call #3043
Conversation
Ohayo, sensei! Here’s the detailed breakdown of the changes: WalkthroughThe pull request adds a new configuration option for setting gas limits on RPC calls. A Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 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 (1)
🪧 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
|
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
🧹 Nitpick comments (2)
crates/katana/executor/src/implementation/blockifier/mod.rs (2)
47-47
: Ohayo! Let's add some documentation here, sensei!Consider adding documentation for the
max_call_gas
field to explain its purpose and constraints.+ /// Maximum gas limit for contract calls. + /// This value is used to limit the gas consumption of the `starknet_call` RPC method. max_call_gas: u64,
53-54
: Consider extracting the default value as a constant, sensei!The default value of 1_000_000_000 should be extracted as a named constant for better maintainability.
+ /// Default maximum gas limit for contract calls. + const DEFAULT_MAX_CALL_GAS: u64 = 1_000_000_000; + pub fn new(cfg: CfgEnv, flags: ExecutionFlags, limits: BlockLimits) -> Self { - Self { cfg, flags, limits, max_call_gas: 1_000_000_000 } + Self { cfg, flags, limits, max_call_gas: Self::DEFAULT_MAX_CALL_GAS } } + /// Sets the maximum gas limit for contract calls. pub fn set_max_call_gas(&mut self, max_call_gas: u64) { self.max_call_gas = max_call_gas; }Also applies to: 56-58
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
crates/katana/cli/src/options.rs
(4 hunks)crates/katana/executor/src/implementation/blockifier/mod.rs
(6 hunks)crates/katana/node/src/config/rpc.rs
(3 hunks)crates/katana/node/src/lib.rs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/katana/node/src/config/rpc.rs
🔇 Additional comments (2)
crates/katana/node/src/lib.rs (1)
185-194
: Ohayo! Clean implementation of configurable max gas, sensei!The executor factory initialization properly handles the optional max_call_gas configuration, maintaining clean error handling and consistent configuration patterns.
crates/katana/cli/src/options.rs (1)
133-137
: Ohayo! Well-structured CLI option implementation, sensei!The max_call_gas option is properly integrated with clear documentation, consistent with the existing CLI patterns, and correctly uses feature flags.
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.
I took the liberty to change how the limit is passed to the executor. Other than that, looks good overall.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3043 +/- ##
==========================================
+ Coverage 56.19% 57.69% +1.49%
==========================================
Files 437 437
Lines 58821 59231 +410
==========================================
+ Hits 33057 34173 +1116
+ Misses 25764 25058 -706 ☔ View full report in Codecov by Sentry. |
#2969
Summary by CodeRabbit
New Features
Refactor