feat(blockifier): use same versioned constant as snos
#2956
Conversation
WalkthroughOhayo, sensei! The pull request introduces a new constant Changes
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/executor/src/implementation/blockifier/utils.rs (1)
Line range hint
584-584: Consider addressing these TODOs, sensei.There are several TODOs that might need attention:
- Legacy class compiled hash hack needs review
- Missing source for ABI length
- Gas prices need correct values
Would you like me to help create issues to track these TODOs?
Also applies to: 673-673, 789-789
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/katana/executor/src/implementation/blockifier/utils.rs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: fmt
🔇 Additional comments (2)
crates/katana/executor/src/implementation/blockifier/utils.rs (2)
455-457: Heads up about potential invalid runs, sensei!The comment warns about potential invalid runs due to overridden values unknown to
snos. This is an important operational consideration.Let's check what values are being overridden:
✅ Verification successful
Ohayo! The warning is valid and important, sensei!
The comment correctly warns about a critical operational concern. The code overrides important execution parameters (recursion depth and step limits) that directly affect transaction processing. Since
snosruns independently without knowledge of these overridden values, it could lead to inconsistent behavior where transactions that pass in one environment might fail in another.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find where these values are used in snos # Search for max_recursion_depth, validate_max_n_steps, and invoke_tx_max_n_steps rg -A 5 'max_recursion_depth|validate_max_n_steps|invoke_tx_max_n_steps'Length of output: 4358
446-454: Ohayo, sensei! Important version pinning change looks good.The explicit version pinning to
V0_13_3and the detailed comments explaining the rationale are well done. This change ensures consistent fee calculations by matching the version used insnos.Let's verify the version compatibility with
snos:
kariy
changed the title
SNOSsnos
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2956 +/- ##
==========================================
+ Coverage 56.69% 56.71% +0.02%
==========================================
Files 420 420
Lines 55554 55557 +3
==========================================
+ Hits 31497 31511 +14
+ Misses 24057 24046 -11 ☔ View full report in Codecov by Sentry. |
snosis using an older version ofblockifierwhich only supports up to Starknet versionv0.13.3whilekatana's is up tov0.13.4. So, we pin the version onkatanato matchsnosto make sure the execution outcome is similar.The call to
VersionedConstants::latest_constants()will basically use the latest version iev0.13.4.